Skip to content
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

refactor: migrates user password to it's own table #13628

Merged
merged 7 commits into from
Feb 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions apps/web/pages/api/auth/changepw.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ export default async function handler(req: NextApiRequest, res: NextApiResponse)
const oldPassword = req.body.oldPassword;
const newPassword = req.body.newPassword;

const currentPassword = user.password;
const currentPassword = user.password?.hash;
if (!currentPassword) {
return res.status(400).json({ error: ErrorCode.UserMissingPassword });
}
Expand All @@ -53,12 +53,12 @@ export default async function handler(req: NextApiRequest, res: NextApiResponse)
}

const hashedPassword = await hashPassword(newPassword);
await prisma.user.update({
await prisma.userPassword.update({
where: {
id: user.id,
userId: user.id,
},
data: {
password: hashedPassword,
hash: hashedPassword,
},
});

Expand Down
6 changes: 5 additions & 1 deletion apps/web/pages/api/auth/reset-password.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,11 @@ export default async function handler(req: NextApiRequest, res: NextApiResponse)
email: maybeRequest.email,
},
data: {
password: hashedPassword,
password: {
update: {
hash: hashedPassword,
},
},
emailVerified: new Date(),
},
});
Expand Down
2 changes: 1 addition & 1 deletion apps/web/pages/api/auth/setup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ async function handler(req: NextApiRequest) {
data: {
username,
email: userEmail,
password: hashedPassword,
password: { create: { hash: hashedPassword } },
role: "ADMIN",
name: parsedQuery.data.full_name,
emailVerified: new Date(),
Expand Down
8 changes: 4 additions & 4 deletions apps/web/pages/api/auth/two-factor/totp/disable.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,22 +23,22 @@ export default async function handler(req: NextApiRequest, res: NextApiResponse)
return res.status(500).json({ error: ErrorCode.InternalServerError });
}

const user = await prisma.user.findUnique({ where: { id: session.user.id } });
const user = await prisma.user.findUnique({ where: { id: session.user.id }, include: { password: true } });
if (!user) {
console.error(`Session references user that no longer exists.`);
return res.status(401).json({ message: "Not authenticated" });
}

if (!user.password && user.identityProvider === IdentityProvider.CAL) {
if (!user.password?.hash && user.identityProvider === IdentityProvider.CAL) {
return res.status(400).json({ error: ErrorCode.UserMissingPassword });
}

if (!user.twoFactorEnabled) {
return res.json({ message: "Two factor disabled" });
}

if (user.password && user.identityProvider === IdentityProvider.CAL) {
const isCorrectPassword = await verifyPassword(req.body.password, user.password);
if (user.password?.hash && user.identityProvider === IdentityProvider.CAL) {
const isCorrectPassword = await verifyPassword(req.body.password, user.password.hash);
if (!isCorrectPassword) {
return res.status(400).json({ error: ErrorCode.IncorrectPassword });
}
Expand Down
6 changes: 3 additions & 3 deletions apps/web/pages/api/auth/two-factor/totp/setup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ export default async function handler(req: NextApiRequest, res: NextApiResponse)
return res.status(500).json({ error: ErrorCode.InternalServerError });
}

const user = await prisma.user.findUnique({ where: { id: session.user.id } });
const user = await prisma.user.findUnique({ where: { id: session.user.id }, include: { password: true } });
if (!user) {
console.error(`Session references user that no longer exists.`);
return res.status(401).json({ message: "Not authenticated" });
Expand All @@ -35,7 +35,7 @@ export default async function handler(req: NextApiRequest, res: NextApiResponse)
return res.status(400).json({ error: ErrorCode.ThirdPartyIdentityProviderEnabled });
}

if (!user.password) {
if (!user.password?.hash) {
return res.status(400).json({ error: ErrorCode.UserMissingPassword });
}

Expand All @@ -48,7 +48,7 @@ export default async function handler(req: NextApiRequest, res: NextApiResponse)
return res.status(500).json({ error: ErrorCode.InternalServerError });
}

const isCorrectPassword = await verifyPassword(req.body.password, user.password);
const isCorrectPassword = await verifyPassword(req.body.password, user.password.hash);
if (!isCorrectPassword) {
return res.status(400).json({ error: ErrorCode.IncorrectPassword });
}
Expand Down
3 changes: 2 additions & 1 deletion apps/web/playwright/auth/forgot-password.e2e.ts
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,8 @@ testBothFutureAndLegacyRoutes.describe("Forgot password", async () => {
});

// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
expect(await verifyPassword(newPassword, updatedUser.password!)).toBeTruthy();
const updatedPassword = updatedUser.password!.hash;
expect(await verifyPassword(newPassword, updatedPassword)).toBeTruthy();

// finally, make sure the same URL cannot be used to reset the password again, as it should be expired.
await page.goto(`/auth/forgot-password/${id}`);
Expand Down
12 changes: 8 additions & 4 deletions apps/web/playwright/fixtures/users.ts
Original file line number Diff line number Diff line change
Expand Up @@ -652,7 +652,6 @@ type SupportedTestWorkflows = PrismaType.WorkflowCreateInput;

type CustomUserOptsKeys =
| "username"
| "password"
| "completedOnboarding"
| "locale"
| "name"
Expand All @@ -669,6 +668,7 @@ type CustomUserOpts = Partial<Pick<Prisma.User, CustomUserOptsKeys>> & {
useExactUsername?: boolean;
roleInOrganization?: MembershipRole;
schedule?: Schedule;
password?: string | null;
};

// creates the actual user in the db.
Expand All @@ -690,7 +690,11 @@ const createUser = (
username: uname,
name: opts?.name,
email: opts?.email ?? `${uname}@example.com`,
password: hashPassword(uname),
password: {
create: {
hash: hashPassword(uname),
},
},
emailVerified: new Date(),
completedOnboarding: opts?.completedOnboarding ?? true,
timeZone: opts?.timeZone ?? TimeZoneEnum.UK,
Expand Down Expand Up @@ -791,7 +795,7 @@ async function confirmPendingPayment(page: Page) {

// login using a replay of an E2E routine.
export async function login(
user: Pick<Prisma.User, "username"> & Partial<Pick<Prisma.User, "password" | "email">>,
user: Pick<Prisma.User, "username"> & Partial<Pick<Prisma.User, "email">> & { password?: string | null },
page: Page
) {
// get locators
Expand All @@ -812,7 +816,7 @@ export async function login(
}

export async function apiLogin(
user: Pick<Prisma.User, "username"> & Partial<Pick<Prisma.User, "password" | "email">>,
user: Pick<Prisma.User, "username"> & Partial<Pick<Prisma.User, "email">> & { password: string | null },
page: Page
) {
const csrfToken = await page
Expand Down
35 changes: 20 additions & 15 deletions packages/features/auth/lib/next-auth-options.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,8 @@ const ORGANIZATIONS_AUTOLINK =

const usernameSlug = (username: string) => `${slugify(username)}-${randomString(6).toLowerCase()}`;

const loginWithTotp = async (user: { email: string }) =>
`/auth/login?totp=${await (await import("./signJwt")).default({ email: user.email })}`;
const loginWithTotp = async (email: string) =>
`/auth/login?totp=${await (await import("./signJwt")).default({ email })}`;

type UserTeams = {
teams: (Membership & {
Expand Down Expand Up @@ -126,18 +126,18 @@ const providers: Provider[] = [
if (user.identityProvider !== IdentityProvider.CAL && !credentials.totpCode) {
throw new Error(ErrorCode.ThirdPartyIdentityProviderEnabled);
}
if (!user.password && user.identityProvider == IdentityProvider.CAL) {
if (!user.password?.hash && user.identityProvider == IdentityProvider.CAL) {
throw new Error(ErrorCode.IncorrectEmailPassword);
}
if (!user.password && user.identityProvider !== IdentityProvider.CAL && !credentials.totpCode) {
if (!user.password?.hash && user.identityProvider !== IdentityProvider.CAL && !credentials.totpCode) {
throw new Error(ErrorCode.IncorrectEmailPassword);
}

if (user.password && !credentials.totpCode) {
if (!user.password) {
if (user.password?.hash && !credentials.totpCode) {
if (!user.password?.hash) {
throw new Error(ErrorCode.IncorrectEmailPassword);
}
const isCorrectPassword = await verifyPassword(credentials.password, user.password);
const isCorrectPassword = await verifyPassword(credentials.password, user.password.hash);
if (!isCorrectPassword) {
throw new Error(ErrorCode.IncorrectEmailPassword);
}
Expand Down Expand Up @@ -717,7 +717,7 @@ export const AUTH_OPTIONS: AuthOptions = {
}
}
if (existingUser.twoFactorEnabled && existingUser.identityProvider === idP) {
return loginWithTotp(existingUser);
return loginWithTotp(existingUser.email);
} else {
return true;
}
Expand All @@ -733,7 +733,7 @@ export const AUTH_OPTIONS: AuthOptions = {
if (!userWithNewEmail) {
await prisma.user.update({ where: { id: existingUser.id }, data: { email: user.email } });
if (existingUser.twoFactorEnabled) {
return loginWithTotp(existingUser);
return loginWithTotp(existingUser.email);
} else {
return true;
}
Expand All @@ -752,6 +752,9 @@ export const AUTH_OPTIONS: AuthOptions = {
mode: "insensitive",
},
},
include: {
password: true,
},
});

if (existingUserWithEmail) {
Expand All @@ -762,15 +765,15 @@ export const AUTH_OPTIONS: AuthOptions = {
existingUserWithEmail.identityProvider !== IdentityProvider.CAL
) {
if (existingUserWithEmail.twoFactorEnabled) {
return loginWithTotp(existingUserWithEmail);
return loginWithTotp(existingUserWithEmail.email);
} else {
return true;
}
}

// check if user was invited
if (
!existingUserWithEmail.password &&
!existingUserWithEmail.password?.hash &&
!existingUserWithEmail.emailVerified &&
!existingUserWithEmail.username
) {
Expand All @@ -792,7 +795,7 @@ export const AUTH_OPTIONS: AuthOptions = {
});

if (existingUserWithEmail.twoFactorEnabled) {
return loginWithTotp(existingUserWithEmail);
return loginWithTotp(existingUserWithEmail.email);
} else {
return true;
}
Expand All @@ -807,14 +810,16 @@ export const AUTH_OPTIONS: AuthOptions = {
where: { email: existingUserWithEmail.email },
// also update email to the IdP email
data: {
password: null,
password: {
delete: true,
},
email: user.email,
identityProvider: idP,
identityProviderId: account.providerAccountId,
},
});
if (existingUserWithEmail.twoFactorEnabled) {
return loginWithTotp(existingUserWithEmail);
return loginWithTotp(existingUserWithEmail.email);
} else {
return true;
}
Expand Down Expand Up @@ -853,7 +858,7 @@ export const AUTH_OPTIONS: AuthOptions = {
await calcomAdapter.linkAccount(linkAccountNewUserData);

if (account.twoFactorEnabled) {
return loginWithTotp(newUser);
return loginWithTotp(newUser.email);
} else {
return true;
}
Expand Down
12 changes: 8 additions & 4 deletions packages/features/auth/signup/handlers/calcomHandler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -138,18 +138,22 @@ async function handler(req: RequestWithUsernameStatus, res: NextApiResponse) {
where: { email },
update: {
username,
password: hashedPassword,
emailVerified: new Date(Date.now()),
identityProvider: IdentityProvider.CAL,
password: {
upsert: {
create: { hash: hashedPassword },
update: { hash: hashedPassword },
},
},
},
create: {
username,
email,
password: hashedPassword,
identityProvider: IdentityProvider.CAL,
password: { create: { hash: hashedPassword } },
},
});

// Wrapping in a transaction as if one fails we want to rollback the whole thing to preventa any data inconsistencies
const { membership } = await createOrUpdateMemberships({
teamMetadata,
Expand Down Expand Up @@ -180,7 +184,7 @@ async function handler(req: RequestWithUsernameStatus, res: NextApiResponse) {
data: {
username,
email,
password: hashedPassword,
password: { create: { hash: hashedPassword } },
metadata: {
stripeCustomerId: customer.id,
checkoutSessionId,
Expand Down
18 changes: 14 additions & 4 deletions packages/features/auth/signup/handlers/selfHostedHandler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -79,14 +79,19 @@ export default async function handler(req: NextApiRequest, res: NextApiResponse)
where: { email: userEmail },
update: {
username: correctedUsername,
password: hashedPassword,
password: {
upsert: {
create: { hash: hashedPassword },
update: { hash: hashedPassword },
},
},
emailVerified: new Date(Date.now()),
identityProvider: IdentityProvider.CAL,
},
create: {
username: correctedUsername,
email: userEmail,
password: hashedPassword,
password: { create: { hash: hashedPassword } },
identityProvider: IdentityProvider.CAL,
},
});
Expand Down Expand Up @@ -128,14 +133,19 @@ export default async function handler(req: NextApiRequest, res: NextApiResponse)
where: { email: userEmail },
update: {
username: correctedUsername,
password: hashedPassword,
password: {
upsert: {
create: { hash: hashedPassword },
update: { hash: hashedPassword },
},
},
emailVerified: new Date(Date.now()),
identityProvider: IdentityProvider.CAL,
},
create: {
username: correctedUsername,
email: userEmail,
password: hashedPassword,
password: { create: { hash: hashedPassword } },
identityProvider: IdentityProvider.CAL,
},
});
Expand Down
7 changes: 2 additions & 5 deletions packages/features/ee/users/server/trpc-router.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,9 +67,7 @@ const authedAdminProcedureWithRequestedUser = authedAdminProcedure.use(async ({
return next({
ctx: {
user: ctx.user,
requestedUser:
/** Don't leak the password */
exclude(user, ["password"]),
requestedUser: user,
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need to exclude anymore

},
});
});
Expand All @@ -84,8 +82,7 @@ export const userAdminRouter = router({
// TODO: Add search, pagination, etc.
const users = await prisma.user.findMany();
return users.map((user) => ({
/** Don't leak the password */
...exclude(user, ["password"]),
...user,
/**
* FIXME: This should be either a prisma extension or middleware
* @see https://www.prisma.io/docs/concepts/components/prisma-client/middleware
Expand Down
1 change: 0 additions & 1 deletion packages/lib/test/builder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,6 @@ export const buildUser = <T extends Partial<UserPayload>>(
invitedTo: null,
locale: "en",
metadata: null,
password: null,
role: "USER",
schedules: [],
selectedCalendars: [],
Expand Down
4 changes: 1 addition & 3 deletions packages/lib/validateUsername.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,9 +56,7 @@ export const validateAndGetCorrectedUsernameAndEmail = async ({
{
OR: [
{ emailVerified: { not: null } },
{
AND: [{ password: { not: null } }, { username: { not: null } }],
},
{ AND: [{ password: { isNot: null } }, { username: { not: null } }] },
],
},
],
Expand Down
Loading
Loading