Skip to content

Commit

Permalink
Show a different message error when the password is wrong (#1274)
Browse files Browse the repository at this point in the history
## Problem

The login form does not differentiate between an authentication error or
an server error.

## Solution

Show a different error depending on the response status code.

## Screenshots

|Authentication Error | Server error | 
| --- | --- |
|
![image](https://github.com/openSUSE/agama/assets/7056681/da28a82c-1ace-474d-8f73-809ebe75fa22)
| ![Screenshot from 2024-06-03
08-39-40](https://github.com/openSUSE/agama/assets/7056681/fcd23d95-8d5e-4ea7-9050-c0f689cb08a8)
|
  • Loading branch information
teclator authored Jun 6, 2024
2 parents 4b20db3 + d5fbf9b commit e79faf3
Show file tree
Hide file tree
Showing 4 changed files with 91 additions and 8 deletions.
8 changes: 8 additions & 0 deletions web/package/agama-web-ui.changes
Original file line number Diff line number Diff line change
@@ -1,3 +1,11 @@
-------------------------------------------------------------------
Thu Jun 6 07:43:50 UTC 2024 - Knut Anderssen <[email protected]>

- Try to reconnect silently when the WebSocket is closed displaying
a page error if it is not possible (gh#openSUSE/agama#1254).
- Display a different login error message depending on the request
response (gh@openSUSE/agama#1274).

-------------------------------------------------------------------
Thu May 23 07:28:44 UTC 2024 - Josef Reidinger <[email protected]>

Expand Down
15 changes: 11 additions & 4 deletions web/src/components/core/LoginPage.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ import { Navigate } from "react-router-dom";
import { ActionGroup, Button, Form, FormGroup } from "@patternfly/react-core";
import { sprintf } from "sprintf-js";
import { _ } from "~/i18n";
import { useAuth } from "~/context/auth";
import { AuthErrors, useAuth } from "~/context/auth";
import { About, FormValidationError, If, Page, PasswordInput, Section } from "~/components/core";
import { Center } from "~/components/layout";

Expand All @@ -39,14 +39,21 @@ import { Center } from "~/components/layout";
export default function LoginPage() {
const [password, setPassword] = useState("");
const [error, setError] = useState(false);
const { isLoggedIn, login: loginFn } = useAuth();
const { isLoggedIn, login: loginFn, error: loginError } = useAuth();

const login = async (e) => {
e.preventDefault();
const result = await loginFn(password);
setError(!result);

setError(result.status !== 200);
};

const errorMessage = (authError) => {
if (authError === AuthErrors.AUTH)
return _("Could not log in. Please, make sure that the password is correct.");

return _("Could not authenticate against the server, please check it.");
};
if (isLoggedIn) {
return <Navigate to="/" />;
}
Expand Down Expand Up @@ -83,7 +90,7 @@ export default function LoginPage() {
condition={error}
then={
<FormValidationError
message={_("Could not log in. Please, make sure that the password is correct.")}
message={errorMessage(loginError)}
/>
}
/>
Expand Down
49 changes: 48 additions & 1 deletion web/src/components/core/LoginPage.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -23,23 +23,28 @@ import React from "react";
import { screen, within } from "@testing-library/react";
import { plainRender } from "~/test-utils";
import { LoginPage } from "~/components/core";
import { AuthErrors } from "~/context/auth";

let mockIsAuthenticated;
const mockLoginFn = jest.fn();
let mockLoginError;

jest.mock("~/context/auth", () => ({
...jest.requireActual("~/context/auth"),
useAuth: () => {
return {
isAuthenticated: mockIsAuthenticated,
login: mockLoginFn
login: mockLoginFn,
error: mockLoginError
};
}
}));

describe("LoginPage", () => {
beforeAll(() => {
mockIsAuthenticated = false;
mockLoginError = null;
mockLoginFn.mockResolvedValue({ status: 200 });
jest.spyOn(console, "error").mockImplementation();
});

Expand All @@ -65,6 +70,48 @@ describe("LoginPage", () => {
expect(mockLoginFn).toHaveBeenCalledWith("s3cr3t");
});

describe("and the entered password is wrong", () => {
beforeAll(() => {
mockLoginFn.mockResolvedValue({ status: 400 });
mockLoginError = AuthErrors.AUTH;
});

it("renders an authentication error", async () => {
const { user } = plainRender(<LoginPage />);
const form = screen.getByRole("form", { name: "Login form" });
const passwordInput = within(form).getByLabelText("Password input");
const loginButton = within(form).getByRole("button", { name: "Log in" });

await user.type(passwordInput, "s3cr3t");
await user.click(loginButton);

expect(mockLoginFn).toHaveBeenCalledWith("s3cr3t");
const form_error = screen.getByRole("form", { name: "Login form" });
within(form_error).getByText(/Could not log in/);
});
});

describe("and the server is down", () => {
beforeAll(() => {
mockLoginFn.mockResolvedValue({ status: 504 });
mockLoginError = AuthErrors.SERVER;
});

it("renders a server error text", async () => {
const { user } = plainRender(<LoginPage />);
const form = screen.getByRole("form", { name: "Login form" });
const passwordInput = within(form).getByLabelText("Password input");
const loginButton = within(form).getByRole("button", { name: "Log in" });

await user.type(passwordInput, "s3cr3t");
await user.click(loginButton);

expect(mockLoginFn).toHaveBeenCalledWith("s3cr3t");
const form_error = screen.getByRole("form", { name: "Login form" });
within(form_error).getByText(/Could not authenticate/);
});
});

it("renders a button to know more about the project", async () => {
const { user } = plainRender(<LoginPage />);
const button = screen.getByRole("button", { name: "What is this?" });
Expand Down
27 changes: 24 additions & 3 deletions web/src/context/auth.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -37,22 +37,37 @@ function useAuth() {
return context;
}

const AuthErrors = Object.freeze({
SERVER: "server",
AUTH: "auth",
OTHER: "other"
});

/**
* @param {object} props
* @param {React.ReactNode} [props.children] - content to display within the provider
*/
function AuthProvider({ children }) {
const [isLoggedIn, setIsLoggedIn] = useState(false);
const [error, setError] = useState(null);

const login = useCallback(async (password) => {
const response = await fetch("/api/auth", {
method: "POST",
body: JSON.stringify({ password }),
headers: { "Content-Type": "application/json" },
});

const result = response.status === 200;
if ((response.status >= 500) && (response.status < 600)) {
setError(AuthErrors.SERVER);
}
if ((response.status >= 400) && (response.status < 500)) {
setError(AuthErrors.AUTH);
}
setIsLoggedIn(result);
return result;

return response;
}, []);

const logout = useCallback(async () => {
Expand All @@ -68,15 +83,21 @@ function AuthProvider({ children }) {
})
.then((response) => {
setIsLoggedIn(response.status === 200);
if ((response.status >= 500) && (response.status < 600)) {
setError(AuthErrors.SERVER);
}
if ((response.status >= 400) && (response.status < 500)) {
setError(AuthErrors.AUTH);
}
})
.catch(() => setIsLoggedIn(false));
}, []);

return (
<AuthContext.Provider value={{ login, logout, isLoggedIn }}>
<AuthContext.Provider value={{ login, logout, isLoggedIn, error }}>
{children}
</AuthContext.Provider>
);
}

export { AuthProvider, useAuth };
export { AuthProvider, useAuth, AuthErrors };

0 comments on commit e79faf3

Please sign in to comment.