Subject: [OAUTH-WG] Feedback on draft-ietf-oauth-v2-13

From: Mike Jones <[email-masked]>
Date: Tue, 15 Mar 2011 14:52:17 +0000

My last call feedback on draft 13 follows.

NORMATIVE FEEDBACK

2.1.1:  "If no valid redirection URI is available, the authorization server SHOULD" - I don't understand why this is a SHOULD and not a MUST

3:  Restore Client Assertion Credentials

Restore the Client Assertion Credentials feature that was present in draft 11, section 3.2.

4.1.1:  Scope string matching rules are ambiguous

In the "scope" definition, add "The space-delimited strings are case insensitive."

4.1, 4.2, 4.3.2, 4.4.2, 5.1, 6:  "Scope" parameter should be paired with complimentary "resource" parameter

It is desirable to be able to have a single service protecting multiple resources.  To achieve that, there needs to be a parameter specifying what the desired resource being accessed is.  This is different than scope.  At least as we're using it, "scope" would be a space-separated list of kinds of access requested.  For instance you might be requesting read access to someone's calendar free/busy times and the right to post new calendar requests.  Those would be scope statements and would use URIs specifying those rights.

Therefore, we request that an additional optional "resource" parameter be added to the specification in the same places that "scope" appears.  "Resource" would be the URI (probably a URL) of the resource being protected by the service.

4.2.2:  Add optional "expires_at" parameter

SAML, WS-Trust, JWT, and several other specifications use absolute, rather than relative expiration times.  Please add an optional "expires_at" parameter, enabling absolute times to be used.  The value should be an integer specifying the number of seconds from 1970-01-01T0:0:0Z as measured in UTC until the desired date/time. See RFC 3339 for details regarding date/times in general and UTC in particular.

4.4.2 etc.:  The name "client_credentials" is confusing

The name client_credentials does not refer to the same concept as the uses of the term "Client Credentials" in 1.4.4 and other locations in the document.  It would be far better to rename this parameter "none" or "implicit", to indicate that no explicit credentials are being passed in the protocol.  It might also clarify this concept if you added an example.

7:  Restore WWW-Authenticate Response Header Field

Restore the WWW-Authenticate Response Header Field that was present in draft 11, section 6.2.

10:  Add OAuth Errors Registry

Add the OAuth Errors Registry from the bearer token specification, draft 03, section 4.3, to the IANA Considerations.

10.2:  Add parameter locations to OAuth Parameters Registry

Add the "resource request" and "resource response" parameter locations, as defined in the bearer token specification, draft 03, section 4.2, to the OAuth Parameters Registry.

EDITORIAL FEEDBACK

1.2 item (D) :  "and if valid issues a access token" -> "and if valid, issues an access token and optional refresh token"
1.3:  "an access token is a string" -> "an access token is a string encoded"
1.4.5:  "OAuth and other trust frameworks" -> "OAuth" - The term "trust framework" seems to have taken on a different meaning than this one these days.
1.5:  "used to obtain a new access token when the current access token expires" -> "used to obtain a new access token when the current access token is no longer valid".  "expires" is just one case.
1.5 item (F) :  "access token is invalid (expired)" -> "access token is invalid (e.g., expired)"
2.1.1:  "resource owner's user-agent" this may not be possible if the resource owner is a device or if the resource owner does not have a user agent
2.1.1:  "which MUST be retained" -> "which MUST be preserved"
2.2:  "The location of the token endpoint can be found in the service documentation" - There is no service documentation; suggest that this just say "The location of the token endpoint discovery is out of scope of this document"
3:  "keeping their secrets confidential" -> "keeping their credentials confidential"
4:  "which the client uses to requesting the access" -> "which the client uses to request the access"
4.1:  "with the resource owner's user-agent" this may not be possible if the resource owner is a device or if the resource owner does not have a user agent
4.1.1:  "parameters are present and valid" - there are no processing requirements section to determine what "valid" means; suggest that a processing section be added to understand what "valid" means otherwise remove the references to "valid"
4.1.3:  Usage of "verify" and "validate", should consistent. "Validate the client credentials and ensure they match the authorization code" this is misleading and should read "Validate the client credentials and ensure that the authorization code given is authorized"
4.1.4:  "the authorization server return" -> "the authorization server returns"
4.1.2.1, 4.2.2, 4.2.2.1:  List REQUIRED parameters before OPTIONAL parameters.
4.2:  It's unclear what the actual meaning of "user-agent's same-origin policy" really is as there are many and I don't think there is common agreement on this; I suggest that the actual policy that we want be spelled out here in the document
4.2  option (A) under figure 4: "the client includes its client identifier, requested scope" -> "the client includes its client identifier, optional requested scope"
4.2.1:  "Due to lack of client authentication" - not sure what this is trying to get at here as this is just a client id; nothing about its usage is called out, thus this should go in the Security Considerations section, not here in the main text
4.2.2:  "The clients SHOULD ignore unrecognized" -> "The clients SHOULD ignore undocumented response"
4.5:  Remove "I-D.ietf-oauth-saml2-bearer" until this becomes a standard
5.1:  "The parameters are included in the entity body of the HTTP ...." - This should be a new generic section on JSON encoded responses
6:  Item #7 "has not expired and that its scope covers the requested resource" -> "has not expired and that the scope expressed is within the required scope of the requested resource"
11.2:  remove references to "I-D.hammer-oauth-v2-mac-token-02", "I-D.ietf-oauth-v2-bearer-02", "I-D.ietf-oauth-saml2-bearer-03" until these become standards

---

From: Eran Hammer-Lahav <[email-masked]>
Date: Thu, 24 Mar 2011 19:22:27 -0700

> -----Original Message-----
> From: [email-masked] [mailto:[email-masked]] On Behalf
> Of Mike Jones
> Sent: Tuesday, March 15, 2011 7:52 AM

> 2.1.1:  "If no valid redirection URI is available, the authorization server
> SHOULD" - I don't understand why this is a SHOULD and not a MUST

Because we cannot mandate how the authorization server interacts with the resource owner. Some server may choose ignore the error and send the user to some landing page.

> Restore the Client Assertion Credentials feature that was present in draft 11,

Previously rejected due to no working group consensus.
 
> In the "scope" definition, add "The space-delimited strings are case

Good call.

> complimentary "resource" parameter
 
Previously rejected due to no working group consensus.

> 4.2.2:  Add optional "expires_at" parameter

Previously rejected due to no working group consensus.

> The name client_credentials does not refer to the same concept as the uses
> of the term "Client Credentials" in 1.4.4 and other locations in the
> document.  It would be far better to rename this parameter "none" or
> "implicit", to indicate that no explicit credentials are being passed in the
> protocol.  It might also clarify this concept if you added an example.

We had a long discussion about this and the consensus was to name it 'client_credentials'. Both 'none' and 'implicit' were rejected.

> Restore the WWW-Authenticate Response Header Field that was present in

Previously rejected due to no working group consensus.
 
> section 4.3, to the IANA Considerations.

Previously rejected due to no working group consensus. An alternative proposal coming.

> Add the "resource request" and "resource response" parameter locations, as
> defined in the bearer token specification, draft 03, section 4.2, to the OAuth
> Parameters Registry.

Previously rejected due to no working group consensus.

> 2.1.1:  "resource owner's user-agent" this may not be possible if the resource
> owner is a device or if the resource owner does not have a user agent

User-agent is an generic HTTP term.

   user agent
      The client which initiates a request. These are often browsers,
      editors, spiders (web-traversing robots), or other end user tools.

> 4.1.1:  "parameters are present and valid" - there are no processing
> requirements section to determine what "valid" means; suggest that a
> processing section be added to understand what "valid" means otherwise
> remove the references to "valid"

It is pretty obvious that valid == as defined (case sensitive, value format, not repeating, etc.).

> 4.2.1:  "Due to lack of client authentication" - not sure what this is trying to
> get at here as this is just a client id; nothing about its usage is called out, thus
> this should go in the Security Considerations section, not here in the main
> text

This has been a big PR issue for part versions and needs to be called out explicitly right there. Changed to:

    "The client identifier alone MUST NOT be relied upon for client identification as it provides no means for authentication."

> 11.2:  remove references to "I-D.hammer-oauth-v2-mac-token-02", "I-D.ietf-
> oauth-v2-bearer-02", "I-D.ietf-oauth-saml2-bearer-03" until these become
> standards

The RFC editor takes care of that.

EHL

---

From: Brian Campbell <[email-masked]>
Date: Fri, 25 Mar 2011 09:00:02 -0600

>> In the "scope" definition, add "The space-delimited strings are case

I'd like to better understand why?  Other than a means to delimit and
allow for multiple values, the spec is otherwise leaving the
interpretation of scope values up to the implementation/deployment.

---

From: Eran Hammer-Lahav <[email-masked]>
Date: Fri, 25 Mar 2011 08:02:48 -0700

It's a safe language to align it with common assumptions. Also, in the case of URI values, they are usually case insensitive.

EHL

> From: Brian Campbell [mailto:[email-masked]]
> Sent: Friday, March 25, 2011 8:00 AM
> To: Eran Hammer-Lahav
> Cc: Mike Jones; [email-masked]
> Subject: Re: [OAUTH-WG] Feedback on draft-ietf-oauth-v2-13
> 
> I'd like to better understand why?  Other than a means to delimit and allow
> for multiple values, the spec is otherwise leaving the interpretation of scope
> values up to the implementation/deployment.

---

From: Brian Campbell <[email-masked]>
Date: Fri, 25 Mar 2011 10:16:36 -0600

It didn't align with the my assumption when I was implementing, which
is why I asked:)

With respect to URIs, I believe that the scheme and host are always
case-intensive but the case-sensitivity of the other parts are scheme
dependent and that, in general, comparison is hard and error prone. It
just feels like a slippery slope to start adding
normalization/comparison rules to scope.  But with that said, it's not
a big deal and I won't quibble about it.

---

From: Mike Jones <[email-masked]>
Date: Fri, 25 Mar 2011 16:24:01 +0000

Actually, despite having submitted this comment, I'm having second thoughts about it as well based upon some possible use cases I'm aware of.  If, for instance, a scope element contains base64url encoded content in a URL, case-folding it will destroy it.  Therefore, on reconsideration, I'd like to withdraw the comment and have the sentence about scope elements being case-insensitive removed from the draft.

				Thanks,
				-- Mike

-----Original Message-----
From: [email-masked] [mailto:[email-masked]] On Behalf Of Brian Campbell
Sent: Friday, March 25, 2011 9:17 AM
To: Eran Hammer-Lahav
Cc: [email-masked]
Subject: Re: [OAUTH-WG] Feedback on draft-ietf-oauth-v2-13

It didn't align with the my assumption when I was implementing, which is why I asked:)

With respect to URIs, I believe that the scheme and host are always case-intensive but the case-sensitivity of the other parts are scheme dependent and that, in general, comparison is hard and error prone. It just feels like a slippery slope to start adding normalization/comparison rules to scope.  But with that said, it's not a big deal and I won't quibble about it.

>> interpretation of scope values up to the implementation/deployment.
_______________________________________________
OAuth mailing list
[email-masked]
https://www.ietf.org/mailman/listinfo/oauth

---

From: Eran Hammer-Lahav <[email-masked]>
Date: Fri, 25 Mar 2011 12:11:11 -0700

That makes sense too. I'll change it to explicitly say case sensitive. Either way, this is a useful comment.

EHL

> From: Mike Jones [mailto:[email-masked]]
> Sent: Friday, March 25, 2011 9:24 AM
> To: Brian Campbell; Eran Hammer-Lahav
> Cc: [email-masked]
> Subject: RE: [OAUTH-WG] Feedback on draft-ietf-oauth-v2-13
> 
> Actually, despite having submitted this comment, I'm having second
> thoughts about it as well based upon some possible use cases I'm aware of.
> If, for instance, a scope element contains base64url encoded content in a
> URL, case-folding it will destroy it.  Therefore, on reconsideration, I'd like to
> withdraw the comment and have the sentence about scope elements being
> case-insensitive removed from the draft.
> 
> 				Thanks,
> 				-- Mike
> 
> Of Brian Campbell
> With respect to URIs, I believe that the scheme and host are always case-
> intensive but the case-sensitivity of the other parts are scheme dependent
> and that, in general, comparison is hard and error prone. It just feels like a
> slippery slope to start adding normalization/comparison rules to scope.  But
> with that said, it's not a big deal and I won't quibble about it.
> 
> > It's a safe language to align it with common assumptions. Also, in the case
> of URI values, they are usually case insensitive.
> >
> > EHL
> >