[[Apologies if you receive this twice, I accidentally sent this from one of my
other email addresses this morning (Outlook seems to have been confused).]]
Hello,
I have reviewed the Dynamic Registration draft and offer some comments below:
After section 1.2, I suggest adding a flow diagram (similar to those in the
core OAuth 2.0 spec), showing the interaction of the client/developer with the
respective endpoints, and the requests and responses involved. I think this
will make the spec easier to read.
Inside the 3rd paragraph of section 1.3, there is text buried that talks about
client credential rotation (client_secret and Registration Access Token). I
think this notion of secret rotation should be called out in its own paragraph
or subsection and labeled as such. Suggested text (I'm not sure where it should
go):
Section X.X Client Credential Rotation
The Authorization Server may rotate the Client's issued client_secret and/or
Registration Access Token. The client_secret MAY be rotated at any time, in
which case the Client will likely discover that their secret has expired via
attempting and failing to make a request. The Registration Access Token SHOULD
only be rotated in response to an update or read request, in which case the new
Registration Access Token will be returned in the response back to the Client.
The Client can check their current credentials at any time by performing a READ
or UPDATE operation at the Client Configuration Endpoint.
Section 3 Client Registration Endpoint
This section should use the phrase "this endpoint may be open, or it may be an
OAuth 2.0 Protected Resource", rather than just stating that it may accept an
initial token. I *think* that the choice is an either/or for a given server
(ie, a server cannot offer both open and protected registration), but that
should be clarified as well.
In the 4th paragraph, the final sentence ("As such, the Client Configuration
Endpoint MUST…") refers to the Configuration endpoint, not the Registration
endpoint. The text is already in Section 4 so it should just be deleted here.
Section 3.1 Client Registration Request
The lead-ups to the two example requests are phrased oddly. The difference
between the two sample requests is not whether the client has a token, it's
whether the endpoint is open or protected. Suggest changing them to "For
example, if the Client Registration Endpoint supports open registration, the
client could send the following request" and "Alternatively, if the endpoint is
an OAuth 2.0 protected resource, the client MUST include an OAuth 2.0 Access
Token/the Initial Access Token in its request, presented as a Bearer token
using the Authorization header according to RFC6749." (My phrasing at the end
regarding how to present the AT may be off; point is that it should be called
out.)
Nits in section 7 Security Considerations:
2nd paragraph, first sentence: "…requests to the *Client* Registration Endpoint"
6th paragraph, first sentence: "…the Registration Access Token should not
expire…" I think this should be a SHOULD NOT?
Same paragraph, 4th sentence has a non-capitolized "client" that should be
"Client" (although, after reading Hannes' review, maybe the capitalized
instances should all be lowercased instead).
I also second Hannes' comment that the RFC 2119 language feels off throughout
the spec, suggest doing a careful read to check those.
Finally, to address the Initial Access Token / Registration Access Token
discussion that has been ongoing:
My initial response after reading this draft (and having followed the
discussion) was to say, remove the "Initial Access Token" term completely and
instead just clarify the text to say that "the Client Registration Endpoint MAY
be an OAuth 2.0 protected resource, but the details of how a given client or
developer goes about acquiring an Access Token for use at this endpoint is
out-of-scope". I spoke to Justin about this and he pointed out that this term
was only added recently, and it was added because of confusion around how the
Client Registration Endpoint was defined, and what it means to authenticate to
it. I don't think the new name/definition/explanation has helped; but the
previous drafts were also missing the "OAuth 2.0 protected resource" language.
To be clear, I think this functionality is absolutely necessary, but we need to
clarify its explanation
On the other hand, the Registration Access Token seems very clear to me. I
think that term should be called out as a named entity, since it is 'special' -
it's not issued by a token endpoint, but by the Client Registration Endpoint.
I see a few options that might help:
1) Perhaps "Initial Access Token" is a bad name. Unfortunately, I think the
right names are "Registration Access Token" for accessing the Registration
Endpoint, and "Configuration Access Token" for accessing the Configuration
Endpoint. This would require changing code, since the configuration token is
returned in the Client Registration Response.
1.a) The examples in Appendix B only use the IAT for Developer
authentication/tracking (all clients registered using the same IAT can be
traced to the developer that was issued that particular token). Is that the
only use case? If there is always a developer in the loop in the protected
case, then "Developer Access Token" might be appropriate. This is less generic
than the suggestion above, but would not require changing code and might be an
improvement over what's there now.
2) Perhaps if the spec language were clarified and used the "OAuth 2.0
protected resource" language, the Initial Access Token term could be removed
from the document entirely. I don't think the previous drafts got it right, but
I think we can do better than those explanations while still avoiding giving a
fancy name to something that is *just* an OAuth 2.0 Access Token.
--Amanda
_______________________________________________
OAuth mailing list
OAuth@ietf.org
https://www.ietf.org/mailman/listinfo/oauth
