Support for OIDC API Auth and OIDC integration tests #16977
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
github-actions
bot
added
area/testing
area/API
area/testing/integration
area/auth
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
nuwang
changed the title
nsoranzo
reviewed
Nov 3, 2023
nuwang
force-pushed
the
oidc_api_auth
branch
3 times, most recently
from
4ed3131 to
273266d
Compare
nuwang
force-pushed
the
oidc_api_auth
branch
2 times, most recently
from
eaade6f to
28a015b
Compare
nuwang
changed the title
4 tasks
|
Please Merge, this will help us a lot. |
mvdbeek
reviewed
Dec 18, 2023
| @@ -295,6 +295,13 @@ def by_api_key(self, api_key: str, sa_session=None): | |||
| raise exceptions.AuthenticationFailed("Provided API key has expired.") | |||
| return provided_key.user | |||
|
|
|||
| def by_oidc_access_token(self, access_token: str): | |||
| if hasattr(self.app, "authnz_manager") and self.app.authnz_manager: | |||
There was a problem hiding this comment.
It might be more explicit to find which type of app we need (is it UniverseApplication) with an isinstance check ?
There was a problem hiding this comment.
Yeah, it's not great, I've simply followed the pattern used elsewhere. This is because authnz_manager is assigned conditionally even in the UniverseApplication constructor and is undefined by default. This is something that should probably be handled during the proposed refactoring run.
|
Thanks for reviewing @mvdbeek. Have fixed the conflict. |
bgruening
reviewed
Dec 19, 2023
| # Sets the prefix for OIDC scopes specific to this Galaxy instance. | ||
| # If an API call is made against this Galaxy instance using an OIDC bearer token, | ||
| # it must include a scope with "<prefix>:*". e.g "https://galaxyproject.org/api:*" | ||
| #oidc_scope_prefix: https://galaxyproject.org/api |
There was a problem hiding this comment.
The config setting should specify only the prefix. The * is the exact scope that's requested, meaning all permissions. That leaves space for future expansion, like perhaps <prefix>:datasets to request access to datasets etc. For now, we only support <prefix>:*
There was a problem hiding this comment.
Should then e.g "https://galaxyproject.org/api" be changed to be clear?
There was a problem hiding this comment.
I've expanded the description a bit. Let me know whether this looks better.
bgruening
approved these changes
Dec 20, 2023
|
Thanks all for the comments and feedback. |
|
Youhou, it seems also very usefull for our french earth-system/biodiversity/climate Gaia Data project, where we will deploy a keycloack solution who can benefit from this if I am not wrong |
|
Hmm, I've been looking into adding tests for #17516, but the tests don't look clean: It's unclear if that's just a consequence of not having passed proper credentials to the test interactor, or something's really broken. |
|
If you're testing PSA, a probably cause for this is that galaxy/lib/galaxy/authnz/__init__.py Line 91 in 566731b |
|
No, that's just the literal test added here: |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR adds support for the following:
HTTPBearer is displayed as an auth option in the swagger UI.
How to test the changes?
(Select all options that apply)
License