Skip to content

Commit

Permalink
fix(social): show account linking page for guest users attempting to …
Browse files Browse the repository at this point in the history
…sign in with social
  • Loading branch information
coldlink committed May 23, 2024
1 parent 4c5aadf commit 98f6bad
Show file tree
Hide file tree
Showing 7 changed files with 38 additions and 15 deletions.
2 changes: 1 addition & 1 deletion cypress/integration/shared/sign_in.shared.ts
Original file line number Diff line number Diff line change
Expand Up @@ -370,7 +370,7 @@ export const showsAnErrorMessageAndInformationParagraphWhenAccountLinkingRequire
cy.contains(
'We cannot sign you in with your social account credentials. Please enter your account password below to sign in.',
);
cy.contains('Account already exists');
cy.contains('Social sign-in unsuccessful');
},
] as const;
};
Expand Down
6 changes: 3 additions & 3 deletions src/client/pages/SignIn.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ WithPageLevelErrorAndEmail.story = {
export const SocialSigninBlocked = (args: SignInProps) => (
<SignIn
{...args}
pageError={SignInErrors.ACCOUNT_ALREADY_EXISTS}
pageError={SignInErrors.SOCIAL_SIGNIN_ERROR}
email="someone@theguardian.com"
/>
);
Expand All @@ -72,7 +72,7 @@ WithFormLevelErrorAndEmail.story = {
export const WithFormLevelErrorAndSocialSigninBlocked = (args: SignInProps) => (
<SignIn
{...args}
pageError={SignInErrors.ACCOUNT_ALREADY_EXISTS}
pageError={SignInErrors.SOCIAL_SIGNIN_ERROR}
formError="This is an error"
email="someone@theguardian.com"
/>
Expand Down Expand Up @@ -107,7 +107,7 @@ WithJobs.story = {
export const WithJobsAndSocialSigninBlocked = (args: SignInProps) => (
<SignIn
{...{ ...args, queryParams: { ...args.queryParams, clientId: 'jobs' } }}
pageError={SignInErrors.ACCOUNT_ALREADY_EXISTS}
pageError={SignInErrors.SOCIAL_SIGNIN_ERROR}
email="someone@theguardian.com"
/>
);
Expand Down
32 changes: 25 additions & 7 deletions src/client/pages/SignIn.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -56,12 +56,30 @@ const Links = ({ children }: { children: React.ReactNode }) => (
</div>
);

const getErrorContext = (error: string | undefined) => {
if (error === SignInErrors.ACCOUNT_ALREADY_EXISTS) {
const getErrorContext = (
error: string | undefined,
queryParams: QueryParams,
) => {
if (error === SignInErrors.SOCIAL_SIGNIN_ERROR) {
return (
<>
We cannot sign you in with your social account credentials. Please enter
your account password below to sign in.
<div>
We could not sign you in with your social account credentials. Please
sign in with your email below. If you do not know your password, you
can{' '}
<Link
href={buildUrlWithQueryParams('/reset-password', {}, queryParams)}
>
reset it here
</Link>
.
</div>
<br />
<div>
If you are still having trouble, please contact our customer service
team at{' '}
<Link href={locations.SUPPORT_EMAIL_MAILTO}>{SUPPORT_EMAIL}</Link>.
</div>
</>
);
} else if (error === RegistrationErrors.PROVISIONING_FAILURE) {
Expand Down Expand Up @@ -113,7 +131,7 @@ export const SignIn = ({
const formTrackingName = 'sign-in';

// The page level error is equivalent to this enum if social signin has been blocked.
const socialSigninBlocked = pageError === SignInErrors.ACCOUNT_ALREADY_EXISTS;
const socialSigninBlocked = pageError === SignInErrors.SOCIAL_SIGNIN_ERROR;

const { clientId } = queryParams;
const isJobs = clientId === 'jobs';
Expand All @@ -129,7 +147,7 @@ export const SignIn = ({
return (
<MainLayout
errorOverride={pageError}
errorContext={getErrorContext(pageError)}
errorContext={getErrorContext(pageError, queryParams)}
tabs={tabs}
errorSmallMarginBottom={!!pageError}
pageHeader="Sign in"
Expand All @@ -139,7 +157,7 @@ export const SignIn = ({
{showAuthProviderButtons(socialSigninBlocked, queryParams, isJobs)}
<MainForm
formErrorMessageFromParent={formError}
formErrorContextFromParent={getErrorContext(formError)}
formErrorContextFromParent={getErrorContext(formError, queryParams)}
formAction={buildUrlWithQueryParams(
isReauthenticate ? '/reauthenticate' : '/signin',
{},
Expand Down
8 changes: 6 additions & 2 deletions src/server/routes/oauth.ts
Original file line number Diff line number Diff line change
Expand Up @@ -801,8 +801,9 @@ router.get(
// and redirect to the sign in page with the social sign in blocked error
if (
callbackParams.error === OpenIdErrors.ACCESS_DENIED &&
callbackParams.error_description ===
OpenIdErrorDescriptions.ACCOUNT_LINKING_DENIED_GROUPS
(callbackParams.error_description ===
OpenIdErrorDescriptions.ACCOUNT_LINKING_DENIED_GROUPS ||
OpenIdErrorDescriptions.USER_STATUS_INVALID)
) {
trackMetric('OAuthAuthorization::Failure');
return res.redirect(
Expand All @@ -811,6 +812,9 @@ router.get(
}),
);
}

// handle the rest of the errors generically
return redirectForGenericError(req, res);
}

// exchange the auth code for access token + id token
Expand Down
2 changes: 1 addition & 1 deletion src/server/routes/signIn.ts
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ export const getErrorMessageFromQueryParams = (
) => {
// show error if account linking required
if (error === FederationErrors.SOCIAL_SIGNIN_BLOCKED) {
return SignInErrors.ACCOUNT_ALREADY_EXISTS;
return SignInErrors.SOCIAL_SIGNIN_ERROR;
}
// Show error if provisioning failed
if (error === RegistrationErrors.PROVISIONING_FAILURE) {
Expand Down
2 changes: 1 addition & 1 deletion src/shared/model/Errors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ export enum ResetPasswordErrors {
export enum SignInErrors {
GENERIC = 'There was a problem signing in, please try again.',
AUTHENTICATION_FAILED = "Email and password don't match",
ACCOUNT_ALREADY_EXISTS = 'Account already exists',
SOCIAL_SIGNIN_ERROR = 'Social sign-in unsuccessful',
}

export enum RegistrationErrors {
Expand Down
1 change: 1 addition & 0 deletions src/shared/model/OpenIdErrors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,4 +17,5 @@ export enum OpenIdErrors {

export enum OpenIdErrorDescriptions {
ACCOUNT_LINKING_DENIED_GROUPS = 'User linking was denied because the user is not in any of the specified groups.',
USER_STATUS_INVALID = 'User status is invalid.',
}

0 comments on commit 98f6bad

Please sign in to comment.