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" <aanga...@mitre.org> 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" <aanga...@mitre.org> > Date: Tuesday, June 4, 2013 1:56 PM > To: "oauth@ietf.org" <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 > https://www.ietf.org/mailman/listinfo/oauth
_______________________________________________ OAuth mailing list OAuth@ietf.org https://www.ietf.org/mailman/listinfo/oauth
