Implement custom use_refresh_token #1103
Conversation
spec/lib/oauth/base_request_spec.rb
Outdated
| access_token_expires_in: 100, | ||
| custom_access_token_expires_in: ->(_context) { nil }, | ||
| refresh_token_enabled?: lambda { |context| | ||
| context.scopes == "public" ? true : false |
There was a problem hiding this comment.
Style/RedundantConditional: This conditional expression can just be replaced by context.scopes == "public".
| # Issue access tokens with refresh token (disabled by default) | ||
| # Issue access tokens with refresh token (disabled by default), you may also | ||
| # pass a block which accepts `context` to customize when to give a refresh token | ||
| # or not. Similar to `custom_access_token_expires_in`, `context` has the properties: |
There was a problem hiding this comment.
Metrics/LineLength: Line is too long. [86/80]
| @@ -66,7 +66,13 @@ | |||
| # | |||
| # reuse_access_token | |||
|
|
|||
| # Issue access tokens with refresh token (disabled by default) | |||
| # Issue access tokens with refresh token (disabled by default), you may also | |||
| # pass a block which accepts `context` to customize when to give a refresh token | |||
There was a problem hiding this comment.
Metrics/LineLength: Line is too long. [82/80]
| end | ||
|
|
||
| private |
There was a problem hiding this comment.
Lint/UselessAccessModifier: Useless private access modifier.
| end | ||
|
|
||
| def access_token_expires_in(server, context) | ||
| if (expiration = server.custom_access_token_expires_in.call(context)) |
There was a problem hiding this comment.
Metrics/LineLength: Line is too long. [81/80]
lib/doorkeeper/config.rb
Outdated
| def use_refresh_token | ||
| @config.instance_variable_set(:@refresh_token_enabled, true) | ||
| # Issue access tokens with refresh token (disabled if not set) | ||
| def use_refresh_token(enabled = true, &custom) |
There was a problem hiding this comment.
I added the enabled = true parameter here in case people do use_refresh_token false. Since this takes a block parameter, I thought to make it a bit consistent.
There was a problem hiding this comment.
I prefer &block instead of &custom as a common practice, what do you think?
Also check out force_ssl_in_redirect_uri option and how it works with block or boolean variables.
thatsmydoing
force-pushed
the
configurable-refresh-token
branch
from
7c5fbab
to
38ca199
Compare
| private | ||
|
|
||
| def custom_expiration(server, pre_auth_or_oauth_client, grant_type, scopes) | ||
| def make_context(pre_auth_or_oauth_client, grant_type, scopes) |
There was a problem hiding this comment.
Could you please rename it to build_context ?
lib/doorkeeper/config.rb
Outdated
| def use_refresh_token | ||
| @config.instance_variable_set(:@refresh_token_enabled, true) | ||
| # Issue access tokens with refresh token (disabled if not set) | ||
| def use_refresh_token(enabled = true, &custom) |
There was a problem hiding this comment.
I prefer &block instead of &custom as a common practice, what do you think?
Also check out force_ssl_in_redirect_uri option and how it works with block or boolean variables.
thatsmydoing
force-pushed
the
configurable-refresh-token
branch
from
38ca199
to
de9e894
Compare
| def use_refresh_token | ||
| @config.instance_variable_set(:@refresh_token_enabled, true) | ||
| # Issue access tokens with refresh token (disabled if not set) | ||
| def use_refresh_token(enabled = true, &block) |
There was a problem hiding this comment.
I renamed it to block here. but I'm keeping the structure so that it remains compatible with how it worked before.
|
Is this better? |
|
Yep, much better. Could you please resolve the conflicts? |
thatsmydoing
force-pushed
the
configurable-refresh-token
branch
from
de9e894
to
e42ee06
Compare
|
Thank you very much 👍 |
Summary
This is meant to address #646, it lets you customize when to give a refresh token or not. This uses the same
contextascustom_access_token_expires_inthat was added in #1102