Skip to content

feat: integrate backend with idp sync page #14755

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 13 commits into from
Sep 24, 2024
24 changes: 24 additions & 0 deletions site/src/api/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -704,6 +704,30 @@ class ApiMethods {
return response.data;
};

/**
* @param organization Can be the organization's ID or name
*/
getGroupIdpSyncSettingsByOrganization = async (
organization: string,
): Promise<TypesGen.GroupSyncSettings> => {
const response = await this.axios.get<TypesGen.GroupSyncSettings>(
`/api/v2/organizations/${organization}/settings/idpsync/groups`,
);
return response.data;
};

/**
* @param organization Can be the organization's ID or name
*/
Comment on lines +719 to +721
Copy link
Member

Choose a reason for hiding this comment

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

can you add this above the getGroup... function too pls?

getRoleIdpSyncSettingsByOrganization = async (
organization: string,
): Promise<TypesGen.RoleSyncSettings> => {
const response = await this.axios.get<TypesGen.RoleSyncSettings>(
`/api/v2/organizations/${organization}/settings/idpsync/roles`,
);
return response.data;
};

getTemplate = async (templateId: string): Promise<TypesGen.Template> => {
const response = await this.axios.get<TypesGen.Template>(
`/api/v2/templates/${templateId}`,
Expand Down
33 changes: 33 additions & 0 deletions site/src/api/queries/organizations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,32 @@ export const provisionerDaemonGroups = (organization: string) => {
};
};

export const getGroupIdpSyncSettingsKey = (organization: string) => [
"organizations",
organization,
"groupIdpSyncSettings",
];

export const groupIdpSyncSettings = (organization: string) => {
return {
queryKey: getGroupIdpSyncSettingsKey(organization),
queryFn: () => API.getGroupIdpSyncSettingsByOrganization(organization),
};
};

export const getRoleIdpSyncSettingsKey = (organization: string) => [
"organizations",
organization,
"roleIdpSyncSettings",
];

export const roleIdpSyncSettings = (organization: string) => {
return {
queryKey: getRoleIdpSyncSettingsKey(organization),
queryFn: () => API.getRoleIdpSyncSettingsByOrganization(organization),
};
};

/**
* Fetch permissions for a single organization.
*
Expand Down Expand Up @@ -243,6 +269,13 @@ export const organizationsPermissions = (
},
action: "read",
},
viewIdpSyncSettings: {
object: {
resource_type: "idpsync_settings",
organization_id: organizationId,
},
action: "read",
},
});

// The endpoint takes a flat array, so to avoid collisions prepend each
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ export const AppearanceSettingsPageView: FC<
<PopoverContent css={{ transform: "translateY(-28px)" }}>
<PopoverPaywall
message="Appearance"
description="With a Premium license, you can customize the appearance of your deployment."
description="With a Premium license, you can customize the appearance and branding of your deployment."
Copy link
Member

Choose a reason for hiding this comment

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

Just curious: since we now have the branding role, have there been any conversations about extending that down the line to work with customer-specific branding?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nothing yet, so far this has purely been focused on the enterprise and premium badges and banners.

Copy link
Member

Choose a reason for hiding this comment

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

nit: theme.branding is not a role, it lives outside of theme.roles, it's just an object/namespace

documentationLink="https://coder.com/docs/admin/appearance"
/>
</PopoverContent>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,12 +29,12 @@ import { Link as RouterLink, useNavigate } from "react-router-dom";
import { docs } from "utils/docs";
import { PermissionPillsList } from "./PermissionPillsList";

export type CustomRolesPageViewProps = {
interface CustomRolesPageViewProps {
Copy link
Member

Choose a reason for hiding this comment

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

Have we decided on whether we prefer interfaces over type aliases? Asking because I generally avoid interfaces so that I can't do accidental interface merging, but I've been seeing a lot of recent code using interfaces

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My goal was just to be consistent and I saw more interfaces used for all the organization related pages. I don't personally have a strong preference other than being consistent.

Copy link
Member

Choose a reason for hiding this comment

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

I like interface because I think interface WibbleProps extends WobbleProps {} is a lot clearer than type WibbleProps = WobbleProps & {} in the places where we need it. and I think we just generally use it in more places rn.

roles: Role[] | undefined;
onDeleteRole: (role: Role) => void;
canAssignOrgRole: boolean;
isCustomRolesEnabled: boolean;
};
}

export const CustomRolesPageView: FC<CustomRolesPageViewProps> = ({
roles,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
import type { Meta, StoryObj } from "@storybook/react";
import { expect, fn, userEvent, waitFor, within } from "@storybook/test";
import {
MockGroupSyncSettings,
MockOrganization,
MockRoleSyncSettings,
} from "testHelpers/entities";
import { ExportPolicyButton } from "./ExportPolicyButton";

const meta: Meta<typeof ExportPolicyButton> = {
title: "modules/resources/ExportPolicyButton",
component: ExportPolicyButton,
args: {
syncSettings: MockGroupSyncSettings,
type: "groups",
organization: MockOrganization,
},
};

export default meta;
type Story = StoryObj<typeof ExportPolicyButton>;

export const Default: Story = {};

export const ClickExportGroupPolicy: Story = {
args: {
syncSettings: MockGroupSyncSettings,
type: "groups",
organization: MockOrganization,
download: fn(),
},
play: async ({ canvasElement, args }) => {
const canvas = within(canvasElement);
await userEvent.click(
canvas.getByRole("button", { name: "Export Policy" }),
);
await waitFor(() =>
expect(args.download).toHaveBeenCalledWith(
expect.anything(),
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
expect.anything(),
expect.any(Blob)

`${MockOrganization.name}_groups-policy.json`,
),
);
const blob: Blob = (args.download as jest.Mock).mock.lastCall[0];
await expect(blob.type).toEqual("application/json");
Copy link
Member

Choose a reason for hiding this comment

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

Can we also assert that the blob contents match the MockGroupSyncSettings value?

await expect(await blob.text()).toEqual(
JSON.stringify(MockGroupSyncSettings, null, 2),
);
},
};

export const ClickExportRolePolicy: Story = {
args: {
syncSettings: MockRoleSyncSettings,
type: "roles",
organization: MockOrganization,
download: fn(),
},
play: async ({ canvasElement, args }) => {
const canvas = within(canvasElement);
await userEvent.click(
canvas.getByRole("button", { name: "Export Policy" }),
);
await waitFor(() =>
expect(args.download).toHaveBeenCalledWith(
expect.anything(),
`${MockOrganization.name}_roles-policy.json`,
),
);
const blob: Blob = (args.download as jest.Mock).mock.lastCall[0];
await expect(blob.type).toEqual("application/json");
await expect(await blob.text()).toEqual(
JSON.stringify(MockRoleSyncSettings, null, 2),
);
},
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
import DownloadOutlined from "@mui/icons-material/DownloadOutlined";
import Button from "@mui/material/Button";
import type {
GroupSyncSettings,
Organization,
RoleSyncSettings,
} from "api/typesGenerated";
import { displayError } from "components/GlobalSnackbar/utils";
import { saveAs } from "file-saver";
import { type FC, useMemo, useState } from "react";

interface DownloadPolicyButtonProps {
syncSettings: RoleSyncSettings | GroupSyncSettings | undefined;
type: "groups" | "roles";
organization: Organization;
download?: (file: Blob, filename: string) => void;
}

export const ExportPolicyButton: FC<DownloadPolicyButtonProps> = ({
syncSettings,
type,
organization,
download = saveAs,
Copy link
Member

Choose a reason for hiding this comment

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

Not sure how big of a deal this is, but the current implementation means that we can't add additional functionality without completely overriding the default saveAs function. Every consumer would have to import saveAs themselves, and then wire up the functionality for component implementation

How often would we have a use case where we'd want to get rid of the saveAs functionality vs wanting to always have saveAs, and then potentially add more functionality on top?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was meant to just support the Storybook test case. Is there a better way to handle this?

}) => {
const [isDownloading, setIsDownloading] = useState(false);

const policyJSON = useMemo(() => {
return syncSettings?.field && syncSettings.mapping
? JSON.stringify(syncSettings, null, 2)
: null;
}, [syncSettings]);

return (
<Button
startIcon={<DownloadOutlined />}
disabled={!policyJSON || isDownloading}
onClick={async () => {
if (policyJSON) {
try {
setIsDownloading(true);
const file = new Blob([policyJSON], {
type: "application/json",
});
download(file, `${organization.name}_${type}-policy.json`);
} catch (e) {
console.error(e);
displayError("Failed to export policy json");
} finally {
setIsDownloading(false);
}
}
}}
>
Export Policy
</Button>
);
};
106 changes: 106 additions & 0 deletions site/src/pages/ManagementSettingsPage/IdpSyncPage/IdpPillList.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,106 @@
import { type Interpolation, type Theme, useTheme } from "@emotion/react";
import Stack from "@mui/material/Stack";
import { Pill } from "components/Pill/Pill";
import {
Popover,
PopoverContent,
PopoverTrigger,
} from "components/Popover/Popover";
import type { FC } from "react";

interface PillListProps {
roles: readonly string[];
}

// used to check if the role is a UUID
const UUID =
/^[0-9a-f]{8}-[0-9a-f]{4}-[0-5][0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/i;

export const IdpPillList: FC<PillListProps> = ({ roles }) => {
return (
<Stack direction="row" spacing={1}>
{roles.length > 0 ? (
<Pill css={UUID.test(roles[0]) ? styles.errorPill : styles.pill}>
{roles[0]}
</Pill>
) : (
<p>None</p>
)}

{roles.length > 1 && <OverflowPill roles={roles.slice(1)} />}
</Stack>
);
};

interface OverflowPillProps {
roles: string[];
}

const OverflowPill: FC<OverflowPillProps> = ({ roles }) => {
const theme = useTheme();

return (
<Popover mode="hover">
<PopoverTrigger>
<Pill
css={{
backgroundColor: theme.palette.background.paper,
borderColor: theme.palette.divider,
}}
data-testid="overflow-pill"
>
+{roles.length} more
</Pill>
</PopoverTrigger>
Comment on lines +44 to +54
Copy link
Member

Choose a reason for hiding this comment

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

Potential accessibility issue since we're adding interactive functionality to a plain div

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This pattern originally came from UserRoleCell.tsx and also exists in PermissionsPillsList.tsx, what would you suggest to use instead of a div? or would we have to move away from using PopoverTrigger for pills?


<PopoverContent
disableRestoreFocus
disableScrollLock
css={{
".MuiPaper-root": {
display: "flex",
flexFlow: "column wrap",
columnGap: 8,
rowGap: 12,
padding: "12px 16px",
alignContent: "space-around",
minWidth: "auto",
backgroundColor: theme.palette.background.default,
},
}}
anchorOrigin={{
vertical: -4,
horizontal: "center",
}}
transformOrigin={{
vertical: "bottom",
horizontal: "center",
}}
>
{roles.map((role) => (
<Pill
key={role}
css={UUID.test(role) ? styles.errorPill : styles.pill}
>
{role}
</Pill>
))}
</PopoverContent>
</Popover>
);
};

const styles = {
pill: (theme) => ({
backgroundColor: theme.experimental.pillDefault.background,
borderColor: theme.experimental.pillDefault.outline,
color: theme.experimental.pillDefault.text,
width: "fit-content",
}),
errorPill: (theme) => ({
backgroundColor: theme.roles.error.background,
borderColor: theme.roles.error.outline,
color: theme.roles.error.text,
width: "fit-content",
}),
} satisfies Record<string, Interpolation<Theme>>;
Loading
Loading