Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Provide scopes for custom node operations #3164

Merged
merged 9 commits into from
Apr 28, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion packages/cli/src/CredentialTypes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import {
ICredentialTypeData,
ICredentialTypes as ICredentialTypesInterface,
} from 'n8n-workflow';
import { RESPONSE_ERROR_MESSAGES } from './constants';

class CredentialTypesClass implements ICredentialTypesInterface {
credentialTypes: ICredentialTypeData = {};
Expand All @@ -16,7 +17,11 @@ class CredentialTypesClass implements ICredentialTypesInterface {
}

getByName(credentialType: string): ICredentialType {
return this.credentialTypes[credentialType].type;
try {
return this.credentialTypes[credentialType].type;
} catch (error) {
throw new Error(`${RESPONSE_ERROR_MESSAGES.NO_CREDENTIAL}: ${credentialType}`);
}
}
}

Expand Down
27 changes: 27 additions & 0 deletions packages/cli/src/CredentialsHelper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -286,6 +286,33 @@ export class CredentialsHelper extends ICredentialsHelper {
return combineProperties;
}

/**
* Returns the scope of a credential type
*
* @param {string} type
* @returns {string[]}
* @memberof CredentialsHelper
*/
getScopes(type: string): string[] {
const scopeProperty = this.getCredentialsProperties(type).find(({ name }) => name === 'scope');

if (!scopeProperty?.default || typeof scopeProperty.default !== 'string') {
const errorMessage = `No \`scope\` property found for credential type: ${type}`;

Logger.error(errorMessage);

throw new Error(errorMessage);
}

const { default: scopeDefault } = scopeProperty;

if (/ /.test(scopeDefault)) return scopeDefault.split(' ');

if (/,/.test(scopeDefault)) return scopeDefault.split(',');

return [scopeDefault];
}

/**
* Returns the decrypted credential data with applied overwrites
*
Expand Down
1 change: 1 addition & 0 deletions packages/cli/src/LoadNodesAndCredentials.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ class LoadNodesAndCredentialsClass {
// In case "n8n" package is the root and the packages are
// in the "node_modules" folder underneath it.
path.join(__dirname, '..', '..', 'node_modules', 'n8n-workflow'),
path.join(__dirname, '..', 'node_modules', 'n8n-workflow'), // for test run
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not ideal, but larger refactoring would seem out of scope.

];
for (const checkPath of checkPaths) {
try {
Expand Down
3 changes: 3 additions & 0 deletions packages/cli/src/Server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,7 @@ import { ExecutionEntity } from './databases/entities/ExecutionEntity';
import { SharedWorkflow } from './databases/entities/SharedWorkflow';
import { AUTH_COOKIE_NAME, RESPONSE_ERROR_MESSAGES } from './constants';
import { credentialsController } from './api/credentials.api';
import { oauth2CredentialController } from './api/oauth2Credential.api';
import { getInstanceBaseUrl, isEmailSetUp } from './UserManagement/UserManagementHelper';

require('body-parser-xml')(bodyParser);
Expand Down Expand Up @@ -1933,6 +1934,8 @@ class App {
// OAuth2-Credential/Auth
// ----------------------------------------

this.app.use(`/${this.restEndpoint}/oauth2-credential`, oauth2CredentialController);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In future we can move /oauth2-credential/auth and /oauth2-credential/callback to the new controller.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should 😊 can you create a Tech Debt ticket for it?


// Authorize OAuth Data
this.app.get(
`/${this.restEndpoint}/oauth2-credential/auth`,
Expand Down
61 changes: 61 additions & 0 deletions packages/cli/src/api/oauth2Credential.api.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
/* eslint-disable import/no-cycle */
import express from 'express';
import { UserSettings } from 'n8n-core';
import { LoggerProxy } from 'n8n-workflow';
import { ResponseHelper } from '..';
import { RESPONSE_ERROR_MESSAGES } from '../constants';
import { CredentialsHelper } from '../CredentialsHelper';
import { getLogger } from '../Logger';
import { OAuthRequest } from '../requests';

export const oauth2CredentialController = express.Router();

/**
* Initialize Logger if needed
*/
oauth2CredentialController.use((req, res, next) => {
try {
LoggerProxy.getInstance();
} catch (error) {
LoggerProxy.init(getLogger());
}
next();
});

/**
* GET /oauth2-credential/scopes
*/
oauth2CredentialController.get(
'/scopes',
ResponseHelper.send(async (req: OAuthRequest.OAuth2Credential.Scopes): Promise<string[]> => {
const { credentialType: type } = req.query;

if (!type) {
LoggerProxy.debug(
'Request for OAuth2 credential scopes failed because of missing credential type in query string',
);

throw new ResponseHelper.ResponseError(
RESPONSE_ERROR_MESSAGES.NO_CREDENTIAL_TYPE,
undefined,
400,
);
}

if (!type.endsWith('OAuth2Api')) {
LoggerProxy.debug(
'Request for OAuth2 credential scopes failed because requested credential type is not OAuth2',
);

throw new ResponseHelper.ResponseError(
RESPONSE_ERROR_MESSAGES.CREDENTIAL_TYPE_NOT_OAUTH2,
undefined,
400,
);
}

const encryptionKey = await UserSettings.getEncryptionKey();

return new CredentialsHelper(encryptionKey).getScopes(type);
}),
);
2 changes: 2 additions & 0 deletions packages/cli/src/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ import { RESPONSE_ERROR_MESSAGES as CORE_RESPONSE_ERROR_MESSAGES } from 'n8n-cor
export const RESPONSE_ERROR_MESSAGES = {
NO_CREDENTIAL: 'Credential not found',
NO_ENCRYPTION_KEY: CORE_RESPONSE_ERROR_MESSAGES.NO_ENCRYPTION_KEY,
NO_CREDENTIAL_TYPE: 'Missing query string param: `credentialType`',
CREDENTIAL_TYPE_NOT_OAUTH2: 'Credential type is not OAuth2 - no scopes can be provided',
};

export const AUTH_COOKIE_NAME = 'n8n-auth';
5 changes: 2 additions & 3 deletions packages/cli/src/requests.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -244,9 +244,8 @@ export declare namespace OAuthRequest {

namespace OAuth2Credential {
type Auth = OAuth1Credential.Auth;
type Callback = AuthenticatedRequest<{}, {}, {}, { code: string; state: string }> & {
user?: User;
};
type Callback = AuthenticatedRequest<{}, {}, {}, { code: string; state: string }>;
type Scopes = AuthenticatedRequest<{}, {}, {}, { credentialType: string }>;
}
}

Expand Down
125 changes: 125 additions & 0 deletions packages/cli/test/integration/oauth2.api.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,125 @@
import express from 'express';

import * as utils from './shared/utils';
import * as testDb from './shared/testDb';
import { RESPONSE_ERROR_MESSAGES } from '../../src/constants';

import type { Role } from '../../src/databases/entities/Role';
import { CredentialTypes, LoadNodesAndCredentials } from '../../src';

const SCOPES_ENDPOINT = '/oauth2-credential/scopes';

let app: express.Application;
let testDbName = '';
let globalOwnerRole: Role;

beforeAll(async () => {
app = utils.initTestServer({ endpointGroups: ['oauth2-credential'], applyAuth: true });
const initResult = await testDb.init();
testDbName = initResult.testDbName;

utils.initConfigFile();

globalOwnerRole = await testDb.getGlobalOwnerRole();
utils.initTestLogger();

const loadNodesAndCredentials = LoadNodesAndCredentials();
await loadNodesAndCredentials.init();

const credentialTypes = CredentialTypes();
await credentialTypes.init(loadNodesAndCredentials.credentialTypes);
});

afterAll(async () => {
await testDb.terminate(testDbName);
});

describe('OAuth2 scopes', () => {
beforeEach(async () => {
await testDb.truncate(['User'], testDbName);
});

test(`GET ${SCOPES_ENDPOINT} should return scopes - comma-delimited`, async () => {
const ownerShell = await testDb.createUserShell(globalOwnerRole);
const authOwnerShellAgent = utils.createAgent(app, { auth: true, user: ownerShell });

const response = await authOwnerShellAgent
.get(SCOPES_ENDPOINT)
.query({ credentialType: 'twistOAuth2Api' });

expect(response.statusCode).toBe(200);

const scopes = response.body.data;
const TWIST_OAUTH2_API_SCOPES_TOTAL = 6;

expect(Array.isArray(scopes)).toBe(true);
expect(scopes.length).toBe(TWIST_OAUTH2_API_SCOPES_TOTAL);
});

test(`GET ${SCOPES_ENDPOINT} should return scopes - whitespace-delimited`, async () => {
const ownerShell = await testDb.createUserShell(globalOwnerRole);
const authOwnerShellAgent = utils.createAgent(app, { auth: true, user: ownerShell });

const response = await authOwnerShellAgent
.get(SCOPES_ENDPOINT)
.query({ credentialType: 'dropboxOAuth2Api' });

expect(response.statusCode).toBe(200);

const scopes = response.body.data;
const DROPBOX_OAUTH2_API_SCOPES_TOTAL = 4;

expect(Array.isArray(scopes)).toBe(true);
expect(scopes.length).toBe(DROPBOX_OAUTH2_API_SCOPES_TOTAL);
});

test(`GET ${SCOPES_ENDPOINT} should return scope - non-delimited`, async () => {
const ownerShell = await testDb.createUserShell(globalOwnerRole);
const authOwnerShellAgent = utils.createAgent(app, { auth: true, user: ownerShell });

const response = await authOwnerShellAgent
.get(SCOPES_ENDPOINT)
.query({ credentialType: 'harvestOAuth2Api' });

expect(response.statusCode).toBe(200);

const scopes = response.body.data;

expect(Array.isArray(scopes)).toBe(true);
expect(scopes.length).toBe(1);
});

test(`GET ${SCOPES_ENDPOINT} should fail with missing credential type`, async () => {
const ownerShell = await testDb.createUserShell(globalOwnerRole);
const authOwnerShellAgent = utils.createAgent(app, { auth: true, user: ownerShell });

const response = await authOwnerShellAgent.get(SCOPES_ENDPOINT);

expect(response.statusCode).toBe(400);
expect(response.body.message).toBe(RESPONSE_ERROR_MESSAGES.NO_CREDENTIAL_TYPE);
});

test(`GET ${SCOPES_ENDPOINT} should fail with non-OAuth2 credential type`, async () => {
const ownerShell = await testDb.createUserShell(globalOwnerRole);
const authOwnerShellAgent = utils.createAgent(app, { auth: true, user: ownerShell });

const response = await authOwnerShellAgent
.get(SCOPES_ENDPOINT)
.query({ credentialType: 'disqusApi' });

expect(response.statusCode).toBe(400);
expect(response.body.message).toBe(RESPONSE_ERROR_MESSAGES.CREDENTIAL_TYPE_NOT_OAUTH2);
});

test(`GET ${SCOPES_ENDPOINT} should fail with wrong OAuth2 credential type`, async () => {
const ownerShell = await testDb.createUserShell(globalOwnerRole);
const authOwnerShellAgent = utils.createAgent(app, { auth: true, user: ownerShell });

const response = await authOwnerShellAgent
.get(SCOPES_ENDPOINT)
.query({ credentialType: 'wrongOAuth2Api' });

expect(response.statusCode).toBe(500);
expect(response.body.message).toBe(`${RESPONSE_ERROR_MESSAGES.NO_CREDENTIAL}: wrongOAuth2Api`);
});
});
2 changes: 1 addition & 1 deletion packages/cli/test/integration/shared/types.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ export type SmtpTestAccount = {
};
};

type EndpointGroup = 'me' | 'users' | 'auth' | 'owner' | 'passwordReset' | 'credentials';
type EndpointGroup = 'me' | 'users' | 'auth' | 'owner' | 'passwordReset' | 'credentials' | 'oauth2-credential';

export type CredentialPayload = {
name: string;
Expand Down
7 changes: 6 additions & 1 deletion packages/cli/test/integration/shared/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import { credentialsController } from '../../../src/api/credentials.api';
import type { User } from '../../../src/databases/entities/User';
import type { EndpointGroup, SmtpTestAccount } from './types';
import type { N8nApp } from '../../../src/UserManagement/Interfaces';
import { oauth2CredentialController } from '../../../src/api/oauth2Credential.api';

/**
* Initialize a test server.
Expand Down Expand Up @@ -63,6 +64,7 @@ export function initTestServer({
if (routerEndpoints.length) {
const map: Record<string, express.Router> = {
credentials: credentialsController,
'oauth2-credential': oauth2CredentialController,
};

for (const group of routerEndpoints) {
Expand Down Expand Up @@ -105,7 +107,10 @@ const classifyEndpointGroups = (endpointGroups: string[]) => {
const functionEndpoints: string[] = [];

endpointGroups.forEach((group) =>
(group === 'credentials' ? routerEndpoints : functionEndpoints).push(group),
(['credentials', 'oauth2-credential'].includes(group)
? routerEndpoints
: functionEndpoints
).push(group),
);

return [routerEndpoints, functionEndpoints];
Expand Down