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

Passwordless | Move sign in with password instead option #3058

Merged
merged 2 commits into from
Feb 11, 2025
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
43 changes: 39 additions & 4 deletions cypress/integration/ete/sign_in_passcode.8.cy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,42 @@ describe('Sign In flow, with passcode', () => {
});
});

it('selects password option to sign in', () => {
it('selects password option to sign in from initial sign in page', () => {
cy
.createTestUser({
isUserEmailValidated: true,
})
?.then(({ emailAddress, finalPassword }) => {
cy.visit(`/signin`);
cy.get('input[name=email]').type(emailAddress);

cy.contains('Sign in with a password instead').click();

// password page
cy.url().should('include', '/signin/password');
cy.get('input[name=email]').should('have.value', emailAddress);
cy.get('input[name=password]').type(finalPassword);
cy.get('[data-cy="main-form-submit-button"]').click();
cy.url().should('include', 'https://m.code.dev-theguardian.com/');
});
});

it('selects password option to sign in from the initial sign in page and show correct error page on incorrect password', () => {
const emailAddress = randomMailosaurEmail();
cy.visit(`/signin`);
cy.get('input[name=email]').type(emailAddress);
cy.contains('Sign in with a password instead').click();

// password page
cy.url().should('include', '/signin/password');
cy.get('input[name=email]').should('have.value', emailAddress);
cy.get('input[name=password]').type(randomPassword());
cy.get('[data-cy="main-form-submit-button"]').click();
cy.url().should('include', '/signin/password');
cy.contains('Email and password don’t match');
});

it('selects password option to sign in from passcode page', () => {
cy
.createTestUser({
isUserEmailValidated: true,
Expand All @@ -184,7 +219,7 @@ describe('Sign In flow, with passcode', () => {
// passcode page
cy.url().should('include', '/signin/code');
cy.contains('Enter your one-time code');
cy.contains('Sign in with password instead').click();
cy.contains('sign in with a password instead').click();

// password page
cy.url().should('include', '/signin/password');
Expand All @@ -195,15 +230,15 @@ describe('Sign In flow, with passcode', () => {
});
});

it('selects password option to sign in and show correct error page on incorrect password', () => {
it('selects password option to sign in from passcode page and show correct error page on incorrect password', () => {
const emailAddress = randomMailosaurEmail();
cy.visit(`/signin?usePasscodeSignIn=true`);
cy.get('input[name=email]').type(emailAddress);
cy.get('[data-cy="main-form-submit-button"]').click();
// passcode page
cy.url().should('include', '/signin/code');
cy.contains('Enter your one-time code');
cy.contains('Sign in with password instead').click();
cy.contains('sign in with a password instead').click();

// password page
cy.url().should('include', '/signin/password');
Expand Down
16 changes: 16 additions & 0 deletions src/client/components/EmailSentInformationBox.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,22 @@ export const WithNoAccountInfo = () => {
};
WithNoAccountInfo.storyName = 'with noAccountInfo';

export const WithShowSignInWithPasswordOption = () => {
return (
<EmailSentInformationBox
setRecaptchaErrorContext={() => {}}
setRecaptchaErrorMessage={() => {}}
changeEmailPage="#"
email="[email protected]"
resendEmailAction="#"
noAccountInfo
showSignInWithPasswordOption
/>
);
};
WithShowSignInWithPasswordOption.storyName =
'with showSignInWithPasswordOption';

export const WithTimer = () => {
return (
<EmailSentInformationBox
Expand Down
12 changes: 11 additions & 1 deletion src/client/components/EmailSentInformationBox.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ type EmailSentInformationBoxProps = Pick<
>;
setRecaptchaErrorMessage: React.Dispatch<React.SetStateAction<string>>;
sendAgainTimerInSeconds?: number;
showSignInWithPasswordOption?: boolean;
};

const sendAgainFormWrapperStyles = css`
Expand All @@ -50,6 +51,7 @@ export const EmailSentInformationBox = ({
queryString,
shortRequestId,
sendAgainTimerInSeconds,
showSignInWithPasswordOption,
}: EmailSentInformationBoxProps) => {
const timer = useCountdownTimer(sendAgainTimerInSeconds || 0);

Expand Down Expand Up @@ -89,12 +91,20 @@ export const EmailSentInformationBox = ({
)}
{changeEmailPage && (
<>
, or{' '}
,{!showSignInWithPasswordOption ? <> or </> : <> </>}
Copy link
Contributor

@pvighi pvighi Feb 10, 2025

Choose a reason for hiding this comment

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

sorry I'm having a bit of trouble understanding the intent (or maybe the syntax) here:

is the intention that if it has to show both links it would say
try another address or sign in with a password instead
but if it only has to one it would just say or try another address / or sign in with a password instead ?

Copy link
Member Author

Choose a reason for hiding this comment

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

the latter option!

Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Contributor

@pvighi pvighi Feb 10, 2025

Choose a reason for hiding this comment

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

is a comma before 'or' correct ?
It looks weird to me but sadly I don't have an english degree to know this kind of things for sure.

edit: I mean in cases like this:
image

Copy link
Member Author

Choose a reason for hiding this comment

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

In generally I've always used a comma before the or in the last item in a list, so it separated from the other items in the same way, like an array.

See Oxford comma in the Guardian style guide: https://www.theguardian.com/guardian-observer-style-guide-o#:~:text=what%20we%20use-,Oxford%20comma,-a%20comma%20before

<ThemedLink href={`${changeEmailPage}${queryString}`}>
try another address
</ThemedLink>
</>
)}
{showSignInWithPasswordOption && (
<>
, or{' '}
<ThemedLink href={`${buildUrl('/signin/password')}${queryString}`}>
sign in with a password instead
</ThemedLink>
</>
)}
.
</InformationBoxText>
{noAccountInfo && (
Expand Down
2 changes: 2 additions & 0 deletions src/client/components/PasscodeInput.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ export const PasscodeInput = ({
passcode = '',
fieldErrors,
formRef,
autoFocus,
}: PasscodeInputProps) => {
/**
* In gateway we normally avoid using client side javascript, but in this case
Expand Down Expand Up @@ -103,6 +104,7 @@ export const PasscodeInput = ({
value={input.value}
onChange={onChange}
onPaste={onPaste}
autoFocus={autoFocus}
/>
</div>
);
Expand Down
3 changes: 3 additions & 0 deletions src/client/components/PasswordInput.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ export type PasswordInputProps = {
supporting?: string;
onChange?: (e: React.ChangeEvent<HTMLInputElement>) => void;
autoComplete?: PasswordAutoComplete;
autoFocus?: boolean;
};

// hide the microsoft password reveal eye if we're using
Expand Down Expand Up @@ -171,6 +172,7 @@ export const PasswordInput = ({
onChange,
displayEye = true,
autoComplete,
autoFocus,
}: PasswordInputProps) => {
const [passwordVisible, setPasswordVisible] = useState(false);
const [fieldIsFocused, setFieldIsFocused] = useState(false);
Expand Down Expand Up @@ -203,6 +205,7 @@ export const PasswordInput = ({
hideMsReveal(displayEye),
]}
id="password"
autoFocus={autoFocus}
/>

{displayEye && (
Expand Down
19 changes: 1 addition & 18 deletions src/client/pages/EmailSent.tsx
Original file line number Diff line number Diff line change
@@ -1,9 +1,4 @@
import React, {
PropsWithChildren,
ReactNode,
useState,
useEffect,
} from 'react';
import React, { PropsWithChildren, ReactNode, useState } from 'react';
import { MainBodyText } from '@/client/components/MainBodyText';
import { MinimalLayout } from '@/client/layouts/MinimalLayout';
import { EmailSentInformationBox } from '@/client/components/EmailSentInformationBox';
Expand Down Expand Up @@ -44,18 +39,6 @@ export const EmailSent = ({
const [recaptchaErrorContext, setRecaptchaErrorContext] =
useState<ReactNode>(null);

// autofocus the code input field when the page loads
useEffect(() => {
if (typeof window !== 'undefined') {
const codeInput: HTMLInputElement | null =
window.document.querySelector('input[name="code"]');

if (codeInput) {
codeInput.focus();
}
}
}, []);

return (
<MinimalLayout
shortRequestId={shortRequestId}
Expand Down
23 changes: 2 additions & 21 deletions src/client/pages/PasscodeEmailSent.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,6 @@ import { MinimalLayout } from '@/client/layouts/MinimalLayout';
import { PasscodeInput } from '@/client/components/PasscodeInput';
import { EmailSentInformationBox } from '@/client/components/EmailSentInformationBox';
import { EmailSentProps } from '@/client/pages/EmailSent';
import { buildUrl } from '@/shared/lib/routeUtils';
import ThemedLink from '@/client/components/ThemedLink';

type TextType = 'verification' | 'security' | 'generic' | 'signin';

Expand Down Expand Up @@ -137,18 +135,6 @@ export const PasscodeEmailSent = ({
}
}, [timeUntilTokenExpiry, expiredPage, queryString]);

// autofocus the code input field when the page loads
useEffect(() => {
if (typeof window !== 'undefined') {
const codeInput: HTMLInputElement | null =
window.document.querySelector('input[name="code"]');

if (codeInput) {
codeInput.focus();
}
}
}, []);

return (
<MinimalLayout
shortRequestId={shortRequestId}
Expand Down Expand Up @@ -181,15 +167,9 @@ export const PasscodeEmailSent = ({
fieldErrors={fieldErrors}
label={text.passcodeInputLabel}
formRef={formRef}
autoFocus
/>
</MainForm>
{showSignInWithPasswordOption && (
<MainBodyText>
<ThemedLink href={`${buildUrl('/signin/password')}${queryString}`}>
Sign in with password instead
</ThemedLink>
</MainBodyText>
)}
<EmailSentInformationBox
setRecaptchaErrorContext={setRecaptchaErrorContext}
setRecaptchaErrorMessage={setRecaptchaErrorMessage}
Expand All @@ -203,6 +183,7 @@ export const PasscodeEmailSent = ({
shortRequestId={shortRequestId}
noAccountInfo={noAccountInfo}
sendAgainTimerInSeconds={sendAgainTimerInSeconds}
showSignInWithPasswordOption={showSignInWithPasswordOption}
/>
</MinimalLayout>
);
Expand Down
38 changes: 35 additions & 3 deletions src/client/pages/SignIn.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ export type SignInProps = {
// flag to determine whether to show the passcode view or password view
usePasscodeSignIn?: boolean;
hideSocialButtons?: boolean;
focusPasswordField?: boolean;
};

const resetPassword = css`
Expand Down Expand Up @@ -145,7 +146,10 @@ export const SignIn = ({
shortRequestId,
usePasscodeSignIn = false,
hideSocialButtons = false,
focusPasswordField = false,
}: SignInProps) => {
const [currentEmail, setCurrentEmail] = React.useState(email);

// status of the OTP checkbox
const selectedView = usePasscodeSignIn ? 'passcode' : 'password';

Expand Down Expand Up @@ -183,7 +187,7 @@ export const SignIn = ({
queryParams,
)}
submitButtonText={
selectedView === 'passcode' ? 'Continue with email' : 'Sign in'
selectedView === 'passcode' ? 'Sign in with email' : 'Sign in'
}
recaptchaSiteKey={recaptchaSiteKey}
formTrackingName={formTrackingName}
Expand All @@ -193,10 +197,17 @@ export const SignIn = ({
hasGuardianTerms={!isJobs && socialSigninBlocked}
hasJobsTerms={isJobs && socialSigninBlocked}
>
<EmailInput defaultValue={email} />
<EmailInput
defaultValue={email}
onChange={(e) => setCurrentEmail(e.target.value)}
/>
{selectedView === 'password' && (
<>
<PasswordInput label="Password" autoComplete="current-password" />
<PasswordInput
label="Password"
autoComplete="current-password"
autoFocus={!!(focusPasswordField && email)}
/>
<ThemedLink
href={buildUrlWithQueryParams('/reset-password', {}, queryParams)}
cssOverrides={resetPassword}
Expand All @@ -212,6 +223,27 @@ export const SignIn = ({
)
}
</MainForm>
{
// Hidden input to determine whether passcode view is selected
selectedView === 'passcode' && (
<>
<MainBodyText>
<ThemedLink
href={buildUrlWithQueryParams(
'/signin/password',
{},
queryParams,
{
signInEmail: currentEmail,
},
)}
>
Sign in with a password instead
</ThemedLink>
</MainBodyText>
</>
)
}
{!isReauthenticate && (
<>
<Divider size="full" cssOverrides={divider} />
Expand Down
5 changes: 3 additions & 2 deletions src/client/pages/SignInPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ export const SignInPage = ({
queryParams,
recaptchaConfig,
} = clientState;
const { email, formError } = pageData;
const { email, formError, focusPasswordField } = pageData;
const { error: pageError } = globalMessage;
const { recaptchaSiteKey } = recaptchaConfig;

Expand All @@ -31,7 +31,7 @@ export const SignInPage = ({
// determines if the passcode view of the sign in page should be shown
const usePasscodeSignIn: boolean = (() => {
// if the forcePasswordPage flag is set, we should always show the password view
// for example when the user clicks "sign in with password instead"
// for example when the user clicks "sign in with a password instead"
if (forcePasswordPage) {
return false;
}
Expand Down Expand Up @@ -62,6 +62,7 @@ export const SignInPage = ({
shortRequestId={clientState.shortRequestId}
usePasscodeSignIn={usePasscodeSignIn}
hideSocialButtons={hideSocialButtons}
focusPasswordField={focusPasswordField}
/>
);
};
2 changes: 2 additions & 0 deletions src/server/lib/queryParams.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ export const parseExpressQueryParams = (
maxAge,
useOktaClassic,
usePasswordSignIn,
signInEmail,
}: Record<keyof QueryParams, string | undefined>, // parameters from req.query
// some parameters may be manually passed in req.body too,
// generally for tracking purposes
Expand All @@ -78,6 +79,7 @@ export const parseExpressQueryParams = (
maxAge: stringToNumber(maxAge),
useOktaClassic: isStringBoolean(useOktaClassic),
usePasswordSignIn: isStringBoolean(usePasswordSignIn),
signInEmail,
};
};

Expand Down
Loading