Split Login and Register, enable OIDC Registration.
This change introduces the ability to configure OIDC registration endpoints. In order to achieve this functionality Login and Register need to be split. The new functionality will do the following:
-
Add enduser registration endpoint to oidc_backends.xml
-
Expose them in /api/configuration
-
Add a galaxy configuration option to disable local accounts
-
Change the frontend to: a. Split Login / Registration into two buttons
b. In case only one OIDC provider is configured and local accounts are disabled, Login will redirect the user to the OIDC endpoint. In case local accounts are enabled the behavior of previous galaxy is retained.
c) In case local accounts are disabled and one OIDC provider with end user registration endpoint exists register will redirect the user straight to the registration of the OIDC provider. In case no OIDC provider with end user registration is configured and local accounts are disabled, or local registration is disabled in galaxy.yml, registration is not shown on the UI (as per current behavior) If more than one OIDC provider with an endpoint are defined, or local accounts are enabled the Registration page will show all options.
How to test the changes?
(Select all options that apply)
- [x] I've included appropriate automated tests.
- [x] This is a refactoring of components with existing test coverage.
- [x] Instructions for manual testing are as follows:
In order to test oidc registration, or disabling local accounts manual testing is still required. The best way is to test the following:
- disable local accounts, and configure one OIDC provider -> Only Login is visible on the UI, clicking login will directly point the user to the OIDC login.
- Enable local accounts and configure one OIDC provider -> Behavior as today.
- Enable local accounts and configure one OIDC provider with end user registration endpoint -> Registration page shows button <Register with ...>
- Disable local accounts and configure one OIDC provider with end user registration endpoint -> Clicking registration will redirect the user to the end user registration point.
License
- [x] I agree to license these and all my past contributions to the core galaxy codebase under the MIT license.
@dannon
Thanks for these changes @uwwint !!! I've made some minor styling/refactoring/linting/import ordering etc fixes, used Galaxy base components ...
Will try out all the use cases you've added next!
Hi @ahmedhamidawan do you need any help on setting up the Tests?
Hi @ahmedhamidawan do you need any help on setting up the Tests?
Hey @uwwint 👋 !
Feel free to add/adjust tests, and make changes on top of mine if needed!
Just make sure to lint the changes before you commit:
cd client && yarn run prettier --checkcd client && yarn run eslintmake format(for backend)tox -e mypy(for backend)
I think these are fine. Do you think we can merge?
I think these are fine. Do you think we can merge?
Hey @uwwint ! I'll get back to this now and resolve the conflicts, then this should be good to go
Update: I've rebased and cleaned up the commits. @uwwint it would be ideal if you can test this out locally to see if it looks good.
@marius-mather do you have capacity to give this a test - ? I can only get to it next week
Hi @ahmedhamidawan , I've checked this PR out locally and I've confirmed that I'm able to disable local accounts, add a single OIDC provider, and have the separate Login and Register buttons work as expected (going to the OIDC login, or the configured registration page). Everything's looking good to me
Several seleniums are failing, maybe at the login/registration step? I reran them but will investigate this further as well
Update: Yes, they were failing because we changed the singular Login or Register button to two Login and Register buttons. I've fixed the selectors for those.
Thanks @ahmedhamidawan ! the remaining test failures look unrelated now
Can this be merged now @nuwang ?
lgtm @ahmedhamidawan. You ok with merging this?
lgtm @ahmedhamidawan. You ok with merging this?
Hey @nuwang , @dannon wanted to take a look at this and provide some feedback
Thanks @uwwint and @ahmedhamidawan for the work on this!
I'm a bit concerned with the configuration changes here though. We're spreading auth config even more in galaxy.yml, when we've been talking about converging everything auth-related into a single place.
A few thoughts:
-
The allow_local_account_creation rename - This looks like it'll break existing configs. We should probably keep backward compatibility here.
-
The auto-redirect behavior - Having it magically redirect when there's only one OIDC provider feels a bit too implicit. Admins should be able to control this explicitly.
-
Split login/register buttons - This goes against how most sites handle auth these days. Usually there's just one "Sign in" button that figures out if you need to login or register.
I'm not against merging this since it solves a real problem, but what if we:
- Make sure old configs work identically (and then I think the changes to the 1607 config sample should not be required)
- Add an explicit
auto_redirectoption (maybe roll this into below) - Open a follow-up to consolidate everything into a single auth block in galaxy.yml
I'd love to see us move toward something like this eventually (rough, but kind of what I'm thinking)
auth:
require_login: false
allow_registration: true
registration_warning: "Please use institutional account if available"
providers:
- name: google
client_id: ${GOOGLE_CLIENT_ID}
client_secret: ${GOOGLE_CLIENT_SECRET}
- name: keycloak
url: https://auth.example.org/realms/galaxy
client_id: galaxy
client_secret: ${KEYCLOAK_SECRET}
auto_redirect: true
- name: galaxy
type: local
allow_creation: false
This would give us one clean place to understand all auth behavior, and we could revisit the unified login/register flow as part of that work. We could also then use this same config block/handling in parent config management, I guess? (auth block if provided, otherwise auth.yml or whatever)
What do you think? (again, not -1, but I think we need to make sure we're on track to clean the config matrix back up)