feat(oauth): PKCE exemption for allowlisted confidential clients (Copilot Studio)

.env.example

@@ -82,6 +82,11 @@ ALLOW_OPEN_REGISTRATION=false
 #   oauth2  — OAuth 2.0 Authorization Code (PKCE) + Client Credentials
 #   both    — OAuth 2.0 + Legacy (accepts either)
 MCP_AUTH_MODE=oauth2
+# PKCE (S256) is required on /authorize. Platforms that cannot send it, such as
+# Microsoft Copilot Studio connectors, can be let through by client id. Only
+# confidential clients (registered with client_secret_basic/post and a secret)
+# are exempted; a listed public client still has to use PKCE.
+# OAUTH_PKCE_EXEMPT_CLIENT_IDS=
 
 # Legacy auth tokens (used when MCP_AUTH_MODE=legacy or both)
 # MCP_BEARER_TOKEN=your-secret-token

docker-compose.cloud.yml

@@ -109,6 +109,11 @@ x-backend: &backend
     - SERVER_URL=https://${DOMAIN}
     - FRONTEND_URL=https://${DOMAIN}
     - MCP_AUTH_MODE=${MCP_AUTH_MODE:-oauth2}
+    # OAuth clients allowed to authorize without PKCE (comma-separated ids).
+    # Only for confidential clients registered with a secret, such as the
+    # Microsoft Copilot Studio connector, whose platform cannot send PKCE.
+    # Unset = PKCE required for every client.
+    - OAUTH_PKCE_EXEMPT_CLIENT_IDS=${OAUTH_PKCE_EXEMPT_CLIENT_IDS:-}
     # MCP Streamable HTTP response framing. `true` = single-shot
     # `application/json` responses; `false` = SSE-framed `text/event-stream`.
     # Microsoft Copilot Studio's MCP client cannot deserialize SSE-framed

packages/backend/src/auth/authorize-pkce.middleware.spec.ts

@@ -25,10 +25,10 @@ describe('AuthorizePkceMiddleware', () => {
 
   const bodyOf = () => res.json.mock.calls[0][0];
 
-  it('allows a conforming S256 authorization request', () => {
+  it('allows a conforming S256 authorization request', async () => {
     const challenge = s256(randomBytes(32).toString('base64url'));
 
-    middleware.use(
+    await middleware.use(
       req({ client_id: 'c1', code_challenge: challenge, code_challenge_method: 'S256' }),
       res,
       next,
@@ -38,20 +38,20 @@ describe('AuthorizePkceMiddleware', () => {
     expect(res.status).not.toHaveBeenCalled();
   });
 
-  it('rejects a request with no code_challenge', () => {
-    middleware.use(req({ client_id: 'c1' }), res, next);
+  it('rejects a request with no code_challenge', async () => {
+    await middleware.use(req({ client_id: 'c1' }), res, next);
 
     expect(next).not.toHaveBeenCalled();
     expect(res.status).toHaveBeenCalledWith(400);
     expect(bodyOf().error).toBe('invalid_request');
     expect(bodyOf().error_description).toMatch(/code_challenge is required/);
   });
 
-  it("rejects code_challenge_method 'plain'", () => {
+  it("rejects code_challenge_method 'plain'", async () => {
     // `plain` puts the verifier in the clear on the authorization request, so
     // it defends against nothing an attacker who can read the request cannot
     // already do. Upstream defaults to exactly this when the method is absent.
-    middleware.use(
+    await middleware.use(
       req({ code_challenge: s256('v'), code_challenge_method: 'plain' }),
       res,
       next,
@@ -61,15 +61,15 @@ describe('AuthorizePkceMiddleware', () => {
     expect(bodyOf().error_description).toMatch(/S256/);
   });
 
-  it('rejects a challenge with no method (upstream would assume plain)', () => {
-    middleware.use(req({ code_challenge: s256('v') }), res, next);
+  it('rejects a challenge with no method (upstream would assume plain)', async () => {
+    await middleware.use(req({ code_challenge: s256('v') }), res, next);
 
     expect(next).not.toHaveBeenCalled();
     expect(res.status).toHaveBeenCalledWith(400);
   });
 
-  it('rejects a malformed challenge', () => {
-    middleware.use(
+  it('rejects a malformed challenge', async () => {
+    await middleware.use(
       req({ code_challenge: 'too-short', code_challenge_method: 'S256' }),
       res,
       next,
@@ -79,10 +79,10 @@ describe('AuthorizePkceMiddleware', () => {
     expect(bodyOf().error_description).toMatch(/base64url/);
   });
 
-  it('never redirects to a client-supplied redirect_uri', () => {
+  it('never redirects to a client-supplied redirect_uri', async () => {
     // Redirecting on error would mean trusting an unvalidated client URI,
     // which is how an authorization endpoint becomes an open redirector.
-    middleware.use(
+    await middleware.use(
       req({ client_id: 'c1', redirect_uri: 'https://evil.tld/cb' }),
       res,
       next,
@@ -93,8 +93,106 @@ describe('AuthorizePkceMiddleware', () => {
     expect(JSON.stringify(bodyOf())).not.toContain('evil.tld');
   });
 
-  it('leaves non-GET requests alone', () => {
-    middleware.use(req({}, 'POST'), res, next);
+  it('leaves non-GET requests alone', async () => {
+    await middleware.use(req({}, 'POST'), res, next);
     expect(next).toHaveBeenCalled();
   });
+
+  describe('OAUTH_PKCE_EXEMPT_CLIENT_IDS', () => {
+    const original = process.env.OAUTH_PKCE_EXEMPT_CLIENT_IDS;
+    let findUnique: jest.Mock;
+
+    const withClient = (client: Record<string, unknown> | null) => {
+      findUnique = jest.fn().mockResolvedValue(client);
+      middleware = new AuthorizePkceMiddleware({
+        oAuthClient: { findUnique },
+      } as any);
+    };
+
+    const confidential = {
+      tokenEndpointAuthMethod: 'client_secret_post',
+      clientSecret: 's3cret',
+    };
+
+    beforeEach(() => {
+      process.env.OAUTH_PKCE_EXEMPT_CLIENT_IDS = 'copilot-studio, other';
+    });
+    afterAll(() => {
+      if (original === undefined) delete process.env.OAUTH_PKCE_EXEMPT_CLIENT_IDS;
+      else process.env.OAUTH_PKCE_EXEMPT_CLIENT_IDS = original;
+    });
+
+    it('lets a listed confidential client authorize without PKCE', async () => {
+      withClient(confidential);
+      await middleware.use(req({ client_id: 'copilot-studio' }), res, next);
+
+      expect(next).toHaveBeenCalled();
+      expect(res.status).not.toHaveBeenCalled();
+      expect(findUnique).toHaveBeenCalledWith(
+        expect.objectContaining({ where: { clientId: 'copilot-studio' } }),
+      );
+    });
+
+    it('accepts client_secret_basic too', async () => {
+      withClient({ ...confidential, tokenEndpointAuthMethod: 'client_secret_basic' });
+      await middleware.use(req({ client_id: 'other' }), res, next);
+      expect(next).toHaveBeenCalled();
+    });
+
+    it('keeps PKCE required for a listed public client', async () => {
+      // A public client has no secret, so nothing would stand in for PKCE.
+      withClient({ tokenEndpointAuthMethod: 'none', clientSecret: null });
+      await middleware.use(req({ client_id: 'copilot-studio' }), res, next);
+
+      expect(next).not.toHaveBeenCalled();
+      expect(res.status).toHaveBeenCalledWith(400);
+    });
+
+    it('keeps PKCE required for a listed client with no stored secret', async () => {
+      withClient({ tokenEndpointAuthMethod: 'client_secret_post', clientSecret: null });
+      await middleware.use(req({ client_id: 'copilot-studio' }), res, next);
+      expect(res.status).toHaveBeenCalledWith(400);
+    });
+
+    it('keeps PKCE required for a listed id that is not registered', async () => {
+      withClient(null);
+      await middleware.use(req({ client_id: 'copilot-studio' }), res, next);
+      expect(res.status).toHaveBeenCalledWith(400);
+    });
+
+    it('never looks up clients that are not listed', async () => {
+      withClient(confidential);
+      await middleware.use(req({ client_id: 'some-dcr-client' }), res, next);
+
+      expect(findUnique).not.toHaveBeenCalled();
+      expect(res.status).toHaveBeenCalledWith(400);
+    });
+
+    it('still validates a challenge a listed client chooses to send', async () => {
+      withClient(confidential);
+      await middleware.use(
+        req({ client_id: 'copilot-studio', code_challenge: 'x', code_challenge_method: 'plain' }),
+        res,
+        next,
+      );
+      expect(res.status).toHaveBeenCalledWith(400);
+    });
+
+    it('does not exempt a request that names a method without a challenge', async () => {
+      withClient(confidential);
+      await middleware.use(
+        req({ client_id: 'copilot-studio', code_challenge_method: 'plain' }),
+        res,
+        next,
+      );
+      expect(res.status).toHaveBeenCalledWith(400);
+    });
+
+    it('fails closed when the lookup throws', async () => {
+      findUnique = jest.fn().mockRejectedValue(new Error('db down'));
+      middleware = new AuthorizePkceMiddleware({ oAuthClient: { findUnique } } as any);
+      await middleware.use(req({ client_id: 'copilot-studio' }), res, next);
+      expect(res.status).toHaveBeenCalledWith(400);
+    });
+  });
 });

packages/backend/src/auth/authorize-pkce.middleware.ts

@@ -1,5 +1,9 @@
-import { Injectable, Logger, NestMiddleware } from '@nestjs/common';
+import { Injectable, Logger, NestMiddleware, Optional } from '@nestjs/common';
 import type { Request, Response, NextFunction } from 'express';
+import { PrismaService } from '../common/prisma.service';
+
+/** Token-endpoint auth methods that prove possession of a client secret. */
+const SECRET_AUTH_METHODS = new Set(['client_secret_basic', 'client_secret_post']);
 
 /**
  * Enforces PKCE with S256 on GET /authorize.
@@ -24,12 +28,25 @@ import type { Request, Response, NextFunction } from 'express';
  * `redirect_uri`. Redirecting would require trusting a client-supplied URI
  * before it has been validated against a registration, which is exactly how an
  * authorization endpoint becomes an open redirector.
+ *
+ * ONE EXCEPTION. Some platforms cannot send PKCE at all: Power Platform
+ * connectors (Microsoft Copilot Studio) run a plain OAuth 2.0 authorization
+ * code flow with a client secret. An operator can list such clients in
+ * `OAUTH_PKCE_EXEMPT_CLIENT_IDS`; a listed client may omit the challenge only
+ * while its registration is confidential (client_secret_basic/post with a
+ * stored secret). The token endpoint then refuses the code without that
+ * secret, which is what PKCE would otherwise protect, and the code is still
+ * bound to the client's registered redirect URI. A request that does carry a
+ * challenge is validated as usual, and a client registered through open DCR is
+ * never exempt unless the operator names it.
  */
 @Injectable()
 export class AuthorizePkceMiddleware implements NestMiddleware {
   private readonly logger = new Logger(AuthorizePkceMiddleware.name);
 
-  use(req: Request, res: Response, next: NextFunction): void {
+  constructor(@Optional() private readonly prisma?: PrismaService) {}
+
+  async use(req: Request, res: Response, next: NextFunction): Promise<void> {
     // Only the authorization request itself carries PKCE parameters.
     if (req.method !== 'GET') {
       return next();
@@ -40,6 +57,12 @@ export class AuthorizePkceMiddleware implements NestMiddleware {
     const method = query.code_challenge_method;
 
     if (typeof challenge !== 'string' || challenge.trim() === '') {
+      if (
+        method === undefined &&
+        (await this.isExemptConfidentialClient(query.client_id))
+      ) {
+        return next();
+      }
       this.logger.warn(
         `Rejecting /authorize without PKCE (client_id=${String(query.client_id ?? '<none>')})`,
       );
@@ -72,6 +95,43 @@ export class AuthorizePkceMiddleware implements NestMiddleware {
     next();
   }
 
+  /**
+   * True only for a client the operator listed in OAUTH_PKCE_EXEMPT_CLIENT_IDS
+   * whose registration requires a client secret at the token endpoint. The
+   * database is consulted only for listed ids, so ordinary requests never pay
+   * for a lookup.
+   */
+  private async isExemptConfidentialClient(clientId: unknown): Promise<boolean> {
+    if (typeof clientId !== 'string' || !clientId) return false;
+    const listed = (process.env.OAUTH_PKCE_EXEMPT_CLIENT_IDS ?? '')
+      .split(',')
+      .map((id) => id.trim())
+      .filter(Boolean);
+    if (!listed.includes(clientId) || !this.prisma) return false;
+
+    try {
+      const client = await this.prisma.oAuthClient.findUnique({
+        where: { clientId },
+        select: { tokenEndpointAuthMethod: true, clientSecret: true },
+      });
+      if (
+        client &&
+        SECRET_AUTH_METHODS.has(client.tokenEndpointAuthMethod) &&
+        !!client.clientSecret
+      ) {
+        return true;
+      }
+      this.logger.warn(
+        `Client ${clientId} is listed in OAUTH_PKCE_EXEMPT_CLIENT_IDS but is not a confidential client; PKCE stays required`,
+      );
+    } catch (err) {
+      this.logger.warn(
+        `PKCE exemption lookup failed for ${clientId}: ${(err as Error).message}`,
+      );
+    }
+    return false;
+  }
+
   private reject(res: Response, description: string): void {
     res
       .status(400)