Skip to content
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.

Sign up for GitHub

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

Jump to bottom

return unauthorized_client in oauth2 error code #2575

Closed
wants to merge 2 commits into from
Closed

return unauthorized_client in oauth2 error code #2575

wants to merge 2 commits into from

Conversation

strehle
Copy link
Member

@strehle strehle commented Oct 25, 2023 •
edited
Loading

return unauthorized_client in oauth2 error response

Fix for issue #2545

@cf-gitbot
Copy link

We have created an issue in Pivotal Tracker to manage this:

https://www.pivotaltracker.com/story/show/186333540

The labels on this github issue will be updated when the story is started.

@strehle strehle linked an issue Oct 25, 2023 that may be closed by this pull request
Error response code not compliant to RFC 6749 #2545
Closed
@strehle strehle added this to the 76.25.0 milestone Oct 26, 2023
@strehle strehle added the in_review The PR is currently in review label Oct 26, 2023
@Tallicia Tallicia self-assigned this Oct 31, 2023
Tallicia
Tallicia suggested changes Oct 31, 2023
View reviewed changes
Copy link
Contributor

@Tallicia Tallicia left a comment

Choose a reason for hiding this comment

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

I want to validate the one question inline before approving.

uaa/src/test/java/org/cloudfoundry/identity/uaa/mock/token/TokenMvcMockTests.java Outdated Show resolved Hide resolved
@Tallicia Tallicia added clarification needed The issue is not accepted but we need clarification and removed unscheduled labels Oct 31, 2023
@strehle
Ensure www-authenticate header
223116a 
In case of an exception there is a generic
www-authenticate header which ends in a browser
popup. For Oauth2 errors this is normally not
required. Ensure with this commit to stay
with same header message
@strehle strehle requested a review from Tallicia November 1, 2023 08:53
@strehle strehle modified the milestones: 76.25.0, 76.26.0 Nov 3, 2023
@strehle
Copy link
Member Author

strehle commented Nov 7, 2023

@bruce-ricard since you are active on some issues, can you please check if you see an issue with this, I would like to solve this for next release

@neelalex
Copy link

neelalex commented Nov 8, 2023

@strehle Should this be invalid_client as you mentioned in the ticket?

@strehle
Copy link
Member Author

strehle commented Nov 8, 2023

@strehle Should this be invalid_client as you mentioned in the ticket?

not necessarily, but currently used unauthorized_client to have it similar to non stand unauthorized code.

My tests with others have shown, that they use invalid_client, but unauthorized_client is also oauth2 complaint.

But therefore asked the other UAA maintainer for their thouths

@neelalex
Copy link

neelalex commented Nov 8, 2023 •
edited
Loading

@strehle Should this be invalid_client as you mentioned in the ticket?

not necessarily, but currently used unauthorized_client to have it similar to non stand unauthorized code.

My tests with others have shown, that they use invalid_client, but unauthorized_client is also oauth2 complaint.

But therefore asked the other UAA maintainer for their thouths

The issue addressed in this ticket is with invalid client credentials, the error response was expected to be as follows as per https://www.rfc-editor.org/rfc/rfc6749#section-5.2

Code: 401 Unauthorized
Body:
“error”: “invalid_client”
“error_description”: “Bad credentials”

whereas -> unauthorized_client : The authenticated client is not authorized to use this authorization grant type.
This means this error occurs when the client is not authorized to request the requested resource, even if the client credentials are valid. It typically indicates that the client does not have the necessary permissions or scope to access the requested resource.

@Tallicia
Copy link
Contributor

Tallicia commented Nov 9, 2023

Thank you for clarifying this @neelalex

@strehle strehle mentioned this pull request Nov 13, 2023
Merged
@strehle strehle removed a link to an issue Nov 13, 2023
Error response code not compliant to RFC 6749 #2545
Closed
@strehle strehle linked an issue Nov 13, 2023 that may be closed by this pull request
Error response code not compliant to RFC 6749 #2545
Closed
@strehle strehle removed in_review The PR is currently in review clarification needed The issue is not accepted but we need clarification labels Nov 16, 2023
@strehle
Copy link
Member Author

strehle commented Nov 16, 2023

close this, because we have clarifid we go with the alternative PR #2596

@strehle strehle closed this Nov 16, 2023
@strehle strehle deleted the fix/issue/2545-1 branch December 1, 2023 12:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@peterhaochen47 peterhaochen47 Awaiting requested review from peterhaochen47

@Tallicia Tallicia Awaiting requested review from Tallicia

@torsten-sap torsten-sap Awaiting requested review from torsten-sap

@tack-sap tack-sap Awaiting requested review from tack-sap

@bruce-ricard bruce-ricard Awaiting requested review from bruce-ricard

Assignees

@Tallicia Tallicia

Labels
None yet
Projects
Foundational Infrastructure Working Group
Archived in project
Milestone
76.26.0
Development

Successfully merging this pull request may close these issues.

Error response code not compliant to RFC 6749
4 participants
@strehle @cf-gitbot @neelalex @Tallicia