fix: add oauth client owner email to admin view (#27243)

* add owner email to admin oauth view

* fix type error

* fix oauth flow with sign up

* improve e2e tests

* fix: use dynamic user email in OAuth client e2e test

The test 'editing redirect uri of an approved client triggers reapproval'
creates a new user and assigns the OAuth client to that user, but was
incorrectly expecting 'admin@example.com' as the owner email. Changed to
use user.email to correctly validate the owner email matches the created
user.

Addresses Cubic AI review feedback (confidence: 9/10)

Co-Authored-By: unknown <>

---------

Co-authored-by: CarinaWolli <wollencarina@gmail.com>
Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
This commit is contained in:
Carina Wollendorfer
2026-02-05 10:39:29 +00:00
committed by GitHub
co-authored by unknown <> CarinaWolli Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
parent 357c2882ad
commit 80e07a8d35
3 changed files with 156 additions and 52 deletions
@@ -60,6 +60,9 @@ export const OAuthClientsList = ({
/>
<div>
<div className="text-emphasis font-medium">{client.name}</div>
{client.user?.email && (
<div className="text-subtle text-sm">{client.user.email}</div>
)}
</div>
</div>
<div className="flex items-center gap-4">
@@ -10,7 +10,13 @@ import { useLocale } from "@calcom/lib/hooks/useLocale";
import { Alert } from "@calcom/ui/components/alert";
import { Badge } from "@calcom/ui/components/badge";
import { Button } from "@calcom/ui/components/button";
import { ConfirmationDialogContent, DialogClose, DialogContent, DialogFooter } from "@calcom/ui/components/dialog";
import {
ConfirmationDialogContent,
DialogClose,
DialogContent,
DialogFooter,
} from "@calcom/ui/components/dialog";
import { showToast } from "@calcom/ui/components/toast";
import { Tooltip } from "@calcom/ui/components/tooltip";
import { Label, TextArea } from "@calcom/ui/components/form";
@@ -30,6 +36,9 @@ type OAuthClientDetails = {
clientSecret?: string;
isPkceEnabled?: boolean;
clientType?: string;
user?: {
email: string;
} | null;
};
const OAuthClientDetailsDialog = ({
@@ -69,7 +78,8 @@ const OAuthClientDetailsDialog = ({
const [isDeleteConfirmOpen, setIsDeleteConfirmOpen] = useState(false);
const [isRejectConfirmOpen, setIsRejectConfirmOpen] = useState(false);
const [rejectionReason, setRejectionReason] = useState("");
const [showRejectionReasonError, setShowRejectionReasonError] = useState(false);
const [showRejectionReasonError, setShowRejectionReasonError] =
useState(false);
const form = useForm<OAuthClientCreateFormValues>({
defaultValues: {
name: "",
@@ -93,7 +103,10 @@ const OAuthClientDetailsDialog = ({
if (!client) return;
const enablePkce =
client.isPkceEnabled ?? (client.clientType ? client.clientType.toUpperCase() === "PUBLIC" : false);
client.isPkceEnabled ??
(client.clientType
? client.clientType.toUpperCase() === "PUBLIC"
: false);
const nextLogo = client.logo ?? "";
setLogo(nextLogo);
@@ -139,8 +152,10 @@ const OAuthClientDetailsDialog = ({
);
if (showAdminActions) {
const canReject = Boolean(onReject) && (status === "PENDING" || status === "APPROVED");
const canApprove = Boolean(onApprove) && (status === "PENDING" || status === "REJECTED");
const canReject =
Boolean(onReject) && (status === "PENDING" || status === "APPROVED");
const canApprove =
Boolean(onApprove) && (status === "PENDING" || status === "REJECTED");
return (
<div className="flex gap-2 justify-end items-center w-full">
@@ -156,7 +171,8 @@ const OAuthClientDetailsDialog = ({
setIsRejectConfirmOpen(true);
setRejectionReason("");
setShowRejectionReasonError(false);
}}>
}}
>
{t("reject")}
</Button>
) : null}
@@ -170,7 +186,8 @@ const OAuthClientDetailsDialog = ({
onClick={() => {
if (!clientId) return;
onApprove?.(clientId);
}}>
}}
>
{t("approve")}
</Button>
) : null}
@@ -182,14 +199,20 @@ const OAuthClientDetailsDialog = ({
return (
<div className="flex gap-2 justify-end items-center w-full">
{closeButton}
<Button type="submit" loading={isUpdatePending} data-testid="oauth-client-details-save">
<Button
type="submit"
loading={isUpdatePending}
data-testid="oauth-client-details-save"
>
{t("save")}
</Button>
</div>
);
}
return <div className="flex justify-end items-center w-full">{closeButton}</div>;
return (
<div className="flex justify-end items-center w-full">{closeButton}</div>
);
})();
return (
@@ -209,26 +232,42 @@ const OAuthClientDetailsDialog = ({
websiteUrl: values.websiteUrl.trim() || "",
logo: values.logo,
});
})}>
})}
>
{status ? (
<div className="flex justify-start items-center">
<Badge data-testid="oauth-client-details-status-badge" variant={getStatusBadgeVariant(status).variant}>
<Badge
data-testid="oauth-client-details-status-badge"
variant={getStatusBadgeVariant(status).variant}
>
{t(getStatusBadgeVariant(status).labelKey)}
</Badge>
</div>
) : null}
{status === "PENDING" ? (
<Alert severity="warning" title={t("oauth_client_pending_info_description")} />
<Alert
severity="warning"
title={t("oauth_client_pending_info_description")}
/>
) : null}
{status === "APPROVED" ? (
<Alert severity="warning" title={t("oauth_client_approved_reapproval_info")} />
<Alert
severity="warning"
title={t("oauth_client_approved_reapproval_info")}
/>
) : null}
{status === "REJECTED" && client.rejectionReason ? (
<div className="text-sm text-subtle" data-testid="oauth-client-details-rejection-reason-display">
<span className="font-medium">{t("oauth_client_rejection_reason")}:</span> {client.rejectionReason}
<div
className="text-sm text-subtle"
data-testid="oauth-client-details-rejection-reason-display"
>
<span className="font-medium">
{t("oauth_client_rejection_reason")}:
</span>{" "}
{client.rejectionReason}
</div>
) : null}
@@ -237,27 +276,39 @@ const OAuthClientDetailsDialog = ({
<div className="flex">
<code
data-testid="oauth-client-details-client-id"
className="px-2 py-1 w-full font-mono text-sm truncate align-middle rounded-md rounded-r-none bg-subtle text-default">
className="px-2 py-1 w-full font-mono text-sm truncate align-middle rounded-md rounded-r-none bg-subtle text-default"
>
{client.clientId}
</code>
<Tooltip side="top" content={t("copy_to_clipboard")}>
<Button
onClick={() => {
copyToClipboard(client.clientId, {
onSuccess: () => showToast(t("client_id_copied"), "success"),
onSuccess: () =>
showToast(t("client_id_copied"), "success"),
onFailure: () => showToast(t("error"), "error"),
});
}}
type="button"
size="sm"
className="rounded-l-none"
StartIcon="clipboard">
StartIcon="clipboard"
>
{t("copy")}
</Button>
</Tooltip>
</div>
</div>
{client.user?.email ? (
<div>
<div className="mb-1 text-sm text-subtle">{t("owner")}</div>
<div className="text-sm text-default" data-testid="oauth-client-details-user-email">
{client.user.email}
</div>
</div>
) : null}
<OAuthClientFormFields
form={form}
logo={logo}
@@ -275,10 +326,14 @@ const OAuthClientDetailsDialog = ({
data-testid="oauth-client-details-delete-trigger"
loading={isDeletePending}
className="w-auto"
onClick={() => setIsDeleteConfirmOpen(true)}>
onClick={() => setIsDeleteConfirmOpen(true)}
>
{t("delete_oauth_client")}
</Button>
<Dialog open={isDeleteConfirmOpen} onOpenChange={setIsDeleteConfirmOpen}>
<Dialog
open={isDeleteConfirmOpen}
onOpenChange={setIsDeleteConfirmOpen}
>
<ConfirmationDialogContent
variety="danger"
title={t("delete_oauth_client")}
@@ -292,21 +347,24 @@ const OAuthClientDetailsDialog = ({
onClick={() => {
if (!client) return;
onDelete?.(client.clientId);
}}>
}}
>
{isDeletePending ? t("deleting") : t("delete")}
</DialogClose>
}>
}
>
<p className="mb-4">{t("confirm_delete_oauth_client")}</p>
</ConfirmationDialogContent>
</Dialog>
</div>
) : null}
<DialogFooter className="mt-6">
{footerActions}
</DialogFooter>
<DialogFooter className="mt-6">{footerActions}</DialogFooter>
<Dialog open={isRejectConfirmOpen} onOpenChange={setIsRejectConfirmOpen}>
<Dialog
open={isRejectConfirmOpen}
onOpenChange={setIsRejectConfirmOpen}
>
<ConfirmationDialogContent
variety="danger"
title={t("reject_oauth_client")}
@@ -319,23 +377,32 @@ const OAuthClientDetailsDialog = ({
data-testid="oauth-client-details-reject-confirm"
loading={isStatusChangePending}
className="not-disabled:hover:!bg-error not-disabled:hover:!text-white not-disabled:hover:!border-semantic-error"
onClick={handleConfirmReject}>
onClick={handleConfirmReject}
>
{t("reject")}
</Button>
}>
}
>
<div className="mt-4 space-y-2">
<Label htmlFor="oauth-rejection-reason">{t("reason_for_rejection")}</Label>
<Label htmlFor="oauth-rejection-reason">
{t("reason_for_rejection")}
</Label>
<TextArea
id="oauth-rejection-reason"
data-testid="oauth-client-details-rejection-reason"
value={rejectionReason}
onChange={(e) => {
setRejectionReason(e.target.value);
if (showRejectionReasonError && e.target.value.trim().length > 0) {
if (
showRejectionReasonError &&
e.target.value.trim().length > 0
) {
setShowRejectionReasonError(false);
}
}}
className={showRejectionReasonError ? "border-error" : undefined}
className={
showRejectionReasonError ? "border-error" : undefined
}
/>
{showRejectionReasonError ? (
<p className="text-sm text-error">{t("is_required")}</p>
@@ -360,7 +427,7 @@ function getStatusBadgeVariant(status: string) {
default:
return { variant: "orange" as const, labelKey: "pending" as const };
}
};
}
export type { OAuthClientDetails };
export { OAuthClientDetailsDialog };
@@ -1,9 +1,7 @@
import { expect, type Locator, type Page } from "@playwright/test";
import path from "node:path";
import type { PrismaClient } from "@calcom/prisma";
import type { OAuthClientType } from "@calcom/prisma/enums";
import { expect, type Locator, type Page } from "@playwright/test";
import { test } from "../lib/fixtures";
async function loginAsSeededAdminAndGoToOAuthSettings(page: Page) {
@@ -80,7 +78,10 @@ const oAuthClientSelect = {
logo: true,
} as const;
async function createOAuthClient(page: Page, input: CreateOAuthClientInput): Promise<CreateOAuthClientResult> {
async function createOAuthClient(
page: Page,
input: CreateOAuthClientInput
): Promise<CreateOAuthClientResult> {
await goToOAuthSettings(page);
await page.locator("header").getByTestId("open-oauth-client-create-dialog").click();
@@ -116,9 +117,7 @@ async function createOAuthClient(page: Page, input: CreateOAuthClientInput): Pro
expect(clientId.length).toBeGreaterThan(1);
const secretLocator = page.getByTestId("oauth-client-submitted-client-secret");
const clientSecret = input.enablePkce
? null
: ((await secretLocator.textContent()) ?? "").trim();
const clientSecret = input.enablePkce ? null : ((await secretLocator.textContent()) ?? "").trim();
if (input.enablePkce) {
await expect(secretLocator).toHaveCount(0);
@@ -146,7 +145,9 @@ async function updateOAuthClient(page: Page, input: UpdateOAuthClientInput): Pro
await uploadOAuthClientLogo(page, input.logoFileName);
}
const updateResponsePromise = page.waitForResponse((res) => res.url().includes("/api/trpc/oAuth/updateClient"));
const updateResponsePromise = page.waitForResponse((res) =>
res.url().includes("/api/trpc/oAuth/updateClient")
);
await page.getByTestId("oauth-client-details-save").click();
const updateResponse = await updateResponsePromise;
expect(updateResponse.ok()).toBe(true);
@@ -156,14 +157,20 @@ async function updateOAuthClient(page: Page, input: UpdateOAuthClientInput): Pro
await expectOAuthClientInList(page, { clientId: input.clientId, name: input.name });
}
async function deleteOAuthClient(page: Page, clientId: string, expectedNameToDisappear: string): Promise<void> {
async function deleteOAuthClient(
page: Page,
clientId: string,
expectedNameToDisappear: string
): Promise<void> {
await goToOAuthSettings(page);
await openOAuthClientDetails(page, clientId);
await page.getByTestId("oauth-client-details-delete-trigger").click();
await expect(page.getByTestId("oauth-client-details-delete-confirm")).toBeVisible();
const deleteResponsePromise = page.waitForResponse((res) => res.url().includes("/api/trpc/oAuth/deleteClient"));
const deleteResponsePromise = page.waitForResponse((res) =>
res.url().includes("/api/trpc/oAuth/deleteClient")
);
await page.getByTestId("oauth-client-details-delete-confirm").click();
const deleteResponse = await deleteResponsePromise;
expect(deleteResponse.ok()).toBe(true);
@@ -183,7 +190,9 @@ async function closeOAuthClientDetails(page: Page): Promise<void> {
async function openOAuthClientDetails(page: Page, clientId: string): Promise<Locator> {
const details = getOAuthClientDetailsForm(page);
if (await details.isVisible()) {
const visibleClientId = ((await details.getByTestId("oauth-client-details-client-id").textContent()) ?? "").trim();
const visibleClientId = (
(await details.getByTestId("oauth-client-details-client-id").textContent()) ?? ""
).trim();
if (visibleClientId === clientId) return details;
await closeOAuthClientDetails(page);
@@ -196,7 +205,10 @@ async function openOAuthClientDetails(page: Page, clientId: string): Promise<Loc
}
async function expectOAuthClientDeletedInDb(prisma: PrismaClient, clientId: string): Promise<void> {
const dbClientAfterDelete = await prisma.oAuthClient.findUnique({ where: { clientId }, select: { clientId: true } });
const dbClientAfterDelete = await prisma.oAuthClient.findUnique({
where: { clientId },
select: { clientId: true },
});
expect(dbClientAfterDelete).toBeNull();
}
@@ -205,14 +217,18 @@ async function expectOAuthClientInDb(
clientId: string,
expected: OAuthClientDbExpectations
): Promise<void> {
const dbClient = await prisma.oAuthClient.findUniqueOrThrow({ where: { clientId }, select: oAuthClientSelect });
const dbClient = await prisma.oAuthClient.findUniqueOrThrow({
where: { clientId },
select: oAuthClientSelect,
});
if (expected.name !== undefined) expect(dbClient.name).toBe(expected.name);
if (expected.purpose !== undefined) expect(dbClient.purpose).toBe(expected.purpose);
if (expected.redirectUri !== undefined) expect(dbClient.redirectUri).toBe(expected.redirectUri);
if (expected.websiteUrl !== undefined) expect(dbClient.websiteUrl).toBe(expected.websiteUrl);
if (expected.status !== undefined) expect(dbClient.status).toBe(expected.status);
if (expected.clientType !== undefined) expect(dbClient.clientType as OAuthClientType).toBe(expected.clientType);
if (expected.clientType !== undefined)
expect(dbClient.clientType as OAuthClientType).toBe(expected.clientType);
if (expected.clientSecret !== undefined) {
if (typeof expected.clientSecret === "object" && expected.clientSecret?.kind === "truthy") {
@@ -231,7 +247,10 @@ async function expectOAuthClientInDb(
}
}
async function expectOAuthClientDetails(details: Locator, expected: ExpectedOAuthClientDetails): Promise<void> {
async function expectOAuthClientDetails(
details: Locator,
expected: ExpectedOAuthClientDetails
): Promise<void> {
await expect(details.getByTestId("oauth-client-details-status-badge")).toHaveText(expected.statusLabel);
await expect(details.getByTestId("oauth-client-details-client-id")).toHaveText(expected.clientId);
await expect(getOAuthClientDetailsNameInput(details)).toHaveValue(expected.name);
@@ -314,7 +333,10 @@ test.describe("OAuth client creation", () => {
await expect(redirectUriInput).toHaveJSProperty("validationMessage", "Please fill out this field.");
});
test("creates a private (confidential) OAuth client with minimal fields; submitted modal shows id+secret; list/details/DB reflect values", async ({ page, prisma }, testInfo) => {
test("creates a private (confidential) OAuth client with minimal fields; submitted modal shows id+secret; list/details/DB reflect values", async ({
page,
prisma,
}, testInfo) => {
await loginAsSeededAdminAndGoToOAuthSettings(page);
const testPrefix = `e2e-oauth-client-creation-${testInfo.testId}-`;
@@ -411,7 +433,11 @@ test.describe("OAuth client creation", () => {
await expectOAuthClientDeletedInDb(prisma, clientId);
});
test("editing redirect uri of an approved client triggers reapproval (status becomes PENDING)", async ({ page, prisma, users }, testInfo) => {
test("editing redirect uri of an approved client triggers reapproval (status becomes PENDING)", async ({
page,
prisma,
users,
}, testInfo) => {
const testPrefix = `e2e-oauth-client-creation-${testInfo.testId}-`;
const user = await users.create();
@@ -462,7 +488,9 @@ test.describe("OAuth client creation", () => {
await getOAuthClientDetailsRedirectUriInput(detailsBeforeUpdate).fill(updatedRedirectUri);
const updateResponsePromise = page.waitForResponse((res) => res.url().includes("/api/trpc/oAuth/updateClient"));
const updateResponsePromise = page.waitForResponse((res) =>
res.url().includes("/api/trpc/oAuth/updateClient")
);
await page.getByTestId("oauth-client-details-save").click();
const updateResponse = await updateResponsePromise;
expect(updateResponse.ok()).toBe(true);
@@ -492,7 +520,10 @@ test.describe("OAuth client creation", () => {
});
});
test("creates a private (confidential) OAuth client; submitted modal shows id+secret; list/details/DB reflect values", async ({ page, prisma }, testInfo) => {
test("creates a private (confidential) OAuth client; submitted modal shows id+secret; list/details/DB reflect values", async ({
page,
prisma,
}, testInfo) => {
await loginAsSeededAdminAndGoToOAuthSettings(page);
const testPrefix = `e2e-oauth-client-creation-${testInfo.testId}-`;
@@ -593,7 +624,10 @@ test.describe("OAuth client creation", () => {
await expectOAuthClientDeletedInDb(prisma, clientId);
});
test("creates a public (PKCE) OAuth client; submitted modal shows id but no secret; list/details/DB reflect values", async ({ page, prisma }, testInfo) => {
test("creates a public (PKCE) OAuth client; submitted modal shows id but no secret; list/details/DB reflect values", async ({
page,
prisma,
}, testInfo) => {
await loginAsSeededAdminAndGoToOAuthSettings(page);
const testPrefix = `e2e-oauth-client-creation-${testInfo.testId}-`;