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

[24.1] prevent "missing refresh_token" errors by supporting <extra_scopes> also with Keycloak backend #18826

Merged
merged 3 commits into from
Sep 18, 2024

Conversation

ljocha
Copy link
Contributor

@ljocha ljocha commented Sep 17, 2024

The <extra_scopes> attribute of oidc_backend_config.xml is not supported in the Keycloak backend. Consequently, it was not possible to request 'offline_access' from the IdP, which is expected in the current implementation, and we kept receiving:

Sep 16 11:23:37 usegalaxy-test galaxyctl[11560]: Traceback (most recent call last):
Sep 16 11:23:37 usegalaxy-test galaxyctl[11560]: File "/srv/galaxy/server/lib/galaxy/authnz/managers.py", line 357, in refresh_expiring_oidc_tokens_for_provider
Sep 16 11:23:37 usegalaxy-test galaxyctl[11560]: refreshed = backend.refresh(trans, auth)
Sep 16 11:23:37 usegalaxy-test galaxyctl[11560]: File "/srv/galaxy/server/lib/galaxy/authnz/custos_authnz.py", line 140, in refresh
Sep 16 11:23:37 usegalaxy-test galaxyctl[11560]: token = oauth2_session.refresh_token(token_endpoint, **params)
Sep 16 11:23:37 usegalaxy-test galaxyctl[11560]: File "/srv/galaxy/venv/lib/python3.9/site-packages/requests_oauthlib/oauth2_session.py", line 496, in refresh_token
Sep 16 11:23:37 usegalaxy-test galaxyctl[11560]: self.token = self._client.parse_request_body_response(r.text, scope=self.scope)
Sep 16 11:23:37 usegalaxy-test galaxyctl[11560]: File "/srv/galaxy/venv/lib/python3.9/site-packages/oauthlib/oauth2/rfc6749/clients/base.py", line 427, in parse_request_body_response
Sep 16 11:23:37 usegalaxy-test galaxyctl[11560]: self.token = parse_token_response(body, scope=scope)
Sep 16 11:23:37 usegalaxy-test galaxyctl[11560]: File "/srv/galaxy/venv/lib/python3.9/site-packages/oauthlib/oauth2/rfc6749/parameters.py", line 441, in parse_token_response
Sep 16 11:23:37 usegalaxy-test galaxyctl[11560]: validate_token_parameters(params)
Sep 16 11:23:37 usegalaxy-test galaxyctl[11560]: File "/srv/galaxy/venv/lib/python3.9/site-packages/oauthlib/oauth2/rfc6749/parameters.py", line 448, in validate_token_parameters
Sep 16 11:23:37 usegalaxy-test galaxyctl[11560]: raise_from_error(params.get('error'), params)
Sep 16 11:23:37 usegalaxy-test galaxyctl[11560]: File "/srv/galaxy/venv/lib/python3.9/site-packages/oauthlib/oauth2/rfc6749/errors.py", line 399, in raise_from_error
Sep 16 11:23:37 usegalaxy-test galaxyctl[11560]: raise cls(**kwargs)
Sep 16 11:23:37 usegalaxy-test galaxyctl[11560]: oauthlib.oauth2.rfc6749.errors.InvalidClientIdError: (invalid_request) refresh_token parameter not provided

The patch is conservative, unless <extra_scopes> appears in the config, it does nothing.

@github-actions github-actions bot added the area/auth Authentication and authorization label Sep 17, 2024
@github-actions github-actions bot added this to the 24.1 milestone Sep 17, 2024
@@ -213,6 +213,8 @@ def _parse_custos_config(self, config_xml):
rtv["ca_bundle"] = config_xml.find("ca_bundle").text
if config_xml.find("icon") is not None:
rtv["icon"] = config_xml.find("icon").text
if config_xml.find("extra_scopes") is not None:
Copy link
Member

@mvdbeek mvdbeek Sep 17, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a downside to setting extra_scopes to an empty list ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Empty extra_scopes would do nothing. Anyway, I copied these two lines from _parse_idp_config() above ...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see, so the way you're dealing with the token refresh is to pass in the required extra scopes ?

@mvdbeek mvdbeek changed the title prevent "missing refresh_token" errors by supporting <extra_scopes> also with Keycloak backend [24.1] prevent "missing refresh_token" errors by supporting <extra_scopes> also with Keycloak backend Sep 17, 2024
@mvdbeek mvdbeek merged commit 07fa2e3 into galaxyproject:release_24.1 Sep 18, 2024
47 of 50 checks passed
@galaxyproject galaxyproject deleted a comment from github-actions bot Sep 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/auth Authentication and authorization kind/bug kind/enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants