-
Notifications
You must be signed in to change notification settings - Fork 37
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
service: Add admin user concept and app definition scaling endpoint #400
Conversation
- Introduce the `@AdminOnly` annotation to mark REST resources or methods as accessible only to admin users. - Implement AdminOnlyFilter that intercepts requests and aborts non-admin users with a 403 Forbidden response. - Update ApplicationProperties to include a configurable admin group name property ("theia.cloud.auth.admin.group") with a default value of "theia-cloud/admin". - Enhance TheiaCloudUser by adding an admin flag. - Modify TheiaCloudUserProducer to derive the admin status from the MicroProfile JWT's groups claim. - Add tests for the new admin-only filter, properties, and user producer functionality. - Extend service Dockerfile to allow configuring the admin group name via environment variable.
- Add new resource AdminAppDefinitionAdminResource for all admin endpoints regarding app definitions - Minor extensions in K8SUtil, AppDefinitionSpec to allow editing min/max instances - Add tests for AdminAppDefintionAdminResource - Add RootAdminResource with a ping endpoint that only returns if the user is an admin
Extend Keycloak terraform module: - Define an admin group with the default name - Export realm, admin group and test users via outputs Test setup: - Add test user foo to the admin group
This is required for the Quarkus dev server to discover the OIDC config. Also see the official documentation: https://quarkus.io/guides/security-oidc-configuration-properties-reference#quarkus-oidc_quarkus-oidc-auth-server-url
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, looks great! I just have two comments about log messages.
Also, could you include this part of the testing instructions in a README?
curl -s --insecure --request POST \
--url https://$(minikube ip).nip.io/keycloak/realms/TheiaCloud/protocol/openid-connect/token \
--header 'content-type: application/x-www-form-urlencoded' \
--data grant_type=password \
--data client_id=theia-cloud \
--data username=foo \
--data password=foo | jq .access_token
I think it would be useful to have this easily accessible.
Besides that, we need to regenerate the openapi.json and probably also regenerate the common npm library + markdown documentation.
...lipse.theia.cloud.service/src/main/java/org/eclipse/theia/cloud/service/AdminOnlyFilter.java
Outdated
Show resolved
Hide resolved
...ain/java/org/eclipse/theia/cloud/service/admin/appdefinition/AppDefinitionUpdateRequest.java
Show resolved
Hide resolved
Hi @jfaltermeier , thanks for the review. I addressed all issues mentioned in your review :) |
Summary
This PR adds the concept of admin users to the Theia Cloud REST service. They are identified by a configurable admin group name in MicroProfile JWT's groups claim.
Admin only endpoints and resources can be configured via new annotation
@AdminOnly
. This is used for a new endpoint that allows updating an app definition's min and max instances. In addition an admin ping endpoint was added.The terraform Keycloak module was extended to configure the default admin group and export realm, group, and test user infos for further usage in consuming modules.
A corresponding PR for the Helm chart allows configuring the admin group name as a value: eclipse-theia/theia-cloud-helm#87
How to test
keycloak.adminGroup
on the theia-cloud Helm chart.foo
which is configured to be an admin user by the terraform test setuphttps://$(minikube ip).nip.io/service/service/admin/asdfghjkl
should return true with a token for user foo. It should return403
for user bar because bar is not an admin user$(minikube ip).nip.io/service/service/admin/appdefinition/<appdef-name>
. The payload is a JSON object like so:Implementation details
Implementation details
Add admin user support via annotation
@AdminOnly
annotation to mark REST resources or methods as accessible only to admin users.with a default value of "theia-cloud/admin".
Add admin endpoint to update app def's min/max instances
terrafom: Add admin user group and outputs for Keycloak module
Extend Keycloak terraform module:
Test setup:
fix: Update Keycloak URL in tasks.json to include realm path
This is required for the Quarkus dev server to discover the OIDC config.
Also see the official documentation: https://quarkus.io/guides/security-oidc-configuration-properties-reference#quarkus-oidc_quarkus-oidc-auth-server-url