Skip to content

Commit 1fbe884

Browse files
bobymicrobybobymicroby
committed
feat(auth): add EntraId integration tests
- Add integration tests for token renewal and re-authentication flows - Update credentials provider to use uniqueId as username instead of account username - Add test utilities for loading Redis endpoint configurations - Split TypeScript configs into separate files for samples and integration tests
1 parent ac972bd commit 1fbe884

File tree

15 files changed

+545
-116
lines changed

15 files changed

+545
-116
lines changed

‎packages/authx/lib/token-manager.spec.ts‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -328,7 +328,7 @@ describe('TokenManager', () => {
328328
assert.equal(listener.receivedTokens.length, 1, 'Should not receive new token after failure');
329329
assert.equal(listener.errors.length, 1, 'Should receive error');
330330
assert.equal(listener.errors[0].message, 'Fatal error', 'Should have correct error message');
331-
assert.equal(listener.errors[0].isFatal, true, 'Should be a fatal error');
331+
assert.equal(listener.errors[0].isRetryable, false, 'Should be a fatal error');
332332

333333
// verify that the token manager is stopped and no more requests are made after the error and expected refresh time
334334
await delay(80);
@@ -352,7 +352,7 @@ describe('TokenManager', () => {
352352
initialDelayMs: 100,
353353
maxDelayMs: 1000,
354354
backoffMultiplier: 2,
355-
shouldRetry: (error: unknown) => error instanceof Error && error.message === 'Temporary failure'
355+
isRetryable: (error: unknown) => error instanceof Error && error.message === 'Temporary failure'
356356
}
357357
};
358358

@@ -389,7 +389,7 @@ describe('TokenManager', () => {
389389
// Should have first error but not stop due to retry config
390390
assert.equal(listener.errors.length, 1, 'Should have first error');
391391
assert.ok(listener.errors[0].message.includes('attempt 1'), 'Error should indicate first attempt');
392-
assert.equal(listener.errors[0].isFatal, false, 'Should not be a fatal error');
392+
assert.equal(listener.errors[0].isRetryable, true, 'Should not be a fatal error');
393393
assert.equal(manager.isRunning(), true, 'Should continue running during retries');
394394

395395
// Advance past first retry (delay: 100ms due to backoff)
@@ -401,7 +401,7 @@ describe('TokenManager', () => {
401401

402402
assert.equal(listener.errors.length, 2, 'Should have second error');
403403
assert.ok(listener.errors[1].message.includes('attempt 2'), 'Error should indicate second attempt');
404-
assert.equal(listener.errors[0].isFatal, false, 'Should not be a fatal error');
404+
assert.equal(listener.errors[0].isRetryable, true, 'Should not be a fatal error');
405405
assert.equal(manager.isRunning(), true, 'Should continue running during retries');
406406

407407
// Advance past second retry (delay: 200ms due to backoff)
@@ -435,7 +435,7 @@ describe('TokenManager', () => {
435435
maxDelayMs: 1000,
436436
backoffMultiplier: 2,
437437
jitterPercentage: 0,
438-
shouldRetry: (error: unknown) => error instanceof Error && error.message === 'Temporary failure'
438+
isRetryable: (error: unknown) => error instanceof Error && error.message === 'Temporary failure'
439439
}
440440
};
441441

@@ -470,7 +470,7 @@ describe('TokenManager', () => {
470470
// First error
471471
assert.equal(listener.errors.length, 1, 'Should have first error');
472472
assert.equal(manager.isRunning(), true, 'Should continue running after first error');
473-
assert.equal(listener.errors[0].isFatal, false, 'Should not be a fatal error');
473+
assert.equal(listener.errors[0].isRetryable, true, 'Should not be a fatal error');
474474

475475
// Advance past first retry
476476
await delay(100);
@@ -483,7 +483,7 @@ describe('TokenManager', () => {
483483
// Second error
484484
assert.equal(listener.errors.length, 2, 'Should have second error');
485485
assert.equal(manager.isRunning(), true, 'Should continue running after second error');
486-
assert.equal(listener.errors[1].isFatal, false, 'Should not be a fatal error');
486+
assert.equal(listener.errors[1].isRetryable, true, 'Should not be a fatal error');
487487

488488
// Advance past second retry
489489
await delay(200);
@@ -495,7 +495,7 @@ describe('TokenManager', () => {
495495

496496
// Should stop after max retries
497497
assert.equal(listener.errors.length, 3, 'Should have final error');
498-
assert.equal(listener.errors[2].isFatal, true, 'Should not be a fatal error');
498+
assert.equal(listener.errors[2].isRetryable, false, 'Should be a fatal error');
499499
assert.equal(manager.isRunning(), false, 'Should stop after max retries exceeded');
500500
assert.equal(identityProvider.getRequestCount(), 4, 'Should have made exactly 4 requests');
501501

‎packages/authx/lib/token-manager.ts‎

Lines changed: 70 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -5,18 +5,70 @@ import { Token } from './token';
55
* The configuration for retrying token refreshes.
66
*/
77
export interface RetryPolicy {
8-
// The maximum number of attempts to retry token refreshes.
8+
/**
9+
* The maximum number of attempts to retry token refreshes.
10+
*/
911
maxAttempts: number;
10-
// The initial delay in milliseconds before the first retry.
12+
13+
/**
14+
* The initial delay in milliseconds before the first retry.
15+
*/
1116
initialDelayMs: number;
12-
// The maximum delay in milliseconds between retries (the calculated delay will be capped at this value).
17+
18+
/**
19+
* The maximum delay in milliseconds between retries.
20+
* The calculated delay will be capped at this value.
21+
*/
1322
maxDelayMs: number;
14-
// The multiplier for exponential backoff between retries. e.g. 2 will double the delay each time.
23+
24+
/**
25+
* The multiplier for exponential backoff between retries.
26+
* @example
27+
* A value of 2 will double the delay each time:
28+
* - 1st retry: initialDelayMs
29+
* - 2nd retry: initialDelayMs * 2
30+
* - 3rd retry: initialDelayMs * 4
31+
*/
1532
backoffMultiplier: number;
16-
// The percentage of jitter to apply to the delay. e.g. 0.1 will add or subtract up to 10% of the delay.
33+
34+
/**
35+
* The percentage of jitter to apply to the delay.
36+
* @example
37+
* A value of 0.1 will add or subtract up to 10% of the delay.
38+
*/
1739
jitterPercentage?: number;
18-
// A custom function to determine if a retry should be attempted based on the error and attempt number.
19-
shouldRetry?: (error: unknown, attempt: number) => boolean;
40+
41+
/**
42+
* Function to classify errors from the identity provider as retryable or non-retryable.
43+
* Used to determine if a token refresh failure should be retried based on the type of error.
44+
*
45+
* The default behavior is to retry all types of errors if no function is provided.
46+
*
47+
* Common use cases:
48+
* - Network errors that may be transient (should retry)
49+
* - Invalid credentials (should not retry)
50+
* - Rate limiting responses (should retry)
51+
*
52+
* @param error - The error from the identity provider3
53+
* @param attempt - Current retry attempt (0-based)
54+
* @returns `true` if the error is considered transient and the operation should be retried
55+
*
56+
* @example
57+
* ```typescript
58+
* const retryPolicy: RetryPolicy = {
59+
* maxAttempts: 3,
60+
* initialDelayMs: 1000,
61+
* maxDelayMs: 5000,
62+
* backoffMultiplier: 2,
63+
* isRetryable: (error) => {
64+
* // Retry on network errors or rate limiting
65+
* return error instanceof NetworkError ||
66+
* error instanceof RateLimitError;
67+
* }
68+
* };
69+
* ```
70+
*/
71+
isRetryable?: (error: unknown, attempt: number) => boolean;
2072
}
2173

2274
/**
@@ -36,14 +88,13 @@ export interface TokenManagerConfig {
3688
}
3789

3890
/**
39-
* IDPError is an error that occurs while calling the underlying IdentityProvider.
91+
* IDPError indicates a failure from the identity provider.
4092
*
41-
* It can be transient and if retry policy is configured, the token manager will attempt to obtain a token again.
42-
* This means that receiving non-fatal error is not a stream termination event.
43-
* The stream will be terminated only if the error is fatal.
93+
* The `isRetryable` flag is determined by the RetryPolicy's error classification function - if an error is
94+
* classified as retryable, it will be marked as transient and the token manager will attempt to recover.
4495
*/
4596
export class IDPError extends Error {
46-
constructor(public readonly message: string, public readonly isFatal: boolean) {
97+
constructor(public readonly message: string, public readonly isRetryable: boolean) {
4798
super(message);
4899
this.name = 'IDPError';
49100
}
@@ -105,7 +156,6 @@ export class TokenManager<T> {
105156
*/
106157
public start(listener: TokenStreamListener<T>, initialDelayMs: number = 0): Disposable {
107158
if (this.listener) {
108-
console.log('TokenManager is already running, stopping the previous instance');
109159
this.stop();
110160
}
111161

@@ -142,14 +192,14 @@ export class TokenManager<T> {
142192
private shouldRetry(error: unknown): boolean {
143193
if (!this.config.retry) return false;
144194

145-
const { maxAttempts, shouldRetry } = this.config.retry;
195+
const { maxAttempts, isRetryable } = this.config.retry;
146196

147197
if (this.retryAttempt >= maxAttempts) {
148198
return false;
149199
}
150200

151-
if (shouldRetry) {
152-
return shouldRetry(error, this.retryAttempt);
201+
if (isRetryable) {
202+
return isRetryable(error, this.retryAttempt);
153203
}
154204

155205
return false;
@@ -172,10 +222,10 @@ export class TokenManager<T> {
172222
if (this.shouldRetry(error)) {
173223
this.retryAttempt++;
174224
const retryDelay = this.calculateRetryDelay();
175-
this.notifyError(`Token refresh failed (attempt ${this.retryAttempt}), retrying in ${retryDelay}ms: ${error}`, false)
225+
this.notifyError(`Token refresh failed (attempt ${this.retryAttempt}), retrying in ${retryDelay}ms: ${error}`, true)
176226
this.scheduleNextRefresh(retryDelay);
177227
} else {
178-
this.notifyError(error, true);
228+
this.notifyError(error, false);
179229
this.stop();
180230
}
181231
}
@@ -255,13 +305,13 @@ export class TokenManager<T> {
255305
return this.currentToken;
256306
}
257307

258-
private notifyError = (error: unknown, isFatal: boolean): void => {
308+
private notifyError(error: unknown, isRetryable: boolean): void {
259309
const errorMessage = error instanceof Error ? error.message : String(error);
260310

261311
if (!this.listener) {
262312
throw new Error(`TokenManager is not running but received an error: ${errorMessage}`);
263313
}
264314

265-
this.listener.onError(new IDPError(errorMessage, isFatal));
315+
this.listener.onError(new IDPError(errorMessage, isRetryable));
266316
}
267317
}

‎packages/client/lib/client/index.ts‎

Lines changed: 16 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -303,7 +303,7 @@ export default class RedisClient<
303303
#epoch: number;
304304
#watchEpoch?: number;
305305

306-
private credentialsSubscription: Disposable | null = null;
306+
#credentialsSubscription: Disposable | null = null;
307307

308308
get options(): RedisClientOptions<M, F, S, RESP> | undefined {
309309
return this._self.#options;
@@ -382,7 +382,7 @@ export default class RedisClient<
382382
/**
383383
* @param credentials
384384
*/
385-
private reAuthenticate = async (credentials: BasicAuth) => {
385+
#reAuthenticate = async (credentials: BasicAuth) => {
386386
// Re-authentication is not supported on RESP2 with PubSub active
387387
if (!(this.isPubSubActive && !this.#options?.RESP)) {
388388
await this.sendCommand(
@@ -394,19 +394,17 @@ export default class RedisClient<
394394
}
395395
}
396396

397-
private subscribeForStreamingCredentials(cp: StreamingCredentialsProvider): Promise<[BasicAuth, Disposable]> {
397+
#subscribeForStreamingCredentials(cp: StreamingCredentialsProvider): Promise<[BasicAuth, Disposable]> {
398398
return cp.subscribe({
399399
onNext: credentials => {
400-
this.reAuthenticate(credentials).catch(error => {
400+
this.#reAuthenticate(credentials).catch(error => {
401401
const errorMessage = error instanceof Error ? error.message : String(error);
402-
console.error('Error during re-authentication', errorMessage);
403402
cp.onReAuthenticationError(new CredentialsError(errorMessage));
404403
});
405404

406405
},
407406
onError: (e: Error) => {
408407
const errorMessage = `Error from streaming credentials provider: ${e.message}`;
409-
console.error(errorMessage);
410408
cp.onReAuthenticationError(new UnableToObtainNewCredentialsError(errorMessage));
411409
}
412410
});
@@ -431,8 +429,8 @@ export default class RedisClient<
431429

432430
if (cp && cp.type === 'streaming-credentials-provider') {
433431

434-
const [credentials, disposable] = await this.subscribeForStreamingCredentials(cp)
435-
this.credentialsSubscription = disposable;
432+
const [credentials, disposable] = await this.#subscribeForStreamingCredentials(cp)
433+
this.#credentialsSubscription = disposable;
436434

437435
if (credentials.password) {
438436
hello.AUTH = {
@@ -467,8 +465,8 @@ export default class RedisClient<
467465

468466
if (cp && cp.type === 'streaming-credentials-provider') {
469467

470-
const [credentials, disposable] = await this.subscribeForStreamingCredentials(cp)
471-
this.credentialsSubscription = disposable;
468+
const [credentials, disposable] = await this.#subscribeForStreamingCredentials(cp)
469+
this.#credentialsSubscription = disposable;
472470

473471
if (credentials.username || credentials.password) {
474472
commands.push(
@@ -1105,8 +1103,8 @@ export default class RedisClient<
11051103
const chainId = Symbol('Reset Chain'),
11061104
promises = [this._self.#queue.reset(chainId)],
11071105
selectedDB = this._self.#options?.database ?? 0;
1108-
this.credentialsSubscription?.[Symbol.dispose]();
1109-
this.credentialsSubscription = null;
1106+
this._self.#credentialsSubscription?.[Symbol.dispose]();
1107+
this._self.#credentialsSubscription = null;
11101108
for (const command of (await this._self.#handshake(selectedDB))) {
11111109
promises.push(
11121110
this._self.#queue.addCommand(command, {
@@ -1158,8 +1156,8 @@ export default class RedisClient<
11581156
* @deprecated use .close instead
11591157
*/
11601158
QUIT(): Promise<string> {
1161-
this.credentialsSubscription?.[Symbol.dispose]();
1162-
this.credentialsSubscription = null;
1159+
this._self.#credentialsSubscription?.[Symbol.dispose]();
1160+
this._self.#credentialsSubscription = null;
11631161
return this._self.#socket.quit(async () => {
11641162
clearTimeout(this._self.#pingTimer);
11651163
const quitPromise = this._self.#queue.addCommand<string>(['QUIT']);
@@ -1198,8 +1196,8 @@ export default class RedisClient<
11981196
resolve();
11991197
};
12001198
this._self.#socket.on('data', maybeClose);
1201-
this.credentialsSubscription?.[Symbol.dispose]();
1202-
this.credentialsSubscription = null;
1199+
this._self.#credentialsSubscription?.[Symbol.dispose]();
1200+
this._self.#credentialsSubscription = null;
12031201
});
12041202
}
12051203

@@ -1210,8 +1208,8 @@ export default class RedisClient<
12101208
clearTimeout(this._self.#pingTimer);
12111209
this._self.#queue.flushAll(new DisconnectsClientError());
12121210
this._self.#socket.destroy();
1213-
this.credentialsSubscription?.[Symbol.dispose]();
1214-
this.credentialsSubscription = null;
1211+
this._self.#credentialsSubscription?.[Symbol.dispose]();
1212+
this._self.#credentialsSubscription = null;
12151213
}
12161214

12171215
ref() {

0 commit comments

Comments
 (0)