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 JWT out-of-sync detection middleware and move it into LexQueries #1218

Merged
merged 11 commits into from
Nov 13, 2024

Conversation

myieye
Copy link
Contributor

@myieye myieye commented Nov 6, 2024

Resolves #1180

This PR moves the JWT refreshing code into LexQueries.
It's no longer quite as generic as it was before (i.e. it won't automatically trigger based on GQL types), but it's much less magical, not so tricky to wire-up and easier to test.

The middleware was pretty buggy before. The unit tests should demonstrate that it handles every situation we're expecting our users to maybe land in.

Also, previously we used the old token for the scope of the request and only refreshed it at the end.
Now we look for updates up front and use the current state for the scope of the GQL Query.

Projecting a few Id's and UserId's lets us write less complicated code, so I thought it was worth it.

Copy link
Contributor Author

myieye commented Nov 6, 2024

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @myieye and the rest of your teammates on Graphite Graphite

Copy link

github-actions bot commented Nov 6, 2024

C# Unit Tests

90 tests   90 ✅  5s ⏱️
14 suites   0 💤
 1 files     0 ❌

Results for commit 4147991.

♻️ This comment has been updated with latest results.

@myieye myieye force-pushed the bug/optimize-middleware branch from d1d36ff to 5db3179 Compare November 6, 2024 20:58
@myieye myieye changed the title Fix org sorting (#1183) Fix middleware and move it into LexQueries Nov 6, 2024
@myieye myieye marked this pull request as ready for review November 6, 2024 20:58
Copy link

github-actions bot commented Nov 6, 2024

UI unit Tests

12 tests  ±0   12 ✅ ±0   0s ⏱️ ±0s
 4 suites ±0    0 💤 ±0 
 1 files   ±0    0 ❌ ±0 

Results for commit 4147991. ± Comparison against base commit 9a099a3.

♻️ This comment has been updated with latest results.

@myieye myieye changed the title Fix middleware and move it into LexQueries Fix JWT out-of-sync detection and move it into LexQueries Nov 6, 2024
@myieye myieye changed the title Fix JWT out-of-sync detection and move it into LexQueries Fix JWT out-of-sync detection middleware and move it into LexQueries Nov 6, 2024
@myieye myieye force-pushed the bug/optimize-middleware branch from 5db3179 to b597b52 Compare November 7, 2024 12:20
This ensures our "middleware" can detect when the user was added to the project and stops hiding members on non-confidential projects
@myieye myieye force-pushed the bug/optimize-middleware branch from b597b52 to a36419e Compare November 7, 2024 13:01
Copy link
Collaborator

@hahn-kev hahn-kev left a comment

Choose a reason for hiding this comment

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

I'd like to fast track this as there's an open issue in production that it fixes.

backend/LexBoxApi/GraphQL/LexQueries.cs Outdated Show resolved Hide resolved
backend/LexBoxApi/Services/PermissionService.cs Outdated Show resolved Hide resolved
backend/Testing/ApiTests/GqlMiddlewareTests.cs Outdated Show resolved Hide resolved
@myieye myieye requested a review from hahn-kev November 12, 2024 12:37
hahn-kev
hahn-kev previously approved these changes Nov 13, 2024
@myieye myieye merged commit 0fcb7ad into develop Nov 13, 2024
17 checks passed
@myieye myieye deleted the bug/optimize-middleware branch November 13, 2024 12:12
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.

JWT refresh middleware is buggy and out of date
2 participants