-
Notifications
You must be signed in to change notification settings - Fork 31
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
Add response codes for error scenarios #220
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Following discussion at the TSC meeting last week, we need to be clear which parts of our documents are normative and which are informative.
So is this new section intended to be normative? - i.e. it will apply, even if it turns out that OIDC says something different compared to this section.
Or is it informative? So a summary of how we think OIDC errors apply to the CAMARA scenario, but OIDC remains normative?
The use of "MUST" in the first sentence implies normative, in which case, that status should be added in brackets to the section header e.g. "Appendix A (Normative) : Error Responses"
As discussed in the last WG call, this proposal is to make the definitions of error scenarios normative. |
* [RFC 8259 - The JavaScript Object Notation (JSON) Data Interchange Format](https://www.rfc-editor.org/info/rfc8259) | ||
* [RFC 8414 - OAuth 2.0 Authorization Server Metadata](https://www.rfc-editor.org/info/rfc8414) | ||
|
||
## Appendix A: Error Responses |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
## Appendix A: Error Responses | |
## Appendix A (Normative): Error Responses |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Everything in this specification is normative except examples.
I would like to have this sentence in the Appendix
For the convenience of implementers this section is a collection of error responses defined in CIBA, OIDC and OAuth2. This Appendix does not change the definition of error responses specified in CIBA, OIDC and OAuth2.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, let's see what 3GPP has to say on the subject:
The annex heading of a TS shall be followed by the indication "(normative):" or "(informative):", and by the title on the next line.
So a normative 3GPP technical specification (TS) may have informative annexes, and hence whether an annex is normative or informative must be clearly marked. Other standards organisations (e.g. OMA) do the same.
Nobody can accuse 3GPP of not knowing how to write a specification, and I think this is one rule we should follow to avoid any possibility of confusion in the reader's mind. What is stated in the annex is normative, and not just a handy "aide memoire".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we need to make sure that the new Appendix does not change the standards.
For the convenience of implementers this section is a collection of error responses defined in CIBA, OIDC and OAuth2. This Appendix does not change the definition of error responses specified in CIBA, OIDC and OAuth2.
If we change or clarify the standards for .e.g. interoperability reasons then I would prefer to have that change in the main body of the specification. When we create a long list of something that is already standardized then we are creating a document that is hard to read and it becomes hard for the operator of the OpenId Provider to make sure that they follow the profile. If an operator buys an openid provider of the shelf then they can be quite certain that the OpenId Provider implements the standards. If it does not, than that must be corrected. CAMARA authorization servers MUST implement the OpenId Connect standard (and CIBA). The OpenId Foundation recommends to use certified OpenId Providers. Even major Internet players and Identity companies got their OpenId Provider implementation wrong which led to security issues. It is a major undertaking to implement an OpenId provider.
#### Authorization Code Flow | ||
|
||
If the request fails due to a missing, invalid, or mismatching redirection URI, or if the client identifier is missing or invalid, | ||
the authorization server MUST NOT automatically redirect the user-agent and SHOULD inform the resource owner of the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we be more specific on how the authorization server informs the resource owner? This section repeats what is already defined in OIDC, OAuth2 and CIBA. Why does it not mention invalid_request_uri
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As far as I know, the only suggestion ever made was to send the request object by value, not by reference. So this error would not occur as it would return request_uri_not_supported
.
Added clarification about how to inform resource owner when no redirection.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@garciasolero
I don't understand what an invalid request_uri has to do with whether a request object is send by value or reference.
Common invalid redirect_uri cases are:
- value is not matching an registered values for the client_id
e.g. redirect_uri=https://attacker.example.com/ with a registered value being https://client.example.com/
e.g. redirect_uri=ts43client: with a registered value being https://ts43client.example.com/
In these cases the OP MUST not use https://attacker.example.com/ or ts43client in any way but must respond with an HTTP 400 Bad Request
|
||
### Authentication Error Response | ||
|
||
#### Authorization Code Flow |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this new section is valuable, particularly for implementor technical teams we've spoken to that are not so close to OpenID specification, and for integrators who can use it as a concise requirements guide. I think it would be helpful if the sub-sections linked out to the relevant OIDC Error specification for readers to review the full context of the expected error response (e.g. Authentication Error Response). For example, in Auth Code flow, there are other parameters that can be returned (e.g. a state parameter may be required depending on the input).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added new links to relevant error specifications.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where does the "SHOULD inform the resource owner of the error" come from?
That mean that the authorization server returns a HTTP 200 OK and the user agent shows the error page that "informs" the user, right?
What are the pros and cons to returning a HTTP 400 BAD REQUEST?
HTTP/1.1 400 Bad Request
Content-Type: application/json
Cache-Control: no-store
{
"error": "invalid_request_uri"
}
* [RFC 8259 - The JavaScript Object Notation (JSON) Data Interchange Format](https://www.rfc-editor.org/info/rfc8259) | ||
* [RFC 8414 - OAuth 2.0 Authorization Server Metadata](https://www.rfc-editor.org/info/rfc8414) | ||
|
||
## Appendix A: Error Responses | ||
|
||
This section describes the error responses that the Authorization Server MUST return for all flows defined in this profile. Based on the supported functionality, the set of errors and the scenarios that can generate them are defined from the specifications already referenced in this document. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This section describes the error responses that the Authorization Server MUST return for all flows defined in this profile. Based on the supported functionality, the set of errors and the scenarios that can generate them are defined from the specifications already referenced in this document. | |
For the convenience of implementers this section is a collection of error responses defined in CIBA, OIDC and OAuth2. This Appendix does not change the definition of error responses specified in CIBA, OIDC and OAuth2. |
|
||
| Error Code | Scenario | | ||
|:---------------------------:|-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------| | ||
| `invalid_request` | The request is missing a required parameter, includes an invalid parameter value, includes a parameter more than once, or is otherwise malformed.<br/>If the server requires PKCE and the client does not send the `code_challenge` in the request.<br/>If the server supporting PKCE does not support the requested `code_challenge_method`. | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| `invalid_request` | The request is missing a required parameter, includes an invalid parameter value, includes a parameter more than once, or is otherwise malformed.<br/>If the server requires PKCE and the client does not send the `code_challenge` in the request.<br/>If the server supporting PKCE does not support the requested `code_challenge_method`. | | |
| `invalid_request` | The request is missing a required parameter, includes an invalid parameter value, includes a parameter more than once, or is otherwise malformed.<br/>If the server requires [Proof Key for Code Exchange (PKCE)](https://www.rfc-editor.org/rfc/rfc7636#section-4.4.1) by OAuth public clients and the client does not send the `code_challenge` in the request, the authorization endpoint MUST return the authorization error response with the `error` value set to `invalid_request`. The `error_description` or the response of `error_uri` SHOULD explain the nature of error, e.g., code challenge required..<br/>If the server supporting PKCE does not support the requested `code_challenge_method`. | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm okay with adding the reference to the PKCE standard. However, I don't see why there should be any reference to error_description
or error_uri
in that section, as they could equally apply to other error codes. It is assumed that the authserver should use the error_description
or the response of error_uri
to explain the nature of the errors.
|
||
#### Authorization Code Flow | ||
|
||
If the request fails due to a missing, invalid, or mismatching redirection URI, or if the client identifier is missing or invalid, the authorization server MUST NOT automatically redirect the user-agent and SHOULD inform the resource owner of the error. For instance, the authorization server MAY display a message to the user describing the problem. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the request fails due to a missing, invalid, or mismatching redirection URI, or if the client identifier is missing or invalid, the authorization server MUST NOT automatically redirect the user-agent and SHOULD inform the resource owner of the error. For instance, the authorization server MAY display a message to the user describing the problem. | |
If the Redirection URI is invalid, the Authorization Server MUST NOT redirect the User Agent to the invalid Redirection URI. If the client identifier is missing or invalid, the authorization server MUST NOT redirect the User Agent to the mismatching Redirection URI. | |
The authorization server SHOULD inform the resource owner of the error. For instance, the authorization server MAY display a message to the user describing the problem. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I copied the text from the standard. I think that is safer, because otherwise we might inadvertently change the standard. Specifications must be precise and reformulating specifications bears the danger of introducing misunderstandings and interpretations. The previous sentence contained the adjective "automatically" which some implementer could understand as "if I configure the authorization server" then that is not "automatic" because I configured it manually. Maybe that is contrived, but so is interpreting the specified tel:" prefix for login_hint as "Tel:" being allowed. Whatever.
I suggest that we copy the error response definitions verbatim to this Appendix.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The text is exactly as it is in the OAuth 2.0 specification, with the order of the sentence changed. The only addition, as you suggested, was the new sentence regarding how the user should be informed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I looked at OIDC https://openid.net/specs/openid-connect-core-1_0.html#AuthError and copied the text from there.
Regarding "inform", if the user agent is a browser and the AZ returns HTTP 200 OK "information" then the resource owner would be informed. Or the AZ redirects to location: https://errors.mno.com/invalid_request_uri.
If the AZ returns HTTP 400 Bad Request { "error": "invalid_request_uri" } is the user then informed?
If the API Consumer has a mobile app, then how would it react to these cases?
I think HTTP 400 Bad Request is the cleanest error response if the redirect_uri is invalid.
Also that is my understanding of the standards OIDC and OAuth2.
I would remove the "inform" sentence.
Further reading on redirect_uri validation etc:
https://datatracker.ietf.org/doc/html/draft-ietf-oauth-security-topics#:~:text=stage%2C%20maybe%20because-,it,-doesn%27t%20seem%20to
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would always quote the higher standard, meaning OIDC is based on OAuth2. So, I would quote OIDC first.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Your strategy of referencing the higher standard may be good in many cases, but in this case the OIDC text is less precise and does not cover the scenario of the missing redirect_uri.
In the authserver cannot redirect scenario, the standard is clear that you must inform the resource owner, i.e. the user. So you need to communicate with them in a language they understand by displaying a message on a natural language page. Although it may seem strange to us, a human being does not understand HTTP status codes, JSON format or OAuth error codes. ;)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
What type of PR is this?
What this PR does / why we need it:
We know that many scenarios are already conveniently explained in the documentation of various standards, but a new section compiling errors and their scenarios from several specifications would reduce misunderstandings and potential interoperability issues between operators.
Which issue(s) this PR fixes:
Fixes #211