From abadbd69cac13b5324c7c06900f9efc912bccb8d Mon Sep 17 00:00:00 2001 From: Antonio Barcelos Date: Fri, 30 Apr 2021 18:29:43 +0200 Subject: [PATCH] Mark all connections for closure on LDAP Auth Credential expiration When an authenticated connection is made by the drivers, the server takes the raw credentials passed in the HELLO message and uses them to generate a token which is then cached and used to authorize all subsequent requests. In some implementations of auth (principally LDAP), this cached token can expire and the original raw credentials are required to generate a new token. Since these raw credentials are not stored on the server and are only provided during connection initialization, the only way currently to refresh these tokens is by terminating the existing connection and create new connections. When the driver receives an error with status `Status.Security.AuthorizationExpired` this should have the effect of marking all current connections as stale/invalid, forcing the driver to establish new connections and therefore refreshing the credentials cached on the server. Existing connections which are currently in use should not be interrupted. --- .../connection-provider-direct.js | 29 ++- .../connection-provider-routing.js | 12 +- .../connection/connection-error-handler.js | 29 ++- .../connection-provider-direct.test.js | 57 +++++- .../connection-provider-routing.test.js | 177 ++++++++++++++++-- .../connection-error-handler.test.js | 63 ++++++- 6 files changed, 339 insertions(+), 28 deletions(-) rename {test/internal => bolt-connection/test/connection-provider}/connection-provider-direct.test.js (58%) rename {test/internal => bolt-connection/test/connection-provider}/connection-provider-routing.test.js (92%) rename {test/internal => bolt-connection/test/connection}/connection-error-handler.test.js (66%) diff --git a/bolt-connection/src/connection-provider/connection-provider-direct.js b/bolt-connection/src/connection-provider/connection-provider-direct.js index 40cf39f95..76f0f408b 100644 --- a/bolt-connection/src/connection-provider/connection-provider-direct.js +++ b/bolt-connection/src/connection-provider/connection-provider-direct.js @@ -18,13 +18,19 @@ */ import PooledConnectionProvider from './connection-provider-pooled' -import { createChannelConnection, DelegateConnection } from '../connection' -import { internal } from 'neo4j-driver-core' +import { + createChannelConnection, + DelegateConnection, + ConnectionErrorHandler +} from '../connection' +import { internal, error } from 'neo4j-driver-core' const { constants: { BOLT_PROTOCOL_V4_0, BOLT_PROTOCOL_V3 } } = internal +const { SERVICE_UNAVAILABLE, newError } = error + export default class DirectConnectionProvider extends PooledConnectionProvider { constructor ({ id, config, log, address, userAgent, authToken }) { super({ id, config, log, userAgent, authToken }) @@ -37,9 +43,26 @@ export default class DirectConnectionProvider extends PooledConnectionProvider { * its arguments. */ acquireConnection ({ accessMode, database, bookmarks } = {}) { + const databaseSpecificErrorHandler = ConnectionErrorHandler.create({ + errorCode: SERVICE_UNAVAILABLE, + handleAuthorizationExpired: (error, address) => + this._handleAuthorizationExpired(error, address, database) + }) + return this._connectionPool .acquire(this._address) - .then(connection => new DelegateConnection(connection, null)) + .then( + connection => + new DelegateConnection(connection, databaseSpecificErrorHandler) + ) + } + + _handleAuthorizationExpired (error, address, database) { + this._log.warn( + `Direct driver ${this._id} will close connection to ${address} for database '${database}' because of an error ${error.code} '${error.message}'` + ) + this._connectionPool.purge(address).catch(() => {}) + return error } async _hasProtocolVersion (versionPredicate) { diff --git a/bolt-connection/src/connection-provider/connection-provider-routing.js b/bolt-connection/src/connection-provider/connection-provider-routing.js index b1084c27f..fcd6b25ba 100644 --- a/bolt-connection/src/connection-provider/connection-provider-routing.js +++ b/bolt-connection/src/connection-provider/connection-provider-routing.js @@ -100,6 +100,14 @@ export default class RoutingConnectionProvider extends PooledConnectionProvider return error } + _handleAuthorizationExpired (error, address, database) { + this._log.warn( + `Routing driver ${this._id} will close connections to ${address} for database '${database}' because of an error ${error.code} '${error.message}'` + ) + this._connectionPool.purge(address).catch(() => {}) + return error + } + _handleWriteFailure (error, address, database) { this._log.warn( `Routing driver ${this._id} will forget writer ${address} for database '${database}' because of an error ${error.code} '${error.message}'` @@ -122,7 +130,9 @@ export default class RoutingConnectionProvider extends PooledConnectionProvider const databaseSpecificErrorHandler = new ConnectionErrorHandler( SESSION_EXPIRED, (error, address) => this._handleUnavailability(error, address, database), - (error, address) => this._handleWriteFailure(error, address, database) + (error, address) => this._handleWriteFailure(error, address, database), + (error, address) => + this._handleAuthorizationExpired(error, address, database) ) const routingTable = await this._freshRoutingTable({ diff --git a/bolt-connection/src/connection/connection-error-handler.js b/bolt-connection/src/connection/connection-error-handler.js index 43bd71245..d90e8825b 100644 --- a/bolt-connection/src/connection/connection-error-handler.js +++ b/bolt-connection/src/connection/connection-error-handler.js @@ -22,10 +22,30 @@ import { error } from 'neo4j-driver-core' const { SERVICE_UNAVAILABLE, SESSION_EXPIRED } = error export default class ConnectionErrorHandler { - constructor (errorCode, handleUnavailability, handleWriteFailure) { + constructor ( + errorCode, + handleUnavailability, + handleWriteFailure, + handleAuthorizationExpired + ) { this._errorCode = errorCode this._handleUnavailability = handleUnavailability || noOpHandler this._handleWriteFailure = handleWriteFailure || noOpHandler + this._handleAuthorizationExpired = handleAuthorizationExpired || noOpHandler + } + + static create ({ + errorCode, + handleUnavailability, + handleWriteFailure, + handleAuthorizationExpired + }) { + return new ConnectionErrorHandler( + errorCode, + handleUnavailability, + handleWriteFailure, + handleAuthorizationExpired + ) } /** @@ -43,6 +63,9 @@ export default class ConnectionErrorHandler { * @return {Neo4jError} new error that should be propagated to the user. */ handleAndTransformError (error, address) { + if (isAutorizationExpiredError(error)) { + return this._handleAuthorizationExpired(error, address) + } if (isAvailabilityError(error)) { return this._handleUnavailability(error, address) } @@ -53,6 +76,10 @@ export default class ConnectionErrorHandler { } } +function isAutorizationExpiredError (error) { + return error && error.code === 'Neo.ClientError.Security.AuthorizationExpired' +} + function isAvailabilityError (error) { if (error) { return ( diff --git a/test/internal/connection-provider-direct.test.js b/bolt-connection/test/connection-provider/connection-provider-direct.test.js similarity index 58% rename from test/internal/connection-provider-direct.test.js rename to bolt-connection/test/connection-provider/connection-provider-direct.test.js index 2553978be..a9e7568b4 100644 --- a/test/internal/connection-provider-direct.test.js +++ b/bolt-connection/test/connection-provider/connection-provider-direct.test.js @@ -17,12 +17,10 @@ * limitations under the License. */ -import { READ } from '../../src/driver' -import DirectConnectionProvider from '../../bolt-connection/lib/connection-provider/connection-provider-direct' -import Pool from '../../bolt-connection/lib/pool/pool' -import Connection from '../../bolt-connection/lib/connection/connection' -import DelegateConnection from '../../bolt-connection/lib/connection/connection-delegate' -import { internal } from 'neo4j-driver-core' +import DirectConnectionProvider from '../../src/connection-provider/connection-provider-direct' +import { Pool } from '../../src/pool' +import { Connection, DelegateConnection } from '../../src/connection' +import { internal, newError } from 'neo4j-driver-core' const { serverAddress: { ServerAddress }, @@ -36,7 +34,7 @@ describe('#unit DirectConnectionProvider', () => { const connectionProvider = newDirectConnectionProvider(address, pool) connectionProvider - .acquireConnection({ accessMode: READ, database: '' }) + .acquireConnection({ accessMode: 'READ', database: '' }) .then(connection => { expect(connection).toBeDefined() expect(connection.address).toEqual(address) @@ -52,18 +50,59 @@ describe('#unit DirectConnectionProvider', () => { const connectionProvider = newDirectConnectionProvider(address, pool) const conn = await connectionProvider.acquireConnection({ - accessMode: READ, + accessMode: 'READ', database: '' }) expect(conn instanceof DelegateConnection).toBeTruthy() }) + + it('should purge connections for address when AuthorizationExpired happens', async () => { + const address = ServerAddress.fromUrl('localhost:123') + const pool = newPool() + jest.spyOn(pool, 'purge') + const connectionProvider = newDirectConnectionProvider(address, pool) + + const conn = await connectionProvider.acquireConnection({ + accessMode: 'READ', + database: '' + }) + + const error = newError( + 'Message', + 'Neo.ClientError.Security.AuthorizationExpired' + ) + + conn.handleAndTransformError(error, address) + + expect(pool.purge).toHaveBeenCalledWith(address) + }) + + it('should purge not change error when AuthorizationExpired happens', async () => { + const address = ServerAddress.fromUrl('localhost:123') + const pool = newPool() + const connectionProvider = newDirectConnectionProvider(address, pool) + + const conn = await connectionProvider.acquireConnection({ + accessMode: 'READ', + database: '' + }) + + const expectedError = newError( + 'Message', + 'Neo.ClientError.Security.AuthorizationExpired' + ) + + const error = conn.handleAndTransformError(expectedError, address) + + expect(error).toBe(expectedError) + }) }) function newDirectConnectionProvider (address, pool) { const connectionProvider = new DirectConnectionProvider({ id: 0, config: {}, - logger: Logger.noOp(), + log: Logger.noOp(), address: address }) connectionProvider._connectionPool = pool diff --git a/test/internal/connection-provider-routing.test.js b/bolt-connection/test/connection-provider/connection-provider-routing.test.js similarity index 92% rename from test/internal/connection-provider-routing.test.js rename to bolt-connection/test/connection-provider/connection-provider-routing.test.js index 404e0ec8b..ad0c34b44 100644 --- a/test/internal/connection-provider-routing.test.js +++ b/bolt-connection/test/connection-provider/connection-provider-routing.test.js @@ -17,16 +17,19 @@ * limitations under the License. */ -import { READ, WRITE } from '../../src/driver' -import { newError, error, Integer, int, internal } from 'neo4j-driver-core' -import RoutingTable from '../../bolt-connection/lib/rediscovery/routing-table' -import Pool from '../../bolt-connection/lib/pool/pool' -import SimpleHostNameResolver from '../../bolt-connection/lib/channel/browser/browser-host-name-resolver' -import RoutingConnectionProvider from '../../bolt-connection/lib/connection-provider/connection-provider-routing' -import { VERSION_IN_DEV } from '../../src/internal/server-version' -import Connection from '../../bolt-connection/lib/connection/connection' -import DelegateConnection from '../../bolt-connection/lib/connection/connection-delegate' -import { Neo4jError } from '../../src' +import { + newError, + Neo4jError, + error, + Integer, + int, + internal +} from 'neo4j-driver-core' +import { RoutingTable } from '../../src/rediscovery/' +import { Pool } from '../../src/pool' +import SimpleHostNameResolver from '../../src/channel/browser/browser-host-name-resolver' +import RoutingConnectionProvider from '../../src/connection-provider/connection-provider-routing' +import { DelegateConnection, Connection } from '../../src/connection' const { serverAddress: { ServerAddress }, @@ -34,6 +37,8 @@ const { } = internal const { SERVICE_UNAVAILABLE, SESSION_EXPIRED } = error +const READ = 'READ' +const WRITE = 'WRITE' describe('#unit RoutingConnectionProvider', () => { const server0 = ServerAddress.fromUrl('server0') @@ -1414,6 +1419,80 @@ describe('#unit RoutingConnectionProvider', () => { }) }, 10000) + it('should purge connections for address when AuthorizationExpired happens', async () => { + const pool = newPool() + + jest.spyOn(pool, 'purge') + + const connectionProvider = newRoutingConnectionProvider( + [ + newRoutingTable( + null, + [server1, server2], + [server3, server2], + [server2, server4] + ) + ], + pool + ) + + const error = newError( + 'Message', + 'Neo.ClientError.Security.AuthorizationExpired' + ) + + const server2Connection = await connectionProvider.acquireConnection({ + accessMode: 'WRITE', + database: null + }) + + const server3Connection = await connectionProvider.acquireConnection({ + accessMode: 'READ', + database: null + }) + + server3Connection.handleAndTransformError(error, server3) + server2Connection.handleAndTransformError(error, server2) + + expect(pool.purge).toHaveBeenCalledWith(server3) + expect(pool.purge).toHaveBeenCalledWith(server2) + }) + + it('should purge not change error when AuthorizationExpired happens', async () => { + const pool = newPool() + + jest.spyOn(pool, 'purge') + + const connectionProvider = newRoutingConnectionProvider( + [ + newRoutingTable( + null, + [server1, server2], + [server3, server2], + [server2, server4] + ) + ], + pool + ) + + const expectedError = newError( + 'Message', + 'Neo.ClientError.Security.AuthorizationExpired' + ) + + const server2Connection = await connectionProvider.acquireConnection({ + accessMode: 'WRITE', + database: null + }) + + const error = server2Connection.handleAndTransformError( + expectedError, + server2 + ) + + expect(error).toBe(expectedError) + }) + it('should use resolved seed router after accepting table with no writers', done => { const routingTable1 = newRoutingTable( null, @@ -1521,6 +1600,80 @@ describe('#unit RoutingConnectionProvider', () => { expect(conn2.address).toBe(serverA) }, 10000) + it('should purge connections for address when AuthorizationExpired happens', async () => { + const pool = newPool() + + jest.spyOn(pool, 'purge') + + const connectionProvider = newRoutingConnectionProvider( + [ + newRoutingTable( + 'databaseA', + [server1, server2], + [server1], + [server2] + ), + newRoutingTable('databaseB', [serverA, serverB], [serverA], [serverB]) + ], + pool + ) + + const error = newError( + 'Message', + 'Neo.ClientError.Security.AuthorizationExpired' + ) + + const server2Connection = await connectionProvider.acquireConnection({ + accessMode: 'WRITE', + database: 'databaseA' + }) + + const serverAConnection = await connectionProvider.acquireConnection({ + accessMode: 'READ', + database: 'databaseB' + }) + + serverAConnection.handleAndTransformError(error, serverA) + server2Connection.handleAndTransformError(error, server2) + + expect(pool.purge).toHaveBeenCalledWith(serverA) + expect(pool.purge).toHaveBeenCalledWith(server2) + }) + + it('should purge not change error when AuthorizationExpired happens', async () => { + const pool = newPool() + + const connectionProvider = newRoutingConnectionProvider( + [ + newRoutingTable( + 'databaseA', + [server1, server2], + [server1], + [server2] + ), + newRoutingTable('databaseB', [serverA, serverB], [serverA], [serverB]) + ], + pool + ) + + const expectedError = newError( + 'Message', + 'Neo.ClientError.Security.AuthorizationExpired' + ) + + const server2Connection = await connectionProvider.acquireConnection({ + accessMode: 'WRITE', + database: 'databaseA' + }) + + const error = server2Connection.handleAndTransformError( + expectedError, + server2 + ) + + expect(error).toBe(expectedError) + }) + it('should acquire write connection from correct routing table', async () => { const pool = newPool() const connectionProvider = newRoutingConnectionProvider( @@ -1978,9 +2131,7 @@ function setupRoutingConnectionProviderToRememberRouters ( function newPool () { return new Pool({ create: (address, release) => - Promise.resolve( - new FakeConnection(address, release, VERSION_IN_DEV.toString(), 4.0) - ) + Promise.resolve(new FakeConnection(address, release, 'version', 4.0)) }) } diff --git a/test/internal/connection-error-handler.test.js b/bolt-connection/test/connection/connection-error-handler.test.js similarity index 66% rename from test/internal/connection-error-handler.test.js rename to bolt-connection/test/connection/connection-error-handler.test.js index 452ec91e7..7d3c6e8fd 100644 --- a/test/internal/connection-error-handler.test.js +++ b/bolt-connection/test/connection/connection-error-handler.test.js @@ -17,7 +17,7 @@ * limitations under the License. */ -import ConnectionErrorHandler from '../../bolt-connection/lib/connection/connection-error-handler' +import ConnectionErrorHandler from '../../src/connection/connection-error-handler' import { newError, error, internal } from 'neo4j-driver-core' const { @@ -79,6 +79,67 @@ describe('#unit ConnectionErrorHandler', () => { ]) }) + it('should handle and transform authorization expired error', () => { + const errors = [] + const addresses = [] + const transformedError = newError('Message', 'Code') + const handler = ConnectionErrorHandler.create({ + errorCode: SERVICE_UNAVAILABLE, + handleAuthorizationExpired: (error, address) => { + errors.push(error) + addresses.push(address) + return transformedError + } + }) + + const error1 = newError( + 'C', + 'Neo.ClientError.Security.AuthorizationExpired' + ) + + const errorTransformed1 = handler.handleAndTransformError( + error1, + ServerAddress.fromUrl('localhost:0') + ) + + expect(errorTransformed1).toEqual(transformedError) + + expect(addresses).toEqual([ServerAddress.fromUrl('localhost:0')]) + }) + + it('should return original erro if authorization expired handler is not informed', () => { + const errors = [] + const addresses = [] + const transformedError = newError('Message', 'Code') + const handler = ConnectionErrorHandler.create({ + errorCode: SERVICE_UNAVAILABLE, + handleUnavailability: (error, address) => { + errors.push(error) + addresses.push(address) + return transformedError + }, + handleWriteFailure: (error, address) => { + errors.push(error) + addresses.push(address) + return transformedError + } + }) + + const error1 = newError( + 'C', + 'Neo.ClientError.Security.AuthorizationExpired' + ) + + const errorTransformed1 = handler.handleAndTransformError( + error1, + ServerAddress.fromUrl('localhost:0') + ) + + expect(errorTransformed1).toEqual(error1) + + expect(addresses).toEqual([]) + }) + it('should handle and transform failure to write errors', () => { const errors = [] const addresses = []