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

Server should throw relevant errors when token is expired, so apps can refresh their tokens accordingly #5224

Closed
lucasbordeau opened this issue Apr 30, 2024 · 0 comments · Fixed by #5245
Labels
type: bug Something isn't working

Comments

@lucasbordeau
Copy link
Contributor

Bug Description

When a token is expired, the UserWorkspaceMiddleware won't throw any error and call next(), this is causing various other services to throw irrelevant errors like "Schema version mismatch".

Steps to reproduce :

  • set your token expiry in local at 30s or a short time
  • login and wait for the expiry time
  • try to send a request to the server.

Expected behavior

We should have the relevant errors thrown for each specific case. See with @Weiko for cases where we want next() to be called even if no token is present.

@lucasbordeau lucasbordeau added the type: bug Something isn't working label Apr 30, 2024
@lucasbordeau lucasbordeau changed the title Server should throw relevant errors when token is expired, so apps can refresh their token accordingly Server should throw relevant errors when token is expired, so apps can refresh their tokens accordingly Apr 30, 2024
Weiko added a commit that referenced this issue May 2, 2024
## Context
Currently, this middleware validates the token and stores the user,
workspace and cacheversion in the request object.
It only does so when a token is provided and ignores the middleware
logic if not. If the token is invalid or expired, the exception is
swallowed.

This PR removes the try/catch and adds an allowlist to skip the token
validation for operations executed while not signed-in.
I don't know a better way to do that with Nestjs. We can't easily add
the middleware per resolver without refactoring the flexible schema
engine so I'm doing it the other way around.

Fixes #5224
i-am-chitti pushed a commit to i-am-chitti/twenty that referenced this issue May 4, 2024
## Context
Currently, this middleware validates the token and stores the user,
workspace and cacheversion in the request object.
It only does so when a token is provided and ignores the middleware
logic if not. If the token is invalid or expired, the exception is
swallowed.

This PR removes the try/catch and adds an allowlist to skip the token
validation for operations executed while not signed-in.
I don't know a better way to do that with Nestjs. We can't easily add
the middleware per resolver without refactoring the flexible schema
engine so I'm doing it the other way around.

Fixes twentyhq#5224
arnavsaxena17 pushed a commit to arnavsaxena17/twenty that referenced this issue Oct 6, 2024
## Context
Currently, this middleware validates the token and stores the user,
workspace and cacheversion in the request object.
It only does so when a token is provided and ignores the middleware
logic if not. If the token is invalid or expired, the exception is
swallowed.

This PR removes the try/catch and adds an allowlist to skip the token
validation for operations executed while not signed-in.
I don't know a better way to do that with Nestjs. We can't easily add
the middleware per resolver without refactoring the flexible schema
engine so I'm doing it the other way around.

Fixes twentyhq#5224
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug Something isn't working
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

2 participants
@lucasbordeau and others