Skip to content

OAuth2AccessTokenInterceptor can handle Authentication Principal where principalName is null #1049

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
agileknight opened this issue Aug 19, 2024 · 1 comment · Fixed by #1050
Assignees
Labels
bug Something isn't working
Milestone

Comments

@agileknight
Copy link
Contributor

agileknight commented Aug 19, 2024

Is your feature request related to a problem? Please describe.
When OAuth2AccessTokenInterceptor encounters an Authentication Principal where getName is null, it passes the null value on to the OAuth2AuthorizedClientManager, which in practice usually results in a runtime exception like principalName cannot be empty.

Describe the solution you'd like
The code already checks for a null principal an in that case passes ANONYMOUS_AUTHENTICATION. It appears to also make sense to pass ANONYMOUS_AUTHENTICATION for cases with a non-null principal but a null principal name.

Describe alternatives you've considered
Handling a null principal name downstream or upstream appears more complicated and it feels more natural to follow the current implementation of employing the ANONYMOUS_AUTHENTICATION placeholder for cases where the current authentication principal is not suitable.

Additional context
None

agileknight added a commit to agileknight/spring-cloud-openfeign that referenced this issue Aug 19, 2024
The authorizedClientManager.authorize method requires
a non-null principal name or it will usually throw
an exception in practice like "principalName cannot be empty".

Using the anonymous principal in this case like for
a null principal handles the situation more gracefully.

Fixes spring-cloud#1049
@OlgaMaciaszek OlgaMaciaszek added bug Something isn't working and removed waiting-for-triage labels Sep 4, 2024
@OlgaMaciaszek OlgaMaciaszek moved this to In Progress in 2024.0.0-M2 Sep 4, 2024
@OlgaMaciaszek OlgaMaciaszek added this to the 4.1.4 milestone Sep 4, 2024
@OlgaMaciaszek
Copy link
Collaborator

Hello @agileknight, thanks for reporting the issue. Makes sense.

@OlgaMaciaszek OlgaMaciaszek moved this to In Progress in 2023.0.3 Sep 4, 2024
@OlgaMaciaszek OlgaMaciaszek removed this from 2023.0.3 Sep 4, 2024
@OlgaMaciaszek OlgaMaciaszek moved this to In Progress in 2023.0.4 Sep 4, 2024
agileknight added a commit to agileknight/spring-cloud-openfeign that referenced this issue Sep 5, 2024
The authorizedClientManager.authorize method requires
a non-null principal name or it will usually throw
an exception in practice like "principalName cannot be empty".

Using the anonymous principal in this case like for
a null principal handles the situation more gracefully.

Fixes spring-cloud#1049
agileknight added a commit to agileknight/spring-cloud-openfeign that referenced this issue Sep 5, 2024
The authorizedClientManager.authorize method requires
a non-null principal name or it will usually throw
an exception in practice like "principalName cannot be empty".

Using the anonymous principal in this case like for
a null principal handles the situation more gracefully.

Fixes spring-cloud#1049
agileknight added a commit to agileknight/spring-cloud-openfeign that referenced this issue Sep 5, 2024
The authorizedClientManager.authorize method requires
a non-null principal name or it will usually throw
an exception in practice like "principalName cannot be empty".

Using the anonymous principal in this case like for
a null principal handles the situation more gracefully.

Fixes spring-cloud#1049
@github-project-automation github-project-automation bot moved this from In Progress to Done in 2024.0.0-M2 Sep 6, 2024
@github-project-automation github-project-automation bot moved this from In Progress to Done in 2023.0.4 Sep 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
No open projects
Status: Done
Status: Done
Development

Successfully merging a pull request may close this issue.

3 participants