Hi Hannes,

thanks for your review and feedback. Please find my comments inline.

Am 08.10.2012 13:52, schrieb Tschofenig, Hannes (NSN - FI/Espoo):
Hi all,

I went through draft-ietf-oauth-revocation-01 to what has changed
between version -00 and -01.

A few minor comments:

Title: maybe you should change it from "Token Revocation" to "Revocation
of OAuth Access and Refresh Tokens" to make it a bit more informative.

why not. What do other WG members think?


The abstract is also a bit too short to explain what the document does.
The RFC Editor will at least demand 3 lines of abstract. You may, for
example say that the specification covers revocation of access and
refresh tokens. Important would also to highlight that the revocation is
initiated by the client. You may also want to highlight that this is
mainly used as a logout mechanism.

ok.

I you want to re-word the following sentence a bit:
"
    A revocation request may discard the actual token as well as other
    tokens based on the same access grant and the access grant itself.
"
For example:
"
A revocation request by the client includes the token that shall be
revoked. Revoking a refresh token will, however, also revoke any access
token created earlier via the presented refresh token.

I see the following issues with your proposal:
- I assume not all authorization servers will support access token revocation (none of our servers does) - Access tokens belonging to the same access grant might have been created based on different refresh tokens, since refresh tokens can be rotated (we do this for refresh tokens issued to installed applications). see also http://www.ietf.org/mail-archive/web/oauth/current/msg09244.html

That's why I suggested on the list to add the concept of an underlying access grant. This concept proved to be useful during discussions about the different token implementation philosophies at the last IIW.

It also allows an authorization server to delete access grants underlying an access token even if it does not issue refresh tokens at all - some authorization servers manage permanent access grants internally and issue access tokens automatically in implicite and code grant type (auto-approval).

I therefore would suggest the following wording:
"A revocation request by the client includes the token that shall be revoked. Revoking a token may, however, also revoke other tokens based on the same access grant and the acces grant itself."

"

Introduction: I would not include RFC 2119 language in the introduction.

And put the compliance statement into another section?

I am also wondering why a specification shouldn't support the revocation
of both tokens when requested.

The assumption is that not all authorization servers will support access token revocation because this comes at a cost. It either requires a lookup of the RS at the AS on every service request OR a push notification of the AS towards all relevant RS on revocation.


You write that the specification supports the following use cases and
you list two items. The issue, however, is that item #2 isn't what the
specification deals with. Instead, the focus is on item #1 and the
revocation feature is more a logout feature, as you describe it rather
than a way to deal with stolen tokens.

What about the self-care portal (see below)?


    o  The end-user triggers revocation from within the client that sends
       the appropriate revocation request to the autorization server.
       The request causes the removal of the client's access grant the
       particular token refers to.  From the end-user's perspective, this
       looks like a "logout" or "reset" function.  This use case makes it
       even more comfortable to the end-user to revoke his access grant
       immediately via the client.

    o  In contrast to revocation by a client, the authorization server
       (or a related entity) may offer its end-users a self-care portal
       to delete access grants given to clients independent of any token
       storing devices.  Such a portal offers the possibility to an end-
       user to look at and revoke all access grants he once authorized.
       In cases the token storing device is not available, e.g. it is
       lost or stolen, revocation by a self-care portal is the only
       possibility to limit or avoid abuse.


Terminology: I would include a Terminology section with the following
content:
"
    The key words "MUST", "MUST NOT", "REQUIRED", "SHALL", "SHALL NOT",
    "SHOULD", "SHOULD NOT", "RECOMMENDED", "MAY", and "OPTIONAL" in this
    document are to be interpreted as described in RFC 2119 [RFC2119].
"

ok.


You write:

    In the end, security, usability, and ease of use are increased by
    token revocation.

Could you explain this a bit? The specification does not help with
stolen tokens.

What about the self-care portal?

It also does not help with clients who behave
unsupportive.

I don't understand this statement. "unsupportive" with respective to token revocation?



Example: I suggest to expand the example token a bit to avoid the
impression that tokens are so short.
s/token=45ghiukldjahdnhzdauz&/token=45ghiukldj....ahdnhzdauz&/

I'm wondering why the core spec shows example tokens of roughly the same size.


I don't understand this note:

"It is also conceivable to allow a dedicated user self-care portal to
revoke all kinds of tokens."
The self-care portal shouldn't have tokens and hence cannot really
revoke them. As such, the self-care portal would be either at the
authorization server or very closely related to it (for example with
access to the same backend database).

Yes, that's true. Such a portal must have access to the token database. It may be part of the authorization server or a general purpose portal, which access the token database via well-defined (internal) interfaces. We implemented the later pattern.



Regarding:
"
If the particular token is a refresh
    token and the authorization server supports the revocation of access
    tokens, then the authorization server SHOULD also invalidate all
    access tokens based on the same access grant.
"

Why "SHOULD" and not MUST?

See above.


Shouldn't this sentence say

If the presented token is a refresh token  then the authorization server
MUST invalidate all access tokens created by that refresh token."

See above. "that refresh token" might not be meaningful in conjunction with refresh token rotation.


Note that I
  - clarified what gets deleted, and
  - removed unnecessary options.


You write:
"
    Whether the revocation takes effect instantly or with some delay
    depends on the architecture of the particular deployment.  The client
    MUST NOT make any assumptions about the timing and MUST NOT use the
    token again.
"

This is an interesting aspect since one would wonder why it isn't
sufficient for the client to just delete the tokens it has locally.
Nobody else should have the same set of tokens since they are supposed
to be minted on the fly for the different clients.

Basically, you are right. I'm wondering whether there could be issues in clustered environment, potentielly different nodes could use the same access token?


Then, wouldn't it be useful for a logout functionality that the
revocation actually works instantly rather than at an unpredictable time
later ?

Yes.


Could you explain why the cross-origin support is needed?

I think this is the only way for a user-agent based application to revoke tokens due to the same-origin policy.

@Marius: Could you please shed some more light on this aspect?


You write:
"
unsupported_token_type: the client
            tried to revoke an access token on a server not supporting
            this feature.
"
Does this mean that the server does not support the revocation
specification at all or just not the ability to revoke access
(refresh???) tokens?

Revocation of access tokens is an optional feature


Security considerations: I would expect to see a discussion about what
scenarios this revocation mechanism does not cover.

I thought such a section was intended to discuss the security of the proposed protocol?

Do you have specific scenarios in mind?

regards,
Torsten.


Ciao
Hannes

_______________________________________________
OAuth mailing list
[email protected]
https://www.ietf.org/mailman/listinfo/oauth

_______________________________________________
OAuth mailing list
[email protected]
https://www.ietf.org/mailman/listinfo/oauth

Reply via email to