Skip to content

Commit

Permalink
Merge branch 'master' into ask-assistant
Browse files Browse the repository at this point in the history
* master:
  fix(core): Prevent XSS via static cache dir (#10339)
  fix(editor): Enable credential sharing between all types of projects (#10233)
  refactor(core): Extract webhook request handler to own file (#10301)
  feat: Allow sharing to and from team projects (no-changelog) (#10144)
  refactor(editor): Convert ChangePasswordModal to composition api (no-changelog) (#10337)
  docs: Change display name for WhatsApp Trigger API Credential (#10334)
  fix(core): Do not load ScalingService in regular mode (no-changelog) (#10333)
  docs: Update wording in X credentials (#10327)
  fix(editor): Fixing XSS vulnerability in toast messages (#10329)
  fix(core): Rate limit MFA activation and verification endpoints (#10330)
  refactor(core): Decouple emailing and workflow sharing from internal hooks (no-changelog) (#10326)
  refactor(core): Stop reporting disk I/O error to Sentry (no-changelog) (#10324)
  • Loading branch information
MiloradFilipovic committed Aug 9, 2024
2 parents 451f983 + 4f392b5 commit badcb54
Show file tree
Hide file tree
Showing 56 changed files with 964 additions and 578 deletions.
20 changes: 8 additions & 12 deletions cypress/composables/projects.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@ const credentialsModal = new CredentialsModal();

export const getHomeButton = () => cy.getByTestId('project-home-menu-item');
export const getMenuItems = () => cy.getByTestId('project-menu-item');
export const getAddProjectButton = () => cy.getByTestId('add-project-menu-item');
export const getAddProjectButton = () =>
cy.getByTestId('add-project-menu-item').should('contain', 'Add project').should('be.visible');
export const getProjectTabs = () => cy.getByTestId('project-tabs').find('a');
export const getProjectTabWorkflows = () => getProjectTabs().filter('a[href$="/workflows"]');
export const getProjectTabCredentials = () => getProjectTabs().filter('a[href$="/credentials"]');
Expand All @@ -28,7 +29,7 @@ export const getResourceMoveConfirmModal = () =>
export const getProjectMoveSelect = () => cy.getByTestId('project-move-resource-modal-select');

export function createProject(name: string) {
getAddProjectButton().should('be.visible').click();
getAddProjectButton().click();

getProjectNameInput()
.should('be.visible')
Expand All @@ -46,21 +47,16 @@ export function createWorkflow(fixtureKey: string, name: string) {
workflowPage.actions.zoomToFit();
}

export function createCredential(name: string) {
export function createCredential(name: string, closeModal = true) {
credentialsModal.getters.newCredentialModal().should('be.visible');
credentialsModal.getters.newCredentialTypeSelect().should('be.visible');
credentialsModal.getters.newCredentialTypeOption('Notion API').click();
credentialsModal.getters.newCredentialTypeButton().click();
credentialsModal.getters.connectionParameter('Internal Integration Secret').type('1234567890');
credentialsModal.actions.setName(name);
credentialsModal.actions.save();
credentialsModal.actions.close();
}

export const actions = {
createProject: (name: string) => {
getAddProjectButton().click();
getProjectSettingsNameInput().type(name);
getProjectSettingsSaveButton().click();
},
};
if (closeModal) {
credentialsModal.actions.close();
}
}
73 changes: 70 additions & 3 deletions cypress/e2e/17-sharing.cy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import {
WorkflowSharingModal,
WorkflowsPage,
} from '../pages';
import { getVisibleSelect } from '../utils';
import { getVisibleDropdown, getVisibleSelect } from '../utils';
import * as projects from '../composables/projects';

/**
Expand Down Expand Up @@ -192,6 +192,73 @@ describe('Sharing', { disableAutoLogin: true }, () => {
credentialsModal.actions.saveSharing();
credentialsModal.actions.close();
});

it('credentials should work between team and personal projects', () => {
cy.resetDatabase();
cy.enableFeature('sharing');
cy.enableFeature('advancedPermissions');
cy.enableFeature('projectRole:admin');
cy.enableFeature('projectRole:editor');
cy.changeQuota('maxTeamProjects', -1);

cy.signinAsOwner();
cy.visit('/');

projects.createProject('Development');

projects.getHomeButton().click();
workflowsPage.getters.newWorkflowButtonCard().click();
projects.createWorkflow('Test_workflow_1.json', 'Test workflow');

projects.getHomeButton().click();
projects.getProjectTabCredentials().click();
credentialsPage.getters.emptyListCreateCredentialButton().click();
projects.createCredential('Notion API');

credentialsPage.getters.credentialCard('Notion API').click();
credentialsModal.actions.changeTab('Sharing');
credentialsModal.getters.usersSelect().click();
getVisibleSelect()
.find('li')
.should('have.length', 4)
.filter(':contains("Development")')
.should('have.length', 1)
.click();
credentialsModal.getters.saveButton().click();
credentialsModal.actions.close();

projects.getProjectTabWorkflows().click();
workflowsPage.getters.workflowCardActions('Test workflow').click();
getVisibleDropdown().find('li').contains('Share').click();

workflowSharingModal.getters.usersSelect().filter(':visible').click();
getVisibleSelect().find('li').should('have.length', 3).first().click();
workflowSharingModal.getters.saveButton().click();

projects.getMenuItems().first().click();
workflowsPage.getters.newWorkflowButtonCard().click();
projects.createWorkflow('Test_workflow_1.json', 'Test workflow 2');
workflowPage.actions.openShareModal();
workflowSharingModal.getters.usersSelect().should('not.exist');

cy.get('body').type('{esc}');

projects.getMenuItems().first().click();
projects.getProjectTabCredentials().click();
credentialsPage.getters.createCredentialButton().click();
projects.createCredential('Notion API 2', false);
credentialsModal.actions.changeTab('Sharing');
credentialsModal.getters.usersSelect().click();
getVisibleSelect().find('li').should('have.length', 4).first().click();
credentialsModal.getters.saveButton().click();
credentialsModal.actions.close();

credentialsPage.getters
.credentialCards()
.should('have.length', 2)
.filter(':contains("Owned by me")')
.should('have.length', 1);
});
});

describe('Credential Usage in Cross Shared Workflows', () => {
Expand All @@ -217,13 +284,13 @@ describe('Credential Usage in Cross Shared Workflows', () => {
credentialsModal.actions.createNewCredential('Notion API');

// Create a notion credential in one project
projects.actions.createProject('Development');
projects.createProject('Development');
projects.getProjectTabCredentials().click();
credentialsPage.getters.emptyListCreateCredentialButton().click();
credentialsModal.actions.createNewCredential('Notion API');

// Create a notion credential in another project
projects.actions.createProject('Test');
projects.createProject('Test');
projects.getProjectTabCredentials().click();
credentialsPage.getters.emptyListCreateCredentialButton().click();
credentialsModal.actions.createNewCredential('Notion API');
Expand Down
2 changes: 1 addition & 1 deletion cypress/e2e/39-projects.cy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,7 @@ describe('Projects', { disableAutoLogin: true }, () => {
cy.signinAsMember(1);
cy.visit(workflowsPage.url);

projects.getAddProjectButton().should('not.exist');
cy.getByTestId('add-project-menu-item').should('not.exist');
projects.getMenuItems().should('not.exist');
});

Expand Down
10 changes: 10 additions & 0 deletions packages/cli/BREAKING-CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,16 @@

This list shows all the versions which include breaking changes and how to upgrade.

## 1.55.0

### What changed?

The `N8N_BLOCK_FILE_ACCESS_TO_N8N_FILES` environment variable now also blocks access to n8n's static cache directory at `~/.cache/n8n/public`.

### When is action necessary?

If you are writing to or reading from a file at n8n's static cache directory via a node, e.g. `Read/Write Files from Disk`, please update your node to use a different path.

## 1.52.0

### What changed?
Expand Down
19 changes: 9 additions & 10 deletions packages/cli/src/AbstractServer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import { rawBodyReader, bodyParser, corsMiddleware } from '@/middlewares';
import { WaitingForms } from '@/WaitingForms';
import { TestWebhooks } from '@/webhooks/TestWebhooks';
import { WaitingWebhooks } from '@/webhooks/WaitingWebhooks';
import { webhookRequestHandler } from '@/webhooks/WebhookHelpers';
import { createWebhookHandlerFor } from '@/webhooks/WebhookRequestHandler';
import { ActiveWebhooks } from '@/webhooks/ActiveWebhooks';
import { generateHostInstanceId } from './databases/utils/generators';
import { Logger } from '@/Logger';
Expand Down Expand Up @@ -185,33 +185,32 @@ export abstract class AbstractServer {

// Setup webhook handlers before bodyParser, to let the Webhook node handle binary data in requests
if (this.webhooksEnabled) {
const activeWebhooks = Container.get(ActiveWebhooks);

const activeWebhooksRequestHandler = createWebhookHandlerFor(Container.get(ActiveWebhooks));
// Register a handler for active forms
this.app.all(`/${this.endpointForm}/:path(*)`, webhookRequestHandler(activeWebhooks));
this.app.all(`/${this.endpointForm}/:path(*)`, activeWebhooksRequestHandler);

// Register a handler for active webhooks
this.app.all(`/${this.endpointWebhook}/:path(*)`, webhookRequestHandler(activeWebhooks));
this.app.all(`/${this.endpointWebhook}/:path(*)`, activeWebhooksRequestHandler);

// Register a handler for waiting forms
this.app.all(
`/${this.endpointFormWaiting}/:path/:suffix?`,
webhookRequestHandler(Container.get(WaitingForms)),
createWebhookHandlerFor(Container.get(WaitingForms)),
);

// Register a handler for waiting webhooks
this.app.all(
`/${this.endpointWebhookWaiting}/:path/:suffix?`,
webhookRequestHandler(Container.get(WaitingWebhooks)),
createWebhookHandlerFor(Container.get(WaitingWebhooks)),
);
}

if (this.testWebhooksEnabled) {
const testWebhooks = Container.get(TestWebhooks);
const testWebhooksRequestHandler = createWebhookHandlerFor(Container.get(TestWebhooks));

// Register a handler
this.app.all(`/${this.endpointFormTest}/:path(*)`, webhookRequestHandler(testWebhooks));
this.app.all(`/${this.endpointWebhookTest}/:path(*)`, webhookRequestHandler(testWebhooks));
this.app.all(`/${this.endpointFormTest}/:path(*)`, testWebhooksRequestHandler);
this.app.all(`/${this.endpointWebhookTest}/:path(*)`, testWebhooksRequestHandler);
}

// Block bots from scanning the application
Expand Down
2 changes: 1 addition & 1 deletion packages/cli/src/ErrorReporting.ts
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ export const initErrorHandling = async () => {

if (
originalException instanceof QueryFailedError &&
originalException.message.includes('SQLITE_FULL')
['SQLITE_FULL', 'SQLITE_IOERR'].some((errMsg) => originalException.message.includes(errMsg))
) {
return null;
}
Expand Down
26 changes: 0 additions & 26 deletions packages/cli/src/InternalHooks.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import { Service } from 'typedi';
import type { ITelemetryTrackProperties } from 'n8n-workflow';
import type { User } from '@db/entities/User';
import { Telemetry } from '@/telemetry';
import { MessageEventBus } from './eventbus/MessageEventBus/MessageEventBus';
Expand All @@ -23,16 +22,6 @@ export class InternalHooks {
await this.telemetry.init();
}

onWorkflowSharingUpdate(workflowId: string, userId: string, userList: string[]) {
const properties: ITelemetryTrackProperties = {
workflow_id: workflowId,
user_id_sharer: userId,
user_id_list: userList,
};

this.telemetry.track('User updated workflow sharing', properties);
}

onUserInviteEmailClick(userInviteClickData: { inviter: User; invitee: User }) {
this.telemetry.track('User clicked invite link from email', {
user_id: userInviteClickData.invitee.id,
Expand Down Expand Up @@ -63,19 +52,4 @@ export class InternalHooks {
user_id: userPasswordResetData.user.id,
});
}

onEmailFailed(failedEmailData: {
user: User;
message_type:
| 'Reset password'
| 'New user invite'
| 'Resend invite'
| 'Workflow shared'
| 'Credentials shared';
public_api: boolean;
}) {
this.telemetry.track('Instance failed to send transactional email to user', {
user_id: failedEmailData.user.id,
});
}
}
12 changes: 2 additions & 10 deletions packages/cli/src/UserManagement/email/UserManagementMailer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -120,14 +120,10 @@ export class UserManagementMailer {

return result;
} catch (e) {
Container.get(InternalHooks).onEmailFailed({
user: sharer,
message_type: 'Workflow shared',
public_api: false,
});
Container.get(EventService).emit('email-failed', {
user: sharer,
messageType: 'Workflow shared',
publicApi: false,
});

const error = toError(e);
Expand Down Expand Up @@ -179,14 +175,10 @@ export class UserManagementMailer {

return result;
} catch (e) {
Container.get(InternalHooks).onEmailFailed({
user: sharer,
message_type: 'Credentials shared',
public_api: false,
});
Container.get(EventService).emit('email-failed', {
user: sharer,
messageType: 'Credentials shared',
publicApi: false,
});

const error = toError(e);
Expand Down
17 changes: 9 additions & 8 deletions packages/cli/src/WorkflowRunner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ import { ExternalHooks } from '@/ExternalHooks';
import type { IExecutionResponse, IWorkflowExecutionDataProcess } from '@/Interfaces';
import { NodeTypes } from '@/NodeTypes';
import type { Job, JobData, JobResult } from '@/scaling/types';
import { ScalingService } from '@/scaling/scaling.service';
import type { ScalingService } from '@/scaling/scaling.service';
import * as WorkflowHelpers from '@/WorkflowHelpers';
import * as WorkflowExecuteAdditionalData from '@/WorkflowExecuteAdditionalData';
import { generateFailedExecutionFromError } from '@/WorkflowHelpers';
Expand All @@ -40,7 +40,7 @@ import { EventService } from './events/event.service';

@Service()
export class WorkflowRunner {
private readonly scalingService: ScalingService;
private scalingService: ScalingService;

private executionsMode = config.getEnv('executions.mode');

Expand All @@ -53,11 +53,7 @@ export class WorkflowRunner {
private readonly nodeTypes: NodeTypes,
private readonly permissionChecker: PermissionChecker,
private readonly eventService: EventService,
) {
if (this.executionsMode === 'queue') {
this.scalingService = Container.get(ScalingService);
}
}
) {}

/** The process did error */
async processError(
Expand Down Expand Up @@ -360,6 +356,11 @@ export class WorkflowRunner {
loadStaticData: !!loadStaticData,
};

if (!this.scalingService) {
const { ScalingService } = await import('@/scaling/scaling.service');
this.scalingService = Container.get(ScalingService);
}

let priority = 100;
if (realtime === true) {
// Jobs which require a direct response get a higher priority
Expand Down Expand Up @@ -404,7 +405,7 @@ export class WorkflowRunner {
async (resolve, reject, onCancel) => {
onCancel.shouldReject = false;
onCancel(async () => {
await Container.get(ScalingService).stopJob(job);
await this.scalingService.stopJob(job);

// We use "getWorkflowHooksWorkerExecuter" as "getWorkflowHooksWorkerMain" does not contain the
// "workflowExecuteAfter" which we require.
Expand Down
2 changes: 1 addition & 1 deletion packages/cli/src/commands/webhook.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import { ApplicationError } from 'n8n-workflow';

import config from '@/config';
import { ActiveExecutions } from '@/ActiveExecutions';
import { ScalingService } from '@/scaling/scaling.service';
import { WebhookServer } from '@/webhooks/WebhookServer';
import { BaseCommand } from './BaseCommand';

Expand Down Expand Up @@ -96,6 +95,7 @@ export class Webhook extends BaseCommand {
);
}

const { ScalingService } = await import('@/scaling/scaling.service');
await Container.get(ScalingService).setupQueue();
await this.server.start();
this.logger.debug(`Webhook listener ID: ${this.server.uniqueInstanceId}`);
Expand Down
3 changes: 2 additions & 1 deletion packages/cli/src/commands/worker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import { sleep, ApplicationError } from 'n8n-workflow';
import * as Db from '@/Db';
import * as ResponseHelper from '@/ResponseHelper';
import config from '@/config';
import { ScalingService } from '@/scaling/scaling.service';
import type { ScalingService } from '@/scaling/scaling.service';
import { N8N_VERSION, inTest } from '@/constants';
import type { ICredentialsOverwrite } from '@/Interfaces';
import { CredentialsOverwrites } from '@/CredentialsOverwrites';
Expand Down Expand Up @@ -171,6 +171,7 @@ export class Worker extends BaseCommand {
}

async initScalingService() {
const { ScalingService } = await import('@/scaling/scaling.service');
this.scalingService = Container.get(ScalingService);

await this.scalingService.setupQueue();
Expand Down
2 changes: 1 addition & 1 deletion packages/cli/src/config/schema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -341,7 +341,7 @@ export const schema = {
env: 'N8N_RESTRICT_FILE_ACCESS_TO',
},
blockFileAccessToN8nFiles: {
doc: 'If set to true it will block access to all files in the ".n8n" directory and user defined config files.',
doc: 'If set to true it will block access to all files in the ".n8n" directory, the static cache dir at ~/.cache/n8n/public, and user defined config files.',
format: Boolean,
default: true,
env: 'N8N_BLOCK_FILE_ACCESS_TO_N8N_FILES',
Expand Down
Loading

0 comments on commit badcb54

Please sign in to comment.