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

Support OpenID Connect better in the OAuth2 'custom' provider. #6154

Closed

Conversation

ssssam
Copy link
Contributor

@ssssam ssssam commented Feb 4, 2025

The OpenID Connect standard specifies that sub contains the user ID. Until now, openQA only looked for a field named id.

When using Keycloak as identity provider there is no such field, it returns only sub as expected in OpenID Connect.

To avoid breaking any existing configs, this patch adds a new id_from config field which defaults to id, so existing behaviour is preserved. Set id_from = sub in the OAuth2 provider config to get the new behaviour.

@ssssam ssssam force-pushed the sam/fix-custom-oauth2-provider branch 2 times, most recently from 3033a91 to 5295acb Compare February 4, 2025 21:07
lib/OpenQA/Setup.pm Show resolved Hide resolved
@ssssam ssssam force-pushed the sam/fix-custom-oauth2-provider branch from 5295acb to 131b279 Compare February 5, 2025 16:45
The OpenID Connect standard specifies that `sub` contains the user ID.
Until now, openQA only looked for a field named `id`.

When using Keycloak as identity provider there is no such field, it
returns only `sub` as expected in OpenID Connect.

To avoid breaking any existing configs, this patch adds a new `id_from`
config field which defaults to `id`, so existing behaviour is preserved.
Set `id_from = sub` in the OAuth2 provider config to get the new
behaviour.

Fixes: os-autoinst#5771
@ssssam ssssam force-pushed the sam/fix-custom-oauth2-provider branch from 131b279 to 3eba657 Compare February 5, 2025 17:26
@Martchus
Copy link
Contributor

Martchus commented Feb 5, 2025

Looks like the Codecov check couldn't be triggered. The related CircleCI job ran for "0 s" and if one clicks on the job one gets only "Something Unexpected Happened". All buttons for re-running are grayed out and the workflow status is also "Error" (and not e.g. "Failed").

So I suppose the easiest way to get green checkmarks on the CI is to amend the commit and force-push again.

@ssssam ssssam force-pushed the sam/fix-custom-oauth2-provider branch from 3eba657 to 1115a51 Compare February 6, 2025 10:58
@Martchus
Copy link
Contributor

Martchus commented Feb 6, 2025

The CI checks failed (or rather "errored") in the same way as before despite the commit being based on a recent commit from master. I also tried to re-deliver the webhook to CircleCI but it didn't have any effect.

Maybe we can merge this forcefully considering all steps of the CircleCI pipeline worked except Codecov? Otherwise you would probably need to create a new PR.

@okurz
Copy link
Member

okurz commented Feb 6, 2025

I created

as a re-submit which might help to trigger the codecov report generation

@okurz
Copy link
Member

okurz commented Feb 6, 2025

Merged as part of

thank you for the contribution!

@okurz okurz closed this Feb 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants