Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Reactive OAuth2 using query parameters for access_token can cause HTTP 500s #7011

Closed
bhavikkumar opened this issue Jun 17, 2019 · 1 comment · Fixed by #7020
Closed

Reactive OAuth2 using query parameters for access_token can cause HTTP 500s #7011

bhavikkumar opened this issue Jun 17, 2019 · 1 comment · Fixed by #7020
Assignees
@rwinch
Labels
in: oauth2 An issue in OAuth2 modules (oauth2-core, oauth2-client, oauth2-resource-server, oauth2-jose) status: backported An issue that has been backported to maintenance branches type: bug A general bug
Milestone
5.2.0.M4

Comments

@bhavikkumar
Copy link
Contributor

bhavikkumar commented Jun 17, 2019 •
edited
Loading

Summary

When using ServerBearerTokenAuthenticationConverter with setAllowUriQueryParameter(true) then there is a possibility that the web server will return a HTTP 500 when the query parameter is not filled in. E.g: /hello?access_token=

Edit: This problem potentially also affects DefaultBearerTokenResolver

Actual Behavior

ServerBearerTokenAuthenticationConverter.convert method throws a IllegalArgumentException due to the string being passed to BearerTokenAuthenticationToken::new being empty.

Expected Behavior

The expected behaviour should be that the query parameter is validated in the ServerBearerTokenAuthenticationConverter.token method and a OAuth2AuthenticationException is thrown.

The validation should be similar/same as if the token was being provided from HTTP Header.

Version

    implementation group: 'org.springframework.boot', name: 'spring-boot-starter-webflux', version: '2.1.5.RELEASE'
    implementation group: 'org.springframework.boot', name: 'spring-boot-starter-security', version: '2.1.5.RELEASE'
    implementation group: 'org.springframework.security', name: 'spring-security-oauth2-resource-server', version: '5.1.5.RELEASE'
    implementation group: 'org.springframework.security', name: 'spring-security-oauth2-jose', version: '5.1.5.RELEASE'

Sample

https://github.com/bhavikkumar/reactive-spring-auth
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Jun 17, 2019
@bhavikkumar bhavikkumar mentioned this issue Jun 17, 2019
Closed
rwinch added a commit to rwinch/spring-security that referenced this issue Jun 19, 2019
@rwinch
ServerBearerTokenAuthenticationConverter Handles Empty Tokens
fe97325 
Previously ServerBearerTokenAuthenticationConverter would throw an
IllegalArgumentException when the access token in a URI was empty String.
It also incorrectly provided HttpStatus.BAD_REQUEST for an empty String
access token in the headers.

This changes ServerBearerTokenAuthenticationConverter to consistently
throw a OAuth2AuthenticationException with an HttpStatus.UNAUTHORIZED

Fixes spring-projectsgh-7011
@rwinch rwinch mentioned this issue Jun 19, 2019
Merged
@rwinch rwinch added in: oauth2 An issue in OAuth2 modules (oauth2-core, oauth2-client, oauth2-resource-server, oauth2-jose) type: bug A general bug and removed status: waiting-for-triage An issue we've not yet triaged labels Jun 19, 2019
@rwinch rwinch self-assigned this Jun 19, 2019
@rwinch rwinch added this to the 5.2.0.RC1 milestone Jun 19, 2019
@spring-projects-issues spring-projects-issues added status: backported An issue that has been backported to maintenance branches and removed for: backport-to-5.1.x labels Jun 19, 2019
Closed
@rwinch
Copy link
Member

rwinch commented Jun 19, 2019 •
edited
Loading

Thanks for the detailed report @bhavikkumar! I put together a pull request that I'll have @jzheaux review. This particular issue is scheduled for 5.2.0.RC1 but I also created a backport issue (gh-7021) to get this in 5.1.6

bhavikkumar added a commit to bhavikkumar/spring-security that referenced this issue Jun 19, 2019
@bhavikkumar
Validate bearer token for all flows
5e0605b 
Fixes spring-projectsgh-7011
jzheaux pushed a commit that referenced this issue Jun 24, 2019
@rwinch @jzheaux
ServerBearerTokenAuthenticationConverter Handles Empty Tokens
6f5a443 
Previously ServerBearerTokenAuthenticationConverter would throw an
IllegalArgumentException when the access token in a URI was empty String.
It also incorrectly provided HttpStatus.BAD_REQUEST for an empty String
access token in the headers.

This changes ServerBearerTokenAuthenticationConverter to consistently
throw a OAuth2AuthenticationException with an HttpStatus.UNAUTHORIZED

Fixes gh-7011
jzheaux pushed a commit that referenced this issue Jun 28, 2019
@rwinch @jzheaux
ServerBearerTokenAuthenticationConverter Handles Empty Tokens
aac8544 
Previously ServerBearerTokenAuthenticationConverter would throw an
IllegalArgumentException when the access token in a URI was empty String.
It also incorrectly provided HttpStatus.BAD_REQUEST for an empty String
access token in the headers.

This changes ServerBearerTokenAuthenticationConverter to consistently
throw a OAuth2AuthenticationException with an HttpStatus.UNAUTHORIZED

Fixes gh-7011
kostya05983 pushed a commit to kostya05983/spring-security that referenced this issue Aug 26, 2019
@rwinch @kostya05983
ServerBearerTokenAuthenticationConverter Handles Empty Tokens
821bf21 
Previously ServerBearerTokenAuthenticationConverter would throw an
IllegalArgumentException when the access token in a URI was empty String.
It also incorrectly provided HttpStatus.BAD_REQUEST for an empty String
access token in the headers.

This changes ServerBearerTokenAuthenticationConverter to consistently
throw a OAuth2AuthenticationException with an HttpStatus.UNAUTHORIZED

Fixes spring-projectsgh-7011
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@rwinch rwinch

Labels
in: oauth2 An issue in OAuth2 modules (oauth2-core, oauth2-client, oauth2-resource-server, oauth2-jose) status: backported An issue that has been backported to maintenance branches type: bug A general bug
Projects
None yet
Milestone
5.2.0.M4
3 participants
@rwinch @bhavikkumar @spring-projects-issues