Skip to content

Conversation

trbs
Copy link
Contributor

@trbs trbs commented May 18, 2015

Simple implementation of cleanup strategy for expired tokens.

This cleanups expired access_token's (when there is no refresh_token) and expired grant entries. When refresh_token_expire_seconds is set it will expire refresh_token's as well.

Happy to write documentation if we decide to move forward with this or similar PR.
But I first wanted to submit this PR so we can get the discussion on how to expire token and cleanup the database periodically going.

Simple implementation of cleanup strategy for expired tokens.
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.6%) to 95.34% when pulling 7ed2d86 on trbs:cleartokens into 75cd656 on evonove:master.

@masci masci added this to the 0.9.0 milestone May 23, 2015
@masci
Copy link
Contributor

masci commented May 23, 2015

👍 very nice!

@trbs
Copy link
Contributor Author

trbs commented May 24, 2015

To clarify cause I think it's only clear after looking at the code :)

In this PR the idea is that a refresh_token can expire REFRESH_TOKEN_EXPIRE_SECONDS after the access_token has expired. This way no changes to model are necessary and it seems like a logical thing to do :)

I would like to see if we can/should tie this in with the custom models for tokens and applications
PR. So expire cleanup is flexible enough for people to easily write their own custom expire strategies.

For example; Being able to use a custom refresh_token model that has an expires field (configurable via REFRESH_TOKEN_EXPIRE_FIELDNAME ?) or an Application model that defines a refresh_token_expires_after field or similar.

I would assume that different applications with different expire strategies could be fairly common. Like having a part of the api for IOS/Android apps that rarely (or never) expire and a public user/development api which should in expire in minutes or hours.

@pySilver
Copy link

pySilver commented Oct 8, 2015

I like this PR. Refresh tokens should not be immortal and should be removed at some point, they just lasts more longer than access tokens.

@palazzem palazzem modified the milestones: 0.9.0, 0.10.0 Oct 15, 2015
@synasius
Copy link
Contributor

I want to merge this PR :)
Can you please add some docs for the django command and the REFRESH_TOKEN_EXPIRE_SECONDS setting??

Also add yourself to contributors.

@trbs
Copy link
Contributor Author

trbs commented Nov 1, 2015

Documentation is only for REFRESH_TOKEN_EXPIRE_SECONDS at the moment. Information on the management command still needs to be written.

What would be the most reasonable default for REFRESH_TOKEN_EXPIRE_SECONDS ? Leave it set to None which is backwards compatible but will fill up the database at some point... or set it too some multiple of ACCESS_TOKEN_EXPIRE_SECONDS ?

I will try to look at that and the conflicts as soon as I can.

@synasius
Copy link
Contributor

Merged as cd46299.
We need to add documentation for the cleartokens command too. I opened #324 to keep track of it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants