Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merging in U of Chicago changes Spring 2024 #6

Merged
merged 12 commits into from
Jun 11, 2024

Conversation

rkboyce
Copy link
Collaborator

@rkboyce rkboyce commented Jun 11, 2024

These changes are important for bringing in global artifacts sharing

Gennadiy Anisimov and others added 12 commits May 23, 2024 15:02
* fix: reinstate tests

* tmp: skip broken CohortCharacterizationServiceTest tests

This allows the other tests to be reinstated while these two
can then be fixed separately.

* tmp: skip broken PermissionTest tests

This allows the other tests to be reinstated while these two
can then be fixed separately.

* tmp: skip broken StudyInfoTest

...this one seems to only work depending on the order of execution...
it lacks the setup() method where a pre-filled db is guaranteed, like
for example in CohortCharacterizationServiceTest

* fix: mark abstract class as abstract
feat: add CTDS CI build and push

Co-authored-by: Andrew Prokhorenkov <[email protected]>
- feat: introduce custom configuration option
  Update pom.xml with a better default authorization url
- feat: improve logging of jwt
- fix: add "Atlas users" as default system role
- feat: add more log statements for PermissionManager
- feat: ensure /user/me endpoint also triggers the UPDATE_TOKEN filter
- feat: ensure the teamproject is stored per user
   ...and allow reading current teamproject from cache in case of a
   request to /user/refresh endpoint
- feat: main logic in new filter class TeamProjectBasedAuthorizingFilter
- fix: ensure reset of roles always happens
- feat: remove unnecessary method from PermissionManager
- fix: use lower() in SQL query itself for finding login
- fix: take login from shiro-parsed principal instead of DB
  ... to avoid the issue where the login is all lowercase in db
- feat: move the defaultRoles definition into AtlasSecurity
- fix: move authorizationMode check to PostConstruct
  ...to avoid NullPointerException as attributes
  are not yet wired when in constructor

- fix: remove session.stop() call from UpdateAccessTokenFilter
...and therefore from the flow of endpoints like /user/refresh.
Not sure why this was added there, as the /user/logout should be
the place to remove a session.
This solves a org.apache.shiro.subject.support.DisabledSessionException.
If the worry is that logout won`t be called, then the
expiry time should just be set to a short period.
The adjustment in JwtAuthRealm.java was to deal with the side
effect that occurred after the removal of the .stop in
the UpdateAccessTokenFilter filter:
java.lang.ClassCastException: io.buji.pac4j.subject.Pac4jPrincipal cannot be cast to java.lang.String

- fix: do not create a new session when requesting current session
…_permission

Update src/main/resources/db/migration/postgresql/V2.15.0.20240515220400_atlas_global_share_permission.sql
i.e. also add the "Source user (omop)" role to the list of defaultRoles for each user.
TODO - replace with final solution from https://ctds-planx.atlassian.net/browse/VADC-1086
@rkboyce rkboyce merged commit 4ba1e93 into vinci-ohdsi:master Jun 11, 2024
1 of 2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants