Thanks Mike! Comments inline. > Normative Issues > > 4.1, 4.2, 4.3.1, 5, 5.2, 5.3.1, 6.2, 6.2.1 "Scope" parameter should be paired > with > complimentary "resource" parameter
I am more inclined to drop 'scope' than to include 'resource'. Scope as currently defined can easily accommodate resource - we specifically chose space-delimited to allow URI values. If you want a new parameter, *you* will have to build consensus. Not wearing my editor hat, I'm -1. > 4.1, 5 Parameters without a value > > These sections say that "Parameters sent without a value MUST be treated > as if they were omitted from the request." This excludes the possibility of > the semantics of a value which is supplied as the empty string being different > from an omitted parameter. That's a good thing for interop. Do you have examples where this is a problem for the parameters defined in this specification? This does not have to apply to extensions which are allowed to opt-out of this. > 6.2 "Error" result should be required unless the request lacks any > authentication information > > For the error response from the protected resource, the error code is > presently optional. By making it required, clients can programmatically > detect this condition and request a new access token using the refresh > token. I already upgraded it to a SHOULD. Any strong support for a MUST for the 'error' attribute when the request includes authentication? > 5.2, 6.1, 6.2 Type information only communicated in early legs of the > protocol > > While a grant_type is required in 5 and 5.1, no facility is present for > communicating token/grant types in the subsequent legs of the > protocol. Please add an optional parameter enabling a token/grant type to > be communicated in 5.2, 6.1, and 6.2. What for? The grant type only matters when including an access grant. Including it in 5.2 is odd and in 6.1 or 6.2 would violate the architecture (isolating the access grant from the token request). > 6.1.3 Restriction to POST, PUT, and DELETE verbs overly restrictive > > The more logical restriction would be to verbs containing a body (i.e. - not > GET). (Yes, I realize that this comment actually applies to my spec after the > split, but I'm including it here for completeness.) As noted, no my problem. This should be any verb with a form-encoded body allowed (by HTTP). > 6.2 error_uri simplification > > We should mandate that the error_uri is fully qualified. (Relative URLs are > unnecessarily difficult to manage for this context.) Done. > 6.2 Other response parameters > > Shouldn't there be language in 6.2 that explicitly allows for other arguments > to be added to the response and mandates that they be ignored if not > recognized? Also, possibly some language about how to add new values such > as registering them with IANA? That's 7.3 but I can highlight it. > (many) Unifying treatment of tokens > > And finally, as we discussed, it would be desirable to be able use the same > protocol flow to obtain an authorization code as to obtain access and refresh > tokens. One way to achieve this would be, for instance, to use tokens that > represent username/password and then present those through the OAuth > scheme in the HTTP Authorization header. Other kinds of tokens could be > used as well. Then the authorization server could be simply another instance > of a protected resource. > > I realize that this isn't fully fleshed out but I wanted to raise this issue > now > and follow up with a concrete proposal soon. I don't like this, but I'll wait for a proposal. > Editorial Issues > > 1.2 Terminology > > . "authorization endpoint" and "authorization server" are both used in the > draft. The draft doesn't make it clear whether these terms are equivalent or > not. If not, they should both be defined. If so, you should choose one. One is a server and the other an endpoint on the server. The endpoint is not a term. > . 3.0 uses term "client operator". This should either be just "client" or you > should define "client operator". Dropped 'operator'. > . The term "Authorization code" is underspecified. In practice, the working > definition of the term "authorization code" appears to be scattered > throughout the document, including 1.2, 1.4.1, 3.2 , and 5.1.1. Nowhere, for > instance, does it say what the syntax of a code can be. (I'm pretty sure I > know, but the document should make this explicit.) OK. > 3.0 Security considerations statement in Client Credentials section > > This section says "Authorization servers SHOULD NOT issue client secrets to > clients incapable of keeping their secrets confidential." This should go in > the > security considerations section, issue is there is, in general, no way to > figure > this out at runtime. This is important enough to call attention to right there. This was always a problem in 1.0. > 3.1 Statement about shared symmetric secrets > > This section says that "client password credentials use a shared symmetric > secret". In fact, these could be OTPs values based upon an asymmetric > secret. This is clearly intended to be a password, in the most obvious sense. > 3.2 Security considerations statement in Client Assertion Credentials section > > This section says "The client assertion credentials are used in cases where a > password (clear-text shared symetric secret) is unsuitable or does not > provide sufficient security for client authentication. In such cases it is > common to use other mechanisms such as HMAC or digital signatures that do > not require sending clear-text secrets." This is a security considerations > statement - not a normative statement. No, it is giving context and explaining why we need it. It helps develop choose. > 4, 5 "service documentation" and relationship to discovery underspecified > > The term "service documentation" is used twice without being defined. The > possibility of these endpoints being obtained through discovery, rather than > service documentation, should also likely be explicitly acknowledged. Feel free to suggest text. EHL _______________________________________________ OAuth mailing list OAuth@ietf.org https://www.ietf.org/mailman/listinfo/oauth
