[OAUTH-WG] Shepherd Review of draft-ietf-oauth-dyn-reg-management-05

[Hannes Tschofenig <[e-mail]>]

Hi Justin, Mike, John, Maciej,

as part of my shepherd write-up I carefully read through
draft-ietf-oauth-dyn-reg-management-05. Overall the document is in good
shape but I have a few minor clarification questions that should be
resolved before the document goes to the IESG.

In Section 1.4. "Registration Tokens and Client Credentials" you explain
the different credential types and I was wondering why you aren't
issuing client_secrets to all clients instead of introducing another
credential, the registration access token. While you try to explain the
reasons those appear somewhat constructed. For example, you say that
"not all types of clients have client credentials" but you are the
author of the specification and you can essentially decide what to do.
The registration access token is essentially the same as the client
credential since it is "is uniquely bound to the client identifier".

I believe we have discussed this issue in the past but I don't remember
what the reasoning was. Can you describe what your design rational was
(and add it to the document)?

In Section 1.4.1. "Credential Rotation" you write:

"
   The Authorization Server MAY rotate the client's registration access
   token and/or client credentials (such as a "client_secret")
   throughout the lifetime of the client.
"

You might want to add reasons why the authorization server may want to
take this step.

There are also a bit too many SHOULDs in the document. Every time you
write SHOULD you have to keep in mind to tell the reader what happens in
the other case.
For example, in Section Section 1.4.1 you write:

"
   The registration access token SHOULD be rotated only in response to
   an update request to the client configuration endpoint, at which
   point the new registration access token is returned to the client and
   the old registration access token SHOULD be discarded by both
   parties.
"

Here the question arises whether you are using the SHOULD to indicate
that the authorization server does not always need to rotate the
registration access token and if he does then is must only happen in
response to an update request or does it indicate that the authorization
server could also rotate it in response to other calls?

Also, in the second line one would wonder why the old registration
access token isn't mandatorily discarded. If there are good reasons,
then tell the reader.
Furthermore, the text in Section 2.2 seems to contract this statement:
"
   If the authorization server includes a new client secret and/or
   registration access token in its response, the client MUST
   immediately discard its previous client secret and/or registration
   access token.
"


In Section 2.2 you write:
"
   However, since read
   operations are intended to be idempotent, the read request itself
   SHOULD NOT cause changes to the client's registered metadata values.
"

I am wondering whether this SHOULD NOT statement adds a lot of value
since you allow the request to change metadata values.
You also write the security consideration section:
"
   the registration access token MAY be
   rotated when the developer or client does a read or update operation
   on the client's client configuration endpoint.
"

This means that the content of the registration access token may also
change with a read operation.


Terminology:

Sometimes you write "Client Information Response" and sometimes "client
information response"
The same with "Authorization Server" and "authorization server"

Typo:

"
   Some values in the response,
   including the "client_secret" and r"egistration_access_token", MAY be
                                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^
   different from those in the initial registration response.
"

In Section 2.4 "Client Delete Request" you write:

"
   The authorization server SHOULD immediately invalidate all existing
   authorization grants and currently-active tokens associated with this
   client.
"

Under what circumstances wouldn't the authorization invalidate all
grants and active tokens?
You might also want to say what tokens you are talking about since there
are at least the following tokens around:
- access tokens
- refresh tokens
- registration access tokens
- initial access token

"
   If the server does not support the delete method, the server MUST
   respond with an HTTP 405 Not Supported.
"

Why aren't you demanding that the server must support this method?
This would essentially mean that there would be some cases where
deregistration isn't possible.
Of course, there may be the question how important deregistration
actually is if the registration
automatically expires.

You write:
"
   If the client does not exist on this server, the server MUST respond
   with HTTP 401 Unauthorized and the registration access token used to
   make this request SHOULD be immediately revoked.
"

If the client does not exist and someone sends a request with a random
registration access token I am not sure what exactly you want to revoke
here.

In Section 3.1. "Client Information Response" you state the new elements
that are returned to the client. While the client_secret has an
expires_at field the registration_access_token doesn't. Does the expiry
value of the client_secret_expires_at automatically indicate the
lifetime of the  registration access token? I think so. But what happens
if the client_secret is not issued? To make it more complicated you
write the following text in the security consideration section:

"
   While the client secret can expire, the registration access token
   SHOULD NOT expire while a client is still actively registered.
"

The IANA consideration section is empty. Wouldn't it be necessary to
register 'registration_access_token' and 'registration_client_uri' into
the OAuth Dynamic Registration Client Metadata Registry?

Ciao
Hannes