I am the assigned Gen-ART reviewer for this draft. For background on Gen-ART,
please see the FAQ at <http://wiki.tools.ietf.org/area/gen/trac/wiki/GenArtfaq>.
Document: draft-ietf-oauth-v2.txt
Reviewer: Richard Barnes
Review Date: 10 Apr 2012
IETF LC End Date:
IESG Telechat Date: 12 Apr 2012
Summary: Almost ready, but with some gaps that need to be addressed
MAJOR:
General: At several points, the document seems to deal in plain-text usernames
and passwords, ranging from the requirement for HTTP Basic authentication
(2.3.1) to the explicit username and password fields one of the grant types
(4.3.2, 10.7). In these cases, it seems like it would encourage better
security practices to use some sort of digest or other secure scheme for
proving ownership of passwords. Otherwise, there's a risk that proxies,
caches, etc. will have access to users' plaintext credentials.
General: The document requires the Authorization Server to distinguish between
confidential and public clients at several points. It's not clear, however,
how the server is supposed to tell which is which. Perhaps Section 2.1 could
elaborate a little more on this?
4: Throughout the document, the assumption is that the client is moved from one
URI to another via HTTP redirects (302 responses). Could the protocol also
accommodate other techniques of moving clients around, e.g., in JavaScript
(setting window.location). It seems like this could make deployment much
easier in some cases.
10.3: This section seems to ignore the risk of client impersonation at the
resource server. Just as with client credentials in Section 10.2, if an access
token is compromised, then it could be presented by another party in order to
access the protected resource. So the Resource Server needs to authenticate
the client and validate that the token goes with the client, in addition to
just checking the validity of the token.
10: Throughout this section, there are mentions of which parameters require
secure transmission/storage and which do not. It would be helpful to summarize
these requirements, say in a table at the end of the section.
MINOR:
2.3.1: When you say "request body" in this section, do you actually mean
"query"? I don't see a prohibition on GET here, in which case the parameters
would be in the URI query section.
3.1.2.2: It seems like an explicit requirement would be helpful here, e.g.:
"
The Authorization Server MUST verify that either no redirect_uri is provided
(in which case it uses the registered URI), or else that the provided
redirect_uri matches the registered redirection URI for the authenticated
client_id (where the match can be based on a full or partial registered URI).
"
4.1.3: Why does this request use redirect_uri and not client_id to identify the
client? (Or both?) It seems like involving the client_id would better support
the goal of client authentication, in order to mitigate the risk of client
impersonation.
4.3.2: Shouldn't there be a client authentication here? (In particular, a
client_id.) Otherwise, it seems like this is essentially the same as the flow
in 4.4.
7. It seems like it would be helpful to have a way to pass access tokens in the
request URI / query in some deployment cases (e.g., a JavaScript based client).
Is there any particular reason this was excluded?
EDITORIAL:
3. Would be helpful to note here that these endpoints are in addition to the
resource endpoint (the one started out to protect), which is handled in Section
7.
3.2: It's not clear why only POST is allowed here. A couple of words of
explanation would help.
5.1: Why bother with a case-insensitive token_type here? It doesn't look like
anything else in the whole document has this property.