Skip to content

Add OAuth2Authorization success/failure handlers #7986

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

jgrandja
Copy link
Contributor

Fixes gh-7840

@jgrandja jgrandja added type: enhancement A general enhancement in: oauth2 An issue in OAuth2 modules (oauth2-core, oauth2-client, oauth2-resource-server, oauth2-jose) labels Feb 15, 2020
@jgrandja jgrandja added this to the 5.3.0 milestone Feb 15, 2020
@jgrandja jgrandja self-assigned this Feb 15, 2020
Copy link
Contributor

@philsttr philsttr left a comment

Choose a reason for hiding this comment

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

This looks great @jgrandja ! I just had one comment on a potential improvement to both the servlet and reactive impls.

* @return a {@link Mono} that completes empty after the authorization failure handler completes
*/
private Mono<Void> handleHttpStatus(ClientRequest request, int httpStatusCode, @Nullable Exception exception) {
return Mono.justOrEmpty(this.httpStatusToOAuth2ErrorCodeMap.get(httpStatusCode))
Copy link
Contributor

Choose a reason for hiding this comment

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

This applies to both the servlet impl and the reactive impl.... But this could theoretically check for (and parse) a WWW-Authenticate header returned from the resource server. It likely contains the details necessary to create an OAuth2Error object. This would be preferred (and then perhaps fallback to the httpStatusCode -> oauth2ErrorCode mapping if details could not be extracted from WWW-Authenticate).

For example:

HTTP/1.1 403 Forbidden
WWW-Authenticate: Bearer error="insufficient_scope", error_description="The request requires higher privileges than provided by the access token.", error_uri="https://tools.ietf.org/html/rfc6750#section-3.1"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good idea! Let me see what I can do here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@philsttr I added the improvement as suggested. This is now in master.

@jgrandja
Copy link
Contributor Author

Merged via 69156b7

@jgrandja jgrandja closed this Feb 21, 2020
@jgrandja jgrandja deleted the gh-7840-authz-handlers branch February 21, 2020 20:20
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) type: enhancement A general enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Introduce OAuth2Authorization success/failure handlers
2 participants