Skip to content

Commit

Permalink
Fix a login error when using a password manager (#5261)
Browse files Browse the repository at this point in the history
* fix too many attempts login error when using a password manager

* tests

* remove promise

---------

Co-authored-by: Paweł Chyła <[email protected]>
  • Loading branch information
Cloud11PL and poulch committed Dec 11, 2024
1 parent 20b10c7 commit fb56837
Show file tree
Hide file tree
Showing 7 changed files with 92 additions and 4 deletions.
5 changes: 5 additions & 0 deletions .changeset/good-actors-yawn.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"saleor-dashboard": patch
---

Now there can only be one login request running at a time. This means using a password manager to log in no longer cause an error. If there are too many login requests Dashboard now shows a corresponding error message.
4 changes: 4 additions & 0 deletions locale/defaultMessages.json
Original file line number Diff line number Diff line change
Expand Up @@ -4612,6 +4612,10 @@
"context": "order does not require shipping",
"string": "does not apply"
},
"RyZd9J": {
"context": "error message",
"string": "Please wait a moment before trying again."
},
"Ryh3iR": {
"context": "header",
"string": "Create Webhook"
Expand Down
45 changes: 45 additions & 0 deletions src/auth/AuthProvider.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -211,4 +211,49 @@ describe("AuthProvider", () => {
expect(hook.result.current.errors).toEqual(["noPermissionsError"]);
expect(hook.result.current.authenticated).toBe(false);
});

it("should handle concurrent login attempts correctly", async () => {
const intl = useIntl();
const notify = useNotifier();
const apolloClient = useApolloClient();

(useAuthState as jest.Mock).mockImplementation(() => ({
authenticated: false,
authenticating: false,
}));

const loginMock = jest.fn(
() =>
new Promise(resolve => {
return resolve({
data: {
tokenCreate: {
errors: [],
user: {
userPermissions: [
{
code: "MANAGE_USERS",
name: "Handle checkouts",
},
],
},
},
},
});
}),
);

(useAuth as jest.Mock).mockImplementation(() => ({
login: loginMock,
logout: jest.fn(),
}));

const { result } = renderHook(() => useAuthProvider({ intl, notify, apolloClient }));

// Simulate two concurrent login attempts
result.current.login!("email", "password");
result.current.login!("email", "password");

expect(loginMock).toHaveBeenCalledTimes(1);
});
});
7 changes: 7 additions & 0 deletions src/auth/components/LoginPage/messages.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,11 @@ export const errorMessages = defineMessages({
defaultMessage: "You don't have permission to login.",
description: "error message",
},
loginAttemptDelay: {
defaultMessage: "Please wait a moment before trying again.",
description: "error message",
id: "RyZd9J",
},
});

export function getErrorMessage(err: UserContextError, intl: IntlShape): string {
Expand All @@ -36,5 +41,7 @@ export function getErrorMessage(err: UserContextError, intl: IntlShape): string
return intl.formatMessage(errorMessages.serverError);
case "noPermissionsError":
return intl.formatMessage(errorMessages.noPermissionsError);
case "loginAttemptDelay":
return intl.formatMessage(errorMessages.loginAttemptDelay);
}
}
28 changes: 25 additions & 3 deletions src/auth/hooks/useAuthProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { ApolloClient, ApolloError } from "@apollo/client";
import { IMessageContext } from "@dashboard/components/messages";
import { useAnalytics } from "@dashboard/components/ProductAnalytics/useAnalytics";
import { DEMO_MODE } from "@dashboard/config";
import { useUserDetailsQuery } from "@dashboard/graphql";
import { AccountErrorCode, useUserDetailsQuery } from "@dashboard/graphql";
import useLocalStorage from "@dashboard/hooks/useLocalStorage";
import useNavigator from "@dashboard/hooks/useNavigator";
import { commonMessages } from "@dashboard/intl";
Expand Down Expand Up @@ -34,13 +34,15 @@ export interface UseAuthProviderOpts {
notify: IMessageContext;
apolloClient: ApolloClient<any>;
}
type AuthErrorCodes = `${AccountErrorCode}`;

export function useAuthProvider({ intl, notify, apolloClient }: UseAuthProviderOpts): UserContext {
const { login, getExternalAuthUrl, getExternalAccessToken, logout } = useAuth();
const analytics = useAnalytics();
const navigate = useNavigator();
const { authenticated, authenticating, user } = useAuthState();
const [requestedExternalPluginId] = useLocalStorage("requestedExternalPluginId", null);
const [isAuthRequestRunning, setIsAuthRequestRunning] = useState(false);
const [errors, setErrors] = useState<UserContextError[]>([]);
const permitCredentialsAPI = useRef(true);

Expand Down Expand Up @@ -116,27 +118,45 @@ export function useAuthProvider({ intl, notify, apolloClient }: UseAuthProviderO
}
}
};

const handleLogin = async (email: string, password: string) => {
if (isAuthRequestRunning) {
return;
}

try {
setIsAuthRequestRunning(true);

const result = await login({
email,
password,
includeDetails: false,
});

const errorList = result.data?.tokenCreate?.errors?.map(
({ code }) => code,
// SDK is deprecated and has outdated types - we need to use ones from Dashboard
) as AuthErrorCodes[];

if (isEmpty(result.data?.tokenCreate?.user?.userPermissions)) {
setErrors(["noPermissionsError"]);
await handleLogout();
}

if (result && !result.data?.tokenCreate?.errors.length) {
if (result && !errorList?.length) {
if (DEMO_MODE) {
displayDemoMessage(intl, notify);
}

saveCredentials(result.data?.tokenCreate?.user!, password);
} else {
setErrors(["loginError"]);
// While login page can show multiple errors, "loginError" doesn't match "attemptDelay"
// and should be shown when no other error is present
if (errorList?.includes(AccountErrorCode.LOGIN_ATTEMPT_DELAYED)) {
setErrors(["loginAttemptDelay"]);
} else {
setErrors(["loginError"]);
}
}

await logoutNonStaffUser(result.data?.tokenCreate!);
Expand All @@ -148,6 +168,8 @@ export function useAuthProvider({ intl, notify, apolloClient }: UseAuthProviderO
} else {
setErrors(["unknownLoginError"]);
}
} finally {
setIsAuthRequestRunning(false);
}
};
const handleRequestExternalLogin = async (pluginId: string, input: RequestExternalLoginInput) => {
Expand Down
1 change: 1 addition & 0 deletions src/auth/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ export const UserContextError = {
serverError: "serverError",
noPermissionsError: "noPermissionsError",
externalLoginError: "externalLoginError",
loginAttemptDelay: "loginAttemptDelay",
unknownLoginError: "unknownLoginError",
} as const;

Expand Down
6 changes: 5 additions & 1 deletion src/auth/views/Login.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,11 @@ const LoginView: React.FC<LoginViewProps> = ({ params }) => {
setRequestedExternalPluginId,
} = useAuthParameters();
const handleSubmit = async (data: LoginFormData) => {
const result = await login!(data.email, data.password);
if (!login) {
return;
}

const result = await login(data.email, data.password);
const errors = result?.errors || [];

return errors;
Expand Down

0 comments on commit fb56837

Please sign in to comment.