mirror of
https://github.com/n8n-io/n8n.git
synced 2026-08-28 17:22:01 +08:00
feat(Databricks Node): Add user-delegated OAuth2 credential (authorization code + PKCE) (#37007)
This commit is contained in:
@@ -19,26 +19,129 @@ export class DatabricksOAuth2Api implements ICredentialType {
|
||||
default: '',
|
||||
placeholder: 'https://adb-xxxxx.xx.azure.databricks.com',
|
||||
required: true,
|
||||
description: 'Domain of your Databricks workspace',
|
||||
description: 'Domain of your Databricks workspace, must be <code>https</code>',
|
||||
},
|
||||
{
|
||||
displayName: 'Grant Type',
|
||||
name: 'grantType',
|
||||
type: 'hidden',
|
||||
type: 'options',
|
||||
options: [
|
||||
{
|
||||
name: 'Client Credentials (Service Principal)',
|
||||
value: 'clientCredentials',
|
||||
},
|
||||
{
|
||||
name: 'Authorization Code (User)',
|
||||
value: 'authorizationCode',
|
||||
},
|
||||
],
|
||||
// Default stays clientCredentials so already-saved credentials keep working
|
||||
default: 'clientCredentials',
|
||||
},
|
||||
{
|
||||
displayName: 'Custom Scopes',
|
||||
name: 'customScopes',
|
||||
type: 'boolean',
|
||||
default: false,
|
||||
description: 'Whether to define custom OAuth scopes instead of the default all-apis',
|
||||
},
|
||||
{
|
||||
displayName:
|
||||
'The default scopes needed for the node to work are already set. If you change them the node may not function correctly.',
|
||||
name: 'customScopesNotice',
|
||||
type: 'notice',
|
||||
default: '',
|
||||
displayOptions: {
|
||||
show: {
|
||||
customScopes: [true],
|
||||
grantType: ['clientCredentials'],
|
||||
},
|
||||
},
|
||||
},
|
||||
{
|
||||
// Same name suffix pattern as the scopes fields: the extends-chain merge
|
||||
// dedupes by name, so the per-grant notices need distinct names
|
||||
displayName:
|
||||
'The default scopes needed for the node to work are already set. If you change them the node may not function correctly. <code>offline_access</code> is required to keep the connection alive past one hour and is re-added automatically if removed.',
|
||||
name: 'userCustomScopesNotice',
|
||||
type: 'notice',
|
||||
default: '',
|
||||
displayOptions: {
|
||||
show: {
|
||||
customScopes: [true],
|
||||
grantType: ['authorizationCode'],
|
||||
},
|
||||
},
|
||||
},
|
||||
{
|
||||
// One field per grant type (a default can't depend on another field, and
|
||||
// the extends-chain property merge dedupes by name, so the names must
|
||||
// differ): the user grant pre-fills offline_access so it's visible up front
|
||||
displayName: 'Enabled Scopes',
|
||||
name: 'enabledScopes',
|
||||
type: 'string',
|
||||
displayOptions: {
|
||||
show: {
|
||||
customScopes: [true],
|
||||
grantType: ['clientCredentials'],
|
||||
},
|
||||
},
|
||||
default: 'all-apis',
|
||||
description: 'Space-separated OAuth scopes to request',
|
||||
},
|
||||
{
|
||||
displayName: 'Enabled Scopes',
|
||||
name: 'userEnabledScopes',
|
||||
type: 'string',
|
||||
displayOptions: {
|
||||
show: {
|
||||
customScopes: [true],
|
||||
grantType: ['authorizationCode'],
|
||||
},
|
||||
},
|
||||
default: 'all-apis offline_access',
|
||||
description: 'Space-separated OAuth scopes to request',
|
||||
},
|
||||
{
|
||||
// Trailing slash is stripped because users paste the host straight from the
|
||||
// browser, and `host/` + `/oidc/...` yields a double slash Databricks 404s on
|
||||
displayName: 'Authorization URL',
|
||||
name: 'authUrl',
|
||||
type: 'hidden',
|
||||
default: '={{$self["host"].replace(/\\/$/, "")}}/oidc/v1/authorize',
|
||||
required: true,
|
||||
},
|
||||
{
|
||||
displayName: 'Access Token URL',
|
||||
name: 'accessTokenUrl',
|
||||
type: 'hidden',
|
||||
default: '={{$self["host"]}}/oidc/v1/token',
|
||||
default: '={{$self["host"].replace(/\\/$/, "")}}/oidc/v1/token',
|
||||
required: true,
|
||||
},
|
||||
{
|
||||
// offline_access is what makes Databricks issue a refresh token, so the user
|
||||
// grant survives past the one-hour access token. It is force-appended on that
|
||||
// grant even with custom scopes.
|
||||
//
|
||||
// For this expression to win on reconnect, OauthService.getOAuthCredentials
|
||||
// must delete any stale stored scope (e.g. `all-apis` saved before a switch
|
||||
// to the user grant). That cleanup only runs while:
|
||||
// 1. `scope` stays hidden, and
|
||||
// 2. this credential stays OUT of GENERIC_OAUTH2_CREDENTIALS_WITH_EDITABLE_SCOPE
|
||||
// (packages/cli/src/constants.ts).
|
||||
// It never touches the customScopes/enabledScopes/userEnabledScopes fields,
|
||||
// so the user's custom scopes survive it.
|
||||
displayName: 'Scope',
|
||||
name: 'scope',
|
||||
type: 'hidden',
|
||||
default: 'all-apis',
|
||||
default:
|
||||
'={{$self["customScopes"] ? ($self["grantType"] === "authorizationCode" ? (($self["userEnabledScopes"].trim() || "all-apis") + ($self["userEnabledScopes"].trim().split(" ").includes("offline_access") ? "" : " offline_access")) : ($self["enabledScopes"].trim() || "all-apis")) : ($self["grantType"] === "authorizationCode" ? "all-apis offline_access" : "all-apis")}}',
|
||||
},
|
||||
{
|
||||
displayName: 'Use PKCE',
|
||||
name: 'usePkce',
|
||||
type: 'hidden',
|
||||
default: true,
|
||||
},
|
||||
{
|
||||
displayName: 'Auth URI Query Parameters',
|
||||
@@ -59,16 +162,14 @@ export class DatabricksOAuth2Api implements ICredentialType {
|
||||
// when tokens expire, so the default must be 403.
|
||||
displayName: 'Token Expired Status Code',
|
||||
name: 'tokenExpiredStatusCode',
|
||||
type: 'number',
|
||||
type: 'hidden',
|
||||
default: 403,
|
||||
description:
|
||||
'HTTP status code that indicates the token has expired. Databricks returns 403 when tokens expire.',
|
||||
},
|
||||
];
|
||||
|
||||
test: ICredentialTestRequest = {
|
||||
request: {
|
||||
baseURL: '={{$credentials.host}}',
|
||||
baseURL: '={{$credentials.host.replace(/\\/$/, "")}}',
|
||||
url: '/api/2.0/preview/scim/v2/Me',
|
||||
method: 'GET',
|
||||
},
|
||||
|
||||
@@ -2,24 +2,156 @@ import { DatabricksOAuth2Api } from '../DatabricksOAuth2Api.credentials';
|
||||
|
||||
describe('DatabricksOAuth2Api Credential', () => {
|
||||
const databricksOAuth2Api = new DatabricksOAuth2Api();
|
||||
const property = (name: string) => databricksOAuth2Api.properties.find((p) => p.name === name);
|
||||
|
||||
it('should have correct credential metadata', () => {
|
||||
expect(databricksOAuth2Api.name).toBe('databricksOAuth2Api');
|
||||
expect(databricksOAuth2Api.extends).toEqual(['oAuth2Api']);
|
||||
});
|
||||
|
||||
const grantType = databricksOAuth2Api.properties.find((p) => p.name === 'grantType');
|
||||
expect(grantType?.default).toBe('clientCredentials');
|
||||
describe('grantType', () => {
|
||||
const field = property('grantType');
|
||||
|
||||
const accessTokenUrl = databricksOAuth2Api.properties.find((p) => p.name === 'accessTokenUrl');
|
||||
expect(accessTokenUrl?.default).toContain('/oidc/v1/token');
|
||||
it('should offer exactly client credentials and authorization code', () => {
|
||||
expect(field?.type).toBe('options');
|
||||
expect(field?.options?.map((o) => 'value' in o && o.value)).toEqual([
|
||||
'clientCredentials',
|
||||
'authorizationCode',
|
||||
]);
|
||||
});
|
||||
|
||||
it('should default to clientCredentials for backward compatibility', () => {
|
||||
expect(field?.default).toBe('clientCredentials');
|
||||
});
|
||||
});
|
||||
|
||||
describe('OAuth URLs', () => {
|
||||
it('should build authUrl from the host with the trailing slash stripped', () => {
|
||||
const field = property('authUrl');
|
||||
expect(field?.type).toBe('hidden');
|
||||
expect(field?.default).toBe('={{$self["host"].replace(/\\/$/, "")}}/oidc/v1/authorize');
|
||||
});
|
||||
|
||||
it('should build accessTokenUrl from the host with the trailing slash stripped', () => {
|
||||
const field = property('accessTokenUrl');
|
||||
expect(field?.type).toBe('hidden');
|
||||
expect(field?.default).toBe('={{$self["host"].replace(/\\/$/, "")}}/oidc/v1/token');
|
||||
});
|
||||
|
||||
it('should strip the trailing slash from the credential test baseURL', () => {
|
||||
expect(databricksOAuth2Api.test.request.baseURL).toBe(
|
||||
'={{$credentials.host.replace(/\\/$/, "")}}',
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe('scope', () => {
|
||||
const field = property('scope');
|
||||
|
||||
// Hidden is load-bearing: an editable scope property would make
|
||||
// OauthService.getOAuthCredentials keep stale stored scopes on reconnect
|
||||
it('should stay hidden and force offline_access on the authorization code grant', () => {
|
||||
expect(field?.type).toBe('hidden');
|
||||
expect(field?.default).toBe(
|
||||
'={{$self["customScopes"] ? ($self["grantType"] === "authorizationCode" ? (($self["userEnabledScopes"].trim() || "all-apis") + ($self["userEnabledScopes"].trim().split(" ").includes("offline_access") ? "" : " offline_access")) : ($self["enabledScopes"].trim() || "all-apis")) : ($self["grantType"] === "authorizationCode" ? "all-apis offline_access" : "all-apis")}}',
|
||||
);
|
||||
});
|
||||
|
||||
const evaluate = (customScopes: boolean, scopes: string, grantType: string) => {
|
||||
// Evaluates the credential's real default expression, not a transcription
|
||||
// of it, so a regression in the formula fails this table directly. The
|
||||
// grant picks its own field, so `scopes` stands in for enabledScopes or
|
||||
// userEnabledScopes accordingly.
|
||||
const expression = (field?.default as string).replace(/^=\{\{/, '').replace(/\}\}$/, '');
|
||||
// eslint-disable-next-line @typescript-eslint/no-implied-eval
|
||||
const evalScope = new Function('$self', `return (${expression});`) as (
|
||||
$self: Record<string, unknown>,
|
||||
) => string;
|
||||
return evalScope({
|
||||
customScopes,
|
||||
enabledScopes: scopes,
|
||||
userEnabledScopes: scopes,
|
||||
grantType,
|
||||
});
|
||||
};
|
||||
|
||||
it.each([
|
||||
[false, 'all-apis', 'clientCredentials', 'all-apis'],
|
||||
[false, 'all-apis', 'authorizationCode', 'all-apis offline_access'],
|
||||
[true, 'sql files', 'clientCredentials', 'sql files'],
|
||||
[true, 'sql files', 'authorizationCode', 'sql files offline_access'],
|
||||
[true, 'sql offline_access', 'authorizationCode', 'sql offline_access'],
|
||||
// offline_access must match as a whole token, not a substring
|
||||
[
|
||||
true,
|
||||
'sql offline_access_extra',
|
||||
'authorizationCode',
|
||||
'sql offline_access_extra offline_access',
|
||||
],
|
||||
// A cleared custom-scopes field falls back to the grant's default scopes
|
||||
[true, '', 'clientCredentials', 'all-apis'],
|
||||
[true, ' ', 'authorizationCode', 'all-apis offline_access'],
|
||||
])('customScopes=%s scopes=%s %s -> %s', (customScopes, scopes, grantType, expected) => {
|
||||
expect(evaluate(customScopes, scopes, grantType)).toBe(expected);
|
||||
});
|
||||
});
|
||||
|
||||
describe('custom scopes fields', () => {
|
||||
it('should default customScopes off so existing credentials keep the default scope', () => {
|
||||
const field = property('customScopes');
|
||||
expect(field?.type).toBe('boolean');
|
||||
expect(field?.default).toBe(false);
|
||||
});
|
||||
|
||||
it.each([
|
||||
['customScopesNotice', 'clientCredentials'],
|
||||
['userCustomScopesNotice', 'authorizationCode'],
|
||||
])('should only show %s with customScopes on for %s', (name, grantType) => {
|
||||
expect(property(name)?.displayOptions).toEqual({
|
||||
show: { customScopes: [true], grantType: [grantType] },
|
||||
});
|
||||
});
|
||||
|
||||
it('should mention offline_access only in the authorization code notice', () => {
|
||||
expect(property('customScopesNotice')?.displayName).not.toContain('offline_access');
|
||||
expect(property('userCustomScopesNotice')?.displayName).toContain('offline_access');
|
||||
});
|
||||
|
||||
// One differently-named scopes field per grant type: a default can't depend
|
||||
// on another field, and the extends-chain property merge
|
||||
// (NodeHelpers.mergeNodeProperties) dedupes by name, so a same-name pair
|
||||
// would collapse to one field and never display
|
||||
it.each([
|
||||
['enabledScopes', 'clientCredentials', 'all-apis'],
|
||||
['userEnabledScopes', 'authorizationCode', 'all-apis offline_access'],
|
||||
])('should show %s for %s defaulting to %s', (name, grantType, expected) => {
|
||||
const field = property(name);
|
||||
expect(field?.displayOptions).toEqual({
|
||||
show: { customScopes: [true], grantType: [grantType] },
|
||||
});
|
||||
expect(field?.default).toBe(expected);
|
||||
});
|
||||
|
||||
it('should not declare duplicate property names (the extends merge would drop one)', () => {
|
||||
const names = databricksOAuth2Api.properties.map((p) => p.name);
|
||||
expect(new Set(names).size).toBe(names.length);
|
||||
});
|
||||
});
|
||||
|
||||
describe('usePkce', () => {
|
||||
const field = property('usePkce');
|
||||
|
||||
// Core only consults usePkce for the authorizationCode grant, so a plain
|
||||
// hidden default is inert for service principals
|
||||
it('should be hidden and enabled by default', () => {
|
||||
expect(field?.type).toBe('hidden');
|
||||
expect(field?.default).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
describe('tokenExpiredStatusCode', () => {
|
||||
const field = databricksOAuth2Api.properties.find((p) => p.name === 'tokenExpiredStatusCode');
|
||||
const field = property('tokenExpiredStatusCode');
|
||||
|
||||
// The base `oAuth2Api` field is `doNotInherit`, so it must be re-declared
|
||||
// here or `credentials.tokenExpiredStatusCode` is always undefined and token
|
||||
// refresh stays hardcoded to 401.
|
||||
it('should be declared so it reaches the decrypted credential', () => {
|
||||
expect(field).toBeDefined();
|
||||
});
|
||||
@@ -28,9 +160,8 @@ describe('DatabricksOAuth2Api Credential', () => {
|
||||
expect(field?.default).toBe(403);
|
||||
});
|
||||
|
||||
it('should be a configurable number field (not hidden)', () => {
|
||||
expect(field?.type).toBe('number');
|
||||
expect(field?.type).not.toBe('hidden');
|
||||
it('should be hidden from the credential form', () => {
|
||||
expect(field?.type).toBe('hidden');
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
+16
-6
@@ -2,7 +2,12 @@ import { NodeOperationError } from 'n8n-workflow';
|
||||
import type { IDataObject, IExecuteFunctions, INodeExecutionData } from 'n8n-workflow';
|
||||
|
||||
import { sleep } from '@n8n/utils/sleep';
|
||||
import { extractResourceLocatorValue, getActiveCredentialType, getHost } from '../helpers';
|
||||
import {
|
||||
extractResourceLocatorValue,
|
||||
getActiveCredentialType,
|
||||
getHost,
|
||||
sanitizeApiMessage,
|
||||
} from '../helpers';
|
||||
import type { DatabricksStatementResponse } from '../interfaces';
|
||||
|
||||
export async function execute(this: IExecuteFunctions, i: number): Promise<INodeExecutionData[]> {
|
||||
@@ -71,11 +76,16 @@ export async function execute(this: IExecuteFunctions, i: number): Promise<INode
|
||||
}
|
||||
|
||||
if (status === 'FAILED' || status === 'CANCELED') {
|
||||
throw new NodeOperationError(
|
||||
this.getNode(),
|
||||
`Query ${status.toLowerCase()}: ${JSON.stringify(queryResult.status)}`,
|
||||
{ itemIndex: i },
|
||||
);
|
||||
// Databricks reports SQL permission failures in-band on an HTTP 200: prefer
|
||||
// the legible status.error.message over the raw status blob when present
|
||||
const apiMessage = queryResult.status.error?.message;
|
||||
const reason =
|
||||
typeof apiMessage === 'string' && apiMessage
|
||||
? sanitizeApiMessage(apiMessage)
|
||||
: JSON.stringify(queryResult.status);
|
||||
throw new NodeOperationError(this.getNode(), `Query ${status.toLowerCase()}: ${reason}`, {
|
||||
itemIndex: i,
|
||||
});
|
||||
}
|
||||
|
||||
if (retries >= maxRetries) {
|
||||
|
||||
@@ -2,7 +2,7 @@ import mime from 'mime-types';
|
||||
import { NodeOperationError } from 'n8n-workflow';
|
||||
import type { IExecuteFunctions, INodeExecutionData } from 'n8n-workflow';
|
||||
|
||||
import { getActiveCredentialType, getHost } from '../helpers';
|
||||
import { getActiveCredentialType, getHost, makePermissionErrorLegible } from '../helpers';
|
||||
|
||||
export async function execute(this: IExecuteFunctions, i: number): Promise<INodeExecutionData[]> {
|
||||
const credentialType = getActiveCredentialType(this, i);
|
||||
@@ -49,6 +49,7 @@ export async function execute(this: IExecuteFunctions, i: number): Promise<INode
|
||||
},
|
||||
];
|
||||
} catch (error) {
|
||||
makePermissionErrorLegible(error);
|
||||
if (this.continueOnFail()) {
|
||||
return [
|
||||
{
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
import { UserError } from 'n8n-workflow';
|
||||
import type { IExecuteFunctions, ILoadOptionsFunctions } from 'n8n-workflow';
|
||||
import { NodeApiError, UserError } from 'n8n-workflow';
|
||||
import type { IDataObject, IExecuteFunctions, ILoadOptionsFunctions } from 'n8n-workflow';
|
||||
|
||||
import type { DatabricksCredentials, OpenAPISchema } from './interfaces';
|
||||
|
||||
@@ -23,6 +23,44 @@ export async function getHost(
|
||||
return credentials.host.replace(/\/$/, '');
|
||||
}
|
||||
|
||||
// Body text comes from whatever server `host` points at — truncate and strip
|
||||
// control chars before promoting it to a visible error message
|
||||
export function sanitizeApiMessage(message: string): string {
|
||||
// eslint-disable-next-line no-control-regex
|
||||
return message.replace(/[\x00-\x1f\x7f]+/g, ' ').slice(0, 500);
|
||||
}
|
||||
|
||||
// Must be called at every request entry point (router catch, listSearch wrapper) —
|
||||
// the node has no shared transport helper. Keyed on PERMISSION_DENIED only; widen
|
||||
// the key if other Databricks error_codes with legible messages show up. Keyed on
|
||||
// the error_code, not HTTP 403, so expired-token 403s (which core retries via
|
||||
// refresh) aren't mislabeled if they leak through. Mutates rather than re-wraps:
|
||||
// `new NodeApiError(node, existingNodeApiError)` returns the original untouched.
|
||||
export function makePermissionErrorLegible(error: unknown): void {
|
||||
if (!(error instanceof NodeApiError)) return;
|
||||
|
||||
// Requests with encoding: 'arraybuffer' (file downloads) receive their 403
|
||||
// JSON body as raw bytes, so parse Buffer/string bodies before reading it
|
||||
let data = error.context.data;
|
||||
if (Buffer.isBuffer(data) || typeof data === 'string') {
|
||||
try {
|
||||
data = JSON.parse(data.toString()) as IDataObject;
|
||||
} catch {
|
||||
return;
|
||||
}
|
||||
}
|
||||
|
||||
const body = data as IDataObject | undefined;
|
||||
if (body?.error_code !== 'PERMISSION_DENIED') return;
|
||||
|
||||
const apiMessage = body.message;
|
||||
if (typeof apiMessage === 'string' && apiMessage) {
|
||||
error.message = sanitizeApiMessage(apiMessage);
|
||||
error.description =
|
||||
'Grant the named permission to the signed-in user or service principal in Databricks, then retry.';
|
||||
}
|
||||
}
|
||||
|
||||
export function extractResourceLocatorValue(param: unknown): string {
|
||||
if (typeof param === 'object' && param !== null) {
|
||||
return (param as { value?: string }).value || '';
|
||||
|
||||
@@ -4,6 +4,7 @@ import type { IExecuteFunctions, INodeExecutionData } from 'n8n-workflow';
|
||||
import * as databricksSql from './databricksSql/DatabricksSql.resource';
|
||||
import * as files from './files/Files.resource';
|
||||
import * as genie from './genie/Genie.resource';
|
||||
import { makePermissionErrorLegible } from './helpers';
|
||||
import * as modelServing from './modelServing/ModelServing.resource';
|
||||
import * as unityCatalog from './unityCatalog/UnityCatalog.resource';
|
||||
import * as vectorSearch from './vectorSearch/VectorSearch.resource';
|
||||
@@ -67,6 +68,7 @@ export async function router(this: IExecuteFunctions): Promise<INodeExecutionDat
|
||||
const result = await operationModule.execute.call(this, i);
|
||||
returnData.push(...result);
|
||||
} catch (error) {
|
||||
makePermissionErrorLegible(error);
|
||||
if (this.continueOnFail()) {
|
||||
returnData.push({ json: { error: (error as Error).message }, pairedItem: { item: i } });
|
||||
continue;
|
||||
|
||||
@@ -1,6 +1,35 @@
|
||||
import type { ILoadOptionsFunctions, INodeListSearchResult } from 'n8n-workflow';
|
||||
import type {
|
||||
IHttpRequestOptions,
|
||||
ILoadOptionsFunctions,
|
||||
INodeListSearchResult,
|
||||
} from 'n8n-workflow';
|
||||
|
||||
import { extractResourceLocatorValue, getActiveCredentialType, getHost } from '../actions/helpers';
|
||||
import {
|
||||
extractResourceLocatorValue,
|
||||
getActiveCredentialType,
|
||||
getHost,
|
||||
makePermissionErrorLegible,
|
||||
sanitizeApiMessage,
|
||||
} from '../actions/helpers';
|
||||
|
||||
// Dropdown requests never pass through the router, so its permission-error hook
|
||||
// doesn't cover them — apply it here for every listSearch call site instead
|
||||
async function listRequest<T>(
|
||||
context: ILoadOptionsFunctions,
|
||||
credentialType: 'databricksApi' | 'databricksOAuth2Api',
|
||||
options: IHttpRequestOptions,
|
||||
): Promise<T> {
|
||||
try {
|
||||
return (await context.helpers.httpRequestWithAuthentication.call(
|
||||
context,
|
||||
credentialType,
|
||||
options,
|
||||
)) as T;
|
||||
} catch (error) {
|
||||
makePermissionErrorLegible(error);
|
||||
throw error;
|
||||
}
|
||||
}
|
||||
|
||||
export async function getWarehouses(
|
||||
this: ILoadOptionsFunctions,
|
||||
@@ -9,12 +38,14 @@ export async function getWarehouses(
|
||||
const credentialType = getActiveCredentialType(this);
|
||||
const host = await getHost(this, credentialType);
|
||||
|
||||
const response = (await this.helpers.httpRequestWithAuthentication.call(this, credentialType, {
|
||||
const response = await listRequest<{
|
||||
warehouses?: Array<{ id: string; name: string; size?: string }>;
|
||||
}>(this, credentialType, {
|
||||
method: 'GET',
|
||||
url: `${host}/api/2.0/sql/warehouses`,
|
||||
headers: { Accept: 'application/json' },
|
||||
json: true,
|
||||
})) as { warehouses?: Array<{ id: string; name: string; size?: string }> };
|
||||
});
|
||||
|
||||
const warehouses = response.warehouses ?? [];
|
||||
|
||||
@@ -39,12 +70,7 @@ export async function getEndpoints(
|
||||
const credentialType = getActiveCredentialType(this);
|
||||
const host = await getHost(this, credentialType);
|
||||
|
||||
const response = (await this.helpers.httpRequestWithAuthentication.call(this, credentialType, {
|
||||
method: 'GET',
|
||||
url: `${host}/api/2.0/serving-endpoints`,
|
||||
headers: { Accept: 'application/json' },
|
||||
json: true,
|
||||
})) as {
|
||||
const response = await listRequest<{
|
||||
endpoints?: Array<{
|
||||
name: string;
|
||||
config?: {
|
||||
@@ -54,7 +80,12 @@ export async function getEndpoints(
|
||||
}>;
|
||||
};
|
||||
}>;
|
||||
};
|
||||
}>(this, credentialType, {
|
||||
method: 'GET',
|
||||
url: `${host}/api/2.0/serving-endpoints`,
|
||||
headers: { Accept: 'application/json' },
|
||||
json: true,
|
||||
});
|
||||
|
||||
const endpoints = response.endpoints ?? [];
|
||||
|
||||
@@ -93,12 +124,16 @@ export async function getCatalogs(
|
||||
const credentialType = getActiveCredentialType(this);
|
||||
const host = await getHost(this, credentialType);
|
||||
|
||||
const response = (await this.helpers.httpRequestWithAuthentication.call(this, credentialType, {
|
||||
method: 'GET',
|
||||
url: `${host}/api/2.1/unity-catalog/catalogs`,
|
||||
headers: { Accept: 'application/json' },
|
||||
json: true,
|
||||
})) as { catalogs?: Array<{ name: string; comment?: string }> };
|
||||
const response = await listRequest<{ catalogs?: Array<{ name: string; comment?: string }> }>(
|
||||
this,
|
||||
credentialType,
|
||||
{
|
||||
method: 'GET',
|
||||
url: `${host}/api/2.1/unity-catalog/catalogs`,
|
||||
headers: { Accept: 'application/json' },
|
||||
json: true,
|
||||
},
|
||||
);
|
||||
|
||||
const catalogs = response.catalogs ?? [];
|
||||
|
||||
@@ -137,7 +172,7 @@ export async function getSchemas(
|
||||
}
|
||||
|
||||
try {
|
||||
const schemasResponse = (await this.helpers.httpRequestWithAuthentication.call(
|
||||
const schemasResponse = await listRequest<{ schemas?: Array<{ name: string }> }>(
|
||||
this,
|
||||
credentialType,
|
||||
{
|
||||
@@ -146,7 +181,7 @@ export async function getSchemas(
|
||||
headers: { Accept: 'application/json' },
|
||||
json: true,
|
||||
},
|
||||
)) as { schemas?: Array<{ name: string }> };
|
||||
);
|
||||
|
||||
const schemas = schemasResponse.schemas ?? [];
|
||||
|
||||
@@ -163,8 +198,11 @@ export async function getSchemas(
|
||||
|
||||
return { results: allSchemas };
|
||||
} catch (e) {
|
||||
const message = sanitizeApiMessage(e instanceof Error ? e.message : String(e));
|
||||
return {
|
||||
results: [{ name: `Error loading schemas for catalog: ${selectedCatalog}`, value: '' }],
|
||||
results: [
|
||||
{ name: `Error loading schemas for catalog ${selectedCatalog}: ${message}`, value: '' },
|
||||
],
|
||||
};
|
||||
}
|
||||
}
|
||||
@@ -178,16 +216,12 @@ async function fetchResourcesInSchema<T extends { name: string }>(
|
||||
schemaName: string,
|
||||
responseKey: string,
|
||||
): Promise<T[]> {
|
||||
const response = (await context.helpers.httpRequestWithAuthentication.call(
|
||||
context,
|
||||
credentialType,
|
||||
{
|
||||
method: 'GET',
|
||||
url: `${host}${apiPath}?catalog_name=${catalogName}&schema_name=${schemaName}`,
|
||||
headers: { Accept: 'application/json' },
|
||||
json: true,
|
||||
},
|
||||
)) as Record<string, T[] | undefined>;
|
||||
const response = await listRequest<Record<string, T[] | undefined>>(context, credentialType, {
|
||||
method: 'GET',
|
||||
url: `${host}${apiPath}?catalog_name=${catalogName}&schema_name=${schemaName}`,
|
||||
headers: { Accept: 'application/json' },
|
||||
json: true,
|
||||
});
|
||||
return response[responseKey] ?? [];
|
||||
}
|
||||
|
||||
@@ -259,9 +293,13 @@ export async function getVolumes(
|
||||
|
||||
return { results: allResults };
|
||||
} catch (e) {
|
||||
const message = sanitizeApiMessage(e instanceof Error ? e.message : String(e));
|
||||
return {
|
||||
results: [
|
||||
{ name: `Error loading volumes for ${selectedCatalog}.${selectedSchema}`, value: '' },
|
||||
{
|
||||
name: `Error loading volumes for ${selectedCatalog}.${selectedSchema}: ${message}`,
|
||||
value: '',
|
||||
},
|
||||
],
|
||||
};
|
||||
}
|
||||
@@ -316,7 +354,7 @@ export async function getTables(
|
||||
|
||||
return { results: allResults };
|
||||
} catch (e) {
|
||||
const message = e instanceof Error ? e.message : String(e);
|
||||
const message = sanitizeApiMessage(e instanceof Error ? e.message : String(e));
|
||||
return {
|
||||
results: [
|
||||
{
|
||||
@@ -377,9 +415,13 @@ export async function getFunctions(
|
||||
|
||||
return { results: allResults };
|
||||
} catch (e) {
|
||||
const message = sanitizeApiMessage(e instanceof Error ? e.message : String(e));
|
||||
return {
|
||||
results: [
|
||||
{ name: `Error loading functions for ${selectedCatalog}.${selectedSchema}`, value: '' },
|
||||
{
|
||||
name: `Error loading functions for ${selectedCatalog}.${selectedSchema}: ${message}`,
|
||||
value: '',
|
||||
},
|
||||
],
|
||||
};
|
||||
}
|
||||
|
||||
@@ -1,5 +1,18 @@
|
||||
import { NodeTestHarness } from '@nodes-testing/node-test-harness';
|
||||
import { NodeApiError } from 'n8n-workflow';
|
||||
import type {
|
||||
IExecuteFunctions,
|
||||
ILoadOptionsFunctions,
|
||||
INode,
|
||||
JsonObject,
|
||||
WorkflowTestData,
|
||||
} from 'n8n-workflow';
|
||||
import nock from 'nock';
|
||||
import { mockDeep } from 'vitest-mock-extended';
|
||||
|
||||
import { execute as executeQuery } from '../actions/databricksSql/executeQuery.operation';
|
||||
import { makePermissionErrorLegible } from '../actions/helpers';
|
||||
import { getCatalogs, getSchemas } from '../methods/listSearch';
|
||||
|
||||
// Mock sleep from @n8n/utils so polling tests run without real delays
|
||||
vi.mock('@n8n/utils/sleep', () => ({
|
||||
@@ -8,6 +21,37 @@ vi.mock('@n8n/utils/sleep', () => ({
|
||||
|
||||
const HOST = 'https://adb-1234567890.1.azuredatabricks.net';
|
||||
|
||||
const PERMISSION_MESSAGE = "User does not have USE CATALOG on Catalog 'main'.";
|
||||
|
||||
const node: INode = {
|
||||
id: '1',
|
||||
name: 'Databricks',
|
||||
type: 'n8n-nodes-base.databricks',
|
||||
typeVersion: 1,
|
||||
position: [0, 0],
|
||||
parameters: {},
|
||||
};
|
||||
|
||||
// Mirrors what core's httpRequest throws and authentication.ts wraps: an axios
|
||||
// error with the response body under `response.data`
|
||||
class AxiosError extends Error {
|
||||
constructor(
|
||||
message: string,
|
||||
readonly response: { status: number; data: unknown },
|
||||
) {
|
||||
super(message);
|
||||
}
|
||||
}
|
||||
|
||||
const apiErrorFromBody = (status: number, data: unknown) =>
|
||||
new NodeApiError(
|
||||
node,
|
||||
new AxiosError(`Request failed with status code ${status}`, {
|
||||
status,
|
||||
data,
|
||||
}) as unknown as JsonObject,
|
||||
);
|
||||
|
||||
describe('Databricks', () => {
|
||||
const credentials = {
|
||||
databricksApi: {
|
||||
@@ -592,4 +636,193 @@ describe('Databricks', () => {
|
||||
workflowFiles: ['vector-search.workflow.json'],
|
||||
});
|
||||
});
|
||||
|
||||
describe('Router -> PERMISSION_DENIED surfaces the Databricks message', () => {
|
||||
// A 403 PERMISSION_DENIED body must surface its legible Databricks message
|
||||
// instead of the generic "Forbidden - perhaps check your credentials?" —
|
||||
// deleting the makePermissionErrorLegible call in the router must fail this
|
||||
beforeAll(() => {
|
||||
nock(HOST)
|
||||
.get('/api/2.1/unity-catalog/catalogs')
|
||||
.reply(403, { error_code: 'PERMISSION_DENIED', message: PERMISSION_MESSAGE });
|
||||
});
|
||||
|
||||
afterAll(() => nock.cleanAll());
|
||||
|
||||
const harness = new NodeTestHarness();
|
||||
const testData: WorkflowTestData = {
|
||||
description: 'permission-denied.workflow',
|
||||
input: { workflowData: harness.readWorkflowJSON('permission-denied.workflow.json') },
|
||||
output: { nodeData: {}, error: PERMISSION_MESSAGE },
|
||||
credentials,
|
||||
};
|
||||
harness.setupTest(testData, { credentials });
|
||||
});
|
||||
});
|
||||
|
||||
describe('makePermissionErrorLegible', () => {
|
||||
it('should promote the PERMISSION_DENIED body message and add a remediation description', () => {
|
||||
const error = apiErrorFromBody(403, {
|
||||
error_code: 'PERMISSION_DENIED',
|
||||
message: PERMISSION_MESSAGE,
|
||||
});
|
||||
// Without the helper the user sees a generic status-code message instead
|
||||
expect(error.message).not.toBe(PERMISSION_MESSAGE);
|
||||
|
||||
makePermissionErrorLegible(error);
|
||||
|
||||
expect(error.message).toBe(PERMISSION_MESSAGE);
|
||||
expect(error.description).toBe(
|
||||
'Grant the named permission to the signed-in user or service principal in Databricks, then retry.',
|
||||
);
|
||||
});
|
||||
|
||||
it('should strip control characters and truncate the promoted message to 500 characters', () => {
|
||||
const error = apiErrorFromBody(403, {
|
||||
error_code: 'PERMISSION_DENIED',
|
||||
message: `bad\x00\x1f\x7fmessage${'x'.repeat(600)}`,
|
||||
});
|
||||
|
||||
makePermissionErrorLegible(error);
|
||||
|
||||
expect(error.message).toBe(`bad message${'x'.repeat(600)}`.slice(0, 500));
|
||||
});
|
||||
|
||||
// encoding: 'arraybuffer' requests (file downloads) get their 403 JSON body
|
||||
// as raw bytes — the helper must parse it before reading error_code
|
||||
it('should parse a Buffer body from arraybuffer requests', () => {
|
||||
const error = apiErrorFromBody(
|
||||
403,
|
||||
Buffer.from(JSON.stringify({ error_code: 'PERMISSION_DENIED', message: PERMISSION_MESSAGE })),
|
||||
);
|
||||
|
||||
makePermissionErrorLegible(error);
|
||||
|
||||
expect(error.message).toBe(PERMISSION_MESSAGE);
|
||||
});
|
||||
|
||||
it('should leave errors with a non-JSON Buffer body untouched', () => {
|
||||
const error = apiErrorFromBody(403, Buffer.from('not json'));
|
||||
const messageBefore = error.message;
|
||||
|
||||
makePermissionErrorLegible(error);
|
||||
|
||||
expect(error.message).toBe(messageBefore);
|
||||
});
|
||||
|
||||
it('should leave non-PERMISSION_DENIED errors untouched', () => {
|
||||
const error = apiErrorFromBody(403, {
|
||||
error_code: 'IP_ACCESS_DENIED',
|
||||
message: 'Source IP is blocked',
|
||||
});
|
||||
const messageBefore = error.message;
|
||||
|
||||
makePermissionErrorLegible(error);
|
||||
|
||||
expect(error.message).toBe(messageBefore);
|
||||
});
|
||||
|
||||
it.each([
|
||||
['missing', { error_code: 'PERMISSION_DENIED' }],
|
||||
['non-string', { error_code: 'PERMISSION_DENIED', message: 123 }],
|
||||
])('should leave PERMISSION_DENIED errors with a %s body message untouched', (_case, body) => {
|
||||
const error = apiErrorFromBody(403, body);
|
||||
const messageBefore = error.message;
|
||||
|
||||
makePermissionErrorLegible(error);
|
||||
|
||||
expect(error.message).toBe(messageBefore);
|
||||
});
|
||||
});
|
||||
|
||||
describe('listSearch -> PERMISSION_DENIED surfaces the Databricks message', () => {
|
||||
// One dropdown suffices: all five listSearch call sites share the listRequest
|
||||
// wrapper this exercises
|
||||
it('should reject with the legible message from getCatalogs', async () => {
|
||||
const context = mockDeep<ILoadOptionsFunctions>();
|
||||
context.getNodeParameter.mockReturnValue('accessToken');
|
||||
context.getCredentials.mockResolvedValue({ host: HOST });
|
||||
context.helpers.httpRequestWithAuthentication.mockRejectedValue(
|
||||
apiErrorFromBody(403, { error_code: 'PERMISSION_DENIED', message: PERMISSION_MESSAGE }),
|
||||
);
|
||||
|
||||
await expect(getCatalogs.call(context)).rejects.toMatchObject({
|
||||
message: PERMISSION_MESSAGE,
|
||||
});
|
||||
});
|
||||
|
||||
// getSchemas swallows the error into a placeholder row instead of throwing —
|
||||
// the legible message must still be wired into that row
|
||||
it('should append the legible message to the placeholder row from getSchemas', async () => {
|
||||
const context = mockDeep<ILoadOptionsFunctions>();
|
||||
context.getNodeParameter.mockReturnValue('accessToken');
|
||||
context.getCredentials.mockResolvedValue({ host: HOST });
|
||||
context.getCurrentNodeParameter.mockReturnValue('main');
|
||||
context.helpers.httpRequestWithAuthentication.mockRejectedValue(
|
||||
apiErrorFromBody(403, { error_code: 'PERMISSION_DENIED', message: PERMISSION_MESSAGE }),
|
||||
);
|
||||
|
||||
const { results } = await getSchemas.call(context);
|
||||
|
||||
expect(results[0].name).toContain(PERMISSION_MESSAGE);
|
||||
});
|
||||
});
|
||||
|
||||
describe('Databricks SQL -> Execute Query (FAILED/CANCELED statement)', () => {
|
||||
const setupContext = (status: unknown) => {
|
||||
const context = mockDeep<IExecuteFunctions>();
|
||||
context.getNode.mockReturnValue(node);
|
||||
context.getNodeParameter.mockImplementation((name) => {
|
||||
if (name === 'warehouseId') return 'warehouse123';
|
||||
if (name === 'query') return 'SELECT * FROM x';
|
||||
return [];
|
||||
});
|
||||
context.getCredentials.mockResolvedValue({ host: HOST });
|
||||
context.helpers.httpRequestWithAuthentication.mockResolvedValue({
|
||||
statement_id: 'stmt-403',
|
||||
status,
|
||||
});
|
||||
return context;
|
||||
};
|
||||
|
||||
it.each(['FAILED', 'CANCELED'])(
|
||||
'should surface the in-band error message of a %s statement',
|
||||
async (state) => {
|
||||
// SQL permission failures arrive on an HTTP 200 with the legible text in
|
||||
// status.error.message — the raw JSON blob must not be the whole story
|
||||
const context = setupContext({
|
||||
state,
|
||||
error: {
|
||||
error_code: 'PERMISSION_DENIED',
|
||||
message: "User does not have SELECT on Table 'x'.",
|
||||
},
|
||||
});
|
||||
|
||||
await expect(executeQuery.call(context, 0)).rejects.toThrow(
|
||||
`Query ${state.toLowerCase()}: User does not have SELECT on Table 'x'.`,
|
||||
);
|
||||
},
|
||||
);
|
||||
|
||||
it('should strip control characters and truncate the in-band error message', async () => {
|
||||
const context = setupContext({
|
||||
state: 'FAILED',
|
||||
error: {
|
||||
error_code: 'PERMISSION_DENIED',
|
||||
message: `bad\x00\x1f\x7fmessage${'x'.repeat(600)}`,
|
||||
},
|
||||
});
|
||||
|
||||
await expect(executeQuery.call(context, 0)).rejects.toThrow(
|
||||
`Query failed: ${`bad message${'x'.repeat(600)}`.slice(0, 500)}`,
|
||||
);
|
||||
});
|
||||
|
||||
it('should fall back to the stringified status when no error message is present', async () => {
|
||||
const context = setupContext({ state: 'FAILED' });
|
||||
|
||||
await expect(executeQuery.call(context, 0)).rejects.toThrow(
|
||||
`Query failed: ${JSON.stringify({ state: 'FAILED' })}`,
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -0,0 +1,47 @@
|
||||
{
|
||||
"name": "Databricks Permission Denied Test Workflow",
|
||||
"nodes": [
|
||||
{
|
||||
"parameters": {},
|
||||
"type": "n8n-nodes-base.manualTrigger",
|
||||
"typeVersion": 1,
|
||||
"position": [0, 0],
|
||||
"id": "trigger-001",
|
||||
"name": "When clicking 'Execute workflow'"
|
||||
},
|
||||
{
|
||||
"parameters": {
|
||||
"resource": "unityCatalog",
|
||||
"operation": "listCatalogs"
|
||||
},
|
||||
"type": "n8n-nodes-base.databricks",
|
||||
"typeVersion": 1,
|
||||
"position": [220, 0],
|
||||
"id": "catalog-list-403",
|
||||
"name": "List Catalogs",
|
||||
"credentials": {
|
||||
"databricksApi": {
|
||||
"id": "cred-001",
|
||||
"name": "Databricks account"
|
||||
}
|
||||
}
|
||||
}
|
||||
],
|
||||
"connections": {
|
||||
"When clicking 'Execute workflow'": {
|
||||
"main": [
|
||||
[
|
||||
{
|
||||
"node": "List Catalogs",
|
||||
"type": "main",
|
||||
"index": 0
|
||||
}
|
||||
]
|
||||
]
|
||||
}
|
||||
},
|
||||
"active": false,
|
||||
"settings": {
|
||||
"executionOrder": "v1"
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user