-
Notifications
You must be signed in to change notification settings - Fork 603
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add support for RFC 7009 (revoking) #228
Comments
Good hint - thanks. Definitely think that should be implemented.
Revoking the last access token after is has been refreshed is not necessary as its only refreshed because it will expire anyway. |
Sounds good. The spec allows to revoke access token or refresh token. I don't see any use of revoking a refresh token or how it could be integrated into ember simple auth. In my php library, the refresh token is usually automatically rotated so that old one is removed. |
Refresh tokens usually don't expire so those are the ones we make sure to revoke. Access token revocation doesn't seem to be a required feature according to the spec. However, Ember Simple Auth simply tries to revoke both and ignores if revocation of one kind if token isn't supported by the server.
|
I'd say the opposite lol. In my ZfrOAuth2Server for instance, when you ask a new access token using a refresh token, a new refresh token is issued. It's in the code of the Refresh Token grant that I automatically rotate refresh tokens. Allowing to remove access tokens when logging out allow to automatically "clean" the database and avoid to have millions of rows of access tokens. I usually have a cron job that removes expired tokens, but once you log out, it can be assumed that a new access token will be issued for the next log in, so it's useless to keep the access token, isn't it? |
Not really. If the access token doesn't expire and it's not deleted then the client is actually still logged in. Whatever - I'd like to not start a lengthy discussion here ;) I think if Ember Simple Auth simply tries to invalidate all tokens it has that should cover every case and work with all kinds if servers.
|
Awesome! Just an additional note: the spec defines that it can return a 503 error if token cannot be removed. In my PHP library, this is returned from my server when everything is valid (valid token, valid token hint) but for any reason I cannot remove it (for instance: if my database is down temporarily). Maybe this code should be taken into account too. |
The problem is that everything gets very complex when the different error cases are handled. It gets especially complex when there are both a refresh and an access token on the client which would both have to be invalidated. Imagine one works while the other doesn't. What would the client do in that case? Also if session invalidation is rejected when there's a problem on the server side the user can effectively not log out (while if the server response is ignored the client still securely logs out, just maybe didn't successfully invalidate the token). I don't want to add all that stuff to Ember Simple Auth up front - maybe the need to cover certain cases arises later. |
You're right. Let's keep the "Simple" of Ember Simple Auth ;). |
@marcoow @novaugust just discovered on my PR to Ghost that the ember-simple-auth option is containing a typo. |
@bakura10 @marcoow maybe I'm reading it all wrong but https://tools.ietf.org/html/rfc7009#section-2.1 states that the Client credentials should be put in the |
The revoked token is sent in the request body actually. Client credentials generally make no sense for public clients because as soon as you deploy the client to the internet the credentials are public and cannot be trusted anymore. |
@marcoow I was not talking about the token that should be revoked, I meant the token that is used to authenticate with the API server. I understand that Client credentials don't make any sense for public clients, but the spec still seems to specify that behavior. tl;dr current request: |
Yeah, that's true - the bearer token shouldn't be sent with that request. |
on second thought I'm not even sure anymore if that was currently the case... my head is spinning from all that OAuth crazyness ;) |
thanks for the quick reply though 👍 |
Hi,
There is a spec now for OAuth 2 token revocation: https://tools.ietf.org/html/rfc7009
I think Ember-Simple-Auth should allow to have an option to activate it. The idea would be as follow:
The spec is quite simple: you just need to do a POST request with those two parameters:
I'm implementing the spec as part of the next version of ZfrOAuth2Server (PHP OAuth lib)
The text was updated successfully, but these errors were encountered: