Merge pull request #376 from huwshimi/oidc-next-url

WD-13860 - feat: return to URL that initiated login

ui/src/App.tsx

@@ -13,9 +13,13 @@ import SchemaList from "pages/schemas/SchemaList";
 import Layout from "components/Layout/Layout";
 import { queryKeys } from "util/queryKeys";
 import { axiosInstance } from "./api/axios";
+import { urls } from "urls";
+import { useNext } from "util/useNext";
 
 const App: FC = () => {
   const queryClient = useQueryClient();
+  // Redirect to the ?next=/... URL returned by the authentication step.
+  useNext();
 
   useRef(
     axiosInstance.interceptors.response.use(
@@ -43,14 +47,17 @@ const App: FC = () => {
 
   return (
     <Routes>
-      <Route path="/" element={<Layout />}>
-        <Route path="/" element={<Navigate to="/provider" replace={true} />} />
-        <Route path="/provider" element={<ProviderList />} />
-        <Route path="/client" element={<ClientList />} />
-        <Route path="/identity" element={<IdentityList />} />
-        <Route path="/schema" element={<SchemaList />} />
+      <Route path={urls.index} element={<Layout />}>
         <Route
-          path="/*"
+          path={urls.index}
+          element={<Navigate to={urls.providers.index} replace={true} />}
+        />
+        <Route path={urls.providers.index} element={<ProviderList />} />
+        <Route path={urls.clients.index} element={<ClientList />} />
+        <Route path={urls.identities.index} element={<IdentityList />} />
+        <Route path={urls.schemas.index} element={<SchemaList />} />
+        <Route
+          path={`${urls.index}*`}
           element={
             <ReBACAdmin
               asidePanelId="app-layout"

ui/src/components/Login/Login.test.tsx

@@ -21,3 +21,13 @@ test("displays the login button", () => {
     screen.getByRole("link", { name: "Sign in to Identity platform" }),
   ).toBeInTheDocument();
 });
+
+test("passes the current path to the 'next' param", () => {
+  renderComponent(<Login />, { url: "/test" });
+  expect(
+    screen
+      .getByRole("link", { name: "Sign in to Identity platform" })
+      .getAttribute("href")
+      ?.endsWith("?next=%2ftest"),
+  );
+});

ui/src/components/Login/Login.tsx

@@ -10,13 +10,16 @@ import { FC, ReactNode } from "react";
 import { SITE_NAME } from "consts";
 import { Label } from "./types";
 import { appendAPIBasePath } from "util/basePaths";
+import { useLocation } from "react-router-dom";
+import { getURLKey } from "util/getURLKey";
 
 type Props = {
   isLoading?: boolean;
   error?: string;
 };
 
 const Login: FC<Props> = ({ error, isLoading }) => {
+  const location = useLocation();
   let loginContent: ReactNode;
   if (isLoading) {
     loginContent = <Spinner />;
@@ -32,14 +35,17 @@ const Login: FC<Props> = ({ error, isLoading }) => {
       />
     );
   } else {
+    const path = getURLKey(location.pathname);
     loginContent = (
       <Button
         appearance={
           isLoading ? ButtonAppearance.DEFAULT : ButtonAppearance.POSITIVE
         }
         disabled={isLoading}
         element="a"
-        href={appendAPIBasePath(authURLs.login)}
+        href={[appendAPIBasePath(authURLs.login), path ? `next=${path}` : null]
+          .filter(Boolean)
+          .join("?")}
       >
         Sign in to {SITE_NAME}
       </Button>

ui/src/index.tsx

@@ -5,6 +5,7 @@ import App from "./App";
 import "./sass/styles.scss";
 import { NotificationProvider } from "@canonical/react-components";
 import { basePath } from "util/basePaths";
+import { removeTrailingSlash } from "util/removeTrailingSlash";
 
 const queryClient = new QueryClient({
   defaultOptions: {
@@ -21,7 +22,7 @@ const rootElement = document.getElementById("app");
 if (!rootElement) throw new Error("Failed to find the root element");
 const root = createRoot(rootElement);
 root.render(
-  <Router basename={basePath}>
+  <Router basename={removeTrailingSlash(basePath)}>
     <QueryClientProvider client={queryClient}>
       <NotificationProvider>
         <App />

ui/src/test/ComponentProviders.tsx

@@ -1,15 +1,31 @@
 import { QueryClient } from "@tanstack/react-query";
 import { QueryClientProvider } from "@tanstack/react-query";
-import { type PropsWithChildren } from "react";
-import type { RouteObject } from "react-router-dom";
-import { createBrowserRouter, RouterProvider } from "react-router-dom";
+import { useEffect, type PropsWithChildren } from "react";
+import type { Location, RouteObject } from "react-router-dom";
+import {
+  createBrowserRouter,
+  RouterProvider,
+  useLocation,
+} from "react-router-dom";
 
 export type ComponentProps = {
   path: string;
   routeChildren?: RouteObject[];
   queryClient?: QueryClient;
+  setLocation?: (location: Location) => void;
 } & PropsWithChildren;
 
+const Wrapper = ({
+  children,
+  setLocation,
+}: PropsWithChildren & { setLocation?: (location: Location) => void }) => {
+  const location = useLocation();
+  useEffect(() => {
+    setLocation?.(location);
+  }, [location, setLocation]);
+  return children;
+};
+
 const ComponentProviders = ({
   children,
   routeChildren,
@@ -21,17 +37,18 @@ const ComponentProviders = ({
       },
     },
   }),
+  setLocation,
 }: ComponentProps) => {
   const router = createBrowserRouter([
     {
       path,
-      element: children,
+      element: <Wrapper setLocation={setLocation}>{children}</Wrapper>,
       children: routeChildren,
     },
     {
       // Capture other paths to prevent warnings when navigating in tests.
       path: "*",
-      element: <span />,
+      element: <Wrapper setLocation={setLocation} />,
     },
   ]);
   return (

ui/src/test/utils.tsx

@@ -1,5 +1,5 @@
 import { QueryClient } from "@tanstack/react-query";
-import { render } from "@testing-library/react";
+import { render, renderHook } from "@testing-library/react";
 
 import ComponentProviders from "./ComponentProviders";
 import type { ComponentProps } from "./ComponentProviders";
@@ -9,6 +9,7 @@ type Options = {
   path?: string;
   routeChildren?: ComponentProps["routeChildren"];
   queryClient?: QueryClient;
+  setLocation?: ComponentProps["setLocation"];
 };
 
 export const changeURL = (url: string) => window.happyDOM.setURL(url);
@@ -37,6 +38,27 @@ export const renderComponent = (
         routeChildren={options?.routeChildren}
         path={options?.path ?? "*"}
         queryClient={queryClient}
+        setLocation={options?.setLocation}
+      />
+    ),
+  });
+  return { changeURL, result, queryClient };
+};
+
+export const renderWrappedHook = <Result, Props>(
+  hook: (initialProps: Props) => Result,
+  options?: Options | null,
+) => {
+  const queryClient = getQueryClient(options);
+  changeURL(options?.url ?? "/");
+  const { result } = renderHook(hook, {
+    wrapper: (props) => (
+      <ComponentProviders
+        {...props}
+        routeChildren={options?.routeChildren}
+        path={options?.path ?? "*"}
+        queryClient={queryClient}
+        setLocation={options?.setLocation}
       />
     ),
   });

ui/src/urls.ts

@@ -0,0 +1,21 @@
+import { urls as rebacURLs } from "@canonical/rebac-admin";
+
+const rebacAdminURLS = rebacURLs("/");
+
+export const urls = {
+  clients: {
+    index: "/client",
+  },
+  groups: rebacAdminURLS.groups,
+  identities: {
+    index: "/identity",
+  },
+  index: "/",
+  providers: {
+    index: "/provider",
+  },
+  roles: rebacAdminURLS.roles,
+  schemas: {
+    index: "/schema",
+  },
+};

ui/src/util/basePaths.spec.ts

@@ -21,6 +21,12 @@ describe("calculateBasePath", () => {
     expect(result).toBe("/ui/");
   });
 
+  it("resolves with ui path without trailing slash", () => {
+    vi.stubGlobal("location", { pathname: "/ui" });
+    const result = calculateBasePath();
+    expect(result).toBe("/ui/");
+  });
+
   it("resolves with ui path and discards detail page location", () => {
     vi.stubGlobal("location", { pathname: "/ui/foo/bar" });
     const result = calculateBasePath();
@@ -44,6 +50,12 @@ describe("calculateBasePath", () => {
     const result = calculateBasePath();
     expect(result).toBe("/");
   });
+
+  it("resolves with root path for partial ui substrings", () => {
+    vi.stubGlobal("location", { pathname: "/prefix/uipartial" });
+    const result = calculateBasePath();
+    expect(result).toBe("/");
+  });
 });
 
 describe("appendBasePath", () => {

ui/src/util/basePaths.ts

@@ -1,11 +1,12 @@
+import { removeTrailingSlash } from "util/removeTrailingSlash";
 type BasePath = `/${string}`;
 
 export const calculateBasePath = (): BasePath => {
   const path = window.location.pathname;
   // find first occurrence of /ui/ and return the string before it
-  const basePath = path.match(/(.*\/ui\/)/);
+  const basePath = path.match(/(.*\/ui(?:\/|$))/);
   if (basePath) {
-    return basePath[0] as BasePath;
+    return `${removeTrailingSlash(basePath[0])}/` as BasePath;
   }
   return "/";
 };
@@ -14,7 +15,7 @@ export const basePath: BasePath = calculateBasePath();
 export const apiBasePath: BasePath = `${basePath}../api/v0/`;
 
 export const appendBasePath = (path: string) =>
-  `${basePath.replace(/\/$/, "")}/${path.replace(/^\//, "")}`;
+  `${removeTrailingSlash(basePath)}/${path.replace(/^\//, "")}`;
 
 export const appendAPIBasePath = (path: string) =>
-  `${apiBasePath.replace(/\/$/, "")}/${path.replace(/^\//, "")}`;
+  `${removeTrailingSlash(apiBasePath)}/${path.replace(/^\//, "")}`;

ui/src/util/getDomain.test.ts

@@ -0,0 +1,9 @@
+import { getDomain } from "./getDomain";
+
+test("handles extracting domain", () => {
+  expect(getDomain("http://example.com/?next=/here#hash")).toBe("example.com");
+});
+
+test("handles no domain", () => {
+  expect(getDomain("/a/path")).toBeUndefined();
+});

ui/src/util/getDomain.ts

@@ -0,0 +1,2 @@
+// Extract the domain from the URL.
+export const getDomain = (url: string) => url.match(/(?<=.+:\/\/)[^/]+/)?.[0];

ui/src/util/getFullPath.test.ts

@@ -0,0 +1,25 @@
+import { getFullPath } from "./getFullPath";
+
+vi.mock("./basePaths", async () => {
+  const actual = await vi.importActual("./basePaths");
+  return {
+    ...actual,
+    basePath: "/example/ui/",
+  };
+});
+
+test("handles paths with query and hash", () => {
+  expect(getFullPath("http://example.com/?next=/here#hash")).toBe(
+    "/?next=/here#hash",
+  );
+});
+
+test("removes base", () => {
+  expect(
+    getFullPath("http://example.com/example/ui/roles/?next=/here#hash", true),
+  ).toBe("/roles/?next=/here#hash");
+});
+
+test("handles no path", () => {
+  expect(getFullPath("http://example.com/")).toBeUndefined();
+});

ui/src/util/getFullPath.ts

@@ -0,0 +1,10 @@
+import { basePath } from "./basePaths";
+import { removeTrailingSlash } from "./removeTrailingSlash";
+
+// Extract the path from the URL including query params, hash etc.
+export const getFullPath = (url: string, removeBase = false) => {
+  const path = url.match(/(?<!\/)\/(?!\/).+$/)?.[0];
+  return removeBase
+    ? path?.replace(new RegExp(`^${removeTrailingSlash(basePath)}`), "")
+    : path;
+};

ui/src/util/getURLKey.test.ts

@@ -0,0 +1,17 @@
+import { urls } from "urls";
+import { getURLKey } from "./getURLKey";
+test("handles indexes", () => {
+  expect(getURLKey(urls.clients.index)).toBe("clients");
+});
+
+test("handles named paths", () => {
+  expect(getURLKey(urls.roles.add)).toBe("roles.add");
+});
+
+test("handles functions", () => {
+  expect(getURLKey(urls.roles.edit({ id: "role1" }))).toBe("roles.edit");
+});
+
+test("handles paths that don't exist", () => {
+  expect(getURLKey("/nothing/here")).toBeNull();
+});

ui/src/util/getURLKey.ts

@@ -0,0 +1,43 @@
+import { matchPath } from "react-router-dom";
+import { urls } from "urls";
+
+export type URLS = {
+  [key: string]:
+    | string
+    | URLS
+    | ((args: unknown, relativeTo?: string) => string);
+};
+
+const findPath = (
+  sections: URLS,
+  pathname: string,
+  keyPath = "",
+): string | null => {
+  const entries = Object.entries(sections);
+  let path: string | null = null;
+  for (const entry of entries) {
+    const [key, section] = entry;
+    const thisPath = [keyPath, key].filter(Boolean).join(".");
+    if (typeof section === "string" && section === pathname) {
+      path = thisPath;
+      break;
+    } else if (
+      typeof section === "function" &&
+      !!matchPath(section(null), pathname)
+    ) {
+      path = thisPath;
+      break;
+    } else if (typeof section === "object") {
+      const matchingPath = findPath(section, pathname, thisPath);
+      if (matchingPath) {
+        path = matchingPath;
+        break;
+      }
+    }
+  }
+  // Don't expose the index key. The index is handled when the path is returned
+  // in the useNext hook.
+  return path ? path.replace(/\.index$/, "") : null;
+};
+
+export const getURLKey = (pathname: string) => findPath(urls as URLS, pathname);