Skip to content

Commit

Permalink
Merge pull request #2739 from guardian/mm/social-guest-fix
Browse files Browse the repository at this point in the history
Show account linking page for social sign in guest users
  • Loading branch information
coldlink authored May 23, 2024
2 parents 4c5aadf + 2caf3e1 commit e0c9fba
Show file tree
Hide file tree
Showing 7 changed files with 39 additions and 16 deletions.
4 changes: 2 additions & 2 deletions cypress/integration/shared/sign_in.shared.ts
Original file line number Diff line number Diff line change
Expand Up @@ -368,9 +368,9 @@ export const showsAnErrorMessageAndInformationParagraphWhenAccountLinkingRequire
: '/signin?error=accountLinkingRequired';
cy.visit(visitUrl);
cy.contains(
'We cannot sign you in with your social account credentials. Please enter your account password below to sign in.',
'We could not sign you in with your social account credentials. Please sign in with your email below.',
);
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 e0c9fba

Please sign in to comment.