-
Notifications
You must be signed in to change notification settings - Fork 0
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
feat: Authorization Code Flow with PKCE #92
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
trym-b
reviewed
Dec 20, 2023
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.
Nicely logically divded commits
maeb
force-pushed
the
feat/pkce
branch
2 times, most recently
from
January 3, 2024 12:43
df75d0b
to
b256e5a
Compare
@trym-b ready for next round. |
trym-b
approved these changes
Jan 3, 2024
This commit removes a scope that is not defined by the OpenID Connect specification and DOES NOT WORK with the Keycloak authorization server. The specification states that scope values used that are not understood by an application should be IGNORED, but apparently not all authorization servers adheres to the spec. Ref: https://openid.net/specs/openid-connect-basic-1_0.html#Scopes
This commit is motivated by code readability.
This commit implements Proof Key for Code Exchange (PKCE) for the Authorization Code Flow. See https://oauth.net/2/pkce/ and https://tools.ietf.org/html/rfc7636. PKCE ensures that the client application exchanging the authorization code is the same client application that requested it and that the authorization code was not stolen and injected in a different session.
A client secret allows the authorization server to determine the identity of the client. This is recommended for confidential clients and PKCE is an additional layer of security. Since this application is considered a public client it does not make sense to use a default client secret that is hardcoded in the application. The reason it was there in the first place was lack of knowledge.
This commit does not store the client secret in the configuration file if it is empty when logging in. This is the case in a normal login scenario when using the OpenID Connect Authorization Code Flow with PKCE. As stated in the previous commit this application is considered a public client and does not require the user to keep the client secret confidential. It is still possible to manually set the client secret field in the configuration file, but it will be removed when logging out.
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 implements Authorization Code Flow with Proof Key for Code Exchange (PKCE).
https://oauth.net/2/pkce/
This PR also seeks to standardise the OIDC login flow such that specifics about a particular authorization server implementation is removed.