Skip to content

Improved OIDC authentication #7445

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

H3xaChad
Copy link

@H3xaChad H3xaChad commented Nov 19, 2024

Improve OIDC Authentication

Summary:

  • Simplified Configuration: Now only requires wellKnownURL to auto-fetch OIDC endpoints.
  • Claim Merging: Added mergeIdTokenClaims option to merge claims from the ID token with the user profile.

Why:

  • Easier Setup: Reduces configuration complexity by leveraging OIDC discovery, also lowering the risk of misconfiguration
  • Accurate Group Mapping: I actually had massive issues with the group mapping, because with Azure Entra ID the 'roles' claim used to assign roles/groups is inside the ID token. But since the ID token is ignored it was impossible to get this working.

One dependency was added (jwks-rsa) since it is mandatory to verify the ID token

Hope you find some time to look into the PR give some feedback! :)

Screenshot_20241119_183223

…configuration and added the option to merge claims from the id token
@auto-assign auto-assign bot requested a review from NGPixel November 19, 2024 17:36
@NGPixel
Copy link
Member

NGPixel commented Nov 22, 2024

What if the oidc server doesn't have a .well-known endpoint? It should still allows to specify the endpoints manually.

@H3xaChad
Copy link
Author

H3xaChad commented Feb 26, 2025

I actually wasn't fully aware that the discovery endpoint is optional for OIDC.
For representing this in the settings, my thought would be to add the well-known URL as an additional parameter while keeping all others. We could include a note stating that the URL is sufficient if it exists, and that if other properties (e.g., token URL) are additionally specified, they will override the values from the well-known URL.
@NGPixel Do you have any further suggestions?

@NGPixel
Copy link
Member

NGPixel commented Feb 28, 2025

I actually wasn't fully aware that the discovery endpoint is optional for OIDC. For representing this in the settings, my thought would be to add the well-known URL as an additional parameter while keeping all others. We could include a note stating that the URL is sufficient if it exists, and that if other properties (e.g., token URL) are additionally specified, they will override the values from the well-known URL. @NGPixel Do you have any further suggestions?

That would work for me.

@MaBauMeBad
Copy link

@H3xaChad Fist of all, thank you for your PR and the work behind it.
We are also have need for this.
Do you need any support to make the required adjustments ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants