Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Add an API method to allow refreshing of the access-token #44

Open
bummzack opened this issue Jun 19, 2015 · 2 comments
Open

Add an API method to allow refreshing of the access-token #44

bummzack opened this issue Jun 19, 2015 · 2 comments

Comments

@bummzack
Copy link
Contributor

I created a pull request that enables auto-refreshing tokens (#38).

While this addition adds convenience, it worsens the application security. Having a token that expires within a reasonable amount of time gives a potential attacker less time to do damage.

The client gets the token expiry when logging in. It should be the responsibility of the client-application to issue a token refresh when the token is about to expire.

Issuing a token-refresh should not use the regular access-token, since it would enable the attacker to completely take over access.

So there are two approaches for a token-refresh:

  • Use the user credentials (this would mean that user credentials have to be stored client-side which is not optimal)
  • Use a special refresh-token

I would propose the following changes to the RESTful API

  • Add an additional field RefreshToken to RESTfulAPI_TokenAuthExtension
  • When a user logs in (api/auth/login), generate a RefreshToken and the regular access-token and pass both back to the client.
  • Introduce an action api/auth/refreshToken which has to be called by the client when the access-token is about to expire. The client has to submit the obtained RefreshToken and gets a new access-token in return.

I'd gladly create a PR for this if you agree that this would be a worthwhile addition.
bummzack added a commit to bummzack/silverstripe-restfulapi that referenced this issue Jun 19, 2015
@bummzack
FIX colymba#39: Check for token uniqueness when generating tokens.
e5b87a7 
ADD colymba#44: Implement API method to refresh token.

- Reverted to non-unique indexes on `RESTfulAPI_TokenAuthExtension` since there's an issue with MsSQL DBs.
- Implemented token refresh methods.
- Updated documentation.
- Added test for "refreshToken".
- Updated token uniqueness test.
@colymba
Copy link
Owner

colymba commented Jun 20, 2015

I can see the security concern about auto refreshing token, but that is left to the dev choice via config.

Also, users can easily refresh their token via the login method. So the refresh method is a bit redundant?

Am not 100% convinced, and it would be great to look at best practices maybe...

@bummzack
Copy link
Contributor Author

Yes, you can use the login method, but that would mean that I'll have to persist username and password on the client, which I don't like at all.

The refresh-token approach is inspired by OAuth refresh-tokens. And since this is a purely optional addition to the current API, I deemed it worthwhile.

A developer can still choose to:

  • use the RESTfulAPI and refresh token using login
  • use autoRefreshLifetime to create auto-refreshing tokens
  • set the token lifetime to a very high value, so it basically won't expire
  • or use the slightly more secure refresh-token approach

It's just about giving more options to the developer.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@colymba @bummzack