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.

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

oauth-client check added in Oauth2KeyExists MW #2095

Merged
merged 1 commit into from
Feb 3, 2019

Conversation

dencoded
Copy link
Contributor

@dencoded dencoded commented Feb 1, 2019

Added changes #2093

  • check if oauth client is present in Redis
  • trying to avoid hitting Redis every time:
    • for deleted oauth client there will be only one hit, then result cached in memory cache
    • for not deleted oauth client result is cached for 1 sec

Also, was exploring option just deleting all tokens at the time of oauth-client deletion but that might not work as sync call in case of big amount of issued tokens.

@dencoded dencoded requested a review from asoorm February 1, 2019 02:28
@buger
Copy link
Member

buger commented Feb 3, 2019

Makes sense

@buger
Copy link
Member

buger commented Feb 3, 2019

Deleting all tokens is not an options, since their analytics data potentially can be useful. And we already have options to control expiration of oauth tokens.

@buger buger merged commit 90f3a14 into master Feb 3, 2019
@buger buger deleted the invalidate-tokens-delete-oauthclient branch February 3, 2019 19:52
buger pushed a commit that referenced this pull request Feb 19, 2019
Added changes #2093

- check if oauth client is present in Redis
- trying to avoid hitting Redis every time:
  - for deleted oauth client there will be only one hit, then result cached in memory cache
  - for not deleted oauth client result is cached for 1 sec

Also, was exploring option just deleting all tokens at the time of oauth-client deletion but that might not work as sync call in case of big amount of issued tokens.
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.

None yet

2 participants