Note that this review applies to the latest spec edits from Justin's
Github, which can be found here:
https://github.com/jricher/oauth-spec. The --12 revision has not been
published yet, but Justin asked me to review based off of what was in
the tracker, since it is more up-to-date.
--Amanda
From: <Anganes>, "Anganes, Amanda L" <aanga...@mitre.org
<mailto:aanga...@mitre.org>>
Date: Tuesday, June 4, 2013 1:56 PM
To: "oauth@ietf.org <mailto:oauth@ietf.org>" <oauth@ietf.org
<mailto:oauth@ietf.org>>
Subject: LC Review of draft-ietf-oauth-dyn-reg-12
[[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 <mailto:OAuth@ietf.org>
https://www.ietf.org/mailman/listinfo/oauth
