diff --git a/components/account_management/NewEmailForm.vue b/components/account_management/NewEmailForm.vue index 45972ed..d2efd06 100644 --- a/components/account_management/NewEmailForm.vue +++ b/components/account_management/NewEmailForm.vue @@ -88,6 +88,7 @@ import ThemedButton from '../ThemedButton.vue'; import { useI18n } from 'vue-i18n'; import joi from 'joi'; + import { normalizeEmail } from '../../email'; const props = defineProps<{ user?: Record; }>(); @@ -124,6 +125,9 @@ e.preventDefault(); newEmailError.value = ''; if (!props.user || !props.user.uuid) return; + // Canonicalize before validating so the verification step and the confirmation + // message use the address the change request was filed under. + newEmail.value = normalizeEmail(newEmail.value); if (!newEmail.value || !isValidEmail(newEmail.value)) { newEmailError.value = t('security.newemailinvalid'); return; diff --git a/components/registration/AuthForm.vue b/components/registration/AuthForm.vue index fddb0f4..dbc424a 100644 --- a/components/registration/AuthForm.vue +++ b/components/registration/AuthForm.vue @@ -181,6 +181,7 @@ import { Heading, Input, Button } from '@libretexts/davis-vue'; import PasswordStrengthMeter from '@components/PasswordStrengthMeter.vue'; import { getPasswordStrength } from '@renderer/utils/auth'; + import { normalizeEmail } from '../../email'; defineProps<{ loginURL: string; @@ -233,6 +234,9 @@ async function submitForm(e: Event) { e.preventDefault(); resetFormErrors(); + // Canonicalize before validating so the address shown on the verification step is + // the one the account was actually created with. + email.value = normalizeEmail(email.value); if (!validateForm()) return; loading.value = true; try { diff --git a/email.ts b/email.ts new file mode 100644 index 0000000..f060b03 --- /dev/null +++ b/email.ts @@ -0,0 +1,18 @@ +/** + * Canonical email address normalization, shared by the server and the browser bundle. + * + * The contract is: trim surrounding whitespace, apply Unicode NFC so visually identical + * addresses have identical bytes, then lowercase with the locale-invariant `toLowerCase` + * (never `toLocaleLowerCase`, which would fold `I` differently under a Turkish locale). + * + * Diacritics are preserved on purpose: `jose@example.com` and `josé@example.com` are + * distinct addresses per RFC 5321 and are treated as such here. Gmail-style dot and + * plus-tag stripping is also deliberately absent, since both are significant at most + * providers and folding them would merge unrelated accounts. + */ +export function normalizeEmail(value: unknown): string { + if (typeof value !== 'string') { + return ''; + } + return value.trim().normalize('NFC').toLowerCase(); +} diff --git a/migrations/0003_normalize_email_case.sql b/migrations/0003_normalize_email_case.sql new file mode 100644 index 0000000..39d79a5 --- /dev/null +++ b/migrations/0003_normalize_email_case.sql @@ -0,0 +1,86 @@ +-- 0003_normalize_email_case.sql +-- Canonicalizes stored email addresses and pins the collation of every email column. +-- +-- The application now writes addresses in one canonical form (trim, Unicode NFC, +-- lowercase) via normalizeEmail() in email.ts, enforced at the boundary by +-- emailValidator in server/validators/shared.ts and again by the @BeforeValidate hooks +-- on User, EmailVerification, and EmailEvent. This file brings existing rows in line +-- and makes the database enforce the same policy. +-- +-- Diacritics are preserved on purpose: jose@x.com and jose@x.com (with an accent) are +-- distinct addresses, so the target collation is case-insensitive but ACCENT-SENSITIVE. +-- +-- RUN THIS FILE IN THREE PASSES, WITH A HUMAN BETWEEN THEM. Do not pipe it in whole. + +-- --------------------------------------------------------------------------- +-- PASS 0 - survey. Record the answers before changing anything. +-- --------------------------------------------------------------------------- + +-- utf8mb4_0900_as_ci in PASS 3 requires MySQL 8.0 or newer. +SELECT VERSION(); + +-- What the columns are today. A *_ci collation here means no case-variant duplicates +-- can already exist, so PASS 1 returning zero rows is confirmation rather than luck. +SHOW FULL COLUMNS FROM `users` LIKE 'email'; +SHOW FULL COLUMNS FROM `email_verifications` LIKE 'email'; +SHOW FULL COLUMNS FROM `email_events` LIKE 'email'; + +-- Addresses holding characters outside ASCII. Expected to be zero or near it. MySQL +-- cannot apply Unicode NFC, so any rows listed here are normalized by the model hook +-- the next time the record is written; note the count and move on. +SELECT `uuid`, `email` FROM `users` WHERE `email` <> CONVERT(`email` USING ASCII); + +-- --------------------------------------------------------------------------- +-- PASS 1 - collision detection. If this returns ANY rows, STOP. +-- --------------------------------------------------------------------------- +-- Two accounts differing only by case cannot both survive the lowercase backfill, and +-- the unique index on users.email will reject the UPDATE. Resolve each pair by hand: +-- decide which account is real, then migrate or disable the other. Do not automate it, +-- because the wrong choice locks a real person out of their account. + +SELECT LOWER(`email`) AS normalized, COUNT(*) AS n, GROUP_CONCAT(`uuid`) AS uuids +FROM `users` +GROUP BY normalized +HAVING n > 1; + +-- Useful when triaging a collision: which of the pair has been used most recently. +-- SELECT uuid, email, last_access, created_at, disabled +-- FROM users WHERE LOWER(email) = ''; + +-- --------------------------------------------------------------------------- +-- PASS 2 - backfill. Only once PASS 1 returns zero rows. +-- --------------------------------------------------------------------------- +-- email_verifications rows expire after 24 hours and email_events has no unique +-- constraint on email alone, so neither needs a collision pre-check. + +UPDATE `users` SET `email` = LOWER(`email`) WHERE `email` <> BINARY LOWER(`email`); +UPDATE `email_verifications` SET `email` = LOWER(`email`) WHERE `email` <> BINARY LOWER(`email`); +UPDATE `email_events` SET `email` = LOWER(`email`) WHERE `email` <> BINARY LOWER(`email`); + +-- Confirm. All three must return 0. +SELECT COUNT(*) FROM `users` WHERE `email` <> BINARY LOWER(`email`); +SELECT COUNT(*) FROM `email_verifications` WHERE `email` <> BINARY LOWER(`email`); +SELECT COUNT(*) FROM `email_events` WHERE `email` <> BINARY LOWER(`email`); + +-- --------------------------------------------------------------------------- +-- PASS 3 - collation. Only once PASS 2 reports zero on all three counts. +-- --------------------------------------------------------------------------- +-- utf8mb4_0900_as_ci is accent-sensitive and case-insensitive, which is exactly the +-- application policy. On MySQL 5.7, use utf8mb4_bin instead (exact match, correct +-- because the application always writes lowercase). Do NOT substitute +-- utf8mb4_unicode_ci or utf8mb4_general_ci: both are accent-INSENSITIVE and would +-- collapse jose@x.com and jose@x.com-with-an-accent into one address. + +ALTER TABLE `users` + MODIFY `email` VARCHAR(255) CHARACTER SET utf8mb4 COLLATE utf8mb4_0900_as_ci NOT NULL; + +ALTER TABLE `email_verifications` + MODIFY `email` VARCHAR(255) CHARACTER SET utf8mb4 COLLATE utf8mb4_0900_as_ci NOT NULL; + +ALTER TABLE `email_events` + MODIFY `email` VARCHAR(255) CHARACTER SET utf8mb4 COLLATE utf8mb4_0900_as_ci NOT NULL; + +-- Note on the test database: connectDatabase() builds it with sync({ alter: true }) +-- from the model decorators, which does not reproduce these collations. Tests must +-- therefore assert application-level normalization rather than lean on the database +-- folding case for them. diff --git a/migrations/README.md b/migrations/README.md index 2c65134..20cbc80 100644 --- a/migrations/README.md +++ b/migrations/README.md @@ -21,3 +21,4 @@ and production databases. | --- | --- | --- | --- | | `0001_users_search_indexes.sql` | | | | | `0002_api_user_password_write_permission.sql` | | | | +| `0003_normalize_email_case.sql` | | | | diff --git a/pages/passwordrecovery/+Page.vue b/pages/passwordrecovery/+Page.vue index 5bd8ef5..6115dd5 100644 --- a/pages/passwordrecovery/+Page.vue +++ b/pages/passwordrecovery/+Page.vue @@ -79,6 +79,7 @@ import { IconCircleArrowLeft } from '@tabler/icons-vue'; import { Input, Button } from '@libretexts/davis-vue'; import { usePageProps } from '@renderer/usePageProps'; + import { normalizeEmail } from '../../email'; const props = usePageProps<{ source: string; @@ -117,6 +118,9 @@ async function submitForm(e: Event) { e.preventDefault(); resetFormErrors(); + // Canonicalize before validating so the confirmation screen echoes the address the + // reset link was actually sent to. + email.value = normalizeEmail(email.value); if (!validateForm()) return; loading.value = true; try { diff --git a/server/controllers/AuthController.ts b/server/controllers/AuthController.ts index d3dd300..ebec889 100644 --- a/server/controllers/AuthController.ts +++ b/server/controllers/AuthController.ts @@ -24,7 +24,7 @@ import { EmailVerificationController } from './EmailVerificationController'; import { MailController } from './MailController'; import { emailTemplates } from '../emails/templates'; import { DEFAULT_AVATAR, UUID_V4_REGEX } from './UserController'; -import { DEFAULT_FIRST_NAME, DEFAULT_LAST_NAME, getPrettyDefaultAccessString, getProductionURL } from '../helpers'; +import { DEFAULT_FIRST_NAME, DEFAULT_LAST_NAME, getPrettyDefaultAccessString, getProductionURL, normalizeEmail } from '../helpers'; import errors from '../errors'; import type { CookieOptions, Request, Response } from 'express'; import type { @@ -415,12 +415,14 @@ export class AuthController { .map((l) => l.get('main_url')); const foundLib = userLibs?.find((u) => u === `https://${source}`); + const principalEmail = normalizeEmail(principal.attributes?.email); + const payload = { first_name: principal.attributes?.first_name ?? principal.attributes?.given_name, last_name: principal.attributes?.last_name ?? principal.attributes?.family_name, - email: principal.attributes?.email, + email: principalEmail || undefined, picture: principal.attributes?.picture, - educational: /(?<=.*?)@.*?\.edu/.test(principal.attributes?.email), + educational: /(?<=.*?)@.*?\.edu/.test(principalEmail), }; console.log(`CAS Bridge authentication successful for user ${resolvedUUID} (${foundUser.get('email')})`); @@ -772,7 +774,7 @@ export class AuthController { } }; - return email; + return email ? normalizeEmail(email) : null; }; const email = getEmailFromPayload(body.clientName, payload); @@ -892,7 +894,10 @@ export class AuthController { } return 'external_subject_id'; }; - const attrMatch = { [getAttrMatchKey(username)]: username }; + const attrMatchKey = getAttrMatchKey(username); + const attrMatch = { + [attrMatchKey]: attrMatchKey === 'email' ? normalizeEmail(username) : username, + }; const foundUser = await User.findOne({ where: attrMatch, @@ -1264,7 +1269,7 @@ export class AuthController { where: { [Op.or]: [ { uuid: userID }, - { email: userID } + { email: normalizeEmail(userID) } ] }, attributes: ['uuid'] diff --git a/server/controllers/EmailVerificationController.ts b/server/controllers/EmailVerificationController.ts index 74f69f6..e30a1d4 100644 --- a/server/controllers/EmailVerificationController.ts +++ b/server/controllers/EmailVerificationController.ts @@ -2,6 +2,7 @@ import { Op } from 'sequelize'; import { EmailVerification } from '../models'; import { MailController } from './MailController'; import { emailTemplates } from '../emails/templates'; +import { normalizeEmail } from '../helpers'; export class EmailVerificationController { static generateCode() { @@ -14,7 +15,7 @@ export class EmailVerificationController { const verification = await EmailVerification.create({ user_id: uuid, - email, + email: normalizeEmail(email), code: EmailVerificationController.generateCode(), expires_at: verifyExpiry, }); @@ -26,7 +27,7 @@ export class EmailVerificationController { const foundVerification = await EmailVerification.findOne({ where: { [Op.and]: [ - { email }, + { email: normalizeEmail(email) }, { code }, ], }, diff --git a/server/controllers/SnsEventController.ts b/server/controllers/SnsEventController.ts index 0a83082..6a88eb1 100644 --- a/server/controllers/SnsEventController.ts +++ b/server/controllers/SnsEventController.ts @@ -1,6 +1,7 @@ import type { Request, Response } from 'express'; import MessageValidator from 'sns-validator'; import errors from '@server/errors'; +import { normalizeEmail } from '@server/helpers'; import { EmailEvent, type EmailEventType } from '@server/models/EmailEvent'; import { User } from '@server/models/User'; import { recordConsentChange } from '@server/services/marketingConsent'; @@ -180,7 +181,7 @@ export class SnsEventController { for (const rcpt of recipients) { try { - const email = rcpt.email.toLowerCase(); + const email = normalizeEmail(rcpt.email); const user = await User.findOne({ where: { email } }); /** diff --git a/server/controllers/UserController.ts b/server/controllers/UserController.ts index 6b9b755..2547d1a 100644 --- a/server/controllers/UserController.ts +++ b/server/controllers/UserController.ts @@ -52,7 +52,7 @@ import { AuthController } from './AuthController'; import { DeleteAccountRequest } from '@server/models/DeleteAccountRequest'; import { EventSubscriberEmitter } from '@server/events/EventSubscriberEmitter'; import { UserNote } from '../models/UserNote'; -import { generateSecureRandomString } from '@server/helpers'; +import { generateSecureRandomString, normalizeEmail } from '@server/helpers'; import type { ApplicationLaunchpadVisibility } from '@server/types/applications'; export const DEFAULT_AVATAR = 'https://cdn.libretexts.net/DefaultImages/avatar.png'; @@ -107,12 +107,17 @@ function buildUserSearchCriteria(query: string): { match: string; relevance: str const exact = sequelize.escape(query); const prefixOf = (value: string) => sequelize.escape(`${escapeLikePattern(value)}%`); const containsOf = (value: string) => sequelize.escape(`%${escapeLikePattern(value)}%`); + // Addresses are stored in canonical form, so anything compared against `email` is + // normalized first. Name and UUID matching keeps the raw input. + const emailExact = sequelize.escape(normalizeEmail(query)); + const emailPrefixOf = (value: string) => prefixOf(normalizeEmail(value)); + const emailContainsOf = (value: string) => containsOf(normalizeEmail(value)); const fullName = 'CONCAT(`User`.`first_name`, \' \', `User`.`last_name`)'; - const exactClause = `(\`User\`.\`email\` = ${exact} OR \`User\`.\`uuid\` = ${exact})`; + const exactClause = `(\`User\`.\`email\` = ${emailExact} OR \`User\`.\`uuid\` = ${exact})`; const tokenClause = `(${tokens.map((token) => ( - `(\`User\`.\`email\` LIKE ${containsOf(token)}` + `(\`User\`.\`email\` LIKE ${emailContainsOf(token)}` + ` OR \`User\`.\`first_name\` LIKE ${containsOf(token)}` + ` OR \`User\`.\`last_name\` LIKE ${containsOf(token)}` + ` OR \`User\`.\`uuid\` LIKE ${containsOf(token)})` @@ -131,7 +136,7 @@ function buildUserSearchCriteria(query: string): { match: string; relevance: str const relevance = 'CASE' + ` WHEN ${exactClause} THEN 100` - + ` WHEN \`User\`.\`email\` LIKE ${prefixOf(query)} THEN 80` + + ` WHEN \`User\`.\`email\` LIKE ${emailPrefixOf(query)} THEN 80` + ` WHEN ${fullName} LIKE ${prefixOf(query)}${bothNamesClause} THEN 70` + ` WHEN ${anyNamePrefixClause} THEN 50` // Everything left matched somewhere inside a field rather than at its start. @@ -837,7 +842,10 @@ export class UserController { } return 'external_subject_id'; }; - const attrMatch = { [getAttrMatchKey(username)]: username }; + const attrMatchKey = getAttrMatchKey(username); + const attrMatch = { + [attrMatchKey]: attrMatchKey === 'email' ? normalizeEmail(username) : username, + }; const foundUser = await User.findOne({ where: { diff --git a/server/helpers.ts b/server/helpers.ts index e77663e..713cc0a 100644 --- a/server/helpers.ts +++ b/server/helpers.ts @@ -1,6 +1,9 @@ import type { NextFunction, Request, Response } from 'express'; import errors from './errors'; import type { Application } from './types/applications'; +import { normalizeEmail } from '../email'; + +export { normalizeEmail }; type APIFunction = (request: Request, response: Response, next: NextFunction) => Response | void; type AsyncAPIFunction = (request: Request, response: Response, next: NextFunction) => Promise; diff --git a/server/models/EmailEvent.ts b/server/models/EmailEvent.ts index cd87f8e..fd8093c 100644 --- a/server/models/EmailEvent.ts +++ b/server/models/EmailEvent.ts @@ -1,5 +1,6 @@ import { AllowNull, + BeforeValidate, BelongsTo, Column, CreatedAt, @@ -12,6 +13,7 @@ import { UpdatedAt, } from 'sequelize-typescript'; import { User } from './User'; +import { normalizeEmail } from '../../email'; export type EmailEventType = 'HARD_BOUNCE' | 'SOFT_BOUNCE' | 'COMPLAINT'; @@ -29,6 +31,17 @@ export type EmailEventType = 'HARD_BOUNCE' | 'SOFT_BOUNCE' | 'COMPLAINT'; tableName: 'email_events', }) export class EmailEvent extends Model { + /** + * Guarantees the canonical form reaches storage no matter which code path wrote the + * record. Query predicates are normalized separately at their call sites. + */ + @BeforeValidate + static normalizeEmailAddress(instance: EmailEvent) { + if (typeof instance.email === 'string') { + instance.email = normalizeEmail(instance.email); + } + } + @PrimaryKey @AutoIncrement @Column(DataType.BIGINT) diff --git a/server/models/EmailVerification.ts b/server/models/EmailVerification.ts index 645678e..77205e2 100644 --- a/server/models/EmailVerification.ts +++ b/server/models/EmailVerification.ts @@ -1,5 +1,6 @@ import { AllowNull, + BeforeValidate, Column, CreatedAt, DataType, @@ -9,12 +10,24 @@ import { UpdatedAt, } from 'sequelize-typescript'; import { User } from './User'; +import { normalizeEmail } from '../../email'; @Table({ timestamps: true, tableName: 'email_verifications', }) export class EmailVerification extends Model { + /** + * Guarantees the canonical form reaches storage no matter which code path wrote the + * record. Query predicates are normalized separately at their call sites. + */ + @BeforeValidate + static normalizeEmailAddress(instance: EmailVerification) { + if (typeof instance.email === 'string') { + instance.email = normalizeEmail(instance.email); + } + } + @ForeignKey(() => User) @AllowNull(false) @Column(DataType.STRING) diff --git a/server/models/User.ts b/server/models/User.ts index b6fb92f..2f6a161 100644 --- a/server/models/User.ts +++ b/server/models/User.ts @@ -1,5 +1,6 @@ import { AllowNull, + BeforeValidate, BelongsTo, BelongsToMany, Column, @@ -28,6 +29,7 @@ import { VerificationRequest } from './VerificationRequest'; import { Session } from './Session'; import { UserNote } from './UserNote'; import { UserLicenseEntitlement } from './UserLicenseEntitlement'; +import { normalizeEmail } from '../../email'; @DefaultScope(() => ({ attributes: { @@ -45,6 +47,17 @@ import { UserLicenseEntitlement } from './UserLicenseEntitlement'; ], }) export class User extends Model { + /** + * Guarantees the canonical form reaches storage no matter which code path wrote the + * record. Query predicates are normalized separately at their call sites. + */ + @BeforeValidate + static normalizeEmailAddress(instance: User) { + if (typeof instance.email === 'string') { + instance.email = normalizeEmail(instance.email); + } + } + @PrimaryKey @AllowNull(false) @Column(DataType.STRING) diff --git a/server/routes/auth.ts b/server/routes/auth.ts index 15dad4c..34cc99b 100644 --- a/server/routes/auth.ts +++ b/server/routes/auth.ts @@ -58,6 +58,7 @@ authRouter.route('/cas-interrupt-check').get( verifyAPIAuthentication, ensureActorIsAPIUser, ensureAPIUserHasPermission(['users:read']), + validate(AuthValidator.checkCASInterruptQuerySchema, 'query'), catchInternal((req, res) => controller.checkCASInterrupt(req, res)), ); diff --git a/server/routes/users.ts b/server/routes/users.ts index 06d2e2f..b6c8ce6 100644 --- a/server/routes/users.ts +++ b/server/routes/users.ts @@ -27,6 +27,7 @@ usersRouter.route('/principal-attributes').get( verifyAPIAuthentication, ensureActorIsAPIUser, ensureAPIUserHasPermission(['users:read']), + validate(UserValidator.resolvePrincipalAttributesSchema, 'query'), catchInternal((req, res) => controller.resolvePrincipalAttributes(req, res)), ); diff --git a/server/tests/auth.spec.ts b/server/tests/auth.spec.ts index 4650c24..83249d4 100644 --- a/server/tests/auth.spec.ts +++ b/server/tests/auth.spec.ts @@ -69,6 +69,43 @@ describe('Authentication and Authorization', async () => { await emailVerify1?.destroy(); await user1?.destroy(); }); + it('should register and verify a mixed-case address in canonical form', async () => { + const response = await request(server) + .post('/api/v1/auth/register') + .send({ email: ' Info.User@LibreTexts.ORG ', password: 'ThisIsASuperStrongPassword!' }); + + expect(response.status).to.equal(201); + const uuid = response.body?.data?.uuid; + expect(uuid).to.exist; + + const user1 = await User.unscoped().findOne({ where: { uuid } }); + expect(user1?.get('email')).to.equal('info.user@libretexts.org'); + + const emailVerify1 = await EmailVerification.findOne({ where: { user_id: uuid } }); + expect(emailVerify1?.get('email')).to.equal('info.user@libretexts.org'); + + // The address the user types on the verification screen need not match the casing + // they typed on the registration screen. + const verifyResponse = await request(server) + .post('/api/v1/auth/verify-email') + .send({ email: 'INFO.USER@libretexts.org', code: emailVerify1?.get('code') }); + + expect(verifyResponse.status).to.equal(200); + expect(verifyResponse.body?.data).to.deep.equal({ uuid }); + + await user1?.destroy(); + }); + it('should error on existing user differing only by case', async () => { + await User.create({ + uuid: uuidv4(), + email: 'info@libretexts.org', + }); + const response = await request(server) + .post('/api/v1/auth/register') + .send({ email: 'Info@LibreTexts.ORG', password: 'ThisIsASuperStrongPassword!' }); + + expect(response.status).to.equal(409); + }); it('should error on existing user', async () => { const user1 = await User.create({ uuid: uuidv4(), @@ -219,6 +256,22 @@ describe('Authentication and Authorization', async () => { await foundToken?.destroy(); }); + it('should send reset link when the address differs only by case', async () => { + const user1 = await User.create({ + uuid: uuidv4(), + email: 'info@libretexts.org', + }); + + const response = await request(server) + .post('/api/v1/auth/passwordrecovery') + .send({ email: 'Info@LibreTexts.ORG' }); + + expect(response.status).to.equal(200); + const foundToken = await ResetPasswordToken.findOne({ where: { uuid: user1.uuid } }); + expect(foundToken).to.exist; + + await foundToken?.destroy(); + }); it('should complete password reset', async () => { const user1 = await User.create({ uuid: uuidv4(), diff --git a/server/tests/email.unit.ts b/server/tests/email.unit.ts new file mode 100644 index 0000000..a079f9f --- /dev/null +++ b/server/tests/email.unit.ts @@ -0,0 +1,41 @@ +import { describe, it } from 'mocha'; +import { expect } from 'chai'; +import { normalizeEmail } from '../../email'; + +describe('normalizeEmail', () => { + it('should strip surrounding whitespace', () => { + expect(normalizeEmail(' info@libretexts.org \t')).to.equal('info@libretexts.org'); + }); + + it('should lowercase both the local part and the domain', () => { + expect(normalizeEmail('Info.User+Tag@LibreTexts.ORG')).to.equal('info.user+tag@libretexts.org'); + }); + + it('should leave an already canonical address untouched', () => { + expect(normalizeEmail('info@libretexts.org')).to.equal('info@libretexts.org'); + }); + + it('should compose decomposed characters to NFC', () => { + // 'e' followed by U+0301 COMBINING ACUTE ACCENT vs. the single codepoint U+00E9. + const decomposed = 'josé@libretexts.org'; + const composed = 'josé@libretexts.org'; + expect(decomposed).to.not.equal(composed); + expect(normalizeEmail(decomposed)).to.equal(composed); + expect(normalizeEmail(decomposed)).to.equal(normalizeEmail(composed)); + }); + + it('should preserve diacritics rather than folding them away', () => { + expect(normalizeEmail('JOSÉ@libretexts.org')).to.equal('josé@libretexts.org'); + expect(normalizeEmail('José@libretexts.org')).to.not.equal(normalizeEmail('Jose@libretexts.org')); + }); + + it('should not strip dots or plus tags', () => { + expect(normalizeEmail('first.last+libreone@gmail.com')).to.equal('first.last+libreone@gmail.com'); + }); + + it('should return an empty string for non-string input', () => { + expect(normalizeEmail(undefined)).to.equal(''); + expect(normalizeEmail(null)).to.equal(''); + expect(normalizeEmail(42)).to.equal(''); + }); +}); diff --git a/server/tests/snsEvents.spec.ts b/server/tests/snsEvents.spec.ts index f20eeee..2b4f9d7 100644 --- a/server/tests/snsEvents.spec.ts +++ b/server/tests/snsEvents.spec.ts @@ -155,6 +155,28 @@ describe('SNS Event Webhook', () => { expect(ledger[0].email_event_id).to.equal(events[0].id); }); + it('attaches a bounce to the user when SES reports a different casing', async () => { + const user = await User.create({ + uuid: uuidv4(), + email: 'bouncer@example.com', + mktg_email_opt_in: true, + }); + const res = await request(server) + .post(WEBHOOK_PATH) + .set('Content-Type', 'application/json') + .send(envelope({ message: bouncePayload({ email: 'Bouncer@Example.COM' }) })); + expect(res.status).to.equal(200); + expect(res.body.processed).to.equal(1); + + const events = await EmailEvent.findAll(); + expect(events).to.have.length(1); + expect(events[0].email).to.equal('bouncer@example.com'); + expect(events[0].user_uuid).to.equal(user.uuid); + + const reloaded = await User.findOne({ where: { uuid: user.uuid } }); + expect(reloaded?.email_deliverability_status).to.equal('SUPPRESSED'); + }); + it('records a complaint and suppresses the user', async () => { const user = await User.create({ uuid: uuidv4(), diff --git a/server/tests/users.spec.ts b/server/tests/users.spec.ts index d773d72..1c9e57f 100644 --- a/server/tests/users.spec.ts +++ b/server/tests/users.spec.ts @@ -889,6 +889,27 @@ describe('Users', async () => { await org1.destroy(); await orgSystem1.destroy(); }); + it('should resolve CAS principal attributes when the username differs only by case', async () => { + const user1 = await User.create({ + uuid: uuidv4(), + email: 'info@libretexts.org', + first_name: 'Info', + last_name: 'LibreTexts', + disabled: false, + expired: false, + user_type: 'instructor', + verify_status: 'not_attempted', + }); + + const response = await request(server) + .get('/api/v1/users/principal-attributes') + .query({ username: ' Info@LibreTexts.ORG ' }) + .auth(mainAPIUserUsername, mainAPIUserPassword); + + expect(response.status).to.equal(200); + expect(response.body.uuid).to.equal(user1.uuid); + expect(response.body.email).to.equal('info@libretexts.org'); + }); }); describe('UPDATE', () => { @@ -1144,6 +1165,68 @@ describe('Users', async () => { code: 'bad_request', }); }); + it('should store a verified email change in canonical form', async () => { + const user1 = await User.create({ + uuid: uuidv4(), + email: 'info@libretexts.org', + disabled: false, + expired: false, + }); + const emailVerify1 = await new EmailVerificationController().createVerification( + user1.uuid, + 'Info+New@LibreTexts.ORG', + ); + + const response = await request(server) + .post(`/api/v1/users/${user1.uuid}/verify-email-change`) + .send({ code: emailVerify1, email: 'INFO+new@libretexts.org' }) + .set('Cookie', await createSessionCookiesForTest(user1.uuid)); + + expect(response.status).to.equal(200); + const updatedUser = await User.findOne({ where: { uuid: user1.uuid } }); + expect(updatedUser?.get('email')).to.equal('info+new@libretexts.org'); + }); + it('should reject a direct email change that collides on case alone', async () => { + const user1 = await User.create({ + uuid: uuidv4(), + email: 'info@libretexts.org', + disabled: false, + expired: false, + }); + await User.create({ + uuid: uuidv4(), + email: 'taken@libretexts.org', + disabled: false, + expired: false, + }); + + const response = await request(server) + .post(`/api/v1/users/${user1.uuid}/email-change-direct`) + .send({ email: 'Taken@LibreTexts.ORG' }) + .auth(mainAPIUserUsername, mainAPIUserPassword); + + expect(response.status).to.equal(400); + const unchanged = await User.findOne({ where: { uuid: user1.uuid } }); + expect(unchanged?.get('email')).to.equal('info@libretexts.org'); + }); + it('should store a direct email change in canonical form', async () => { + const user1 = await User.create({ + uuid: uuidv4(), + email: 'info@libretexts.org', + disabled: false, + expired: false, + }); + + const response = await request(server) + .post(`/api/v1/users/${user1.uuid}/email-change-direct`) + .send({ email: ' Info.User@LibreTexts.ORG ' }) + .auth(mainAPIUserUsername, mainAPIUserPassword); + + expect(response.status).to.equal(200); + expect(response.body?.data?.email).to.equal('info.user@libretexts.org'); + const updatedUser = await User.findOne({ where: { uuid: user1.uuid } }); + expect(updatedUser?.get('email')).to.equal('info.user@libretexts.org'); + }); it('should add user to existing organization', async () => { const org1 = await Organization.create({ name: 'LibreTexts' }); const user1 = await User.create({ diff --git a/server/validators/auth.ts b/server/validators/auth.ts index 420a91f..f489d39 100644 --- a/server/validators/auth.ts +++ b/server/validators/auth.ts @@ -1,8 +1,8 @@ import joi from 'joi'; -import { passwordValidator, timeZoneValidator } from './shared'; +import { emailValidator, passwordValidator, principalUsernameValidator, timeZoneValidator } from './shared'; export const registerSchema = joi.object({ - email: joi.string().email().required(), + email: emailValidator.required(), password: passwordValidator, }); @@ -56,7 +56,7 @@ export const externalProvisionUserSchema = joi.object({ }).unknown(true); export const autoProvisionUserSchema = joi.object({ - email: joi.string().email().required(), + email: emailValidator.required(), first_name: joi.string().min(1).max(100).trim().required(), last_name: joi.string().min(1).max(100).trim().required(), user_type: joi.string().valid('student', 'instructor').required(), @@ -64,7 +64,7 @@ export const autoProvisionUserSchema = joi.object({ }); export const verifyEmailSchema = joi.object({ - email: joi.string().email().required(), + email: emailValidator.required(), code: joi.number().integer().min(100000).max(999999).required(), }); @@ -84,7 +84,7 @@ export const completeRegistrationSchema = joi.object({ }); export const initResetPasswordSchema = joi.object({ - email: joi.string().email().required(), + email: emailValidator.required(), redirectURI: joi.string().uri(), }); @@ -92,3 +92,9 @@ export const resetPasswordSchema = joi.object({ token: joi.string().length(64).required(), password: passwordValidator, }); + +export const checkCASInterruptQuerySchema = joi.object({ + username: principalUsernameValidator.required(), + registeredService: joi.string(), + service: joi.string(), +}).unknown(true); diff --git a/server/validators/shared.ts b/server/validators/shared.ts index 0609d5f..764fd60 100644 --- a/server/validators/shared.ts +++ b/server/validators/shared.ts @@ -2,6 +2,28 @@ import joi from 'joi'; import { zxcvbn, zxcvbnOptions } from '@zxcvbn-ts/core'; import { passwordStrengthOptions } from '../../passwordstrength'; import { TimeZone } from '../models'; +import { normalizeEmail } from '../../email'; + +/** + * Validates an email address after rewriting it into its canonical form. Joi applies + * rules in the order they are chained, so the value reaching `.email()` (and reaching + * the controller, since `validate()` assigns the validated result back onto the + * request) is already trimmed, NFC-normalized, and lowercased. + */ +export const emailValidator = joi.string() + .custom((value) => normalizeEmail(value)) + .email({ minDomainSegments: 2 }) + .max(255); + +/** + * Validates a CAS-supplied username, which may be an email address, a LibreOne UUID, or + * an external identity provider's subject ID. Only the email shape is normalized, since + * subject IDs are opaque and may be case-sensitive at the provider. + */ +export const principalUsernameValidator = joi.string() + .custom((value) => (value.includes('@') ? normalizeEmail(value) : value.trim())) + .min(1) + .max(255); export const passwordValidator = joi.string().custom((password, helper) => { zxcvbnOptions.setOptions(passwordStrengthOptions); diff --git a/server/validators/store.ts b/server/validators/store.ts index e7b0e8d..f144361 100644 --- a/server/validators/store.ts +++ b/server/validators/store.ts @@ -1,4 +1,5 @@ import joi from 'joi'; +import { emailValidator } from './shared'; const uuidValidator = joi.string().uuid({ version: "uuidv4" }); @@ -14,7 +15,7 @@ export const applicationLicenseIdSchema = joi.object({ export const generateAccessCodeSchema = joi.object({ stripe_price_id: joi.string().optional().not().empty(), application_license_id: uuidValidator.optional(), - email: joi.string().email().required(), + email: emailValidator.required(), }).xor('stripe_price_id', 'application_license_id'); export const bulkGenerateAccessCodesSchema = joi.object({ diff --git a/server/validators/user.ts b/server/validators/user.ts index e779e2f..8093200 100644 --- a/server/validators/user.ts +++ b/server/validators/user.ts @@ -4,7 +4,7 @@ import { applicationTypeValidator, } from "./applications"; import { orgIDValidator } from "./organizations"; -import { passwordValidator, timeZoneValidator } from "./shared"; +import { emailValidator, passwordValidator, principalUsernameValidator, timeZoneValidator } from "./shared"; import { UserOrganizationAdminRoleEnum } from "../controllers/PermissionsController"; const uuidValidator = joi.string().uuid({ version: "uuidv4" }).required(); @@ -50,7 +50,7 @@ export const createUserVerificationRequestSchema = joi }); export const createUserEmailChangeRequestSchema = joi.object({ - email: joi.string().email().required(), + email: emailValidator.required(), }); export const disableUserSchema = joi.object({ @@ -79,14 +79,8 @@ export const getMultipleUserOrganizationsSchema = joi.object({ }); export const resolvePrincipalAttributesSchema = joi.object({ - username: joi - .alternatives() - .try( - joi.string().email().message("Invalid email address"), - joi.string().uuid().message("Invalid UUID") - ) - .required(), -}); + username: principalUsernameValidator.required(), +}).unknown(true); export const updateUserSchema = joi.object({ first_name: joi.string().min(1).max(100).trim(), @@ -103,11 +97,11 @@ export const updateUserSchema = joi.object({ export const updateUserEmailSchema = joi.object({ code: joi.number().integer().min(100000).max(999999).required(), - email: joi.string().email().required(), + email: emailValidator.required(), }); export const updateUserEmailDirectSchema = joi.object({ - email: joi.string().email().required(), + email: emailValidator.required(), remove_external_auth: joi.boolean().default(false), });