Subject: [OAUTH-WG] Partial set of last call comments on OAuth draft 20 from Yaron Goland

From: Mike Jones <[email-masked]>
Date: Wed, 10 Aug 2011 21:38:40 +0000

1.4. Authorization Grant: Change "resource owner authorization" to "resource owner's authorization".

1.4.1. Authorization Code:  Comment: "It seems odd that we can discuss the authorization code without also discussing the security issues it raises (e.g. passing secure information via a browser) and thus motivating the introduction of the refresh token. I would expect this to be referred to here.  Having read the security considerations section I can find no coherent discussion there either of the relationship between the authorization code and the refresh token and why one motivated the other. I think this is something important for implementers to understand."

1.4.2.  Implicit:  Comment: "I find this section confusing. I think an example is all but mandatory here if the reader is to understand what is intended.  For example, when I first read this section I thought it was some kind of short cut used in cases where the client and authorization server had a close relationship and could share information such as the client's identity so no access code was needed.  No where in here is any hint that this is about browser based clients who don't have servers and who need a way to get tokens. Seriously. Try to read this section as someone who knows nothing about OAuth and tell me that you get the right idea back from it. It needs to be written to be both explicit as to its goals and clearer as to its means."

1.4.2.  Implicit:  Change "authenticate the client and the" to "authenticate the client directly. The".

1.4.2.  Implicit:  Change "weighted" to "weighed".

1.4.3.  Resource Owner Password Credentials: Comment on "(when combined with a refresh token)": "This is the first time that refresh tokens are mentioned in the spec. And yet there is no explanation of what they are. I suspect they should anyway be introduced in section 1.4.1 (as previously noted) and then their use here will make sense.  If that isn't possible then it would be good to have a forward reference to section 1.5 below so the reader has some idea of what's going on."

1.4.4.  Client Credentials:  Comment on "(the client is also the resource owner)": "I think this is misleading. Imagine something like Office where a client has been granted access to a particular resource by the resource owner. The client can then ask for an access token to the resource using the client credentials and no access code or refresh token. Just having the address of the intended resource (probably provided via SCOPE) along with the client credentials is more than enough to decide if an access token should be issued.

My point is that the client credentials scenario is fully applicable to cases where the client is not the resource owner but in which the resource URI provides all the context needed to determine if the client should be able to get an access token. I think the current text would imply otherwise.

So I would propose changing the sentence to "Client credentials are used as an authorization grant when the client is acting on its own behalf (the client is also the resource owner) or when the authorization server has enough context to determine from the access token request if the client has authorization to access the requested resource.""

1.4.5. Extensions: Comment: "I would change this sentence to read "Additional grant types may be defined to support new approaches to authentication/authorization as well as to provide a bridge between OAuth and other protocols.""

1.5. Refresh Token: Comment on "Refresh tokens are credentials used to obtain access tokens.": "This sentence presumes that refresh tokens are self-contained credentials. In other words that one can get an access token just using a refresh token and nothing else. This presumption is rebutted later in the document but I think it's confusing to imply one thing here and state otherwise later on."

1.5. Refresh Token: Change "a protected resource requests" to "a protected resource request".

1.5. Refresh Token: Comment on "(G)  The client requests a new access token by authenticating with the authorization server and presenting the refresh token.": "Hum... now the text seems to contradict itself. Above it seemed like you could use the refresh token by itself to get an access token and I had to ask for language that made it clear that you could use a refresh token with other credentials.  But the text of point G now implies the exact opposite, that refresh tokens can only be used with other credentials and not by themselves. (Even though the picture in figure 2 for step G implies the opposite). I would edit this language to say "The client requests a new access token. Depending on the authorization server's policies this request might be made with the refresh token by itself or with a combination of the refresh token and other client credentials.""

2.1. Client Types: Comment on "user-agent-based application": "Give the poor reader a hint and mention 'browser' somewhere in the paragraph below. For example "A user-agent-based application is a public client (such as a web browser) in which the....""

2.1. Client Types: web application: Change "access tokens or refresh tokens, can receive an acceptable level" to "access tokens or refresh tokens, can, in some cases, receive an acceptable level".

2.4. Client Authentication:  Add a period after "etc".

2.4.1. Client Password: Comment on "charset=UTF-8": "The use of UTF-8 with x-www-form-urlencoded is explicitly banned by HTML 4.01 section 17.13.1. If we want to use UTF-8 then we have to use multipart/form-data, also defined by HTML 4.01 (section 17.13.4). But since nobody would actually want to do that I would suggest putting in a reference to section 4.2 of RFC 5552 which actually defines how to use UTF-8 with x-www-form-urlencoded and specifying that the 5552 extension MUST be supported by OAuth participants."

3.1.  Authorization Endpoint: Comment on first sentence:  "There is a third option - none of the above.  It would be a shame if the text of this draft explicitly prohibits that pattern."

3.1.  Authorization Endpoint:  Comment on "[RFC3986] section 3": "Shouldn't we point directly to section 3.4 which exactly defines the query component?"

3.1.  Authorization Endpoint:  Comment on "MUST be retained when adding additional query
   parameters": "HIGH IMPORTANCE - This specification is incomplete. Section 3.4 of RFC 3986 simply asserts the existence of a query component. It says nothing about the syntax of that component. The spec assumes that the component is using x-www-form-urlencoded but that is not in any way mandated by RFC 3986. So there is a normative sentence missing here which states that any query parameter on the authorization endpoint's URI MUST be defined as specified in x-www-form-urlencoded."

3.1.  Authorization Endpoint:  Comment on "The authorization server SHOULD ignore unrecognized request parameters.": "Although that is normal behavior for application protocols it seems rather dangerous for a security protocol. After all what if the client put in a parameter specifying something security related about the request thinking that the authorization endpoint would honor it and then the authorization endpoint silently ignores it?  Again, this is a security protocol, not a generic application protocol, it seems to be that unrecognized parameters should cause a failure."

3.1.2.  Redirection Endpoint: Comment on "when initiating the authorization request": "I would suggest more explicit language "or as specified in the redirection URI value in the authorization request.""

3.1.2.2. Registration Requirements: Comment on last word "path": "Huh? Scheme, authorization and path is a complete URI minus a query component.  Is the goal to specifically mandate that only the query component can vary and the rest of the URI MUST be static? If so that should be stated explicitly rather than implicitly as it is now.  But I think, if that is the intent, it goes too far. We should also allow for additional path segments to be added to the URI path beyond the registered prefix. Assuming we can address the security problem in the next comment."

3.1.2.3. Dynamic Configuration: Comment on "section 6":  "The reference to section 6 is incomplete. Section 6 defines no less than 6 different axis's on which URI comparisons can vary. Furthermore as recently documented in draft-thaler-iab-identifier-comparison-00.txt it is trivially easy to screw up URI comparisons and the security implications are pretty dire. Personally I think that the only sane approach given the issues in the previous link is to adopt section 6.2.1 of RFC 3986.

I am a bit scared of how to handle partial matches. It's tempting to argue that so long as the schema has an authority and at least one path segment then we can just use a partial string match but boy oh boy do I see people screwing that one up royally. This is scary enough that I think I can be argued into saying that partial pre-registered URIs MUST only differ by the query component.

In any case this issues needs to be explicitly called out for all URI comparisons required by the spec and normatively defined. Otherwise we will end up with all the security issues in Dave's ID."

3.1.2.4.  Invalid Endpoint: Comment on "open redirector": "How many people even know what the heck an open redirector is? I think we need a section in the security considerations section that defines what an open redirector is and why it's bad. Alternatively a normative reference to a complete definition somewhere else is also fine."

3.2.1. Client Authentication: Change "Recovery" to "Recovering".

3.2.1. Client Authentication: Change "credentials, by preventing an attacker from abusing" to "credentials. This prevents an attacker from abusing"

3.2.1. Client Authentication: Change "periodic credentials rotation" to "periodic credential rotation".

3.2.1. Client Authentication: Comment on "while rotation of a single set of client credentials is significantly easier": "The spec calls out rotation of credentials as being crucial but then doesn't define how this is to be done? That seems kind of odd. Shouldn't we define an endpoint where one can submit one's old but unexpired credentials for new ones? This still leaves the edge case of what to do if the old credentials have expired and new ones haven't been issued but that is unavoidable and in that case we can require out of scope action."

4.1. Authorization Code: Comment on first "^": "Shouldn't this be a v character and not a ^? This would make it consistent with A which also crosses two boxes and points in the same direction."

4.1. Authorization Code: Change "If valid, responds back with an access token" to "If the request is valid, then the authorization server will responds back with an access token and optionally a refresh token".

4.1.2.  Authorization Response: Comment "The language around scopes in the document is really frustrating. One only finds out much later in the document that it is perfectly allowable for an authorization server to more or less ignore what scopes are submitted and instead to just return whatever the hell they want. This is a fine design choice but it seems like the sort of thing that should be forcefully repeated anywhere in the spec that we allow people to submit scopes so nobody forgets."

4.1.2.  Authorization Response: Comment on "state": "Don't we have to provide some guidance on how large the state value can safely be? Otherwise we are asking clients to rewrite their state mechanisms every time they throw in support for a new protected resource server. We have to set expectations across the industry if we are to hope for actual interoperability."

4.1.2.1. Error Response: Comment on "UTF-8 encoded text": "I think just saying UTF-8 encoded text can be misleading. It's not legal to put UTF-8 characters into a x-www-form-urlencoded used in a GET (as defined by section 17.13.1 of HTML 4.01). So what you have to do is take the UTF-8 String and then URL encode it. Which is fine but we should call this out.  Note that this is a separate issue than UTF-8 support for x-www-form-urlencoded. That issue only applies when the form is in the request body. In this scenario the form is in a URL. There is no question that URLs in HTTP request lists don't support UTF-8."

4.1.3. Access Token Request: Change "For example, the client makes the following HTTP using" to "For example, if the client makes the following HTTP request using"

4.1.3. Access Token Request: Comment on "that their values are identical": "This verbiage isn't much use, for example, if the client can always send the same redirect_uri. So, just to be clear, this text here doesn't in anyway change my previous recommendation regarding the attack previously described."

4.1.4. Access Token Response: Comment on ""token_type":"example"": "Just to prevent any confusion it would be good to actually define the token_type "example" in this document (Probably in section 7.1 and later in the IANA section) and specify that it is reserved for use in examples where one does not wish to be more specific."

4.2.  Implicit Grant: Comment "I have run into multiple people (including myself) who have read the OAuth spec and came away completely confused about when the heck one is supposed to use the implicit grant flow for. It's not immediately obvious that it's a way to support standalone browser based clients. Can we put in an example or something to make it more obvious why it's here?"

4.2.1. Authorization Request: Comment on redirect_uri:  "Given that the only way to identify the client in the implicit grant flow is via the redirect_uri, how can it be optional?"

4.3. Resource Owner Password Credentials: Change "enabling the grant type, and only when other flows" to "enabling the grant type and only use it when other flows".

4.3. Resource Owner Password Credentials: Change "resource owner credentials" to "resource owner's credentials".

4.3.2. Access Token Request:  Comment on "authorization server MUST protect the endpoint against brute force attacks": "Shouldn't the requirement to prevent against brute force attacks apply to all credentials, resource owner or otherwise? In other words in section 2.4.1 we talk about how clients submit their name/password. Shouldn't endpoints accepting client name/passwords have the same protections against brute force attack?"

4.4.3. Access Token Response: Comment on "A refresh token SHOULD NOT be included": "Why isn't this a MUST NOT?"

4.5. Extensions: Comment "Isn't the text in this section and 8.3 redundant with each other? It seems like we should take the text from here and merge it with section 8.3 so all the extension information is recorded in one place.  It's just plain that all other extension points are in section 8 but this one."

5.1. Successful Response: Comment on "parameter at the highest structure level": "Are there any guarantees about the order of the members in the JSON object? If not we should say so explicitly."

5.2. Error Response: Comment on "multiple client authentications included" in invalid_client: "Shouldn't this be an invalid_request?"

5.2. Error Response: Comment on "Numerical values are included as JSON numbers": "Same issue as before, does ordering matter and if not we need to say that."

8.1. Defining Access Token Types: Change "or use a unique" to "or using a unique".

9.  Native Applications:  Change "an scheme" to "a scheme"

9.  Native Applications:  Change "offer an improved usability" to "offer improved usability"

9.  Native Applications:  Change "inability to keep credentials confidential" to "inability to keep client credentials confidential".

9.  Native Applications:  Change "When using the implicit grant type flow a refresh token is not returned" to "When using the implicit grant type flow a refresh token is not returned so once the access token expires the application will have to repeat the authorization process".

10.2. Client Impersonation:  Change "keep is client" to "keep its client".

10.2. Client Impersonation:  Change "due to the client nature" to "due to the client's nature".

10.2. Client Impersonation:  Change "URI used for receiving authorization" to "URI used for receiving authorization requests"

10.2. Client Impersonation:  Change "engage the resource owner" to "engaging the resource owner".

10.2. Client Impersonation:  Change "and authorize the request" to "and authorize or reject the request".

10.3. Access Tokens: Change "Access token (as well as any access token type-specific attributes)" to "Access tokens (as well as any access token type specific attributes)".

10.3. Access Tokens: Change "guessed to produce" to "guessed so as to prevent unauthorized parties from producing".

10.4. Refresh Tokens: Comment "As mentioned previously we really should explain why we introduced refresh tokens. Without understanding the intent of the mechanism it's unlikely implementers will use them appropriately."

10.4. Refresh Tokens:  Change "storage, and shared only among" to "storage, and are to be shared only among".

10.4. Refresh Tokens:  Change "refresh tokens rotation" to "refresh token rotation".

10.4. Refresh Tokens:  Change "guessed to produce" to "guessed so as to prevent unauthorized parties from producing".

10.6. Authorization Code Leakage: Comment "I fancy myself as being reasonably intelligent and I'm unclear what attack is actually being described here."

10.6. Authorization Code Leakage: Comment on "The authorization server SHOULD require the client to register their redirection URI": "Why is this a should?"

10.7. Resource Owner Password Credentials: Comment on "password anti-pattern": "Is it fair to expect that the reader knows what the password anti-pattern is? Shouldn't some reference to a definition of this pattern be required?"

10.7. Resource Owner Password Credentials: Comment on "The authorization server SHOULD restrict the scope and lifetime of access tokens issued via this grant type": "Restrict in what ways? Based on what criteria? This statement is the equivalent of saying "don't do bad stuff". Perhaps true but not terribly useful."

10.12. Cross-Site Request Forgery: Comment on first paragraph: "I challenge any reasonably intelligent person who does not already know what a CSRF is to read this paragraph and come away any clearer as to what is being discussed. At a minimum isn't some reference to a complete definition of CSRF needed if there is to be any hope of user compliance?"

10.12. Cross-Site Request Forgery: Comment on "The "state" request parameter MUST contain a non-guessable value": "Actually a non-guessable value won't stop the attack discussed in the previous paragraph on its own. What's also needed is a way to uniquely (and unbreakably) associate the state with a particular user so that if an authorization code swap occurs it can be reliably detected."

10.13. Clickjacking: Comment on "x-frame-options": "The recommendation seems confused. Is it o.k. to just rely on x-frame-options or MUST other actions be taken?"

11.1.  The OAuth Access Token Type Registry: Change "toke type" to "token type".

11.1.1.  Registration Template: Change "protected resources requests using access token" to "protected resource requests using access tokens".

11.1.1.  Registration Template:  Change "Reference to document" to "Reference to the document".

________________________________

---

From: Eran Hammer-Lahav <[email-masked]>
Date: Wed, 10 Aug 2011 17:18:40 -0700

Is there a reason why Mr Goland isnt sending his own comments in?

On Aug 10, 2011, at 17:39, "Mike Jones" <[email-masked]<mailto:[email-masked]>> wrote:

1.4. Authorization Grant: Change “resource owner authorization” to “resource owner’s authorization”.

1.4.1. Authorization Code:  Comment: “It seems odd that we can discuss the authorization code without also discussing the security issues it raises (e.g. passing secure information via a browser) and thus motivating the introduction of the refresh token. I would expect this to be referred to here.  Having read the security considerations section I can find no coherent discussion there either of the relationship between the authorization code and the refresh token and why one motivated the other. I think this is something important for implementers to understand.”

1.4.2.  Implicit:  Comment: “I find this section confusing. I think an example is all but mandatory here if the reader is to understand what is intended.  For example, when I first read this section I thought it was some kind of short cut used in cases where the client and authorization server had a close relationship and could share information such as the client’s identity so no access code was needed.  No where in here is any hint that this is about browser based clients who don’t have servers and who need a way to get tokens. Seriously. Try to read this section as someone who knows nothing about OAuth and tell me that you get the right idea back from it. It needs to be written to be both explicit as to its goals and clearer as to its means.”

1.4.2.  Implicit:  Change “authenticate the client and the” to “authenticate the client directly. The”.

1.4.2.  Implicit:  Change “weighted” to “weighed”.

1.4.3.  Resource Owner Password Credentials: Comment on “(when combined with a refresh token)”: “This is the first time that refresh tokens are mentioned in the spec. And yet there is no explanation of what they are. I suspect they should anyway be introduced in section 1.4.1 (as previously noted) and then their use here will make sense.  If that isn’t possible then it would be good to have a forward reference to section 1.5 below so the reader has some idea of what’s going on.”

1.4.4.  Client Credentials:  Comment on “(the client is also the resource owner)”: “I think this is misleading. Imagine something like Office where a client has been granted access to a particular resource by the resource owner. The client can then ask for an access token to the resource using the client credentials and no access code or refresh token. Just having the address of the intended resource (probably provided via SCOPE) along with the client credentials is more than enough to decide if an access token should be issued.

So I would propose changing the sentence to “Client credentials are used as an authorization grant when the client is acting on its own behalf (the client is also the resource owner) or when the authorization server has enough context to determine from the access token request if the client has authorization to access the requested resource.””

1.4.5. Extensions: Comment: “I would change this sentence to read “Additional grant types may be defined to support new approaches to authentication/authorization as well as to provide a bridge between OAuth and other protocols.””

1.5. Refresh Token: Comment on “Refresh tokens are credentials used to obtain access tokens.”: “This sentence presumes that refresh tokens are self-contained credentials. In other words that one can get an access token just using a refresh token and nothing else. This presumption is rebutted later in the document but I think it’s confusing to imply one thing here and state otherwise later on.”

1.5. Refresh Token: Change “a protected resource requests” to “a protected resource request”.

1.5. Refresh Token: Comment on “(G)  The client requests a new access token by authenticating with the authorization server and presenting the refresh token.”: “Hum… now the text seems to contradict itself. Above it seemed like you could use the refresh token by itself to get an access token and I had to ask for language that made it clear that you could use a refresh token with other credentials.  But the text of point G now implies the exact opposite, that refresh tokens can only be used with other credentials and not by themselves. (Even though the picture in figure 2 for step G implies the opposite). I would edit this language to say “The client requests a new access token. Depending on the authorization server’s policies this request might be made with the refresh token by itself or with a combination of the refresh token and other client credentials.””

2.1. Client Types: Comment on “user-agent-based application”: “Give the poor reader a hint and mention ‘browser’ somewhere in the paragraph below. For example “A user-agent-based application is a public client (such as a web browser) in which the….””

2.1. Client Types: web application: Change “access tokens or refresh tokens, can receive an acceptable level” to “access tokens or refresh tokens, can, in some cases, receive an acceptable level”.

2.4. Client Authentication:  Add a period after “etc”.

2.4.1. Client Password: Comment on “charset=UTF-8”: “The use of UTF-8 with x-www-form-urlencoded is explicitly banned by HTML 4.01 section 17.13.1. If we want to use UTF-8 then we have to use multipart/form-data, also defined by HTML 4.01 (section 17.13.4). But since nobody would actually want to do that I would suggest putting in a reference to section 4.2 of RFC 5552 which actually defines how to use UTF-8 with x-www-form-urlencoded and specifying that the 5552 extension MUST be supported by OAuth participants.”

3.1.  Authorization Endpoint: Comment on first sentence:  “There is a third option – none of the above.  It would be a shame if the text of this draft explicitly prohibits that pattern.”

3.1.  Authorization Endpoint:  Comment on “[RFC3986] section 3”: “Shouldn’t we point directly to section 3.4 which exactly defines the query component?”

3.1.  Authorization Endpoint:  Comment on “MUST be retained when adding additional query
   parameters”: “HIGH IMPORTANCE – This specification is incomplete. Section 3.4 of RFC 3986 simply asserts the existence of a query component. It says nothing about the syntax of that component. The spec assumes that the component is using x-www-form-urlencoded but that is not in any way mandated by RFC 3986. So there is a normative sentence missing here which states that any query parameter on the authorization endpoint’s URI MUST be defined as specified in x-www-form-urlencoded.”

3.1.  Authorization Endpoint:  Comment on “The authorization server SHOULD ignore unrecognized request parameters.”: “Although that is normal behavior for application protocols it seems rather dangerous for a security protocol. After all what if the client put in a parameter specifying something security related about the request thinking that the authorization endpoint would honor it and then the authorization endpoint silently ignores it?  Again, this is a security protocol, not a generic application protocol, it seems to be that unrecognized parameters should cause a failure.”

3.1.2.  Redirection Endpoint: Comment on “when initiating the authorization request”: “I would suggest more explicit language “or as specified in the redirection URI value in the authorization request.””

3.1.2.2. Registration Requirements: Comment on last word “path”: “Huh? Scheme, authorization and path is a complete URI minus a query component.  Is the goal to specifically mandate that only the query component can vary and the rest of the URI MUST be static? If so that should be stated explicitly rather than implicitly as it is now.  But I think, if that is the intent, it goes too far. We should also allow for additional path segments to be added to the URI path beyond the registered prefix. Assuming we can address the security problem in the next comment.”

3.1.2.3. Dynamic Configuration: Comment on “section 6”:  “The reference to section 6 is incomplete. Section 6 defines no less than 6 different axis’s on which URI comparisons can vary. Furthermore as recently documented in draft-thaler-iab-identifier-comparison-00.txt it is trivially easy to screw up URI comparisons and the security implications are pretty dire. Personally I think that the only sane approach given the issues in the previous link is to adopt section 6.2.1 of RFC 3986.

I am a bit scared of how to handle partial matches. It’s tempting to argue that so long as the schema has an authority and at least one path segment then we can just use a partial string match but boy oh boy do I see people screwing that one up royally. This is scary enough that I think I can be argued into saying that partial pre-registered URIs MUST only differ by the query component.

In any case this issues needs to be explicitly called out for all URI comparisons required by the spec and normatively defined. Otherwise we will end up with all the security issues in Dave’s ID.”

3.1.2.4.  Invalid Endpoint: Comment on “open redirector”: “How many people even know what the heck an open redirector is? I think we need a section in the security considerations section that defines what an open redirector is and why it’s bad. Alternatively a normative reference to a complete definition somewhere else is also fine.”

3.2.1. Client Authentication: Change “Recovery” to “Recovering”.

3.2.1. Client Authentication: Change “credentials, by preventing an attacker from abusing” to “credentials. This prevents an attacker from abusing”

3.2.1. Client Authentication: Change “periodic credentials rotation” to “periodic credential rotation”.

3.2.1. Client Authentication: Comment on “while rotation of a single set of client credentials is significantly easier”: “The spec calls out rotation of credentials as being crucial but then doesn’t define how this is to be done? That seems kind of odd. Shouldn’t we define an endpoint where one can submit one’s old but unexpired credentials for new ones? This still leaves the edge case of what to do if the old credentials have expired and new ones haven’t been issued but that is unavoidable and in that case we can require out of scope action.”

4.1. Authorization Code: Comment on first “^”: “Shouldn’t this be a v character and not a ^? This would make it consistent with A which also crosses two boxes and points in the same direction.”

4.1. Authorization Code: Change “If valid, responds back with an access token” to “If the request is valid, then the authorization server will responds back with an access token and optionally a refresh token”.

4.1.2.  Authorization Response: Comment “The language around scopes in the document is really frustrating. One only finds out much later in the document that it is perfectly allowable for an authorization server to more or less ignore what scopes are submitted and instead to just return whatever the hell they want. This is a fine design choice but it seems like the sort of thing that should be forcefully repeated anywhere in the spec that we allow people to submit scopes so nobody forgets.”

4.1.2.  Authorization Response: Comment on “state”: “Don’t we have to provide some guidance on how large the state value can safely be? Otherwise we are asking clients to rewrite their state mechanisms every time they throw in support for a new protected resource server. We have to set expectations across the industry if we are to hope for actual interoperability.”

4.1.2.1. Error Response: Comment on “UTF-8 encoded text”: “I think just saying UTF-8 encoded text can be misleading. It’s not legal to put UTF-8 characters into a x-www-form-urlencoded used in a GET (as defined by section 17.13.1 of HTML 4.01). So what you have to do is take the UTF-8 String and then URL encode it. Which is fine but we should call this out.  Note that this is a separate issue than UTF-8 support for x-www-form-urlencoded. That issue only applies when the form is in the request body. In this scenario the form is in a URL. There is no question that URLs in HTTP request lists don’t support UTF-8.”

4.1.3. Access Token Request: Change “For example, the client makes the following HTTP using” to “For example, if the client makes the following HTTP request using”

4.1.3. Access Token Request: Comment on “that their values are identical”: “This verbiage isn’t much use, for example, if the client can always send the same redirect_uri. So, just to be clear, this text here doesn’t in anyway change my previous recommendation regarding the attack previously described.”

4.1.4. Access Token Response: Comment on “"token_type":"example"”: “Just to prevent any confusion it would be good to actually define the token_type “example” in this document (Probably in section 7.1 and later in the IANA section) and specify that it is reserved for use in examples where one does not wish to be more specific.”

4.2.  Implicit Grant: Comment “I have run into multiple people (including myself) who have read the OAuth spec and came away completely confused about when the heck one is supposed to use the implicit grant flow for. It’s not immediately obvious that it’s a way to support standalone browser based clients. Can we put in an example or something to make it more obvious why it’s here?”

4.2.1. Authorization Request: Comment on redirect_uri:  “Given that the only way to identify the client in the implicit grant flow is via the redirect_uri, how can it be optional?”

4.3. Resource Owner Password Credentials: Change “enabling the grant type, and only when other flows” to “enabling the grant type and only use it when other flows”.

4.3. Resource Owner Password Credentials: Change “resource owner credentials” to “resource owner’s credentials”.

4.3.2. Access Token Request:  Comment on “authorization server MUST protect the endpoint against brute force attacks”: “Shouldn’t the requirement to prevent against brute force attacks apply to all credentials, resource owner or otherwise? In other words in section 2.4.1 we talk about how clients submit their name/password. Shouldn’t endpoints accepting client name/passwords have the same protections against brute force attack?”

4.4.3. Access Token Response: Comment on “A refresh token SHOULD NOT be included”: “Why isn’t this a MUST NOT?”

4.5. Extensions: Comment “Isn’t the text in this section and 8.3 redundant with each other? It seems like we should take the text from here and merge it with section 8.3 so all the extension information is recorded in one place.  It’s just plain that all other extension points are in section 8 but this one.”

5.1. Successful Response: Comment on “parameter at the highest structure level”: “Are there any guarantees about the order of the members in the JSON object? If not we should say so explicitly.”

5.2. Error Response: Comment on “multiple client authentications included” in invalid_client: “Shouldn’t this be an invalid_request?”

5.2. Error Response: Comment on “Numerical values are included as JSON numbers”: “Same issue as before, does ordering matter and if not we need to say that.”

8.1. Defining Access Token Types: Change “or use a unique” to “or using a unique”.

9.  Native Applications:  Change “an scheme” to “a scheme”

9.  Native Applications:  Change “offer an improved usability” to “offer improved usability”

9.  Native Applications:  Change “inability to keep credentials confidential” to “inability to keep client credentials confidential”.

9.  Native Applications:  Change “When using the implicit grant type flow a refresh token is not returned” to “When using the implicit grant type flow a refresh token is not returned so once the access token expires the application will have to repeat the authorization process”.

10.2. Client Impersonation:  Change “keep is client” to “keep its client”.

10.2. Client Impersonation:  Change “due to the client nature” to “due to the client’s nature”.

10.2. Client Impersonation:  Change “URI used for receiving authorization” to “URI used for receiving authorization requests”

10.2. Client Impersonation:  Change “engage the resource owner” to “engaging the resource owner”.

10.2. Client Impersonation:  Change “and authorize the request” to “and authorize or reject the request”.

10.3. Access Tokens: Change “Access token (as well as any access token type-specific attributes)” to “Access tokens (as well as any access token type specific attributes)”.

10.3. Access Tokens: Change “guessed to produce” to “guessed so as to prevent unauthorized parties from producing”.

10.4. Refresh Tokens: Comment “As mentioned previously we really should explain why we introduced refresh tokens. Without understanding the intent of the mechanism it’s unlikely implementers will use them appropriately.”

10.4. Refresh Tokens:  Change “storage, and shared only among” to “storage, and are to be shared only among”.

10.4. Refresh Tokens:  Change “refresh tokens rotation” to “refresh token rotation”.

10.4. Refresh Tokens:  Change “guessed to produce” to “guessed so as to prevent unauthorized parties from producing”.

10.6. Authorization Code Leakage: Comment “I fancy myself as being reasonably intelligent and I’m unclear what attack is actually being described here.”

10.6. Authorization Code Leakage: Comment on “The authorization server SHOULD require the client to register their redirection URI”: “Why is this a should?”

10.7. Resource Owner Password Credentials: Comment on “password anti-pattern”: “Is it fair to expect that the reader knows what the password anti-pattern is? Shouldn’t some reference to a definition of this pattern be required?”

10.7. Resource Owner Password Credentials: Comment on “The authorization server SHOULD restrict the scope and lifetime of access tokens issued via this grant type”: “Restrict in what ways? Based on what criteria? This statement is the equivalent of saying “don’t do bad stuff”. Perhaps true but not terribly useful.”

10.12. Cross-Site Request Forgery: Comment on first paragraph: “I challenge any reasonably intelligent person who does not already know what a CSRF is to read this paragraph and come away any clearer as to what is being discussed. At a minimum isn’t some reference to a complete definition of CSRF needed if there is to be any hope of user compliance?”

10.12. Cross-Site Request Forgery: Comment on “The "state" request parameter MUST contain a non-guessable value”: “Actually a non-guessable value won’t stop the attack discussed in the previous paragraph on its own. What’s also needed is a way to uniquely (and unbreakably) associate the state with a particular user so that if an authorization code swap occurs it can be reliably detected.”

10.13. Clickjacking: Comment on “x-frame-options”: “The recommendation seems confused. Is it o.k. to just rely on x-frame-options or MUST other actions be taken?”

11.1.  The OAuth Access Token Type Registry: Change “toke type” to “token type”.

11.1.1.  Registration Template: Change “protected resources requests using access token” to “protected resource requests using access tokens”.

11.1.1.  Registration Template:  Change “Reference to document” to “Reference to the document”.

________________________________
_______________________________________________
OAuth mailing list
[email-masked]<mailto:[email-masked]>
https://www.ietf.org/mailman/listinfo/oauth

---

From: Anthony Nadalin <[email-masked]>
Date: Thu, 11 Aug 2011 14:58:00 +0000

He is out on vacation with no access and he left Mike with a bunch of comments to pull together to submit and we knew that you wanted these comments to ensure that the spec is the best it can be.

From: [email-masked] [mailto:[email-masked]] On Behalf Of Eran Hammer-Lahav
Sent: Wednesday, August 10, 2011 5:19 PM
To: Mike Jones
Cc: [email-masked]
Subject: Re: [OAUTH-WG] Partial set of last call comments on OAuth draft 20 from Yaron Goland

---

From: Eran Hammer-Lahav <[email-masked]>
Date: Thu, 11 Aug 2011 12:14:48 -0700

Just want to make sure it is covered by the normal contribution rule.

On Aug 11, 2011, at 10:58, "Anthony Nadalin" <[email-masked]<mailto:[email-masked]>> wrote:

From: [email-masked]<mailto:[email-masked]> [mailto:[email-masked]] On Behalf Of Eran Hammer-Lahav
Sent: Wednesday, August 10, 2011 5:19 PM
To: Mike Jones
Cc: <mailto:[email-masked]> [email-masked]<mailto:[email-masked]>
Subject: Re: [OAUTH-WG] Partial set of last call comments on OAuth draft 20 from Yaron Goland

On Aug 10, 2011, at 17:39, "Mike Jones" <<mailto:[email-masked]>[email-masked]<mailto:[email-masked]>> wrote:
1.4. Authorization Grant: Change “resource owner authorization” to “resource owner’s authorization”.

_______________________________________________
OAuth mailing list
<mailto:[email-masked]>[email-masked]<mailto:[email-masked]>
<https://www.ietf.org/mailman/listinfo/oauth>https://www.ietf.org/mailman/listinfo/oauth

---

From: Anthony Nadalin <[email-masked]>
Date: Thu, 11 Aug 2011 19:18:21 +0000

Yep, I believe they are

From: Eran Hammer-Lahav [mailto:[email-masked]]
Sent: Thursday, August 11, 2011 12:15 PM
To: Anthony Nadalin
Cc: Mike Jones; [email-masked]
Subject: Re: [OAUTH-WG] Partial set of last call comments on OAuth draft 20 from Yaron Goland

From: [email-masked]<mailto:[email-masked]> [mailto:[email-masked]]<mailto:[mailto:[email-masked]]> On Behalf Of Eran Hammer-Lahav
Sent: Wednesday, August 10, 2011 5:19 PM
To: Mike Jones
Cc: [email-masked]<mailto:[email-masked]>
Subject: Re: [OAUTH-WG] Partial set of last call comments on OAuth draft 20 from Yaron Goland

---

From: Eran Hammer-Lahav <[email-masked]>
Date: Thu, 11 Aug 2011 23:16:53 -0700

The following comments require a proposed text. Without it, they will not be included in my next editorial revision. I will respond or incorporate the rest of the comments after the close of LC. Others are welcomed and encouraged to discuss this (very useful and thorough) list before LC ends.

EHL

From: Mike Jones <[email-masked]<mailto:[email-masked]>>
Date: Wed, 10 Aug 2011 14:38:40 -0700
To: "[email-masked]<mailto:[email-masked]>" <[email-masked]<mailto:[email-masked]>>
Subject: [OAUTH-WG] Partial set of last call comments on OAuth draft 20 from Yaron Goland

---

From: Eran Hammer-Lahav <[email-masked]>
Date: Wed, 17 Aug 2011 00:06:06 -0700

This is fantastic feedback. Some parts moved to new threads.

> -----Original Message-----
> From: [email-masked] [mailto:[email-masked]] On Behalf
> Of Mike Jones
> Sent: Wednesday, August 10, 2011 2:39 PM

> "resource owner's authorization".

Ok.

> 1.4.1. Authorization Code:  Comment: "It seems odd that we can discuss the
> authorization code without also discussing the security issues it raises (e.g.
> passing secure information via a browser) and thus motivating the
> introduction of the refresh token. I would expect this to be referred to
> here.  Having read the security considerations section I can find no coherent
> discussion there either of the relationship between the authorization code
> and the refresh token and why one motivated the other. I think this is
> something important for implementers to understand."

This particular relationship has not been discussed on the list before.

The primary justifications of refresh tokens were the need to allow access token rotations because of the weaker nature of bearer tokens, and to support large scale deployments in which access tokens are self-contained and do not require a DB lookup (which implies short-lived, similar to self-contained session cookies).

If we are going to add a discussion, it should cover these as well, however, such a discussion really belongs in the threat model document. We have not included any other motivations in the document and I don't feel this justifies an exception.

Either way, please provide text and we can decide where it belongs.

> 1.4.2.  Implicit:  Comment: "I find this section confusing. I think an example is
> all but mandatory here if the reader is to understand what is intended.  For
> example, when I first read this section I thought it was some kind of short cut
> used in cases where the client and authorization server had a close
> relationship and could share information such as the client's identity so no
> access code was needed.  No where in here is any hint that this is about
> browser based clients who don't have servers and who need a way to get
> tokens. Seriously. Try to read this section as someone who knows nothing
> about OAuth and tell me that you get the right idea back from it. It needs to
> be written to be both explicit as to its goals and clearer as to its means."

Changed first paragraph to:

            The implicit grant is a simplified authorization code flow optimized for clients
            implemented in a browser using a scripting language such as JavaScript. In the implicit
            flow, instead of issuing the client an authorization code, the client is issued an
            access token directly (as the result of the resource owner authorization). The grant
            type is implicit as no intermediate credentials (such as an authorization code) are
            issued (and later used to obtain an access token).

> 1.4.2.  Implicit:  Change "authenticate the client and the" to "authenticate the

Text already changed.

> 1.4.2.  Implicit:  Change "weighted" to "weighed".

Ok.

> 1.4.3.  Resource Owner Password Credentials: Comment on "(when
> combined with a refresh token)": "This is the first time that refresh tokens
> are mentioned in the spec. And yet there is no explanation of what they are.
> I suspect they should anyway be introduced in section 1.4.1 (as previously
> noted) and then their use here will make sense.  If that isn't possible then it
> would be good to have a forward reference to section 1.5 below so the
> reader has some idea of what's going on."

I removed '(when combined with a refresh token)'. This is actually not true as there is no assumption that access tokens are always short-lived or that refresh tokens will always be issued to native applications using this grant type.

> 1.4.4.  Client Credentials:  Comment on "(the client is also the resource
> owner)": "I think this is misleading. Imagine something like Office where a
> client has been granted access to a particular resource by the resource
> owner. The client can then ask for an access token to the resource using the
> client credentials and no access code or refresh token. Just having the
> address of the intended resource (probably provided via SCOPE) along with
> the client credentials is more than enough to decide if an access token should
> be issued.
>
> My point is that the client credentials scenario is fully applicable to cases
> where the client is not the resource owner but in which the resource URI
> provides all the context needed to determine if the client should be able to
> get an access token. I think the current text would imply otherwise.
>
> So I would propose changing the sentence to "Client credentials are used as
> an authorization grant when the client is acting on its own behalf (the client is
> also the resource owner) or when the authorization server has enough
> context to determine from the access token request if the client has
> authorization to access the requested resource.""

Added:

"or is requesting access to protected resources based on an authorization previously arranged with the authorization server."

To make is more in line with the existing language of 4.4.

> 1.4.5. Extensions: Comment: "I would change this sentence to read
> "Additional grant types may be defined to support new approaches to
> authentication/authorization as well as to provide a bridge between OAuth
> and other protocols.""

Took out the entire section. The introduction to 1.4 already contained all the relevant information.

> 1.5. Refresh Token: Change "a protected resource requests" to "a protected

Ok.

> 1.5. Refresh Token: Comment on "Refresh tokens are credentials used to
> obtain access tokens.": "This sentence presumes that refresh tokens are
> self-contained credentials. In other words that one can get an access token
> just using a refresh token and nothing else. This presumption is rebutted
> later in the document but I think it's confusing to imply one thing here and
> state otherwise later on."

> 1.5. Refresh Token: Comment on "(G)  The client requests a new access
> token by authenticating with the authorization server and presenting the
> refresh token.": "Hum... now the text seems to contradict itself. Above it
> seemed like you could use the refresh token by itself to get an access token
> and I had to ask for language that made it clear that you could use a refresh
> token with other credentials.  But the text of point G now implies the exact
> opposite, that refresh tokens can only be used with other credentials and
> not by themselves. (Even though the picture in figure 2 for step G implies the
> opposite). I would edit this language to say "The client requests a new access
> token. Depending on the authorization server's policies this request might be
> made with the refresh token by itself or with a combination of the refresh
> token and other client credentials.""

Added to (G): "The client authentication requirements are based on the client type and on the authorization server policies."

> 2.1. Client Types: Comment on "user-agent-based application": "Give the
> poor reader a hint and mention 'browser' somewhere in the paragraph
> below. For example "A user-agent-based application is a public client (such as
> a web browser) in which the....""

Ok.

> 2.1. Client Types: web application: Change "access tokens or refresh tokens,
> can receive an acceptable level" to "access tokens or refresh tokens, can, in
> some cases, receive an acceptable level".

This doesn't really add anything.

> 2.4. Client Authentication:  Add a period after "etc".

Huh?

> 2.4.1. Client Password: Comment on "charset=UTF-8": "The use of UTF-8 with
> x-www-form-urlencoded is explicitly banned by HTML 4.01 section 17.13.1. If
> we want to use UTF-8 then we have to use multipart/form-data, also defined
> by HTML 4.01 (section 17.13.4). But since nobody would actually want to do
> that I would suggest putting in a reference to section 4.2 of RFC 5552 which
> actually defines how to use UTF-8 with x-www-form-urlencoded and
> specifying that the 5552 extension MUST be supported by OAuth
> participants."

http://www.w3.org/TR/html401/interact/forms.html#h-17.13.1

Where?

> 3.1.  Authorization Endpoint: Comment on first sentence:  "There is a third
> option - none of the above.  It would be a shame if the text of this draft
> explicitly prohibits that pattern."

Dropped "which is expressed explicitly as an authorization code (later exchanged for an access token), or implicitly by direct issuance of an access token" as it is no longer accurate given the new response type extensibility. Either way, section 3.1.1 covers this.

> 3.1.  Authorization Endpoint:  Comment on "[RFC3986] section 3": "Shouldn't
> we point directly to section 3.4 which exactly defines the query component?"

Ok.

>    parameters": "HIGH IMPORTANCE - This specification is incomplete.
> Section 3.4 of RFC 3986 simply asserts the existence of a query component. It
> says nothing about the syntax of that component. The spec assumes that the
> component is using x-www-form-urlencoded but that is not in any way
> mandated by RFC 3986. So there is a normative sentence missing here which
> states that any query parameter on the authorization endpoint's URI MUST
> be defined as specified in x-www-form-urlencoded."

Changed to:

   The endpoint URI MAY include an "application/x-www-form-urlencoded"
   formatted ([W3C.REC-html401-19991224]) query component ([RFC3986]
   section 3.4), which MUST be retained when adding additional query
   parameters.  The endpoint URI MUST NOT include a fragment component.

> 3.1.  Authorization Endpoint:  Comment on "The authorization server
> SHOULD ignore unrecognized request parameters.": "Although that is normal
> behavior for application protocols it seems rather dangerous for a security
> protocol. After all what if the client put in a parameter specifying something
> security related about the request thinking that the authorization endpoint
> would honor it and then the authorization endpoint silently ignores it?  Again,
> this is a security protocol, not a generic application protocol, it seems to be
> that unrecognized parameters should cause a failure."

This is an issue for those defining additional parameters. Basically, you should not define new parameters that when not-understood cause degradation of the base protocol security.

> 3.1.2.  Redirection Endpoint: Comment on "when initiating the authorization
> request": "I would suggest more explicit language "or as specified in the
> redirection URI value in the authorization request.""

Changed to "during the client registration process or when making the authorization request".

> 3.2.1. Client Authentication: Change "Recovery" to "Recovering".

Ok.

> 3.2.1. Client Authentication: Change "credentials, by preventing an attacker
> from abusing" to "credentials. This prevents an attacker from abusing"

Changed 'by' to 'thus'.

> "periodic credential rotation".

Ok.

> 3.2.1. Client Authentication: Comment on "while rotation of a single set of
> client credentials is significantly easier": "The spec calls out rotation of
> credentials as being crucial but then doesn't define how this is to be done?
> That seems kind of odd. Shouldn't we define an endpoint where one can
> submit one's old but unexpired credentials for new ones? This still leaves the
> edge case of what to do if the old credentials have expired and new ones
> haven't been issued but that is unavoidable and in that case we can require
> out of scope action."

The spec leaves much out of scope. You are welcome to submit a proposal for such a new endpoint.

> 4.1. Authorization Code: Comment on first "^": "Shouldn't this be a v
> character and not a ^? This would make it consistent with A which also
> crosses two boxes and points in the same direction."

(B) is bi-directional (server serves a login page, user enter password, fetches another page...).

> 4.1. Authorization Code: Change "If valid, responds back with an access
> token" to "If the request is valid, then the authorization server will responds
> back with an access token and optionally a refresh token".

Ok.

> 4.1.2.  Authorization Response: Comment on "state": "Don't we have to
> provide some guidance on how large the state value can safely be?
> Otherwise we are asking clients to rewrite their state mechanisms every time
> they throw in support for a new protected resource server. We have to set
> expectations across the industry if we are to hope for actual
> interoperability."

Need proposed text.

> 4.1.2.1. Error Response: Comment on "UTF-8 encoded text": "I think just
> saying UTF-8 encoded text can be misleading. It's not legal to put UTF-8
> characters into a x-www-form-urlencoded used in a GET (as defined by
> section 17.13.1 of HTML 4.01). So what you have to do is take the UTF-8 String
> and then URL encode it. Which is fine but we should call this out.  Note that
> this is a separate issue than UTF-8 support for x-www-form-urlencoded. That
> issue only applies when the form is in the request body. In this scenario the
> form is in a URL. There is no question that URLs in HTTP request lists don't
> support UTF-8."

Whatever value goes into the parameter has to be form-encoded. I think that's pretty clear throughout the specification. We don't put any restrictions on most values we expect to be delivered using form-encoded strings. The expectation is that implementers will use a library that takes some structure with parameter names and values and turn it into a valid URI or body.

> 4.1.3. Access Token Request: Change "For example, the client makes the
> following HTTP using" to "For example, if the client makes the following HTTP
> request using"

Already fixed.

> 4.1.3. Access Token Request: Comment on "that their values are identical":
> "This verbiage isn't much use, for example, if the client can always send the
> same redirect_uri. So, just to be clear, this text here doesn't in anyway
> change my previous recommendation regarding the attack previously
> described."

Huh?

> 4.1.4. Access Token Response: Comment on ""token_type":"example"":
> "Just to prevent any confusion it would be good to actually define the
> token_type "example" in this document (Probably in section 7.1 and later in
> the IANA section) and specify that it is reserved for use in examples where
> one does not wish to be more specific."

Added to 8.1: "The token type 'example' is reserved for use in examples."

> 4.2.  Implicit Grant: Comment "I have run into multiple people (including
> myself) who have read the OAuth spec and came away completely confused
> about when the heck one is supposed to use the implicit grant flow for. It's
> not immediately obvious that it's a way to support standalone browser based
> clients. Can we put in an example or something to make it more obvious why
> it's here?"

I think the text is pretty clear:

   These clients are typically implemented in a browser using a scripting language
   such as JavaScript.

But I'm open to suggestions. Please propose text.

> 4.2.1. Authorization Request: Comment on redirect_uri:  "Given that the only
> way to identify the client in the implicit grant flow is via the redirect_uri, how
> can it be optional?"

client_id is required.

> 4.3. Resource Owner Password Credentials: Change "enabling the grant type,
> and only when other flows" to "enabling the grant type and only use it when
> other flows".

Ok.

> 4.3. Resource Owner Password Credentials: Change "resource owner
> credentials" to "resource owner's credentials".

Ok.

> 4.3.2. Access Token Request:  Comment on "authorization server MUST
> protect the endpoint against brute force attacks": "Shouldn't the
> requirement to prevent against brute force attacks apply to all credentials,
> resource owner or otherwise? In other words in section 2.4.1 we talk about
> how clients submit their name/password. Shouldn't endpoints accepting
> client name/passwords have the same protections against brute force
> attack?"

Yes, but in general brute force attacks are more likely on human-friendly passwords than machine-issued client credentials. I'll add it to 2.4.1:

            Since this client authentication method involves a password, the authorization server
            MUST protect any endpoint utilizing it against brute force attacks.

> 4.4.3. Access Token Response: Comment on "A refresh token SHOULD NOT
> be included": "Why isn't this a MUST NOT?"

Working group consensus. It was discussed and some people expressed use cases for allowing it.

> 4.5. Extensions: Comment "Isn't the text in this section and 8.3 redundant
> with each other? It seems like we should take the text from here and merge
> it with section 8.3 so all the extension information is recorded in one
> place.  It's just plain that all other extension points are in section 8 but this
> one."

This one is about usage white the others are about registration. This is the right place to inform developers of the need to handle URI values.

> 5.1. Successful Response: Comment on "parameter at the highest structure
> level": "Are there any guarantees about the order of the members in the
> JSON object? If not we should say so explicitly."

Added: "The order of parameters does not matter and can vary."

> 5.2. Error Response: Comment on "multiple client authentications included"
> in invalid_client: "Shouldn't this be an invalid_request?"

Yep.

> 5.2. Error Response: Comment on "Numerical values are included as JSON
> numbers": "Same issue as before, does ordering matter and if not we need
> to say that."

Same text added.

> 8.1. Defining Access Token Types: Change "or use a unique" to "or using a

Ok.

> 9.  Native Applications:  Change "an scheme" to "a scheme"

Ok.

> 9.  Native Applications:  Change "offer an improved usability" to "offer

Ok.

> 9.  Native Applications:  Change "inability to keep credentials confidential" to
> "inability to keep client credentials confidential".

Ok.

> 9.  Native Applications:  Change "When using the implicit grant type flow a
> refresh token is not returned" to "When using the implicit grant type flow a
> refresh token is not returned so once the access token expires the
> application will have to repeat the authorization process".

Ok.

> 10.2. Client Impersonation:  Change "keep is client" to "keep its client".

Already fixed.

> 10.2. Client Impersonation:  Change "due to the client nature" to "due to the

Ok.

> 10.2. Client Impersonation:  Change "URI used for receiving authorization" to
> "URI used for receiving authorization requests"

Actually, responses.

> "engaging the resource owner".

Ok.

> authorize or reject the request".

Ok.

> 10.3. Access Tokens: Change "Access token (as well as any access token type-
> specific attributes)" to "Access tokens (as well as any access token type
> specific attributes)".

Nah. I like the -.

> 10.3. Access Tokens: Change "guessed to produce" to "guessed so as to
> prevent unauthorized parties from producing".

Ok.

> 10.4. Refresh Tokens: Comment "As mentioned previously we really should
> explain why we introduced refresh tokens. Without understanding the
> intent of the mechanism it's unlikely implementers will use them
> appropriately."

Disagree. Feel free to propose text.

> and are to be shared only among".

Nah.

> 10.4. Refresh Tokens:  Change "refresh tokens rotation" to "refresh token

Ok.

> 10.4. Refresh Tokens:  Change "guessed to produce" to "guessed so as to
> prevent unauthorized parties from producing".

Ok.

> 10.7. Resource Owner Password Credentials: Comment on "password anti-
> pattern": "Is it fair to expect that the reader knows what the password anti-
> pattern is? Shouldn't some reference to a definition of this pattern be
> required?"

In this case, lack of understanding does not take away from the rest of the prose and a reader can look it up. It is a well-established term and the reason for OAuth in the first place. But feel free to propose text.

> 10.7. Resource Owner Password Credentials: Comment on "The
> authorization server SHOULD restrict the scope and lifetime of access tokens
> issued via this grant type": "Restrict in what ways? Based on what criteria?
> This statement is the equivalent of saying "don't do bad stuff". Perhaps true
> but not terribly useful."

Yeah... especially with a normative SHOULD. Changed 'restrict' to 'consider' and lowercased the should (I know it doesn't really matter but it reads better now):

         The authorization server should consider the scope
         and lifetime of access tokens issued via this grant type.

> 10.12. Cross-Site Request Forgery: Comment on first paragraph: "I challenge
> any reasonably intelligent person who does not already know what a CSRF is
> to read this paragraph and come away any clearer as to what is being
> discussed. At a minimum isn't some reference to a complete definition of
> CSRF needed if there is to be any hope of user compliance?"

It seems we are still discussing this section on another thread. But generally, we cannot be expected to provide a comprehensive web security guide within this protocol. The attack is presented in very general terms and the reader should go and study it if they are not familiar with it. I think the extra prose in the beginning doesn't hurt but I don't mind dropping it and just starting with "A cross-site request forgery (CSRF) allow an attacker to inject..." and drop the entire first paragraph.

No change for now.

> 10.12. Cross-Site Request Forgery: Comment on "The "state" request
> parameter MUST contain a non-guessable value": "Actually a non-guessable
> value won't stop the attack discussed in the previous paragraph on its own.
> What's also needed is a way to uniquely (and unbreakably) associate the
> state with a particular user so that if an authorization code swap occurs it can
> be reliably detected."

Discussed on another thread.

> 10.13. Clickjacking: Comment on "x-frame-options": "The recommendation
> seems confused. Is it o.k. to just rely on x-frame-options or MUST other
> actions be taken?"

Seems pretty clear to me. There is no full solution, but using the header is one way to prevent newer browsers from allowing such an attack. Can't put a MUST here because there is no way to completely close this attack vector.

> 11.1.  The OAuth Access Token Type Registry: Change "toke type" to "token

Ok.

> 11.1.1.  Registration Template: Change "protected resources requests using
> access token" to "protected resource requests using access tokens".

Ok.

> "Reference to the document".

Ok.

Thanks!

EHL

---

From: "Lodderstedt, Torsten" <[email-masked]>
Date: Thu, 18 Aug 2011 10:01:04 +0200

>I removed '(when combined with a refresh token)'. This is actually not true as there is no assumption that >access tokens are always short-lived or that refresh tokens will always be issued to native applications using >this grant type.

-1 against removing this text (w/o an suitable replacement) and w/o group consent. 

The -20 text clearly points out that this combination "... eliminates the need for the client to store the resource owner credentials for future use". The resource owner grant type alone does not justify this statement.
It's true that the spec does not explicitly defines the lifetime assumption for access and refresh tokens (which is pity in my opinion). So at least add something like "if the token lifetime is reasonable long enough".

regards,
Torsten.

---

From: Anthony Nadalin <[email-masked]>
Date: Thu, 18 Aug 2011 16:58:10 +0000

Agree,  against the removal of text

-----Original Message-----
From: [email-masked] [mailto:[email-masked]] On Behalf Of Lodderstedt, Torsten
Sent: Thursday, August 18, 2011 1:01 AM
To: Eran Hammer-Lahav; [email-masked]
Subject: Re: [OAUTH-WG] Partial set of last call comments on OAuth draft 20 from Yaron Goland

>> combined with a refresh token)": "This is the first time that refresh 
>> tokens are mentioned in the spec. And yet there is no explanation of what they are.
>> noted) and then their use here will make sense.  If that isn't 
>> possible then it would be good to have a forward reference to section 
>> 1.5 below so the reader has some idea of what's going on."

regards,
Torsten.
[email-masked]

---

From: Igor Faynberg <[email-masked]>
Date: Thu, 18 Aug 2011 13:16:40 -0400

+1  (against the removal)

On 8/18/2011 12:58 PM, Anthony Nadalin wrote:
>> I removed '(when combined with a refresh token)'. This is actually not true as there is no assumption that>access tokens are always short-lived or that refresh tokens will always be issued to native applications using>this grant type.

---

From: Eran Hammer-Lahav <[email-masked]>
Date: Thu, 18 Aug 2011 10:20:11 -0700

Chairs - please open an issue for this: "Clarifying reference to refresh tokens in section 1.4.3 of -20".

> From: Lodderstedt, Torsten [mailto:[email-masked]]
> Subject: AW: [OAUTH-WG] Partial set of last call comments on OAuth draft 20
> from Yaron Goland
> 
> >I removed '(when combined with a refresh token)'. This is actually not true
> as there is no assumption that >access tokens are always short-lived or that
> refresh tokens will always be issued to native applications using >this grant
> type.
> 

Since you felt it necessary to make this a process issue, I've asked the chairs to open a ticket.

> The -20 text clearly points out that this combination "... eliminates the need
> for the client to store the resource owner credentials for future use". The
> resource owner grant type alone does not justify this statement.
> It's true that the spec does not explicitly defines the lifetime assumption for
> access and refresh tokens (which is pity in my opinion). So at least add
> something like "if the token lifetime is reasonable long enough".

How about:

   This grant type can eliminates the need for the client to store the resource owner
   credentials for future use, by exchanging the credentials with a long-lived access
   token or refresh token.

As for Yaron's original comment, I think this text is sufficient and no forward reference is needed to 1.5 (which is a few lines lower in the same page). Also, with the new organization proposed by Justin, the access token term isn't full defined yet either and it reads just fine.

EHL

---

From: Barry Leiba <[email-masked]>
Date: Thu, 18 Aug 2011 14:26:59 -0400

> in section 1.4.3 of -20".

http://trac.tools.ietf.org/wg/oauth/trac/ticket/25

b