diff --git a/src/browser/modules/Stream/Auth/ConnectionFormController.tsx b/src/browser/modules/Stream/Auth/ConnectionFormController.tsx index 5ec27b653f4..fdc7a30928b 100644 --- a/src/browser/modules/Stream/Auth/ConnectionFormController.tsx +++ b/src/browser/modules/Stream/Auth/ConnectionFormController.tsx @@ -47,8 +47,10 @@ import { setActiveConnection, updateConnection } from 'shared/modules/connections/connectionsDuck' -import { AuthenticationMethod } from 'shared/modules/connections/connectionsDuck' -import { FORCE_CHANGE_PASSWORD } from 'shared/modules/cypher/cypherDuck' +import { + AuthenticationMethod, + FORCE_CHANGE_PASSWORD +} from 'shared/modules/connections/connectionsDuck' import { shouldRetainConnectionCredentials } from 'shared/modules/dbMeta/dbMetaDuck' import { CONNECTION_ID } from 'shared/modules/discovery/discoveryDuck' import { fetchBrowserDiscoveryDataFromUrl } from 'shared/modules/discovery/discoveryHelpers' diff --git a/src/shared/modules/connections/connectionsDuck.test.ts b/src/shared/modules/connections/connectionsDuck.test.ts index 4dc36b40484..7af830369e6 100644 --- a/src/shared/modules/connections/connectionsDuck.test.ts +++ b/src/shared/modules/connections/connectionsDuck.test.ts @@ -28,11 +28,15 @@ import { DONE as DISCOVERY_DONE, updateDiscoveryConnection } from 'shared/modules/discovery/discoveryDuck' +import forceResetPasswordQueryHelper, { + MultiDatabaseNotSupportedError +} from './forceResetPasswordQueryHelper' jest.mock('services/bolt/bolt', () => { return { closeConnection: jest.fn(), - openConnection: jest.fn() + openConnection: jest.fn(), + directConnect: jest.fn() } }) @@ -457,3 +461,310 @@ describe('switchConnectionEpic', () => { return p }) }) + +describe('handleForcePasswordChangeEpic', () => { + const bus = createBus() + const epicMiddleware = createEpicMiddleware( + connections.handleForcePasswordChangeEpic + ) + const mockStore = configureMockStore([ + epicMiddleware, + createReduxMiddleware(bus) + ]) + + let store: any + + const $$responseChannel = 'test-channel' + const action = { + host: 'bolt://localhost:7687', + type: connections.FORCE_CHANGE_PASSWORD, + password: 'changeme', + newPassword: 'password1', + $$responseChannel + } + + const executePasswordResetQuerySpy = jest.spyOn( + forceResetPasswordQueryHelper, + 'executePasswordResetQuery' + ) + + const executeAlterCurrentUserQuerySpy = jest.spyOn( + forceResetPasswordQueryHelper, + 'executeAlterCurrentUserQuery' + ) + + const executeCallChangePasswordQuerySpy = jest.spyOn( + forceResetPasswordQueryHelper, + 'executeCallChangePasswordQuery' + ) + + const mockSessionClose = jest.fn() + const mockSessionExecuteWrite = jest.fn() + + const mockDriver = { + session: jest.fn().mockReturnValue({ + close: mockSessionClose, + executeWrite: mockSessionExecuteWrite + }), + close: jest.fn().mockReturnValue(true) + } + + beforeAll(() => { + store = mockStore({}) + }) + + beforeEach(() => { + ;(bolt.directConnect as jest.Mock).mockResolvedValue(mockDriver) + }) + + afterEach(() => { + store.clearActions() + bus.reset() + jest.clearAllMocks() + }) + + test('handleForcePasswordChangeEpic resolves with an error if directConnect fails', () => { + // Given + const message = 'An error occurred.' + ;(bolt.directConnect as jest.Mock).mockRejectedValue(new Error(message)) + + const p = new Promise((resolve, reject) => { + bus.take($$responseChannel, currentAction => { + // Then + const actions = store.getActions() + try { + expect(actions).toEqual([action, currentAction]) + + expect(executeAlterCurrentUserQuerySpy).not.toHaveBeenCalled() + + expect(executeCallChangePasswordQuerySpy).not.toHaveBeenCalled() + + expect(executePasswordResetQuerySpy).not.toHaveBeenCalled() + + expect(currentAction).toEqual({ + error: expect.objectContaining({ + message + }), + success: false, + type: $$responseChannel + }) + + resolve() + + expect(mockDriver.close).not.toHaveBeenCalled() + expect(mockSessionClose).not.toHaveBeenCalled() + } catch (e) { + reject(e) + } + }) + }) + + // When + epicMiddleware.replaceEpic(connections.handleForcePasswordChangeEpic) + store.clearActions() + store.dispatch(action) + + // Return + return p + }) + + test('handleForcePasswordChangeEpic resolves when successfully executing cypher query', () => { + // Given + mockSessionExecuteWrite.mockResolvedValue(true) + + const p = new Promise((resolve, reject) => { + bus.take($$responseChannel, currentAction => { + // Then + const actions = store.getActions() + try { + expect(actions).toEqual([action, currentAction]) + + expect(executeAlterCurrentUserQuerySpy).toHaveBeenCalledTimes(1) + + expect(executeCallChangePasswordQuerySpy).not.toHaveBeenCalled() + + expect(executePasswordResetQuerySpy).toHaveBeenCalledTimes(1) + + expect(executePasswordResetQuerySpy).toHaveBeenCalledWith( + expect.anything(), + expect.objectContaining({ + parameters: { newPw: 'password1', oldPw: 'changeme' }, + query: 'ALTER CURRENT USER SET PASSWORD FROM $oldPw TO $newPw' + }), + expect.anything(), + { database: 'system' } + ) + + expect(currentAction).toEqual({ + result: { meta: 'bolt://localhost:7687' }, + success: true, + type: $$responseChannel + }) + + resolve() + + expect(mockDriver.close).toHaveBeenCalledTimes(1) + expect(mockSessionClose).toHaveBeenCalledTimes(1) + } catch (e) { + reject(e) + } + }) + }) + + // When + epicMiddleware.replaceEpic(connections.handleForcePasswordChangeEpic) + store.clearActions() + store.dispatch(action) + + // Return + return p + }) + + test('handleForcePasswordChangeEpic resolves with an error if cypher query fails', () => { + // Given + const message = 'A password must be at least 8 characters.' + mockSessionExecuteWrite + .mockRejectedValueOnce(new Error(message)) + .mockResolvedValue(true) + + const p = new Promise((resolve, reject) => { + bus.take($$responseChannel, currentAction => { + // Then + const actions = store.getActions() + try { + expect(actions).toEqual([action, currentAction]) + + expect(executeAlterCurrentUserQuerySpy).toHaveBeenCalledTimes(1) + + expect(executeCallChangePasswordQuerySpy).not.toHaveBeenCalled() + + expect(executePasswordResetQuerySpy).toHaveBeenCalledTimes(1) + + expect(currentAction).toEqual({ + error: expect.objectContaining({ + message + }), + success: false, + type: $$responseChannel + }) + + resolve() + + expect(mockDriver.close).toHaveBeenCalledTimes(1) + expect(mockSessionClose).toHaveBeenCalledTimes(1) + } catch (e) { + reject(e) + } + }) + }) + + // When + epicMiddleware.replaceEpic(connections.handleForcePasswordChangeEpic) + store.clearActions() + store.dispatch(action) + + // Return + return p + }) + + test('handleForcePasswordChangeEpic resolves when successfully falling back to dbms function call', () => { + // Given + mockSessionExecuteWrite + .mockRejectedValueOnce(new MultiDatabaseNotSupportedError()) + .mockResolvedValue(true) + + const p = new Promise((resolve, reject) => { + bus.take($$responseChannel, currentAction => { + // Then + const actions = store.getActions() + try { + expect(actions).toEqual([action, currentAction]) + + expect(executeAlterCurrentUserQuerySpy).toHaveBeenCalledTimes(1) + + expect(executeCallChangePasswordQuerySpy).toHaveBeenCalledTimes(1) + + expect(executePasswordResetQuerySpy).toHaveBeenCalledTimes(2) + + expect(executePasswordResetQuerySpy).toHaveBeenLastCalledWith( + expect.anything(), + expect.objectContaining({ + parameters: { password: 'password1' }, + query: 'CALL dbms.security.changePassword($password)' + }), + expect.anything(), + undefined + ) + + expect(currentAction).toEqual({ + result: { meta: 'bolt://localhost:7687' }, + success: true, + type: $$responseChannel + }) + + resolve() + + expect(mockDriver.close).toHaveBeenCalledTimes(1) + expect(mockSessionClose).toHaveBeenCalledTimes(2) + } catch (e) { + reject(e) + } + }) + }) + + // When + epicMiddleware.replaceEpic(connections.handleForcePasswordChangeEpic) + store.clearActions() + store.dispatch(action) + + // Return + return p + }) + + test('handleForcePasswordChangeEpic resolves with an error if dbms function call fails', () => { + // Given + const message = 'A password must be at least 8 characters.' + mockSessionExecuteWrite + .mockRejectedValueOnce(new MultiDatabaseNotSupportedError()) + .mockRejectedValue(new Error(message)) + + const p = new Promise((resolve, reject) => { + bus.take($$responseChannel, currentAction => { + // Then + const actions = store.getActions() + try { + expect(actions).toEqual([action, currentAction]) + + expect(executeAlterCurrentUserQuerySpy).toHaveBeenCalledTimes(1) + + expect(executeCallChangePasswordQuerySpy).toHaveBeenCalledTimes(1) + + expect(executePasswordResetQuerySpy).toHaveBeenCalledTimes(2) + + expect(currentAction).toEqual({ + error: expect.objectContaining({ + message + }), + success: false, + type: $$responseChannel + }) + + resolve() + + expect(mockDriver.close).toHaveBeenCalledTimes(1) + expect(mockSessionClose).toHaveBeenCalledTimes(2) + } catch (e) { + reject(e) + } + }) + }) + + // When + epicMiddleware.replaceEpic(connections.handleForcePasswordChangeEpic) + store.clearActions() + store.dispatch(action) + + // Return + return p + }) +}) diff --git a/src/shared/modules/connections/connectionsDuck.ts b/src/shared/modules/connections/connectionsDuck.ts index eea15721b66..a08585a5bf1 100644 --- a/src/shared/modules/connections/connectionsDuck.ts +++ b/src/shared/modules/connections/connectionsDuck.ts @@ -39,6 +39,10 @@ import { import { NEO4J_CLOUD_DOMAINS } from 'shared/modules/settings/settingsDuck' import { isCloudHost } from 'shared/services/utils' import { fetchMetaData } from '../dbMeta/dbMetaDuck' +import { isError } from 'shared/utils/typeguards' +import forceResetPasswordQueryHelper, { + MultiDatabaseNotSupportedError +} from './forceResetPasswordQueryHelper' export const NAME = 'connections' export const SET_ACTIVE = 'connections/SET_ACTIVE' @@ -53,6 +57,7 @@ export const STARTUP_CONNECTION_SUCCESS = export const STARTUP_CONNECTION_FAILED = 'connections/STARTUP_CONNECTION_FAILED' export const CONNECTION_SUCCESS = 'connections/CONNECTION_SUCCESS' export const DISCONNECTION_SUCCESS = 'connections/DISCONNECTION_SUCCESS' +export const FORCE_CHANGE_PASSWORD = 'connections/FORCE_CHANGE_PASSWORD' export const LOST_CONNECTION = 'connections/LOST_CONNECTION' export const UPDATE_CONNECTION_STATE = 'connections/UPDATE_CONNECTION_STATE' export const UPDATE_RETAIN_CREDENTIALS = `connections/UPDATE_RETAIN_CREDENTIALS` @@ -919,3 +924,82 @@ export const retainCredentialsSettingsEpic = (action$: any, store: any) => { }) .ignoreElements() } + +/** + * Epic to handle a FORCE_CHANGE_PASSWORD event. + * + * We need this because this is the only case where we still + * want to execute cypher even though we get an connection error back. + * + * Previously, we were attempting to read the version of Neo4j in state, falling + * back to querying the database if it was not present. This was problematic, because + * if the user was logging in for the first time, this request would fail since they + * were not authorized to execute queries against the database. + * + * This problem was further compounded if the default (neo4j) database did not exist. + * + * In this approach, we simply attempt to change the password using current syntax, + * falling back to the legacy DBMS function if this fails with a specific error message. + */ +export const handleForcePasswordChangeEpic = (some$: any) => + some$ + .ofType(FORCE_CHANGE_PASSWORD) + .mergeMap( + ( + action: Connection & { $$responseChannel: string; newPassword: string } + ) => { + if (!action.$$responseChannel) return Rx.Observable.of(null) + + return new Promise(resolve => { + const resolveAction = (error?: Error | void) => { + resolve({ + type: action.$$responseChannel, + success: error === undefined, + ...(error === undefined + ? { + result: { + meta: action.host + } + } + : { + error + }) + }) + } + + bolt + .directConnect( + action, + {}, + undefined, + false // Ignore validation errors + ) + .then(async driver => { + try { + // Attempt to change the password using Cypher syntax + const result = await forceResetPasswordQueryHelper + .executeAlterCurrentUserQuery(driver, action) + .then(resolveAction) + .catch(error => error) + + if (isError(result)) { + if (result instanceof MultiDatabaseNotSupportedError) { + // If we get a multi database not supported error, + // fall back to the legacy dbms function + await forceResetPasswordQueryHelper + .executeCallChangePasswordQuery(driver, action) + .then(resolveAction) + .catch(resolveAction) + } else { + // Otherwise, return the error for the UI to handle e.g. invalid password + resolveAction(result) + } + } + } finally { + driver.close() + } + }) + .catch(resolveAction) + }) + } + ) diff --git a/src/shared/modules/connections/forceResetPasswordQueryHelper.ts b/src/shared/modules/connections/forceResetPasswordQueryHelper.ts new file mode 100644 index 00000000000..b8a7f303004 --- /dev/null +++ b/src/shared/modules/connections/forceResetPasswordQueryHelper.ts @@ -0,0 +1,87 @@ +import neo4j from 'neo4j-driver' +import { Driver } from 'neo4j-driver' +import { userActionTxMetadata } from 'services/bolt/txMetadata' +import { Connection } from './connectionsDuck' +import { SYSTEM_DB } from '../dbMeta/dbMetaDuck' +import { isError } from 'shared/utils/typeguards' + +const MULTIDATABASE_NOT_SUPPORTED_ERROR_MESSAGE = + 'Driver is connected to the database that does not support multiple databases.' + +export class MultiDatabaseNotSupportedError extends Error {} + +export default { + executePasswordResetQuery: async ( + driver: Driver, + action: { + query: string + parameters: { oldPw: string; newPw: string } | { password: string } + }, + metadata: { type: string; app: string }, + useDb = {} + ): Promise => { + const session = driver.session({ + defaultAccessMode: neo4j.session.WRITE, + ...useDb + }) + + try { + await session.executeWrite( + tx => tx.run(action.query, action.parameters), + { + metadata + } + ) + } catch (e) { + throw isError(e) && + e.message.startsWith(MULTIDATABASE_NOT_SUPPORTED_ERROR_MESSAGE) + ? new MultiDatabaseNotSupportedError(e.message) + : e + } finally { + session.close() + } + }, + /** + * Executes a query to change the user's password using Cypher available + * on Neo4j versions since 4.0 + */ + executeAlterCurrentUserQuery: function ( + driver: Driver, + action: Connection & { newPassword: string } + ): Promise { + const payload = { + query: 'ALTER CURRENT USER SET PASSWORD FROM $oldPw TO $newPw', + parameters: { + oldPw: action.password, + newPw: action.newPassword + } + } + + return this.executePasswordResetQuery( + driver, + payload, + userActionTxMetadata.txMetadata, + { database: SYSTEM_DB } + ) + }, + /** + * Executes a query to change the user's password using legacy DBMS function + * for versions of Neo4j older than 4.0 + */ + executeCallChangePasswordQuery: function ( + driver: Driver, + action: { newPassword: string } + ): Promise { + const payload = { + query: 'CALL dbms.security.changePassword($password)', + parameters: { password: action.newPassword } + } + + return this.executePasswordResetQuery( + driver, + payload, + userActionTxMetadata.txMetadata, + undefined + ) + } +} diff --git a/src/shared/modules/cypher/cypherDuck.ts b/src/shared/modules/cypher/cypherDuck.ts index b5d706fe3d2..9bf5486e820 100644 --- a/src/shared/modules/cypher/cypherDuck.ts +++ b/src/shared/modules/cypher/cypherDuck.ts @@ -20,17 +20,6 @@ import neo4j, { QueryResult } from 'neo4j-driver' import Rx from 'rxjs' -import { - getRawVersion, - serverInfoQuery, - SYSTEM_DB, - updateServerInfo -} from '../dbMeta/dbMetaDuck' -import { - FIRST_MULTI_DB_SUPPORT, - FIRST_NO_MULTI_DB_SUPPORT, - changeUserPasswordQuery -} from '../features/versionedFeatures' import { getClusterAddresses } from './queriesProcedureHelper' import bolt from 'services/bolt/bolt' import { buildTxFunctionByMode } from 'services/bolt/boltHelpers' @@ -39,10 +28,7 @@ import { userActionTxMetadata } from 'services/bolt/txMetadata' import { flatten } from 'services/utils' -import { - Connection, - getActiveConnectionData -} from 'shared/modules/connections/connectionsDuck' +import { getActiveConnectionData } from 'shared/modules/connections/connectionsDuck' const NAME = 'cypher' export const CYPHER_REQUEST = `${NAME}/REQUEST` @@ -50,7 +36,6 @@ export const ROUTED_CYPHER_READ_REQUEST = `${NAME}/ROUTED_READ_REQUEST` export const ROUTED_CYPHER_WRITE_REQUEST = `${NAME}/ROUTED_WRITE_REQUEST` export const AD_HOC_CYPHER_REQUEST = `${NAME}/AD_HOC_REQUEST` export const CLUSTER_CYPHER_REQUEST = `${NAME}/CLUSTER_REQUEST` -export const FORCE_CHANGE_PASSWORD = `${NAME}/FORCE_CHANGE_PASSWORD` // Helpers const queryAndResolve = async ( @@ -262,76 +247,3 @@ export const clusterCypherRequestEpic = (some$: any, store: any) => result: { records } } }) - -// We need this because this is the only case where we still -// want to execute cypher even though we get an connection error back -export const handleForcePasswordChangeEpic = (some$: any, store: any) => - some$ - .ofType(FORCE_CHANGE_PASSWORD) - .mergeMap( - ( - action: Connection & { $$responseChannel: string; newPassword: string } - ) => { - if (!action.$$responseChannel) return Rx.Observable.of(null) - - return new Promise(resolve => { - bolt - .directConnect( - action, - {}, - undefined, - false // Ignore validation errors - ) - .then(async driver => { - // Let's establish what server version we're connected to if not in state - if (!getRawVersion(store.getState())) { - const versionRes: any = await queryAndResolve( - driver, - { ...action, query: serverInfoQuery, parameters: {} }, - undefined, - userActionTxMetadata.txMetadata - ) - - if (versionRes.success) { - store.dispatch(updateServerInfo(versionRes.result)) - } - } - - let versionForPasswordQuery = getRawVersion(store.getState()) - - // if we failed to get a version by querying, do a best effort quess - const supportsMultiDb = await driver.supportsMultiDb() - if (!versionForPasswordQuery) { - versionForPasswordQuery = supportsMultiDb - ? FIRST_MULTI_DB_SUPPORT - : FIRST_NO_MULTI_DB_SUPPORT - } - - // Figure out how to change the pw on that server version - const queryObj = changeUserPasswordQuery( - versionForPasswordQuery, - action.password, - action.newPassword - ) - - // and then change the password - const res = await queryAndResolve( - driver, - { ...action, ...queryObj }, - undefined, - userActionTxMetadata.txMetadata, - supportsMultiDb ? { database: SYSTEM_DB } : undefined - ) - driver.close() - resolve(res) - }) - .catch(e => - resolve({ - type: action.$$responseChannel, - success: false, - error: e - }) - ) - }) - } - ) diff --git a/src/shared/modules/features/versionedFeatures.ts b/src/shared/modules/features/versionedFeatures.ts index 1813b38b46a..fa61293b5f7 100644 --- a/src/shared/modules/features/versionedFeatures.ts +++ b/src/shared/modules/features/versionedFeatures.ts @@ -99,28 +99,6 @@ export const getDefaultBoltScheme = (serverVersion: string | null) => { return pre4 } -export const changeUserPasswordQuery = ( - serverVersion: string, - oldPw: any, - newPw: any -) => { - const pre4 = { - query: 'CALL dbms.security.changePassword($password)', - parameters: { password: newPw } - } - const semverVersion = guessSemverVersion(serverVersion) - if (!semver.valid(semverVersion)) { - return pre4 - } - if (semverVersion && semver.gte(semverVersion, NEO4J_4_0)) { - return { - query: 'ALTER CURRENT USER SET PASSWORD FROM $oldPw TO $newPw', - parameters: { oldPw, newPw } - } - } - return pre4 -} - export const driverDatabaseSelection = (state: GlobalState, database: any) => { const pre4 = undefined const serverVersion = guessSemverVersion(getRawVersion(state)) diff --git a/src/shared/rootEpic.ts b/src/shared/rootEpic.ts index 2b375b6e55f..4933d6e1766 100644 --- a/src/shared/rootEpic.ts +++ b/src/shared/rootEpic.ts @@ -31,6 +31,7 @@ import { detectActiveConnectionChangeEpic, disconnectEpic, disconnectSuccessEpic, + handleForcePasswordChangeEpic, initialSwitchConnectionFailEpic, retainCredentialsSettingsEpic, silentDisconnectEpic, @@ -52,7 +53,6 @@ import { adHocCypherRequestEpic, clusterCypherRequestEpic, cypherRequestEpic, - handleForcePasswordChangeEpic, routedCypherReadRequestEpic, routedCypherWriteRequestEpic } from './modules/cypher/cypherDuck' diff --git a/src/shared/services/bolt/globalDrivers.ts b/src/shared/services/bolt/globalDrivers.ts index 585b12be96c..d68ed33c998 100644 --- a/src/shared/services/bolt/globalDrivers.ts +++ b/src/shared/services/bolt/globalDrivers.ts @@ -62,7 +62,7 @@ export const buildGlobalDriversObject = async ( opts, () => {} ) - routed && (await routed.verifyConnectivity()) + routed && (await routed.verifyConnectivity({ database: 'system' })) routingSupported = true } catch (e) { if (e && isError(e)) {