If my understanding is correct, a successful authentication always create a new access token.
Is there an option to return an existing token, if found for the same (resource_owner_id, application_id) pair?
Some lazy app developers (or bugs) could just ask for a new token every time, rather than correctly store and handle the token with expiration information.
The result is ever growing database records. It's like permanent memory leak, and there's no chance for garbage collection if the expiration is set at a distant future. It's so easy to accidentally attack the system.
I would like to bound the maximum number of records in oauth_access_tokens for predictable server sizing and provisioning, and to do so, guaranteed uniqueness of (resource_owner_id, application_id) sounds reasonable.
Was there any discussion over the matter? And would it be accepted if I create a pull request to add that option?
OK, now @miyagawa told me that #191 changed this behavior, and I think my point is still valid.
In my case, I would like to return the same access token to multiple devices (e.g. iPhone and iPad) for the same user, and I wouldn't extend expiration - I'd just set expiration at a distant future and when expires / revoked, it invalidates on all devices that use the same token.
I think it works as intended. We now get a growing DB table (cheap!), no memory leaks. If you needed you could delete the old/unused tokens. Closing for now, but happy to continue discussion!
We cannot simply delete because we set expiration at 1 year from now. Those unused tokens accumulate over time. We don't plan to use refresh token so there's no problem reusing the existing token for the same user.
Also, deleting a bunch of records is by far the costliest operations (than INSERTs or UPDATEs) for MVCC-based databases like InnoDB (MySQL) or PostgreSQL. If you've ever tried to delete millions of records on MySQL, you know it would take weeks to finish, plus totally saturate 100% of main thread's (or purge thread's) CPU and disk I/O in the entire period. This is a well known, serious horror story for large scale systems. Garbage on the disk ain't cheap at scale!
That said, not everyone has millions of users / tokens, so it's a trade-off for sure.
I believe reusing access token is not prohibited by the spec per se, and large scale providers like Facebook do just that. So I thought it would be great if Doorkeeper could support that use case.
I'd like to discuss over a PR. :smiley:
Will do. :)
@kenn Thanks for bringing this up and getting this reuse_access_token feature implemented. I am in 100% agreement to all your points you have and which I have quoted below:
Some lazy app developers (or bugs) could just ask for a new token every time, rather than correctly store and handle the token with expiration information.
In my case, I would like to return the same access token to multiple devices (e.g. iPhone and iPad) for the same user, and I wouldn't extend expiration - I'd just set expiration at a distant future and when expires / revoked, it invalidates on all devices that use the same token.
We cannot simply delete because we set expiration at 1 year from now. Those unused tokens accumulate over time. We don't plan to use refresh token so there's no problem reusing the existing token for the same user.
I believe reusing access token is not prohibited by the spec per se, and large scale providers like Facebook do just that. So I thought it would be great if Doorkeeper could support that use case.
And using adding reuse_access_token option (which by default is disabled) to Doorkeeper.configure block resolved the issue I had.
In case anybody ending up here wants to know about the background of the scenario I have created it as a gist at https://gist.github.com/jiggneshhgohel/193c013ea7ac8cead4c77cef1127c55c
Thanks
BTW I really think there should be section in wiki about all the options supported with their usage purpose with a brief background.
I couldn't find any detail regarding using reuse_access_token in the wiki and after going through this issue and the related PR #387 changes I found that there is already this option available in default generated Doorkeeper.configure block
# Reuse access token for the same resource owner within an application (disabled by default)
# Rationale: https://github.com/doorkeeper-gem/doorkeeper/issues/383
# reuse_access_token
but I couldn't get the intent of using it when I started integrating Doorkeeper in my application and hence ignored it and its purpose became clear when I personally noticed a problem for which that option really exists.
Thanks.
Most helpful comment
BTW I really think there should be section in wiki about all the options supported with their usage purpose with a brief background.
I couldn't find any detail regarding using
reuse_access_tokenin the wiki and after going through this issue and the related PR #387 changes I found that there is already this option available in default generatedDoorkeeper.configureblockbut I couldn't get the intent of using it when I started integrating Doorkeeper in my application and hence ignored it and its purpose became clear when I personally noticed a problem for which that option really exists.
Thanks.