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

Change: check expiration of legacy tokens, reject if necessary #3645

Merged
merged 2 commits into from
Feb 9, 2024

Conversation

shreddedbacon
Copy link
Member

@shreddedbacon shreddedbacon commented Jan 31, 2024

General Checklist

  • Affected Issues have been mentioned in the Closing issues section
  • Documentation has been written/updated
  • PR title is ready for inclusion in changelog

Database Migrations

  • If your PR contains a database migation, it MUST be the latest in date order alphabetically

As we look to phase out the usage of legacy tokens, we need to be able to reject long lived or non-expiring legacy tokens initially.
This introduces two new environment variables that can be set in the API.

  • LEGACY_EXPIRY_MAX (default 3600s) - This is the maximum expiry that can be allowed to interact with the API
  • LEGACY_EXPIRY_REJECT (default false) - This is what controls if a legacy token is rejected based on the checks

If a legacy token has the exp field, the remaining duration of the token is calculated against the iss timestamp. If this duration is greater than the LEGACY_EXPIRY_MAX, then depending on the LEGACY_EXPIRY_REJECT setting, it will log, or log and reject the request.

If there is no exp field on the token, then depending on the LEGACY_EXPIRY_REJECT setting, it will log, or log and reject the request.

@shreddedbacon shreddedbacon marked this pull request as ready for review January 31, 2024 22:10
@shreddedbacon shreddedbacon added this to the 2.18.0 milestone Feb 1, 2024
@tobybellwood tobybellwood self-requested a review February 1, 2024 22:52
@shreddedbacon shreddedbacon marked this pull request as draft February 1, 2024 23:10
@shreddedbacon shreddedbacon marked this pull request as ready for review February 2, 2024 01:55
Copy link
Member

@rocketeerbkw rocketeerbkw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Discussed in meeting, looks good 👍

@shreddedbacon shreddedbacon force-pushed the legacy-expire branch 4 times, most recently from e286737 to 29a3be1 Compare February 6, 2024 02:09
Copy link
Member

@tobybellwood tobybellwood left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving additional logic to set internal tokens to 60sec

@tobybellwood tobybellwood merged commit 9060483 into main Feb 9, 2024
1 check passed
@tobybellwood tobybellwood deleted the legacy-expire branch February 9, 2024 03:13
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.

3 participants