-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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 POST for authorization code request flow #1874
base: main
Are you sure you want to change the base?
Support POST for authorization code request flow #1874
Conversation
The removal of the oidc scope matcher now allow OAuth2AuthorizationEndpointFilter to send POST /authorize request without the openid scope to the converters. But I see two issues with this development: 1- The default converter Tackling this issue seems a bit out of scope for this PR so I didn't touch anything, and it can be surely be solved with some custom converters. 2- Another minor issue I see is that the oauth2 specification is saying that a /authorize request (GET or POST) is supposed to have the parameters in the query string,
contradicting the OIDC spec saying that POST requests must use the body
So this way of getting the request parameters may have to be updated, but again it can be solved with a custom converter, so it also felt out of scope of this PR. Considering these 2 issues, I am not sure the PR is answering the "Support POST for authorization code request flow", and I would like your opinion on this |
Rewording my thoughts, I don't think these issues are completely out of scope, but they need significant refactor with more regression risks than the small change here. |
String scope = request.getParameter(OAuth2ParameterNames.SCOPE); | ||
return StringUtils.hasText(scope) && scope.contains(OidcScopes.OPENID); | ||
}; | ||
RequestMatcher responseTypeParameterMatcher = ( |
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.
responseTypeParameterMatcher
should not be removed as it will break the Authorization Consent flow. Only remove openidScopeMatcher
here as well as in OAuth2AuthorizationCodeRequestAuthenticationConverter
Closes gh-1811