Fix improper double attempt to get external access token (#2504)

* Fix improper double attempt to get external access token

* Update messages

* Add auth error types
This commit is contained in:
Dawid 2022-11-23 12:09:48 +01:00 committed by GitHub
parent 1fa9213871
commit 2e45f04802
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
12 changed files with 110 additions and 41 deletions

View file

@ -1162,6 +1162,10 @@
"context": "product field", "context": "product field",
"string": "Export Product Weight" "string": "Export Product Weight"
}, },
"7Jg9ec": {
"context": "error message",
"string": "You don't have permission to login."
},
"7KRGqz": { "7KRGqz": {
"context": "Payment card title", "context": "Payment card title",
"string": "Payment balance" "string": "Payment balance"
@ -2215,6 +2219,10 @@
"Fn3bE0": { "Fn3bE0": {
"string": "Order line updated" "string": "Order line updated"
}, },
"FopBSj": {
"context": "error message",
"string": "Your username and/or password are incorrect. Please try again."
},
"FpIcp9": { "FpIcp9": {
"string": "No customers found" "string": "No customers found"
}, },
@ -3051,10 +3059,6 @@
"context": "collection", "context": "collection",
"string": "Not Published" "string": "Not Published"
}, },
"M4q0Ye": {
"context": "error message",
"string": "Sorry, login went wrong. Please try again."
},
"M6s/9e": { "M6s/9e": {
"context": "unassign country, dialog header", "context": "unassign country, dialog header",
"string": "Remove from Shipping Zone" "string": "Remove from Shipping Zone"
@ -4515,6 +4519,10 @@
"context": "title", "context": "title",
"string": "Theres a problem with app." "string": "Theres a problem with app."
}, },
"Wr5UOV": {
"context": "error message",
"string": "Login went wrong. Please try again."
},
"Ww69SE": { "Ww69SE": {
"context": "search input placeholder", "context": "search input placeholder",
"string": "Search tax classes" "string": "Search tax classes"
@ -7197,10 +7205,6 @@
"tR+UuE": { "tR+UuE": {
"string": "User doesn't exist. Please check your email in URL" "string": "User doesn't exist. Please check your email in URL"
}, },
"tTtoKd": {
"context": "error message",
"string": "Sorry, your username and/or password are incorrect. Please try again."
},
"tTuCYj": { "tTuCYj": {
"context": "all gift cards label", "context": "all gift cards label",
"string": "All Gift Cards" "string": "All Gift Cards"

View file

@ -15,6 +15,7 @@ const props: Omit<LoginCardProps, "classes"> = {
}, },
], ],
loading: false, loading: false,
errors: [],
onExternalAuthentication: () => undefined, onExternalAuthentication: () => undefined,
onSubmit: () => undefined, onSubmit: () => undefined,
}; };
@ -23,9 +24,9 @@ storiesOf("Views / Authentication / Log in", module)
.addDecorator(CardDecorator) .addDecorator(CardDecorator)
.addDecorator(Decorator) .addDecorator(Decorator)
.add("default", () => <LoginPage {...props} />) .add("default", () => <LoginPage {...props} />)
.add("error login", () => <LoginPage {...props} error={"loginError"} />) .add("error login", () => <LoginPage {...props} errors={["loginError"]} />)
.add("error external login", () => ( .add("error external login", () => (
<LoginPage {...props} error={"externalLoginError"} /> <LoginPage {...props} errors={["externalLoginError"]} />
)) ))
.add("disabled", () => <LoginPage {...props} disabled={true} />) .add("disabled", () => <LoginPage {...props} disabled={true} />)
.add("loading", () => <LoginPage {...props} loading={true} />); .add("loading", () => <LoginPage {...props} loading={true} />);

View file

@ -21,7 +21,7 @@ import LoginForm, { LoginFormData } from "./form";
import { getErrorMessage } from "./messages"; import { getErrorMessage } from "./messages";
export interface LoginCardProps { export interface LoginCardProps {
error?: UserContextError; errors: UserContextError[];
disabled: boolean; disabled: boolean;
loading: boolean; loading: boolean;
externalAuthentications?: AvailableExternalAuthenticationsQuery["shop"]["availableExternalAuthentications"]; externalAuthentications?: AvailableExternalAuthenticationsQuery["shop"]["availableExternalAuthentications"];
@ -31,7 +31,7 @@ export interface LoginCardProps {
const LoginCard: React.FC<LoginCardProps> = props => { const LoginCard: React.FC<LoginCardProps> = props => {
const { const {
error, errors,
disabled, disabled,
loading, loading,
externalAuthentications = [], externalAuthentications = [],
@ -62,11 +62,15 @@ const LoginCard: React.FC<LoginCardProps> = props => {
description="card header" description="card header"
/> />
</Typography> </Typography>
{error && ( {errors.map(error => (
<div className={classes.panel} data-test-id="login-error-message"> <div
className={classes.panel}
key={error}
data-test-id="login-error-message"
>
{getErrorMessage(error, intl)} {getErrorMessage(error, intl)}
</div> </div>
)} ))}
<TextField <TextField
autoFocus autoFocus
fullWidth fullWidth

View file

@ -3,14 +3,14 @@ import { defineMessages, IntlShape } from "react-intl";
export const errorMessages = defineMessages({ export const errorMessages = defineMessages({
loginError: { loginError: {
id: "tTtoKd", id: "FopBSj",
defaultMessage: defaultMessage:
"Sorry, your username and/or password are incorrect. Please try again.", "Your username and/or password are incorrect. Please try again.",
description: "error message", description: "error message",
}, },
externalLoginError: { unknownLoginError: {
id: "M4q0Ye", id: "Wr5UOV",
defaultMessage: "Sorry, login went wrong. Please try again.", defaultMessage: "Login went wrong. Please try again.",
description: "error message", description: "error message",
}, },
serverError: { serverError: {
@ -19,6 +19,11 @@ export const errorMessages = defineMessages({
"Saleor is unavailable, please check your network connection and try again.", "Saleor is unavailable, please check your network connection and try again.",
description: "error message", description: "error message",
}, },
noPermissionsError: {
id: "7Jg9ec",
defaultMessage: "You don't have permission to login.",
description: "error message",
},
}); });
export function getErrorMessage( export function getErrorMessage(
@ -29,8 +34,12 @@ export function getErrorMessage(
case "loginError": case "loginError":
return intl.formatMessage(errorMessages.loginError); return intl.formatMessage(errorMessages.loginError);
case "externalLoginError": case "externalLoginError":
return intl.formatMessage(errorMessages.externalLoginError); return intl.formatMessage(errorMessages.unknownLoginError);
case "unknownLoginError":
return intl.formatMessage(errorMessages.unknownLoginError);
case "serverError": case "serverError":
return intl.formatMessage(errorMessages.serverError); return intl.formatMessage(errorMessages.serverError);
case "noPermissionsError":
return intl.formatMessage(errorMessages.noPermissionsError);
} }
} }

View file

@ -1,12 +1,22 @@
import { ApolloError } from "@apollo/client";
import { findValueInEnum } from "@saleor/misc"; import { findValueInEnum } from "@saleor/misc";
import { GraphQLError } from "graphql"; import { GraphQLError } from "graphql";
import { UserContextError } from "./types";
export enum JWTError { export enum JWTError {
invalid = "InvalidTokenError", invalid = "InvalidTokenError",
invalidSignature = "InvalidSignatureError", invalidSignature = "InvalidSignatureError",
expired = "ExpiredSignatureError", expired = "ExpiredSignatureError",
} }
export const AuthError = {
PermissionDenied: "PermissionDenied",
OAuthError: "OAuthError",
} as const;
export type AuthError = typeof AuthError[keyof typeof AuthError];
export function isJwtError(error: GraphQLError): boolean { export function isJwtError(error: GraphQLError): boolean {
let jwtError: boolean; let jwtError: boolean;
try { try {
@ -21,3 +31,22 @@ export function isJwtError(error: GraphQLError): boolean {
export function isTokenExpired(error: GraphQLError): boolean { export function isTokenExpired(error: GraphQLError): boolean {
return error.extensions.exception.code === JWTError.expired; return error.extensions.exception.code === JWTError.expired;
} }
export function getAuthErrorType(graphQLError: GraphQLError): UserContextError {
switch (graphQLError.extensions?.exception?.code as AuthError) {
case AuthError.PermissionDenied:
return UserContextError.noPermissionsError;
case AuthError.OAuthError:
return UserContextError.externalLoginError;
default:
return UserContextError.unknownLoginError;
}
}
export function parseAuthError(authError: ApolloError): UserContextError[] {
return (
authError?.graphQLErrors?.map(graphQLError =>
getAuthErrorType(graphQLError),
) || []
);
}

View file

@ -1,4 +1,4 @@
import { ApolloClient } from "@apollo/client"; import { ApolloClient, ApolloError } from "@apollo/client";
import { IMessageContext } from "@saleor/components/messages"; import { IMessageContext } from "@saleor/components/messages";
import { DEMO_MODE } from "@saleor/config"; import { DEMO_MODE } from "@saleor/config";
import { useUserDetailsQuery } from "@saleor/graphql"; import { useUserDetailsQuery } from "@saleor/graphql";
@ -21,6 +21,7 @@ import { useEffect, useRef, useState } from "react";
import { IntlShape } from "react-intl"; import { IntlShape } from "react-intl";
import urlJoin from "url-join"; import urlJoin from "url-join";
import { parseAuthError } from "../errors";
import { import {
ExternalLoginInput, ExternalLoginInput,
RequestExternalLoginInput, RequestExternalLoginInput,
@ -53,12 +54,12 @@ export function useAuthProvider({
"requestedExternalPluginId", "requestedExternalPluginId",
null, null,
); );
const [error, setError] = useState<UserContextError>(); const [errors, setErrors] = useState<UserContextError[]>([]);
const permitCredentialsAPI = useRef(true); const permitCredentialsAPI = useRef(true);
useEffect(() => { useEffect(() => {
if (authenticating && error) { if (authenticating && errors.length) {
setError(undefined); setErrors([]);
} }
}, [authenticating]); }, [authenticating]);
@ -88,6 +89,16 @@ export function useAuthProvider({
fetchPolicy: "cache-and-network", fetchPolicy: "cache-and-network",
}); });
const handleLoginError = (error: ApolloError) => {
const parsedErrors = parseAuthError(error);
if (parsedErrors.length) {
setErrors(parsedErrors);
} else {
setErrors(["unknownLoginError"]);
}
};
const handleLogout = async () => { const handleLogout = async () => {
const returnTo = urlJoin( const returnTo = urlJoin(
window.location.origin, window.location.origin,
@ -139,14 +150,14 @@ export function useAuthProvider({
} }
saveCredentials(result.data.tokenCreate.user, password); saveCredentials(result.data.tokenCreate.user, password);
} else { } else {
setError("loginError"); setErrors(["loginError"]);
} }
await logoutNonStaffUser(result.data.tokenCreate); await logoutNonStaffUser(result.data.tokenCreate);
return result.data.tokenCreate; return result.data.tokenCreate;
} catch (error) { } catch (error) {
setError("serverError"); handleLoginError(error);
} }
}; };
@ -177,14 +188,14 @@ export function useAuthProvider({
displayDemoMessage(intl, notify); displayDemoMessage(intl, notify);
} }
} else { } else {
setError("externalLoginError"); setErrors(["externalLoginError"]);
} }
await logoutNonStaffUser(result.data.externalObtainAccessTokens); await logoutNonStaffUser(result.data.externalObtainAccessTokens);
return result?.data?.externalObtainAccessTokens; return result?.data?.externalObtainAccessTokens;
} catch (error) { } catch (error) {
setError("serverError"); handleLoginError(error);
} }
}; };
@ -206,9 +217,9 @@ export function useAuthProvider({
requestLoginByExternalPlugin: handleRequestExternalLogin, requestLoginByExternalPlugin: handleRequestExternalLogin,
loginByExternalPlugin: handleExternalLogin, loginByExternalPlugin: handleExternalLogin,
logout: handleLogout, logout: handleLogout,
authenticating: authenticating && !error, authenticating: authenticating && !errors.length,
authenticated: authenticated && user?.isStaff, authenticated: authenticated && user?.isStaff,
user: userDetails.data?.me, user: userDetails.data?.me,
error, errors,
}; };
} }

View file

@ -29,6 +29,7 @@ export const UserContext = React.createContext<Context>({
requestLoginByExternalPlugin: undefined, requestLoginByExternalPlugin: undefined,
authenticating: false, authenticating: false,
authenticated: false, authenticated: false,
errors: [],
}); });
const AuthRouter: React.FC = () => ( const AuthRouter: React.FC = () => (

View file

@ -18,10 +18,15 @@ export interface RequestExternalLogoutInput {
returnTo: string; returnTo: string;
} }
export type UserContextError = export const UserContextError = {
| "loginError" loginError: "loginError",
| "externalLoginError" serverError: "serverError",
| "serverError"; noPermissionsError: "noPermissionsError",
externalLoginError: "externalLoginError",
unknownLoginError: "unknownLoginError",
} as const;
export type UserContextError = typeof UserContextError[keyof typeof UserContextError];
export interface UserContext { export interface UserContext {
login: (username: string, password: string) => Promise<LoginData>; login: (username: string, password: string) => Promise<LoginData>;
@ -37,5 +42,5 @@ export interface UserContext {
user?: UserFragment; user?: UserFragment;
authenticating: boolean; authenticating: boolean;
authenticated: boolean; authenticated: boolean;
error?: UserContextError; errors: UserContextError[];
} }

View file

@ -23,7 +23,7 @@ const LoginView: React.FC<LoginViewProps> = ({ params }) => {
requestLoginByExternalPlugin, requestLoginByExternalPlugin,
loginByExternalPlugin, loginByExternalPlugin,
authenticating, authenticating,
error, errors,
} = useUser(); } = useUser();
const { const {
data: externalAuthentications, data: externalAuthentications,
@ -79,14 +79,17 @@ const LoginView: React.FC<LoginViewProps> = ({ params }) => {
const { code, state } = params; const { code, state } = params;
const isCallbackPath = location.pathname.includes(loginCallbackPath); const isCallbackPath = location.pathname.includes(loginCallbackPath);
if (code && state && isCallbackPath) { const externalAuthParamsExist = code && state && isCallbackPath;
const externalAuthNotPerformed = !authenticating && !errors.length;
if (externalAuthParamsExist && externalAuthNotPerformed) {
handleExternalAuthentication(code, state); handleExternalAuthentication(code, state);
} }
}, []); }, []);
return ( return (
<LoginPage <LoginPage
error={error} errors={errors}
disabled={authenticating} disabled={authenticating}
externalAuthentications={ externalAuthentications={
externalAuthentications?.shop?.availableExternalAuthentications externalAuthentications?.shop?.availableExternalAuthentications

View file

@ -12,6 +12,7 @@ export const UserDecorator = (user: UserFragment) => storyFn => (
user, user,
authenticated: false, authenticated: false,
authenticating: false, authenticating: false,
errors: [],
}} }}
> >
{storyFn()} {storyFn()}

View file

@ -37127,7 +37127,7 @@ exports[`Storyshots Views / Authentication / Log in error external login 1`] = `
class="Login-panel-id" class="Login-panel-id"
data-test-id="login-error-message" data-test-id="login-error-message"
> >
Sorry, login went wrong. Please try again. Login went wrong. Please try again.
</div> </div>
<div <div
class="MuiFormControl-root-id MuiTextField-root-id MuiFormControl-fullWidth-id" class="MuiFormControl-root-id MuiTextField-root-id MuiFormControl-fullWidth-id"
@ -37311,7 +37311,7 @@ exports[`Storyshots Views / Authentication / Log in error login 1`] = `
class="Login-panel-id" class="Login-panel-id"
data-test-id="login-error-message" data-test-id="login-error-message"
> >
Sorry, your username and/or password are incorrect. Please try again. Your username and/or password are incorrect. Please try again.
</div> </div>
<div <div
class="MuiFormControl-root-id MuiTextField-root-id MuiFormControl-fullWidth-id" class="MuiFormControl-root-id MuiTextField-root-id MuiFormControl-fullWidth-id"

View file

@ -24,6 +24,7 @@ export const MockedUserProvider: React.FC<{
avatar: null, avatar: null,
__typename: "User", __typename: "User",
}, },
errors: [],
}} }}
> >
{children} {children}