-
Notifications
You must be signed in to change notification settings - Fork 169
feat: add OpenID Connect (OIDC) provider #444
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
base: main
Are you sure you want to change the base?
Conversation
src/runtime/server/lib/oauth/oidc.ts
Outdated
| client_secret: config.clientSecret, | ||
| redirect_uri: redirectURL, | ||
| code: query.code, | ||
| code_verifier: verifier?.code_verifier, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this part does not seem to omit itself when a clientSecret is provided, resulting in a 400 Bad Request error when fetching the tokens.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In line 269 the verifier is set to undefined if a client secret is passed so its omitted in this case.
For me using a client secret works as expected 🤔 Which OIDC provider are you using?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
using a client_secret works indeed, however I need to comment out line 300 even though the verifier returns undefined. I'm using the Surfconext OIDC provider and that one returns with a 'bad request' if the code_verifier is submitted as undefined in the request. Commenting out the line solved the issue.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for testing! I updated the implementation to only include the PKCE query parameters if the client ID is unset.
Would you mind testing it again?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Works as expected now!
9729859 to
909a33b
Compare
|
Hi @larsrickert ! Great work. |
That's already implemented :) |
Thanks for the quick reply! Could you point me to where this is implemented? I'm not seeing it in the PR code. Currently, it seems we only retrieve user info via the userinfo endpoint (which is optional), and the OidcTokens interface doesn't contain the idToken. Maybe I'm missing something? |
Maybe I misunderstood? As far a I saw during testing the user data is decoded in this server route which is used when redirecting from the oidc provider back to the Frontend. playground/server/routes/auth/oidc.ts |
Yes the user data is retrieved by sending the access token to the userinfo endpoint of the OIDC provider (thats already implemented). There is also a second way how the user data could be retrieved (e.g. because some providers do not offer a userinfo endpoint) and thats by requesting an idtoken (JWT with user data encoded in it) that is then validated and decoded. I'd suggest to aleady merge the OIDC provider as it is so its not blocked by another optional feature :) We can then extend it if needed in follow up PRs |
closes #25
closes #89
This PR adds support for a generic OpenID connect (OIDC) provider that can be used with any provider that supports the OIDC standard.
It supports the 'code' response type and grant type 'authorization_code'. If a client secret is provided, it will be used to fetch the token. Otherwise the PKCE flow will be used where no client secret is needed.
Since the existing PR #25 has not seen progress in almost two years, this PR is intended to replace #25.
Example usage