From 92d4befea6050532dfb57bf11d713960e1bd8c66 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E0=A4=95=E0=A4=BE=E0=A4=B0=E0=A4=A4=E0=A5=8B=E0=A4=AB?= =?UTF-8?q?=E0=A5=8D=E0=A4=AB=E0=A5=87=E0=A4=B2=E0=A4=B8=E0=A5=8D=E0=A4=95?= =?UTF-8?q?=E0=A5=8D=E0=A4=B0=E0=A4=BF=E0=A4=AA=E0=A5=8D=E0=A4=9F=E2=84=A2?= Date: Thu, 24 Aug 2023 15:31:37 +0200 Subject: [PATCH] fix(core): Fix AddMfaColumns migration for sqlite (no-changelog) (#7006) When ever we have migrations that use `.addColumn` or `.dropColumn`, typeorm recreates tables for sqlite. so, we need to disable foreign key enforcement for sqlite, or else data in some tables can get deleted because of `ON DELETE CASCADE` [This has happened in the past](https://github.com/n8n-io/n8n/pull/6739), and we should really come up with a way to prevent this from happening again. --------- Signed-off-by: Oleg Ivaniv Co-authored-by: Oleg Ivaniv --- cypress/e2e/27-two-factor-authentication.cy.ts | 17 ++++++++--------- cypress/pages/settings-personal.ts | 11 ++++++----- cypress/support/commands.ts | 5 ----- cypress/support/index.ts | 1 - .../sqlite/1690000000040-AddMfaColumns.ts | 5 +++++ .../src/databases/migrations/sqlite/index.ts | 2 +- 6 files changed, 20 insertions(+), 21 deletions(-) create mode 100644 packages/cli/src/databases/migrations/sqlite/1690000000040-AddMfaColumns.ts diff --git a/cypress/e2e/27-two-factor-authentication.cy.ts b/cypress/e2e/27-two-factor-authentication.cy.ts index 47dcc02fc..e2aa0d0f9 100644 --- a/cypress/e2e/27-two-factor-authentication.cy.ts +++ b/cypress/e2e/27-two-factor-authentication.cy.ts @@ -3,6 +3,7 @@ import { INSTANCE_OWNER, BACKEND_BASE_URL } from '../constants'; import { SigninPage } from '../pages'; import { PersonalSettingsPage } from '../pages/settings-personal'; import { MfaLoginPage } from '../pages/mfa-login'; +import generateOTPToken from 'cypress-otp'; const MFA_SECRET = 'KVKFKRCPNZQUYMLXOVYDSQKJKZDTSRLD'; @@ -41,10 +42,9 @@ describe('Two-factor authentication', () => { signinPage.actions.loginWithEmailAndPassword(email, password); personalSettingsPage.actions.enableMfa(); mainSidebar.actions.signout(); - cy.generateToken(user.mfaSecret).then((token) => { - mfaLoginPage.actions.loginWithMfaToken(email, password, token); - mainSidebar.actions.signout(); - }); + const token = generateOTPToken(user.mfaSecret) + mfaLoginPage.actions.loginWithMfaToken(email, password, token); + mainSidebar.actions.signout(); }); it('Should be able to login with recovery code', () => { @@ -61,10 +61,9 @@ describe('Two-factor authentication', () => { signinPage.actions.loginWithEmailAndPassword(email, password); personalSettingsPage.actions.enableMfa(); mainSidebar.actions.signout(); - cy.generateToken(user.mfaSecret).then((token) => { - mfaLoginPage.actions.loginWithMfaToken(email, password, token); - personalSettingsPage.actions.disableMfa(); - mainSidebar.actions.signout(); - }); + const token = generateOTPToken(user.mfaSecret) + mfaLoginPage.actions.loginWithMfaToken(email, password, token); + personalSettingsPage.actions.disableMfa(); + mainSidebar.actions.signout(); }); }); diff --git a/cypress/pages/settings-personal.ts b/cypress/pages/settings-personal.ts index bb738119c..671b5e21a 100644 --- a/cypress/pages/settings-personal.ts +++ b/cypress/pages/settings-personal.ts @@ -1,6 +1,7 @@ import { ChangePasswordModal } from './modals/change-password-modal'; import { MfaSetupModal } from './modals/mfa-setup-modal'; import { BasePage } from './base'; +import generateOTPToken from 'cypress-otp'; const changePasswordModal = new ChangePasswordModal(); const mfaSetupModal = new MfaSetupModal(); @@ -61,11 +62,11 @@ export class PersonalSettingsPage extends BasePage { this.getters.enableMfaButton().click(); mfaSetupModal.getters.copySecretToClipboardButton().realClick(); cy.readClipboard().then((secret) => { - cy.generateToken(secret).then((token) => { - mfaSetupModal.getters.tokenInput().type(token); - mfaSetupModal.getters.downloadRecoveryCodesButton().click(); - mfaSetupModal.getters.saveButton().click(); - }); + const token = generateOTPToken(secret) + + mfaSetupModal.getters.tokenInput().type(token); + mfaSetupModal.getters.downloadRecoveryCodesButton().click(); + mfaSetupModal.getters.saveButton().click(); }); }, disableMfa: () => { diff --git a/cypress/support/commands.ts b/cypress/support/commands.ts index 48f251501..c86db382a 100644 --- a/cypress/support/commands.ts +++ b/cypress/support/commands.ts @@ -1,7 +1,6 @@ import 'cypress-real-events'; import { WorkflowPage } from '../pages'; import { BACKEND_BASE_URL, N8N_AUTH_COOKIE } from '../constants'; -import generateOTPToken from 'cypress-otp'; Cypress.Commands.add('getByTestId', (selector, ...args) => { return cy.get(`[data-test-id="${selector}"]`, ...args); @@ -162,7 +161,3 @@ Cypress.Commands.add('draganddrop', (draggableSelector, droppableSelector) => { } }); }); - -Cypress.Commands.add('generateToken', (secret: string) => { - return generateOTPToken(secret); -}); diff --git a/cypress/support/index.ts b/cypress/support/index.ts index bd74453bf..37140351f 100644 --- a/cypress/support/index.ts +++ b/cypress/support/index.ts @@ -37,7 +37,6 @@ declare global { options?: { abs?: boolean; index?: number; realMouse?: boolean }, ): void; draganddrop(draggableSelector: string, droppableSelector: string): void; - generateToken(mfaSecret: string): Chainable; } } } diff --git a/packages/cli/src/databases/migrations/sqlite/1690000000040-AddMfaColumns.ts b/packages/cli/src/databases/migrations/sqlite/1690000000040-AddMfaColumns.ts new file mode 100644 index 000000000..1047f44f5 --- /dev/null +++ b/packages/cli/src/databases/migrations/sqlite/1690000000040-AddMfaColumns.ts @@ -0,0 +1,5 @@ +import { AddMfaColumns1690000000030 as BaseMigration } from '../common/1690000000040-AddMfaColumns'; + +export class AddMfaColumns1690000000030 extends BaseMigration { + transaction = false as const; +} diff --git a/packages/cli/src/databases/migrations/sqlite/index.ts b/packages/cli/src/databases/migrations/sqlite/index.ts index ea5c970cf..eaf3fd3cb 100644 --- a/packages/cli/src/databases/migrations/sqlite/index.ts +++ b/packages/cli/src/databases/migrations/sqlite/index.ts @@ -41,7 +41,7 @@ import { RemoveSkipOwnerSetup1681134145997 } from './1681134145997-RemoveSkipOwn import { FixMissingIndicesFromStringIdMigration1690000000020 } from './1690000000020-FixMissingIndicesFromStringIdMigration'; import { RemoveResetPasswordColumns1690000000030 } from './1690000000030-RemoveResetPasswordColumns'; import { CreateWorkflowNameIndex1691088862123 } from '../common/1691088862123-CreateWorkflowNameIndex'; -import { AddMfaColumns1690000000030 } from './../common/1690000000040-AddMfaColumns'; +import { AddMfaColumns1690000000030 } from './1690000000040-AddMfaColumns'; const sqliteMigrations: Migration[] = [ InitialMigration1588102412422,