oauth2: Add custom TTL to refresh tokens #2942

bob983 · 2017-10-09T20:41:24Z

Summary

Refresh token TTL used to be hardcoded to 14 days. This pose a problem with scenarios where the token/refresh_token is only used sporadically. This PR adds new config option refresh_token_ttl that specify refresh token's TTL. If the value is nil or 0 it means keep forever.

add refresh_token_ttl to the plugin's schema
pass the value down to generate_token method as ttl option, replace it with nil if the is either not defined or lower than 0.
add schema spec test
add access spec test
update documentation - that's a separate PR I guess

Full changelog

kong/plugins/oauth2/schema.lua
kong/plugins/oauth2/access.lua

Test cases

spec/03-plugins/26-oauth2/01-schema_spec.lua
spec/03-plugins/26-oauth2/03-access_spec.lua

Issues resolved

Fix #2024

Misc

I'm of course open to suggestion on how to make the code better or how to test it better

bob983 · 2017-10-09T20:48:48Z

OK, I tuned the tests against 0.10.3, because that's what we are using, but one of the helpers that I used is now somewhere else in current master. I'll have a look at it 👁

Tieske · 2017-10-10T13:13:29Z

kong/plugins/oauth2/schema.lua

@@ -31,6 +31,7 @@ return {
    accept_http_if_already_terminated = { required = false, type = "boolean", default = false },
    anonymous = {type = "string", default = "", func = check_user},
    global_credentials = {type = "boolean", default = false},
+    refresh_token_ttl = {required = false, type = "number", default = 1209600} -- original hardcoded value - 14 days


Do not allow nil here, so required = true.

This also means that a migration is needed to insert the default value in all existing entries, see this code for an example.

Because of the migration, we also cannot target the master branch, but must go to the next major release, which is in the next branch.

OK, gotcha, let me add the required flag, the migration and rebase/switch to next branch.

bob983 · 2017-10-11T14:49:48Z

@Tieske I added the migration, rebased against next and set the required option to true

Tieske

too much copy-pasta 😄

Tieske · 2017-10-23T08:12:53Z

kong/plugins/oauth2/migrations/postgres.lua

+          return config
+        end
+        if config.run_on_preflight == nil then
+          config.run_on_preflight = true


too much copy-pasta here, run_on_preflight is not being added here but the config option refresh_token_ttl.

🤕 oh my, Bob! Thanks

bob983 · 2017-10-23T12:43:41Z

OK, let's pretend the migration code didn't contain run_on_preflight config option ...

Tieske · 2017-10-23T12:58:07Z

final item seems docs, on https://github.com/Kong/getkong.org

bob983 · 2017-10-25T09:25:16Z

Updated documentation - Kong/docs.konghq.com#535

Tieske · 2017-10-25T21:13:59Z

This crossed another PR and now needs a rebase

Refresh token TTL used to be hardcoded to 14 days. This pose a problem with scenarios where the token/refresh_token is only used sporadically. This change adds new config option refresh_token_ttl that specify refresh token's TTL. If the value is nil or 0 it means keep forever. Fix Kong#2024

thibaultcha

I think this will do it. There are many aspects to this plugins that warrant a refactor/improvement but they would be outside the scope of this PR, which I think is suffering from it.

Thank for contributing this feature @bob983!

required changes addressed by contributor

thibaultcha · 2017-11-01T23:49:07Z

Thank you for the contribution @bob983 ! This shall be released in our next major version (0.12).

See Kong/kong#2942 From #535 Signed-off-by: Thibault Charbonnier <thibaultcha@me.com>

A `refresh_token` TTL used to be hard-coded to 14 days. This pose a problem with scenarios where the token/refresh_token is only used sporadically. This change adds new config option `refresh_token_ttl` that specifies a refresh token's TTL. If the value is `nil` or 0, it means keep forever. Fix #2024 From #2942

bob983 force-pushed the feat/plugins/oauth2/refresh-token-ttl branch 3 times, most recently from 126d113 to 72e62a9 Compare October 9, 2017 21:33

Tieske requested changes Oct 10, 2017

View reviewed changes

bob983 changed the base branch from master to next October 11, 2017 13:29

bob983 force-pushed the feat/plugins/oauth2/refresh-token-ttl branch from 72e62a9 to a709b4d Compare October 11, 2017 14:24

bob983 mentioned this pull request Oct 11, 2017

[oauth2] Specifying the TTL of refresh tokens #2024

Closed

Tieske previously requested changes Oct 23, 2017

View reviewed changes

bob983 force-pushed the feat/plugins/oauth2/refresh-token-ttl branch 2 times, most recently from 520e5c0 to fe143a7 Compare October 23, 2017 12:42

bob983 mentioned this pull request Oct 25, 2017

docs(plugin) add refresh_token_ttl to oauth2 plugin Kong/docs.konghq.com#535

Closed

bob983 force-pushed the feat/plugins/oauth2/refresh-token-ttl branch from fe143a7 to 735f72d Compare October 31, 2017 12:56

thibaultcha approved these changes Nov 1, 2017

View reviewed changes

thibaultcha merged commit 8b86700 into Kong:next Nov 1, 2017

bob983 deleted the feat/plugins/oauth2/refresh-token-ttl branch December 1, 2017 16:43

thibaultcha pushed a commit to Kong/docs.konghq.com that referenced this pull request Dec 19, 2017

docs(oauth2) add config.refresh_token_ttl property

51a1f98

See Kong/kong#2942 From #535 Signed-off-by: Thibault Charbonnier <thibaultcha@me.com>

thibaultcha pushed a commit to Kong/docs.konghq.com that referenced this pull request Jan 12, 2018

docs(oauth2) add config.refresh_token_ttl property

16993f6

See Kong/kong#2942 From #535 Signed-off-by: Thibault Charbonnier <thibaultcha@me.com>

thibaultcha pushed a commit to Kong/docs.konghq.com that referenced this pull request Jan 16, 2018

docs(oauth2) add config.refresh_token_ttl property

1e90602

See Kong/kong#2942 From #535 Signed-off-by: Thibault Charbonnier <thibaultcha@me.com>

thibaultcha pushed a commit to Kong/docs.konghq.com that referenced this pull request Jan 16, 2018

docs(oauth2) add config.refresh_token_ttl property

df2ae8f

See Kong/kong#2942 From #535 Signed-off-by: Thibault Charbonnier <thibaultcha@me.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

oauth2: Add custom TTL to refresh tokens #2942

oauth2: Add custom TTL to refresh tokens #2942

bob983 commented Oct 9, 2017 •

edited

Loading

bob983 commented Oct 9, 2017

Tieske Oct 10, 2017

bob983 Oct 10, 2017

bob983 commented Oct 11, 2017

Tieske left a comment

Tieske Oct 23, 2017

bob983 Oct 23, 2017

bob983 commented Oct 23, 2017

Tieske commented Oct 23, 2017

bob983 commented Oct 25, 2017

Tieske commented Oct 25, 2017

thibaultcha left a comment

thibaultcha commented Nov 1, 2017

oauth2: Add custom TTL to refresh tokens #2942

oauth2: Add custom TTL to refresh tokens #2942

Conversation

bob983 commented Oct 9, 2017 • edited Loading

Summary

Full changelog

Test cases

Issues resolved

Misc

bob983 commented Oct 9, 2017

Tieske Oct 10, 2017

Choose a reason for hiding this comment

bob983 Oct 10, 2017

Choose a reason for hiding this comment

bob983 commented Oct 11, 2017

Tieske left a comment

Choose a reason for hiding this comment

Tieske Oct 23, 2017

Choose a reason for hiding this comment

bob983 Oct 23, 2017

Choose a reason for hiding this comment

bob983 commented Oct 23, 2017

Tieske commented Oct 23, 2017

bob983 commented Oct 25, 2017

Tieske commented Oct 25, 2017

thibaultcha left a comment

Choose a reason for hiding this comment

thibaultcha commented Nov 1, 2017

bob983 commented Oct 9, 2017 •

edited

Loading