From 3aa6e0536c991685a876d50402472210a7cc05cc Mon Sep 17 00:00:00 2001 From: Austin Turner Date: Thu, 26 Dec 2024 18:52:03 -0700 Subject: [PATCH] Loosen remember device restrictions Remove IP address from remember device check, since that often changes for people regularly and causes this to go through the process again. Lengthen the cookie time and DB time to 90 days and upon successful verification, extend by another 90 days in DB and cookie #1076 --- .../src/app/controllers/auth.controller.ts | 316 +++++++++--------- apps/api/src/app/routes/route.middleware.ts | 3 - .../components/auth/VerifyEmailOr2fa.tsx | 2 +- libs/auth/server/src/lib/auth.db.service.ts | 20 +- libs/auth/server/src/lib/auth.utils.ts | 5 +- 5 files changed, 178 insertions(+), 168 deletions(-) diff --git a/apps/api/src/app/controllers/auth.controller.ts b/apps/api/src/app/controllers/auth.controller.ts index 130e119a..63877774 100644 --- a/apps/api/src/app/controllers/auth.controller.ts +++ b/apps/api/src/app/controllers/auth.controller.ts @@ -320,182 +320,186 @@ const signin = createRoute(routeDefinition.signin.validators, async ({ body, par /** * FIXME: This should probably be broken up and logic moved to the auth service */ -const callback = createRoute(routeDefinition.callback.validators, async ({ body, params, query, clearCookie }, req, res, next) => { - let provider: Provider | null = null; - try { - const providers = listProviders(); +const callback = createRoute( + routeDefinition.callback.validators, + async ({ body, params, query, setCookie, clearCookie }, req, res, next) => { + let provider: Provider | null = null; + try { + const providers = listProviders(); - provider = providers[params.provider]; - if (!provider) { - throw new InvalidParameters('Missing provider'); - } + provider = providers[params.provider]; + if (!provider) { + throw new InvalidParameters('Missing provider'); + } + + let isNewUser = false; + const { + pkceCodeVerifier, + nonce, + linkIdentity: linkIdentityCookie, + returnUrl, + rememberDevice, + } = getCookieConfig(ENV.USE_SECURE_COOKIES); + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + const cookies = parseCookie(req.headers.cookie!); + clearOauthCookies(res); - let isNewUser = false; - const { - pkceCodeVerifier, - nonce, - linkIdentity: linkIdentityCookie, - returnUrl, - rememberDevice, - } = getCookieConfig(ENV.USE_SECURE_COOKIES); - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - const cookies = parseCookie(req.headers.cookie!); - clearOauthCookies(res); + if (provider.type === 'oauth') { + // oauth flow + const { userInfo } = await validateCallback( + provider.provider as OauthProviderType, + new URLSearchParams(query), + cookies[pkceCodeVerifier.name], + cookies[nonce.name] + ); + + if (!userInfo.email) { + throw new InvalidParameters('Missing email from OAuth provider'); + } - if (provider.type === 'oauth') { - // oauth flow - const { userInfo } = await validateCallback( - provider.provider as OauthProviderType, - new URLSearchParams(query), - cookies[pkceCodeVerifier.name], - cookies[nonce.name] - ); - - if (!userInfo.email) { - throw new InvalidParameters('Missing email from OAuth provider'); - } + const providerUser = { + id: userInfo.sub, + email: userInfo.email, + emailVerified: userInfo.email_verified ?? false, + givenName: userInfo.given_name, + familyName: userInfo.family_name, + username: userInfo.preferred_username || (userInfo.username as string | undefined) || userInfo.email, + name: + userInfo.name ?? + (userInfo.given_name && userInfo.family_name ? `${userInfo.given_name} ${userInfo.family_name}` : userInfo.email), + picture: (userInfo.picture_thumbnail as string | undefined) || userInfo.picture, + }; + + // If user has an active session and user is linking an identity to an existing account + // link and redirect to profile page + if (req.session.user && cookies[linkIdentityCookie.name] === 'true') { + await linkIdentityToUser({ + userId: req.session.user.id, + provider: provider.provider, + providerUser, + }); + createUserActivityFromReq(req, res, { + action: 'LINK_IDENTITY', + method: provider.provider.toUpperCase(), + success: true, + }); + redirect(res, cookies[returnUrl.name] || `${ENV.JETSTREAM_CLIENT_URL}/profile`); + return; + } - const providerUser = { - id: userInfo.sub, - email: userInfo.email, - emailVerified: userInfo.email_verified ?? false, - givenName: userInfo.given_name, - familyName: userInfo.family_name, - username: userInfo.preferred_username || (userInfo.username as string | undefined) || userInfo.email, - name: - userInfo.name ?? - (userInfo.given_name && userInfo.family_name ? `${userInfo.given_name} ${userInfo.family_name}` : userInfo.email), - picture: (userInfo.picture_thumbnail as string | undefined) || userInfo.picture, - }; - - // If user has an active session and user is linking an identity to an existing account - // link and redirect to profile page - if (req.session.user && cookies[linkIdentityCookie.name] === 'true') { - await linkIdentityToUser({ - userId: req.session.user.id, + const sessionData = await handleSignInOrRegistration({ + providerType: provider.type, provider: provider.provider, providerUser, }); - createUserActivityFromReq(req, res, { - action: 'LINK_IDENTITY', - method: provider.provider.toUpperCase(), - success: true, - }); - redirect(res, cookies[returnUrl.name] || `${ENV.JETSTREAM_CLIENT_URL}/profile`); - return; - } - - const sessionData = await handleSignInOrRegistration({ - providerType: provider.type, - provider: provider.provider, - providerUser, - }); - isNewUser = sessionData.isNewUser; + isNewUser = sessionData.isNewUser; - initSession(req, sessionData); - } else if (provider.type === 'credentials' && req.method === 'POST') { - if (!body || !('action' in body)) { - throw new InvalidAction('Missing action in body'); + initSession(req, sessionData); + } else if (provider.type === 'credentials' && req.method === 'POST') { + if (!body || !('action' in body)) { + throw new InvalidAction('Missing action in body'); + } + const { action, csrfToken, email, password } = body; + await verifyCSRFFromRequestOrThrow(csrfToken, req.headers.cookie || ''); + + const sessionData = + action === 'login' + ? await handleSignInOrRegistration({ + providerType: 'credentials', + action, + email, + password, + }) + : await handleSignInOrRegistration({ + providerType: 'credentials', + action, + email, + name: body.name, + password, + }); + + isNewUser = sessionData.isNewUser; + + initSession(req, sessionData); + } else { + throw new InvalidProvider(`Provider type ${provider.type} is not supported. Method=${req.method}`); } - const { action, csrfToken, email, password } = body; - await verifyCSRFFromRequestOrThrow(csrfToken, req.headers.cookie || ''); - - const sessionData = - action === 'login' - ? await handleSignInOrRegistration({ - providerType: 'credentials', - action, - email, - password, - }) - : await handleSignInOrRegistration({ - providerType: 'credentials', - action, - email, - name: body.name, - password, - }); - - isNewUser = sessionData.isNewUser; - - initSession(req, sessionData); - } else { - throw new InvalidProvider(`Provider type ${provider.type} is not supported. Method=${req.method}`); - } - if (!req.session.user) { - throw new AuthError('Session not initialized'); - } + if (!req.session.user) { + throw new AuthError('Session not initialized'); + } - // check for remembered device - emailVerification cannot be bypassed - if ( - cookies[rememberDevice.name] && - Array.isArray(req.session.pendingVerification) && - req.session.pendingVerification.length > 0 && - req.session.pendingVerification.find((item) => item.type !== 'email') - ) { - const deviceId = cookies[rememberDevice.name]; - const isDeviceRemembered = await hasRememberDeviceRecord({ - userId: req.session.user.id, - deviceId, - ipAddress: res.locals.ipAddress || getApiAddressFromReq(req), - userAgent: req.get('User-Agent'), - }); - if (isDeviceRemembered) { - req.session.pendingVerification = null; - } else { - // deviceId is not valid, remove cookie - clearCookie(rememberDevice.name, rememberDevice.options); + // check for remembered device - emailVerification cannot be bypassed + if ( + cookies[rememberDevice.name] && + Array.isArray(req.session.pendingVerification) && + req.session.pendingVerification.length > 0 && + req.session.pendingVerification.find((item) => item.type !== 'email') + ) { + const deviceId = cookies[rememberDevice.name]; + const isDeviceRemembered = await hasRememberDeviceRecord({ + userId: req.session.user.id, + deviceId, + userAgent: req.get('User-Agent'), + }); + if (isDeviceRemembered) { + req.session.pendingVerification = null; + // refresh cookie expiration + setCookie(rememberDevice.name, deviceId, rememberDevice.options); + } else { + // deviceId is not valid, remove cookie + clearCookie(rememberDevice.name, rememberDevice.options); + } } - } - if (Array.isArray(req.session.pendingVerification) && req.session.pendingVerification.length > 0) { - const initialVerification = req.session.pendingVerification[0]; + if (Array.isArray(req.session.pendingVerification) && req.session.pendingVerification.length > 0) { + const initialVerification = req.session.pendingVerification[0]; - if (initialVerification.type === 'email') { - await sendEmailVerification(req.session.user.email, initialVerification.token, EMAIL_VERIFICATION_TOKEN_DURATION_HOURS); - } else if (initialVerification.type === '2fa-email') { - await sendVerificationCode(req.session.user.email, initialVerification.token, TOKEN_DURATION_MINUTES); - } + if (initialVerification.type === 'email') { + await sendEmailVerification(req.session.user.email, initialVerification.token, EMAIL_VERIFICATION_TOKEN_DURATION_HOURS); + } else if (initialVerification.type === '2fa-email') { + await sendVerificationCode(req.session.user.email, initialVerification.token, TOKEN_DURATION_MINUTES); + } - await setCsrfCookie(res); + await setCsrfCookie(res); - if (provider.type === 'oauth') { - redirect(res, `/auth/verify`); - } else { - sendJson(res, { error: false, redirect: `/auth/verify` }); - } - } else { - if (isNewUser) { - await sendWelcomeEmail(req.session.user.email); - } - // No verification required - if (provider.type === 'oauth') { - redirect(res, ENV.JETSTREAM_CLIENT_URL); + if (provider.type === 'oauth') { + redirect(res, `/auth/verify`); + } else { + sendJson(res, { error: false, redirect: `/auth/verify` }); + } } else { - // this was an API call, client will handle redirect - sendJson(res, { - error: false, - redirect: ENV.JETSTREAM_CLIENT_URL, - }); + if (isNewUser) { + await sendWelcomeEmail(req.session.user.email); + } + // No verification required + if (provider.type === 'oauth') { + redirect(res, ENV.JETSTREAM_CLIENT_URL); + } else { + // this was an API call, client will handle redirect + sendJson(res, { + error: false, + redirect: ENV.JETSTREAM_CLIENT_URL, + }); + } } - } - createUserActivityFromReq(req, res, { - action: 'LOGIN', - method: provider.provider.toUpperCase(), - success: true, - }); - } catch (ex) { - createUserActivityFromReqWithError(req, res, ex, { - action: 'LOGIN', - email: body && 'email' in body ? body.email : undefined, - method: provider?.provider?.toUpperCase(), - success: false, - }); - next(ensureAuthError(ex)); + createUserActivityFromReq(req, res, { + action: 'LOGIN', + method: provider.provider.toUpperCase(), + success: true, + }); + } catch (ex) { + createUserActivityFromReqWithError(req, res, ex, { + action: 'LOGIN', + email: body && 'email' in body ? body.email : undefined, + method: provider?.provider?.toUpperCase(), + success: false, + }); + next(ensureAuthError(ex)); + } } -}); +); const verification = createRoute(routeDefinition.verification.validators, async ({ body, user, setCookie }, req, res, next) => { try { diff --git a/apps/api/src/app/routes/route.middleware.ts b/apps/api/src/app/routes/route.middleware.ts index 98e9954c..438f2692 100644 --- a/apps/api/src/app/routes/route.middleware.ts +++ b/apps/api/src/app/routes/route.middleware.ts @@ -138,9 +138,6 @@ export async function checkAuth(req: express.Request, res: express.Response, nex } } - // TODO: consider adding a check for IP address - but should allow some buffer in case people change networks - // especially if the ip addresses are very far away - if (user && !pendingVerification) { telemetryAddUserToAttributes(user); return next(); diff --git a/apps/landing/components/auth/VerifyEmailOr2fa.tsx b/apps/landing/components/auth/VerifyEmailOr2fa.tsx index 3a31c95b..01d7fe2d 100644 --- a/apps/landing/components/auth/VerifyEmailOr2fa.tsx +++ b/apps/landing/components/auth/VerifyEmailOr2fa.tsx @@ -17,7 +17,7 @@ const FormSchema = z.object({ csrfToken: z.string(), code: z.string().min(6).max(6), type: z.enum(['email', '2fa-otp', '2fa-email']), - rememberDevice: z.boolean().optional().default(false), + rememberDevice: z.boolean().optional().default(true), }); function getTitleText(authFactor: TwoFactorType, email?: Maybe) { diff --git a/libs/auth/server/src/lib/auth.db.service.ts b/libs/auth/server/src/lib/auth.db.service.ts index 2a6efe36..b95eacaa 100644 --- a/libs/auth/server/src/lib/auth.db.service.ts +++ b/libs/auth/server/src/lib/auth.db.service.ts @@ -29,6 +29,8 @@ import { import { ensureAuthError, verifyAuth0CredentialsOrThrow_MIGRATION_TEMPORARY } from './auth.service'; import { hashPassword, verifyPassword } from './auth.utils'; +const REMEMBER_DEVICE_DAYS = 60; + const userSelect = Prisma.validator()({ id: true, userId: true, @@ -121,33 +123,39 @@ export async function createRememberDevice({ deviceId, ipAddress, userAgent, - expiresAt: addDays(new Date(), 30), + expiresAt: addDays(new Date(), REMEMBER_DEVICE_DAYS), }, }); } export async function hasRememberDeviceRecord({ deviceId, - ipAddress, userId, userAgent = null, }: { userId: string; deviceId: string; - ipAddress: string; userAgent?: Maybe; }) { try { - const matchingRecords = await prisma.rememberedDevice.count({ + const matchingRecords = await prisma.rememberedDevice.findFirst({ + select: { id: true }, where: { userId, deviceId, - ipAddress, userAgent, expiresAt: { gte: new Date() }, }, }); - return matchingRecords > 0; + // update expiration date if record exists + if (matchingRecords) { + await prisma.rememberedDevice.update({ + select: { id: true }, + data: { expiresAt: addDays(new Date(), REMEMBER_DEVICE_DAYS) }, + where: { id: matchingRecords.id }, + }); + } + return !!matchingRecords; } catch (ex) { logger.error({ ...getErrorMessageAndStackObj(ex) }, 'Error checking for remember device record'); return false; diff --git a/libs/auth/server/src/lib/auth.utils.ts b/libs/auth/server/src/lib/auth.utils.ts index fb44d3ea..f8a7c682 100644 --- a/libs/auth/server/src/lib/auth.utils.ts +++ b/libs/auth/server/src/lib/auth.utils.ts @@ -4,7 +4,7 @@ import * as bcrypt from 'bcrypt'; import * as Bowser from 'bowser'; const TIME_15_MIN = 60 * 15; -const TIME_30_DAYS = 30 * 24 * 60 * 60; +const TIME_90_DAYS = 90 * 24 * 60 * 60; export function getCookieConfig(useSecureCookies: boolean): CookieConfig { const cookiePrefix = useSecureCookies ? '__Secure-' : ''; @@ -95,7 +95,8 @@ export function getCookieConfig(useSecureCookies: boolean): CookieConfig { sameSite: 'lax', path: '/', secure: useSecureCookies, - maxAge: TIME_30_DAYS, + // The time in the database is 60 days and refreshes every time the user logs in + maxAge: TIME_90_DAYS, }, }, } as const;