diff --git a/packages/twenty-server/src/engine/core-modules/application/application-oauth/oauth.service.ts b/packages/twenty-server/src/engine/core-modules/application/application-oauth/oauth.service.ts index df4a3bb4d0..148d483591 100644 --- a/packages/twenty-server/src/engine/core-modules/application/application-oauth/oauth.service.ts +++ b/packages/twenty-server/src/engine/core-modules/application/application-oauth/oauth.service.ts @@ -70,6 +70,13 @@ export class OAuthService { const applicationRegistration = clientValidation; + if (applicationRegistration.oAuthClientSecretHash && !clientSecret) { + return this.errorResponse( + 'invalid_client', + 'Client authentication required for confidential clients', + ); + } + if (clientSecret) { const secretError = await this.validateClientSecret( applicationRegistration, diff --git a/packages/twenty-server/test/integration/oauth/suites/oauth.integration-spec.ts b/packages/twenty-server/test/integration/oauth/suites/oauth.integration-spec.ts index 2953f05cb6..f3aa4adb19 100644 --- a/packages/twenty-server/test/integration/oauth/suites/oauth.integration-spec.ts +++ b/packages/twenty-server/test/integration/oauth/suites/oauth.integration-spec.ts @@ -29,7 +29,7 @@ const insertRegistration = async ( ds: DataSource, params: { name: string; - clientSecretHash: string; + clientSecretHash: string | null; redirectUris: string[]; scopes: string[]; }, @@ -129,6 +129,8 @@ describe('OAuth (integration)', () => { let testClientSecret: string; let testApplication: TestApplication; + let publicRegistration: TestRegistration; + let autoInstallRegistration: TestRegistration; let autoInstallClientSecret: string; @@ -159,6 +161,22 @@ describe('OAuth (integration)', () => { applicationRegistrationId: testRegistration.id, }); + publicRegistration = await insertRegistration(ds, { + name: 'OAuth Public PKCE Test App', + clientSecretHash: null, + redirectUris: ['https://example.com/callback'], + scopes: ['read', 'write'], + }); + createdEntityIds.registrations.push(publicRegistration.id); + + const publicApplication = await insertApplication(ds, { + universalIdentifier: publicRegistration.universalIdentifier, + name: publicRegistration.name, + workspaceId: TEST_WORKSPACE_ID, + applicationRegistrationId: publicRegistration.id, + }); + createdEntityIds.applications.push(publicApplication.id); + autoInstallClientSecret = crypto.randomBytes(32).toString('hex'); const autoInstallSecretHash = await bcrypt.hash( autoInstallClientSecret, @@ -430,13 +448,13 @@ describe('OAuth (integration)', () => { it('should require either client_secret or code_verifier', async () => { const code = await createAuthorizationCode( - testRegistration.oAuthClientId, + publicRegistration.oAuthClientId, ); const res = await postToken({ grant_type: 'authorization_code', code, - client_id: testRegistration.oAuthClientId, + client_id: publicRegistration.oAuthClientId, redirect_uri: 'https://example.com/callback', }).expect(400); @@ -479,13 +497,13 @@ describe('OAuth (integration)', () => { it('should exchange code with valid PKCE verifier', async () => { const { code, codeVerifier } = await createAuthCodeWithPkce( - testRegistration.oAuthClientId, + publicRegistration.oAuthClientId, ); const res = await postToken({ grant_type: 'authorization_code', code, - client_id: testRegistration.oAuthClientId, + client_id: publicRegistration.oAuthClientId, code_verifier: codeVerifier, redirect_uri: 'https://example.com/callback', }).expect(200); @@ -497,13 +515,13 @@ describe('OAuth (integration)', () => { it('should reject code with wrong PKCE verifier', async () => { const { code } = await createAuthCodeWithPkce( - testRegistration.oAuthClientId, + publicRegistration.oAuthClientId, ); const res = await postToken({ grant_type: 'authorization_code', code, - client_id: testRegistration.oAuthClientId, + client_id: publicRegistration.oAuthClientId, code_verifier: 'wrong-verifier', redirect_uri: 'https://example.com/callback', }).expect(400); @@ -513,6 +531,22 @@ describe('OAuth (integration)', () => { it('should require code_verifier when PKCE was used in authorization', async () => { const { code } = await createAuthCodeWithPkce( + publicRegistration.oAuthClientId, + ); + + const res = await postToken({ + grant_type: 'authorization_code', + code, + client_id: publicRegistration.oAuthClientId, + redirect_uri: 'https://example.com/callback', + }).expect(400); + + expect(res.body.error).toBe('invalid_request'); + expect(res.body.error_description).toContain('code_verifier is required'); + }); + + it('should reject a confidential client that presents only PKCE and no client_secret', async () => { + const { code, codeVerifier } = await createAuthCodeWithPkce( testRegistration.oAuthClientId, ); @@ -520,12 +554,11 @@ describe('OAuth (integration)', () => { grant_type: 'authorization_code', code, client_id: testRegistration.oAuthClientId, - client_secret: testClientSecret, + code_verifier: codeVerifier, redirect_uri: 'https://example.com/callback', - }).expect(400); + }).expect(401); - expect(res.body.error).toBe('invalid_request'); - expect(res.body.error_description).toContain('code_verifier is required'); + expect(res.body.error).toBe('invalid_client'); }); });