From e2f0b4230d533ccfbc962e995ff1f71eacad5d60 Mon Sep 17 00:00:00 2001 From: Huw Wilkins Date: Tue, 11 Jun 2024 15:44:35 +1000 Subject: [PATCH] feat: display OIDC login form --- .../IdentityProviderForm.tsx | 8 +++- .../LogIn/IdentityProviderForm/index.ts | 1 + .../LogIn/IdentityProviderForm/types.ts | 3 ++ src/components/LogIn/LogIn.test.tsx | 21 ++++++++++- src/components/LogIn/LogIn.tsx | 22 ++++++++--- .../LogIn/OIDCForm/OIDCForm.test.tsx | 17 +++++++++ src/components/LogIn/OIDCForm/OIDCForm.tsx | 22 +++++++++++ src/components/LogIn/OIDCForm/index.ts | 2 + src/components/LogIn/OIDCForm/types.ts | 3 ++ src/index.test.tsx | 21 +++++++++++ src/index.tsx | 2 +- src/juju/jimm/thunks.ts | 3 +- src/store/middleware/model-poller.test.ts | 37 +++++++++++++++++++ src/store/middleware/model-poller.ts | 9 +++++ 14 files changed, 161 insertions(+), 10 deletions(-) create mode 100644 src/components/LogIn/IdentityProviderForm/types.ts create mode 100644 src/components/LogIn/OIDCForm/OIDCForm.test.tsx create mode 100644 src/components/LogIn/OIDCForm/OIDCForm.tsx create mode 100644 src/components/LogIn/OIDCForm/index.ts create mode 100644 src/components/LogIn/OIDCForm/types.ts diff --git a/src/components/LogIn/IdentityProviderForm/IdentityProviderForm.tsx b/src/components/LogIn/IdentityProviderForm/IdentityProviderForm.tsx index 7e42611c2..df7e5586a 100644 --- a/src/components/LogIn/IdentityProviderForm/IdentityProviderForm.tsx +++ b/src/components/LogIn/IdentityProviderForm/IdentityProviderForm.tsx @@ -6,6 +6,8 @@ import type { RootState } from "store/store"; import { Label } from "../types"; +import { TestId } from "./types"; + type Props = { userIsLoggedIn: boolean; }; @@ -21,7 +23,11 @@ const IdentityProviderForm = ({ userIsLoggedIn }: Props) => { }); return visitURL ? ( - + {Label.LOGIN_TO_DASHBOARD} ) : ( diff --git a/src/components/LogIn/IdentityProviderForm/index.ts b/src/components/LogIn/IdentityProviderForm/index.ts index 67844a944..4c0453ec4 100644 --- a/src/components/LogIn/IdentityProviderForm/index.ts +++ b/src/components/LogIn/IdentityProviderForm/index.ts @@ -1 +1,2 @@ export { default } from "./IdentityProviderForm"; +export { TestId as IdentityProviderFormTestId } from "./types"; diff --git a/src/components/LogIn/IdentityProviderForm/types.ts b/src/components/LogIn/IdentityProviderForm/types.ts new file mode 100644 index 000000000..a7461b5da --- /dev/null +++ b/src/components/LogIn/IdentityProviderForm/types.ts @@ -0,0 +1,3 @@ +export enum TestId { + CANDID_LOGIN = "candid-login", +} diff --git a/src/components/LogIn/LogIn.test.tsx b/src/components/LogIn/LogIn.test.tsx index fbdc99cab..21417d708 100644 --- a/src/components/LogIn/LogIn.test.tsx +++ b/src/components/LogIn/LogIn.test.tsx @@ -7,7 +7,9 @@ import { configFactory, generalStateFactory } from "testing/factories/general"; import { rootStateFactory } from "testing/factories/root"; import { renderComponent } from "testing/utils"; +import { IdentityProviderFormTestId } from "./IdentityProviderForm"; import LogIn from "./LogIn"; +import { OIDCFormTestId } from "./OIDCForm"; import { ErrorResponse, Label } from "./types"; describe("LogIn", () => { @@ -49,7 +51,24 @@ describe("LogIn", () => { renderComponent(App content, { state }); expect( screen.getByRole("link", { name: Label.LOGIN_TO_DASHBOARD }), - ).toBeInTheDocument(); + ).toHaveAttribute("data-testid", IdentityProviderFormTestId.CANDID_LOGIN); + expect( + screen.queryByRole("textbox", { name: "Username" }), + ).not.toBeInTheDocument(); + }); + + it("renders an OIDC login UI if the user is not logged in", () => { + const state = rootStateFactory.build({ + general: generalStateFactory.withConfig().build({ + config: configFactory.build({ + authMethod: AuthMethod.OIDC, + }), + }), + }); + renderComponent(App content, { state }); + expect( + screen.getByRole("link", { name: Label.LOGIN_TO_DASHBOARD }), + ).toHaveAttribute("data-testid", OIDCFormTestId.OIDC_LOGIN); expect( screen.queryByRole("textbox", { name: "Username" }), ).not.toBeInTheDocument(); diff --git a/src/components/LogIn/LogIn.tsx b/src/components/LogIn/LogIn.tsx index 63fc628bd..a59f527e2 100644 --- a/src/components/LogIn/LogIn.tsx +++ b/src/components/LogIn/LogIn.tsx @@ -1,4 +1,4 @@ -import type { PropsWithChildren } from "react"; +import type { PropsWithChildren, ReactNode } from "react"; import { useEffect, useRef } from "react"; import reactHotToast from "react-hot-toast"; import { useSelector } from "react-redux"; @@ -20,6 +20,7 @@ import { useAppSelector } from "store/store"; import "./_login.scss"; import IdentityProviderForm from "./IdentityProviderForm"; +import OIDCForm from "./OIDCForm"; import UserPassForm from "./UserPassForm"; import { ErrorResponse, Label } from "./types"; @@ -69,6 +70,19 @@ export default function LogIn({ children }: PropsWithChildren) { }); }, [visitURLs]); + let form: ReactNode = null; + switch (config?.authMethod) { + case AuthMethod.CANDID: + form = ; + break; + case AuthMethod.OIDC: + form = ; + break; + default: + form = ; + break; + } + return ( <> {!userIsLoggedIn && ( @@ -76,11 +90,7 @@ export default function LogIn({ children }: PropsWithChildren) {
- {config?.authMethod === AuthMethod.CANDID ? ( - - ) : ( - - )} + {form} {generateErrorMessage(loginError)}
diff --git a/src/components/LogIn/OIDCForm/OIDCForm.test.tsx b/src/components/LogIn/OIDCForm/OIDCForm.test.tsx new file mode 100644 index 000000000..f98558616 --- /dev/null +++ b/src/components/LogIn/OIDCForm/OIDCForm.test.tsx @@ -0,0 +1,17 @@ +import { screen } from "@testing-library/react"; + +import { endpoints } from "juju/jimm/api"; +import { renderComponent } from "testing/utils"; + +import { Label } from "../types"; + +import OIDCForm from "./OIDCForm"; + +describe("OIDCForm", () => { + it("should render a login link", () => { + renderComponent(); + expect( + screen.getByRole("link", { name: Label.LOGIN_TO_DASHBOARD }), + ).toHaveAttribute("href", endpoints.login); + }); +}); diff --git a/src/components/LogIn/OIDCForm/OIDCForm.tsx b/src/components/LogIn/OIDCForm/OIDCForm.tsx new file mode 100644 index 000000000..eabcb8949 --- /dev/null +++ b/src/components/LogIn/OIDCForm/OIDCForm.tsx @@ -0,0 +1,22 @@ +import { Button } from "@canonical/react-components"; + +import { endpoints } from "juju/jimm/api"; + +import { Label } from "../types"; + +import { TestId } from "./types"; + +const OIDCForm = () => { + return ( + + ); +}; + +export default OIDCForm; diff --git a/src/components/LogIn/OIDCForm/index.ts b/src/components/LogIn/OIDCForm/index.ts new file mode 100644 index 000000000..44fa8612d --- /dev/null +++ b/src/components/LogIn/OIDCForm/index.ts @@ -0,0 +1,2 @@ +export { default } from "./OIDCForm"; +export { TestId as OIDCFormTestId } from "./types"; diff --git a/src/components/LogIn/OIDCForm/types.ts b/src/components/LogIn/OIDCForm/types.ts new file mode 100644 index 000000000..5462a14ee --- /dev/null +++ b/src/components/LogIn/OIDCForm/types.ts @@ -0,0 +1,3 @@ +export enum TestId { + OIDC_LOGIN = "oidc-login", +} diff --git a/src/index.test.tsx b/src/index.test.tsx index 01115d5db..603333b63 100644 --- a/src/index.test.tsx +++ b/src/index.test.tsx @@ -216,9 +216,30 @@ describe("renderApp", () => { expect(dispatch).toHaveBeenCalledWith({ type: "connectAndStartPolling" }); }); + it("connects when using OIDC", async () => { + // Mock the result of the thunk a normal action so that it can be tested + // for. This is necessary because we don't have a full store set up which + // can dispatch thunks (and we don't need to handle the thunk, just know it + // was dispatched). + vi.spyOn(appThunks, "connectAndStartPolling").mockImplementation( + vi.fn().mockReturnValue({ type: "connectAndStartPolling" }), + ); + const dispatch = vi + .spyOn(storeModule.default, "dispatch") + .mockImplementation(vi.fn().mockResolvedValue({ catch: vi.fn() })); + const config = configFactory.build({ + controllerAPIEndpoint: "wss://example.com/api", + isJuju: false, + }); + window.jujuDashboardConfig = config; + renderApp(); + expect(dispatch).toHaveBeenCalledWith({ type: "connectAndStartPolling" }); + }); + it("renders the app", async () => { const config = configFactory.build({ controllerAPIEndpoint: "wss://example.com/api", + isJuju: true, }); window.jujuDashboardConfig = config; renderApp(); diff --git a/src/index.tsx b/src/index.tsx index 7868b516f..359dbfeaa 100644 --- a/src/index.tsx +++ b/src/index.tsx @@ -146,7 +146,7 @@ function bootstrap() { reduxStore.dispatch(generalActions.storeConfig(config)); reduxStore.dispatch(generalActions.storeVersion(appVersion)); - if (config.authMethod === AuthMethod.CANDID) { + if ([AuthMethod.CANDID, AuthMethod.OIDC].includes(config.authMethod)) { // If using Candid authentication then try and connect automatically // If not then wait for the login UI to trigger this reduxStore diff --git a/src/juju/jimm/thunks.ts b/src/juju/jimm/thunks.ts index 9a3b51dfb..de6e57018 100644 --- a/src/juju/jimm/thunks.ts +++ b/src/juju/jimm/thunks.ts @@ -1,3 +1,4 @@ +import type { LoginResult } from "@canonical/jujulib/dist/api/facades/admin/AdminV3"; import { createAsyncThunk } from "@reduxjs/toolkit"; import type { RootState } from "store/store"; @@ -29,7 +30,7 @@ export const logout = createAsyncThunk< Get the authenticated user from the JIMM API. */ export const whoami = createAsyncThunk< - void, + LoginResult | null, void, { state: RootState; diff --git a/src/store/middleware/model-poller.test.ts b/src/store/middleware/model-poller.test.ts index dc53df830..e8589db87 100644 --- a/src/store/middleware/model-poller.test.ts +++ b/src/store/middleware/model-poller.test.ts @@ -192,6 +192,43 @@ describe("model poller", () => { ); }); + it("stops when not logged in and is using OIDC", async () => { + vi.spyOn(fakeStore, "getState").mockReturnValue(storeState); + vi.spyOn(fakeStore, "dispatch").mockReturnValue({ type: "whoami" }); + const loginWithBakerySpy = vi.spyOn(jujuModule, "loginWithBakery"); + await runMiddleware( + appActions.connectAndPollControllers({ + controllers: [[wsControllerURL, undefined, AuthMethod.OIDC]], + isJuju: true, + poll: 0, + }), + ); + expect(loginWithBakerySpy).not.toHaveBeenCalled(); + }); + + it("continues when not logged in and is using OIDC", async () => { + vi.spyOn(fakeStore, "getState").mockReturnValue(storeState); + vi.spyOn(fakeStore, "dispatch").mockReturnValue({ + type: "whoami", + payload: { user: "user-test@external" }, + }); + const loginWithBakerySpy = vi + .spyOn(jujuModule, "loginWithBakery") + .mockImplementation(async () => ({ + conn, + intervalId, + juju, + })); + await runMiddleware( + appActions.connectAndPollControllers({ + controllers: [[wsControllerURL, undefined, AuthMethod.OIDC]], + isJuju: true, + poll: 0, + }), + ); + expect(loginWithBakerySpy).toHaveBeenCalled(); + }); + it("fetches and stores data", async () => { const fetchControllerList = vi.spyOn(jujuModule, "fetchControllerList"); conn.facades.modelManager.listModels.mockResolvedValue({ diff --git a/src/store/middleware/model-poller.ts b/src/store/middleware/model-poller.ts index 6d9dd66a8..b166d02e3 100644 --- a/src/store/middleware/model-poller.ts +++ b/src/store/middleware/model-poller.ts @@ -1,4 +1,5 @@ import type { Client } from "@canonical/jujulib"; +import { unwrapResult } from "@reduxjs/toolkit"; import * as Sentry from "@sentry/browser"; import { isAction, type Middleware } from "redux"; @@ -12,6 +13,7 @@ import { setModelSharingPermissions, } from "juju/api"; import { JIMMRelation } from "juju/jimm/JIMMV4"; +import { whoami } from "juju/jimm/thunks"; import type { ConnectionWithFacades } from "juju/types"; import { actions as appActions, thunks as appThunks } from "store/app"; import { actions as generalActions } from "store/general"; @@ -77,6 +79,13 @@ export const modelPollerMiddleware: Middleware< let juju: Client | undefined; let error: unknown; let intervalId: number | null | undefined; + if (authMethod === AuthMethod.OIDC) { + const whoamiResponse = await reduxStore.dispatch(whoami()); + const user = unwrapResult(whoamiResponse); + if (!user) { + return; + } + } try { ({ conn, error, juju, intervalId } = await loginWithBakery( wsControllerURL,