Comments inline. On Wed, 2011-08-17 at 14:15 -0400, Eran Hammer-Lahav wrote: > Thanks for the feedback. > > > -----Original Message----- > > From: [email protected] [mailto:[email protected]] On Behalf > > Of Justin Richer > > Sent: Thursday, August 11, 2011 2:07 PM > > > 1.2/1.4: The term "authorization grant" remains confusing and the > > introduction is riddled with jargon like "intermediate credential". With the > > diagram in 1.2, it appears to be an explicit technological underpinning of > > the > > protocol flow instead of a general conceptual construct that is used in > > several > > different ways. Basically, what "authorization grant" *means* is not obvious > > within this document. > > > > Section 4 makes much more sense than the introduction text does here. > > Perhaps we should just replace most of 1.4 with just the introductory text > > to > > 4 (perhaps slightly expanded), and then a reference to the sub-parts of > > section 4 for the meat of the concept (and in the process, nix the > > subsections > > of 1.4 entirely). > > > > 1.2(B): "Provided" is wrong here (it implies a direct hand-over), and the > > last > > sentence is awkward. Suggest reword to: > > The client receives an authorization grant which represents the > > authorization granted by the resource owner. The type of > > authorization grant is dependent on which methods are supported > > by both the client and authorization server. > > Change (B) in 1.2 to: > > The client receives an authorization grant which is a > credential representing > the resource owner's authorization, expressed using one of four > grant types defined > in this specification or using an extension grant type. The > authorization grant type > depends on the method used by the client to request > authorization and the types > supported by the authorization server. > > And changed 1.4 to: > > An authorization grant is a credential representing the resource > owner's authorization > (to access its protected resources) used by the client to obtain an > access token. This > specification defines four grant types: authorization code, > implicit, resource owner > password credentials, and client credentials, as well as an > extensibility mechanism for > defining additional types.
Much better for both overall though. > > 1.3/1.4/1.5: Consider switching order to Authorization Grant, Access Token, > > Refresh Token > > Not sure. What do others think? I put access token first because it is a more > important term to get out of the way. I agree that access tokens are a more important topic to OAuth overall, but the rest of the document presents things in protocol flow order: you get the auth grant, then you get the tokens. > > 1.4.1: We probably want to mention a user agent here in the exposure risk at > > the end. Since that's really the problem -- the browser could steal the > > token, > > not the end user. > > Proposed text? The authorization code provides a few important security benefits such as the ability to authenticate the client and issuing the access token directly to the client without potentially exposing it to others, including the resource owner or other applications in the resource owner's user agent. (Still not good wording... but the idea is to point out that you the leak possibility here stems from using the front channel.) > > 1.4.2: Still don't like the term "implicit". It's misleading. Even "direct > > authorization" would be better, though still not ideal. > > It's the best we've got. "Direct authorization" is not a grant type, but a > flow description. Fair enough. I shall remain a conscientious objector. :) > > 1.4.5: Throw a simple reference to 8.3 here? > > No forward references whenever possible. Fair style point, but I think this one is worth it. This is why our documents have hyperlinks. For each of 1.4.x, I'd still rather see the 1.4.x sections just go away. > > 2: Isn't "means" generally treated as singular in instances like this? > > Thus "means ... is" instead of "means ... are". > > Don't know. I think it is, unless someone can put a stake in to say that's wrong. > > 2.1/2.2: The requirements (2.2) should go first in section 2. The client > > types > > are part of deciding the requirements, and the concepts flow better this > > way. > > You need to first define client types before you can require it. No you don't, you just need to reference them. You already don't define the other requirements until after (such as the redirection urls). I think it reads better to have the requirements up front, when the full matter of what they're all about in the following sections. The client As it is now, there are both forward and backward references in the requirements paragraph and it's confusing to read like that. > > 2.1: I like the calling out of the types of clients, it helps cement things. > > > > 2.3: Suggest renaming to "Client Identification" to parallel "Client > > Authentication" in 2.4 > > It's not about identification. Then why is it called "Client Identifier"? Usually, one uses an identifier for identification. :) Really, this comment was about parallelizing the language in the two subsequent headers: Identification and Authentication. > > 2.4.2: Want to mention the MAC binding as an example here? This would > > parallel the OAuth2 method of signing the fetch for a request token more > > directly. > > Nah. Let me put it stronger: I would like to see an explicit reference to the MAC binding here as an example of a stronger auth binding, along with an example. OAuth1 allowed for binding against the token endpoint using a client secret that was not passed across the wire, and the MAC spec gives that capability to OAuth2. I would really like to see that. I see no problem in the core document pointing out the MAC and Bearer documents as prime examples where appropriate. -- Justin _______________________________________________ OAuth mailing list [email protected] https://www.ietf.org/mailman/listinfo/oauth
