feat: Refresh token expiration window

[bill-robbins-ss]

• Introduce optional configuration value oauth2.refresh_token_rotation.grace_period that can be set as a duration (1m,
• Add a new migration: boolean used SQL column in hydra_oauth2_refresh

When grace_period has positive duration the act of using a refresh token will not invalidate the token and instead mark it as used in the database and set its expiration time to the UTCNOW + the value of the grace period. This will allow a client to continue to use a refresh token for the duration of the grace period.

Related issue(s)

#1831 Fosite 255

• [x] I have read the contributing guidelines.
• [x] I have referenced an issue containing the design document if my change introduces a new feature.
• [x] I am following the contributing code guidelines.
• [x] I have read the security policy.
• [x] I confirm that this pull request does not address a security vulnerability. If this pull request addresses a security. vulnerability, I confirm that I got green light (please contact [email protected]) from the maintainers to push the changes.
• [x] I have added tests that prove my fix is effective or that my feature works.
• [x] I have added or changed the documentation.

Further Comments

This change includes a modification to Fosite in PR 634

[aeneasr]

Is this good for another review @bill-robbins-ss ? :)

[bill-robbins-ss]

@aeneasr yes it's ready :)

[aeneasr]

@bill-robbins-ss I fixed the conflicts and got parts of the CI to pass again, however there seems to be an linting issue. Would it be possible for you to take a look?

https://app.circleci.com/pipelines/github/ory/hydra/3404/workflows/bd1ecd98-8d78-4ea0-835a-5b45ff9a7aae/jobs/34581

ps: For motivation, we have merged several large-scale changes to Ory Hydra so there's a good chance this will land very quickly as well :)

[bill-robbins-ss]

@aeneasr this is great, I'll be checking it out this week hopefully getting those rules to pass. Do you know how releasing will be done? We'll want to upgrade our instance to take advantage of the great new things!

[aeneasr]

Once this is merged we'll do a new minor release

[CLAassistant]

All committers have signed the CLA.

[bill-robbins-ss]

hey @aeneasr I fixed the linting errors that came from me, but he other 3 are on code that I didn't touch. Please advise.

[bill-robbins-ss]

Hey @aeneasr thank you for checking in on this.

1. I can introduce a new struct to handle the new table schema, does that seem reasonable?
2. I would be delighted to add e2e tests, I think they belong here?
3. It looks like you added documentation with this change. If there is more that needs to be added just let me know where and I'll add it !

Also, I saw that you removed the SetConfig method - I introduced it for the test refresh token enters grace period when configured. Is that same method available somewhere else ?

[aeneasr]

Thank you @bill-robbins-ss for the fast response and keeping engaged. I have this PR on my prio list now and will be more active reviewing it. You can also ping me on Slack if I am too slow :)

Regarding your questions:

I can introduce a new struct to handle the new table schema, does that seem reasonable?

It does! I would encourage you though to maybe rebase this PR right away with https://github.com/ory/hydra/pull/2796 to avoid any big conflicts during merge.

I would be delighted to add e2e tests, I think they belong here?

Exactly, I would suggest to cover the following use cases:

1. [ ] Refreshing a refresh token without grace period (I believe we have this already)
2. [ ] Refreshing a refresh token without grace period that is expired (I believe we have this already)
3. [ ] Refreshing a refresh token with grace period
4. [ ] Refreshing a refresh token with grace period that is expired
5. [ ] Refreshing a refresh token 1 (before expiry) 2 (after expiry) 3 (after expiry) with grace period

We also need to cover revokation and deleting the consent:

1. [ ] Revoke a refresh token that was granted before expiry
2. [ ] Revoke consent of refresh token that was granted before expiry
3. [ ] Revoke a refresh token that was granted before expiry with grace period
4. [ ] Revoke consent of refresh token that was granted before expiry with grace period
5. [ ] Revoke a refresh token that was granted before expiry with grace period that was expired
6. [ ] Revoke consent of refresh token that was granted before expiry with grace period that was expired
7. [ ] Revoke a 1 (before expiry) 2 (after expiry) 3 (after expiry) refresh token grace period or consent
8. [ ] Revoke consent of 1 (before expiry) 2 (after expiry) 3 (after expiry) refresh token grace period or consent
9. [ ] For the token revokation and consent deletion we need to check whether all tokens have been revoked. It should not be possible to revoke just one refresh token "chain".

Checking whether we can refresh graceful tokens is important too:

1. [ ] Refresh a 1 (before expiry) 2 (after expiry) 3 (after expiry) with grace period or consent that was granted normally with grace period. We should refresh at least 2 or 3 times to ensure that the "refresh token tree" works correctly.

As well as introspection:

1. [ ] Refreshing a 1 (before expiry) 2 (after expiry) 3 (after expiry) times with grace period and introspecting their access tokens

I believe most functions are there already to test this, so it should not be too much work hopefully.

Besides e2e tests, it probably also makes sense to start with some Go test first, as that will make debugging so much easier for you!

https://github.com/ory/hydra/blob/8a966e8866d0a054f4d6cd0581db42690ba78805/oauth2/oauth2_auth_code_test.go#L277

It looks like you added documentation with this change. If there is more that needs to be added just let me know where and I'll add it !

Apologies, I got in review mode and am not fully used to the new docs system yet 😓

Also, I saw that you removed the SetConfig method - I introduced it for the test refresh token enters grace period when configured. Is that same method available somewhere else ?

Yes, it's now defined on the interface level so no reflection is needed!

[aeneasr]

@bill-robbins-ss please let me know when you think that this is good for another review :)

[yuekun0707]

Hi, any update here? Is configuration described here https://www.ory.sh/docs/hydra/guides/token-expiration#refresh-token-rotation ready for using?

[aeneasr]

Looks like that made it to prod on mistake, the feature is not yet usable!

[yuekun0707]

@aeneasr thank you for your reply! I really need feature like this. We use hydra in our project and it works well. But now we want to connect to google assistant by account linking. There is a problem with the refresh token rotation. The error from google like this: "Refresh token has been rotated. This is not forbidden, however we do not see much benefit that rotating the refresh token can provide but the potential problem it has. We also tried to refresh token with the old refresh token after it has been rotated. Refresh didn't work, this means partner invalidated the old refresh token right after the rotation. Partners shall only invalidate the old refresh token after seeing we use the new one to ensure we got it successfully." So, is there any way I can try for this?

[misl6]

Hi @aeneasr and @bill-robbins-ss ! As other users pointed out, this is pretty much a requirement to pass the Google Assistant Actions certification.

Is this feature something scheduled and that we may expect in a near future? There's anything we can do on our side (as users) to help you to make that happen?

[StarAurryon]

Hi @aeneasr, I am willing to continue the work on the coming months as we are currently experiencing network token loss when Windows goes to sleep with our app and also with our android / ios app.

Did @bill-robbins-ss signed the Ory Code Agreement so that I can reuse his code and pursue the implementation ?

Kind regards,