Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions changelog/7747-oauth-clients-list-page.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
type: Added
description: Added OAuth API clients list page with paginated table and nav entry
pr: 7747
labels: []
15 changes: 5 additions & 10 deletions clients/admin-ui/src/features/common/ClipboardButton.tsx
Original file line number Diff line number Diff line change
@@ -1,10 +1,4 @@
import {
Button,
ButtonProps,
Icons,
Tooltip,
useChakraClipboard as useClipboard,
} from "fidesui";
import { Button, ButtonProps, Icons, Tooltip } from "fidesui";
import React, { useState } from "react";

enum TooltipText {
Expand All @@ -13,12 +7,13 @@ enum TooltipText {
}

const useClipboardButton = (copyText: string) => {
const { onCopy } = useClipboard(copyText);
const [tooltipText, setTooltipText] = useState(TooltipText.COPY);

const handleClick = () => {
setTooltipText(TooltipText.COPIED);
onCopy();
navigator.clipboard.writeText(copyText).then(
() => setTooltipText(TooltipText.COPIED),
() => setTooltipText(TooltipText.COPY),
);
};

return {
Expand Down
1 change: 1 addition & 0 deletions clients/admin-ui/src/features/common/api.slice.ts
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ export const baseApi = createApi({
"User",
"Configuration Settings",
"TCF Purpose Override",
"OAuth Client",
"OpenID Provider",
"Chat Provider Config",
"Taxonomy",
Expand Down
11 changes: 11 additions & 0 deletions clients/admin-ui/src/features/common/nav/nav-config.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -293,6 +293,17 @@ export const NAV_CONFIG: NavConfigGroup[] = [
ScopeRegistryEnum.USER_READ,
],
},
{
title: "API clients",
path: routes.API_CLIENTS_ROUTE,
scopes: [ScopeRegistryEnum.CLIENT_READ],
},
{
title: "API client detail",
path: routes.API_CLIENT_DETAIL_ROUTE,
hidden: true,
scopes: [ScopeRegistryEnum.CLIENT_READ],
},
{
title: "User detail",
path: routes.USER_DETAIL_ROUTE,
Expand Down
2 changes: 2 additions & 0 deletions clients/admin-ui/src/features/common/nav/routes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,8 @@ export const PROPERTIES_ROUTE = "/properties";
export const ADD_PROPERTY_ROUTE = "/properties/add-property";
export const EDIT_PROPERTY_ROUTE = "/properties/[id]";

export const API_CLIENTS_ROUTE = "/api-clients";
export const API_CLIENT_DETAIL_ROUTE = "/api-clients/[id]";
export const USER_MANAGEMENT_ROUTE = "/user-management";
export const USER_PROFILE_ROUTE = "/user-management/profile/[id]";
export const USER_DETAIL_ROUTE = "/user-management/profile/[id]";
Expand Down
184 changes: 184 additions & 0 deletions clients/admin-ui/src/features/oauth/OAuthClientsTable.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,184 @@
import { render, screen } from "@testing-library/react";

import OAuthClientsList from "./OAuthClientsTable";

// --- Module mocks ---

const mockUseHasPermission = jest.fn();
jest.mock("~/features/common/Restrict", () => ({
useHasPermission: () => mockUseHasPermission(),
}));

const mockUseListOAuthClientsQuery = jest.fn();
jest.mock("./oauth-clients.slice", () => ({
useListOAuthClientsQuery: () => mockUseListOAuthClientsQuery(),
}));

// LinkCell uses NextLink which doesn't work in jsdom (NodeList.includes bug)
jest.mock("~/features/common/table/cells/LinkCell", () => ({
LinkCell: ({ href, children }: any) =>
href ? <a href={href}>{children}</a> : <span>{children}</span>,
}));

// Tooltip mock: real Tooltip only renders content in a portal on hover,
// so we use a lightweight stand-in that exposes the title as a DOM attribute
// for static assertions.
const MockTooltip = ({ title, children }: any) => (
<span data-tooltip={title}>{children}</span>
);
MockTooltip.displayName = "MockTooltip";

jest.mock(
"fidesui",
() =>
new Proxy(jest.requireActual("fidesui"), {
get(target, prop) {
if (prop === "Tooltip") {
return MockTooltip;
}
return target[prop as keyof typeof target];
},
}),
);

// --- Helpers ---

const makeClient = (overrides = {}) => ({
client_id: "abc123",
name: "My Client",
description: "A test client",
scopes: ["client:create", "client:read"],
...overrides,
});

const renderList = () => render(<OAuthClientsList />);

// --- Tests ---

describe("OAuthClientsList", () => {
beforeEach(() => {
jest.clearAllMocks();
Object.defineProperty(navigator, "clipboard", {
value: { writeText: jest.fn() },
configurable: true,
});
mockUseHasPermission.mockReturnValue(true);
mockUseListOAuthClientsQuery.mockReturnValue({
data: { items: [makeClient()], total: 1, page: 1, size: 25 },
isLoading: false,
});
});

describe("list item rendering", () => {
it("renders the client name", () => {
renderList();
expect(screen.getByText("My Client")).toBeInTheDocument();
});

it("renders the client ID in monospace", () => {
renderList();
expect(screen.getByText("abc123")).toBeInTheDocument();
});

it("renders the scope count tag", () => {
renderList();
expect(screen.getByText("2 scopes")).toBeInTheDocument();
});

it("renders the description when present", () => {
renderList();
expect(screen.getByText("A test client")).toBeInTheDocument();
});

it("does not render a description section when absent", () => {
mockUseListOAuthClientsQuery.mockReturnValue({
data: { items: [makeClient({ description: null })], total: 1 },
isLoading: false,
});
renderList();
expect(screen.queryByText("A test client")).not.toBeInTheDocument();
});

it("renders a copy button for the client ID", () => {
renderList();
expect(screen.getByTestId("clipboard-btn")).toBeInTheDocument();
});

it("shows 'Unnamed' when client has no name", () => {
mockUseListOAuthClientsQuery.mockReturnValue({
data: { items: [makeClient({ name: null })], total: 1 },
isLoading: false,
});
renderList();
expect(screen.getByText("Unnamed")).toBeInTheDocument();
});
});

describe("empty state", () => {
it("renders empty state text when there are no clients", () => {
mockUseListOAuthClientsQuery.mockReturnValue({
data: { items: [], total: 0 },
isLoading: false,
});
renderList();
expect(screen.getByText(/No API clients yet/)).toBeInTheDocument();
});
});

describe("loading state", () => {
it("renders without crashing while loading", () => {
mockUseListOAuthClientsQuery.mockReturnValue({
data: undefined,
isLoading: true,
});
expect(() => renderList()).not.toThrow();
});
});

describe("client name link — with CLIENT_UPDATE permission", () => {
it("renders the name as a link to the detail page", () => {
mockUseHasPermission.mockReturnValue(true);
renderList();
const link = screen.getByText("My Client").closest("a");
expect(link).toHaveAttribute("href", "/api-clients/abc123");
});

it("does not show a permission tooltip", () => {
mockUseHasPermission.mockReturnValue(true);
renderList();
const tooltip = screen.getByText("My Client").closest("[data-tooltip]");
expect(tooltip?.getAttribute("data-tooltip")).toBeFalsy();
});
});

describe("client name — without CLIENT_UPDATE permission", () => {
beforeEach(() => mockUseHasPermission.mockReturnValue(false));

it("renders the name as plain text (no link)", () => {
renderList();
expect(screen.getByText("My Client").closest("a")).toBeNull();
});

it("shows a tooltip explaining the permission requirement", () => {
renderList();
const tooltip = screen.getByText("My Client").closest("[data-tooltip]");
expect(tooltip?.getAttribute("data-tooltip")).toMatch(/permission/i);
});
});

describe("pagination", () => {
it("always renders the pagination component", () => {
renderList();
expect(screen.getByText(/Total 1 items/)).toBeInTheDocument();
});

it("shows the total item count", () => {
mockUseListOAuthClientsQuery.mockReturnValue({
data: { items: [makeClient()], total: 42 },
isLoading: false,
});
renderList();
expect(screen.getByText(/Total 42 items/)).toBeInTheDocument();
});
});
});
141 changes: 141 additions & 0 deletions clients/admin-ui/src/features/oauth/OAuthClientsTable.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,141 @@
import { Flex, List, Pagination, Tag, Tooltip, Typography } from "fidesui";
import { useState } from "react";

import ClipboardButton from "~/features/common/ClipboardButton";
import { API_CLIENTS_ROUTE } from "~/features/common/nav/routes";
import { useHasPermission } from "~/features/common/Restrict";
import { LinkCell } from "~/features/common/table/cells/LinkCell";
import {
DEFAULT_PAGE_SIZE,
DEFAULT_PAGE_SIZES,
} from "~/features/common/table/constants";
import { ClientResponse, ScopeRegistryEnum } from "~/types/api";

import { useListOAuthClientsQuery } from "./oauth-clients.slice";

const { Text } = Typography;

const ClientListItem = ({
client,
canUpdate,
}: {
client: ClientResponse;
canUpdate: boolean;
}) => {
return (
<List.Item data-testid={`client-list-item-${client.client_id}`}>
<List.Item.Meta
title={
<Flex align="center" gap={8}>
<Tooltip
title={
!canUpdate
? "You don't have permission to edit API clients."
: undefined
}
>
<span>
<LinkCell
href={
canUpdate
? `${API_CLIENTS_ROUTE}/${client.client_id}`
: undefined
}
>
{client.name ?? "Unnamed"}
</LinkCell>
</span>
</Tooltip>
<Tag>{client.scopes.length} scopes</Tag>
</Flex>
}
description={
<Flex vertical gap={4}>
<Flex align="center" gap={4}>
<Text className="font-mono text-xs text-gray-400">
{client.client_id}
</Text>
<ClipboardButton copyText={client.client_id} size="small" />
</Flex>
{client.description && (
<Text className="text-sm text-gray-700">
{client.description}
</Text>
)}
</Flex>
}
/>
</List.Item>
);
};

const useOAuthClientsList = () => {
const [page, setPage] = useState(1);
const [pageSize, setPageSize] = useState(DEFAULT_PAGE_SIZE);

const { data, isLoading, error } = useListOAuthClientsQuery({
page,
size: pageSize,
});

return {
data: data?.items ?? [],
total: data?.total ?? 0,
isLoading,
error,
page,
pageSize,
setPage,
setPageSize,
};
};
Comment on lines +72 to +91
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Custom pagination state bypasses URL sync hook

The useOAuthClientsList hook manages page and pageSize with plain useState, but the project provides a dedicated usePagination (or its Ant Design wrapper useAntPagination) hook in src/features/common/pagination/ that synchronises pagination state with URL query parameters. Using local state means the current page is lost on refresh or navigation, and back/forward browser history won't work correctly.

Consider using useAntPagination from ~/features/common/pagination/useAntPagination:

import { useAntPagination } from "~/features/common/pagination/useAntPagination";

const useOAuthClientsList = () => {
  const { pageIndex, pageSize, paginationProps } = useAntPagination();

  const { data, isLoading, error } = useListOAuthClientsQuery({
    page: pageIndex,
    size: pageSize,
  });

  return {
    data: data?.items ?? [],
    total: data?.total ?? 0,
    isLoading,
    error,
    paginationProps,
  };
};

Rule Used: Use the existing usePagination hook for paginati... (source)

Learnt From
ethyca/fides#6594

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!


const OAuthClientsList = () => {
const { data, total, isLoading, page, pageSize, setPage, setPageSize } =
useOAuthClientsList();
const canUpdate = useHasPermission([ScopeRegistryEnum.CLIENT_UPDATE]);

return (
<div>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Prefer Flex over bare div wrapper

The outer <div> on line 94 is the layout wrapper for the list and pagination. Per project style, prefer Ant Design's Flex (vertical) component over plain div elements when possible.

Suggested change
<div>
<Flex vertical>

Rule Used: Avoid using div elements when possible. Use sema... (source)

Learnt From
ethyca/fides#6763

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

<List
loading={isLoading}
itemLayout="horizontal"
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: surface API errors to the user

error is returned from useOAuthClientsList but never destructured or rendered in OAuthClientsList. If the API call fails, the component shows an empty list with no feedback — the user has no way to distinguish a genuine "no clients" state from a failed request.

Consider adding a simple error banner or inline message, e.g.:

const { data, total, isLoading, error, page, pageSize, setPage, setPageSize } =
  useOAuthClientsList();

if (error) {
  return <Alert type="error" message="Failed to load API clients." />;
}

dataSource={data}
rowKey={(client) => client.client_id}
locale={{
emptyText: (
<div className="px-4 py-8 text-center">
<Typography.Paragraph type="secondary">
No API clients yet. Click &quot;Create API client&quot; to get
started.
</Typography.Paragraph>
</div>
),
}}
renderItem={(client) => (
<ClientListItem client={client} canUpdate={canUpdate} />
)}
/>
<Flex justify="end" className="mt-4">
<Pagination
current={page}
total={total}
pageSize={pageSize}
onChange={(newPage, newPageSize) => {
if (newPageSize !== pageSize) {
setPageSize(newPageSize);
setPage(1);
} else {
setPage(newPage);
}
}}
showSizeChanger
pageSizeOptions={DEFAULT_PAGE_SIZES}
showTotal={(totalItems) => `Total ${totalItems} items`}
/>
</Flex>
</div>
);
};

export default OAuthClientsList;
Loading
Loading