Skip to content

Fix #8797: Add OAuth2AuthenticationException to allowlist #8827

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
wants to merge 5 commits into from

Conversation

schnapster
Copy link
Contributor

Add mixins for

  • OAuth2AuthenticationException
  • OAuth2Error

See #8797 for more details.

@schnapster schnapster changed the title Whitelist OAuth2AuthenticationException Fix #8797: Whitelist OAuth2AuthenticationException Jul 11, 2020
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Jul 11, 2020
@schnapster
Copy link
Contributor Author

@jgrandja here's my take on #8797, what do you think?

Side note: I could not run the whole test suite locally to verify nothing else breaks. Master branch is failing locally with the same error as the CI build:

org.springframework.security.concurrent.CurrentDelegatingSecurityContextScheduledExecutorServiceTests > invokeAnyTimeout FAILED
    java.lang.NoSuchMethodError at PowerMockitoInjectingAnnotationEngine.java:40

@jgrandja jgrandja 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 Jul 16, 2020
@jgrandja jgrandja added this to the 5.4.0-RC1 milestone Jul 16, 2020
Copy link
Contributor

@jgrandja jgrandja 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 @napstr! I left a couple of minor comments.

After you update, please rebase on master before you push and it should be ready to merge.

@jgrandja jgrandja added the status: duplicate A duplicate of another issue label Jul 16, 2020
@schnapster
Copy link
Contributor Author

schnapster commented Jul 16, 2020

@jgrandja thanks for the detailed feedback. I think I have now addressed all your points, and expanded the test cases following the example set by other mixin tests. Please have another look.
Oh, also, using a local snapshot of this branch, I can confirm this solves the issue in my application.

@jgrandja jgrandja changed the title Fix #8797: Whitelist OAuth2AuthenticationException Fix #8797: Add OAuth2AuthenticationException to allowlist Jul 21, 2020
@jgrandja
Copy link
Contributor

Thanks for the updates @napstr ! This is now in master and backported to 5.3.x

@jgrandja jgrandja closed this Jul 21, 2020
@schnapster schnapster deleted the gh-8797 branch July 21, 2020 15:10
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: duplicate A duplicate of another issue type: bug A general bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants