Skip to content

Commit

Permalink
Ensure captcha resets on login attempt
Browse files Browse the repository at this point in the history
The Captcha reset was not working properly after any failed login/reset attempt - there was an issue with the ref being passed to useImperativeHandle

Added custom messages to auth errors so the authentication login history has more context on where the errors are coming from

resolves #1101
  • Loading branch information
paustint committed Dec 8, 2024
1 parent 045d620 commit 8ec5416
Show file tree
Hide file tree
Showing 5 changed files with 58 additions and 48 deletions.
40 changes: 20 additions & 20 deletions apps/api/src/app/controllers/auth.controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -280,7 +280,7 @@ const signin = createRoute(routeDefinition.signin.validators, async ({ body, par

if (isAccountLink) {
if (!req.session.user) {
throw new InvalidSession();
throw new InvalidSession('Cannot link account without an active session');
}
setCookie(cookieConfig.linkIdentity.name, 'true', cookieConfig.linkIdentity.options);
}
Expand All @@ -305,7 +305,7 @@ const signin = createRoute(routeDefinition.signin.validators, async ({ body, par
return;
}

throw new InvalidAction();
throw new InvalidAction(`Can only sign in with OAuth providers. isAccountLink=${isAccountLink}, provider:${JSON.stringify(provider)}`);
} catch (ex) {
createUserActivityFromReqWithError(req, res, ex, {
action: query?.isAccountLink ? 'LINK_IDENTITY_INIT' : 'OAUTH_INIT',
Expand All @@ -327,7 +327,7 @@ const callback = createRoute(routeDefinition.callback.validators, async ({ body,

provider = providers[params.provider];
if (!provider) {
throw new InvalidParameters();
throw new InvalidParameters('Missing provider');
}

let isNewUser = false;
Expand All @@ -352,7 +352,7 @@ const callback = createRoute(routeDefinition.callback.validators, async ({ body,
);

if (!userInfo.email) {
throw new InvalidParameters();
throw new InvalidParameters('Missing email from OAuth provider');
}

const providerUser = {
Expand Down Expand Up @@ -395,7 +395,7 @@ const callback = createRoute(routeDefinition.callback.validators, async ({ body,
initSession(req, sessionData);
} else if (provider.type === 'credentials' && req.method === 'POST') {
if (!body || !('action' in body)) {
throw new InvalidAction();
throw new InvalidAction('Missing action in body');
}
const { action, csrfToken, email, password } = body;
await verifyCSRFFromRequestOrThrow(csrfToken, req.headers.cookie || '');
Expand All @@ -420,11 +420,11 @@ const callback = createRoute(routeDefinition.callback.validators, async ({ body,

initSession(req, sessionData);
} else {
throw new InvalidProvider();
throw new InvalidProvider(`Provider type ${provider.type} is not supported. Method=${req.method}`);
}

if (!req.session.user) {
throw new AuthError();
throw new AuthError('Session not initialized');
}

// check for remembered device - emailVerification cannot be bypassed
Expand Down Expand Up @@ -500,7 +500,7 @@ const callback = createRoute(routeDefinition.callback.validators, async ({ body,
const verification = createRoute(routeDefinition.verification.validators, async ({ body, user, setCookie }, req, res, next) => {
try {
if (!req.session.user || !req.session.pendingVerification) {
throw new InvalidSession();
throw new InvalidSession('Missing user or pending verification');
}

const { csrfToken, code, type, rememberDevice } = body;
Expand All @@ -510,28 +510,28 @@ const verification = createRoute(routeDefinition.verification.validators, async
const cookieConfig = getCookieConfig(ENV.USE_SECURE_COOKIES);

if (!pendingVerification) {
throw new InvalidSession();
throw new InvalidSession('Missing pending verification');
}

await verifyCSRFFromRequestOrThrow(csrfToken, req.headers.cookie || '');

if (pendingVerification.exp <= new Date().getTime()) {
throw new ExpiredVerificationToken();
throw new ExpiredVerificationToken(`Pending verification is expired: ${pendingVerification.exp}`);
}

switch (pendingVerification.type) {
case 'email': {
const { token } = pendingVerification;
if (token !== code) {
throw new InvalidVerificationToken();
throw new InvalidVerificationToken('Provided code does not match');
}
req.session.user = (await setUserEmailVerified(req.session.user.id)) as UserProfileSession;
break;
}
case '2fa-email': {
const { token } = pendingVerification;
if (token !== code) {
throw new InvalidVerificationToken();
throw new InvalidVerificationToken('Provided code does not match');
}
rememberDeviceId = rememberDevice ? generateRandomString(32) : undefined;
break;
Expand All @@ -544,7 +544,7 @@ const verification = createRoute(routeDefinition.verification.validators, async
break;
}
default: {
throw new InvalidVerificationToken();
throw new InvalidVerificationToken(`Invalid verification type`);
}
}

Expand Down Expand Up @@ -589,14 +589,14 @@ const verification = createRoute(routeDefinition.verification.validators, async
const resendVerification = createRoute(routeDefinition.resendVerification.validators, async ({ body }, req, res, next) => {
try {
if (!req.session.user || !req.session.pendingVerification) {
throw new InvalidSession();
throw new InvalidSession('Missing user or pending verification');
}

const { csrfToken, type } = body;
const pendingVerification = req.session.pendingVerification.find((item) => item.type === type);

if (!pendingVerification) {
throw new InvalidSession();
throw new InvalidSession('Missing pending verification');
}

await verifyCSRFFromRequestOrThrow(csrfToken, req.headers.cookie || '');
Expand Down Expand Up @@ -726,7 +726,7 @@ const validatePasswordReset = createRoute(routeDefinition.validatePasswordReset.
const verifyEmailViaLink = createRoute(routeDefinition.verification.validators, async ({ query }, req, res, next) => {
try {
if (!req.session.user) {
throw new InvalidSession();
throw new InvalidSession('User not set on session');
}

if (!req.session.pendingVerification?.length) {
Expand All @@ -744,24 +744,24 @@ const verifyEmailViaLink = createRoute(routeDefinition.verification.validators,
});

if (!pendingVerification) {
throw new InvalidSession();
throw new InvalidSession('Missing pending verification');
}

if (pendingVerification.exp <= new Date().getTime()) {
throw new ExpiredVerificationToken();
throw new ExpiredVerificationToken(`Pending verification is expired: ${pendingVerification.exp}`);
}

switch (pendingVerification.type) {
case 'email': {
const { token } = pendingVerification;
if (token !== code) {
throw new InvalidVerificationToken();
throw new InvalidVerificationToken('Provided code does not match');
}
req.session.user = (await setUserEmailVerified(req.session.user.id)) as UserProfileSession;
break;
}
default: {
throw new InvalidVerificationToken();
throw new InvalidVerificationToken('Invalid verification type');
}
}

Expand Down
24 changes: 18 additions & 6 deletions apps/landing/components/auth/Captcha.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import { Maybe } from '@jetstream/types';
import { Turnstile, TurnstileInstance } from '@marsidev/react-turnstile';
import { forwardRef, useId, useImperativeHandle, useRef } from 'react';
import { ENVIRONMENT } from '../../utils/environment';
Expand All @@ -15,20 +14,29 @@ interface CaptchaProps {
* Called once captcha has been successfully completed
* Called immediately if captcha is disabled
*/
onFinished: () => void;
onStateChange: (isFinished: boolean) => void;
}

// eslint-disable-next-line react/display-name
export const Captcha = forwardRef<Maybe<TurnstileInstance>, CaptchaProps>(({ action, formError, onLoad, onChange, onFinished }, ref) => {
export const Captcha = forwardRef<{ reset: () => void }, CaptchaProps>(({ action, formError, onLoad, onChange, onStateChange }, ref) => {
const turnstileRef = useRef<TurnstileInstance>(null);
const id = useId();

useImperativeHandle<unknown, Maybe<TurnstileInstance>>(ref, () => turnstileRef.current, [turnstileRef]);
useImperativeHandle<unknown, { reset: () => void }>(
ref,
() => ({
reset: () => {
onStateChange(false);
turnstileRef.current?.reset();
},
}),
[onStateChange]
);

// Skip rendering the captcha if we're running in Playwright or if the key is not set
// In real environments the server will still validate and prevent access if there isn't a valid token
if (!ENVIRONMENT.CAPTCHA_KEY || (window as any)?.playwright) {
onFinished();
onStateChange(true);
return null;
}

Expand All @@ -47,9 +55,13 @@ export const Captcha = forwardRef<Maybe<TurnstileInstance>, CaptchaProps>(({ act
feedbackEnabled: true,
}}
onWidgetLoad={onLoad}
onError={(error) => {
console.error('Captcha error:', error);
onStateChange(false);
}}
onSuccess={(token) => {
onChange(token);
onFinished();
onStateChange(true);
}}
/>
{formError && (
Expand Down
5 changes: 2 additions & 3 deletions apps/landing/components/auth/LoginOrSignUp.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
/* eslint-disable @next/next/no-img-element */
import { zodResolver } from '@hookform/resolvers/zod';
import { Providers } from '@jetstream/auth/types';
import { TurnstileInstance } from '@marsidev/react-turnstile';
import Link from 'next/link';
import { useRouter } from 'next/router';
import { Fragment, useRef, useState } from 'react';
Expand Down Expand Up @@ -54,7 +53,7 @@ export function LoginOrSignUp({ action, providers, csrfToken }: LoginOrSignUpPro
const router = useRouter();
const [showPasswordActive, setShowPasswordActive] = useState(false);
const [finishedCaptcha, setFinishedCaptcha] = useState(false);
const captchaRef = useRef<TurnstileInstance>(null);
const captchaRef = useRef<{ reset: () => void }>(null);

const {
register,
Expand Down Expand Up @@ -269,7 +268,7 @@ export function LoginOrSignUp({ action, providers, csrfToken }: LoginOrSignUpPro
formError={errors?.captchaToken?.message}
action={action}
onChange={(token) => setValue('captchaToken', token)}
onFinished={() => setFinishedCaptcha(true)}
onStateChange={setFinishedCaptcha}
/>

<div>
Expand Down
5 changes: 2 additions & 3 deletions apps/landing/components/auth/PasswordResetInit.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
/* eslint-disable @next/next/no-img-element */
import { zodResolver } from '@hookform/resolvers/zod';
import { TurnstileInstance } from '@marsidev/react-turnstile';
import Link from 'next/link';
import { useRouter } from 'next/router';
import { Fragment, useRef, useState } from 'react';
Expand Down Expand Up @@ -30,7 +29,7 @@ export function PasswordResetInit({ csrfToken }: PasswordResetInitProps) {
const [isSubmitted, setIsSubmitted] = useState(false);
const [finishedCaptcha, setFinishedCaptcha] = useState(false);
const [error, setError] = useState<string>();
const captchaRef = useRef<TurnstileInstance>(null);
const captchaRef = useRef<{ reset: () => void }>(null);

const {
register,
Expand Down Expand Up @@ -138,7 +137,7 @@ export function PasswordResetInit({ csrfToken }: PasswordResetInitProps) {
formError={errors?.captchaToken?.message}
action="password-reset-init"
onChange={(token) => setValue('captchaToken', token)}
onFinished={() => setFinishedCaptcha(true)}
onStateChange={setFinishedCaptcha}
/>

<div>
Expand Down
32 changes: 16 additions & 16 deletions libs/auth/server/src/lib/auth.db.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -360,7 +360,7 @@ export async function setPasswordForUser(id: string, password: string) {
});

if (!UNSAFE_userWithPassword) {
return { error: new InvalidCredentials() };
return { error: new InvalidCredentials(`User not found with id ${id}`) };
}

// FIXME: this is kinda dumb since the user can remove the password then re-add it
Expand Down Expand Up @@ -394,11 +394,11 @@ export const generatePasswordResetToken = async (email: string) => {
});

if (!user.length) {
throw new InvalidAction();
throw new InvalidAction('User does not exist or there is no password set');
}

if (user.length > 1) {
throw new InvalidAction();
throw new InvalidAction('Multiple users with the same email address and a password set');
}

// if there is an existing token, delete it
Expand Down Expand Up @@ -431,7 +431,7 @@ export const resetUserPassword = async (email: string, token: string, password:
});

if (!restToken) {
throw new InvalidOrExpiredResetToken();
throw new InvalidOrExpiredResetToken('Missing reset token');
}

// delete token - we don't need it anymore and if we fail later, the user will need to reset again
Expand All @@ -440,7 +440,7 @@ export const resetUserPassword = async (email: string, token: string, password:
});

if (restToken.expiresAt < new Date()) {
throw new InvalidOrExpiredResetToken();
throw new InvalidOrExpiredResetToken(`Expired at ${restToken.expiresAt.toISOString()}`);
}

const hashedPassword = await hashPassword(password);
Expand Down Expand Up @@ -497,7 +497,7 @@ async function getUserAndVerifyPassword(email: string, password: string) {
user: updatedUser,
};
} catch (ex) {
return { error: new InvalidCredentials() };
return { error: new InvalidCredentials('Could not migrate from Auth0') };
}

// Use this after code above is removed
Expand All @@ -515,7 +515,7 @@ async function getUserAndVerifyPassword(email: string, password: string) {
}),
};
}
return { error: new InvalidCredentials() };
return { error: new InvalidCredentials('Incorrect email or password') };
}

async function migratePasswordFromAuth0(email: string, password: string) {
Expand All @@ -529,7 +529,7 @@ async function migratePasswordFromAuth0(email: string, password: string) {

if (!userWithoutSocialIdentities || userWithoutSocialIdentities.identities.length > 0) {
logger.warn({ email }, 'Cannot migrate password on the fly from Auth0, user has linked social identity');
throw new InvalidCredentials();
throw new InvalidCredentials('Could not migrate from Auth0');
}

await verifyAuth0CredentialsOrThrow_MIGRATION_TEMPORARY({ email, password });
Expand Down Expand Up @@ -843,33 +843,33 @@ export async function handleSignInOrRegistration(
const email = payload.email.toLowerCase();

if (!password) {
throw new InvalidCredentials();
throw new InvalidCredentials('Missing Password');
}

if (action === 'login') {
const userOrError = await getUserAndVerifyPassword(email, password);
if (userOrError.error) {
throw userOrError.error;
} else if (!userOrError.user) {
throw new InvalidCredentials();
throw new InvalidCredentials('User not found');
}
user = userOrError.user;
} else if (action === 'register') {
const usersWithEmail = await findUsersByEmail(email);
if (usersWithEmail.length > 0) {
throw new InvalidRegistration();
throw new InvalidRegistration('Email address is in use');
}
user = await createUserFromUserInfo(payload.email, payload.name, password);
isNewUser = true;
} else {
throw new InvalidAction();
throw new InvalidAction(action);
}
} else {
throw new InvalidProvider();
throw new InvalidProvider(providerType);
}

if (!user) {
throw new InvalidCredentials();
throw new InvalidCredentials('User not initialized');
}

await setLastLoginDate(user.id);
Expand All @@ -894,7 +894,7 @@ export async function handleSignInOrRegistration(
},
};
} catch (ex) {
throw ensureAuthError(ex, new InvalidCredentials());
throw ensureAuthError(ex, new InvalidCredentials('Unexpected error'));
}
}

Expand Down Expand Up @@ -924,7 +924,7 @@ export async function linkIdentityToUser({
const existingProviderUser = await findUserByProviderId(provider, providerUser.id);
if (existingProviderUser && existingProviderUser.id !== userId) {
// TODO: is this the correct error message? some other user already has this identity linked
throw new LoginWithExistingIdentity();
throw new LoginWithExistingIdentity('Provider identity already linked to another user');
} else if (existingProviderUser) {
// identity is already linked to this user - NO_OP
return existingUser;
Expand Down

0 comments on commit 8ec5416

Please sign in to comment.