[OAUTH-WG] Comments on -18

[Torsten Lodderstedt <[e-mail]>]

section 1.5

"(A)  The client requests an access token by authenticating with the
         authorization server, and presenting an authorization grant."

This statement does not reflect that client authentication is not always 
required.

Proposal:

"The client requests an access token by presenting an authorization 
grant. The authorization server may require the client to authenticate 
as a pre-requisite."

section 4.1

"(D)  The client requests an access token from the authorization
         server's token endpoint by including the authorization code
         received in the previous step.  When making the request, the
         client authenticates with the authorization server."

Same as above. Proposal:

".... When making the request, the
         client may be required to authenticate with the authorization 
server"

"(E)  The authorization server authenticates the client, validates the
         authorization code, and ensures the redirection URI received
         matches the URI used to redirect the client in step (C)."

Same as above.

Additionally, is the URI check also required if the client did not 
passed a redirect uri but relied on its pre-registered redirect_uri?

section 4.1.1

"state" parameter: Would it make sense to elaborate on the usage of this 
parameter and its importance for preventing CSRF attacks (incl. the 
nessessary entropy)? Alternatively, a reference to the security 
consideration section could be added.

section 4.1.3

"If the client type is private or was issued client credentials ..." 
Isn't the later the most important property of a "private" client? If 
so, "or was issued client credentials" is redundant.

section 4.4.3

"A refresh token SHOULD NOT be included." Why not "MUST NOT"?

section 5.2

"The authorization server MAY return an HTTP 401
                (Unauthorized) status code to indicate which HTTP
                authentication schemes are supported."

Given the usage of HTTP authentication schemes is the way to 
authenticated client recommended by the spec, status code 401 should be 
the default status code for this kind of error. Usage of status code 400 
should be the exception.

"unauthorized_client"

So above - status code 403 seems to be a more appropriate default.

section 10

"and furthermore that rotation of the client
       authentication credentials is impractical."

What does this mean?

Please update reference [I-D.lodderstedt-oauth-security] to 
[I-D.ietf-oauth-v2-threatmodel].

section 10.2

last sentence: "... ensure the repeated request comes from an authentic 
client and not an
    impersonator."

The authorization server must ensure that the request comes from the 
same client not just any authentic client. So I would propose to adjust 
the text as follows:

"... ensure the repeated request comes from the original client and not an
    impersonator."

section 10.3

"Access token (as well as any type-specific attributes) MUST ..." does 
"type-specific" refer to access token type specific? If so, please add 
this information.

section 10.6

Please replace the first sentence with the following text:

"Such an attack leverages the authorization code ..."