Fix Google Auth displays Status: 401 on screen #7659
Conversation
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
PR Summary
This PR modifies the exception handling in the server to redirect 401 Unauthorized errors to the front-end URL, addressing the issue of displaying a 401 status on screen during Google Auth cancellation.
- Modified
ApplyCorsToExceptionsclass inpackages/twenty-server/src/utils/apply-cors-to-exceptions.tsto handle 401 Unauthorized exceptions - Added redirection logic for 401 errors to
${process.env.FRONT_BASE_URL} - Potential security risk introduced by altering exception handling for authentication errors
- Assumes presence of
FRONT_BASE_URLenvironment variable, which needs proper setting and validation - May affect API consistency and error handling for other scenarios using 401 status codes
1 file(s) reviewed, 2 comment(s)
Edit PR Review Bot Settings | Greptile
| if (status===401 && exception.message === 'Unauthorized') { | ||
| response.redirect(`${process.env.FRONT_BASE_URL}`); | ||
| return; | ||
| } |
There was a problem hiding this comment.
logic: Redirecting on 401 may expose sensitive information. Consider returning a JSON response instead of redirecting.
| @@ -32,7 +32,10 @@ export class ApplyCorsToExceptions implements ExceptionFilter { | |||
|
|
|||
| const status = | |||
| exception instanceof HttpException ? exception.getStatus() : 500; | |||
|
|
|||
| if (status===401 && exception.message === 'Unauthorized') { | |||
There was a problem hiding this comment.
style: Use strict equality (===) for status comparison
|
@Bonapara what do you think? Maybe a snack bar in this case? |
|
@Pushpender1122 Thanks for the PR, waiting for product's input |
|
Sure |
|
Maybe we should redirect to |
|
It redirects to /welcome. If you go to xyz.com, it automatically navigates to /welcome. |
|
Sounds like a good behavior, wondering what @charlesBochet had in mind. |
|
Hi @charlesBochet and @Bonapara , are there any other changes you'd like me to make? |
| @@ -33,6 +34,7 @@ export class GoogleAuthController { | |||
|
|
|||
| @Get('redirect') | |||
| @UseGuards(GoogleProviderEnabledGuard, GoogleOauthGuard) | |||
| @UseFilters(AuthOAuthExceptionFilter) | |||
There was a problem hiding this comment.
using an adhoc filter to redirect instead of the ApplyCorsToExceptions globalFilter which is not meant to do that
| if (request.query.error === 'access_denied') { | ||
| throw new AuthException( | ||
| 'Google OAuth access denied', | ||
| AuthExceptionCode.OAUTH_ACCESS_DENIED, |
There was a problem hiding this comment.
throwing a precise Auth Exception exception in the guard
There was a problem hiding this comment.
Thanks @Pushpender1122. I have made a few changes that are more in the "nestjs" philosophy
|
/award 150 |
|
Awarding Pushpender1122: 150 points 🕹️ Well done! Check out your new contribution on oss.gg/Pushpender1122 |
|
Awarding Pushpender1122: 150 points 🕹️ Well done! Check out your new contribution on oss.gg/Pushpender1122 |
When the user presses the cancel button, the server sends the following response:
{"statusCode": 401, "message": "Unauthorized"}
Now, when the user clicks the cancel button, they are redirected to the home page for login.
Related Issue
Fixes #7584