From 5c37f0a91d2dd86076d08e90386485146f7be497 Mon Sep 17 00:00:00 2001
From: Tom Moor <tom@getoutline.com>
Date: Sat, 13 Dec 2025 12:41:43 -0500
Subject: [PATCH] fix: Details returned from OAuth client list endpoint
 (#10896)

---
 server/policies/oauthClient.ts                |  4 +--
 server/presenters/oauthClient.ts              |  3 +-
 .../api/oauthClients/oauthClients.test.ts     | 30 +++++++++++++++++--
 .../routes/api/oauthClients/oauthClients.ts   | 12 ++++++--
 4 files changed, 41 insertions(+), 8 deletions(-)

diff --git a/server/policies/oauthClient.ts b/server/policies/oauthClient.ts
index d1341ee008..5e4b532ea8 100644
--- a/server/policies/oauthClient.ts
+++ b/server/policies/oauthClient.ts
@@ -3,7 +3,7 @@ import { allow } from "./cancan";
 import { or, isTeamModel, isTeamMutable, and, isTeamAdmin } from "./utils";
 
 allow(User, "createOAuthClient", Team, (actor, team) =>
-  and(isTeamModel(actor, team), isTeamMutable(actor), actor.isAdmin)
+  and(isTeamAdmin(actor, team), isTeamMutable(actor))
 );
 
 allow(User, "listOAuthClients", Team, (actor, team) =>
@@ -15,5 +15,5 @@ allow(User, "read", OAuthClient, (actor, oauthClient) =>
 );
 
 allow(User, ["update", "delete"], OAuthClient, (actor, oauthClient) =>
-  and(isTeamModel(actor, oauthClient), isTeamMutable(actor), actor.isAdmin)
+  and(isTeamAdmin(actor, oauthClient), isTeamMutable(actor))
 );
diff --git a/server/presenters/oauthClient.ts b/server/presenters/oauthClient.ts
index d741c817e5..a7e8a222de 100644
--- a/server/presenters/oauthClient.ts
+++ b/server/presenters/oauthClient.ts
@@ -1,7 +1,8 @@
 import { OAuthClient } from "@server/models";
 
 /**
- * Presents the OAuth client to the user.
+ * Presents the OAuth client to the user, including the client secret.
+ * This should ONLY be used for admin users who need to manage the OAuth client.
  *
  * @param oauthClient The OAuth client to present
  */
diff --git a/server/routes/api/oauthClients/oauthClients.test.ts b/server/routes/api/oauthClients/oauthClients.test.ts
index e68bb48226..b15f0bc693 100644
--- a/server/routes/api/oauthClients/oauthClients.test.ts
+++ b/server/routes/api/oauthClients/oauthClients.test.ts
@@ -67,9 +67,9 @@ describe("oauthClients.info", () => {
     expect(body).toMatchSnapshot();
   });
 
-  it("should return information about an OAuth client when authorized", async () => {
+  it("should return confidential information about an OAuth client when admin", async () => {
     const team = await buildTeam();
-    const user = await buildUser({ teamId: team.id });
+    const user = await buildAdmin({ teamId: team.id });
 
     const client = await OAuthClient.create({
       teamId: team.id,
@@ -90,9 +90,35 @@ describe("oauthClients.info", () => {
     expect(body.data.id).toBeDefined();
     expect(body.data.name).toEqual("Test Client");
     expect(body.data.published).toBeFalsy();
+    expect(body.data.clientSecret).toBeDefined();
     expect(body.data.redirectUris).toEqual(["https://example.com/callback"]);
   });
 
+  it("should return basic information about an OAuth client when member", async () => {
+    const team = await buildTeam();
+    const user = await buildUser({ teamId: team.id });
+
+    const client = await OAuthClient.create({
+      teamId: team.id,
+      createdById: user.id,
+      name: "Test Client",
+      redirectUris: ["https://example.com/callback"],
+    });
+
+    const res = await server.post("/api/oauthClients.info", {
+      body: {
+        token: user.getJwtToken(),
+        id: client.id,
+      },
+    });
+
+    const body = await res.json();
+    expect(res.status).toEqual(200);
+    expect(body.data.id).toBeUndefined();
+    expect(body.data.name).toEqual("Test Client");
+    expect(body.data.clientSecret).toBeUndefined();
+  });
+
   it("should return information about an OAuth client when published", async () => {
     const team = await buildTeam();
     const admin = await buildAdmin({ teamId: team.id });
diff --git a/server/routes/api/oauthClients/oauthClients.ts b/server/routes/api/oauthClients/oauthClients.ts
index 09caa0665b..db06041e99 100644
--- a/server/routes/api/oauthClients/oauthClients.ts
+++ b/server/routes/api/oauthClients/oauthClients.ts
@@ -6,7 +6,7 @@ import { rateLimiter } from "@server/middlewares/rateLimiter";
 import { transaction } from "@server/middlewares/transaction";
 import validate from "@server/middlewares/validate";
 import { OAuthClient } from "@server/models";
-import { authorize } from "@server/policies";
+import { authorize, can } from "@server/policies";
 import {
   presentPolicies,
   presentOAuthClient,
@@ -27,6 +27,7 @@ router.post(
   async (ctx: APIContext<T.OAuthClientsListReq>) => {
     const { user } = ctx.state.auth;
     const where = { teamId: user.teamId };
+    authorize(user, "listOAuthClients", user.team);
 
     const [oauthClients, total] = await Promise.all([
       OAuthClient.findAll({
@@ -40,7 +41,11 @@ router.post(
 
     ctx.body = {
       pagination: { ...ctx.state.pagination, total },
-      data: oauthClients.map(presentOAuthClient),
+      data: oauthClients.map((oauthClient) =>
+        can(user, "update", oauthClient)
+          ? presentOAuthClient(oauthClient)
+          : presentPublishedOAuthClient(oauthClient)
+      ),
       policies: presentPolicies(user, oauthClients),
     };
   }
@@ -64,9 +69,10 @@ router.post(
     }
 
     const isInternalApp = oauthClient.teamId === user.teamId;
+    const canUpdate = can(user, "update", oauthClient);
 
     ctx.body = {
-      data: isInternalApp
+      data: canUpdate
         ? presentOAuthClient(oauthClient)
         : presentPublishedOAuthClient(oauthClient),
       policies: isInternalApp ? presentPolicies(user, [oauthClient]) : [],