added owasp recommendations to IAM methods

Rykuno · Rykuno · commit 4055dcbcf744 · 2024-06-27T13:27:14.000-05:00
diff --git 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
@@ -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_nostalgic_skrulls.sql b/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
@@ -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
@@ -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
@@ -0,0 +1,18 @@
+<html lang='en'>
+	<head>
+		<meta http-equiv='X-UA-Compatible' content='IE=edge' />
+		<meta name='viewport' content='width=device-width, initial-scale=1.0' />
+		<title>Email Change Request</title>
+	</head>
+	<body>
+		<p class='title'>Email address change notice </p>
+		<p>
+			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.</p>
+	</body>
+	<style>
+		.title { font-size: 24px; font-weight: 700; } .token-text { font-size: 24px; font-weight: 700; margin-top: 8px; }
+		.token-title { font-size: 18px; font-weight: 700; margin-bottom: 0px; }
+		.center { display: flex; justify-content: center; align-items: center; flex-direction: column;}
+		.token-subtext { font-size: 12px; margin-top: 0px; }
+	</style>
+</html>
diff --git a/src/lib/server/api/infrastructure/email-templates/email-verification-token.hbs 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.</p>
-		{{!-- <p>{{token}}</p> --}}
 		<div class='center'>
 			<p class="token-title">Verification Code</p>
 			<p class='token-text'>{{token}}</p>
diff --git a/src/lib/server/api/infrastructure/email-templates/welcome.hbs b/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
@@ -26,14 +26,20 @@ export type UpdateUser = Partial<CreateUser>;
 
 @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({
 			where: eq(usersTable.id, id)
 		});
 	}
 
+	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
@@ -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
@@ -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
@@ -46,17 +46,26 @@ 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',
 			html: template({ token: data.props.token })
 		});
 	}
 
+	sendEmailChangeNotification(data: SendTemplate<null>) {
+		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
@@ -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<ReturnType<typeof tokensService.generateTokenWithExpiryAndHash>>)
 
     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
@@ -18,15 +18,15 @@ 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 }
 	},
 	verifyEmail: async ({ request, locals }) => {
 		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
@@ -67,7 +67,7 @@
 			{:else}
 				{@render emailForm()}
 			{/if}
-			<Button variant="outline" class="w-full">Login with Discord</Button>
+			<!-- <Button variant="outline" class="w-full">Login with Discord</Button> -->
 		</div>
 		<div class="mt-4 text-center text-sm">
 			By registering, you agree to our <a href="##" class="underline">Terms of Service</a>

Original file line number	Diff line number	Diff line change
`@@ -1,5 +1,5 @@`
`1`	`1`	`{`
`2`		`- "id": "2e0c1e11-ed33-45bf-8084-c3200d8f65a8",`
	`2`	`+ "id": "2fdb0575-b4b3-4ebb-9ca0-73a655a7fbe7",`
`3`	`3`	`"prevId": "00000000-0000-0000-0000-000000000000",`
`4`	`4`	`"version": "6",`
`5`	`5`	`"dialect": "postgresql",`
Original file line number	Diff line number	Diff line change
`@@ -5,8 +5,8 @@`
`5`	`5`	`{`
`6`	`6`	`"idx": 0,`
`7`	`7`	`"version": "6",`
`8`		`- "when": 1719436322147,`
`9`		`- "tag": "0000_sudden_human_fly",`
	`8`	`+ "when": 1719512747861,`
	`9`	`+ "tag": "0000_nostalgic_skrulls",`
`10`	`10`	`"breakpoints": false`
`11`	`11`	`}`
`12`	`12`	`]`