Fix anonymous auth, login error handling, and data for CI tests#2498
Fix anonymous auth, login error handling, and data for CI tests#2498chrisknoll merged 10 commits intoupgrade/spring-security-authzfrom
Conversation
src/main/java/org/ohdsi/webapi/security/authc/JwtAuthConfig.java
Outdated
Show resolved
Hide resolved
src/main/java/org/ohdsi/webapi/security/spring/SpringSecurityConfig.java
Show resolved
Hide resolved
| "SELECT nextval('public.sec_user_role_sequence'), -1, 2, 'SYSTEM' " + | ||
| "WHERE NOT EXISTS (SELECT 1 FROM public.sec_user_role WHERE user_id = -1 AND role_id = 2)"); | ||
|
|
||
| // Generate a JWT for the anonymous user so HTTP requests are authenticated |
There was a problem hiding this comment.
This is good that you're handling the test case, thanks for updating this.
I don't think we need to make an annonymous JWT token because you will assume an anonymous identity when you connect to the webAPI service. However, it is definitely the case where we would need to insert permissions for the anonymous user into the DB so it has 'full permissions' to do all the tests. Not sure if we need to do that on a per-test basis or if we should set up the test DB once for the entire test db and have all anonymous test examples run with full permissions. I don't know the right path here because we may want to run certain tests based on certain user permissions, but in that case I'd say we'd set up a series of test users with their associated permissions (and in that case, you would need to mint a JWT). So maybe for this test, you should register a new user for your test, grant it permisisons/roles and then run the test. That way we aren't overloading the notion of an 'anonymous user' as 'someone who logs in anonymously.
|
Also, is this PR intended to go into webapi-3.0 or is it to merge into my authz-update branch? |
| -- Anonymous user (required for public endpoints) | ||
| INSERT INTO webapi.sec_user (id, login, name) VALUES (1, 'anonymous', 'anonymous') ON CONFLICT (id) DO NOTHING; | ||
| INSERT INTO webapi.sec_user_role (id, user_id, role_id, origin) VALUES (1, 1, 1, 'SYSTEM') ON CONFLICT (id) DO NOTHING; | ||
| INSERT INTO webapi.sec_user (id, login, name) VALUES (1, 'anonymous', 'anonymous') ON CONFLICT DO NOTHING; |
There was a problem hiding this comment.
Looking further on this, isn't this already handled in the baseline migration found here:
Also, the anonymous id is hard-coded to -1 (to avoid the conflict from the 2.x user IDs in case they exist). Here's the part of the baseline flyway:
-- Anonymous User and anonymous role:
INSERT INTO ${ohdsiSchema}.sec_user (id, login, name, origin)
VALUES (-1, 'anonymous', 'Anonymous', 'SYSTEM');
INSERT INTO ${ohdsiSchema}.sec_role (id, name, system_role)
VALUES (-1, 'anonymous', false);
INSERT INTO ${ohdsiSchema}.sec_user_role (id, user_id, role_id, origin)
VALUES (nextval('${ohdsiSchema}.sec_user_role_sequence'), -1, -1, 'SYSTEM');
This pull request updates the test and integration Docker environments and related SQL setup scripts to migrate from the legacy JDBC authentication configuration to a new database authentication mechanism. It also improves test coverage for JWT authentication and role/permission assignments, and updates Postman integration tests to match new API response formats.