Amanda, thanks for the review -- comments inline.
On Jun 4, 2013, at 1:56 PM, "Anganes, Amanda L"
<aanga...@mitre.org<mailto:aanga...@mitre.org>> wrote:
[[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.
I think that this makes a lot of sense, and I'll add one to the next version of
the spec. This is what I've got to start with:
+--------(A)- Initial Access Token
|
v
+-----------+ +---------------+
| |--(B)- Client Registration Reqeust -->| Client |
| | | Registration |
| |<-(C)- Client Information Response ---| Endpoint |
| | +---------------+
| |
| | +---------------+
| Client or |--(D)- Read or Update Request ------->| |
| Developer | | |
| |<-(E)- Client Information Response ---| Client |
| | | Configuration |
| | | Endpoint |
| | | |
| |--(F)- Delete Request --------------->| |
| | | |
| |<-(G)- Delete Confirmation -----------| |
+-----------+ +---------------+
(Apologies if that ascii art doesn't translate through my mail client.)
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.
This makes sense. I had tried to use §1.3 to talk about this, but I can pull it
out as a separate section, like a subsection to 1.3, and I'll incorporate some
of your text.
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.
It's not an either/or choice, you could easily do both on the server and switch
your functionality based on whether or not there's an authentication token in
the request. There are plenty of OAuth-protected APIs that do this today, where
if you call without a token then you get public information, but if you call
with a token you get protected information. We want this to follow the same
pattern, so I'll work on the language there.
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.
Hm, looks like vestigial text from a very old revision where everything had to
go through a single endpoint (and we had an operation parameter).
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.)
Calling it out more explicitly as an OAuth 2.0 protected resource here (and
elsewhere) will help.
Nits in section 7 Security Considerations:
2nd paragraph, first sentence: "…requests to the *Client* Registration Endpoint"
Yup, my mistake, thanks.
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.
In general, this is an open question I have about where the appropriate use of
the 2119 words should go, and I'm hoping for feedback from people with more
spec-writing experience with this.
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
I agree that a new name would be great -- I picked "Initial Access Token"
because we needed to call it *something*, and Mike suggested that we call it
out in the Terminology section as an explicit term to give us a starting point.
I don't like the name as it is myself -- it's too generic, doesn't really tell
you what the token's for. Even in offline conversations about this document
over the last month, I've taken to just picking a random term to use with my
interlocutor in order to refer back to it as a concept. I completely agree that
the functionality is essential and we can do a better job of explaining it.
Naming things is hard, and I am very open to suggestions on a new name! Please
suggest things!
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.
"Registration Access Token" is pretty baked in because that's the parameter
name (registration_access_token) that's returned from the registration endpoint
in the JSON object. I'm not comfortable with changing that at this stage unless
there's a very wide support for it from across the working group.
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.
I'm not very comfortable with tying the token explicitly to the developer
because that implies that it's some kind of assertion. It might be an assertion
but it might just be an OAuth token that's used in the build/deploy process to
meter access to the registration endpoint. As Phil has suggested, and I agree,
the dynamic registration spec should be silent as to the content and structure
of the token, just like RFC6749/6750 is.
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.
The problem is that I've found that it's a *particular* OAuth 2 access token,
and it's distinct from the Registration Access Token. The Initial Access Token
might be a JWT, it might be a SAML blob, it might be a hex blob issued locally
(as Phil has suggested), and it might not be a bearer token at all (if we can
ever finish other token types in the WG). I think we need it to be "an OAuth
2.0 Access Token optionally used for authorized calls to the Registration
Endpoint" and leave it at that, but I do still think it needs some kind of name.
Thanks,
-- Justin
--Amanda
_______________________________________________
OAuth mailing list
OAuth@ietf.org<mailto:OAuth@ietf.org>
https://www.ietf.org/mailman/listinfo/oauth
_______________________________________________
OAuth mailing list
OAuth@ietf.org
https://www.ietf.org/mailman/listinfo/oauth
