Re: [OAUTH-WG] Comments on draft-ietf-oauth-v2-03.txt

Eran Hammer-Lahav <eran@hueniverse.com> Sun, 09 May 2010 22:25 UTC

Return-Path: <eran@hueniverse.com>
X-Original-To: oauth@core3.amsl.com
Delivered-To: oauth@core3.amsl.com
Received: from localhost (localhost [127.0.0.1]) by core3.amsl.com (Postfix) with ESMTP id B5DE53A68F2 for <oauth@core3.amsl.com>; Sun, 9 May 2010 15:25:36 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -0.856
X-Spam-Level:
X-Spam-Status: No, score=-0.856 tagged_above=-999 required=5 tests=[AWL=-1.457, BAYES_50=0.001, J_CHICKENPOX_52=0.6]
Received: from mail.ietf.org ([64.170.98.32]) by localhost (core3.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id WDEIoUNxNDhz for <oauth@core3.amsl.com>; Sun, 9 May 2010 15:25:34 -0700 (PDT)
Received: from p3plex1out01.prod.phx3.secureserver.net (p3plex1out01.prod.phx3.secureserver.net [72.167.180.17]) by core3.amsl.com (Postfix) with SMTP id D36743A6980 for <oauth@ietf.org>; Sun, 9 May 2010 15:25:25 -0700 (PDT)
Received: (qmail 525 invoked from network); 9 May 2010 22:25:14 -0000
Received: from unknown (HELO smtp.ex1.secureserver.net) (72.167.180.19) by p3plex1out01.prod.phx3.secureserver.net with SMTP; 9 May 2010 22:25:13 -0000
Received: from P3PW5EX1MB01.EX1.SECURESERVER.NET ([10.6.135.20]) by P3PW5EX1HT001.EX1.SECURESERVER.NET ([72.167.180.19]) with mapi; Sun, 9 May 2010 15:25:13 -0700
From: Eran Hammer-Lahav <eran@hueniverse.com>
To: Torsten Lodderstedt <torsten@lodderstedt.net>, "OAuth WG (oauth@ietf.org)" <oauth@ietf.org>
Date: Sun, 09 May 2010 15:25:14 -0700
Thread-Topic: Comments on draft-ietf-oauth-v2-03.txt
Thread-Index: Acrvwzvl4gfH9nBDTD+pipTXezR4UQAAMdhw
Message-ID: <90C41DD21FB7C64BB94121FBBC2E72343B3AB46E24@P3PW5EX1MB01.EX1.SECURESERVER.NET>
References: <4BE730CC.1090607@lodderstedt.net>
In-Reply-To: <4BE730CC.1090607@lodderstedt.net>
Accept-Language: en-US
Content-Language: en-US
X-MS-Has-Attach:
X-MS-TNEF-Correlator:
acceptlanguage: en-US
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
Subject: Re: [OAUTH-WG] Comments on draft-ietf-oauth-v2-03.txt
X-BeenThere: oauth@ietf.org
X-Mailman-Version: 2.1.9
Precedence: list
List-Id: OAUTH WG <oauth.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/listinfo/oauth>, <mailto:oauth-request@ietf.org?subject=unsubscribe>
List-Archive: <http://www.ietf.org/mail-archive/web/oauth>
List-Post: <mailto:oauth@ietf.org>
List-Help: <mailto:oauth-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/oauth>, <mailto:oauth-request@ietf.org?subject=subscribe>
X-List-Received-Date: Sun, 09 May 2010 22:25:36 -0000

> -----Original Message-----
> From: Torsten Lodderstedt [mailto:torsten@lodderstedt.net]
> Sent: Sunday, May 09, 2010 3:02 PM
> To: OAuth WG (oauth@ietf.org); Eran Hammer-Lahav
> Subject: Comments on draft-ietf-oauth-v2-03.txt
> 
> I have put together some comments/suggestions regarding the current
> draft.

Well, there is already -04... :-) I messed up the section numbers in -03.

> 3.2.1.  Response Format
> 
> The authorization server MUST include the HTTP "Cache-Control"
>     response header field with a value of "no-store" in any response
>     containing tokens, secrets, or other sensitive information.
> 
> Would'nt it make sense to require "no-cache", too?

No idea.

> 3.2.1.1.  Access Token Response
>     expires_in
>           OPTIONAL.  The duration in seconds of the access token
>           lifetime.
> 
>     refresh_token
>           OPTIONAL.  The refresh token used to obtain new access tokens
>           using the same end-user access grant as described in Section 6.
>
> What is the exact meaning/idea of OPTIONAL response parameters?
> Who actually decides to use them? The client or the authorization server or
> both?
> Is it optional to implement this feature or is it envisioned the server
> dynamically decides to use those parameters based on its policy?

Optional means the server gets to decide to include them or not. I think it is pretty clear that optional is decided by the entity constructing the message.

> expires_in
> 
> Why is there information passed back about the access tokens duration but
> not about the refresh tokens duration?

No one asked for that. Is there interest in specifying the authorization duration in addition to the token duration?

> 3.2.1.2.  Error Response
> 
>   If the token request is invalid or unauthorized, the authorization
>     server constructs a JSON-formatted response which includes the common
>     parameters set as well as additional flow-specific parameters.  The
>     formatted parameters are sent to the client in the entity body of the
>     HTTP response with a 400 status code (Bad Request).
> 
> To use status code 400 for all kinds of errors is very generic from my point of
> view. How shall the client implement a dedicated error handling if it cannot
> distinguish different errors. I would suggest either to define error codes (not
> only
> messages) in the spec or to use appropriate HTTP status codes for different
> error conditions, e.g. 401 for unauthorized calls.

It does define error codes and sends them in the 'error' parameter.

> 3.4.  Client Credentials
> 
> The client credentials include a client identifier and
>     an OPTIONAL symmetric shared secret.
> 
> What is meant by "symmetric shared secret"? I'm familiar with the term
> "symmetric" in the context of cryptographical algorithms only. I think "shared
> secret" is sufficient.

Shared secret can be symmetric (password) or asymmetric (key pair).

> 3.5.  User-Agent Flow
> 
> These clients
>     cannot keep client secrets confidential and the authentication of the
>     client is based on the user-agent's same-origin policy.
> 
> Does this still hold in the context of "HTTP Origin Headers"
> (http://www.petefreitag.com/item/702.cfm)?
> 
> BTW: Will Brian's security considerations document be updated to be in sync
> with the draft?

Brian's document was written for WRAP. We need to write a full security review for 2.0 that is structure similarly to OAuth 1.0.

> 3.5.1.  Client Requests Authorization
> 
> If the client has previously registered a redirection URI with the
>     authorization server, the authorization server MUST verify that the
>     redirection URI received matches the registered URI associated with
>     the client identifier.
> 
> Does this mean equality? Or just the same base string?

Right now it depends on the server.

> 3.5.1.1.  End-user Grants Authorization
> 
> I would suggest to use JSON encoding here, since the URI fragment is
> handled by a client more or less like a response result.
>
> 3.6.1.1.  End-user Grants Authorization
> 
> Why not call the "code" Parameter "verification_code"? It's called verification
> code in the text.

Longer for no reason.

> 3.6.2.  Client Requests Access Token
> 
> client_secret
>           REQUIRED if the client identifier has a matching secret.  The
>           client secret as described in Section 3.4.
> 
> 1) I'm having trouble to understand the meaning of  "if the client identifier
> has a matching secret". Is the assumption underneath that authorization
> servers require this secrets from all clients they have issued a secret to?
> What about client w/o a secret? Are they also allowed to use this flow?
> Or is there envisioned a dependency
> between the client type, clients credentials and the flows a particular client is
> authorized to use?
> 
> I would have expected a clear policy which flows require authentication and
> which not.
> 
> 2) I would suggest to replace the request parameter by an Authorization
> header. First of all because that's the way credential transmission is handled
> in HTTP. Moreover, it better fits with error handling. If the secret is missing or
> wrong, the authorization server can answer with status code 401 and a
> WWW-Authenticate header(s) specifiying mechanism and realms to be used
> by the client in order to authenticate to the server.
> 
> In the long run it gives deployments more freedom to use other mechanisms
> than sending shared secrets as plain text over the wire.
> 
>      HTTP/1.1 400 Bad Request
>       Content-Type: application/json
>       Cache-Control: no-store
> 
>       {"error"="incorrect_client_credentials"}
> 
>  From my point of view, "redirect_uri_mismatch" and
> "bad_verification_code" call for status code 403, whereas
> "incorrect_client_credentials" is the 401 candidate.
> 
> 3.7.1.  Client Requests Authorization
>       HTTP/1.1 200 OK
>       Content-Type: application/json
>       Cache-Control: no-store
> 
>       {"code":"74tq5miHKB","user_code":"94248","user_uri":"http%3A%2F%2
>       Fwww%2Eexample%2Ecom%2Fdevice","interval"=5}
> 
> Why is the user_uri URL-encoded?

Conversion mistake.

> 3.7.2.  Client Requests Access Token
> 
> "authorization_declined"
> 
> Why does the spec interpret a request as bad request if the user just has
> declined the authorization? According to rfc2616 the semantics of
> 400 is: "The request could not be understood by the server due to
> malformed syntax".
> 
> The calling client did not sent a malformed request, it cannot foresee or
> influence this outcome. I would suggest to either use status 403 or to return
> status code 200 for all syntactically correct and authorized request and to use
> another return codes in the response body to detail the requests outcome.
> 
> 4.  Username and Password Flow
> 4.1.  Client Requests Access Token
> 
> As statet in this section's introduction, this flow is intended to migrate
> existing clients from BASIC/DIGEST to OAuth. I therefore would suggest to
> use excactly these HTTP authentication schemes to transfer user credentials.
> This would mean to remove the parameters "username"
> and "password" and to use Authorization headers instead. At Deutsche
> Telekom, we have to deal with that class of clients. Such clients have already
> implemented their credential handling including header manipulation. The
> proposed change would further simplify their migration.

How would you send both client credentials and user credentials? Migration is only one example.

> 6.  Assertion Flow
> 
> This flow is suitable when the client is acting autonomously or on behalf of
>     the end-user (based on the content of the assertion used).
> 
> Let's assume the assertion attests an end-user's identity. How shall the
> authorization server determine the clients identity in this case? I would
> suggest to add optional client_id/client_secret parameters (or an
> Authorization header) for that case.

That's the whole point - it doesn't need it because it uses a different trust framework where the client identity is part of.

> 7.  Refreshing an Access Token
> 
> I would suggest to add an optional "scope" parameter to this request.
> This could be used to downgrade the scope associated with a token.

That would be the wrong way to do it given that people will expect to also be able to upgrade scope.

> 8.1.  The Authorization Request Header
> 
> I consider the name "Token" of the authentication schema much to generic.
> That was also the feedback of all colleagues I talked to about the upcoming
> standard. Why not call it "OAuth2"?

It is meant to be generic. I consider OAuth to be the part of the protocol dealing with getting a token. There will be many new ways to get a token in the future.

> 8.2.2.  Form-Encoded Body Parameter
> 
> I would suggest to drop this section/feature.
> 
> General note: Would it make sense to add explicit format handling to the
> OAuth API? If we envision authorization servers supporting more than one
> token format (e.g. SAML, SWT, ...), the client should given the option to
> request a certain format and responses returning access tokens should
> indicate the format of this token. The OAuth authorization header could also
> have an optional format attribute for the same purpose.

You mean token format? OAuth defined the token as opaque so that would be counter-productive.

EHL