From 4055dcbcf744c039e7fac3c5f00a87ee6bca344b Mon Sep 17 00:00:00 2001 From: rykuno Date: Thu, 27 Jun 2024 13:27:14 -0500 Subject: [PATCH] added owasp recommendations to IAM methods --- .../server/api/controllers/iam.controller.ts | 10 +++-- src/lib/server/api/index.ts | 1 - ...man_fly.sql => 0000_nostalgic_skrulls.sql} | 0 .../migrations/meta/0000_snapshot.json | 2 +- .../database/migrations/meta/_journal.json | 4 +- .../email-templates/email-change-notice.hbs | 18 ++++++++ ...andlebars => email-verification-token.hbs} | 1 - .../{welcome.handlebars => welcome.hbs} | 0 .../api/repositories/users.repository.ts | 8 +++- .../services/email-verifications.service.ts | 41 +++++++------------ .../api/services/login-requests.service.ts | 1 + src/lib/server/api/services/mailer.service.ts | 17 ++++++-- .../api/tests/login-requests.service.test.ts | 4 +- .../(app)/settings/account/+page.server.ts | 4 +- src/routes/(auth)/register/+page.svelte | 2 +- 15 files changed, 68 insertions(+), 45 deletions(-) rename src/lib/server/api/infrastructure/database/migrations/{0000_sudden_human_fly.sql => 0000_nostalgic_skrulls.sql} (100%) create mode 100644 src/lib/server/api/infrastructure/email-templates/email-change-notice.hbs rename src/lib/server/api/infrastructure/email-templates/{email-verification.handlebars => email-verification-token.hbs} (97%) rename src/lib/server/api/infrastructure/email-templates/{welcome.handlebars => welcome.hbs} (100%) diff --git a/src/lib/server/api/controllers/iam.controller.ts b/src/lib/server/api/controllers/iam.controller.ts index cc42c61..06ad476 100644 --- a/src/lib/server/api/controllers/iam.controller.ts +++ b/src/lib/server/api/controllers/iam.controller.ts @@ -90,14 +90,16 @@ export class IamController implements Controller { }); return c.json({ status: 'success' }); }) - .post('/email/sendVerification', requireAuth, zValidator('json', updateEmailDto), limiter({ limit: 10, minutes: 60 }), async (c) => { + .patch('/email', requireAuth, zValidator('json', updateEmailDto), limiter({ limit: 10, minutes: 60 }), async (c) => { const json = c.req.valid('json'); - await this.emailVerificationsService.dispatchEmailVerificationToken(c.var.user.id, json.email); + await this.emailVerificationsService.dispatchEmailVerificationRequest(c.var.user.id, json.email); return c.json({ message: 'Verification email sent' }); }) - .post('/email/verify', requireAuth, zValidator('json', verifyEmailDto), limiter({ limit: 10, minutes: 60 }), async (c) => { + // this could also be named to use custom methods, aka /email:verify + // https://cloud.google.com/apis/design/custom_methods + .post('/email/verification', requireAuth, zValidator('json', verifyEmailDto), limiter({ limit: 10, minutes: 60 }), async (c) => { const json = c.req.valid('json'); - await this.emailVerificationsService.processEmailVerificationToken(c.var.user.id, json.token); + await this.emailVerificationsService.processEmailVerificationRequest(c.var.user.id, json.token); return c.json({ message: 'Verified and updated' }); }); } diff --git a/src/lib/server/api/index.ts b/src/lib/server/api/index.ts index 257aea9..9820ab3 100644 --- a/src/lib/server/api/index.ts +++ b/src/lib/server/api/index.ts @@ -6,7 +6,6 @@ import { container } from 'tsyringe'; import { validateAuthSession, verifyOrigin } from './middleware/auth.middleware'; import { IamController } from './controllers/iam.controller'; import { config } from './common/config'; -import { UsersController } from './controllers/users.controller'; /* -------------------------------------------------------------------------- */ /* Client Request */ diff --git a/src/lib/server/api/infrastructure/database/migrations/0000_sudden_human_fly.sql b/src/lib/server/api/infrastructure/database/migrations/0000_nostalgic_skrulls.sql similarity index 100% rename from src/lib/server/api/infrastructure/database/migrations/0000_sudden_human_fly.sql rename to src/lib/server/api/infrastructure/database/migrations/0000_nostalgic_skrulls.sql diff --git a/src/lib/server/api/infrastructure/database/migrations/meta/0000_snapshot.json b/src/lib/server/api/infrastructure/database/migrations/meta/0000_snapshot.json index eafc6a1..502d70c 100644 --- a/src/lib/server/api/infrastructure/database/migrations/meta/0000_snapshot.json +++ b/src/lib/server/api/infrastructure/database/migrations/meta/0000_snapshot.json @@ -1,5 +1,5 @@ { - "id": "2e0c1e11-ed33-45bf-8084-c3200d8f65a8", + "id": "2fdb0575-b4b3-4ebb-9ca0-73a655a7fbe7", "prevId": "00000000-0000-0000-0000-000000000000", "version": "6", "dialect": "postgresql", diff --git a/src/lib/server/api/infrastructure/database/migrations/meta/_journal.json b/src/lib/server/api/infrastructure/database/migrations/meta/_journal.json index d4e9fe2..a777334 100644 --- a/src/lib/server/api/infrastructure/database/migrations/meta/_journal.json +++ b/src/lib/server/api/infrastructure/database/migrations/meta/_journal.json @@ -5,8 +5,8 @@ { "idx": 0, "version": "6", - "when": 1719436322147, - "tag": "0000_sudden_human_fly", + "when": 1719512747861, + "tag": "0000_nostalgic_skrulls", "breakpoints": false } ] diff --git a/src/lib/server/api/infrastructure/email-templates/email-change-notice.hbs b/src/lib/server/api/infrastructure/email-templates/email-change-notice.hbs new file mode 100644 index 0000000..4d67bf9 --- /dev/null +++ b/src/lib/server/api/infrastructure/email-templates/email-change-notice.hbs @@ -0,0 +1,18 @@ + + + + + Email Change Request + + +

Email address change notice

+

+ An update to your email address has been requested. If this is unexpected or you did not perform this action, please login and secure your account.

+ + + \ No newline at end of file diff --git a/src/lib/server/api/infrastructure/email-templates/email-verification.handlebars b/src/lib/server/api/infrastructure/email-templates/email-verification-token.hbs similarity index 97% rename from src/lib/server/api/infrastructure/email-templates/email-verification.handlebars rename to src/lib/server/api/infrastructure/email-templates/email-verification-token.hbs index ae2f311..100a761 100644 --- a/src/lib/server/api/infrastructure/email-templates/email-verification.handlebars +++ b/src/lib/server/api/infrastructure/email-templates/email-verification-token.hbs @@ -10,7 +10,6 @@ Thanks for using example.com. We want to make sure it's really you. Please enter the following verification code when prompted. If you don't have an exmaple.com an account, you can ignore this message.

- {{!--

{{token}}

--}}

Verification Code

{{token}}

diff --git a/src/lib/server/api/infrastructure/email-templates/welcome.handlebars b/src/lib/server/api/infrastructure/email-templates/welcome.hbs similarity index 100% rename from src/lib/server/api/infrastructure/email-templates/welcome.handlebars rename to src/lib/server/api/infrastructure/email-templates/welcome.hbs diff --git a/src/lib/server/api/repositories/users.repository.ts b/src/lib/server/api/repositories/users.repository.ts index 659d3de..cbc7e9f 100644 --- a/src/lib/server/api/repositories/users.repository.ts +++ b/src/lib/server/api/repositories/users.repository.ts @@ -26,7 +26,7 @@ export type UpdateUser = Partial; @injectable() export class UsersRepository implements Repository { - constructor(@inject(DatabaseProvider) private db: DatabaseProvider) {} + constructor(@inject(DatabaseProvider) private db: DatabaseProvider) { } async findOneById(id: string) { return this.db.query.usersTable.findFirst({ @@ -34,6 +34,12 @@ export class UsersRepository implements Repository { }); } + async findOneByIdOrThrow(id: string) { + const user = await this.findOneById(id); + if (!user) throw Error('User not found'); + return user; + } + async findOneByEmail(email: string) { return this.db.query.usersTable.findFirst({ where: eq(usersTable.email, email) diff --git a/src/lib/server/api/services/email-verifications.service.ts b/src/lib/server/api/services/email-verifications.service.ts index 31d00af..c668cc3 100644 --- a/src/lib/server/api/services/email-verifications.service.ts +++ b/src/lib/server/api/services/email-verifications.service.ts @@ -6,24 +6,6 @@ import { TokensService } from './tokens.service'; import { UsersRepository } from '../repositories/users.repository'; import { EmailVerificationsRepository } from '../repositories/email-verifications.repository'; -/* -------------------------------------------------------------------------- */ -/* Service */ -/* -------------------------------------------------------------------------- */ -/* -------------------------------------------------------------------------- */ -/* ---------------------------------- About --------------------------------- */ -/* -Services are responsible for handling business logic and data manipulation. -They genreally call on repositories or other services to complete a use-case. -*/ -/* ---------------------------------- Notes --------------------------------- */ -/* -Services should be kept as clean and simple as possible. - -Create private functions to handle complex logic and keep the public methods as -simple as possible. This makes the service easier to read, test and understand. -*/ -/* -------------------------------------------------------------------------- */ - @injectable() export class EmailVerificationsService { constructor( @@ -34,29 +16,36 @@ export class EmailVerificationsService { @inject(EmailVerificationsRepository) private readonly emailVerificationsRepository: EmailVerificationsRepository, ) { } - - async dispatchEmailVerificationToken(userId: string, requestedEmail: string) { + // These steps follow the process outlined in OWASP's "Changing A User's Email Address" guide. + // https://cheatsheetseries.owasp.org/cheatsheets/Authentication_Cheat_Sheet.html#changing-a-users-registered-email-address + async dispatchEmailVerificationRequest(userId: string, requestedEmail: string) { // generate a token and expiry const { token, expiry, hashedToken } = await this.tokensService.generateTokenWithExpiryAndHash(15, 'm') + const user = await this.usersRepository.findOneByIdOrThrow(userId) // create a new email verification record await this.emailVerificationsRepository.create({ requestedEmail, userId, hashedToken, expiresAt: expiry }) - // send the verification email - we don't need to await success and will opt for good-faith since we - // will offer a way to resend the email if it fails - this.mailerService.sendEmailVerification({ + // A confirmation-required email message to the proposed new address, instructing the user to + // confirm the change and providing a link for unexpected situations + this.mailerService.sendEmailVerificationToken({ to: requestedEmail, props: { token } }) + + // A notification-only email message to the current address, alerting the user to the impending change and + // providing a link for an unexpected situation. + this.mailerService.sendEmailChangeNotification({ + to: user.email, + props: null + }) } - async processEmailVerificationToken(userId: string, token: string) { + async processEmailVerificationRequest(userId: string, token: string) { const validRecord = await this.findAndBurnEmailVerificationToken(userId, token) if (!validRecord) throw BadRequest('Invalid token'); - - // burn the token and update the user await this.usersRepository.update(userId, { email: validRecord.requestedEmail, verified: true }); } diff --git a/src/lib/server/api/services/login-requests.service.ts b/src/lib/server/api/services/login-requests.service.ts index a4b5dfa..ccd1601 100644 --- a/src/lib/server/api/services/login-requests.service.ts +++ b/src/lib/server/api/services/login-requests.service.ts @@ -50,6 +50,7 @@ export class LoginRequestsService { private async handleNewUserRegistration(email: string) { const newUser = await this.usersRepository.create({ email, verified: true, avatar: null }) this.mailerService.sendWelcome({ to: email, props: null }); + // TODO: add whatever onboarding process or extra data you need here return newUser } diff --git a/src/lib/server/api/services/mailer.service.ts b/src/lib/server/api/services/mailer.service.ts index 62ed799..368fdb9 100644 --- a/src/lib/server/api/services/mailer.service.ts +++ b/src/lib/server/api/services/mailer.service.ts @@ -46,8 +46,8 @@ export class MailerService { } }); - sendEmailVerification(data: SendTemplate<{ token: string }>) { - const template = handlebars.compile(this.getTemplate('email-verification')); + sendEmailVerificationToken(data: SendTemplate<{ token: string }>) { + const template = handlebars.compile(this.getTemplate('email-verification-token')); return this.send({ to: data.to, subject: 'Email Verification', @@ -55,8 +55,17 @@ export class MailerService { }); } + sendEmailChangeNotification(data: SendTemplate) { + const template = handlebars.compile(this.getTemplate('email-change-notice')); + return this.send({ + to: data.to, + subject: 'Email Change Notice', + html: template(null) + }); + } + sendLoginRequest(data: SendTemplate<{ token: string }>) { - const template = handlebars.compile(this.getTemplate('email-verification')); + const template = handlebars.compile(this.getTemplate('email-verification-token')); return this.send({ to: data.to, subject: 'Login Request', @@ -88,7 +97,7 @@ export class MailerService { const __filename = fileURLToPath(import.meta.url); // get the resolved path to the file const __dirname = path.dirname(__filename); // get the name of the directory return fs.readFileSync( - path.join(__dirname, `../infrastructure/email-templates/${template}.handlebars`), + path.join(__dirname, `../infrastructure/email-templates/${template}.hbs`), 'utf-8' ); } diff --git a/src/lib/server/api/tests/login-requests.service.test.ts b/src/lib/server/api/tests/login-requests.service.test.ts index cc3fbf1..c2b4715 100644 --- a/src/lib/server/api/tests/login-requests.service.test.ts +++ b/src/lib/server/api/tests/login-requests.service.test.ts @@ -36,9 +36,9 @@ describe('LoginRequestService', () => { describe('Create', () => { tokensService.generateTokenWithExpiryAndHash = vi.fn().mockResolvedValue({ - token: "111", + token: "1", expiry: new Date(), - hashedToken: "111" + hashedToken: "xyz" } satisfies Awaited>) loginRequestsRepository.create = vi.fn().mockResolvedValue({ diff --git a/src/routes/(app)/settings/account/+page.server.ts b/src/routes/(app)/settings/account/+page.server.ts index 3e97af6..f5e115a 100644 --- a/src/routes/(app)/settings/account/+page.server.ts +++ b/src/routes/(app)/settings/account/+page.server.ts @@ -18,7 +18,7 @@ export const actions = { updateEmail: async ({ request, locals }) => { const updateEmailForm = await superValidate(request, zod(updateEmailDto)); if (!updateEmailForm.valid) return fail(StatusCodes.BAD_REQUEST, { updateEmailForm }) - const { error } = await locals.api.iam.email.sendVerification.$post({ json: updateEmailForm.data }).then(locals.parseApiResponse); + const { error } = await locals.api.iam.email.$patch({ json: updateEmailForm.data }).then(locals.parseApiResponse); if (error) return setError(updateEmailForm, 'email', error); return { updateEmailForm } }, @@ -26,7 +26,7 @@ export const actions = { const verifyEmailForm = await superValidate(request, zod(verifyEmailDto)); console.log(verifyEmailForm) if (!verifyEmailForm.valid) return fail(StatusCodes.BAD_REQUEST, { verifyEmailForm }) - const { error } = await locals.api.iam.email.verify.$post({ json: verifyEmailForm.data }).then(locals.parseApiResponse); + const { error } = await locals.api.iam.email.verification.$post({ json: verifyEmailForm.data }).then(locals.parseApiResponse); if (error) return setError(verifyEmailForm, 'token', error); return { verifyEmailForm } } diff --git a/src/routes/(auth)/register/+page.svelte b/src/routes/(auth)/register/+page.svelte index 1a961ae..892272b 100644 --- a/src/routes/(auth)/register/+page.svelte +++ b/src/routes/(auth)/register/+page.svelte @@ -67,7 +67,7 @@ {:else} {@render emailForm()} {/if} - +
By registering, you agree to our Terms of Service