Skip to content

Commit 0a1b20e

Browse files
authored
Improve AuthTokenManager interface and factory method (#1123)
Currently, the `AuthTokenManager` is designed to handle token expiration only. The AuthTokenManager interface only has methods to receive notifications on security errors related to token expiration: `AuthTokenManager.onTokenExpired`. The provided implementation `neo4j.expirationBasedAuthTokenManager` is built to support only token expiration. However, we also want to cater for password rotation scenarios. ### Factory Method Changes #### Expiration based and bearer tokens The method `neo4j.expirationBasedAuthTokenManager` was renamed and moved to `neo4j.authTokenManagers.bearer`. ```typescript import neo4j, { AuthToken } from 'neo4j-driver' /** * Method called whenever the driver needs to refresh the token. * * The refresh will happen if the driver is notified by the server * about a token expiration or if the `Date.now() > tokenData.expiry` * * Important, the driver will block all the connections creation until * this function resolves the new auth token. */ async function fetchAuthTokenFromMyProvider () { const bearer: string = await myProvider.getBearerToken() const token: AuthToken = neo4j.auth.bearer(bearer) const expiration: Date = myProvider.getExpiryDate() return { token, // if expiration is not provided, // the driver will only fetch a new token when a failure happens expiration } } const driver = neo4j.driver( 'neo4j://localhost:7687', neo4j.authTokenManagers.bearer({ tokenProvider: fetchAuthTokenFromMyProvider }) ) ``` #### Password rotation and basic auth `neo4j.authTokenManagers.basic` was added to handle password rotation with `AuthTokenManager`. ```typescript import neo4j, { AuthToken } from 'neo4j-driver' /** * Method called whenever the driver needs to refresh the token. * * Important, the driver will block all the connections creation until * this function resolves the new auth token. */ async function fetchMyUserAndPassword () { const { user, password } = await myProvider.getUserAndPassword() return neo4j.auth.basic(user, password) } const driver = neo4j.driver( 'neo4j://localhost:7687', neo4j.authTokenManagers.basic({ tokenProvider: fetchMyUserAndPassword }) ) ``` ### Development checklist * [x] Update AuthTokenManager interface * [x] Change `expirationBasedAuthTokenManager` factory name to `authTokenManagers.bearer` * [x] Add `authTokenManagers.basic` factory * [x] Adapt `testkit-backend`
1 parent c23b651 commit 0a1b20e

32 files changed

+829
-246
lines changed

packages/bolt-connection/src/connection-provider/authentication-provider.js

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -17,17 +17,15 @@
1717
* limitations under the License.
1818
*/
1919

20-
import { expirationBasedAuthTokenManager } from 'neo4j-driver-core'
20+
import { staticAuthTokenManager } from 'neo4j-driver-core'
2121
import { object } from '../lang'
2222

2323
/**
2424
* Class which provides Authorization for {@link Connection}
2525
*/
2626
export default class AuthenticationProvider {
2727
constructor ({ authTokenManager, userAgent, boltAgent }) {
28-
this._authTokenManager = authTokenManager || expirationBasedAuthTokenManager({
29-
tokenProvider: () => {}
30-
})
28+
this._authTokenManager = authTokenManager || staticAuthTokenManager({})
3129
this._userAgent = userAgent
3230
this._boltAgent = boltAgent
3331
}
@@ -56,12 +54,10 @@ export default class AuthenticationProvider {
5654
handleError ({ connection, code }) {
5755
if (
5856
connection &&
59-
[
60-
'Neo.ClientError.Security.Unauthorized',
61-
'Neo.ClientError.Security.TokenExpired'
62-
].includes(code)
57+
code.startsWith('Neo.ClientError.Security.')
6358
) {
64-
this._authTokenManager.onTokenExpired(connection.authToken)
59+
return this._authTokenManager.handleSecurityException(connection.authToken, code)
6560
}
61+
return false
6662
}
6763
}

packages/bolt-connection/src/connection-provider/connection-provider-direct.js

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -50,8 +50,8 @@ export default class DirectConnectionProvider extends PooledConnectionProvider {
5050
async acquireConnection ({ accessMode, database, bookmarks, auth, forceReAuth } = {}) {
5151
const databaseSpecificErrorHandler = ConnectionErrorHandler.create({
5252
errorCode: SERVICE_UNAVAILABLE,
53-
handleAuthorizationExpired: (error, address, conn) =>
54-
this._handleAuthorizationExpired(error, address, conn, database)
53+
handleSecurityError: (error, address, conn) =>
54+
this._handleSecurityError(error, address, conn, database)
5555
})
5656

5757
const connection = await this._connectionPool.acquire({ auth, forceReAuth }, this._address)
@@ -68,12 +68,12 @@ export default class DirectConnectionProvider extends PooledConnectionProvider {
6868
return new DelegateConnection(connection, databaseSpecificErrorHandler)
6969
}
7070

71-
_handleAuthorizationExpired (error, address, connection, database) {
71+
_handleSecurityError (error, address, connection, database) {
7272
this._log.warn(
7373
`Direct driver ${this._id} will close connection to ${address} for database '${database}' because of an error ${error.code} '${error.message}'`
7474
)
7575

76-
return super._handleAuthorizationExpired(error, address, connection)
76+
return super._handleSecurityError(error, address, connection)
7777
}
7878

7979
async _hasProtocolVersion (versionPredicate) {

packages/bolt-connection/src/connection-provider/connection-provider-pooled.js

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@
1919

2020
import { createChannelConnection, ConnectionErrorHandler } from '../connection'
2121
import Pool, { PoolConfig } from '../pool'
22-
import { error, ConnectionProvider, ServerInfo, newError, isStaticAuthTokenManger } from 'neo4j-driver-core'
22+
import { error, ConnectionProvider, ServerInfo, newError } from 'neo4j-driver-core'
2323
import AuthenticationProvider from './authentication-provider'
2424
import { object } from '../lang'
2525

@@ -41,7 +41,6 @@ export default class PooledConnectionProvider extends ConnectionProvider {
4141
this._id = id
4242
this._config = config
4343
this._log = log
44-
this._authTokenManager = authTokenManager
4544
this._authenticationProvider = new AuthenticationProvider({ authTokenManager, userAgent, boltAgent })
4645
this._userAgent = userAgent
4746
this._boltAgent = boltAgent
@@ -224,8 +223,12 @@ export default class PooledConnectionProvider extends ConnectionProvider {
224223
conn._updateCurrentObserver()
225224
}
226225

227-
_handleAuthorizationExpired (error, address, connection) {
228-
this._authenticationProvider.handleError({ connection, code: error.code })
226+
_handleSecurityError (error, address, connection) {
227+
const handled = this._authenticationProvider.handleError({ connection, code: error.code })
228+
229+
if (handled) {
230+
error.retriable = true
231+
}
229232

230233
if (error.code === 'Neo.ClientError.Security.AuthorizationExpired') {
231234
this._connectionPool.apply(address, (conn) => { conn.authToken = null })
@@ -235,10 +238,6 @@ export default class PooledConnectionProvider extends ConnectionProvider {
235238
connection.close().catch(() => undefined)
236239
}
237240

238-
if (error.code === 'Neo.ClientError.Security.TokenExpired' && !isStaticAuthTokenManger(this._authTokenManager)) {
239-
error.retriable = true
240-
}
241-
242241
return error
243242
}
244243
}

packages/bolt-connection/src/connection-provider/connection-provider-routing.js

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -116,12 +116,12 @@ export default class RoutingConnectionProvider extends PooledConnectionProvider
116116
return error
117117
}
118118

119-
_handleAuthorizationExpired (error, address, connection, database) {
119+
_handleSecurityError (error, address, connection, database) {
120120
this._log.warn(
121121
`Routing driver ${this._id} will close connections to ${address} for database '${database}' because of an error ${error.code} '${error.message}'`
122122
)
123123

124-
return super._handleAuthorizationExpired(error, address, connection, database)
124+
return super._handleSecurityError(error, address, connection, database)
125125
}
126126

127127
_handleWriteFailure (error, address, database) {
@@ -150,7 +150,7 @@ export default class RoutingConnectionProvider extends PooledConnectionProvider
150150
(error, address) => this._handleUnavailability(error, address, context.database),
151151
(error, address) => this._handleWriteFailure(error, address, context.database),
152152
(error, address, conn) =>
153-
this._handleAuthorizationExpired(error, address, conn, context.database)
153+
this._handleSecurityError(error, address, conn, context.database)
154154
)
155155

156156
const routingTable = await this._freshRoutingTable({
@@ -584,7 +584,7 @@ export default class RoutingConnectionProvider extends PooledConnectionProvider
584584

585585
const databaseSpecificErrorHandler = ConnectionErrorHandler.create({
586586
errorCode: SESSION_EXPIRED,
587-
handleAuthorizationExpired: (error, address, conn) => this._handleAuthorizationExpired(error, address, conn)
587+
handleSecurityError: (error, address, conn) => this._handleSecurityError(error, address, conn)
588588
})
589589

590590
const delegateConnection = !connection._sticky

packages/bolt-connection/src/connection/connection-error-handler.js

Lines changed: 10 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -26,25 +26,25 @@ export default class ConnectionErrorHandler {
2626
errorCode,
2727
handleUnavailability,
2828
handleWriteFailure,
29-
handleAuthorizationExpired
29+
handleSecurityError
3030
) {
3131
this._errorCode = errorCode
3232
this._handleUnavailability = handleUnavailability || noOpHandler
3333
this._handleWriteFailure = handleWriteFailure || noOpHandler
34-
this._handleAuthorizationExpired = handleAuthorizationExpired || noOpHandler
34+
this._handleSecurityError = handleSecurityError || noOpHandler
3535
}
3636

3737
static create ({
3838
errorCode,
3939
handleUnavailability,
4040
handleWriteFailure,
41-
handleAuthorizationExpired
41+
handleSecurityError
4242
}) {
4343
return new ConnectionErrorHandler(
4444
errorCode,
4545
handleUnavailability,
4646
handleWriteFailure,
47-
handleAuthorizationExpired
47+
handleSecurityError
4848
)
4949
}
5050

@@ -63,8 +63,8 @@ export default class ConnectionErrorHandler {
6363
* @return {Neo4jError} new error that should be propagated to the user.
6464
*/
6565
handleAndTransformError (error, address, connection) {
66-
if (isAutorizationExpiredError(error)) {
67-
return this._handleAuthorizationExpired(error, address, connection)
66+
if (isSecurityError(error)) {
67+
return this._handleSecurityError(error, address, connection)
6868
}
6969
if (isAvailabilityError(error)) {
7070
return this._handleUnavailability(error, address, connection)
@@ -76,11 +76,10 @@ export default class ConnectionErrorHandler {
7676
}
7777
}
7878

79-
function isAutorizationExpiredError (error) {
80-
return error && (
81-
error.code === 'Neo.ClientError.Security.AuthorizationExpired' ||
82-
error.code === 'Neo.ClientError.Security.TokenExpired'
83-
)
79+
function isSecurityError (error) {
80+
return error != null &&
81+
error.code != null &&
82+
error.code.startsWith('Neo.ClientError.Security.')
8483
}
8584

8685
function isAvailabilityError (error) {

packages/bolt-connection/test/connection-provider/authentication-provider.test.js

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@
1616
* See the License for the specific language governing permissions and
1717
* limitations under the License.
1818
*/
19-
import { expirationBasedAuthTokenManager } from 'neo4j-driver-core'
19+
import { authTokenManagers } from 'neo4j-driver-core'
2020
import AuthenticationProvider from '../../src/connection-provider/authentication-provider'
2121

2222
describe('AuthenticationProvider', () => {
@@ -642,8 +642,15 @@ describe('AuthenticationProvider', () => {
642642
authenticationProvider
643643
} = createScenario()
644644

645+
const handleSecurityExceptionSpy = jest.spyOn(authenticationProvider._authTokenManager, 'handleSecurityException')
646+
645647
authenticationProvider.handleError({ code, connection })
646648

649+
if (code.startsWith('Neo.ClientError.Security.')) {
650+
expect(handleSecurityExceptionSpy).toBeCalledWith(connection.authToken, code)
651+
} else {
652+
expect(handleSecurityExceptionSpy).not.toBeCalled()
653+
}
647654
expect(authTokenProvider).not.toHaveBeenCalled()
648655
})
649656

@@ -785,7 +792,7 @@ describe('AuthenticationProvider', () => {
785792
})
786793

787794
function createAuthenticationProvider (authTokenProvider, mocks) {
788-
const authTokenManager = expirationBasedAuthTokenManager({ tokenProvider: authTokenProvider })
795+
const authTokenManager = authTokenManagers.bearer({ tokenProvider: authTokenProvider })
789796
const provider = new AuthenticationProvider({
790797
authTokenManager,
791798
userAgent: USER_AGENT,

packages/bolt-connection/test/connection-provider/connection-provider-direct.test.js

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@
2020
import DirectConnectionProvider from '../../src/connection-provider/connection-provider-direct'
2121
import { Pool } from '../../src/pool'
2222
import { Connection, DelegateConnection } from '../../src/connection'
23-
import { internal, newError, ServerInfo, staticAuthTokenManager, expirationBasedAuthTokenManager } from 'neo4j-driver-core'
23+
import { authTokenManagers, internal, newError, ServerInfo, staticAuthTokenManager } from 'neo4j-driver-core'
2424
import AuthenticationProvider from '../../src/connection-provider/authentication-provider'
2525
import { functional } from '../../src/lang'
2626

@@ -209,7 +209,7 @@ it('should call authenticationAuthProvider.handleError when TokenExpired happens
209209
it('should change error to retriable when error when TokenExpired happens and staticAuthTokenManager is not being used', async () => {
210210
const address = ServerAddress.fromUrl('localhost:123')
211211
const pool = newPool()
212-
const connectionProvider = newDirectConnectionProvider(address, pool, expirationBasedAuthTokenManager({ tokenProvider: () => null }))
212+
const connectionProvider = newDirectConnectionProvider(address, pool, authTokenManagers.bearer({ tokenProvider: () => null }))
213213

214214
const conn = await connectionProvider.acquireConnection({
215215
accessMode: 'READ',
@@ -246,6 +246,26 @@ it('should not change error to retriable when error when TokenExpired happens an
246246
expect(error.retriable).toBe(false)
247247
})
248248

249+
it('should not change error to retriable when error when TokenExpired happens and authTokenManagers.basic is being used', async () => {
250+
const address = ServerAddress.fromUrl('localhost:123')
251+
const pool = newPool()
252+
const connectionProvider = newDirectConnectionProvider(address, pool, authTokenManagers.basic({ tokenProvider: () => null }))
253+
254+
const conn = await connectionProvider.acquireConnection({
255+
accessMode: 'READ',
256+
database: ''
257+
})
258+
259+
const expectedError = newError(
260+
'Message',
261+
'Neo.ClientError.Security.TokenExpired'
262+
)
263+
264+
const error = conn.handleAndTransformError(expectedError, address)
265+
266+
expect(error.retriable).toBe(false)
267+
})
268+
249269
describe('constructor', () => {
250270
describe('newPool', () => {
251271
const server0 = ServerAddress.fromUrl('localhost:123')

packages/bolt-connection/test/connection-provider/connection-provider-routing.test.js

Lines changed: 51 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ import {
2626
internal,
2727
ServerInfo,
2828
staticAuthTokenManager,
29-
expirationBasedAuthTokenManager
29+
authTokenManagers
3030
} from 'neo4j-driver-core'
3131
import { RoutingTable } from '../../src/rediscovery/'
3232
import { Pool } from '../../src/pool'
@@ -1718,7 +1718,8 @@ describe.each([
17181718
],
17191719
pool
17201720
)
1721-
connectionProvider._authTokenManager = expirationBasedAuthTokenManager({ tokenProvider: () => null })
1721+
1722+
setupAuthTokenManager(connectionProvider, authTokenManagers.bearer({ tokenProvider: () => null }))
17221723

17231724
const error = newError(
17241725
'Message',
@@ -1756,8 +1757,51 @@ describe.each([
17561757
)
17571758
],
17581759
pool
1760+
1761+
)
1762+
1763+
setupAuthTokenManager(connectionProvider, staticAuthTokenManager({ authToken: null }))
1764+
1765+
const error = newError(
1766+
'Message',
1767+
'Neo.ClientError.Security.TokenExpired'
1768+
)
1769+
1770+
const server2Connection = await connectionProvider.acquireConnection({
1771+
accessMode: 'WRITE',
1772+
database: null,
1773+
impersonatedUser: user
1774+
})
1775+
1776+
const server3Connection = await connectionProvider.acquireConnection({
1777+
accessMode: 'READ',
1778+
database: null,
1779+
impersonatedUser: user
1780+
})
1781+
1782+
const error1 = server3Connection.handleAndTransformError(error, server3)
1783+
const error2 = server2Connection.handleAndTransformError(error, server2)
1784+
1785+
expect(error1.retriable).toBe(false)
1786+
expect(error2.retriable).toBe(false)
1787+
})
1788+
1789+
it.each(usersDataSet)('should not change error to retriable when error when TokenExpired happens and authTokenManagers.basic is being used [user=%s]', async (user) => {
1790+
const pool = newPool()
1791+
const connectionProvider = newRoutingConnectionProvider(
1792+
[
1793+
newRoutingTable(
1794+
null,
1795+
[server1, server2],
1796+
[server3, server2],
1797+
[server2, server4]
1798+
)
1799+
],
1800+
pool
1801+
17591802
)
1760-
connectionProvider._authTokenManager = staticAuthTokenManager({ authToken: null })
1803+
1804+
setupAuthTokenManager(connectionProvider, authTokenManagers.basic({ tokenProvider: () => {} }))
17611805

17621806
const error = newError(
17631807
'Message',
@@ -3944,3 +3988,7 @@ class FakeDnsResolver {
39443988
return Promise.resolve(this._addresses ? this._addresses : [seedRouter])
39453989
}
39463990
}
3991+
3992+
function setupAuthTokenManager (provider, authTokenManager) {
3993+
provider._authenticationProvider._authTokenManager = authTokenManager
3994+
}

0 commit comments

Comments
 (0)