fix: Surface all auth server errors during login in email input. #5319
Conversation
✅ Deploy Preview for dev-web-novu ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
| const { body } = await session.testAgent.post('/v1/auth/login').send({ | ||
| email: userCredentials.email, | ||
| password: userCredentials.password, | ||
| context('with OAuth', async () => { |
There was a problem hiding this comment.
All the above ☝️ are just indentation changes after adding a context.
There was a problem hiding this comment.
A new one for me! What is the difference between context and describe?
There was a problem hiding this comment.
context is a synonym for describe that is usually available in BDD style APIs. For example https://www.coreycleary.me/better-test-structuring-using-mochas-context
| return Array.isArray(error?.message) ? error?.message[0] : error?.message; | ||
| }, [error]); | ||
| const emailClientError = errors.email?.message; | ||
| let emailServerError = parseServerErrorMessage(error); |
There was a problem hiding this comment.
Similar changes can be applied to the register flow.
| @@ -41,7 +41,8 @@ export class Login { | |||
| throw new UnauthorizedException(`Account blocked, Please try again after ${blockedMinutesLeft} minutes`); | |||
| } | |||
|
|
|||
| if (!user.password) throw new ApiException('OAuth user login attempt'); | |||
| // TODO: Trigger a password reset flow automatically for existing OAuth users instead of throwing an error | |||
| if (!user.password) throw new ApiException('Please sign in using Github.'); | |||
There was a problem hiding this comment.
Note that returning different errors per auth flow results in User enumeration issues. User enumeration is mostly a privacy concern and should be tackled in a separate PR.
There was a problem hiding this comment.
Moreover, the error message mentions Github specifically. Currently, it's the only supported OAuth method. When more OAuth providers are added, the copywriting should be tweaked.
There was a problem hiding this comment.
Btw we have the Google OAuth logic but currently, it's enterprise-only and "disabled".
SokratisVidros
changed the title
SokratisVidros
force-pushed
the
surface_all_auth_error
branch
from
a11c827 to
6db74f3
Compare
This PR patches the following issue: 1. Sign up with Github with user A 2. Sign out 3. Try to sign in with the email of user A 4. The API returns a 400 error not displayed in the Login form. After this PR, all server auth errors will be displayed in the UI. It also contains some minor copywriting fixes and clean-ups.
SokratisVidros
force-pushed
the
surface_all_auth_error
branch
from
6db74f3 to
5741bf4
Compare
What change does this PR introduce?
This PR patches the following issue:
Why was this change needed?
After this PR, all server auth errors will be displayed in the UI.
It also contains some minor copywriting fixes and clean-ups.
Other information (Screenshots)
Before
Screen.Recording.2024-03-20.at.09.57.30.mov
After