Skip to content

Commit

Permalink
Tighten up password reset process
Browse files Browse the repository at this point in the history
Link userId to password reset token via database

Don't allow reset if the user does not have a password set - this is to protect against two users with duplicate email addresses

Don't allow a user to set up a password to login if another user with the same email has a password - that would lead to ambiguous login experience - this is just to ease the migration process to allow duplicate email addresses to be added in. Eventually we would ideally block this.

Moved google/sfdc sign up above other fields during login/sign up so we can nudge the user to do that instead of a password
  • Loading branch information
paustint committed Nov 2, 2024
1 parent bc37e83 commit 299e9f0
Show file tree
Hide file tree
Showing 10 changed files with 136 additions and 95 deletions.
5 changes: 4 additions & 1 deletion apps/api/src/app/controllers/user.controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,10 @@ const getFullUserProfile = createRoute(routeDefinition.getFullUserProfile.valida

const initPassword = createRoute(routeDefinition.initPassword.validators, async ({ body, user }, req, res) => {
const { password } = body;
await setPasswordForUser(user.id, password);
const results = await setPasswordForUser(user.id, password);
if ('error' in results) {
throw new UserFacingError(results.error);
}
sendJson(res, await userDbService.findUserWithIdentitiesById(user.id));

createUserActivityFromReq(req, res, {
Expand Down
2 changes: 1 addition & 1 deletion apps/api/src/app/routes/auth.routes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ routes.get('/csrf', LAX_AuthRateLimit, authController.routeDefinition.getCsrfTok
routes.get('/session', LAX_AuthRateLimit, authController.routeDefinition.getSession.controllerFn());

// Init OAuth flow
routes.post('/signin/:provider', STRICT_AuthRateLimit, verifyCaptcha, authController.routeDefinition.signin.controllerFn());
routes.post('/signin/:provider', STRICT_AuthRateLimit, authController.routeDefinition.signin.controllerFn());
// Login via OAuth or credentials
routes.get('/callback/:provider', STRICT_AuthRateLimit, authController.routeDefinition.callback.controllerFn());
routes.post('/callback/:provider', STRICT_AuthRateLimit, verifyCaptcha, authController.routeDefinition.callback.controllerFn());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,9 @@ export class AuthenticationPage {

await this.goToPasswordReset();
await this.fillOutResetPasswordForm(email);
await expect(this.page.getByText('Check your email to continue the reset process.')).toBeVisible();
await expect(
this.page.getByText('You will receive an email with instructions if an account exists and is eligible for password reset.')
).toBeVisible();

// ensure email verification was sent
await prisma.emailActivity.findFirstOrThrow({ where: { email, subject: { contains: 'Reset your password' } } });
Expand Down
4 changes: 3 additions & 1 deletion apps/jetstream-e2e/src/tests/authentication/login2.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,9 @@ test.describe('Login 2', () => {

await authenticationPage.goToPasswordReset();
await authenticationPage.fillOutResetPasswordForm(email);
await expect(page.getByText('Check your email to continue the reset process.')).toBeVisible();
await expect(
page.getByText('You will receive an email with instructions if an account exists and is eligible for password reset.')
).toBeVisible();

// ensure email verification was sent
await prisma.emailActivity.findFirstOrThrow({ where: { email, subject: { contains: 'Reset your password' } } });
Expand Down
4 changes: 2 additions & 2 deletions apps/jetstream/src/app/components/profile/Profile.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ export const Profile = () => {
trackEvent(ANALYTICS_KEYS.settings_password_action, { action: 'set-password' });
} catch (ex) {
fireToast({
message: 'There was a problem setting your password. Try again or file a support ticket for assistance.',
message: ex.message || 'There was a problem resetting your password. Try again or file a support ticket for assistance.',
type: 'error',
});
rollbar.error('Settings: Error setting password', { stack: ex.stack, message: ex.message });
Expand All @@ -138,7 +138,7 @@ export const Profile = () => {
});
} catch (ex) {
fireToast({
message: 'There was a problem resetting your password. Try again or file a support ticket for assistance.',
message: ex.message || 'There was a problem resetting your password. Try again or file a support ticket for assistance.',
type: 'error',
});
rollbar.error('Settings: Error resetting password', { stack: ex.stack, message: ex.message });
Expand Down
95 changes: 47 additions & 48 deletions apps/landing/components/auth/LoginOrSignUp.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,54 @@ export function LoginOrSignUp({ action, providers, csrfToken }: LoginOrSignUpPro
{action === 'login' ? 'Sign in' : 'Sign up'}
</h2>
</div>

<div className="mt-10 sm:mx-auto sm:w-full sm:max-w-sm">
<div className="grid grid-cols-2 gap-4">
<form action={providers?.google.signinUrl} method="POST">
<input type="hidden" name="csrfToken" value={csrfToken} />

{providers?.google.callbackUrl && <input type="hidden" name="callbackUrl" value={providers?.google.callbackUrl} />}
<button
type="submit"
className="flex w-full items-center justify-center gap-3 rounded-md bg-white px-3 py-2 text-sm font-semibold text-gray-900 shadow-sm ring-1 ring-inset ring-gray-300 hover:bg-gray-50 focus-visible:ring-transparent"
>
<img
src="https://res.cloudinary.com/getjetstream/image/upload/v1693697889/public/google-login-icon_bzw1hi.svg"
alt="Google Logo"
className="h-5 w-5"
/>
<span className="text-sm font-semibold leading-6">Google</span>
</button>
</form>
<form action={providers?.salesforce.signinUrl} method="POST">
<input type="hidden" name="csrfToken" value={csrfToken} />

{providers?.salesforce.callbackUrl && <input type="hidden" name="callbackUrl" value={providers?.salesforce.callbackUrl} />}
<button
type="submit"
className="flex w-full items-center justify-center gap-3 rounded-md bg-white px-3 py-2 text-sm font-semibold text-gray-900 shadow-sm ring-1 ring-inset ring-gray-300 hover:bg-gray-50 focus-visible:ring-transparent"
>
<img
src="https://res.cloudinary.com/getjetstream/image/upload/v1724511801/salesforce-blue_qdptxw.svg"
alt="Salesforce Logo"
className="h-5 w-5"
/>
<span className="text-sm font-semibold leading-6">Salesforce</span>
</button>
</form>
</div>

<div>
<div className="relative mt-10">
<div aria-hidden="true" className="absolute inset-0 flex items-center">
<div className="w-full border-t border-gray-200" />
</div>
<div className="relative flex justify-center text-sm font-medium leading-6">
<span className="bg-white px-6 text-gray-900">Or continue with</span>
</div>
</div>
</div>

<form
action={providers?.credentials.callbackUrl}
onSubmit={handleSubmit(onSubmit)}
Expand Down Expand Up @@ -224,54 +271,6 @@ export function LoginOrSignUp({ action, providers, csrfToken }: LoginOrSignUpPro
</div>
</form>

<div>
<div className="relative mt-10">
<div aria-hidden="true" className="absolute inset-0 flex items-center">
<div className="w-full border-t border-gray-200" />
</div>
<div className="relative flex justify-center text-sm font-medium leading-6">
<span className="bg-white px-6 text-gray-900">Or continue with</span>
</div>
</div>

<div className="mt-6 grid grid-cols-2 gap-4">
<form action={providers?.google.signinUrl} method="POST">
<input type="hidden" name="csrfToken" value={csrfToken} />
<input type="hidden" name="captchaToken" value={captchaToken} />

{providers?.google.callbackUrl && <input type="hidden" name="callbackUrl" value={providers?.google.callbackUrl} />}
<button
type="submit"
className="flex w-full items-center justify-center gap-3 rounded-md bg-white px-3 py-2 text-sm font-semibold text-gray-900 shadow-sm ring-1 ring-inset ring-gray-300 hover:bg-gray-50 focus-visible:ring-transparent"
>
<img
src="https://res.cloudinary.com/getjetstream/image/upload/v1693697889/public/google-login-icon_bzw1hi.svg"
alt="Google Logo"
className="h-5 w-5"
/>
<span className="text-sm font-semibold leading-6">Google</span>
</button>
</form>
<form action={providers?.salesforce.signinUrl} method="POST">
<input type="hidden" name="csrfToken" value={csrfToken} />
<input type="hidden" name="captchaToken" value={captchaToken} />

{providers?.salesforce.callbackUrl && <input type="hidden" name="callbackUrl" value={providers?.salesforce.callbackUrl} />}
<button
type="submit"
className="flex w-full items-center justify-center gap-3 rounded-md bg-white px-3 py-2 text-sm font-semibold text-gray-900 shadow-sm ring-1 ring-inset ring-gray-300 hover:bg-gray-50 focus-visible:ring-transparent"
>
<img
src="https://res.cloudinary.com/getjetstream/image/upload/v1724511801/salesforce-blue_qdptxw.svg"
alt="Salesforce Logo"
className="h-5 w-5"
/>
<span className="text-sm font-semibold leading-6">Salesforce</span>
</button>
</form>
</div>
</div>

<RegisterOrSignUpLink action={action} />
</div>
</div>
Expand Down
7 changes: 6 additions & 1 deletion apps/landing/components/auth/PasswordResetInit.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,12 @@ export function PasswordResetInit({ csrfToken }: PasswordResetInitProps) {
<h2 className="mt-10 text-center text-2xl font-bold leading-9 tracking-tight text-gray-900">Reset Password</h2>
</div>
<div className="mt-10 sm:mx-auto sm:w-full sm:max-w-sm">
{isSubmitted && <Alert message="Check your email to continue the reset process." type="success" />}
{isSubmitted && (
<Alert
message="You will receive an email with instructions if an account exists and is eligible for password reset. Contact [email protected] if you need further assistance."
type="success"
/>
)}
{!isSubmitted && (
<form onSubmit={handleSubmit(onSubmit)} method="POST" noValidate className="space-y-6">
<input type="hidden" {...register('csrfToken')} />
Expand Down
53 changes: 34 additions & 19 deletions libs/auth/server/src/lib/auth.db.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -345,16 +345,29 @@ export async function revokeAllUserSessions(userId: string, exceptId?: Maybe<str
}

export async function setPasswordForUser(id: string, password: string) {
const UNSAFE_userWithPassword = await prisma.user.findFirst({
select: { id: true, password: true },
const UNSAFE_userWithPassword = await prisma.user.findUnique({
select: { id: true, password: true, email: true },
where: { id },
});

if (!UNSAFE_userWithPassword) {
return { error: new InvalidCredentials() };
}

// FIXME: this is kinda dumb since the user can remove the password then re-add it
if (UNSAFE_userWithPassword.password) {
return { error: new Error('Cannot set password when already set, you must go through the password reset flow') };
}

const usersWithSamesEmail = await prisma.user.findFirst({
select: { id: true, email: true, hasPasswordSet: true },
where: { id: { not: id }, email: UNSAFE_userWithPassword.email },
});

if (usersWithSamesEmail && usersWithSamesEmail.hasPasswordSet) {
return { error: new Error('Cannot set password, another user with the same email address already has a password set') };
}

return prisma.user.update({
select: userSelect,
data: { password: await hashPassword(password), passwordUpdatedAt: new Date() },
Expand All @@ -363,11 +376,18 @@ export async function setPasswordForUser(id: string, password: string) {
}

export const generatePasswordResetToken = async (email: string) => {
const user = await prisma.user.findFirst({
where: { email },
// NOTE: There could be duplicate users with the same email, but only one with a password set
// These users were migrated from Auth0, but we do not support this as a standard path
const user = await prisma.user.findMany({
where: { email, password: { not: null } },
take: 2,
});

if (!user) {
if (!user.length) {
throw new InvalidAction();
}

if (user.length > 1) {
throw new InvalidAction();
}

Expand All @@ -384,6 +404,7 @@ export const generatePasswordResetToken = async (email: string) => {

const passwordResetToken = await prisma.passwordResetToken.create({
data: {
userId: user[0].id,
email,
expiresAt: addMinutes(new Date(), 10),
},
Expand All @@ -394,28 +415,20 @@ export const generatePasswordResetToken = async (email: string) => {

export const resetUserPassword = async (email: string, token: string, password: string) => {
// if there is an existing token, delete it
const existingToken = await prisma.passwordResetToken.findUnique({
const restToken = await prisma.passwordResetToken.findUnique({
where: { email_token: { email, token } },
});

if (!existingToken) {
if (!restToken) {
throw new InvalidOrExpiredResetToken();
}

// delete token - we don't need it anymore and if we fail later, the user will need to reset again
await prisma.passwordResetToken.delete({
where: { email_token: { email, token: existingToken.token } },
});

if (existingToken.expiresAt < new Date()) {
throw new InvalidOrExpiredResetToken();
}

const user = await prisma.user.findFirst({
where: { email },
where: { email_token: { email, token: restToken.token } },
});

if (!user) {
if (restToken.expiresAt < new Date()) {
throw new InvalidOrExpiredResetToken();
}

Expand All @@ -427,11 +440,11 @@ export const resetUserPassword = async (email: string, token: string, password:
passwordUpdatedAt: new Date(),
},
where: {
id: user.id,
id: restToken.userId,
},
});

await revokeAllUserSessions(user.id);
await revokeAllUserSessions(restToken.userId);
};

export const removePasswordFromUser = async (id: string) => {
Expand Down Expand Up @@ -464,6 +477,7 @@ async function getUserAndVerifyPassword(email: string, password: string) {
if (!UNSAFE_userWithPassword) {
return { error: new InvalidCredentials() };
}
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
if (await verifyPassword(password, UNSAFE_userWithPassword.password!)) {
return {
error: null,
Expand Down Expand Up @@ -536,6 +550,7 @@ async function createUserFromProvider(providerUser: ProviderUser, provider: Oaut
email: providerUser.email,
// TODO: do we really get any benefit from storing this userId like this?
// TODO: only reason I can think of is user migration since the id is a UUID so we need to different identifier
// TODO: this is nice as we can identify which identity is primary without joining the identity table - but could solve in other ways
userId: `${provider}|${providerUser.id}`,
name: providerUser.name,
emailVerified: providerUser.emailVerified,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
/*
Warnings:
- Added the required column `userId` to the `PasswordResetToken` table without a default value. This is not possible if the table is not empty.
*/
-- AlterTable
ALTER TABLE "PasswordResetToken" ADD COLUMN "userId" UUID NOT NULL;

-- AddForeignKey
ALTER TABLE "PasswordResetToken" ADD CONSTRAINT "PasswordResetToken_userId_fkey" FOREIGN KEY ("userId") REFERENCES "User"("id") ON DELETE CASCADE ON UPDATE CASCADE;
46 changes: 25 additions & 21 deletions prisma/schema.prisma
Original file line number Diff line number Diff line change
Expand Up @@ -8,30 +8,31 @@ datasource db {
}

model User {
id String @id @default(dbgenerated("uuid_generate_v4()")) @db.Uuid
id String @id @default(dbgenerated("uuid_generate_v4()")) @db.Uuid
// TODO: we should deprecate this field - is there a purpose of having two user ids?
// Might be nice to rename this to something like "legacyId" for reference
userId String @unique @db.VarChar
userId String @unique @db.VarChar
// TODO: we want to make this unique
email String @db.VarChar
emailVerified Boolean @default(false)
password String? @db.VarChar
passwordUpdatedAt DateTime?
name String @db.VarChar
nickname String? @db.VarChar
picture String? @db.VarChar
appMetadata Json? @db.Json
preferences UserPreference?
salesforceOrgs SalesforceOrg[]
organizations JetstreamOrganization[]
identities AuthIdentity[]
authFactors AuthFactors[]
loginActivity LoginActivity[]
rememberdDevices RememberedDevice[]
lastLoggedIn DateTime?
deletedAt DateTime?
createdAt DateTime @default(now()) @db.Timestamp(6)
updatedAt DateTime @updatedAt
email String @db.VarChar
emailVerified Boolean @default(false)
password String? @db.VarChar
passwordUpdatedAt DateTime?
name String @db.VarChar
nickname String? @db.VarChar
picture String? @db.VarChar
appMetadata Json? @db.Json
preferences UserPreference?
salesforceOrgs SalesforceOrg[]
organizations JetstreamOrganization[]
identities AuthIdentity[]
authFactors AuthFactors[]
loginActivity LoginActivity[]
rememberdDevices RememberedDevice[]
passwordResetTokens PasswordResetToken[]
lastLoggedIn DateTime?
deletedAt DateTime?
createdAt DateTime @default(now()) @db.Timestamp(6)
updatedAt DateTime @updatedAt
}

model AuthFactors {
Expand All @@ -49,10 +50,13 @@ model AuthFactors {
}

model PasswordResetToken {
userId String @db.Uuid
email String
token String @unique @default(dbgenerated("uuid_generate_v4()")) @db.Uuid
expiresAt DateTime
user User @relation(fields: [userId], references: [id], onDelete: Cascade)
@@unique([email, token])
}

Expand Down

0 comments on commit 299e9f0

Please sign in to comment.