Login Fixes #475
Login Fixes #475
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
| const emailInputRef = useAutoFocus<HTMLInputElement>(true); | ||
| const { referralCode } = useReferral(); | ||
|
|
||
| const getCallbackUrl = () => { |
| return true; | ||
| } catch (error) { | ||
| // Log detailed error information | ||
| const errorType = error instanceof Error ? error.message : 'AuthenticationFailed'; |
There was a problem hiding this comment.
Should standardize error handling and use consistent error classes (:144-150)
Good usage example in auth.service.ts and services/publication.service.ts (Lines 52-54, 69-76):
// Should be something like this:
const errorType = error instanceof Error ? error.message : 'AuthenticationFailed';
console.error('[Auth] Google OAuth error', {
error: errorType,
errorType: error instanceof Error ? error.constructor.name : typeof error,
stack: error instanceof Error ? error.stack : undefined,
timestamp: new Date().toISOString(),
});
throw new NextAuthError(errorType, 'OAUTH_ERROR');
// add this to the top after imports
export class NextAuthError extends Error {
constructor(
message: string,
public readonly code: string
) {
super(message);
this.name = 'NextAuthError';
}
}
There was a problem hiding this comment.
Good catch! I agree!
services/auth.service.ts
Outdated
| throw new AuthError(errorMsg || 'Registration failed', status); | ||
| if (error instanceof ApiError) { | ||
| const data = error.errors || {}; | ||
| const errorMsg = Object.values(data as Record<string, string[]>)?.[0]?.[0]; |
There was a problem hiding this comment.
This could perhaps be refactored for slightly better readability and consistency:
const errorValues = Object.values(data as Record<string, string[]>)?.[0];
const errorMsg = Array.isArray(errorValues)
? errorValues[0]
: typeof errorValues === 'string'
? errorValues
: 'Registration failed';
There was a problem hiding this comment.
Makes sense to me, ill update and test it now!
| import { AuthSharingService } from '@/services/auth-sharing.service'; | ||
|
|
||
| // Debug flag - set to true to enable detailed authentication logging | ||
| const DEBUG_AUTH = true; |
There was a problem hiding this comment.
I see this change has been here a while but it feels like security risk so maybe we should change it to something like:
const DEBUG_AUTH = process.env.NODE_ENV === 'development';
There was a problem hiding this comment.
I'll defer to @yattias on whether this is something he wants changed currently.
There was a problem hiding this comment.
We can remove it in another PR
|
| }, | ||
| callbacks: { | ||
| async signIn({ user, account, profile }) { | ||
| async signIn({ user, account, profile, email, credentials }) { |
There was a problem hiding this comment.
do we need new props?
| errorCode === 'OAuthAccountNotLinked' || | ||
| errorCode === 'OAuthSignin' || | ||
| errorCode === 'OAuthCallback' || | ||
| errorCode === 'AccessDenied' || | ||
| errorCode === 'Callback' || | ||
| errorCode === 'OAuthCreateAccount' || |
There was a problem hiding this comment.
where are these errors specified?
| if (response.status === 400 || response.status === 403 || response.status === 409) { | ||
| throw new Error('OAuthAccountNotLinked'); | ||
| } |
There was a problem hiding this comment.
I think we are doing wrong mapping here. because the error we are trying to handle is resulting with 400 error (When trying to register with an email that's already registered).
| // Return false for OAuthAccountNotLinked to trigger consistent error flow | ||
| // This ensures the error is properly handled by the redirect callback | ||
| if (errorType === 'OAuthAccountNotLinked') { | ||
| return false; |
There was a problem hiding this comment.
According to the NextAuth documentation, the signIn callback can return a URL string to redirect users directly. This would eliminate the current redirect chain and provide a cleaner user experience.
Current approach:
- Return false → NextAuth redirects to
/auth/error - Error page redirects to
/auth/signinwith error param - Sign-in page processes error code and shows message
Proposed approach:
- Return URL directly:
/auth/signin?errorMessage=Please use email and password to login to your account. - Sign-in page reads
errorMessageparam and displays it - Remove
errorMessageparam from URL after rendering to clean up the URL
Benefits:
- Eliminates unnecessary redirect chain
- Cleaner, more maintainable code
- Custom error messages without complex error code mapping
Issue
One large issue we noticed, was that if a user tried to log in using Google's SSO when they already have a base email/password account, it would fail and lead to a blank page. A blank page can be confusing to the user, so the solution would be to redirect them to our individual login page. We ran into registration issues in the past as well with this fix, which is now resolved after thorough testing.
Before
After