Re: [OAUTH-WG] LC Review of draft-ietf-oauth-dyn-reg-12

[Phil Hunt <[e-mail]>]

Those changes off git hub have not been shared with the group and are not necessarily approved. 

Phil

On 2013-06-04, at 11:03, "Anganes, Amanda L" <[e-mail]> wrote:

> 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" <[e-mail]>
> Date: Tuesday, June 4, 2013 1:56 PM
> To: "[e-mail]" <[e-mail]>
> 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
> [e-mail]
> https://www.ietf.org/mailman/listinfo/oauth