diff --git a/packages/cli/src/controllers/oauth/__tests__/oauth1-credential.controller.test.ts b/packages/cli/src/controllers/oauth/__tests__/oauth1-credential.controller.test.ts index ac18c6d5fe3..8bad8f4545a 100644 --- a/packages/cli/src/controllers/oauth/__tests__/oauth1-credential.controller.test.ts +++ b/packages/cli/src/controllers/oauth/__tests__/oauth1-credential.controller.test.ts @@ -43,6 +43,7 @@ describe('OAuth1CredentialController', () => { expect(authUri).toEqual('https://example.domain/oauth/authorize?oauth_token=random-token'); expect(oauthService.generateAOauth1AuthUri).toHaveBeenCalledWith(mockResolvedCredential, { cid: '1', + origin: 'static-credential', userId: '123', }); }); @@ -53,6 +54,7 @@ describe('OAuth1CredentialController', () => { JSON.stringify({ token: 'token', cid: '1', + origin: 'static-credential', createdAt: timestamp, }), ).toString('base64'); @@ -78,8 +80,14 @@ describe('OAuth1CredentialController', () => { ); }); - it('should exchange the code for a valid token, and save it to DB', async () => { + it('should exchange the code for a valid token, and save it to DB for static credential', async () => { const mockResolvedCredential = mock({ id: '1' }); + const mockState = { + token: 'token', + cid: '1', + origin: 'static-credential' as const, + createdAt: timestamp, + }; oauthService.getCredential.mockResolvedValueOnce(mockResolvedCredential); // @ts-ignore oauthService.getDecryptedData.mockResolvedValue({ csrfSecret: 'invalid' }); @@ -92,15 +100,220 @@ describe('OAuth1CredentialController', () => { mockResolvedCredential, { csrfSecret: 'invalid' }, { accessTokenUrl: 'https://example.domain/oauth/access_token' }, + mockState, ]); - jest.mocked(axios).post.mockResolvedValueOnce({ data: { access_token: 'new_token' } }); + jest + .mocked(axios.post) + .mockResolvedValueOnce({ data: 'oauth_token=token&oauth_token_secret=secret' } as any); await controller.handleCallback(req, res); // @ts-ignore expect(oauthService.encryptAndSaveData).toHaveBeenCalledWith( mockResolvedCredential, expect.objectContaining({ - oauthTokenData: { access_token: 'new_token' }, + oauthTokenData: expect.objectContaining({ + oauth_token: 'token', + oauth_token_secret: 'secret', + }), + }), + ['csrfSecret'], + ); + expect(res.render).toHaveBeenCalledWith('oauth-callback'); + }); + + it('should handle dynamic credential callback successfully', async () => { + const mockResolvedCredential = mock({ id: '1' }); + const mockState = { + token: 'token', + cid: '1', + origin: 'dynamic-credential' as const, + credentialResolverId: 'resolver-id', + authorizationHeader: 'Bearer token123', + createdAt: timestamp, + }; + const dynamicState = Buffer.from(JSON.stringify(mockState)).toString('base64'); + const dynamicReq = mock({ + query: { + oauth_verifier: 'verifier', + oauth_token: 'token', + state: dynamicState, + }, + }); + + oauthService.resolveCredential.mockResolvedValueOnce([ + mockResolvedCredential, + { csrfSecret: 'invalid' }, + { accessTokenUrl: 'https://example.domain/oauth/access_token' }, + mockState, + ]); + jest + .mocked(axios.post) + .mockResolvedValueOnce({ data: 'oauth_token=token&oauth_token_secret=secret' } as any); + oauthService.saveDynamicCredential.mockResolvedValueOnce(undefined); + + await controller.handleCallback(dynamicReq, res); + + expect(oauthService.saveDynamicCredential).toHaveBeenCalledWith( + mockResolvedCredential, + expect.objectContaining({ + oauth_token: 'token', + oauth_token_secret: 'secret', + }), + 'token123', + 'resolver-id', + ); + expect(oauthService.encryptAndSaveData).not.toHaveBeenCalled(); + expect(res.render).toHaveBeenCalledWith('oauth-callback'); + }); + + it('should render error when credentialResolverId is missing for dynamic credential', async () => { + const mockResolvedCredential = mock({ id: '1' }); + const mockState = { + token: 'token', + cid: '1', + origin: 'dynamic-credential' as const, + authorizationHeader: 'Bearer token123', + createdAt: timestamp, + }; + const dynamicState = Buffer.from(JSON.stringify(mockState)).toString('base64'); + const dynamicReq = mock({ + query: { + oauth_verifier: 'verifier', + oauth_token: 'token', + state: dynamicState, + }, + }); + + oauthService.resolveCredential.mockResolvedValueOnce([ + mockResolvedCredential, + { csrfSecret: 'invalid' }, + { accessTokenUrl: 'https://example.domain/oauth/access_token' }, + mockState, + ]); + jest + .mocked(axios.post) + .mockResolvedValueOnce({ data: 'oauth_token=token&oauth_token_secret=secret' } as any); + + await controller.handleCallback(dynamicReq, res); + + expect(oauthService.renderCallbackError).toHaveBeenCalledWith( + res, + 'Credential resolver ID is required', + ); + expect(oauthService.saveDynamicCredential).not.toHaveBeenCalled(); + }); + + it('should render error when authorizationHeader is missing for dynamic credential', async () => { + const mockResolvedCredential = mock({ id: '1' }); + const mockState = { + token: 'token', + cid: '1', + origin: 'dynamic-credential' as const, + credentialResolverId: 'resolver-id', + createdAt: timestamp, + }; + const dynamicState = Buffer.from(JSON.stringify(mockState)).toString('base64'); + const dynamicReq = mock({ + query: { + oauth_verifier: 'verifier', + oauth_token: 'token', + state: dynamicState, + }, + }); + + oauthService.resolveCredential.mockResolvedValueOnce([ + mockResolvedCredential, + { csrfSecret: 'invalid' }, + { accessTokenUrl: 'https://example.domain/oauth/access_token' }, + mockState, + ]); + jest + .mocked(axios.post) + .mockResolvedValueOnce({ data: 'oauth_token=token&oauth_token_secret=secret' } as any); + + await controller.handleCallback(dynamicReq, res); + + expect(oauthService.renderCallbackError).toHaveBeenCalledWith( + res, + 'Authorization header is required', + ); + expect(oauthService.saveDynamicCredential).not.toHaveBeenCalled(); + }); + + it('should render error when authorizationHeader does not start with Bearer', async () => { + const mockResolvedCredential = mock({ id: '1' }); + const mockState = { + token: 'token', + cid: '1', + origin: 'dynamic-credential' as const, + credentialResolverId: 'resolver-id', + authorizationHeader: 'Invalid token123', + createdAt: timestamp, + }; + const dynamicState = Buffer.from(JSON.stringify(mockState)).toString('base64'); + const dynamicReq = mock({ + query: { + oauth_verifier: 'verifier', + oauth_token: 'token', + state: dynamicState, + }, + }); + + oauthService.resolveCredential.mockResolvedValueOnce([ + mockResolvedCredential, + { csrfSecret: 'invalid' }, + { accessTokenUrl: 'https://example.domain/oauth/access_token' }, + mockState, + ]); + jest + .mocked(axios.post) + .mockResolvedValueOnce({ data: 'oauth_token=token&oauth_token_secret=secret' } as any); + + await controller.handleCallback(dynamicReq, res); + + expect(oauthService.renderCallbackError).toHaveBeenCalledWith( + res, + 'Authorization header is required', + ); + expect(oauthService.saveDynamicCredential).not.toHaveBeenCalled(); + }); + + it('should handle static credential callback when origin is undefined', async () => { + const mockResolvedCredential = mock({ id: '1' }); + const mockState = { + token: 'token', + cid: '1', + origin: 'static-credential' as const, + createdAt: timestamp, + }; + const undefinedOriginState = Buffer.from(JSON.stringify(mockState)).toString('base64'); + const undefinedOriginReq = mock({ + query: { + oauth_verifier: 'verifier', + oauth_token: 'token', + state: undefinedOriginState, + }, + }); + + oauthService.resolveCredential.mockResolvedValueOnce([ + mockResolvedCredential, + { csrfSecret: 'invalid' }, + { accessTokenUrl: 'https://example.domain/oauth/access_token' }, + mockState, + ]); + jest + .mocked(axios.post) + .mockResolvedValueOnce({ data: 'oauth_token=token&oauth_token_secret=secret' } as any); + + await controller.handleCallback(undefinedOriginReq, res); + + expect(oauthService.encryptAndSaveData).toHaveBeenCalledWith( + mockResolvedCredential, + expect.objectContaining({ + oauthTokenData: expect.objectContaining({ + oauth_token: 'token', + oauth_token_secret: 'secret', + }), }), ['csrfSecret'], ); diff --git a/packages/cli/src/controllers/oauth/__tests__/oauth2-credential.controller.test.ts b/packages/cli/src/controllers/oauth/__tests__/oauth2-credential.controller.test.ts index f58c2f79ae7..57923f5c106 100644 --- a/packages/cli/src/controllers/oauth/__tests__/oauth2-credential.controller.test.ts +++ b/packages/cli/src/controllers/oauth/__tests__/oauth2-credential.controller.test.ts @@ -68,6 +68,7 @@ describe('OAuth2CredentialController', () => { expect(authUri).toContain('https://example.domain/oauth2/auth'); expect(oauthService.generateAOauth2AuthUri).toHaveBeenCalledWith(mockResolvedCredential, { cid: '1', + origin: 'static-credential', userId: '123', }); }); @@ -79,6 +80,7 @@ describe('OAuth2CredentialController', () => { token: 'token', cid: '1', userId: '123', + origin: 'static-credential', createdAt: timestamp, }), ).toString('base64'); @@ -97,7 +99,7 @@ describe('OAuth2CredentialController', () => { ); }); - it('should exchange the code for a valid token, and save it to DB', async () => { + it('should exchange the code for a valid token, and save it to DB for static credential', async () => { const { ClientOAuth2 } = await import('@n8n/client-oauth2'); const mockGetToken = jest.fn().mockResolvedValue({ data: { access_token: 'new_token', refresh_token: 'refresh_token' }, @@ -112,6 +114,13 @@ describe('OAuth2CredentialController', () => { ); const mockResolvedCredential = mock({ id: '1' }); + const mockState = { + token: 'token', + cid: '1', + userId: '123', + origin: 'static-credential' as const, + createdAt: timestamp, + }; oauthService.resolveCredential.mockResolvedValueOnce([ mockResolvedCredential, { csrfSecret: 'csrf-secret' }, @@ -124,6 +133,7 @@ describe('OAuth2CredentialController', () => { grantType: 'authorizationCode', authentication: 'header', }, + mockState, ]); oauthService.getBaseUrl.mockReturnValue('http://localhost:5678/rest/oauth2-credential'); externalHooks.run.mockResolvedValue(undefined); @@ -149,6 +159,306 @@ describe('OAuth2CredentialController', () => { expect(externalHooks.run).toHaveBeenCalledWith('oauth2.callback', expect.any(Array)); }); + it('should handle dynamic credential callback successfully', async () => { + const { ClientOAuth2 } = await import('@n8n/client-oauth2'); + const mockGetToken = jest.fn().mockResolvedValue({ + data: { access_token: 'new_token', refresh_token: 'refresh_token' }, + }); + jest.mocked(ClientOAuth2).mockImplementation( + () => + ({ + code: { + getToken: mockGetToken, + }, + }) as any, + ); + + const mockResolvedCredential = mock({ id: '1' }); + const mockDecryptedData = { csrfSecret: 'csrf-secret', existing: 'data' }; + const mockState = { + token: 'token', + cid: '1', + userId: '123', + origin: 'dynamic-credential' as const, + credentialResolverId: 'resolver-id', + authorizationHeader: 'Bearer token123', + createdAt: timestamp, + }; + const dynamicState = Buffer.from(JSON.stringify(mockState)).toString('base64'); + oauthService.resolveCredential.mockResolvedValueOnce([ + mockResolvedCredential, + mockDecryptedData, + { + clientId: 'client_id', + clientSecret: 'client_secret', + authUrl: 'https://example.domain/oauth2/auth', + accessTokenUrl: 'https://example.domain/oauth2/token', + scope: 'openid', + grantType: 'authorizationCode', + authentication: 'header', + }, + mockState, + ]); + oauthService.getBaseUrl.mockReturnValue('http://localhost:5678/rest/oauth2-credential'); + externalHooks.run.mockResolvedValue(undefined); + oauthService.saveDynamicCredential.mockResolvedValueOnce(undefined); + + const req = mock({ + query: { + code: 'auth_code', + state: dynamicState, + }, + originalUrl: '/oauth2-credential/callback?code=auth_code&state=state', + }); + + await controller.handleCallback(req, res); + + // The controller passes decryptedDataOriginal directly, not the merged oauthTokenData + expect(oauthService.saveDynamicCredential).toHaveBeenCalledWith( + mockResolvedCredential, + mockDecryptedData, + 'token123', + 'resolver-id', + ); + expect(oauthService.encryptAndSaveData).not.toHaveBeenCalled(); + expect(res.render).toHaveBeenCalledWith('oauth-callback'); + }); + + it('should render error when credentialResolverId is missing for dynamic credential', async () => { + const { ClientOAuth2 } = await import('@n8n/client-oauth2'); + const mockGetToken = jest.fn().mockResolvedValue({ + data: { access_token: 'new_token', refresh_token: 'refresh_token' }, + }); + jest.mocked(ClientOAuth2).mockImplementation( + () => + ({ + code: { + getToken: mockGetToken, + }, + }) as any, + ); + + const mockResolvedCredential = mock({ id: '1' }); + const mockState = { + token: 'token', + cid: '1', + userId: '123', + origin: 'dynamic-credential' as const, + authorizationHeader: 'Bearer token123', + createdAt: timestamp, + }; + const dynamicState = Buffer.from(JSON.stringify(mockState)).toString('base64'); + oauthService.resolveCredential.mockResolvedValueOnce([ + mockResolvedCredential, + { csrfSecret: 'csrf-secret' }, + { + clientId: 'client_id', + clientSecret: 'client_secret', + authUrl: 'https://example.domain/oauth2/auth', + accessTokenUrl: 'https://example.domain/oauth2/token', + scope: 'openid', + grantType: 'authorizationCode', + authentication: 'header', + }, + mockState, + ]); + oauthService.getBaseUrl.mockReturnValue('http://localhost:5678/rest/oauth2-credential'); + externalHooks.run.mockResolvedValue(undefined); + + const req = mock({ + query: { + code: 'auth_code', + state: dynamicState, + }, + originalUrl: '/oauth2-credential/callback?code=auth_code&state=state', + }); + + await controller.handleCallback(req, res); + + expect(oauthService.renderCallbackError).toHaveBeenCalledWith( + res, + 'Credential resolver ID is required', + ); + expect(oauthService.saveDynamicCredential).not.toHaveBeenCalled(); + }); + + it('should render error when authorizationHeader is missing for dynamic credential', async () => { + const { ClientOAuth2 } = await import('@n8n/client-oauth2'); + const mockGetToken = jest.fn().mockResolvedValue({ + data: { access_token: 'new_token', refresh_token: 'refresh_token' }, + }); + jest.mocked(ClientOAuth2).mockImplementation( + () => + ({ + code: { + getToken: mockGetToken, + }, + }) as any, + ); + + const mockResolvedCredential = mock({ id: '1' }); + const mockState = { + token: 'token', + cid: '1', + userId: '123', + origin: 'dynamic-credential' as const, + credentialResolverId: 'resolver-id', + createdAt: timestamp, + }; + const dynamicState = Buffer.from(JSON.stringify(mockState)).toString('base64'); + oauthService.resolveCredential.mockResolvedValueOnce([ + mockResolvedCredential, + { csrfSecret: 'csrf-secret' }, + { + clientId: 'client_id', + clientSecret: 'client_secret', + authUrl: 'https://example.domain/oauth2/auth', + accessTokenUrl: 'https://example.domain/oauth2/token', + scope: 'openid', + grantType: 'authorizationCode', + authentication: 'header', + }, + mockState, + ]); + oauthService.getBaseUrl.mockReturnValue('http://localhost:5678/rest/oauth2-credential'); + externalHooks.run.mockResolvedValue(undefined); + + const req = mock({ + query: { + code: 'auth_code', + state: dynamicState, + }, + originalUrl: '/oauth2-credential/callback?code=auth_code&state=state', + }); + + await controller.handleCallback(req, res); + + expect(oauthService.renderCallbackError).toHaveBeenCalledWith( + res, + 'Authorization header is required', + ); + expect(oauthService.saveDynamicCredential).not.toHaveBeenCalled(); + }); + + it('should render error when authorizationHeader does not start with Bearer', async () => { + const { ClientOAuth2 } = await import('@n8n/client-oauth2'); + const mockGetToken = jest.fn().mockResolvedValue({ + data: { access_token: 'new_token', refresh_token: 'refresh_token' }, + }); + jest.mocked(ClientOAuth2).mockImplementation( + () => + ({ + code: { + getToken: mockGetToken, + }, + }) as any, + ); + + const mockResolvedCredential = mock({ id: '1' }); + const mockState = { + token: 'token', + cid: '1', + userId: '123', + origin: 'dynamic-credential' as const, + credentialResolverId: 'resolver-id', + authorizationHeader: 'Invalid token123', + createdAt: timestamp, + }; + const dynamicState = Buffer.from(JSON.stringify(mockState)).toString('base64'); + oauthService.resolveCredential.mockResolvedValueOnce([ + mockResolvedCredential, + { csrfSecret: 'csrf-secret' }, + { + clientId: 'client_id', + clientSecret: 'client_secret', + authUrl: 'https://example.domain/oauth2/auth', + accessTokenUrl: 'https://example.domain/oauth2/token', + scope: 'openid', + grantType: 'authorizationCode', + authentication: 'header', + }, + mockState, + ]); + oauthService.getBaseUrl.mockReturnValue('http://localhost:5678/rest/oauth2-credential'); + externalHooks.run.mockResolvedValue(undefined); + + const req = mock({ + query: { + code: 'auth_code', + state: dynamicState, + }, + originalUrl: '/oauth2-credential/callback?code=auth_code&state=state', + }); + + await controller.handleCallback(req, res); + + expect(oauthService.renderCallbackError).toHaveBeenCalledWith( + res, + 'Authorization header is required', + ); + expect(oauthService.saveDynamicCredential).not.toHaveBeenCalled(); + }); + + it('should handle static credential callback when origin is undefined', async () => { + const { ClientOAuth2 } = await import('@n8n/client-oauth2'); + const mockGetToken = jest.fn().mockResolvedValue({ + data: { access_token: 'new_token', refresh_token: 'refresh_token' }, + }); + jest.mocked(ClientOAuth2).mockImplementation( + () => + ({ + code: { + getToken: mockGetToken, + }, + }) as any, + ); + + const mockResolvedCredential = mock({ id: '1' }); + const mockState = { + token: 'token', + cid: '1', + userId: '123', + origin: 'static-credential' as const, + createdAt: timestamp, + }; + const undefinedOriginState = Buffer.from(JSON.stringify(mockState)).toString('base64'); + oauthService.resolveCredential.mockResolvedValueOnce([ + mockResolvedCredential, + { csrfSecret: 'csrf-secret' }, + { + clientId: 'client_id', + clientSecret: 'client_secret', + authUrl: 'https://example.domain/oauth2/auth', + accessTokenUrl: 'https://example.domain/oauth2/token', + scope: 'openid', + grantType: 'authorizationCode', + authentication: 'header', + }, + mockState, + ]); + oauthService.getBaseUrl.mockReturnValue('http://localhost:5678/rest/oauth2-credential'); + externalHooks.run.mockResolvedValue(undefined); + + const req = mock({ + query: { + code: 'auth_code', + state: undefinedOriginState, + }, + originalUrl: '/oauth2-credential/callback?code=auth_code&state=state', + }); + + await controller.handleCallback(req, res); + + expect(oauthService.encryptAndSaveData).toHaveBeenCalledWith( + mockResolvedCredential, + expect.objectContaining({ + oauthTokenData: { access_token: 'new_token', refresh_token: 'refresh_token' }, + }), + ['csrfSecret'], + ); + expect(res.render).toHaveBeenCalledWith('oauth-callback'); + }); + it('should handle PKCE flow', async () => { const { ClientOAuth2 } = await import('@n8n/client-oauth2'); const mockGetToken = jest.fn().mockResolvedValue({ @@ -164,6 +474,13 @@ describe('OAuth2CredentialController', () => { ); const mockResolvedCredential = mock({ id: '1' }); + const mockState = { + token: 'token', + cid: '1', + userId: '123', + origin: 'static-credential' as const, + createdAt: timestamp, + }; oauthService.resolveCredential.mockResolvedValueOnce([ mockResolvedCredential, { csrfSecret: 'csrf-secret', codeVerifier: 'code_verifier' }, @@ -176,6 +493,7 @@ describe('OAuth2CredentialController', () => { grantType: 'pkce', authentication: 'header', }, + mockState, ]); oauthService.getBaseUrl.mockReturnValue('http://localhost:5678/rest/oauth2-credential'); externalHooks.run.mockResolvedValue(undefined); @@ -214,6 +532,13 @@ describe('OAuth2CredentialController', () => { ); const mockResolvedCredential = mock({ id: '1' }); + const mockState = { + token: 'token', + cid: '1', + userId: '123', + origin: 'static-credential' as const, + createdAt: timestamp, + }; oauthService.resolveCredential.mockResolvedValueOnce([ mockResolvedCredential, { csrfSecret: 'csrf-secret' }, @@ -226,6 +551,7 @@ describe('OAuth2CredentialController', () => { grantType: 'authorizationCode', authentication: 'body', }, + mockState, ]); oauthService.getBaseUrl.mockReturnValue('http://localhost:5678/rest/oauth2-credential'); externalHooks.run.mockResolvedValue(undefined); @@ -266,6 +592,13 @@ describe('OAuth2CredentialController', () => { ); const mockResolvedCredential = mock({ id: '1' }); + const mockState = { + token: 'token', + cid: '1', + userId: '123', + origin: 'static-credential' as const, + createdAt: timestamp, + }; oauthService.resolveCredential.mockResolvedValueOnce([ mockResolvedCredential, { csrfSecret: 'csrf-secret' }, @@ -278,6 +611,7 @@ describe('OAuth2CredentialController', () => { grantType: 'authorizationCode', authentication: 'header', }, + mockState, ]); oauthService.getBaseUrl.mockReturnValue('http://localhost:5678/rest/oauth2-credential'); externalHooks.run.mockResolvedValue(undefined); @@ -321,6 +655,13 @@ describe('OAuth2CredentialController', () => { ); const mockResolvedCredential = mock({ id: '1' }); + const mockState = { + token: 'token', + cid: '1', + userId: '123', + origin: 'static-credential' as const, + createdAt: timestamp, + }; oauthService.resolveCredential.mockResolvedValueOnce([ mockResolvedCredential, { csrfSecret: 'csrf-secret' }, @@ -333,6 +674,7 @@ describe('OAuth2CredentialController', () => { grantType: 'authorizationCode', authentication: 'header', }, + mockState, ]); oauthService.getBaseUrl.mockReturnValue('http://localhost:5678/rest/oauth2-credential'); externalHooks.run.mockResolvedValue(undefined); diff --git a/packages/cli/src/controllers/oauth/oauth1-credential.controller.ts b/packages/cli/src/controllers/oauth/oauth1-credential.controller.ts index 65a5651367b..8e17ce1f7c0 100644 --- a/packages/cli/src/controllers/oauth/oauth1-credential.controller.ts +++ b/packages/cli/src/controllers/oauth/oauth1-credential.controller.ts @@ -26,6 +26,7 @@ export class OAuth1CredentialController { const uri = await this.oauthService.generateAOauth1AuthUri(credential, { cid: credential.id, + origin: 'static-credential', userId: skipAuthOnOAuthCallback ? undefined : req.user.id, }); @@ -51,7 +52,7 @@ export class OAuth1CredentialController { ); } - const [credential, _, oauthCredentials] = + const [credential, _, oauthCredentials, state] = await this.oauthService.resolveCredential(req); // Form URL encoded body https://datatracker.ietf.org/doc/html/rfc5849#section-3.5.2 @@ -67,12 +68,36 @@ export class OAuth1CredentialController { const oauthTokenData = Object.fromEntries(paramParser.entries()); - await this.oauthService.encryptAndSaveData(credential, { oauthTokenData }, ['csrfSecret']); + if (!state.origin || state.origin === 'static-credential') { + await this.oauthService.encryptAndSaveData(credential, { oauthTokenData }, ['csrfSecret']); - this.logger.debug('OAuth1 callback successful for new credential', { - credentialId: credential.id, - }); - return res.render('oauth-callback'); + this.logger.debug('OAuth1 callback successful for new credential', { + credentialId: credential.id, + }); + return res.render('oauth-callback'); + } + + if (state.origin === 'dynamic-credential') { + if (!state.credentialResolverId || typeof state.credentialResolverId !== 'string') { + return this.oauthService.renderCallbackError(res, 'Credential resolver ID is required'); + } + + if ( + !state.authorizationHeader || + typeof state.authorizationHeader !== 'string' || + !state.authorizationHeader.startsWith('Bearer ') + ) { + return this.oauthService.renderCallbackError(res, 'Authorization header is required'); + } + + await this.oauthService.saveDynamicCredential( + credential, + oauthTokenData, + state.authorizationHeader.split('Bearer ')[1], + state.credentialResolverId, + ); + return res.render('oauth-callback'); + } } catch (e) { const error = ensureError(e); return this.oauthService.renderCallbackError( diff --git a/packages/cli/src/controllers/oauth/oauth2-credential.controller.ts b/packages/cli/src/controllers/oauth/oauth2-credential.controller.ts index df0d7bee919..3629279deda 100644 --- a/packages/cli/src/controllers/oauth/oauth2-credential.controller.ts +++ b/packages/cli/src/controllers/oauth/oauth2-credential.controller.ts @@ -11,6 +11,7 @@ import { OAuthRequest } from '@/requests'; import { OauthService, OauthVersion, skipAuthOnOAuthCallback } from '@/oauth/oauth.service'; import { Logger } from '@n8n/backend-common'; import { ExternalHooks } from '@/external-hooks'; +import type { ICredentialDataDecryptedObject } from 'n8n-workflow'; @RestController('/oauth2-credential') export class OAuth2CredentialController { @@ -27,6 +28,7 @@ export class OAuth2CredentialController { const uri = await this.oauthService.generateAOauth2AuthUri(credential, { cid: credential.id, + origin: 'static-credential', userId: req.user.id, }); return uri; @@ -45,7 +47,7 @@ export class OAuth2CredentialController { ); } - const [credential, decryptedDataOriginal, oauthCredentials] = + const [credential, decryptedDataOriginal, oauthCredentials, state] = await this.oauthService.resolveCredential(req); let options: Partial = {}; @@ -88,15 +90,39 @@ export class OAuth2CredentialController { oauthTokenData = { ...(typeof oauthTokenData === 'object' ? oauthTokenData : {}), ...oauthToken.data, - }; + } as ICredentialDataDecryptedObject; - await this.oauthService.encryptAndSaveData(credential, { oauthTokenData }, ['csrfSecret']); + if (!state.origin || state.origin === 'static-credential') { + await this.oauthService.encryptAndSaveData(credential, { oauthTokenData }, ['csrfSecret']); - this.logger.debug('OAuth2 callback successful for credential', { - credentialId: credential.id, - }); + this.logger.debug('OAuth2 callback successful for credential', { + credentialId: credential.id, + }); - return res.render('oauth-callback'); + return res.render('oauth-callback'); + } + + if (state.origin === 'dynamic-credential') { + if (!state.credentialResolverId || typeof state.credentialResolverId !== 'string') { + return this.oauthService.renderCallbackError(res, 'Credential resolver ID is required'); + } + + if ( + !state.authorizationHeader || + typeof state.authorizationHeader !== 'string' || + !state.authorizationHeader.startsWith('Bearer ') + ) { + return this.oauthService.renderCallbackError(res, 'Authorization header is required'); + } + + await this.oauthService.saveDynamicCredential( + credential, + decryptedDataOriginal, + state.authorizationHeader.split('Bearer ')[1], + state.credentialResolverId, + ); + return res.render('oauth-callback'); + } } catch (e) { const error = ensureError(e); return this.oauthService.renderCallbackError( diff --git a/packages/cli/src/modules/dynamic-credentials.ee/__tests__/dynamic-credentials.controller.test.ts b/packages/cli/src/modules/dynamic-credentials.ee/__tests__/dynamic-credentials.controller.test.ts index e0143c259fa..bd4c631f7fc 100644 --- a/packages/cli/src/modules/dynamic-credentials.ee/__tests__/dynamic-credentials.controller.test.ts +++ b/packages/cli/src/modules/dynamic-credentials.ee/__tests__/dynamic-credentials.controller.test.ts @@ -81,6 +81,7 @@ describe('DynamicCredentialsController', () => { expect(enterpriseCredentialsService.getOne).toHaveBeenCalledWith('1'); expect(oauthService.generateAOauth2AuthUri).toHaveBeenCalledWith(mockCredential, { cid: '1', + origin: 'dynamic-credential', authorizationHeader: 'Bearer token123', }); }); @@ -107,6 +108,7 @@ describe('DynamicCredentialsController', () => { expect(enterpriseCredentialsService.getOne).toHaveBeenCalledWith('1'); expect(oauthService.generateAOauth1AuthUri).toHaveBeenCalledWith(mockCredential, { cid: '1', + origin: 'dynamic-credential', authorizationHeader: 'Bearer token123', }); }); @@ -132,7 +134,8 @@ describe('DynamicCredentialsController', () => { expect(authUri).toContain('https://example.domain/oauth2/auth'); expect(oauthService.generateAOauth2AuthUri).toHaveBeenCalledWith(mockCredential, { cid: '1', - authorizationHeader: undefined, + origin: 'dynamic-credential', + authorizationHeader: '', }); }); }); diff --git a/packages/cli/src/modules/dynamic-credentials.ee/dynamic-credentials.controller.ts b/packages/cli/src/modules/dynamic-credentials.ee/dynamic-credentials.controller.ts index fc0316f5f76..b27daca85a6 100644 --- a/packages/cli/src/modules/dynamic-credentials.ee/dynamic-credentials.controller.ts +++ b/packages/cli/src/modules/dynamic-credentials.ee/dynamic-credentials.controller.ts @@ -28,14 +28,16 @@ export class DynamicCredentialsController { if (credential.type.includes('OAuth2')) { return await this.oauthService.generateAOauth2AuthUri(credential, { cid: credential.id, - authorizationHeader: req.headers.authorization, + origin: 'dynamic-credential', + authorizationHeader: req.headers.authorization ?? '', }); } if (credential.type.includes('OAuth1')) { return await this.oauthService.generateAOauth1AuthUri(credential, { cid: credential.id, - authorizationHeader: req.headers.authorization, + origin: 'dynamic-credential', + authorizationHeader: req.headers.authorization ?? '', }); } diff --git a/packages/cli/src/oauth/__tests__/oauth.service.test.ts b/packages/cli/src/oauth/__tests__/oauth.service.test.ts index b9a18dd15de..fdacc903593 100644 --- a/packages/cli/src/oauth/__tests__/oauth.service.test.ts +++ b/packages/cli/src/oauth/__tests__/oauth.service.test.ts @@ -9,6 +9,7 @@ import { mock } from 'jest-mock-extended'; import type { Response } from 'express'; import type { IWorkflowExecuteAdditionalData } from 'n8n-workflow'; import { UnexpectedError } from 'n8n-workflow'; +import type { Cipher } from 'n8n-core'; import { OauthService, @@ -26,6 +27,8 @@ import { UrlService } from '@/services/url.service'; import * as WorkflowExecuteAdditionalData from '@/workflow-execute-additional-data'; import { ExternalHooks } from '@/external-hooks'; import type { OAuth2CredentialData } from '@n8n/client-oauth2'; +import { DynamicCredentialsProxy } from '@/credentials/dynamic-credentials-proxy'; +import { Credentials } from 'n8n-core'; jest.mock('@/workflow-execute-additional-data'); jest.mock('axios'); @@ -40,6 +43,8 @@ describe('OauthService', () => { const urlService = mockInstance(UrlService); const globalConfig = mockInstance(GlobalConfig); const externalHooks = mockInstance(ExternalHooks); + const cipher = mock(); + const dynamicCredentialsProxy = mockInstance(DynamicCredentialsProxy); let service: OauthService; @@ -62,6 +67,17 @@ describe('OauthService', () => { axios.get = jest.fn(); axios.post = jest.fn(); + // Setup cipher mock - encrypt returns the input as-is for testing, decrypt does the reverse + cipher.encrypt.mockImplementation((data: string) => { + // For testing, we'll use base64 encoding as a simple mock + // In production, this would be actual encryption + return Buffer.from(data).toString('base64'); + }); + cipher.decrypt.mockImplementation((data: string) => { + // For testing, decode the base64 + return Buffer.from(data, 'base64').toString(); + }); + service = new OauthService( logger, credentialsHelper, @@ -70,6 +86,8 @@ describe('OauthService', () => { urlService, globalConfig, externalHooks, + cipher, + dynamicCredentialsProxy, ); }); @@ -316,15 +334,20 @@ describe('OauthService', () => { describe('createCsrfState', () => { it('should create CSRF state with correct structure', () => { - const data = { cid: 'credential-id', userId: 'user-id' }; + const data = { + cid: 'credential-id', + userId: 'user-id', + origin: 'static-credential' as const, + }; jest.setSystemTime(new Date(timestamp)); const [csrfSecret, encodedState] = service.createCsrfState(data); expect(typeof csrfSecret).toBe('string'); expect(csrfSecret.length).toBeGreaterThan(0); + expect(cipher.encrypt).toHaveBeenCalled(); - const decoded = JSON.parse(Buffer.from(encodedState, 'base64').toString()); + const decoded = JSON.parse(cipher.decrypt(encodedState)); expect(decoded.cid).toBe('credential-id'); expect(decoded.userId).toBe('user-id'); expect(decoded.token).toBeDefined(); @@ -332,12 +355,17 @@ describe('OauthService', () => { }); it('should include additional data in state', () => { - const data = { cid: 'credential-id', customField: 'custom-value' }; + const data = { + cid: 'credential-id', + customField: 'custom-value', + origin: 'static-credential' as const, + }; jest.setSystemTime(new Date(timestamp)); const [, encodedState] = service.createCsrfState(data); - const decoded = JSON.parse(Buffer.from(encodedState, 'base64').toString()); + expect(cipher.encrypt).toHaveBeenCalled(); + const decoded = JSON.parse(cipher.decrypt(encodedState)); expect(decoded.customField).toBe('custom-value'); }); }); @@ -348,15 +376,18 @@ describe('OauthService', () => { token: 'token', cid: 'credential-id', userId: 'user-id', + origin: 'static-credential' as const, createdAt: timestamp, }; - const encodedState = Buffer.from(JSON.stringify(state)).toString('base64'); + const stateString = JSON.stringify(state); + const encodedState = cipher.encrypt(stateString); const req = mock({ user: mock({ id: 'user-id' }), }); const result = (service as any).decodeCsrfState(encodedState, req); + expect(cipher.decrypt).toHaveBeenCalledWith(encodedState); expect(result).toEqual(state); }); @@ -376,7 +407,8 @@ describe('OauthService', () => { token: 'token', createdAt: timestamp, }; - const encodedState = Buffer.from(JSON.stringify(state)).toString('base64'); + const stateString = JSON.stringify(state); + const encodedState = cipher.encrypt(stateString); const req = mock({ user: mock({ id: 'user-id' }), }); @@ -389,7 +421,8 @@ describe('OauthService', () => { cid: 'credential-id', createdAt: timestamp, }; - const encodedState = Buffer.from(JSON.stringify(state)).toString('base64'); + const stateString = JSON.stringify(state); + const encodedState = cipher.encrypt(stateString); const req = mock({ user: mock({ id: 'user-id' }), }); @@ -402,9 +435,11 @@ describe('OauthService', () => { token: 'token', cid: 'credential-id', userId: 'different-user-id', + origin: 'static-credential' as const, createdAt: timestamp, }; - const encodedState = Buffer.from(JSON.stringify(state)).toString('base64'); + const stateString = JSON.stringify(state); + const encodedState = cipher.encrypt(stateString); const req = mock({ user: mock({ id: 'user-id' }), }); @@ -418,15 +453,110 @@ describe('OauthService', () => { token: 'token', cid: 'credential-id', userId: 'user-id', + origin: 'static-credential' as const, createdAt: timestamp, }; - const encodedState = Buffer.from(JSON.stringify(state)).toString('base64'); + const stateString = JSON.stringify(state); + const encodedState = cipher.encrypt(stateString); const req = mock({ user: undefined, }); expect(() => (service as any).decodeCsrfState(encodedState, req)).toThrow(AuthError); }); + + it('should bypass user validation for dynamic-credential origin', () => { + const state = { + token: 'token', + cid: 'credential-id', + userId: 'different-user-id', + origin: 'dynamic-credential' as const, + createdAt: timestamp, + }; + const stateString = JSON.stringify(state); + const encodedState = cipher.encrypt(stateString); + const req = mock({ + user: mock({ id: 'user-id' }), + }); + + const result = (service as any).decodeCsrfState(encodedState, req); + + expect(result).toEqual(state); + expect(cipher.decrypt).toHaveBeenCalledWith(encodedState); + }); + + it('should bypass user validation for dynamic-credential origin even when req.user is undefined', () => { + const state = { + token: 'token', + cid: 'credential-id', + userId: 'user-id', + origin: 'dynamic-credential' as const, + createdAt: timestamp, + }; + const stateString = JSON.stringify(state); + const encodedState = cipher.encrypt(stateString); + const req = mock({ + user: undefined, + }); + + const result = (service as any).decodeCsrfState(encodedState, req); + + expect(result).toEqual(state); + expect(cipher.decrypt).toHaveBeenCalledWith(encodedState); + }); + + it('should require user validation for static-credential origin', () => { + const state = { + token: 'token', + cid: 'credential-id', + userId: 'different-user-id', + origin: 'static-credential' as const, + createdAt: timestamp, + }; + const stateString = JSON.stringify(state); + const encodedState = cipher.encrypt(stateString); + const req = mock({ + user: mock({ id: 'user-id' }), + }); + + expect(() => (service as any).decodeCsrfState(encodedState, req)).toThrow(AuthError); + expect(() => (service as any).decodeCsrfState(encodedState, req)).toThrow('Unauthorized'); + }); + + it('should require user validation when origin is undefined', () => { + const state = { + token: 'token', + cid: 'credential-id', + userId: 'different-user-id', + createdAt: timestamp, + }; + const stateString = JSON.stringify(state); + const encodedState = cipher.encrypt(stateString); + const req = mock({ + user: mock({ id: 'user-id' }), + }); + + expect(() => (service as any).decodeCsrfState(encodedState, req)).toThrow(AuthError); + expect(() => (service as any).decodeCsrfState(encodedState, req)).toThrow('Unauthorized'); + }); + + it('should require user validation for invalid origin values', () => { + const state = { + token: 'token', + cid: 'credential-id', + userId: 'different-user-id', + origin: 'invalid-origin' as any, + createdAt: timestamp, + }; + const stateString = JSON.stringify(state); + const encodedState = cipher.encrypt(stateString); + const req = mock({ + user: mock({ id: 'user-id' }), + }); + + expect(() => (service as any).decodeCsrfState(encodedState, req)).toThrow(AuthError); + expect(() => (service as any).decodeCsrfState(encodedState, req)).toThrow('Unauthorized'); + }); }); describe('verifyCsrfState', () => { @@ -438,6 +568,25 @@ describe('OauthService', () => { const state = { token: stateToken, cid: 'credential-id', + origin: 'static-credential', + createdAt: Date.now(), + }; + const decrypted = { csrfSecret }; + + const result = (service as any).verifyCsrfState(decrypted, state); + + expect(result).toBe(true); + }); + + it('should return true for valid CSRF state with dynamic credential origin', () => { + const csrfSecret = 'csrf-secret'; + const token = new (require('csrf'))(); + const stateToken = token.create(csrfSecret); + + const state = { + token: stateToken, + cid: 'credential-id', + origin: 'dynamic-credential', createdAt: Date.now(), }; const decrypted = { csrfSecret }; @@ -456,6 +605,7 @@ describe('OauthService', () => { const state = { token: stateToken, cid: 'credential-id', + origin: 'static-credential', createdAt: expiredTime, }; const decrypted = { csrfSecret }; @@ -473,6 +623,7 @@ describe('OauthService', () => { const state = { token: stateToken, cid: 'credential-id', + origin: 'static-credential', createdAt: Date.now(), }; const decrypted = {}; @@ -486,6 +637,7 @@ describe('OauthService', () => { const state = { token: 'invalid-token', cid: 'credential-id', + origin: 'static-credential', createdAt: Date.now(), }; const decrypted = { csrfSecret: 'csrf-secret' }; @@ -502,6 +654,7 @@ describe('OauthService', () => { token: 'token', cid: 'credential-id', userId: 'user-id', + origin: 'static-credential', createdAt: timestamp, }; @@ -515,7 +668,7 @@ describe('OauthService', () => { state.token = stateToken; const req = mock({ - query: { state: Buffer.from(JSON.stringify(state)).toString('base64') }, + query: { state: cipher.encrypt(JSON.stringify(state)) }, user: mock({ id: 'user-id' }), }); @@ -528,7 +681,7 @@ describe('OauthService', () => { const result = await service.resolveCredential(req); - expect(result).toEqual([mockCredential, mockDecryptedData, mockOAuthCredentials]); + expect(result).toEqual([mockCredential, mockDecryptedData, mockOAuthCredentials, state]); }); it('should throw UnexpectedError when credential is not found', async () => { @@ -536,9 +689,11 @@ describe('OauthService', () => { token: 'token', cid: 'credential-id', userId: 'user-id', + origin: 'static-credential', createdAt: timestamp, }; - const encodedState = Buffer.from(JSON.stringify(state)).toString('base64'); + const stateString = JSON.stringify(state); + const encodedState = cipher.encrypt(stateString); const req = mock({ query: { state: encodedState }, @@ -558,9 +713,11 @@ describe('OauthService', () => { token: 'token', cid: 'credential-id', userId: 'user-id', + origin: 'static-credential', createdAt: timestamp, }; - const encodedState = Buffer.from(JSON.stringify(state)).toString('base64'); + const stateString = JSON.stringify(state); + const encodedState = cipher.encrypt(stateString); const mockCredential = mock({ id: 'credential-id' }); const mockDecryptedData = { csrfSecret: 'csrf-secret' }; @@ -584,6 +741,180 @@ describe('OauthService', () => { 'The OAuth callback state is invalid!', ); }); + + it('should resolve dynamic credential without user validation but still verify CSRF', async () => { + const state = { + token: 'token', + cid: 'credential-id', + userId: 'different-user-id', + origin: 'dynamic-credential' as const, + createdAt: timestamp, + }; + + const mockCredential = mock({ id: 'credential-id' }); + const mockDecryptedData = { csrfSecret: 'csrf-secret' }; + const mockOAuthCredentials = { clientId: 'client-id' }; + const mockAdditionalData = mock(); + + const token = new (require('csrf'))(); + const stateToken = token.create('csrf-secret'); + state.token = stateToken; + + const req = mock({ + query: { state: cipher.encrypt(JSON.stringify(state)) }, + user: mock({ id: 'user-id' }), // Different user ID - should be bypassed + }); + + credentialsRepository.findOneBy.mockResolvedValue(mockCredential); + jest.mocked(WorkflowExecuteAdditionalData.getBase).mockResolvedValue(mockAdditionalData); + credentialsHelper.getDecrypted.mockResolvedValue(mockDecryptedData); + credentialsHelper.applyDefaultsAndOverwrites.mockResolvedValue(mockOAuthCredentials); + + jest.spyOn(service as any, 'verifyCsrfState').mockReturnValue(true); + + const result = await service.resolveCredential(req); + + // Should succeed despite different user ID because origin is dynamic-credential + expect(result).toEqual([mockCredential, mockDecryptedData, mockOAuthCredentials, state]); + // CSRF validation should still be called + expect((service as any).verifyCsrfState).toHaveBeenCalledWith(mockDecryptedData, state); + }); + + it('should still verify CSRF for dynamic credentials even when req.user is undefined', async () => { + const state = { + token: 'token', + cid: 'credential-id', + userId: 'user-id', + origin: 'dynamic-credential' as const, + createdAt: timestamp, + }; + + const mockCredential = mock({ id: 'credential-id' }); + const mockDecryptedData = { csrfSecret: 'csrf-secret' }; + const mockOAuthCredentials = { clientId: 'client-id' }; + const mockAdditionalData = mock(); + + const token = new (require('csrf'))(); + const stateToken = token.create('csrf-secret'); + state.token = stateToken; + + const req = mock({ + query: { state: cipher.encrypt(JSON.stringify(state)) }, + user: undefined, // No user - should be bypassed for dynamic credentials + }); + + credentialsRepository.findOneBy.mockResolvedValue(mockCredential); + jest.mocked(WorkflowExecuteAdditionalData.getBase).mockResolvedValue(mockAdditionalData); + credentialsHelper.getDecrypted.mockResolvedValue(mockDecryptedData); + credentialsHelper.applyDefaultsAndOverwrites.mockResolvedValue(mockOAuthCredentials); + + jest.spyOn(service as any, 'verifyCsrfState').mockReturnValue(true); + + const result = await service.resolveCredential(req); + + // Should succeed despite no user because origin is dynamic-credential + expect(result).toEqual([mockCredential, mockDecryptedData, mockOAuthCredentials, state]); + // CSRF validation should still be called + expect((service as any).verifyCsrfState).toHaveBeenCalledWith(mockDecryptedData, state); + }); + }); + + describe('saveDynamicCredential', () => { + beforeEach(() => { + // Mock Credentials.getData to return empty object to avoid decryption issues + jest.spyOn(Credentials.prototype, 'getData').mockReturnValue({}); + }); + + afterEach(() => { + jest.restoreAllMocks(); + }); + + it('should save dynamic credential with correct parameters', async () => { + const credential = mock({ + id: 'credential-id', + name: 'Test Credential', + type: 'googleOAuth2Api', + data: 'encrypted-data', + }); + const oauthTokenData = { + access_token: 'access-token', + refresh_token: 'refresh-token', + }; + const authToken = 'token123'; // Controller splits 'Bearer token123' and passes just 'token123' + const credentialResolverId = 'resolver-id'; + + dynamicCredentialsProxy.storeIfNeeded.mockResolvedValue(undefined); + + await service.saveDynamicCredential( + credential, + oauthTokenData, + authToken, + credentialResolverId, + ); + + expect(dynamicCredentialsProxy.storeIfNeeded).toHaveBeenCalledWith( + { + id: 'credential-id', + name: 'Test Credential', + type: 'googleOAuth2Api', + isResolvable: true, + }, + oauthTokenData, + { version: 1, identity: authToken }, + expect.any(Object), + { credentialResolverId: 'resolver-id' }, + ); + }); + + it('should remove csrfSecret from credential data', async () => { + const credential = mock({ + id: 'credential-id', + name: 'Test Credential', + type: 'googleOAuth2Api', + data: 'encrypted-data', + }); + const oauthTokenData = { + access_token: 'access-token', + csrfSecret: 'csrf-secret', + }; + const authToken = 'token123'; // Controller splits 'Bearer token123' and passes just 'token123' + const credentialResolverId = 'resolver-id'; + + dynamicCredentialsProxy.storeIfNeeded.mockResolvedValue(undefined); + + await service.saveDynamicCredential( + credential, + oauthTokenData, + authToken, + credentialResolverId, + ); + + // Verify that storeIfNeeded was called with data that doesn't include csrfSecret + const callArgs = dynamicCredentialsProxy.storeIfNeeded.mock.calls[0]; + const staticData = callArgs[3] as any; + expect(staticData).not.toHaveProperty('csrfSecret'); + }); + + it('should handle errors from dynamicCredentialsProxy', async () => { + const credential = mock({ + id: 'credential-id', + name: 'Test Credential', + type: 'googleOAuth2Api', + data: 'encrypted-data', + }); + const oauthTokenData = { + access_token: 'access-token', + }; + const authToken = 'token123'; // Controller splits 'Bearer token123' and passes just 'token123' + const credentialResolverId = 'resolver-id'; + + const error = new Error('Storage failed'); + dynamicCredentialsProxy.storeIfNeeded.mockRejectedValue(error); + + await expect( + service.saveDynamicCredential(credential, oauthTokenData, authToken, credentialResolverId), + ).rejects.toThrow('Storage failed'); + }); }); describe('renderCallbackError', () => { @@ -741,6 +1072,7 @@ describe('OauthService', () => { const authUri = await service.generateAOauth2AuthUri(credential, { cid: credential.id, + origin: 'static-credential', userId: 'user-id', }); @@ -791,6 +1123,7 @@ describe('OauthService', () => { const authUri = await service.generateAOauth2AuthUri(credential, { cid: credential.id, + origin: 'static-credential', userId: 'user-id', }); @@ -835,6 +1168,7 @@ describe('OauthService', () => { const authUri = await service.generateAOauth2AuthUri(credential, { cid: credential.id, + origin: 'static-credential', userId: 'user-id', }); @@ -888,6 +1222,7 @@ describe('OauthService', () => { const authUri = await service.generateAOauth2AuthUri(credential, { cid: credential.id, + origin: 'static-credential', userId: 'user-id', }); @@ -936,12 +1271,14 @@ describe('OauthService', () => { await expect( service.generateAOauth2AuthUri(credential, { cid: credential.id, + origin: 'static-credential', userId: 'user-id', }), ).rejects.toThrow(BadRequestError); await expect( service.generateAOauth2AuthUri(credential, { cid: credential.id, + origin: 'static-credential', userId: 'user-id', }), ).rejects.toThrow('Invalid OAuth2 server metadata'); @@ -977,12 +1314,14 @@ describe('OauthService', () => { await expect( service.generateAOauth2AuthUri(credential, { cid: credential.id, + origin: 'static-credential', userId: 'user-id', }), ).rejects.toThrow(BadRequestError); await expect( service.generateAOauth2AuthUri(credential, { cid: credential.id, + origin: 'static-credential', userId: 'user-id', }), ).rejects.toThrow('Invalid client registration response'); @@ -1033,6 +1372,7 @@ describe('OauthService', () => { const authUri = await service.generateAOauth2AuthUri(credential, { cid: credential.id, + origin: 'static-credential', userId: 'user-id', }); @@ -1078,6 +1418,7 @@ describe('OauthService', () => { await service.generateAOauth2AuthUri(credential, { cid: credential.id, + origin: 'static-credential', userId: 'user-id', }); @@ -1111,6 +1452,7 @@ describe('OauthService', () => { const authUri = await service.generateAOauth1AuthUri(credential, { cid: credential.id, + origin: 'static-credential', userId: 'user-id', }); @@ -1143,6 +1485,7 @@ describe('OauthService', () => { const authUri = await service.generateAOauth1AuthUri(credential, { cid: credential.id, + origin: 'static-credential', userId: 'user-id', }); @@ -1168,6 +1511,7 @@ describe('OauthService', () => { await expect( service.generateAOauth1AuthUri(credential, { cid: credential.id, + origin: 'static-credential', userId: 'user-id', }), ).rejects.toThrow('Request token failed'); diff --git a/packages/cli/src/oauth/oauth.service.ts b/packages/cli/src/oauth/oauth.service.ts index de88b654ab7..11c3f246985 100644 --- a/packages/cli/src/oauth/oauth.service.ts +++ b/packages/cli/src/oauth/oauth.service.ts @@ -5,7 +5,7 @@ import { CredentialsRepository } from '@n8n/db'; import { Service } from '@n8n/di'; import Csrf from 'csrf'; import type { Response } from 'express'; -import { Credentials } from 'n8n-core'; +import { Credentials, Cipher } from 'n8n-core'; import type { ICredentialDataDecryptedObject, IWorkflowExecuteAdditionalData } from 'n8n-workflow'; import { jsonParse, UnexpectedError } from 'n8n-workflow'; @@ -49,6 +49,8 @@ import { type CsrfState, type OAuth1CredentialData, } from './types'; +import { CredentialStoreMetadata } from '@/credentials/dynamic-credential-storage.interface'; +import { DynamicCredentialsProxy } from '@/credentials/dynamic-credentials-proxy'; export function shouldSkipAuthOnOAuthCallback() { const value = process.env.N8N_SKIP_AUTH_ON_OAUTH_CALLBACK?.toLowerCase() ?? 'false'; @@ -69,6 +71,8 @@ export class OauthService { private readonly urlService: UrlService, private readonly globalConfig: GlobalConfig, private readonly externalHooks: ExternalHooks, + private readonly cipher: Cipher, + private readonly dynamicCredentialsProxy: DynamicCredentialsProxy, ) {} getBaseUrl(oauthVersion: OauthVersion) { @@ -171,7 +175,9 @@ export class OauthService { } /** Get a credential without user check */ - protected async getCredentialWithoutUser(credentialId: string): Promise { + protected async getCredentialWithoutUser( + credentialId: string, + ): Promise { return await this.credentialsRepository.findOneBy({ id: credentialId }); } @@ -183,12 +189,14 @@ export class OauthService { createdAt: Date.now(), ...data, }; - return [csrfSecret, Buffer.from(JSON.stringify(state)).toString('base64')]; + const encryptedState = this.cipher.encrypt(JSON.stringify(state)); + return [csrfSecret, encryptedState]; } protected decodeCsrfState(encodedState: string, req: AuthenticatedRequest): CsrfState { const errorMessage = 'Invalid state format'; - const decoded = jsonParse(Buffer.from(encodedState, 'base64').toString(), { + const decryptedState = this.cipher.decrypt(encodedState); + const decoded = jsonParse(decryptedState, { errorMessage, }); @@ -196,6 +204,11 @@ export class OauthService { throw new UnexpectedError(errorMessage); } + // user validation not required for dynamic credentials + if (decoded.origin === 'dynamic-credential') { + return decoded; + } + if (decoded.userId !== req.user?.id) { throw new AuthError('Unauthorized'); } @@ -218,7 +231,7 @@ export class OauthService { async resolveCredential( req: OAuthRequest.OAuth1Credential.Callback | OAuthRequest.OAuth2Credential.Callback, - ): Promise<[ICredentialsDb, ICredentialDataDecryptedObject, T]> { + ): Promise<[CredentialsEntity, ICredentialDataDecryptedObject, T, CsrfState]> { const { state: encodedState } = req.query; const state = this.decodeCsrfState(encodedState, req); const credential = await this.getCredentialWithoutUser(state.cid); @@ -242,7 +255,7 @@ export class OauthService { throw new UnexpectedError('The OAuth callback state is invalid!'); } - return [credential, decryptedDataOriginal, oauthCredentials]; + return [credential, decryptedDataOriginal, oauthCredentials, state]; } renderCallbackError(res: Response, message: string, reason?: string) { @@ -555,4 +568,33 @@ export class OauthService { token_endpoint_auth_method: tokenEndpointAuthMethod, }; } + + async saveDynamicCredential( + credential: CredentialsEntity, + oauthTokenData: ICredentialDataDecryptedObject, + authHeader: string, + credentialResolverId: string, + ) { + const credentials = new Credentials(credential, credential.type, credential.data); + credentials.updateData(oauthTokenData, ['csrfSecret']); + + const credentialStoreMetadata: CredentialStoreMetadata = { + id: credential.id, + name: credential.name, + type: credential.type, + isResolvable: true, + }; + + await this.dynamicCredentialsProxy.storeIfNeeded( + { + ...credentialStoreMetadata, + isResolvable: true, + }, + oauthTokenData, + // todo parse this + { version: 1, identity: authHeader }, + credentials.getData(), + { credentialResolverId }, + ); + } } diff --git a/packages/cli/src/oauth/types.ts b/packages/cli/src/oauth/types.ts index 512ffe37b78..e4df10e3db9 100644 --- a/packages/cli/src/oauth/types.ts +++ b/packages/cli/src/oauth/types.ts @@ -9,6 +9,7 @@ export type CsrfStateRequired = { export type CreateCsrfStateData = { cid: string; + origin: 'static-credential' | 'dynamic-credential'; [key: string]: unknown; };