Skip to content

Fix MCP OAuth Connect permission gating #246

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.

Sign up for GitHub

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

Open
NianJiuZst wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Fix MCP OAuth Connect permission gating #246

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
3 changes: 3 additions & 0 deletions apps/web/src/pages/admin/capabilities/MarketplaceTab.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ interface MarketplaceTabProps {
query: string
typeFilter: "mcp" | "skill"
canImport: boolean
canConnect: boolean
canManage: boolean
onSelectItem: (id: string | null) => void
onInstall: (capability: MarketplaceCapability) => void
Expand All @@ -33,6 +34,7 @@ export function MarketplaceTab(props: MarketplaceTabProps) {
itemID={mcpItemID}
query={props.query}
canImport={props.canImport}
canConnect={props.canConnect}
onSelectItem={(id) => props.onSelectItem(id ? `mcp:${id}` : null)}
onSelectMarketplaceItem={props.onSelectItem}
onInstallMarketplace={props.onInstall}
Expand All @@ -54,6 +56,7 @@ export function MarketplaceTab(props: MarketplaceTabProps) {
itemID={null}
query={props.query}
canImport={props.canImport}
canConnect={props.canConnect}
onSelectItem={(id) => props.onSelectItem(id ? `mcp:${id}` : null)}
onSelectMarketplaceItem={props.onSelectItem}
onInstallMarketplace={props.onInstall}
Expand Down
1 change: 1 addition & 0 deletions apps/web/src/pages/admin/capabilities/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -281,6 +281,7 @@ export function CapabilitiesPage() {
query={query}
typeFilter={typeFilter}
canImport={canImportDirectory}
canConnect={isAdmin}
canManage={isAdmin}
onSelectItem={(item) => navigate("capabilities", { tab: "marketplace", item })}
onInstall={goToAgentsForCapability}
Expand Down
7 changes: 5 additions & 2 deletions apps/web/src/pages/admin/capabilities/mcp-directory/MCPDirectory.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ interface MCPDirectoryProps {
itemID: string | null
query: string
canImport: boolean
canConnect: boolean
onSelectItem: (id: string | null) => void
onSelectMarketplaceItem: (id: string | null) => void
onInstallMarketplace: (capability: MarketplaceCapability) => void
Expand All @@ -37,6 +38,7 @@ export function MCPDirectory({
itemID,
query,
canImport,
canConnect,
onSelectItem,
onSelectMarketplaceItem,
onInstallMarketplace,
Expand Down Expand Up @@ -106,7 +108,7 @@ export function MCPDirectory({
}, [detailQ, directoryQ])

const connectOAuth = (id: string) => {
if (!workspaceID) return
if (!workspaceID || !canConnect) return
setOAuthError(false)
const width = 620
const height = 760
Expand Down Expand Up @@ -172,6 +174,7 @@ export function MCPDirectory({
loading={detailQ.isLoading}
error={detailQ.error}
canImport={canImport}
canConnect={canConnect}
onBack={() => onSelectItem(null)}
onRetry={() => void detailQ.refetch()}
onImport={() => requestImport(itemID)}
Expand Down Expand Up @@ -228,7 +231,7 @@ export function MCPDirectory({
) : cards.length > 0 ? (
<div className="grid gap-3 md:grid-cols-2 xl:grid-cols-3" data-testid="mcp-marketplace-grid">
{cards.map((card) => card.kind === "directory" ? (
<DirectoryCard key={`directory:${card.item.id}`} item={card.item} canImport={canImport} onOpen={() => onSelectItem(card.item.id)} onImport={() => requestImport(card.item.id)} onConnect={() => connectOAuth(card.item.id)} onViewCapability={onViewCapability} />
<DirectoryCard key={`directory:${card.item.id}`} item={card.item} canImport={canImport} canConnect={canConnect} onOpen={() => onSelectItem(card.item.id)} onImport={() => requestImport(card.item.id)} onConnect={() => connectOAuth(card.item.id)} onViewCapability={onViewCapability} />
) : (
<MarketplaceMCPCard key={`marketplace:${card.item.id}`} capability={card.item} canManage={canManageMarketplace} onOpen={() => onSelectMarketplaceItem(card.item.id)} onInstall={() => onInstallMarketplace(card.item)} onDelete={() => onDeleteMarketplace(card.item)} onViewCapability={() => onViewCapability(card.item.id)} />
))}
Expand Down
7 changes: 4 additions & 3 deletions apps/web/src/pages/admin/capabilities/mcp-directory/MCPDirectoryCard.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,10 @@ import { Button } from "../../../../components/ui/button"
import { marketplaceSourceName, type MCPDirectoryItem, type MarketplaceCapability } from "../../../../lib/api-marketplace"
import { ConnectorIcon, VerifiedBadge } from "./shared"

export function DirectoryCard({ item, canImport, onOpen, onImport, onConnect, onViewCapability }: {
export function DirectoryCard({ item, canImport, canConnect, onOpen, onImport, onConnect, onViewCapability }: {
item: MCPDirectoryItem
canImport: boolean
canConnect: boolean
onOpen: () => void
onImport: () => void
onConnect: () => void
Expand Down Expand Up @@ -43,8 +44,8 @@ export function DirectoryCard({ item, canImport, onOpen, onImport, onConnect, on
<Check className="h-3.5 w-3.5" /> {t("capabilities.mcpDirectory.actions.installed")}
</Button>
) : item.authentication === "oauth2" && !item.connected ? (
<Button className="w-full" size="sm" onClick={onConnect}>
{t("capabilities.mcpDirectory.oauth.connect")}
<Button className="w-full" size="sm" disabled={!canConnect} title={!canConnect ? t("capabilities.permission.adminOnly") : undefined} onClick={onConnect}>
{canConnect ? t("capabilities.mcpDirectory.oauth.connect") : t("capabilities.permission.adminOnly")}
</Button>
) : (
<Button className="w-full" size="sm" disabled={!canImport} title={!canImport ? t("capabilities.permission.adminOnly") : undefined} onClick={onImport}>
Expand Down
6 changes: 4 additions & 2 deletions apps/web/src/pages/admin/capabilities/mcp-directory/MCPDirectoryDetail.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ export function DirectoryDetail({
loading,
error,
canImport,
canConnect,
onBack,
onRetry,
onImport,
Expand All @@ -24,6 +25,7 @@ export function DirectoryDetail({
loading: boolean
error: unknown
canImport: boolean
canConnect: boolean
onBack: () => void
onRetry: () => void
onImport: () => void
Expand Down Expand Up @@ -136,8 +138,8 @@ export function DirectoryDetail({
</div>
<div className="mt-5 flex flex-wrap justify-end gap-2 border-t border-line pt-4">
{item.authentication === "oauth2" && !item.connected ? (
<Button size="sm" onClick={onConnect}>
{t("capabilities.mcpDirectory.oauth.connect")}
<Button size="sm" disabled={!canConnect} title={!canConnect ? t("capabilities.permission.adminOnly") : undefined} onClick={onConnect}>
{canConnect ? t("capabilities.mcpDirectory.oauth.connect") : t("capabilities.permission.adminOnly")}
</Button>
) : item.installed && item.installed_capability_id ? (
<Button
Expand Down
43 changes: 42 additions & 1 deletion tests/e2e/mcp-directory.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { expect, test, type Page, type Route } from "@playwright/test";

const WORKSPACE_ID = "00000000-0000-0000-0000-000000000011";
const CAPABILITY_ID = "00000000-0000-0000-0000-000000000033";
type WorkspaceRole = "owner" | "admin" | "member" | "viewer";

const directoryItems = [
connector("context7", "Context7", "Documentation", 1),
Expand Down Expand Up @@ -69,6 +70,43 @@ test("retries a failed connector directory request", async ({ page }) => {
await expect(page.getByTestId("mcp-directory-card")).toHaveCount(3);
});

test("allows workspace admins to start MCP OAuth", async ({ page }) => {
await mockApp(page, oauthDirectory, "admin");
await page.goto(`/?admin=capabilities&tab=marketplace&ws=${WORKSPACE_ID}`);

const card = page.locator('[data-catalog-id="notion"]');
await expect(card.getByRole("button", { name: "Connect", exact: true })).toBeEnabled();
await card.getByRole("heading", { name: "Notion" }).click();
await expect(page.getByTestId("mcp-directory-detail").getByRole("button", { name: "Connect", exact: true })).toBeEnabled();
});

for (const role of ["member", "viewer"] as const) {
test(`prevents ${role} users from starting MCP OAuth`, async ({ page }) => {
await mockApp(page, oauthDirectory, role);
await page.goto(`/?admin=capabilities&tab=marketplace&ws=${WORKSPACE_ID}`);

const card = page.locator('[data-catalog-id="notion"]');
await expect(card.getByRole("button", { name: "Owner / admin only", exact: true })).toBeDisabled();
await expect(card.getByRole("button", { name: "Connect", exact: true })).toHaveCount(0);
await card.getByRole("heading", { name: "Notion" }).click();

const detail = page.getByTestId("mcp-directory-detail");
await expect(detail.getByRole("button", { name: "Owner / admin only", exact: true })).toBeDisabled();
await expect(detail.getByRole("button", { name: "Connect", exact: true })).toHaveCount(0);
});
}

const oauthDirectoryItem = {
...connector("notion", "Notion", "Productivity", 1),
authentication: "oauth2",
connected: false,
};

async function oauthDirectory(route: Route) {
await json(route, { items: [oauthDirectoryItem] });
return true;
}

function connector(id: string, name: string, category: string, featuredRank: number) {
return {
id,
Expand All @@ -89,6 +127,7 @@ function connector(id: string, name: string, category: string, featuredRank: num
async function mockApp(
page: Page,
directoryOverride?: (route: Route) => Promise<boolean>,
role: WorkspaceRole = "owner",
) {
await page.route("**/api/v1/**", async (route) => {
const request = route.request();
Expand All @@ -109,7 +148,7 @@ async function mockApp(
name: "Directory Test",
slug: "directory-test",
visibility: "private",
role: "owner",
role,
created_at: "2026-07-23T00:00:00Z",
updated_at: "2026-07-23T00:00:00Z",
}],
Expand Down Expand Up @@ -159,6 +198,8 @@ async function mockApp(
}
if (path === `/api/v1/workspaces/${WORKSPACE_ID}/mcp-directory/context7`)
return json(route, { ...directoryItems[0], url: "https://mcp.context7.com/mcp" });
if (path === `/api/v1/workspaces/${WORKSPACE_ID}/mcp-directory/notion`)
return json(route, { ...oauthDirectoryItem, url: "https://mcp.notion.com/mcp" });
if (path === `/api/v1/workspaces/${WORKSPACE_ID}/mcp-directory/context7/import`)
return json(route, { installed: true, capability_id: CAPABILITY_ID }, 201);
return json(route, {});
Expand Down