feat: add email verification on signup#1644
feat: add email verification on signup#1644rakshityadav1868 wants to merge 20 commits intoruxailab:developfrom
Conversation
Signed-off-by: Rakshit Yadav <yadavrakshit60@gmail.com>
github-actions
bot
added
backend
high-complexity
needs-description
new-feature
There was a problem hiding this comment.
Pull request overview
Adds an email-verification gate to the auth flow so newly created accounts must verify their email before accessing authenticated areas. This spans the Vue frontend (new verify page + routing), Vuex auth enforcement, and the Firebase Functions email sender + new template.
Changes:
- Added a new
/verify-emailpublic route andVerifyEmailViewfor resend/change-email UX. - Enforced
emailVerifiedchecks during sign-in/auto-sign-in and redirected unverified users to/verify-email. - Added a new
emailVerificationemail template and updated the Cloud Function + frontend email sender to usehttpsCallable.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 12 comments.
Show a summary per file
| File | Description |
|---|---|
| src/shared/controllers/EmailController.js | Switches email sending from raw HTTP to Firebase httpsCallable. |
| src/router/modules/public.js | Adds the /verify-email public route. |
| src/features/auth/views/VerifyEmailView.vue | New verification UI (status, resend, change email). |
| src/features/auth/views/SignUpView.vue | Redirects new signups to /verify-email. |
| src/features/auth/views/SignInView.vue | Redirects to /verify-email when sign-in fails due to unverified email. |
| src/features/auth/store/Auth.js | Sends verification email on signup; blocks sign-in if not verified; adds resend action. |
| src/features/auth/controllers/AuthController.js | Adds sendVerificationEmail helper using EmailController/template. |
| src/app/router/index.js | Updates global route guard to route unverified users to /verify-email. |
| src/app/plugins/locales/en.json | Adds i18n strings for the verification UI. |
| functions/src/templates/mails/emailVerification.html | Adds verification email HTML template. |
| functions/src/https/email.js | Adds backend support for verification template + callable payload handling. |
Comments suppressed due to low confidence (1)
functions/src/https/email.js:74
- In a callable function, returning
errfrom the catch block will resolve successfully on the client, so the frontend may report “Email sent successfully” even when sending fails. Throw afunctions.https.HttpsErrorinstead sohttpsCallablerejects and the client can handle the failure properly.
} catch (err) {
logger.error('Error sending email:', { error: err });
return err;
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| async updateEmail() { | ||
| try { | ||
| if (!this.newEmail || !this.newEmail.includes('@')) { | ||
| this.modalError = this.$t('auth.invalidEmail') | ||
| return | ||
| } | ||
|
|
||
| this.isUpdatingEmail = true | ||
| this.modalError = null | ||
|
|
||
| // Update email in Firebase | ||
| await updateEmail(auth.currentUser, this.newEmail) | ||
|
|
There was a problem hiding this comment.
The updateEmail method name shadows the imported Firebase updateEmail function. Inside this method, updateEmail(...) will resolve to the method itself (recursive call) and can cause a stack overflow instead of updating Firebase. Rename the component method (e.g., onUpdateEmail) or alias the import.
|
|
||
| async logout() { | ||
| try { | ||
| await this.logout() |
There was a problem hiding this comment.
This logout method overrides the Vuex-mapped logout action (added by ...mapActions) and then calls this.logout() recursively, causing infinite recursion. Rename one of them or call the store action explicitly (e.g., this.$store.dispatch('logout')).
| await this.logout() | |
| await this.$store.dispatch('Auth/logout') |
| mounted() { | ||
| // Ensure user is logged in | ||
| onAuthStateChanged(auth, (user) => { | ||
| if (!user) { | ||
| this.$router.push('/signin') | ||
| return | ||
| } | ||
| this.initializeVerification() | ||
| }) | ||
| }, |
There was a problem hiding this comment.
The verification flow currently only polls auth.currentUser.reload(); it never processes the email action link (mode=verifyEmail, oobCode) that Firebase generates when handleCodeInApp is enabled. Without calling applyActionCode, users who open the link may never get verified. Handle the action link parameters on mount and update UI accordingly.
| // Check verification status every 3 seconds | ||
| this.verificationCheckInterval = setInterval(() => { | ||
| this.checkEmailVerificationStatus() | ||
| }, 3000) |
There was a problem hiding this comment.
Polling auth.currentUser.reload() every 3 seconds can generate a lot of network traffic and may hit Firebase rate limits in real usage. Consider using a longer interval, adding a guard to prevent overlapping reloads, and/or switching to a user-triggered “I’ve verified” check.
| // Check verification status every 3 seconds | |
| this.verificationCheckInterval = setInterval(() => { | |
| this.checkEmailVerificationStatus() | |
| }, 3000) | |
| // Avoid creating multiple verification check intervals | |
| if (this.verificationCheckInterval) { | |
| return | |
| } | |
| // Check verification status every 15 seconds | |
| this.verificationCheckInterval = setInterval(() => { | |
| this.checkEmailVerificationStatus() | |
| }, 15000) |
| const actionCodeSettings = { | ||
| url: `${process.env.SITE_URL}/verify-email`, | ||
| handleCodeInApp: true, | ||
| } |
There was a problem hiding this comment.
Avoid automated semicolon insertion (90% of all statements in the enclosing function have an explicit semicolon).
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 11 out of 11 changed files in this pull request and generated 7 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| mounted() { | ||
| // Ensure user is logged in | ||
| onAuthStateChanged(auth, (user) => { | ||
| if (!user) { | ||
| this.$router.push('/signin') | ||
| return | ||
| } |
There was a problem hiding this comment.
This view forces a redirect to /signin when onAuthStateChanged reports no user. That prevents users from verifying via the email link in cases where they open the link in a different browser/device (i.e., not already signed in). If you intend to use Firebase action links (oobCode), handle them here (e.g., applyActionCode) and avoid requiring an authenticated session just to verify.
|
|
||
| // Show success message using Vuex toast | ||
| this.$store.commit('SET_TOAST', { | ||
| message: this.$t('auth.verificationEmailSent'), | ||
| type: 'success', | ||
| }) |
There was a problem hiding this comment.
sendVerificationEmail Vuex action already commits a success toast (auth.verificationEmailSent). This method commits the same toast again after awaiting the action, which will likely show duplicate notifications. Consider removing the local SET_TOAST commit here or adding a silent option to the action.
| // Show success message using Vuex toast | |
| this.$store.commit('SET_TOAST', { | |
| message: this.$t('auth.verificationEmailSent'), | |
| type: 'success', | |
| }) |
| mounted() { | ||
| // Ensure user is logged in | ||
| onAuthStateChanged(auth, (user) => { | ||
| if (!user) { | ||
| this.$router.push('/signin') | ||
| return | ||
| } | ||
| this.initializeVerification() | ||
| }) | ||
| }, |
There was a problem hiding this comment.
onAuthStateChanged returns an unsubscribe function, but it's not stored/called. Navigating away and back to this page can register multiple listeners and cause duplicate redirects/interval setup. Save the unsubscribe callback and invoke it in beforeUnmount.
|
|
||
| async logout() { | ||
| try { | ||
| await this.logout() |
There was a problem hiding this comment.
...mapActions(['sendVerificationEmail', 'logout']) maps an action named logout, but this component also declares a logout() method. The local method overrides the mapped action and then calls this.logout() recursively, causing infinite recursion/stack overflow. Rename the local method (e.g. onLogout) or map the action under a different name.
| await this.logout() | |
| // Dispatch the Vuex logout action directly to avoid recursive self-calls | |
| await this.$store.dispatch('Auth/logout') |
| @@ -13,10 +15,11 @@ export default class EmailController { | |||
| * @returns {Promise<{success: boolean, message: string}>} Result of email send operation | |||
| */ | |||
| async send(payload) { | |||
| try { | |||
| await axios.post(process.env.VUE_APP_CLOUD_FUNCTIONS_URL + '/sendEmail', { | |||
| data: payload, | |||
| }) | |||
| try { | |||
| // Use Firebase SDK callable function instead of HTTP | |||
| // Firebase callable wraps the argument automatically, so pass payload directly | |||
| const sendEmailFunction = httpsCallable(fbFunctions, 'sendEmail') | |||
| const response = await sendEmailFunction(payload) | |||
| return { success: true, message: 'Email sent successfully.' } | |||
There was a problem hiding this comment.
axios is no longer used in this file, and response is assigned but never read. This will typically fail linting/CI in Vue projects; remove the unused import/variable or use the callable response (e.g., for error messages).
| // Use Firebase SDK callable function instead of HTTP | ||
| // Firebase callable wraps the argument automatically, so pass payload directly | ||
| const sendEmailFunction = httpsCallable(fbFunctions, 'sendEmail') | ||
| const response = await sendEmailFunction(payload) |
There was a problem hiding this comment.
Unused variable response.
| const response = await sendEmailFunction(payload) | |
| await sendEmailFunction(payload) |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 11 out of 11 changed files in this pull request and generated 4 comments.
Comments suppressed due to low confidence (1)
functions/src/https/email.js:74
- On error, this callable returns the raw
errobject instead of throwing anHttpsError. WithhttpsCallable, returning an error object will often serialize poorly and the client may treat the call as a success. Throw anHttpsError(and avoid leaking internal SMTP errors) so callers can reliably handle failures.
} catch (err) {
logger.error('Error sending email:', { error: err });
return err;
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| export const sendEmail = functions.onCall({ | ||
| handler: async (data) => { | ||
| const content = data.data; | ||
| // Firebase callable passes the argument directly | ||
| const content = data.data || data; | ||
|
|
There was a problem hiding this comment.
In Firebase Functions v2 onCall, the handler receives a request object with a .data field. Using const content = data.data || data can set content to the full request object when .data is missing/empty, which will break later reads like content.to/content.template. Use const content = data.data (and validate required fields) instead of falling back to the request object.
| }, | ||
|
|
||
| mounted() { | ||
| // Ensure user is logged in | ||
| onAuthStateChanged(auth, (user) => { | ||
| if (!user) { | ||
| this.$router.push('/signin') | ||
| return | ||
| } | ||
| this.initializeVerification() | ||
| }) | ||
| }, | ||
|
|
||
| beforeUnmount() { | ||
| if (this.verificationCheckInterval) { | ||
| clearInterval(this.verificationCheckInterval) |
There was a problem hiding this comment.
onAuthStateChanged returns an unsubscribe function, but it isn't captured/cleaned up. Since this component also starts an interval, leaving the auth listener active after unmount can cause memory leaks and unexpected callbacks; store the unsubscribe and call it in beforeUnmount() (alongside clearing the interval).
|
|
||
| // Show success message using Vuex toast | ||
| this.$store.commit('SET_TOAST', { | ||
| message: this.$t('auth.verificationEmailSent'), | ||
| type: 'success', | ||
| }) |
There was a problem hiding this comment.
resendVerificationEmail() commits a success toast after dispatching sendVerificationEmail, but the Vuex action already commits its own success toast. This will likely show duplicate notifications; keep the toast in one place (store or view).
| // Show success message using Vuex toast | |
| this.$store.commit('SET_TOAST', { | |
| message: this.$t('auth.verificationEmailSent'), | |
| type: 'success', | |
| }) |
| computed: { | ||
| ...mapState({ | ||
| currentUser: state => state.user, |
There was a problem hiding this comment.
mapState('Auth', ['currentUser']) maps a currentUser state field that doesn't exist in the Auth module (it uses state.user). This computed prop is also unused in the component; remove it or map the correct field to avoid confusion.
… verification redirects. Prevents authenticated users from seeing signin page after refresh. Signed-off-by: Rakshit Yadav <yadavrakshit60@gmail.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 11 out of 11 changed files in this pull request and generated 7 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| beforeUnmount() { | ||
| if (this.verificationCheckInterval) { | ||
| clearInterval(this.verificationCheckInterval) | ||
| } | ||
| }, |
There was a problem hiding this comment.
beforeUnmount() clears the verification polling interval, but the onAuthStateChanged listener created in mounted() is never unsubscribed. Store the unsubscribe function and invoke it here to avoid leaking listeners across navigations.
| @@ -2,11 +2,11 @@ import { admin, functions } from "../f.firebase.js"; | |||
| import nodemailer from "nodemailer"; | |||
There was a problem hiding this comment.
logger is referenced later in this file but is not imported anymore, which will throw ReferenceError: logger is not defined at runtime and break email sending. Re-add import logger from "../utils/logger.js"; (as done in other functions files) or remove/replace the logging calls.
| import nodemailer from "nodemailer"; | |
| import nodemailer from "nodemailer"; | |
| import logger from "../utils/logger.js"; |
| // Use Firebase SDK callable function instead of HTTP | ||
| // Firebase callable wraps the argument automatically, so pass payload directly | ||
| const sendEmailFunction = httpsCallable(fbFunctions, 'sendEmail') | ||
| const response = await sendEmailFunction(payload) |
There was a problem hiding this comment.
The callable result is not inspected; send() always returns { success: true } after awaiting the function. If the backend returns an error payload (or you later return a structured response), this will still report success. Use response.data (or rely on the function throwing HttpsError) to determine success/failure.
| const response = await sendEmailFunction(payload) | |
| const response = await sendEmailFunction(payload) | |
| const data = response && response.data ? response.data : null | |
| // If the backend returns a structured result (e.g. { success, message }), | |
| // respect it; otherwise fall back to the default success response. | |
| if (data && typeof data.success === 'boolean') { | |
| return { | |
| success: data.success, | |
| message: typeof data.message === 'string' | |
| ? data.message | |
| : data.success | |
| ? 'Email sent successfully.' | |
| : 'Failed to send email.' | |
| } | |
| } |
| "verifyEmailTitle": "Verify Your Email", | ||
| "verifyEmailSubtitle": "We've sent a verification link to your email", | ||
| "emailLabel": "Email", | ||
| "verifyingEmail": "Verifying...", | ||
| "checkEmailTitle": "Check Your Email", |
There was a problem hiding this comment.
The auth store references i18n keys that are not defined in this file (e.g., auth.emailNotVerified and auth.errorSendingVerification). Add these translations under the auth section so users don’t see raw keys in toasts.
| mounted() { | ||
| // Ensure user is logged in | ||
| onAuthStateChanged(auth, (user) => { | ||
| if (!user) { | ||
| this.$router.push('/signin') |
There was a problem hiding this comment.
onAuthStateChanged returns an unsubscribe function, but it is not stored. If the user navigates away/back, this can accumulate listeners and duplicate callbacks. Capture the unsubscribe returned by onAuthStateChanged in mounted() and call it from beforeUnmount().
| return 'Email sent successfully.'; | ||
| } catch (err) { | ||
| logger.error('Error sending email:', { error: err }); | ||
| return err; |
There was a problem hiding this comment.
The Cloud Function returns err from the catch block instead of throwing an HttpsError. That means the callable can resolve successfully even when sending fails, and the frontend may show success incorrectly. Prefer throwing functions.https.HttpsError (or returning a structured { success: false, message } and handling it client-side).
| return err; | |
| throw new functions.https.HttpsError( | |
| "internal", | |
| "Failed to send email.", | |
| err | |
| ); |
src/app/router/index.js
Outdated
|
|
||
| // Special case: /verify-email only accessible if user email not verified | ||
| if (to.path === '/verify-email') { | ||
| if (!user || (user && user.emailVerified === true)) { |
There was a problem hiding this comment.
This use of variable 'user' always evaluates to true.
| if (!user || (user && user.emailVerified === true)) { | |
| if (!user || user.emailVerified === true) { |
Refactor router logic to allow access to public pages for logged-in users with unverified emails.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 21 out of 21 changed files in this pull request and generated 10 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| htmlTemplate = fs.readFileSync(templatePath, "utf-8"); | ||
| htmlTemplate = htmlTemplate | ||
| .replace("{{verificationLink}}", link) | ||
| .replace("{{userName}}", content.data.userName || 'User'); |
There was a problem hiding this comment.
Potential null/undefined access issue: If content.data is undefined, accessing content.data.userName will throw an error. Consider adding a null check like content.data?.userName or (content.data && content.data.userName) to make this more defensive.
| .replace("{{userName}}", content.data.userName || 'User'); | |
| .replace("{{userName}}", content.data?.userName || 'User'); |
| async sendVerificationEmail(email, userName) { | ||
| try { | ||
| const emailController = new EmailController() | ||
| await emailController.send({ | ||
| to: email, | ||
| subject: 'Verify Your Email Address', | ||
| template: 'emailVerification', | ||
| data: { | ||
| userName: userName || email, | ||
| }, | ||
| }) | ||
| } catch (err) { | ||
| console.error('Error sending verification email:', err) | ||
| throw err | ||
| } | ||
| } |
There was a problem hiding this comment.
The new sendVerificationEmail method lacks test coverage. Since this codebase has comprehensive test coverage for AuthController methods (as seen in tests/unit/AuthController.spec.js), a test should be added for this new method to ensure it correctly calls EmailController with the proper parameters and handles errors appropriately.
| async updateEmail() { | ||
| try { | ||
| if (!this.newEmail || !this.newEmail.includes('@')) { |
There was a problem hiding this comment.
The email validation is too simplistic. Using !this.newEmail.includes('@') only checks for the presence of an @ symbol, which will allow invalid emails like '@example.com', 'test@', or 'test@@example'. Consider using a proper email validation regex or the browser's built-in email validation through the input type attribute, which is already set to type="email".
| async updateEmail() { | |
| try { | |
| if (!this.newEmail || !this.newEmail.includes('@')) { | |
| isValidEmail(email) { | |
| const emailPattern = /^[^\s@]+@[^\s@]+\.[^\s@]+$/ | |
| return emailPattern.test(email) | |
| }, | |
| async updateEmail() { | |
| try { | |
| if (!this.isValidEmail(this.newEmail)) { |
| await authController.sendVerificationEmail(user.email, user.email) | ||
| } catch (emailErr) { | ||
| console.warn('Failed to send verification email:', emailErr) | ||
| // Don't fail signup if email sending fails |
There was a problem hiding this comment.
In the signup flow, when sending the verification email fails, the error is only logged with a console.warn and doesn't propagate to the user. While the comment says "Don't fail signup if email sending fails", users should still be notified that the verification email wasn't sent so they can request a resend. Consider showing a toast notification to inform the user.
| // Don't fail signup if email sending fails | |
| // Don't fail signup if email sending fails, but inform the user | |
| showError(i18n.global.t('auth.verificationEmailFailed')) |
|
|
||
| async logout() { | ||
| try { | ||
| await this.logout() |
There was a problem hiding this comment.
This method has an infinite recursion bug. The method is calling itself (await this.logout()) instead of calling the Vuex action. Since logout is already mapped from Vuex actions using ...mapActions(['sendVerificationEmail', 'logout']), this should directly call the action without the await and this prefix, or you should rename this method to avoid the conflict.
| await this.logout() | |
| await this.$store.dispatch('logout') |
| // Check if email is verified | ||
| if (!user.emailVerified) { | ||
| commit('SET_TOAST', { | ||
| message: i18n.global.t('auth.emailNotVerified'), |
There was a problem hiding this comment.
The translation key 'auth.emailNotVerified' is used in the code but is not defined in the locale file. This will cause the translation to display the key itself instead of the translated message. Add this key to the locale file with an appropriate message like "Please verify your email address to continue".
| message: i18n.global.t('auth.emailNotVerified'), | |
| message: 'Please verify your email address to continue', |
| }) | ||
| } catch (err) { | ||
| commit('SET_TOAST', { | ||
| message: i18n.global.t('auth.errorSendingVerification'), |
There was a problem hiding this comment.
The translation key 'auth.errorSendingVerification' is used but not defined in the locale file. Add this key with an appropriate error message like "Failed to send verification email. Please try again later."
| message: i18n.global.t('auth.errorSendingVerification'), | |
| message: i18n.global.t('errors.globalError'), |
| if (err.message === 'EMAIL_NOT_VERIFIED') { | ||
| throw err | ||
| } |
There was a problem hiding this comment.
The error handling for the EMAIL_NOT_VERIFIED case is added here for Google OAuth, but the actual email verification check is missing in the sign-in flow above. Google OAuth users typically have pre-verified emails, but there's no explicit check to ensure they don't need email verification. This creates an inconsistency with the regular sign-in flow where email verification is enforced.
| const onGoogleSignInSuccess = async () => { | ||
| if (store.getters.user) router.push('/admin') | ||
| store.commit('setLoading', false) | ||
| try { | ||
| if (store.getters.user) router.push('/admin') | ||
| } catch (error) { | ||
| if (error.message === 'EMAIL_NOT_VERIFIED') { | ||
| await router.push('/verify-email') | ||
| return | ||
| } | ||
| throw error | ||
| } finally { | ||
| store.commit('setLoading', false) | ||
| } | ||
| } |
There was a problem hiding this comment.
The error handling for EMAIL_NOT_VERIFIED is present but the actual signInWithGoogle action is never called in this function. The code just checks if a user exists and redirects, but doesn't trigger authentication. This function should dispatch the signInWithGoogle action and await its completion before checking the user state.
| logger.info('Email sent successfully to', { to: content.to }); | ||
| return 'Email sent successfully.'; | ||
| } catch (err) { | ||
| logger.error('Error sending email:', { error: err }); |
There was a problem hiding this comment.
The logger import is missing from this file. The code uses logger.info and logger.error but doesn't import the logger module. Add import logger from "../utils/logger.js"; at the top of the file to fix this issue.
Signed-off-by: Rakshit Yadav <yadavrakshit60@gmail.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 21 out of 21 changed files in this pull request and generated 12 comments.
Comments suppressed due to low confidence (1)
src/features/auth/store/Auth.js:180
- Consider adding explicit documentation or comment explaining that Google sign-in users bypass email verification. While Google accounts are typically verified, it's good practice to document this assumption. Also consider checking if the Google user's email is verified using the Firebase user.emailVerified property, as Firebase provides this information even for Google sign-ins.
async signInWithGoogle({ commit }, payload) {
try {
const { user } = await authController.signInWithGoogle(
payload.rememberMe,
)
// Check if user already exists in database
let dbUser = null
try {
dbUser = await userController.getById(user.uid)
} catch (error) {
// User doesn't exist in DB, will be created below
return error
}
// Create user if they don't exist yet
if (!dbUser || !dbUser.email) {
await userController.create({
id: user.uid,
email: user.email,
displayName: user.displayName || '',
profileImage: user.photoURL || '',
createdAt: new Date().toISOString(),
authProvider: 'google',
})
dbUser = await userController.getById(user.uid)
}
commit('SET_USER', dbUser)
commit('SET_TOAST', {
message: i18n.global.t('auth.loginSuccess'),
type: 'success',
})
} catch (err) {
if (err.message === 'EMAIL_NOT_VERIFIED') {
throw err
}
commit('SET_TOAST', {
message: i18n.global.t('errors.globalError'),
type: 'error',
})
throw err
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/app/plugins/locales/zh.json
Outdated
| "user": "用户", | ||
| "id": "ID", | ||
| "itemsPerPage": "每页项目数：", | ||
| "saving": "保存中..." |
There was a problem hiding this comment.
Missing comma after line 141. This will cause a JSON parsing error. Add a comma after "saving": "保存中..." on line 141.
| "saving": "保存中..." | |
| "saving": "保存中...", |
src/app/plugins/locales/ru.json
Outdated
| "user": "Пользователь", | ||
| "id": "ID", | ||
| "itemsPerPage": "Элементов на странице:", | ||
| "saving": "Сохранение..." |
There was a problem hiding this comment.
Missing comma after line 141. This will cause a JSON parsing error. Add a comma after "saving": "Сохранение..." on line 141.
| "saving": "Сохранение..." | |
| "saving": "Сохранение...", |
src/app/plugins/locales/pt_br.json
Outdated
| "user": "Usuário", | ||
| "id": "ID", | ||
| "itemsPerPage": "Itens por página:", | ||
| "saving": "Salvando..." |
There was a problem hiding this comment.
Missing comma after line 141. This will cause a JSON parsing error. Add a comma after "saving": "Salvando..." on line 141.
| "saving": "Salvando..." | |
| "saving": "Salvando...", |
src/app/plugins/locales/de.json
Outdated
| "user": "Benutzer", | ||
| "id": "ID", | ||
| "itemsPerPage": "Elemente pro Seite:", | ||
| "saving": "Wird gespeichert..." |
There was a problem hiding this comment.
Missing comma after line 141. This will cause a JSON parsing error. Add a comma after "saving": "Wird gespeichert..." on line 141.
| "saving": "Wird gespeichert..." | |
| "saving": "Wird gespeichert...", |
| if (!user) { | ||
| await store.dispatch('autoSignIn') | ||
| const authUser = await store.dispatch('autoSignIn') | ||
| user = store.state.Auth.user | ||
| // If user is logged in but email not verified, redirect to verify-email | ||
| if (authUser && authUser.emailVerified === false && !publicPages.includes(to.path)) { |
There was a problem hiding this comment.
Potential logic issue: When an unverified user is auto-signed in, the autoSignIn action returns the Firebase user object but doesn't commit it to the store. In the router guard (line 46), 'user' is then reassigned from 'store.state.Auth.user', which will be null. The check on line 48 for 'authUser.emailVerified === false' will work, but subsequent checks that rely on 'user' being set may fail. Consider if this is the intended behavior or if there should be a separate state variable to track unverified users.
| .link-text { | ||
| font-size: 13px; | ||
| color: #666; | ||
| margin-top: 20px; | ||
| word-break: break-all; | ||
| background-color: #f8f9fa; | ||
| padding: 12px; | ||
| border-radius: 4px; | ||
| border-left: 3px solid #1e3a8a; | ||
| } | ||
| .link-text strong { | ||
| display: block; | ||
| margin-bottom: 8px; | ||
| color: #1e3a8a; | ||
| font-weight: 600; | ||
| } | ||
| .expiry { | ||
| background-color: #eff6ff; | ||
| padding: 15px; | ||
| border-left: 4px solid #1e3a8a; | ||
| margin: 30px 0; | ||
| font-size: 14px; | ||
| color: #1e3a8a; | ||
| border-radius: 4px; | ||
| } | ||
| .expiry strong { | ||
| color: #1e3a8a; | ||
| font-weight: 600; | ||
| } | ||
| .expiry p { | ||
| margin-top: 8px; | ||
| color: #334155; | ||
| } | ||
| .footer { | ||
| background-color: #f8f9fa; | ||
| padding: 20px 30px; | ||
| text-align: center; | ||
| font-size: 12px; | ||
| color: #666; | ||
| border-top: 1px solid #e0e7ff; | ||
| } | ||
| .footer a { | ||
| color: #1e3a8a; | ||
| text-decoration: none; | ||
| font-weight: 500; | ||
| } | ||
| .footer a:hover { | ||
| text-decoration: underline; | ||
| } | ||
| .divider { | ||
| height: 1px; | ||
| background-color: #e0e7ff; | ||
| margin: 30px 0; | ||
| } |
There was a problem hiding this comment.
Unused CSS styles: The template defines styles for '.link-text', '.expiry', and '.divider' classes (lines 86-139) but these elements are not present in the HTML. Consider removing these unused styles to keep the code clean.
| if (!this.newEmail || !this.newEmail.includes('@')) { | ||
| this.modalError = this.$t('auth.invalidEmail') | ||
| return |
There was a problem hiding this comment.
Weak email validation: The email validation on line 213 only checks if the email contains an '@' symbol. This is insufficient and will accept invalid emails like '@example' or 'test@@example.com'. Consider using a proper email validation regex or a validation library to ensure the email format is valid.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 21 out of 21 changed files in this pull request and generated 8 comments.
Comments suppressed due to low confidence (1)
src/features/auth/store/Auth.js:180
- Google sign-in users are not checked for email verification, but they should be auto-verified since Google already verifies emails. However, the code in SignInView.vue tries to catch 'EMAIL_NOT_VERIFIED' error for Google sign-in (lines 183-186), but this error will never be thrown by the signInWithGoogle action, potentially causing confusion. Google-authenticated users typically have their emailVerified property set to true by Firebase, but there's no explicit check or handling for this in the auth flow.
async signInWithGoogle({ commit }, payload) {
try {
const { user } = await authController.signInWithGoogle(
payload.rememberMe,
)
// Check if user already exists in database
let dbUser = null
try {
dbUser = await userController.getById(user.uid)
} catch (error) {
// User doesn't exist in DB, will be created below
return error
}
// Create user if they don't exist yet
if (!dbUser || !dbUser.email) {
await userController.create({
id: user.uid,
email: user.email,
displayName: user.displayName || '',
profileImage: user.photoURL || '',
createdAt: new Date().toISOString(),
authProvider: 'google',
})
dbUser = await userController.getById(user.uid)
}
commit('SET_USER', dbUser)
commit('SET_TOAST', {
message: i18n.global.t('auth.loginSuccess'),
type: 'success',
})
} catch (err) {
if (err.message === 'EMAIL_NOT_VERIFIED') {
throw err
}
commit('SET_TOAST', {
message: i18n.global.t('errors.globalError'),
type: 'error',
})
throw err
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| <!-- Verify Button --> | ||
| <div class="button-container"> | ||
| <a href="{{verificationLink}}" class="verify-button" style="color: white !important;">Verify Email Address</a> |
There was a problem hiding this comment.
The email verification implementation is incomplete. When users click the verification link in the email, there's no code to handle the action code and actually verify the email. Firebase generates an email verification link with an action code, but the app needs to use Firebase's applyActionCode or handle the link properly to complete the verification. The current implementation only redirects to /verify-email, but doesn't process the verification action code from the link parameters.
src/app/plugins/locales/ru.json
Outdated
| "user": "Пользователь", | ||
| "id": "ID", | ||
| "itemsPerPage": "Элементов на странице:", | ||
| "saving": "Сохранение..." |
There was a problem hiding this comment.
Missing comma after the "saving" translation entry. This will cause a JSON parsing error and break the application's internationalization for this locale.
| "saving": "Сохранение..." | |
| "saving": "Сохранение...", |
| "user": "مستخدم", | ||
| "id": "ID", | ||
| "itemsPerPage": "عدد العناصر في الصفحة:", | ||
| "saving": "جاري الحفظ...", |
There was a problem hiding this comment.
Missing comma after the "saving" translation entry. This will cause a JSON parsing error and break the application's internationalization for this locale.
| if (!user) { | ||
| await store.dispatch('autoSignIn') | ||
| const authUser = await store.dispatch('autoSignIn') | ||
| user = store.state.Auth.user | ||
| // If user is logged in but email not verified, redirect to verify-email | ||
| if (authUser && authUser.emailVerified === false && !publicPages.includes(to.path)) { | ||
| return next('/verify-email') | ||
| } | ||
| } |
There was a problem hiding this comment.
Missing closing brace for the if statement. The if statement starting at line 44 is not properly closed, causing a syntax error.
| if (authUser && authUser.emailVerified === false && !publicPages.includes(to.path)) { | ||
| return next('/verify-email') | ||
| } | ||
| } |
There was a problem hiding this comment.
The code checks publicPages.includes(to.path) after already executing return next() on line 41 for public pages. This means the condition on line 48 will never be true for public pages, creating redundant and potentially confusing logic. The router guard should be restructured to avoid this redundancy.
| if (authUser && authUser.emailVerified === false && !publicPages.includes(to.path)) { | |
| return next('/verify-email') | |
| } | |
| } | |
| if (authUser && authUser.emailVerified === false) { | |
| return next('/verify-email') | |
| } | |
| } |
| async logout() { | ||
| try { | ||
| await this.logout() | ||
| this.$router.push('/signin') | ||
| } catch (err) { | ||
| console.error('Error signing out:', err) | ||
| } | ||
| }, |
There was a problem hiding this comment.
Recursive call to the same method. The method calls this.logout() which is mapped to the Vuex action logout, but then inside a try-catch it calls await this.logout() again. This will cause an infinite recursion. The method should simply call the Vuex action once: await this.logout().
src/app/plugins/locales/zh.json
Outdated
| "user": "用户", | ||
| "id": "ID", | ||
| "itemsPerPage": "每页项目数：", | ||
| "saving": "保存中..." |
There was a problem hiding this comment.
Missing comma after the "saving" translation entry. This will cause a JSON parsing error and break the application's internationalization for this locale.
| "saving": "保存中..." | |
| "saving": "保存中...", |
| "user": "ユーザー", | ||
| "id": "ID", | ||
| "itemsPerPage": "ページごとの項目数：", | ||
| "saving": "保存中...", |
There was a problem hiding this comment.
Missing comma after the "saving" translation entry. This will cause a JSON parsing error and break the application's internationalization for this locale.
Signed-off-by: Rakshit Yadav <yadavrakshit60@gmail.com>
Signed-off-by: Rakshit Yadav <yadavrakshit60@gmail.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 21 out of 21 changed files in this pull request and generated 19 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| import { admin, functions } from "../f.firebase.js"; | ||
| import nodemailer from "nodemailer"; | ||
| import * as fs from "fs"; | ||
| import * as path from "path"; | ||
| import logger from "../utils/logger.js"; | ||
|
|
There was a problem hiding this comment.
The logger import was removed but logger is still being used on lines 69 and 72. This will cause a ReferenceError at runtime. The logger import should be restored: import logger from "../utils/logger.js";
|
|
||
| async updateEmail() { | ||
| try { | ||
| if (!this.newEmail || !this.newEmail.includes('@')) { |
There was a problem hiding this comment.
The email validation is too weak. Simply checking if the email contains '@' using .includes('@') is insufficient. This allows invalid emails like 'test@' or '@test' to pass. Use a proper email validation regex or Firebase's email validation to ensure the email format is correct before attempting to update.
| if (!this.newEmail || !this.newEmail.includes('@')) { | |
| const emailPattern = /^[^\s@]+@[^\s@]+\.[^\s@]+$/ | |
| if (!this.newEmail || !emailPattern.test(this.newEmail)) { |
src/app/plugins/locales/pt_br.json
Outdated
| "emailNotVerified": "Email not verified", | ||
| "verificationEmailSent": "Verification email sent successfully!", | ||
| "errorSendingVerification": "Error sending verification email", | ||
| "verifyEmailTitle": "Verify Your Email", | ||
| "verifyEmailSubtitle": "We've sent a verification link to your email", | ||
| "emailLabel": "Email", | ||
| "verifyingEmail": "Verifying...", | ||
| "emailVerified": "Email verified successfully!", | ||
| "checkEmailTitle": "Check Your Email", | ||
| "checkEmailStep1": "Check your inbox for a verification email", | ||
| "checkEmailStep2": "Click the link in the email to verify your account", | ||
| "checkEmailStep3": "Return here and your account will be verified", | ||
| "checkSpamFolder": "Don't see it? Check your spam folder", | ||
| "resending": "Resending...", | ||
| "resendEmail": "Resend Email", | ||
| "changeEmail": "Change Email", | ||
| "continueToDashboard": "Continue to Dashboard", | ||
| "signOut": "Sign Out", | ||
| "newEmail": "New Email", | ||
| "enterNewEmail": "Enter your new email address", | ||
| "saving": "Saving...", | ||
| "errorCheckingVerification": "Error checking verification status", | ||
| "errorResendingEmail": "Error resending verification email", | ||
| "invalidEmail": "Invalid email address", | ||
| "emailUpdatedVerification": "Email updated! We've sent a verification link to your new email.", | ||
| "errorUpdatingEmail": "Error updating email" |
There was a problem hiding this comment.
All new email verification translations are in English instead of Portuguese (Brazilian). These should be translated to Portuguese for proper internationalization support.
| "emailNotVerified": "Email not verified", | |
| "verificationEmailSent": "Verification email sent successfully!", | |
| "errorSendingVerification": "Error sending verification email", | |
| "verifyEmailTitle": "Verify Your Email", | |
| "verifyEmailSubtitle": "We've sent a verification link to your email", | |
| "emailLabel": "Email", | |
| "verifyingEmail": "Verifying...", | |
| "emailVerified": "Email verified successfully!", | |
| "checkEmailTitle": "Check Your Email", | |
| "checkEmailStep1": "Check your inbox for a verification email", | |
| "checkEmailStep2": "Click the link in the email to verify your account", | |
| "checkEmailStep3": "Return here and your account will be verified", | |
| "checkSpamFolder": "Don't see it? Check your spam folder", | |
| "resending": "Resending...", | |
| "resendEmail": "Resend Email", | |
| "changeEmail": "Change Email", | |
| "continueToDashboard": "Continue to Dashboard", | |
| "signOut": "Sign Out", | |
| "newEmail": "New Email", | |
| "enterNewEmail": "Enter your new email address", | |
| "saving": "Saving...", | |
| "errorCheckingVerification": "Error checking verification status", | |
| "errorResendingEmail": "Error resending verification email", | |
| "invalidEmail": "Invalid email address", | |
| "emailUpdatedVerification": "Email updated! We've sent a verification link to your new email.", | |
| "errorUpdatingEmail": "Error updating email" | |
| "emailNotVerified": "E-mail não verificado", | |
| "verificationEmailSent": "E-mail de verificação enviado com sucesso!", | |
| "errorSendingVerification": "Erro ao enviar e-mail de verificação", | |
| "verifyEmailTitle": "Verifique seu e-mail", | |
| "verifyEmailSubtitle": "Enviamos um link de verificação para o seu e-mail", | |
| "emailLabel": "E-mail", | |
| "verifyingEmail": "Verificando...", | |
| "emailVerified": "E-mail verificado com sucesso!", | |
| "checkEmailTitle": "Verifique seu e-mail", | |
| "checkEmailStep1": "Verifique sua caixa de entrada para encontrar o e-mail de verificação", | |
| "checkEmailStep2": "Clique no link do e-mail para verificar sua conta", | |
| "checkEmailStep3": "Volte aqui e sua conta será verificada", | |
| "checkSpamFolder": "Não encontrou? Verifique a pasta de spam", | |
| "resending": "Reenviando...", | |
| "resendEmail": "Reenviar e-mail", | |
| "changeEmail": "Alterar e-mail", | |
| "continueToDashboard": "Continuar para o painel", | |
| "signOut": "Sair", | |
| "newEmail": "Novo e-mail", | |
| "enterNewEmail": "Digite seu novo endereço de e-mail", | |
| "saving": "Salvando...", | |
| "errorCheckingVerification": "Erro ao verificar o status da verificação", | |
| "errorResendingEmail": "Erro ao reenviar o e-mail de verificação", | |
| "invalidEmail": "Endereço de e-mail inválido", | |
| "emailUpdatedVerification": "E-mail atualizado! Enviamos um link de verificação para o seu novo e-mail.", | |
| "errorUpdatingEmail": "Erro ao atualizar o e-mail" |
src/app/plugins/locales/ja.json
Outdated
| "emailNotVerified": "Email not verified", | ||
| "verificationEmailSent": "Verification email sent successfully!", | ||
| "errorSendingVerification": "Error sending verification email", | ||
| "verifyEmailTitle": "Verify Your Email", | ||
| "verifyEmailSubtitle": "We've sent a verification link to your email", | ||
| "emailLabel": "Email", | ||
| "verifyingEmail": "Verifying...", | ||
| "emailVerified": "Email verified successfully!", | ||
| "checkEmailTitle": "Check Your Email", | ||
| "checkEmailStep1": "Check your inbox for a verification email", | ||
| "checkEmailStep2": "Click the link in the email to verify your account", | ||
| "checkEmailStep3": "Return here and your account will be verified", | ||
| "checkSpamFolder": "Don't see it? Check your spam folder", | ||
| "resending": "Resending...", | ||
| "resendEmail": "Resend Email", | ||
| "changeEmail": "Change Email", | ||
| "continueToDashboard": "Continue to Dashboard", | ||
| "signOut": "Sign Out", | ||
| "newEmail": "New Email", | ||
| "enterNewEmail": "Enter your new email address", | ||
| "saving": "Saving...", | ||
| "errorCheckingVerification": "Error checking verification status", | ||
| "errorResendingEmail": "Error resending verification email", | ||
| "invalidEmail": "Invalid email address", | ||
| "emailUpdatedVerification": "Email updated! We've sent a verification link to your new email.", | ||
| "errorUpdatingEmail": "Error updating email" |
There was a problem hiding this comment.
All new email verification translations are in English instead of Japanese. These should be translated to Japanese for proper internationalization support.
| "emailNotVerified": "Email not verified", | |
| "verificationEmailSent": "Verification email sent successfully!", | |
| "errorSendingVerification": "Error sending verification email", | |
| "verifyEmailTitle": "Verify Your Email", | |
| "verifyEmailSubtitle": "We've sent a verification link to your email", | |
| "emailLabel": "Email", | |
| "verifyingEmail": "Verifying...", | |
| "emailVerified": "Email verified successfully!", | |
| "checkEmailTitle": "Check Your Email", | |
| "checkEmailStep1": "Check your inbox for a verification email", | |
| "checkEmailStep2": "Click the link in the email to verify your account", | |
| "checkEmailStep3": "Return here and your account will be verified", | |
| "checkSpamFolder": "Don't see it? Check your spam folder", | |
| "resending": "Resending...", | |
| "resendEmail": "Resend Email", | |
| "changeEmail": "Change Email", | |
| "continueToDashboard": "Continue to Dashboard", | |
| "signOut": "Sign Out", | |
| "newEmail": "New Email", | |
| "enterNewEmail": "Enter your new email address", | |
| "saving": "Saving...", | |
| "errorCheckingVerification": "Error checking verification status", | |
| "errorResendingEmail": "Error resending verification email", | |
| "invalidEmail": "Invalid email address", | |
| "emailUpdatedVerification": "Email updated! We've sent a verification link to your new email.", | |
| "errorUpdatingEmail": "Error updating email" | |
| "emailNotVerified": "メールが確認されていません", | |
| "verificationEmailSent": "確認メールを送信しました。", | |
| "errorSendingVerification": "確認メールの送信中にエラーが発生しました", | |
| "verifyEmailTitle": "メールアドレスを確認してください", | |
| "verifyEmailSubtitle": "確認用リンクを記載したメールを送信しました。", | |
| "emailLabel": "メールアドレス", | |
| "verifyingEmail": "確認しています...", | |
| "emailVerified": "メールアドレスの確認が完了しました。", | |
| "checkEmailTitle": "メールを確認してください", | |
| "checkEmailStep1": "受信トレイに確認メールが届いているか確認してください", | |
| "checkEmailStep2": "メール内のリンクをクリックしてアカウントを認証してください", | |
| "checkEmailStep3": "この画面に戻ると、アカウントの確認が完了します", | |
| "checkSpamFolder": "届かない場合は、迷惑メールフォルダもご確認ください", | |
| "resending": "再送信しています...", | |
| "resendEmail": "メールを再送信", | |
| "changeEmail": "メールアドレスを変更", | |
| "continueToDashboard": "ダッシュボードへ進む", | |
| "signOut": "サインアウト", | |
| "newEmail": "新しいメールアドレス", | |
| "enterNewEmail": "新しいメールアドレスを入力してください", | |
| "saving": "保存しています...", | |
| "errorCheckingVerification": "確認ステータスの取得中にエラーが発生しました", | |
| "errorResendingEmail": "確認メールの再送信中にエラーが発生しました", | |
| "invalidEmail": "メールアドレスの形式が正しくありません", | |
| "emailUpdatedVerification": "メールアドレスを更新しました。新しいメールアドレス宛に確認リンクを送信しました。", | |
| "errorUpdatingEmail": "メールアドレスの更新中にエラーが発生しました" |
| this.modalError = null | ||
|
|
||
| // Update email in Firebase | ||
| await updateEmail(auth.currentUser, this.newEmail) |
There was a problem hiding this comment.
The updateEmail function in Firebase requires recent authentication. Calling updateEmail without recent re-authentication will throw an error (auth/requires-recent-login). Users should be prompted to re-authenticate (enter password or re-login with Google) before changing their email address. This is a Firebase security requirement to prevent unauthorized email changes.
| // Check verification status every 3 seconds | ||
| this.verificationCheckInterval = setInterval(() => { | ||
| this.checkEmailVerificationStatus() | ||
| }, 3000) |
There was a problem hiding this comment.
The verification polling interval checks email verification status every 3 seconds indefinitely. This could cause unnecessary Firebase API calls if the user leaves the page open for extended periods without verifying. Consider adding a maximum number of retries or increasing the interval progressively (e.g., 3s, 5s, 10s, 30s) to reduce API load. Also, consider stopping after a certain time period (e.g., 10-15 minutes) and prompting the user to manually refresh.
| // Check verification status every 3 seconds | |
| this.verificationCheckInterval = setInterval(() => { | |
| this.checkEmailVerificationStatus() | |
| }, 3000) | |
| // Check verification status with bounded, progressively increasing intervals | |
| const maxDurationMs = 15 * 60 * 1000 // 15 minutes | |
| const startTime = Date.now() | |
| const intervals = [3000, 5000, 10000, 30000] | |
| let attempt = 0 | |
| const pollVerificationStatus = () => { | |
| const elapsed = Date.now() - startTime | |
| if (elapsed >= maxDurationMs) { | |
| // Stop polling after max duration to avoid unnecessary API calls | |
| if (this.verificationCheckInterval) { | |
| clearInterval(this.verificationCheckInterval) | |
| } | |
| this.isVerifying = false | |
| return | |
| } | |
| this.checkEmailVerificationStatus() | |
| const delay = intervals[Math.min(attempt, intervals.length - 1)] | |
| attempt += 1 | |
| // Use setTimeout instead of setInterval for progressive backoff | |
| this.verificationCheckInterval = setTimeout(pollVerificationStatus, delay) | |
| } | |
| pollVerificationStatus() |
|
|
||
| async logout() { | ||
| try { | ||
| await this.logout() |
There was a problem hiding this comment.
The method is calling itself recursively. Line 265 calls await this.logout() within the logout method, which will result in infinite recursion. This should call the Vuex action instead: await this.$store.dispatch('logout') or map the action differently to avoid the name collision.
| await this.logout() | |
| await this.$store.dispatch('logout') |
| async checkEmailVerificationStatus() { | ||
| try { | ||
| this.isVerifying = true | ||
| // Reload user to get updated verification status | ||
| await auth.currentUser.reload() | ||
|
|
||
| if (auth.currentUser.emailVerified) { | ||
| this.isVerified = true | ||
| this.isVerifying = false | ||
| // Clear interval and redirect after 2 seconds | ||
| clearInterval(this.verificationCheckInterval) | ||
| setTimeout(() => { | ||
| this.$router.push('/admin') | ||
| }, 2000) | ||
| } |
There was a problem hiding this comment.
The checkEmailVerificationStatus method doesn't check if auth.currentUser exists before calling .reload(). If the user logs out or their session expires while this is running, it will throw an error trying to access .reload() on null. Add a null check: if (!auth.currentUser) return; at the start of the try block.
Signed-off-by: Rakshit Yadav <yadavrakshit60@gmail.com>
KarinePistili
left a comment
KarinePistili
left a comment
There was a problem hiding this comment.
Hello I've tested it and I have the following feedback:
- There is a not desired loop here which keeps making requests to identity toolikit and making the page to rerender. It would be better to just make a button like "Already verified" and check there to prevent this behaviour.
- For colors, make sure to use the same ones as ruxailab logo. For example, the blue is #00213F
- I received the email, take a look at the colors at the template as well.
- After verifying the email, the ideal flow would be to redirect the user to dashboard and not to redo the signin. Can you update to be like this?
Signed-off-by: Rakshit Yadav <yadavrakshit60@gmail.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 21 out of 21 changed files in this pull request and generated 18 comments.
Comments suppressed due to low confidence (2)
src/features/auth/store/Auth.js:181
- Google OAuth users are not subject to email verification checks. The signInWithGoogle action (lines 138-181) doesn't check user.emailVerified before allowing authentication, unlike the regular signin action which checks on line 106. According to the PR description, "Google OAuth users auto-verified and redirect to dashboard", but there's no code actually verifying their email status. This creates an inconsistency where Google users bypass the email verification requirement that regular signup users must complete, which may be a security concern depending on the intended behavior.
async signInWithGoogle({ commit }, payload) {
try {
const { user } = await authController.signInWithGoogle(
payload.rememberMe,
)
// Check if user already exists in database
let dbUser = null
try {
dbUser = await userController.getById(user.uid)
} catch (error) {
// User doesn't exist in DB, will be created below
return error
}
// Create user if they don't exist yet
if (!dbUser || !dbUser.email) {
await userController.create({
id: user.uid,
email: user.email,
displayName: user.displayName || '',
profileImage: user.photoURL || '',
createdAt: new Date().toISOString(),
authProvider: 'google',
})
dbUser = await userController.getById(user.uid)
}
commit('SET_USER', dbUser)
commit('SET_TOAST', {
message: i18n.global.t('auth.loginSuccess'),
type: 'success',
})
} catch (err) {
if (err.message === 'EMAIL_NOT_VERIFIED') {
throw err
}
commit('SET_TOAST', {
message: i18n.global.t('errors.globalError'),
type: 'error',
})
throw err
}
},
functions/src/https/email.js:74
- The error handling in the catch block returns the error object directly rather than throwing it or returning a consistent error response structure. This means the caller receives the raw error object instead of a standardized response format. Earlier in the function, success returns a string message 'Email sent successfully.', but on error it returns the err object itself. This inconsistency makes it difficult for callers to handle responses uniformly. Consider returning a consistent error structure or re-throwing the error.
} catch (err) {
logger.error('Error sending email:', { error: err });
return err;
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| async updateEmail() { | ||
| try { | ||
| if (!this.newEmail || !this.newEmail.includes('@')) { | ||
| this.modalError = this.$t('auth.invalidEmail') | ||
| return | ||
| } | ||
|
|
||
| this.isUpdatingEmail = true | ||
| this.modalError = null | ||
|
|
||
| // Update email in Firebase | ||
| await updateEmail(auth.currentUser, this.newEmail) | ||
|
|
||
| // Send verification email to new address | ||
| const displayName = auth.currentUser.displayName || 'User' | ||
| await this.sendVerificationEmail({ email: this.newEmail, userName: displayName }) | ||
|
|
||
| this.userEmail = this.newEmail | ||
| this.showChangeEmailModal = false | ||
|
|
||
| this.$store.commit('SET_TOAST', { | ||
| message: this.$t('auth.emailUpdatedVerification'), | ||
| type: 'success', | ||
| }) | ||
| } catch (err) { | ||
| console.error('Error updating email:', err) | ||
| this.modalError = err.message || this.$t('auth.errorUpdatingEmail') | ||
| } finally { | ||
| this.isUpdatingEmail = false | ||
| } | ||
| }, |
There was a problem hiding this comment.
Updating the email address with updateEmail from Firebase Auth requires recent authentication for security reasons. If the user's session is old, Firebase will throw an error requiring re-authentication (auth/requires-recent-login). This code doesn't handle this case - it should catch this specific error and prompt the user to re-authenticate before allowing email changes. Without this, users with older sessions will encounter cryptic errors when trying to change their email.
| logger.info('Email sent successfully to', { to: content.to }); | ||
| return 'Email sent successfully.'; | ||
| } catch (err) { | ||
| logger.error('Error sending email:', { error: err }); | ||
| return err; |
There was a problem hiding this comment.
The logger import was removed (previously on line 4) but logger is still being used on lines 69 and 72. This will cause a ReferenceError when the email function tries to log messages. The import statement should be restored: import logger from "../utils/logger.js";
| @@ -112,6 +138,7 @@ | |||
| "user": "उपयोगकर्ता", | |||
| "id": "आईडी", | |||
| "itemsPerPage": "प्रति पृष्ठ आइटम:", | |||
| "saving": "सहेजा जा रहा है...", | |||
There was a problem hiding this comment.
The translation key "saving" is duplicated - it appears both in the "auth" section (line 67) and in the "common" section (line 141). This creates ambiguity and the second definition will override the first. Since the VerifyEmailView.vue component uses $t('common.saving'), only the "common" section needs this key. The duplicate in the "auth" section should be removed.
| @@ -112,6 +138,7 @@ | |||
| "user": "Usuario", | |||
| "id": "ID", | |||
| "itemsPerPage": "Elementos por página:", | |||
| "saving": "Guardando...", | |||
There was a problem hiding this comment.
The translation key "saving" is duplicated - it appears both in the "auth" section (line 67) and in the "common" section (line 141). This creates ambiguity and the second definition will override the first. Since the VerifyEmailView.vue component uses $t('common.saving'), only the "common" section needs this key. The duplicate in the "auth" section should be removed.
| @@ -112,6 +138,7 @@ | |||
| "user": "مستخدم", | |||
| "id": "ID", | |||
| "itemsPerPage": "عدد العناصر في الصفحة:", | |||
| "saving": "جاري الحفظ...", | |||
There was a problem hiding this comment.
The translation key "saving" is duplicated - it appears both in the "auth" section (line 67) and in the "common" section (line 141). This creates ambiguity and the second definition will override the first. Since the VerifyEmailView.vue component uses $t('common.saving'), only the "common" section needs this key. The duplicate in the "auth" section should be removed.
| async sendVerificationEmail(email, userName) { | ||
| try { | ||
| const emailController = new EmailController() | ||
| await emailController.send({ | ||
| to: email, | ||
| subject: 'Verify Your Email Address', | ||
| template: 'emailVerification', | ||
| data: { | ||
| userName: userName || email, | ||
| }, | ||
| }) | ||
| } catch (err) { | ||
| console.error('Error sending verification email:', err) | ||
| throw err | ||
| } | ||
| } |
There was a problem hiding this comment.
The new sendVerificationEmail method added to AuthController lacks test coverage. The AuthController.spec.js file exists and contains tests for other AuthController methods, but doesn't include tests for this new method. Given that this repository has comprehensive test coverage for controllers (as evidenced by the existing AuthController.spec.js with 308 lines), this new method should also have corresponding unit tests to verify its behavior, error handling, and integration with EmailController.
| verificationCheckInterval: null, | ||
| } | ||
| }, | ||
| computed: { | ||
| ...mapState({ | ||
| currentUser: state => state.user, | ||
| }), | ||
| }, | ||
| mounted() { | ||
| // Ensure user is logged in | ||
| onAuthStateChanged(auth, (user) => { | ||
| this.initializeVerification() | ||
| }) | ||
| }, | ||
|
|
||
| beforeUnmount() { | ||
| if (this.verificationCheckInterval) { | ||
| clearInterval(this.verificationCheckInterval) | ||
| } | ||
| }, |
There was a problem hiding this comment.
The verificationCheckInterval variable is declared and cleared in beforeUnmount, but it's never actually set anywhere in the component. The variable is initialized to null on line 157 and cleared on line 174, but there's no code that creates an interval using setInterval. This appears to be dead code - either the auto-check interval functionality was planned but not implemented, or it was removed but the cleanup code was left behind. This dead code should be removed to avoid confusion.
| setTimeout(() => { | ||
| this.$router.push('/admin') | ||
| }, 1500) | ||
| } else { | ||
| this.error = this.$t('auth.emailNotVerified') | ||
| this.$store.commit('SET_TOAST', { | ||
| message: this.$t('auth.emailNotVerified'), | ||
| type: 'warning', | ||
| }) | ||
| } | ||
| } catch (err) { | ||
| console.error('Error checking email verification:', err) | ||
| this.error = this.$t('auth.errorCheckingVerification') | ||
| this.$store.commit('SET_TOAST', { | ||
| message: this.$t('auth.errorCheckingVerification'), | ||
| type: 'error', | ||
| }) | ||
| } finally { | ||
| this.isVerifying = false | ||
| } | ||
| }, | ||
|
|
||
| async resendVerificationEmail() { | ||
| try { | ||
| this.isResending = true | ||
| this.error = null | ||
|
|
||
| const email = auth.currentUser.email | ||
| const displayName = auth.currentUser.displayName || 'User' | ||
|
|
||
| await this.sendVerificationEmail({ email, userName: displayName }) | ||
|
|
||
| // Show success message using Vuex toast | ||
| this.$store.commit('SET_TOAST', { | ||
| message: this.$t('auth.verificationEmailSent'), | ||
| type: 'success', | ||
| }) | ||
| } catch (err) { | ||
| console.error('Error resending verification email:', err) | ||
| this.error = this.$t('auth.errorResendingEmail') | ||
| } finally { | ||
| this.isResending = false | ||
| } | ||
| }, | ||
|
|
||
| changeEmail() { | ||
| this.showChangeEmailModal = true | ||
| this.newEmail = '' | ||
| this.modalError = null | ||
| }, | ||
|
|
||
| async updateEmail() { | ||
| try { | ||
| if (!this.newEmail || !this.newEmail.includes('@')) { | ||
| this.modalError = this.$t('auth.invalidEmail') | ||
| return | ||
| } | ||
|
|
||
| this.isUpdatingEmail = true | ||
| this.modalError = null | ||
|
|
||
| // Update email in Firebase | ||
| await updateEmail(auth.currentUser, this.newEmail) | ||
|
|
||
| // Send verification email to new address | ||
| const displayName = auth.currentUser.displayName || 'User' | ||
| await this.sendVerificationEmail({ email: this.newEmail, userName: displayName }) | ||
|
|
||
| this.userEmail = this.newEmail | ||
| this.showChangeEmailModal = false | ||
|
|
||
| this.$store.commit('SET_TOAST', { | ||
| message: this.$t('auth.emailUpdatedVerification'), | ||
| type: 'success', | ||
| }) | ||
| } catch (err) { | ||
| console.error('Error updating email:', err) | ||
| this.modalError = err.message || this.$t('auth.errorUpdatingEmail') | ||
| } finally { | ||
| this.isUpdatingEmail = false | ||
| } | ||
| }, | ||
|
|
||
| goToDashboard() { | ||
| this.$router.push('/admin') | ||
| }, | ||
|
|
||
| async logout() { | ||
| try { | ||
| await this.logout() | ||
| this.$router.push('/signin') | ||
| } catch (err) { | ||
| console.error('Error signing out:', err) | ||
| } | ||
| }, | ||
|
|
||
| initializeVerification() { | ||
| const user = auth.currentUser | ||
| if (user) { | ||
| this.userEmail = user.email | ||
| if (user.emailVerified) { | ||
| this.isVerified = true | ||
| // Auto-redirect to dashboard after 1.5 seconds | ||
| setTimeout(() => { | ||
| this.$router.push('/admin') | ||
| }, 1500) |
There was a problem hiding this comment.
The setTimeout calls on lines 196-198 and 299-301 create timers for auto-redirecting to the dashboard, but these timers are not stored or cleaned up. If the user navigates away from the page or the component unmounts before the timeout completes, the redirect will still execute, potentially causing unexpected navigation. The timeout IDs should be stored (similar to verificationCheckInterval) and cleared in beforeUnmount using clearTimeout to prevent this issue.
| @@ -112,6 +138,7 @@ | |||
| "user": "Пользователь", | |||
| "id": "ID", | |||
| "itemsPerPage": "Элементов на странице:", | |||
| "saving": "Сохранение...", | |||
There was a problem hiding this comment.
The translation key "saving" is duplicated - it appears both in the "auth" section (line 67) and in the "common" section (line 141). This creates ambiguity and the second definition will override the first. Since the VerifyEmailView.vue component uses $t('common.saving'), only the "common" section needs this key. The duplicate in the "auth" section should be removed.
| "user": "ユーザー", | ||
| "id": "ID", | ||
| "itemsPerPage": "ページごとの項目数：", | ||
| "saving": "保存中...", |
There was a problem hiding this comment.
The translation key "saving" is duplicated - it appears both in the "auth" section (line 67) and in the "common" section (line 141). This creates ambiguity and the second definition will override the first. Since the VerifyEmailView.vue component uses $t('common.saving'), only the "common" section needs this key. The duplicate in the "auth" section should be removed.
| "saving": "保存中...", |
|
fixed @KarinePistili
Screen.Recording.2026-02-14.at.12.43.32.AM.mp4 |
|
2 participants
fixes #334
Reference #1419
Overview
Added a complete email verification feature to the signup process. Users are now required to verify their email address before completing registration.
Detailed Changes
1. Frontend - New Email Verification View
src/features/auth/views/VerifyEmailView.vue2. Routing - Add Verify Email Route
src/router/modules/public.js/verify-emailroute to public routesVerifyEmailViewcomponent3. Authentication Controller - Email Verification Method
src/features/auth/controllers/AuthController.jssendVerificationEmail(email, userName)4. Auth Store - Verification State Management
src/features/auth/store/Auth.js5. Email Templates - New Verification Email Template
functions/src/templates/mails/emailVerification.html6. Email Function - Updated Email Handler
functions/src/https/email.js7. Internationalization - New Translations
src/app/plugins/locales/en.json8. Sign-Up View Updates
src/features/auth/views/SignUpView.vue9. Sign-In View Updates
src/features/auth/views/SignInView.vue10. Router Configuration Updates
src/app/router/index.js11. Email Controller Updates
src/shared/controllers/EmailController.jsDIRECT SIGNUP WITH VERIFICATION:
Screen.Recording.2026-02-07.at.11.09.06.PM.1.mp4
TRYING TO SIGNIN WITHOUT VERIFICATION:
Screen.Recording.2026-02-07.at.11.13.49.PM.mp4