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