Skip to content

Introduce OAuth2Authorization success/failure handlers #7840

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
jgrandja opened this issue Jan 16, 2020 · 7 comments
Closed

Introduce OAuth2Authorization success/failure handlers #7840

jgrandja opened this issue Jan 16, 2020 · 7 comments
Assignees
Labels
in: oauth2 An issue in OAuth2 modules (oauth2-core, oauth2-client, oauth2-resource-server, oauth2-jose) type: enhancement A general enhancement
Milestone

Comments

@jgrandja
Copy link
Contributor

This ticket addresses the Servlet implementation of #7699

@jgrandja jgrandja self-assigned this Jan 16, 2020
@jgrandja jgrandja added in: oauth2 An issue in OAuth2 modules (oauth2-core, oauth2-client, oauth2-resource-server, oauth2-jose) type: enhancement A general enhancement labels Jan 16, 2020
@jgrandja jgrandja added this to the 5.3.0.RC1 milestone Jan 16, 2020
@jgrandja jgrandja changed the title Introduce OAuth2Authorization success/failure handlers #7699 Introduce OAuth2Authorization success/failure handlers Jan 16, 2020
@philsttr
Copy link
Contributor

@jgrandja Feel free to tag me as a reviewer on the implementation if you want an extra set of eyes.

@jgrandja
Copy link
Contributor Author

@philsttr I just submitted the PR for the Servlet implementation #7986. I'm planning on merging this by end of next week. If you have time to review that would be great. No worries if you're busy next week.

@jgrandja
Copy link
Contributor Author

@philsttr I forgot to mention...when I was implementing the Servlet version, I noticed a code path that may not be valid. This specific test throws an OAuth2AuthorizationException directly from the ExchangeFunction, which ultimately branches into this code path. I'm not sure this scenario could happen. Maybe I'm missing something that you are seeing? Could you look at this again and provide some clarification?

@philsttr
Copy link
Contributor

That scenario will not happen by the classes and functionality provided by spring security out-of-the-box.

I specifically added that code path to allow downstream filters to be able to inspect a resource server response and throw an OAuth2AuthorizationException to trigger the logic in the upstream filter function. This could be utilized by applications talking to a non-compliant resource server. For example, maybe their resource server returns a 400 status... or maybe it's an rpc-style server that returns an error in the response body.

@jgrandja
Copy link
Contributor Author

Gotchya! That makes sense. Ok I will add it back into the Servlet impl.

jgrandja added a commit that referenced this issue Feb 24, 2020
@jgrandja
Copy link
Contributor Author

@philsttr There were some package tangles in the Servlet and Reactive implementations. I resolved it via c8cc971. When you get a chance, take a look at the changes and let me know if you have any concerns. Thanks.

@philsttr
Copy link
Contributor

@jgrandja LGTM!

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 a pull request may close this issue.

3 participants