Subject: Re: [OAUTH-WG] error codes in 4.1.2.1 and 4.2.2.1 and extension response types (8.4)

From: Eran Hammer <[email-masked]>
Date: Fri, 20 Jan 2012 12:48:21 -0700

Thanks.

> -----Original Message-----
> From: [email-masked] [mailto:[email-masked]] On Behalf
> Of Roger Crew
> Sent: Sunday, November 13, 2011 4:19 PM

> (1) 4.1.2.1, and 4.2.2.1 both say that in the case that client_id is
>     provided and invalid/unknown, the auth server MUST NOT
>     automatically redirect.
> 
>     However, if the client_id is MISSING, that clause of
>     4.1.2.1/4.2.2.1 does not apply, and thence this becomes a
>     malformed request (i.e., a required parameter is missing)
>     which then results in an error='invalid_request' redirection.
> 
>     Which looks like a mistake to me, because in that situation
>     what reason do we have to be trusting redirect_uri?

In practice, a missing client identifier should also trigger an invalid redirect URI as the server is unable to validate it. But I agree this doesn't read correctly. Changed the text in both places to read:

              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 SHOULD inform
              the resource owner of the error, and MUST NOT automatically redirect the user-agent
              to the invalid redirection URI.

This minor tweak does not change the intended behavior described in -22.

> (2) If the response_type is provided but unknown, is that an
>     'invalid_request' (since this is clearly an "unsupported
>     parameter value") or is it an 'unsupported_response_type'?
> 
>     Seems to me it should be the latter.  If so, then...
> 
>     Given that for ALL currently defined parameters to authorization
>     endpoint requests, there are already provisions for what happens
>     if the value is provided but invalid/unsupported/etc, i.e.,
> 
>        bad client_id     => do not redirect
>        bad redirect_uri  => do not redirect
>        bad response_type => redirect error='unsupported_response_type'
>        bad scope         => redirect error='invalid_scope'
>        bad state         => undetectable from the server side
> 
>     should the description for 'invalid_request' even be referring to
>     "unsupported values" at all?
> 
>     A couple of alternatives (again these are for 4.1.2.1/4.2.2.1):
> 
>        invalid_request
>            a required *extension* parameter is missing
>            or has an unsupported value, or the request is
>            otherwise malformed.
> 
>        invalid_request
>            the request is malformed in some manner not covered by any
>            of the other error codes defined for this response type
> 
>     Either of these would make clear that 'unsupported_response_type'
>     is indeed the error code for the case of a
>     provided-but-unrecognized response_type.
> 
>     Section 5.2 has the same problem w.r.t. 'unsupported_grant_type'

Changed the definition of invalid_request to:

                      The request is missing a required parameter, includes an invalid
                      parameter value, or is otherwise malformed.

I rather not introduce new logic such as define invalid_request as a catch all.

> (3) If 'server_error' and 'temporarily_unavailable' are intended to
>     correspond to HTTP status codes 500 and 503, it might be good to
>     say so explicitly, e.g.,
> 
>     The authorization server SHOULD, where possible, use these
>     redirection error responses in preference to sending the
>     corresponding status=(500|503) HTTP response in situations where
>     the latter would otherwise be appropriate.
> 
>     Admittedly, there's no way an implementation is going to catch all
>     of these, but I'm assuming the intent is to catch as many as
>     possible?

The two error codes are meant to correspond to the HTTP 5xx codes mentioned but adding a direct reference would be confusing and import error handling behavior from HTTP which we did not want to do (e.g. we do not specify the recommended client reaction to any of the other error codes).

No change.

> (4) The lists of error codes in 4.1.2.1 and 4.2.2.1 are essentially
>     identical.  I believe these can be merged without any loss of
>     clarity.
> 
>     (... As a matter of general principle, I'm not a huge fan of
>     having to chase down swaths of cut&pasted text and then having to
>     use 'diff' to figure out whether/how they are different.  Better
>     to combine the common stuff in a single place, use a cross
>     reference and highlight the differences.  And I absolutely do not
>     mind following cross-references...)
> 
>     In this case, the ONLY place where there's ANY variance at all is
>     in the descriptions of 'unauthorized_client' and
>     'unsupported_response_type'.
> 
>     I figure either of the following works:
> 
>         unauthorized_client
>            The client is not authorized to use any of the supported
>            authorization grant types implied by the requested
>            response_type.
> 
>         unauthorized_client
>            The client is not authorized to use this response_type.
> 
>     the latter having the advantage that we don't then get bogged
>     down in the question of how response_types and grant_types relate
>     to each other (see (5) below).
> 
>     And similarly for 'unsupported_response_type', viz
> 
>         unsupported_response_type
>            The authorization server does not support any of the
>            authorization grant types implied by the requested response
>            type.
> 
>         unsupported_response_type
>            The authorization server does not support this response
>            type.
> 
>     at which point you can replace 4.2.2.1 with a single sentence
>     referring to 4.1.2.1.

We had versions of this consolidation in past editions and the WG preference has been to reduce references as much as possible.

No change.

> (5) I am assuming that each authorization endpoint response_type is
>     NOT necessarily unique to a particular authorization grant type.
>     It would be helpful to state this explicitly (probably in 8.4) so
>     that implementers can plan accordingly.
> 
>     And if this IS intended otherwise (i.e., that every response
>     type MUST imply a particular grant type, as is the case for the
>     current spec so long as no new response types are ever defined),
>     then that really should be stated somewhere in 8.4.
> 
>     The reason I assume not is that this seems a straightforward (to
>     me, at least) use of response_type="token code", i.e., to allow
>     the server to respond EITHER as per 4.1.2 or 4.2.2 as it sees fit
>     (but evidently WG members are not agreeing on whether this
>     response_type is needed, hence its absence from the current spec
>     other than as a remark in 8.4).

I am not sure what you mean by this but the intention is that the meaning and handling of combinations is out of scope. For example, 'token code' can mean both or one of the two, and this is left to the spec defining such combination. The only requirement is that once 'token code' is registered, 'code token' cannot, and both mean the same (whatever that is).

EHL