From 92088318433ca9c4afe3d7e95fd39adfd7143dad Mon Sep 17 00:00:00 2001 From: Jayasanka Date: Tue, 21 Nov 2023 13:15:53 +0530 Subject: [PATCH 1/5] O3-2583: Configure Password Field Placement on Login Screen New Configuration Variable: - Introduced a new configuration variable named 'passwordOnSeparateScreen' to allow implementers to customize the placement of the password field on the login screen. - The default value is set to 'true' to maintain the existing behavior of having the password field on a separate screen. Configuration Usage: - When 'passwordOnSeparateScreen' is set to 'true', the password field appears on a separate screen. - When set to 'false', the password field is displayed on the same screen as the username and other login elements. --- .../esm-login-app/__mocks__/config.mock.ts | 6 +- .../apps/esm-login-app/src/config-schema.ts | 9 +- .../src/login/login.component.tsx | 50 ++++------ .../apps/esm-login-app/src/login/login.scss | 4 + .../esm-login-app/src/login/login.test.tsx | 97 +++++++++++++++++-- 5 files changed, 124 insertions(+), 42 deletions(-) diff --git a/packages/apps/esm-login-app/__mocks__/config.mock.ts b/packages/apps/esm-login-app/__mocks__/config.mock.ts index fb0b52db2..273816e33 100644 --- a/packages/apps/esm-login-app/__mocks__/config.mock.ts +++ b/packages/apps/esm-login-app/__mocks__/config.mock.ts @@ -1,4 +1,6 @@ -export const mockConfig = { +import { ConfigSchema } from "../src/config-schema"; + +export const mockConfig: ConfigSchema = { provider: { type: "basic", loginUrl: "", @@ -8,6 +10,7 @@ export const mockConfig = { enabled: true, numberToShow: 3, useLoginLocationTag: true, + locationsPerRequest: 50, }, logo: { src: null, @@ -16,4 +19,5 @@ export const mockConfig = { links: { loginSuccess: "${openmrsSpaBase}/home", }, + passwordOnSeparateScreen: true, }; diff --git a/packages/apps/esm-login-app/src/config-schema.ts b/packages/apps/esm-login-app/src/config-schema.ts index 6346c387d..ecb31aaa8 100644 --- a/packages/apps/esm-login-app/src/config-schema.ts +++ b/packages/apps/esm-login-app/src/config-schema.ts @@ -71,6 +71,12 @@ export const configSchema = { _description: "Alt text, shown on hover", }, }, + passwordOnSeparateScreen: { + _type: Type.Boolean, + _default: true, + _description: + "Whether to show the password field on a separate screen. If false, the password field will be shown on the same screen.", + }, }; export interface ConfigSchema { @@ -81,7 +87,7 @@ export interface ConfigSchema { }; chooseLocation: { enabled: boolean; - numberToShow: boolean; + numberToShow: number; locationsPerRequest: number; useLoginLocationTag: boolean; }; @@ -92,4 +98,5 @@ export interface ConfigSchema { src: string; alt: string; }; + passwordOnSeparateScreen: boolean; } diff --git a/packages/apps/esm-login-app/src/login/login.component.tsx b/packages/apps/esm-login-app/src/login/login.component.tsx index 40ec1af66..2652bcb88 100644 --- a/packages/apps/esm-login-app/src/login/login.component.tsx +++ b/packages/apps/esm-login-app/src/login/login.component.tsx @@ -40,6 +40,7 @@ export interface LoginProps extends LoginReferrer {} const Login: React.FC = () => { const config = useConfig(); + const { passwordOnSeparateScreen } = config; const isLoginEnabled = useConnectivity(); const { t } = useTranslation(); const { user } = useSession(); @@ -52,7 +53,10 @@ const Login: React.FC = () => { const passwordInputRef = useRef(null); const usernameInputRef = useRef(null); const formRef = useRef(null); - const showPassword = location.pathname === "/login/confirm"; + + const showUsername = location.pathname === "/login"; + const showPassword = + !passwordOnSeparateScreen || location.pathname === "/login/confirm"; const handleLogin = useCallback( (session: Session) => { @@ -195,7 +199,7 @@ const Login: React.FC = () => { /> )} - {showPassword ? ( + {passwordOnSeparateScreen && showPassword ? (
+ {passwordOnSeparateScreen && ( + + )}
)} {showPassword && (
- - { it("renders the login form", () => { - renderWithRouter(Login); + renderWithRouter( + Login, + {}, + { + route: "/login", + } + ); screen.getByRole("img", { name: /OpenMRS logo/i }); expect(screen.queryByAltText(/logo/i)).not.toBeInTheDocument(); @@ -79,7 +85,6 @@ describe("Login", () => { {}, { route: "/login", - routes: ["/login", "/login/confirm"], } ); const user = userEvent.setup(); @@ -109,12 +114,11 @@ describe("Login", () => { {}, { route: "/login", - routes: ["/login", "/login/confirm"], } ); const user = userEvent.setup(); - expect(performLogin).not.toHaveBeenCalled(); + mockedLogin.mockClear(); await user.type( screen.getByRole("textbox", { name: /Username/i }), "yoshi" @@ -130,6 +134,7 @@ describe("Login", () => { ); }); + // TODO: Complete the test it("sends the user to the location select page on login if there is more than one location", async () => { let refreshUser = (user: any) => {}; mockedLogin.mockImplementation(() => { @@ -148,11 +153,7 @@ describe("Login", () => { Login, {}, { - routeParams: { - path: "/login(/confirm)?", - exact: true, - }, - routes: ["/login", "/login/confirm"], + route: "/login", } ); @@ -167,4 +168,82 @@ describe("Login", () => { await user.type(screen.getByLabelText(/password/i), "no-tax-fraud"); await user.click(screen.getByRole("button", { name: /log in/i })); }); + + it("should render the both the username and password fields when the passwordOnSeparateScreen config is false", async () => { + mockedUseConfig.mockReturnValue({ + ...mockConfig, + passwordOnSeparateScreen: false, + }); + + renderWithRouter( + Login, + {}, + { + route: "/login", + } + ); + + expect( + screen.getByRole("textbox", { name: /username/i }) + ).toBeInTheDocument(); + expect( + screen.queryByRole("button", { name: /Continue/i }) + ).not.toBeInTheDocument(); + expect(screen.getByLabelText(/password/i)).toBeInTheDocument(); + expect(screen.getByRole("button", { name: /log in/i })).toBeInTheDocument(); + }); + + it("should not render the password field when the passwordOnSeparateScreen config is true (default)", async () => { + mockedUseConfig.mockReturnValue({ + ...mockConfig, + }); + + renderWithRouter( + Login, + {}, + { + route: "/login", + } + ); + + expect( + screen.getByRole("textbox", { name: /username/i }) + ).toBeInTheDocument(); + expect( + screen.getByRole("button", { name: /Continue/i }) + ).toBeInTheDocument(); + expect(screen.queryByLabelText(/password/i)).not.toBeInTheDocument(); + expect( + screen.queryByRole("button", { name: /log in/i }) + ).not.toBeInTheDocument(); + }); + + it("should be able to login when the passwordOnSeparateScreen config is false", async () => { + mockedLogin.mockReturnValue(Promise.resolve({ some: "data" })); + mockedUseConfig.mockReturnValue({ + ...mockConfig, + passwordOnSeparateScreen: false, + }); + const user = userEvent.setup(); + mockedLogin.mockClear(); + + renderWithRouter( + Login, + {}, + { + route: "/login", + } + ); + + await user.type( + screen.getByRole("textbox", { name: /Username/i }), + "yoshi" + ); + await user.type(screen.getByLabelText(/password/i), "no-tax-fraud"); + await user.click(screen.getByRole("button", { name: /log in/i })); + + await waitFor(() => + expect(performLogin).toHaveBeenCalledWith("yoshi", "no-tax-fraud") + ); + }); }); From 66f0e721aa83476e4fe2860480bfa174c20d8d20 Mon Sep 17 00:00:00 2001 From: Jayasanka Date: Tue, 21 Nov 2023 15:54:29 +0530 Subject: [PATCH 2/5] Format tests --- .../esm-login-app/src/login/login.test.tsx | 49 ++++++++++--------- 1 file changed, 25 insertions(+), 24 deletions(-) diff --git a/packages/apps/esm-login-app/src/login/login.test.tsx b/packages/apps/esm-login-app/src/login/login.test.tsx index 483d13b50..ecb02f6ee 100644 --- a/packages/apps/esm-login-app/src/login/login.test.tsx +++ b/packages/apps/esm-login-app/src/login/login.test.tsx @@ -183,14 +183,15 @@ describe("Login", () => { } ); - expect( - screen.getByRole("textbox", { name: /username/i }) - ).toBeInTheDocument(); - expect( - screen.queryByRole("button", { name: /Continue/i }) - ).not.toBeInTheDocument(); - expect(screen.getByLabelText(/password/i)).toBeInTheDocument(); - expect(screen.getByRole("button", { name: /log in/i })).toBeInTheDocument(); + const usernameInput = screen.queryByRole("textbox", { name: /username/i }); + const continueButton = screen.queryByRole("button", { name: /Continue/i }); + const passwordInput = screen.queryByLabelText(/password/i); + const loginButton = screen.queryByRole("button", { name: /log in/i }); + + expect(usernameInput).toBeInTheDocument(); + expect(continueButton).not.toBeInTheDocument(); + expect(passwordInput).toBeInTheDocument(); + expect(loginButton).toBeInTheDocument(); }); it("should not render the password field when the passwordOnSeparateScreen config is true (default)", async () => { @@ -206,16 +207,15 @@ describe("Login", () => { } ); - expect( - screen.getByRole("textbox", { name: /username/i }) - ).toBeInTheDocument(); - expect( - screen.getByRole("button", { name: /Continue/i }) - ).toBeInTheDocument(); - expect(screen.queryByLabelText(/password/i)).not.toBeInTheDocument(); - expect( - screen.queryByRole("button", { name: /log in/i }) - ).not.toBeInTheDocument(); + const usernameInput = screen.queryByRole("textbox", { name: /username/i }); + const continueButton = screen.queryByRole("button", { name: /Continue/i }); + const passwordInput = screen.queryByLabelText(/password/i); + const loginButton = screen.queryByRole("button", { name: /log in/i }); + + expect(usernameInput).toBeInTheDocument(); + expect(continueButton).toBeInTheDocument(); + expect(passwordInput).not.toBeInTheDocument(); + expect(loginButton).not.toBeInTheDocument(); }); it("should be able to login when the passwordOnSeparateScreen config is false", async () => { @@ -235,12 +235,13 @@ describe("Login", () => { } ); - await user.type( - screen.getByRole("textbox", { name: /Username/i }), - "yoshi" - ); - await user.type(screen.getByLabelText(/password/i), "no-tax-fraud"); - await user.click(screen.getByRole("button", { name: /log in/i })); + const usernameInput = screen.getByRole("textbox", { name: /username/i }); + const passwordInput = screen.getByLabelText(/password/i); + const loginButton = screen.getByRole("button", { name: /log in/i }); + + await user.type(usernameInput, "yoshi"); + await user.type(passwordInput, "no-tax-fraud"); + await user.click(loginButton); await waitFor(() => expect(performLogin).toHaveBeenCalledWith("yoshi", "no-tax-fraud") From 53145eb9749325a63da2fcf38795d027fc27eda0 Mon Sep 17 00:00:00 2001 From: Jayasanka Date: Thu, 23 Nov 2023 13:14:43 +0530 Subject: [PATCH 3/5] Add hidden input fields for password managers --- .../src/login/login.component.tsx | 24 ++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/packages/apps/esm-login-app/src/login/login.component.tsx b/packages/apps/esm-login-app/src/login/login.component.tsx index 2652bcb88..c1e46d191 100644 --- a/packages/apps/esm-login-app/src/login/login.component.tsx +++ b/packages/apps/esm-login-app/src/login/login.component.tsx @@ -233,6 +233,17 @@ const Login: React.FC = () => { autoFocus required /> + {/* For password managers */} + {passwordOnSeparateScreen && ( + + )} {passwordOnSeparateScreen && (