Subject: [OAUTH-WG] Bearer Token Last Call Comments

From: Justin Richer <[email-masked]>
Date: Fri, 12 Aug 2011 14:31:31 -0400

1.3: This draft still uses the term "access grant" where the core now
uses "authorization grant". Change all references to "authorization
grant" and reference section 1.4 in core. Also, too many parenthetical
statements in opening paragraph -- suggested rewrite of this bit: 
	Before a client can access a protected resource, it must first 
	obtain an authorization grant [[link to core S.1.4]] from the 
	resource owner, and then exchange the authorization grant for 
	an access token. The access token then represents the scope, 
	duration, and other access attributes granted by the 
	authorization grant.

2: "...SHOULD NOT be used unless no other..." is a triple negative and
while technically correct it is a bit head-spinny to read. Suggested
rewrite: "Due to the Security Considerations (S.3) associated with the
URI method, this method SHOULD NOT be used unless it is the only
feasible method."

2.1/2.2/2.3: Introductory text is non-parallel. Suggest changing intro
to 2.1 to parallel 2.2 and 2.3, with a "When including the access token
in the Authorization header, the client ..." construct.

2.2: "unless the following conditions are met" is ambiguous. All
conditions are met? At least one?

2.3: Are there conditions for this use as well, to match 2.2?

2.2/2.3: Add normative language to: "The entity-body MAY include other
request specific parameters. In which case..." (similar in 2.3's request
URI) Might be useful to have the example show an additional parameter.

2.4: Should this be a top-level section? Since it's dealing with the
from-the-server bit instead of the to-the-server bit that the rest of 2.
is dealing with.

3.1: Missing word: "Token redirect:  An attacker uses the token
generated for consumption by [one] resource server to obtain access to
another resource server."

3: There's a mix of normative and non-normative language throughout this
section, as well as a mix of imperative and descriptive language. We
suggest making the whole section normative and imperative to be
consistent. Particular instances:

  3.2: "the lifetime of the token MUST be limited”

  3.3: validate SSL certificate chains: "The client MUST..."

  3.3: issue short lived bearer tokens: Change to something like "Token 
	servers SHOULD issue short-lived (one hour or less) bearer 
	tokens, particularly when issuing tokens to clients that run 
	within a web browser or other environments where information 
	leakage may occur. Using short-lived bearer tokens can reduce 
	the impact of one of them being leaked."

  3.3: scoped bearer tokens: "Token servers SHOULD issue bearer tokens 
	that contain an audience restriction..."

  3.3: don't pass: "Bearer tokens SHOULD NOT be passed in page URLs 
	(for example as query string parameters). Instead, bearer 
	tokens SHOULD be passed in HTTP message headers or message 
	bodies for which confidentiality measures are taken. Browsers, 
	web servers, and other software may not adequately secure URLs 
	in the browser history, web server logs, and other data 
	structures. If bearer tokens are passed in page URLs, attackers 
	might be able to steal them from the history data, logs, or 
	other unsecured locations."

 -- Justin Richer