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

fix: Drop expiry from internal only JWT #4207

Merged
merged 1 commit into from
Jan 25, 2025
Merged

Conversation

DafyddLlyr
Copy link
Contributor

@DafyddLlyr DafyddLlyr commented Jan 25, 2025

What's the problem?

Internal requests from the API to Hasura are failing due to expired JWTs. See recent failures in the #planx-notifications-internal Slack channel (OSL).

This is happening as the $api client is a singleton, generated once, along with its associated JWT. Calls made by users via the API pass along their JWTs, generated at login, along with their associated roles. The internal JWT used by the API to access Haura is used for various cron jobs, admin endpoints, and other utilities.

Some cron processes are passing as their rely on the admin secret and not the JWT.

We haven't hit this (at time of writing) on staging or production as it's not yet been 24hrs since a deploy.

What's the solution?

Remove the expiry on internal-only JWTs. This isn't actually required and it's only needed for user instantiated requests.

We could (should?) implement a process where this token is still time-limited, but can be automatically refreshed when expired. I'm just opting for a simple and pragmatic roll-back here just now.

@DafyddLlyr DafyddLlyr requested a review from a team January 25, 2025 20:31
@DafyddLlyr DafyddLlyr changed the title chore: Drop expiry from internal only JWT fix: Drop expiry from internal only JWT Jan 25, 2025
@DafyddLlyr DafyddLlyr marked this pull request as ready for review January 25, 2025 20:31
Copy link

github-actions bot commented Jan 25, 2025

Removed vultr server and associated DNS entries

Copy link
Member

@jessicamcinchak jessicamcinchak left a comment

Choose a reason for hiding this comment

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

Thanks for spotting & resolving-for-now !!

@DafyddLlyr DafyddLlyr merged commit 5607aff into main Jan 25, 2025
13 checks passed
@DafyddLlyr DafyddLlyr deleted the dp/api-role-jwt-expiry branch January 25, 2025 20:50
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.

2 participants