-
Notifications
You must be signed in to change notification settings - Fork 1
feat(Users): verify email links #222
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
base: main
Are you sure you want to change the base?
Conversation
| catchInternal((req, res) => controller.verifyRegistrationEmail(req, res)), | ||
| authRouter.route('/verify-email-code').post( | ||
| validate(AuthValidator.verifyEmailCodeSchema, 'body'), | ||
| catchInternal((req, res) => controller.verifyRegistrationEmailCode(req, res)), |
Check failure
Code scanning / CodeQL
Missing rate limiting High
authorization
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 20 hours ago
To fix this, we should add rate limiting middleware to the sensitive authorization routes in this router, particularly /verify-email-code (and likely other similar endpoints like /verify-email-token and /resend-verification-email). The standard way in an Express/TypeScript project is to use a well‑known library such as express-rate-limit, configure a limiter with sensible defaults (e.g., small number of attempts per time window for code verification), and plug it into the route’s middleware chain before the expensive controller logic, without changing any existing controller behavior.
Concretely, in server/routes/auth.ts:
- Add an import for
express-rate-limit. - Define one or more limiter instances, e.g.
emailCodeLimiter, configured for short windows and lowmaxto prevent brute‑force on email verification codes, and possibly a more generalauthLimiterfor other auth endpoints. - Apply
emailCodeLimiterto the/verify-email-coderoute by inserting it into the.post()middleware list betweenvalidate(...)andcatchInternal(...)(or immediately aftervalidate, order between those two doesn’t materially change behavior). For consistency and improved security, we can also apply an appropriate limiter to/verify-email-tokenand/resend-verification-email, but we will not alter the controllers themselves or their return values.
This requires only adding the express-rate-limit import and definitions at the top of this file, and updating the route definitions where they are already shown.
-
Copy modified line R14 -
Copy modified lines R25-R30 -
Copy modified lines R32-R45 -
Copy modified line R52 -
Copy modified line R58 -
Copy modified line R64
| @@ -11,6 +11,7 @@ | ||
| } from '../middleware'; | ||
| import { catchInternal } from '../helpers'; | ||
| import bodyParser from 'body-parser'; | ||
| import rateLimit from 'express-rate-limit'; | ||
|
|
||
| const authRouter = express.Router(); | ||
| const controller = new AuthController(); | ||
| @@ -21,7 +22,27 @@ | ||
| const CAS_BRIDGE_SERVER_URL = (_casPrefix.startsWith('https://') || _casPrefix.startsWith('http://')) ? _casPrefix : `https://${_casPrefix}`; | ||
| const SELF_URL = `${_selfDomainSafe}/api/v1/auth/cas-bridge`; | ||
|
|
||
| const emailCodeLimiter = rateLimit({ | ||
| windowMs: 15 * 60 * 1000, // 15 minutes | ||
| max: 10, // limit each IP to 10 verification attempts per window | ||
| standardHeaders: true, | ||
| legacyHeaders: false, | ||
| }); | ||
|
|
||
| const emailTokenLimiter = rateLimit({ | ||
| windowMs: 15 * 60 * 1000, | ||
| max: 30, | ||
| standardHeaders: true, | ||
| legacyHeaders: false, | ||
| }); | ||
|
|
||
| const resendVerificationEmailLimiter = rateLimit({ | ||
| windowMs: 60 * 60 * 1000, // 1 hour | ||
| max: 5, // limit resends to 5 per hour | ||
| standardHeaders: true, | ||
| legacyHeaders: false, | ||
| }); | ||
|
|
||
| authRouter.route('/register').post( | ||
| validate(AuthValidator.registerSchema, 'body'), | ||
| catchInternal((req, res) => controller.register(req, res)), | ||
| @@ -29,16 +49,19 @@ | ||
|
|
||
| authRouter.route('/verify-email-code').post( | ||
| validate(AuthValidator.verifyEmailCodeSchema, 'body'), | ||
| emailCodeLimiter, | ||
| catchInternal((req, res) => controller.verifyRegistrationEmailCode(req, res)), | ||
| ); | ||
|
|
||
| authRouter.route('/verify-email-token').post( | ||
| validate(AuthValidator.verifyEmailTokenSchema, 'body'), | ||
| emailTokenLimiter, | ||
| catchInternal((req, res) => controller.verifyRegistrationEmailToken(req, res)), | ||
| ); | ||
|
|
||
| authRouter.route('/resend-verification-email').post( | ||
| validate(AuthValidator.resendVerificationEmailSchema, 'body'), | ||
| resendVerificationEmailLimiter, | ||
| catchInternal((req, res) => controller.resendVerificationEmail(req, res)), | ||
| ); | ||
|
|
-
Copy modified lines R72-R73
| @@ -69,7 +69,8 @@ | ||
| "vite": "^6.4.1", | ||
| "vue": "^3.2.45", | ||
| "vue-i18n": "^11.1.10", | ||
| "yargs": "^17.7.2" | ||
| "yargs": "^17.7.2", | ||
| "express-rate-limit": "^8.2.1" | ||
| }, | ||
| "devDependencies": { | ||
| "@aws-sdk/types": "^3.329.0", |
| Package | Version | Security advisories |
| express-rate-limit (npm) | 8.2.1 | None |
|
|
||
| authRouter.route('/verify-email-token').post( | ||
| validate(AuthValidator.verifyEmailTokenSchema, 'body'), | ||
| catchInternal((req, res) => controller.verifyRegistrationEmailToken(req, res)), |
Check failure
Code scanning / CodeQL
Missing rate limiting High
authorization
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 20 hours ago
In general, the fix is to introduce an Express‑compatible rate‑limiting middleware (for example, via express-rate-limit) and apply it to sensitive routes that perform authentication/authorization or other expensive operations. This middleware intercepts requests before they reach the controller, tracks request counts per client (typically keyed by IP), and rejects or delays excessive requests, mitigating denial‑of‑service and brute‑force attacks.
For this file, the least intrusive fix without changing existing functionality is:
- Import a well‑known rate‑limiter (
express-rate-limit). - Define one or more limiter instances with sensible defaults (e.g., separate, stricter limits for auth/verification endpoints).
- Apply the limiter middleware to the affected routes; at minimum the route CodeQL flagged (
/verify-email-token), and, following the same rationale, to adjacent registration/verification endpoints (/register,/verify-email-code,/resend-verification-email,/complete-registration,/external-provision), since they are also auth‑related and may be expensive.
Concretely in server/routes/auth.ts:
-
Add an import for
express-rate-limitnear the top, without modifying existing imports. -
After router/controller initialization, create a limiter, e.g.:
const authRateLimiter = rateLimit({ windowMs: 15 * 60 * 1000, max: 100, standardHeaders: true, legacyHeaders: false });
These values are examples and can be tuned later; they preserve existing behavior for normal users while limiting abuse.
-
Insert
authRateLimiterin the middleware chains of the relevant routes, especially the flagged/verify-email-tokenhandler, ensuring it appears beforecatchInternalso that rate‑limit responses are generated by the limiter itself and existing controller logic is untouched.
No existing controller logic, validation schemas, or helper functions need changes; we’re only extending middleware chains.
-
Copy modified line R14 -
Copy modified lines R19-R25 -
Copy modified line R30 -
Copy modified line R36 -
Copy modified line R42 -
Copy modified line R48 -
Copy modified line R54 -
Copy modified line R61
| @@ -11,10 +11,18 @@ | ||
| } from '../middleware'; | ||
| import { catchInternal } from '../helpers'; | ||
| import bodyParser from 'body-parser'; | ||
| import rateLimit from 'express-rate-limit'; | ||
|
|
||
| const authRouter = express.Router(); | ||
| const controller = new AuthController(); | ||
|
|
||
| const authRateLimiter = rateLimit({ | ||
| windowMs: 15 * 60 * 1000, // 15 minutes | ||
| max: 100, // limit each IP to 100 requests per windowMs | ||
| standardHeaders: true, | ||
| legacyHeaders: false, | ||
| }); | ||
|
|
||
| const _selfDomain = process.env.DOMAIN || 'localhost:5001'; | ||
| const _selfDomainSafe = _selfDomain.startsWith('https://') ? _selfDomain : `https://${_selfDomain}`; | ||
| const _casPrefix = process.env.CAS_BRIDGE_SERVER_URL || 'http://localhost:8443/cas'; | ||
| @@ -23,32 +27,38 @@ | ||
|
|
||
|
|
||
| authRouter.route('/register').post( | ||
| authRateLimiter, | ||
| validate(AuthValidator.registerSchema, 'body'), | ||
| catchInternal((req, res) => controller.register(req, res)), | ||
| ); | ||
|
|
||
| authRouter.route('/verify-email-code').post( | ||
| authRateLimiter, | ||
| validate(AuthValidator.verifyEmailCodeSchema, 'body'), | ||
| catchInternal((req, res) => controller.verifyRegistrationEmailCode(req, res)), | ||
| ); | ||
|
|
||
| authRouter.route('/verify-email-token').post( | ||
| authRateLimiter, | ||
| validate(AuthValidator.verifyEmailTokenSchema, 'body'), | ||
| catchInternal((req, res) => controller.verifyRegistrationEmailToken(req, res)), | ||
| ); | ||
|
|
||
| authRouter.route('/resend-verification-email').post( | ||
| authRateLimiter, | ||
| validate(AuthValidator.resendVerificationEmailSchema, 'body'), | ||
| catchInternal((req, res) => controller.resendVerificationEmail(req, res)), | ||
| ); | ||
|
|
||
| authRouter.route('/complete-registration').post( | ||
| authRateLimiter, | ||
| verifyAPIAuthentication, | ||
| validate(AuthValidator.completeRegistrationSchema, 'body'), | ||
| catchInternal((req, res) => controller.completeRegistration(req, res)), | ||
| ); | ||
|
|
||
| authRouter.route('/external-provision').post( | ||
| authRateLimiter, | ||
| verifyAPIAuthentication, | ||
| ensureActorIsAPIUser, | ||
| ensureAPIUserHasPermission(['users:write']), |
-
Copy modified lines R72-R73
| @@ -69,7 +69,8 @@ | ||
| "vite": "^6.4.1", | ||
| "vue": "^3.2.45", | ||
| "vue-i18n": "^11.1.10", | ||
| "yargs": "^17.7.2" | ||
| "yargs": "^17.7.2", | ||
| "express-rate-limit": "^8.2.1" | ||
| }, | ||
| "devDependencies": { | ||
| "@aws-sdk/types": "^3.329.0", |
| Package | Version | Security advisories |
| express-rate-limit (npm) | 8.2.1 | None |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This pull request adds token-based email verification links alongside the existing code-based verification system for user registration. Users can now verify their email addresses either by entering a 6-digit code or by clicking a link in their verification email.
Changes:
- Added new API endpoints for token-based email verification and resending verification emails
- Updated the EmailVerification model to include token field and removed the email field
- Modified user registration flow to send both verification code and link in emails
- Added new UI page for handling email verification via token link
- Blocked login for users with unverified emails
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 19 comments.
Show a summary per file
| File | Description |
|---|---|
| server/validators/auth.ts | Added validation schemas for token-based verification and resend endpoints |
| server/types/auth.ts | Renamed and added type definitions for new verification methods |
| server/routes/auth.ts | Added routes for verify-email-token and resend-verification-email endpoints |
| server/models/User.ts | Added email_verified boolean field to track verification status |
| server/models/EmailVerification.ts | Removed email field, added token field for link-based verification |
| server/controllers/EmailVerificationController.ts | Added token generation and verification methods, updated email template |
| server/controllers/AuthController.ts | Added new verification handlers and login blocking for unverified users |
| server/tests/auth.spec.ts | Updated test endpoints to use new verify-email-code route |
| pages/verify-email/+onBeforeRender.ts | New server-side page handler to extract token from URL |
| pages/verify-email/+Page.vue | New UI page for token-based verification with resend functionality |
| pages/register/+Page.vue | Renamed VerifyEmail component to VerifyEmailForm for clarity |
| locales/en-us.json | Added localization strings for email verification page |
| components/registration/VerifyEmailForm.vue | Updated API endpoint to use verify-email-code |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| public async verifyRegistrationEmailCode(req: Request, res: Response): Promise<Response> { | ||
| const { email, code } = req.body as VerifyEmailCodeBody; | ||
|
|
||
| const foundVerification = await new EmailVerificationController().checkVerification(email, code); | ||
| if (!foundVerification || !foundVerification.uuid) { | ||
| const foundUser = await User.findOne({ where: { email } }); | ||
| if (!foundUser) { | ||
| return errors.badRequest(res); | ||
| } | ||
|
|
||
| const foundUser = await User.findOne({ where: { uuid: foundVerification.uuid } }); | ||
| if (!foundUser) { | ||
| const foundVerification = await new EmailVerificationController().checkVerificationCode(foundUser.uuid, code); | ||
| if (!foundVerification || !foundVerification.uuid) { | ||
| return errors.badRequest(res); | ||
| } | ||
|
|
||
| foundUser.disabled = false; | ||
|
|
||
| foundUser.email_verified = true; | ||
| await foundUser.save(); |
Copilot
AI
Jan 27, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing check for whether the email is already verified. If a user has already verified their email (either via code or token), they could potentially verify again and create additional sessions. Consider checking foundUser.email_verified before proceeding with the verification process.
| public async verifyRegistrationEmailToken(req: Request, res: Response): Promise<Response> { | ||
| const { token } = req.body as VerifyEmailTokenBody; | ||
|
|
||
| const foundVerification = await new EmailVerificationController().checkVerificationToken(token); | ||
| if (!foundVerification || !foundVerification.uuid) { | ||
| return errors.badRequest(res, "Invalid verification token!"); | ||
| } | ||
|
|
||
| // Verification token was found, but it's expired | ||
| if (foundVerification.expired) { | ||
| return res.send({ | ||
| success: false, | ||
| error: "Verification token has expired. Please request a new verification email.", | ||
| data: { | ||
| uuid: foundVerification.uuid, | ||
| }, | ||
| }) | ||
| } | ||
|
|
||
| const foundUser = await User.findOne({ where: { uuid: foundVerification.uuid } }); | ||
| if (!foundUser) { | ||
| return errors.badRequest(res); | ||
| } | ||
|
|
||
| foundUser.email_verified = true; | ||
| await foundUser.save(); |
Copilot
AI
Jan 27, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing check for whether the email is already verified. If a user has already verified their email, they could potentially verify again. Consider checking foundUser.email_verified and handling the case where it's already true (e.g., returning a success response indicating the email was already verified).
| data: { | ||
| uuid: foundUser.uuid, | ||
| }, | ||
| }) |
Copilot
AI
Jan 27, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing semicolon at the end of the return statement. This is inconsistent with the coding style used elsewhere in the file (e.g., line 575).
| data: { | ||
| uuid: foundVerification.uuid, | ||
| }, | ||
| }) |
Copilot
AI
Jan 27, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing semicolon at the end of the return statement. This is inconsistent with the coding style used elsewhere in the file (e.g., line 575).
| } | ||
|
|
||
| foundUser.disabled = false; | ||
|
|
Copilot
AI
Jan 27, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Extra blank line that should be removed for consistency with the coding style in the rest of the file.
| message: 'We need to verify your email address before you can continue. Please use the link below to begin the verification process.', | ||
| autoRedirect: true, | ||
| links: { | ||
| 'Go': `${SELF_BASE}/api/v1/auth/login?${redirectParams.toString()}`, |
Copilot
AI
Jan 27, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The redirect link for unverified users points back to the login endpoint, which will likely result in an infinite redirect loop. The link should probably direct users to a page where they can request a new verification email or enter their verification code, not back to the login page that will block them again.
| 'Go': `${SELF_BASE}/api/v1/auth/login?${redirectParams.toString()}`, | |
| 'Go': `${SELF_BASE}/verify-email?${redirectParams.toString()}`, |
| where: { | ||
| [Op.and]: [{ user_id }, { code }], | ||
| }, |
Copilot
AI
Jan 27, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The use of [Op.and] is unnecessary here. Sequelize's default behavior for multiple conditions in a where clause is to AND them together. You can simplify this to where: { user_id, code } for better readability.
| ); | ||
|
|
||
| mailSender.destroy(); | ||
|
|
Copilot
AI
Jan 27, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Trailing whitespace on this line should be removed.
| }, | ||
| }) | ||
| } | ||
|
|
Copilot
AI
Jan 27, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing documentation comment for this new public API endpoint. Consider adding a JSDoc comment similar to the other methods in this controller to describe the purpose, parameters, and return value of this endpoint.
| /** | |
| * Resends an email verification message to the user associated with the provided UUID. | |
| * Intended for users who have not yet verified their email and need a new verification email. | |
| * | |
| * @param req - Incoming API request containing the user's UUID in the request body. | |
| * @param res - Outgoing API response used to return success or error details. | |
| * @returns The fulfilled API response indicating whether the resend operation was successful. | |
| */ |
No description provided.