-
Notifications
You must be signed in to change notification settings - Fork 17
Authorization migration #3791
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
base: main
Are you sure you want to change the base?
Authorization migration #3791
Conversation
This class holds information about the current user and his or her access rights. An instance is created during authentication. Signed-off-by: Oliver Heger <[email protected]>
Add a new property to `EffectiveRole` that holds the superuser status, so that this can be checked efficiently. Extend `DbAuthorizationService` to set this flag correctly. Add more test cases for role assignments on the superuser level. Signed-off-by: Oliver Heger <[email protected]>
This interface defines a generic mechanism to perform checks for required permissions when handling requests. It is going to be used in an extended DSL to define routes that can handle such checks automatically. Signed-off-by: Oliver Heger <[email protected]>
Extend the Ktor DSL for route definitions by functions that perform an automatic check for permissions based on provided `AuthorizationChecker` objects. This enables a declarative approach for permission checks. Provide a concrete `AuthorizationChecker` implementation for organization permissions. Add a test class to test this approach. Note that currently failed permission checks yield a return status code of 401 rather than 403. This is going to be changed in an upcoming commit. Signed-off-by: Oliver Heger <[email protected]>
If a request fails because of missing permissions, return the correct status code 403. Add a new `AuthorizationException` that is thrown in this case and can be mapped to the corresponding status code. Signed-off-by: Oliver Heger <[email protected]>
Add support for permissions on product and repository level and superuser checks. Signed-off-by: Oliver Heger <[email protected]>
c81aa3a to
0c77c5d
Compare
Extend `KeycloakAuthorizationService` by a function that can create structures in the database that correspond to the roles and permissions currently kept in Keycloak. The function is going to be called on startup of ORT Server to perform a one-time migration. Signed-off-by: Oliver Heger <[email protected]>
Instead of doing a synchronization of roles in Keycloak, trigger a migration to new access rights structures on startup of the core component. Signed-off-by: Oliver Heger <[email protected]>
0c77c5d to
7fe4de2
Compare
|
Just curious, after the migration, will there be another follow-up PR that removes old code? |
For the migration, the old code is still required. I assume, we need to keep this for a while, since users might not immediately switch to a new version and should still be able to migrate later. But after a grace period, the whole There should also be less functionality for the interaction with Keycloak required. So, the Keycloak client could be stripped down (or replaced by the official one?). |
This PR implements a one-time migration to the new authorization component. The migration is triggered once on ORT Server startup. It creates entries in the database corresponding to the roles and permissions stored in Keycloak.
This should not be merged before all permission checks have been migrated to the new authorization component.