Fix token endpoint filter for authorization code flow #120 #121
Conversation
|
@vincent-hsin Please sign the Contributor License Agreement! Click here to manually synchronize the status of this Pull Request. See the FAQ for frequently asked questions. |
|
@vincent-hsin Thank you for signing the Contributor License Agreement! |
| } | ||
| clientPrincipal = SecurityContextHolder.getContext().getAuthentication(); |
There was a problem hiding this comment.
Moving this line of code does not change any behaviour from the original. As an FYI, if you feel this is a bug, a test must be provided to demonstrate the bug.
vincent-hsin
changed the title
| @@ -257,6 +257,57 @@ public void doFilterWhenAuthorizationCodeTokenRequestValidThenAccessTokenRespons | |||
| assertThat(accessTokenResult.getScopes()).isEqualTo(accessToken.getScopes()); | |||
| } | |||
|
|
|||
| @Test | |||
| MockHttpServletRequest request = new MockHttpServletRequest("POST", requestUri); | ||
| request.setServletPath(requestUri); | ||
|
|
||
| request.addParameter(OAuth2ParameterNames.GRANT_TYPE, AuthorizationGrantType.AUTHORIZATION_CODE.getValue()); |
There was a problem hiding this comment.
@vincent-hsin This request is invalid.
As per spec, in Section 4.1.3. Access Token Request:
If the client type is confidential or the client was issued client
credentials (or assigned other authentication requirements), the
client MUST authenticate with the authorization server as described
in Section 3.2.1.
Only confidential clients are supported at the moment, so the client MUST authenticate at the token endpoint.
The change in this PR would allow a malicious user to obtain a token.
FYI, public client authentication support is coming in #45.
I'm going to close this PR and associated issue based on the above explanation.
There was a problem hiding this comment.
Ok, I got it. I will contribute for #45.
jgrandja
added
the
status: declined
Whether request with client_id parameter or not , client Principal should be get.