Skip to content

Fixed NullPointerException with WWW-Authenticate #9303

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

Closed

Conversation

tristanexsquare
Copy link
Contributor

Fixed possible NullPointerException that happens when the OAuth2 ResourceServer does not return a valid WWW-Authenticate header format.

Copy link
Contributor

@jzheaux jzheaux left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR, @tristanessquare! I've left some feedback inline.

@@ -70,7 +70,7 @@ private OAuth2Error readErrorFromWwwAuthenticate(HttpHeaders headers) {
return null;
}
BearerTokenError bearerTokenError = getBearerToken(wwwAuthenticateHeader);
String errorCode = (bearerTokenError.getCode() != null) ? bearerTokenError.getCode()
String errorCode = (bearerTokenError != null && bearerTokenError.getCode() != null) ? bearerTokenError.getCode()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't appear to address the issue completely since bearerTokenError is referred to later on in the method, potentially causing more NPEs.

If wonder if it would be better to do:

if (bearerTokenError == null) {
    return new OAuth2Error(OAuth2ErrorCodes.SERVER_ERROR, null, null);
}
String errorCode = ...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a good point. I will do that.

@jzheaux jzheaux self-assigned this Jan 5, 2021
@jzheaux jzheaux 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 Jan 5, 2021
@jzheaux jzheaux added the status: waiting-for-feedback We need additional information before we can continue label Jan 21, 2021
@jzheaux
Copy link
Contributor

jzheaux commented Jan 21, 2021

Hi, @tristanessquare! Are you able to make the requested changes?

@jzheaux
Copy link
Contributor

jzheaux commented Jan 21, 2021

Thanks, @tristanessquare! I think the changes make sense.

Since this is addressing a bug, would you please also add a test to OAuth2ErrorResponseErrorHandlerTests that confirms that the bug is fixed? Something that would have failed before the change, but now passes would be ideal.

@jzheaux
Copy link
Contributor

jzheaux commented Jan 21, 2021

Thanks for the PR, @tristanessquare! This is now merged into master via 56db058

@jzheaux jzheaux closed this Jan 21, 2021
@jzheaux jzheaux added this to the 5.5.0-M2 milestone Jan 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: oauth2 An issue in OAuth2 modules (oauth2-core, oauth2-client, oauth2-resource-server, oauth2-jose) status: waiting-for-feedback We need additional information before we can continue type: bug A general bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants