mirror of
https://github.com/n8n-io/n8n.git
synced 2026-10-17 13:39:44 +02:00
fix(core): Apply the credential type policy to credential tests and probes (#39927)
Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
668e71d54a
commit
9191cf3fb0
|
|
@ -1239,6 +1239,82 @@ describe('CredentialsService', () => {
|
|||
});
|
||||
});
|
||||
|
||||
const credentialUseRefusal = () =>
|
||||
new PolicyViolationError([
|
||||
{ kind: 'test', checkId: 'test', message: 'Credential type "githubApi" is blocked' },
|
||||
]);
|
||||
|
||||
describe('decryptForUse', () => {
|
||||
const storedCredential = mock<CredentialsEntity>({ id: 'cred-id', type: 'githubApi' });
|
||||
|
||||
it('checks the policy against the owning project, then returns the raw data', async () => {
|
||||
sharedCredentialsRepository.findCredentialOwningProject.mockResolvedValue(
|
||||
mock<Project>({ id: 'project-1' }),
|
||||
);
|
||||
vi.spyOn(service, 'decrypt').mockResolvedValue({ accessToken: 'secret' });
|
||||
|
||||
await expect(service.decryptForUse(storedCredential, ownerActor)).resolves.toEqual({
|
||||
accessToken: 'secret',
|
||||
});
|
||||
expect(policyEnforcementService.enforceCredentialDecrypt).toHaveBeenCalledExactlyOnceWith(
|
||||
{
|
||||
credentialType: 'githubApi',
|
||||
credentialId: 'cred-id',
|
||||
consumer: null,
|
||||
projectId: 'project-1',
|
||||
},
|
||||
ownerActor,
|
||||
);
|
||||
expect(service.decrypt).toHaveBeenCalledWith(storedCredential, true);
|
||||
});
|
||||
|
||||
it('judges on the given project without looking up the owner', async () => {
|
||||
vi.spyOn(service, 'decrypt').mockResolvedValue({});
|
||||
|
||||
await service.decryptForUse(storedCredential, ownerActor, 'project-2');
|
||||
|
||||
expect(sharedCredentialsRepository.findCredentialOwningProject).not.toHaveBeenCalled();
|
||||
expect(policyEnforcementService.enforceCredentialDecrypt).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ projectId: 'project-2' }),
|
||||
ownerActor,
|
||||
);
|
||||
});
|
||||
|
||||
it('skips the owner lookup when the caller already found no owner', async () => {
|
||||
vi.spyOn(service, 'decrypt').mockResolvedValue({});
|
||||
|
||||
await service.decryptForUse(storedCredential, ownerActor, null);
|
||||
|
||||
expect(sharedCredentialsRepository.findCredentialOwningProject).not.toHaveBeenCalled();
|
||||
expect(policyEnforcementService.enforceCredentialDecrypt).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ projectId: null }),
|
||||
ownerActor,
|
||||
);
|
||||
});
|
||||
|
||||
it('judges on instance policy only when the credential has no owning project', async () => {
|
||||
sharedCredentialsRepository.findCredentialOwningProject.mockResolvedValue(undefined);
|
||||
vi.spyOn(service, 'decrypt').mockResolvedValue({});
|
||||
|
||||
await service.decryptForUse(storedCredential, ownerActor);
|
||||
|
||||
expect(policyEnforcementService.enforceCredentialDecrypt).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ projectId: null }),
|
||||
ownerActor,
|
||||
);
|
||||
});
|
||||
|
||||
it('does not decrypt when the policy refuses', async () => {
|
||||
policyEnforcementService.enforceCredentialDecrypt.mockRejectedValue(credentialUseRefusal());
|
||||
const decrypt = vi.spyOn(service, 'decrypt');
|
||||
|
||||
await expect(
|
||||
service.decryptForUse(storedCredential, ownerActor, 'project-1'),
|
||||
).rejects.toThrow(PolicyViolationError);
|
||||
expect(decrypt).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
describe('testById', () => {
|
||||
it('throws CredentialNotFoundError when the user cannot use the credential', async () => {
|
||||
credentialsFinderService.findCredentialForUser.mockResolvedValue(null);
|
||||
|
|
@ -1298,6 +1374,18 @@ describe('CredentialsService', () => {
|
|||
);
|
||||
expect(result).toEqual(testResult);
|
||||
});
|
||||
|
||||
it('refuses a blocked credential before the tester runs', async () => {
|
||||
credentialsFinderService.findCredentialForUser.mockResolvedValue(
|
||||
mock<CredentialsEntity>({ id: 'credential-id', type: 'githubApi', usageScope: 'project' }),
|
||||
);
|
||||
policyEnforcementService.enforceCredentialDecrypt.mockRejectedValue(credentialUseRefusal());
|
||||
|
||||
await expect(service.testById(ownerUser, 'credential-id')).rejects.toThrow(
|
||||
PolicyViolationError,
|
||||
);
|
||||
expect(credentialsTester.testCredentials).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
describe('getOne', () => {
|
||||
|
|
@ -2079,6 +2167,60 @@ describe('CredentialsService', () => {
|
|||
});
|
||||
|
||||
describe('testWithCredentials', () => {
|
||||
it('refuses a stored credential of a blocked type before the tester runs', async () => {
|
||||
credentialsFinderService.findCredentialForUser.mockResolvedValue(
|
||||
mock<CredentialsEntity>({ id: 'credential-id', type: 'githubApi', isManaged: false }),
|
||||
);
|
||||
policyEnforcementService.enforceCredentialDecrypt.mockRejectedValue(credentialUseRefusal());
|
||||
|
||||
await expect(
|
||||
service.testWithCredentials(ownerUser, {
|
||||
id: 'credential-id',
|
||||
name: 'GitHub account',
|
||||
type: 'githubApi',
|
||||
data: {},
|
||||
}),
|
||||
).rejects.toThrow(PolicyViolationError);
|
||||
expect(credentialsTester.testCredentials).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('rejects a posted type that differs from the stored one before decrypting', async () => {
|
||||
// A managed credential ignores the posted payload, so this needs an unmanaged one.
|
||||
credentialsFinderService.findCredentialForUser.mockResolvedValue(
|
||||
mock<CredentialsEntity>({ id: 'credential-id', type: 'githubApi', isManaged: false }),
|
||||
);
|
||||
|
||||
await expect(
|
||||
service.testWithCredentials(ownerUser, {
|
||||
id: 'credential-id',
|
||||
name: 'GitHub account',
|
||||
type: 'slackApi',
|
||||
data: {},
|
||||
}),
|
||||
).rejects.toThrow(BadRequestError);
|
||||
expect(policyEnforcementService.enforceCredentialDecrypt).not.toHaveBeenCalled();
|
||||
expect(credentialsTester.testCredentials).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('refuses an unsaved provider connection of a blocked type on instance policy', async () => {
|
||||
credentialsFinderService.findCredentialForUser.mockResolvedValue(null);
|
||||
policyEnforcementService.enforceCredentialDecrypt.mockRejectedValue(credentialUseRefusal());
|
||||
|
||||
await expect(
|
||||
service.testWithCredentials(ownerUser, {
|
||||
id: '',
|
||||
name: 'n8n Assistant model',
|
||||
type: 'openAiApi',
|
||||
data: { apiKey: 'key' },
|
||||
}),
|
||||
).rejects.toThrow(PolicyViolationError);
|
||||
expect(policyEnforcementService.enforceCredentialDecrypt).toHaveBeenCalledWith(
|
||||
{ credentialType: 'openAiApi', credentialId: '', consumer: null, projectId: null },
|
||||
{ kind: 'user', user: ownerUser },
|
||||
);
|
||||
expect(credentialsTester.testCredentials).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('tests an unsaved provider connection for an instance credential manager', async () => {
|
||||
const testResult = { status: 'OK', message: 'Credential tested successfully' } as const;
|
||||
credentialsFinderService.findCredentialForUser.mockResolvedValue(null);
|
||||
|
|
@ -4929,6 +5071,14 @@ describe('CredentialsService', () => {
|
|||
);
|
||||
});
|
||||
|
||||
it('should refuse a blocked credential before probing it', async () => {
|
||||
credentialsFinderService.findCredentialForUser.mockResolvedValue(storedCredential);
|
||||
policyEnforcementService.enforceCredentialDecrypt.mockRejectedValue(credentialUseRefusal());
|
||||
|
||||
await expect(service.probeById(ownerUser, 'cred-id')).rejects.toThrow(PolicyViolationError);
|
||||
expect(credentialsTester.probeCredentialAuth).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should throw when the credential has no test URL', async () => {
|
||||
credentialsFinderService.findCredentialForUser.mockResolvedValue(storedCredential);
|
||||
mockDecryptedData({ template: '{}' });
|
||||
|
|
|
|||
|
|
@ -1028,6 +1028,36 @@ export class CredentialsService {
|
|||
await this.update(credential.id, newCredentialData, actor, decryptedData);
|
||||
}
|
||||
|
||||
/**
|
||||
* Decrypts a stored credential whose secret is about to leave n8n, such as a test call to its
|
||||
* provider. Refuses what the credential policy blocks; `decrypt` stays for display and rewrite.
|
||||
* Omit `projectId` to judge on the owning project. Pass `null` when the caller already found
|
||||
* no owner: only instance policy applies, and the lookup is skipped.
|
||||
*/
|
||||
async decryptForUse(
|
||||
credential: CredentialsEntity,
|
||||
actor: PolicyActor,
|
||||
projectId?: string | null,
|
||||
): Promise<ICredentialDataDecryptedObject> {
|
||||
const judgedProjectId =
|
||||
projectId === undefined
|
||||
? ((await this.findCredentialOwningProject(credential.id))?.id ?? null)
|
||||
: projectId;
|
||||
await this.enforceCredentialUse(credential, actor, judgedProjectId);
|
||||
return await this.decrypt(credential, true);
|
||||
}
|
||||
|
||||
private async enforceCredentialUse(
|
||||
credential: Pick<ICredentialsDecrypted, 'id' | 'type'>,
|
||||
actor: PolicyActor,
|
||||
projectId: string | null,
|
||||
) {
|
||||
await this.policyEnforcementService.enforceCredentialDecrypt(
|
||||
{ credentialType: credential.type, credentialId: credential.id, consumer: null, projectId },
|
||||
actor,
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* Decrypts the credentials data and redacts the content by default.
|
||||
*
|
||||
|
|
@ -1488,7 +1518,7 @@ export class CredentialsService {
|
|||
throw new CredentialNotFoundError(credentialId);
|
||||
}
|
||||
|
||||
const credentials = await this.prepareCredentialsForTest({ storedCredential });
|
||||
const credentials = await this.prepareCredentialsForTest({ storedCredential, user });
|
||||
return await this.test(user.id, credentials);
|
||||
}
|
||||
|
||||
|
|
@ -1503,6 +1533,8 @@ export class CredentialsService {
|
|||
if (!storedCredential) {
|
||||
if (credentials.id === '' && hasGlobalScope(user, 'credential:manageInstance')) {
|
||||
this.validateInstanceCredentialData(credentials.data ?? {});
|
||||
// Nothing stored is decrypted, but the test still calls the blocked type's provider.
|
||||
await this.enforceCredentialUse(credentials, { kind: 'user', user }, null);
|
||||
return await this.test(user.id, credentials);
|
||||
}
|
||||
throw new CredentialNotFoundError(credentials.id);
|
||||
|
|
@ -1534,7 +1566,7 @@ export class CredentialsService {
|
|||
throw new CredentialNotFoundError(credentialId);
|
||||
}
|
||||
|
||||
const data = await this.decrypt(storedCredential, true);
|
||||
const data = await this.decryptForUse(storedCredential, { kind: 'user', user });
|
||||
|
||||
// Expressions and non-HTTP values are refused, not resolved.
|
||||
const testTarget = parseHttpUrl(data.testUrl);
|
||||
|
|
@ -2399,10 +2431,21 @@ export class CredentialsService {
|
|||
credentialsToTest,
|
||||
}: {
|
||||
storedCredential: CredentialsEntity;
|
||||
user?: User;
|
||||
user: User;
|
||||
credentialsToTest?: ICredentialsDecrypted;
|
||||
}): Promise<ICredentialsDecrypted> {
|
||||
const decryptedData = await this.decrypt(storedCredential, true);
|
||||
// The tester picks its test from the posted type, so it must match the stored secrets.
|
||||
if (credentialsToTest && credentialsToTest.type !== storedCredential.type) {
|
||||
throw new BadRequestError('The credential type does not match the stored credential');
|
||||
}
|
||||
|
||||
// Find the owning project to prevent leakage of other project data.
|
||||
const owningProject = await this.findCredentialOwningProject(storedCredential.id);
|
||||
const decryptedData = await this.decryptForUse(
|
||||
storedCredential,
|
||||
{ kind: 'user', user },
|
||||
owningProject?.id ?? null,
|
||||
);
|
||||
const mergedCredentials: ICredentialsDecrypted = credentialsToTest
|
||||
? deepCopy(credentialsToTest)
|
||||
: {
|
||||
|
|
@ -2412,8 +2455,6 @@ export class CredentialsService {
|
|||
data: decryptedData,
|
||||
};
|
||||
|
||||
// Find the owning project to prevent leakage of other project data.
|
||||
const owningProject = await this.findCredentialOwningProject(storedCredential.id);
|
||||
if (!owningProject) {
|
||||
mergedCredentials.homeProject = undefined;
|
||||
} else {
|
||||
|
|
@ -2427,7 +2468,7 @@ export class CredentialsService {
|
|||
};
|
||||
}
|
||||
|
||||
if (user && credentialsToTest) {
|
||||
if (credentialsToTest) {
|
||||
await this.replaceCredentialContentsForSharee(
|
||||
user,
|
||||
storedCredential,
|
||||
|
|
|
|||
|
|
@ -8074,6 +8074,61 @@ describe('createContext: aiPreferenceService', () => {
|
|||
});
|
||||
|
||||
describe('createCredentialAdapter', () => {
|
||||
describe('test', () => {
|
||||
const storedCredential = { id: 'cred-1', name: 'Slack account', type: 'slackApi' };
|
||||
|
||||
const adapterWith = (decryptForUse: ReturnType<typeof vi.fn>, test: ReturnType<typeof vi.fn>) =>
|
||||
createNodeAdapterServiceForTests([], {
|
||||
credentialsFinderService: {
|
||||
findCredentialForUser: vi.fn().mockResolvedValue(storedCredential),
|
||||
},
|
||||
credentialsService: { decryptForUse, test },
|
||||
});
|
||||
|
||||
it('decrypts through the policy-checked path as the user', async () => {
|
||||
const decryptForUse = vi.fn().mockResolvedValue({ accessToken: 'secret' });
|
||||
const test = vi.fn().mockResolvedValue({ status: 'OK', message: 'ok' });
|
||||
const { credentialService, mockUser } = adapterWith(decryptForUse, test);
|
||||
|
||||
await expect(credentialService.test('cred-1')).resolves.toEqual({
|
||||
success: true,
|
||||
message: 'ok',
|
||||
});
|
||||
expect(decryptForUse).toHaveBeenCalledWith(
|
||||
storedCredential,
|
||||
{ kind: 'user', user: mockUser },
|
||||
undefined,
|
||||
);
|
||||
});
|
||||
|
||||
it('judges the policy on the project the thread is bound to', async () => {
|
||||
const decryptForUse = vi.fn().mockResolvedValue({});
|
||||
const test = vi.fn().mockResolvedValue({ status: 'OK', message: 'ok' });
|
||||
const { service, mockUser } = adapterWith(decryptForUse, test);
|
||||
const { credentialService } = service.createContext(mockUser, {
|
||||
projectId: 'team-project-1',
|
||||
});
|
||||
|
||||
await credentialService.test('cred-1');
|
||||
|
||||
expect(decryptForUse).toHaveBeenCalledWith(
|
||||
storedCredential,
|
||||
{ kind: 'user', user: mockUser },
|
||||
'team-project-1',
|
||||
);
|
||||
});
|
||||
|
||||
it('does not run the test when the policy refuses the decrypt', async () => {
|
||||
const refusal = new Error('Credential type "slackApi" is blocked by an instance policy');
|
||||
const decryptForUse = vi.fn().mockRejectedValue(refusal);
|
||||
const test = vi.fn();
|
||||
const { credentialService } = adapterWith(decryptForUse, test);
|
||||
|
||||
await expect(credentialService.test('cred-1')).rejects.toBe(refusal);
|
||||
expect(test).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
describe('getCredentialFillState', () => {
|
||||
/** An adapter over a credential type declaring `properties` and holding `data`. */
|
||||
const adapterFor = (
|
||||
|
|
|
|||
|
|
@ -2846,7 +2846,11 @@ export class InstanceAiAdapterService {
|
|||
id: credential.id,
|
||||
name: credential.name,
|
||||
type: credential.type,
|
||||
data: await credentialsService.decrypt(credential, true),
|
||||
data: await credentialsService.decryptForUse(
|
||||
credential,
|
||||
{ kind: 'user', user },
|
||||
boundProjectId,
|
||||
),
|
||||
};
|
||||
|
||||
const result = await credentialsService.test(user.id, credentialsToTest);
|
||||
|
|
|
|||
|
|
@ -1,5 +1,5 @@
|
|||
import type { PolicyViolation } from '@n8n/decorators';
|
||||
import { UserError } from 'n8n-workflow';
|
||||
import { hasPolicyRefusalMarker, UserError } from 'n8n-workflow';
|
||||
|
||||
/** `enforce*` only throws when something objected, so an empty list is a bug, not a case. */
|
||||
export type NonEmptyViolations = [PolicyViolation, ...PolicyViolation[]];
|
||||
|
|
@ -61,14 +61,5 @@ export class PolicyViolationError extends UserError {
|
|||
* is needed, e.g. to read {@link PolicyViolationError.violations}.
|
||||
*/
|
||||
export function isPolicyRefusal(error: unknown): boolean {
|
||||
if (error instanceof PolicyViolationError) return true;
|
||||
|
||||
// `hasOwn`, not `in`: the marker is an own property by contract, and an inherited
|
||||
// one would let an unrelated error suppress cleanup and retries.
|
||||
return (
|
||||
typeof error === 'object' &&
|
||||
error !== null &&
|
||||
Object.hasOwn(error, 'isPolicyRefusal') &&
|
||||
(error as { isPolicyRefusal?: unknown }).isPolicyRefusal === true
|
||||
);
|
||||
return error instanceof PolicyViolationError || hasPolicyRefusalMarker(error);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -0,0 +1,182 @@
|
|||
/**
|
||||
* Pins the credential policy on the routes that decrypt a stored credential to call its provider
|
||||
* without going through a workflow run: the test, probe and public API test routes.
|
||||
*/
|
||||
import { createTeamProject, testDb } from '@n8n/backend-test-utils';
|
||||
import { LICENSE_FEATURES } from '@n8n/constants';
|
||||
import type { CredentialsEntity, Project, User } from '@n8n/db';
|
||||
import { Container } from '@n8n/di';
|
||||
import { mock } from 'vitest-mock-extended';
|
||||
|
||||
import { CredentialsTester } from '@/services/credentials-tester.service';
|
||||
|
||||
import { saveCredential } from '../shared/db/credentials';
|
||||
import { createOwnerWithApiKey } from '../shared/db/users';
|
||||
import type { SuperAgentTest } from '../shared/types';
|
||||
import * as utils from '../shared/utils/';
|
||||
import { denyRule, putCredentialTypePolicy } from './shared/credential-type-policy';
|
||||
import { clearPolicyCache } from './shared/policy-cache';
|
||||
|
||||
const GITHUB_API = 'githubApi';
|
||||
|
||||
const testServer = utils.setupTestServer({
|
||||
endpointGroups: ['credentials', 'publicApi', 'type-availability-policies'],
|
||||
modules: ['policy-infrastructure', 'type-availability-policies'],
|
||||
enabledFeatures: [LICENSE_FEATURES.TYPE_AVAILABILITY_POLICIES],
|
||||
});
|
||||
|
||||
const credentialsTester = mock<CredentialsTester>();
|
||||
Container.set(CredentialsTester, credentialsTester);
|
||||
|
||||
let owner: User;
|
||||
let ownerAgent: SuperAgentTest;
|
||||
let publicApiAgent: SuperAgentTest;
|
||||
|
||||
const PROBEABLE_DATA = {
|
||||
accessToken: 'secret',
|
||||
testUrl: 'https://api.example.com/me',
|
||||
serviceOrigin: 'https://api.example.com',
|
||||
};
|
||||
|
||||
async function saveGithubCredential(owner: { user: User } | { project: Project }) {
|
||||
return await saveCredential(
|
||||
{ name: 'GitHub account', type: GITHUB_API, data: PROBEABLE_DATA },
|
||||
{ ...owner, role: 'credential:owner' },
|
||||
);
|
||||
}
|
||||
|
||||
const denyGithub = async (projectId: string | null) =>
|
||||
await putCredentialTypePolicy(ownerAgent, projectId, {
|
||||
rules: [denyRule('deny-github', GITHUB_API)],
|
||||
});
|
||||
|
||||
const testRoute = async (credential: CredentialsEntity) =>
|
||||
await ownerAgent.post('/credentials/test').send({
|
||||
credentials: { id: credential.id, name: credential.name, type: credential.type, data: {} },
|
||||
});
|
||||
|
||||
beforeAll(async () => {
|
||||
owner = await createOwnerWithApiKey();
|
||||
ownerAgent = testServer.authAgentFor(owner);
|
||||
publicApiAgent = testServer.publicApiAgentFor(owner);
|
||||
|
||||
await utils.initCredentialsTypes();
|
||||
});
|
||||
|
||||
beforeEach(() => {
|
||||
credentialsTester.testCredentials.mockResolvedValue({ status: 'OK', message: 'Tested' });
|
||||
credentialsTester.probeCredentialAuth.mockResolvedValue({
|
||||
status: 'OK',
|
||||
message: 'Tested',
|
||||
outcome: 'accepted',
|
||||
});
|
||||
});
|
||||
|
||||
afterEach(async () => {
|
||||
vi.clearAllMocks();
|
||||
await testDb.truncate([
|
||||
'TypeAvailabilityPolicyAttachment',
|
||||
'TypeAvailabilityPolicyScope',
|
||||
'TypeAvailabilityPolicy',
|
||||
'SharedCredentials',
|
||||
'CredentialsEntity',
|
||||
]);
|
||||
await clearPolicyCache();
|
||||
});
|
||||
|
||||
describe('a credential of a type an instance policy blocks', () => {
|
||||
test('POST /credentials/test is refused before the tester runs', async () => {
|
||||
const credential = await saveGithubCredential({ user: owner });
|
||||
await denyGithub(null);
|
||||
|
||||
const response = await testRoute(credential);
|
||||
|
||||
expect(response.statusCode).toBe(403);
|
||||
expect(response.body.message).toBe(
|
||||
`Credential type "${GITHUB_API}" is blocked by an instance policy`,
|
||||
);
|
||||
expect(credentialsTester.testCredentials).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
test('POST /credentials/:id/probe is refused before the probe runs', async () => {
|
||||
const credential = await saveGithubCredential({ user: owner });
|
||||
await denyGithub(null);
|
||||
|
||||
const response = await ownerAgent.post(`/credentials/${credential.id}/probe`);
|
||||
|
||||
expect(response.statusCode).toBe(403);
|
||||
expect(credentialsTester.probeCredentialAuth).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
test('public API POST /credentials/:id/test is refused before the tester runs', async () => {
|
||||
const credential = await saveGithubCredential({ user: owner });
|
||||
await denyGithub(null);
|
||||
|
||||
const response = await publicApiAgent.post(`/credentials/${credential.id}/test`);
|
||||
|
||||
expect(response.statusCode).toBe(403);
|
||||
expect(credentialsTester.testCredentials).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
test('an unsaved instance credential test is refused', async () => {
|
||||
await denyGithub(null);
|
||||
|
||||
const response = await ownerAgent.post('/credentials/test').send({
|
||||
credentials: { id: '', name: 'GitHub account', type: GITHUB_API, data: {} },
|
||||
});
|
||||
|
||||
expect(response.statusCode).toBe(403);
|
||||
expect(credentialsTester.testCredentials).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
describe('a credential of a type a project policy blocks', () => {
|
||||
test('is refused when the blocking project owns it', async () => {
|
||||
const project = await createTeamProject('Blocking project', owner);
|
||||
const credential = await saveGithubCredential({ project });
|
||||
await denyGithub(project.id);
|
||||
|
||||
const response = await testRoute(credential);
|
||||
|
||||
expect(response.statusCode).toBe(403);
|
||||
expect(credentialsTester.testCredentials).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
test('still tests when another project owns it', async () => {
|
||||
const blockingProject = await createTeamProject('Blocking project', owner);
|
||||
await denyGithub(blockingProject.id);
|
||||
const credential = await saveGithubCredential({ user: owner });
|
||||
|
||||
const response = await testRoute(credential);
|
||||
|
||||
expect(response.statusCode).toBe(200);
|
||||
expect(credentialsTester.testCredentials).toHaveBeenCalledOnce();
|
||||
});
|
||||
});
|
||||
|
||||
describe('a test whose posted type differs from the stored one', () => {
|
||||
test('is rejected before the tester runs', async () => {
|
||||
const credential = await saveGithubCredential({ user: owner });
|
||||
|
||||
const response = await ownerAgent.post('/credentials/test').send({
|
||||
credentials: { id: credential.id, name: credential.name, type: 'httpHeaderAuth', data: {} },
|
||||
});
|
||||
|
||||
expect(response.statusCode).toBe(400);
|
||||
expect(credentialsTester.testCredentials).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
describe('a credential of a type no rule denies', () => {
|
||||
test('tests and probes as before', async () => {
|
||||
const credential = await saveGithubCredential({ user: owner });
|
||||
await putCredentialTypePolicy(ownerAgent, null, {
|
||||
rules: [denyRule('deny-slack', 'slackApi')],
|
||||
});
|
||||
|
||||
expect((await testRoute(credential)).statusCode).toBe(200);
|
||||
expect((await ownerAgent.post(`/credentials/${credential.id}/probe`)).statusCode).toBe(200);
|
||||
expect(credentialsTester.testCredentials).toHaveBeenCalledOnce();
|
||||
expect(credentialsTester.probeCredentialAuth).toHaveBeenCalledOnce();
|
||||
});
|
||||
});
|
||||
|
|
@ -22,6 +22,12 @@ vi.mock('../legacy-request-adapter', () => ({
|
|||
proxyRequestToAxios: vi.fn(),
|
||||
}));
|
||||
|
||||
/** Shaped like cli's `PolicyViolationError`, which core cannot import. */
|
||||
const policyRefusal = () =>
|
||||
Object.assign(new Error('Credential type "slackApi" is blocked by an instance policy'), {
|
||||
isPolicyRefusal: true,
|
||||
});
|
||||
|
||||
describe('httpRequestWithAuthentication', () => {
|
||||
const baseUrl = 'https://api.example.com';
|
||||
const tokenUrl = 'https://auth.example.com';
|
||||
|
|
@ -374,6 +380,25 @@ describe('httpRequestWithAuthentication', () => {
|
|||
expect(result).toEqual({ ok: true });
|
||||
expect(request).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
test('rethrows a policy refusal from the credential read unchanged, with no request sent', async () => {
|
||||
mockAdditionalData.credentialsHelper.getParentTypes.mockReturnValue([]);
|
||||
const refusal = policyRefusal();
|
||||
mockThis.getCredentials.mockRejectedValue(refusal);
|
||||
|
||||
await expect(
|
||||
httpRequestWithAuthentication.call(
|
||||
mockThis,
|
||||
'slackApi',
|
||||
{ method: 'GET', url: `${baseUrl}/items` },
|
||||
mockWorkflow,
|
||||
mockNode,
|
||||
mockAdditionalData,
|
||||
),
|
||||
).rejects.toBe(refusal);
|
||||
|
||||
expect(request).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
describe('requestWithAuthentication (legacy) — preAuthentication retry', () => {
|
||||
|
|
@ -508,4 +533,22 @@ describe('requestWithAuthentication (legacy) — preAuthentication retry', () =>
|
|||
);
|
||||
},
|
||||
);
|
||||
|
||||
test('rethrows a policy refusal from the credential read unchanged, with no request sent', async () => {
|
||||
const refusal = policyRefusal();
|
||||
mockThis.getCredentials.mockRejectedValue(refusal);
|
||||
|
||||
await expect(
|
||||
requestWithAuthentication.call(
|
||||
mockThis,
|
||||
'slackApi',
|
||||
{ method: 'GET', uri: 'https://api.example.com/items' },
|
||||
mockWorkflow,
|
||||
mockNode,
|
||||
mockAdditionalData,
|
||||
),
|
||||
).rejects.toBe(refusal);
|
||||
|
||||
expect(proxyRequestToAxiosMock).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -13,7 +13,12 @@ import type {
|
|||
IWorkflowExecuteAdditionalData,
|
||||
Workflow,
|
||||
} from 'n8n-workflow';
|
||||
import { ExecutionBaseError, NodeApiError, NodeOperationError } from 'n8n-workflow';
|
||||
import {
|
||||
ExecutionBaseError,
|
||||
hasPolicyRefusalMarker,
|
||||
NodeApiError,
|
||||
NodeOperationError,
|
||||
} from 'n8n-workflow';
|
||||
|
||||
import { callEvalMockHandler, normalizeLegacyRequest } from '@/execution-engine/eval-mock-helpers';
|
||||
|
||||
|
|
@ -137,6 +142,9 @@ export async function httpRequestWithAuthentication(
|
|||
requestSent = true;
|
||||
return await Container.get(OutboundHttp).requests().request(requestOptions);
|
||||
} catch (error) {
|
||||
// Keep the policy reason visible instead of reporting it as a failed API call.
|
||||
if (hasPolicyRefusalMarker(error)) throw error;
|
||||
|
||||
// if there is a pre authorization method defined and
|
||||
// the method failed due to unauthorized request
|
||||
if (
|
||||
|
|
@ -316,7 +324,7 @@ export async function requestWithAuthentication(
|
|||
}
|
||||
throw error;
|
||||
} catch (error) {
|
||||
if (error instanceof ExecutionBaseError) throw error;
|
||||
if (error instanceof ExecutionBaseError || hasPolicyRefusalMarker(error)) throw error;
|
||||
|
||||
throw new NodeApiError(this.getNode(), error);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -46,4 +46,5 @@ export { ExpressionClassExtensionError } from './expression-class-extension.erro
|
|||
export { ExpressionReservedVariableError } from './expression-reserved-variable.error';
|
||||
export { ExpressionWithStatementError } from './expression-with-statement.error';
|
||||
export { DbConnectionTimeoutError } from './db-connection-timeout-error';
|
||||
export { hasPolicyRefusalMarker } from './policy-refusal';
|
||||
export { IsolateError } from '@n8n/errors';
|
||||
|
|
|
|||
14
packages/workflow/src/errors/policy-refusal.ts
Normal file
14
packages/workflow/src/errors/policy-refusal.ts
Normal file
|
|
@ -0,0 +1,14 @@
|
|||
/**
|
||||
* Whether an error carries the own `isPolicyRefusal` marker that a policy refusal sets.
|
||||
* Lets packages that cannot import the refusal class recognise one, also after serialization.
|
||||
*/
|
||||
export function hasPolicyRefusalMarker(error: unknown): boolean {
|
||||
// `hasOwn`, not `in` alone: an inherited marker must not count.
|
||||
return (
|
||||
typeof error === 'object' &&
|
||||
error !== null &&
|
||||
Object.hasOwn(error, 'isPolicyRefusal') &&
|
||||
'isPolicyRefusal' in error &&
|
||||
error.isPolicyRefusal === true
|
||||
);
|
||||
}
|
||||
24
packages/workflow/test/errors/policy-refusal.test.ts
Normal file
24
packages/workflow/test/errors/policy-refusal.test.ts
Normal file
|
|
@ -0,0 +1,24 @@
|
|||
import { hasPolicyRefusalMarker } from '../../src/errors';
|
||||
|
||||
describe('hasPolicyRefusalMarker', () => {
|
||||
it('recognises an error that carries the marker as an own property', () => {
|
||||
const refusal = Object.assign(new Error('Blocked'), { isPolicyRefusal: true });
|
||||
|
||||
expect(hasPolicyRefusalMarker(refusal)).toBe(true);
|
||||
});
|
||||
|
||||
it('does not recognise an inherited marker', () => {
|
||||
const inherited = Object.create({ isPolicyRefusal: true }) as object;
|
||||
|
||||
expect(hasPolicyRefusalMarker(inherited)).toBe(false);
|
||||
});
|
||||
|
||||
it.each([
|
||||
['an ordinary error', new Error('boom')],
|
||||
['a non-true marker', { isPolicyRefusal: 'yes' }],
|
||||
['null', null],
|
||||
['undefined', undefined],
|
||||
])('does not recognise %s', (_label, value) => {
|
||||
expect(hasPolicyRefusalMarker(value)).toBe(false);
|
||||
});
|
||||
});
|
||||
Loading…
Reference in New Issue
Block a user