Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 17 additions & 13 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -452,9 +452,9 @@ export class CatalogModule {}
The `AuthStrategy` interface declares four methods that together own the full session lifecycle:

- `authenticate(client: RestClient): Promise<void>` — performs the handshake; the `client` argument is a fully resilient `RestClient`, so auth requests reuse the same resilience policy stack as application calls.
- `isAuthenticated(): boolean` — gates whether the next request needs to re-authenticate.
- `isAuthenticated(): Promise<boolean>` — gates whether the next request needs to re-authenticate.
- `extendRequest(config: AxiosRequestConfig): AxiosRequestConfig` — applies the credentials to a request config (must not mutate the input).
- `invalidate(): void` — drops the current session so the next request triggers a fresh handshake; called by `AuthRestClient` after a 401.
- `invalidate(): Promise<void>` — drops the current session so the next request triggers a fresh handshake; called by `AuthRestClient` after a 401.

```ts
import {
Expand Down Expand Up @@ -489,7 +489,7 @@ class BearerTokenStrategy implements AuthStrategy {
this.expiresAt = Date.now() + response.data.expires_in * 1_000 - 60_000
}

isAuthenticated(): boolean {
async isAuthenticated(): Promise<boolean> {
return this.token !== undefined && Date.now() < this.expiresAt
}

Expand All @@ -500,7 +500,7 @@ class BearerTokenStrategy implements AuthStrategy {
}
}

invalidate(): void {
async invalidate(): Promise<void> {
this.token = undefined
this.expiresAt = 0
}
Expand Down Expand Up @@ -654,10 +654,10 @@ new AuthProcessor(strategy: AuthStrategy, client: RestClient)

Public methods:

- `isAuthenticated(): boolean` — pure delegation to `strategy.isAuthenticated()`. The processor caches nothing; the strategy is the single source of truth.
- `authenticateIfNeeded(): Promise<void>` — short-circuits when `isAuthenticated()` returns `true`; otherwise triggers a fresh `strategy.authenticate(client)` handshake. Concurrent callers share a single in-flight handshake (single-flight via `@DeduplicateInflight`).
- `isAuthenticated(): Promise<boolean>` — awaited delegation to `strategy.isAuthenticated()`. The processor caches nothing; the strategy is the single source of truth.
- `authenticateIfNeeded(): Promise<void>` — short-circuits when `await isAuthenticated()` resolves with `true`; otherwise triggers a fresh `strategy.authenticate(client)` handshake. Concurrent callers share a single in-flight handshake (single-flight via `@DeduplicateInflight`).
- `extendRequest(config: AxiosRequestConfig): AxiosRequestConfig` — pure delegation to `strategy.extendRequest(config)`. The strategy contract forbids mutating the input.
- `clearAuth(): void` — pure delegation to `strategy.invalidate()`. After this call, `isAuthenticated()` returns `false` and the next `authenticateIfNeeded()` triggers a fresh handshake. Used by `AuthRestClient`'s 401 retry path.
- `clearAuth(): Promise<void>` — pure delegation to `strategy.invalidate()`, `await isAuthenticated()` resolves with `false` and the next `authenticateIfNeeded()` triggers a fresh handshake. Used by `AuthRestClient`'s 401 retry path.

#### `AuthRestModule`

Expand Down Expand Up @@ -748,11 +748,13 @@ interface AuthStrategy {
authenticate(client: RestClient): Promise<void>

/**
* Returns `true` while the current credentials are still considered valid.
* Must return `false` when no session has been established yet or after
* `invalidate()` has been called.
* Resolves with `true` while the current credentials are still considered
* valid. Must resolve with `false` when no session has been established yet
* or after `invalidate()` has been called. Asynchronous so implementations
* can consult persisted credential stores or remote token-introspection
* endpoints without blocking the dispatch path on synchronous I/O.
*/
isAuthenticated(): boolean
isAuthenticated(): Promise<boolean>

/**
* Returns a NEW `AxiosRequestConfig` with authentication material applied.
Expand All @@ -762,10 +764,12 @@ interface AuthStrategy {

/**
* Drops the current session so the next request triggers a fresh
* `authenticate()` call. Invoked by `AuthRestClient` after the upstream
* `authenticate()` call. Resolves once the session has been dropped, so
* implementations may flush persisted credentials or await a remote
* sign-out endpoint. Invoked by `AuthRestClient` after the upstream
* service rejects a request with HTTP 401.
*/
invalidate(): void
invalidate(): Promise<void>
}
```

Expand Down
82 changes: 58 additions & 24 deletions src/auth/__tests__/auth-processor.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,9 @@ const delay = (ms: number): Promise<void> =>
*/
interface AuthStrategyStub extends AuthStrategy {
authenticate: jest.Mock<Promise<void>, [RestClient]>;
isAuthenticated: jest.Mock<boolean, []>;
isAuthenticated: jest.Mock<Promise<boolean>, []>;
extendRequest: jest.Mock<AxiosRequestConfig, [AxiosRequestConfig]>;
invalidate: jest.Mock<void, []>;
invalidate: jest.Mock<Promise<void>, []>;
}

/**
Expand All @@ -33,7 +33,7 @@ interface AuthStrategyStub extends AuthStrategy {
function createStrategyStub(): AuthStrategyStub {
return {
authenticate: jest.fn().mockResolvedValue(undefined),
isAuthenticated: jest.fn().mockReturnValue(true),
isAuthenticated: jest.fn().mockResolvedValue(true),
extendRequest: jest.fn(
(config: AxiosRequestConfig): AxiosRequestConfig => ({
...config,
Expand All @@ -43,7 +43,7 @@ function createStrategyStub(): AuthStrategyStub {
},
}),
),
invalidate: jest.fn(),
invalidate: jest.fn().mockResolvedValue(undefined),
};
}

Expand All @@ -56,39 +56,73 @@ const fakeClient = { iAm: "the-rest-client" } as unknown as RestClient;

describe("AuthProcessor", () => {
describe("isAuthenticated()", () => {
it("returns true when the strategy reports authenticated", () => {
it("resolves with true when the strategy reports authenticated", async () => {
const strategy = createStrategyStub();
strategy.isAuthenticated.mockReturnValue(true);
strategy.isAuthenticated.mockResolvedValue(true);
const processor = new AuthProcessor(strategy, fakeClient);

expect(processor.isAuthenticated()).toBe(true);
await expect(processor.isAuthenticated()).resolves.toBe(true);
expect(strategy.isAuthenticated).toHaveBeenCalledTimes(1);
});

it("returns false when the strategy reports unauthenticated", () => {
it("resolves with false when the strategy reports unauthenticated", async () => {
const strategy = createStrategyStub();
strategy.isAuthenticated.mockReturnValue(false);
strategy.isAuthenticated.mockResolvedValue(false);
const processor = new AuthProcessor(strategy, fakeClient);

expect(processor.isAuthenticated()).toBe(false);
await expect(processor.isAuthenticated()).resolves.toBe(false);
expect(strategy.isAuthenticated).toHaveBeenCalledTimes(1);
});

it("awaits the strategy's async isAuthenticated before resolving (deferred Promise ordering)", async () => {
// Pins the awaited-delegation invariant: `AuthProcessor.isAuthenticated`
// MUST `await this.strategy.isAuthenticated()` rather than returning the
// raw Promise unchanged or eagerly resolving. A regression that dropped
// the await would still type-check (the method would simply return
// `Promise<Promise<boolean>>` flattened by the runtime). This test gates
// the ordering with a deferred Promise that only resolves when the test
// explicitly releases it.
const strategy = createStrategyStub();
let resolveIsAuth!: (value: boolean) => void;
const deferred = new Promise<boolean>((resolve) => {
resolveIsAuth = resolve;
});
strategy.isAuthenticated.mockReturnValueOnce(deferred);
const processor = new AuthProcessor(strategy, fakeClient);

let settled = false;
const checkPromise = processor.isAuthenticated().then((value) => {
settled = true;
return value;
});

// Drain microtasks/macrotasks — the inner Promise is still pending, so
// the outer Promise must NOT have settled yet. A regression that
// bypassed the await (e.g., returning a stale cached value) would have
// resolved before the test released the deferred.
await new Promise<void>((resolve) => setImmediate(resolve));
expect(settled).toBe(false);

resolveIsAuth(true);
await expect(checkPromise).resolves.toBe(true);
expect(settled).toBe(true);
});
});

describe("authenticateIfNeeded()", () => {
it("skips strategy.authenticate when strategy.isAuthenticated() returns true", async () => {
it("skips strategy.authenticate when strategy.isAuthenticated() resolves with true", async () => {
const strategy = createStrategyStub();
strategy.isAuthenticated.mockReturnValue(true);
strategy.isAuthenticated.mockResolvedValue(true);
const processor = new AuthProcessor(strategy, fakeClient);

await processor.authenticateIfNeeded();

expect(strategy.authenticate).not.toHaveBeenCalled();
});

it("calls strategy.authenticate when strategy.isAuthenticated() returns false", async () => {
it("calls strategy.authenticate when strategy.isAuthenticated() resolves with false", async () => {
const strategy = createStrategyStub();
strategy.isAuthenticated.mockReturnValue(false);
strategy.isAuthenticated.mockResolvedValue(false);
const processor = new AuthProcessor(strategy, fakeClient);

await processor.authenticateIfNeeded();
Expand All @@ -98,7 +132,7 @@ describe("AuthProcessor", () => {

it("forwards the configured client to strategy.authenticate(client)", async () => {
const strategy = createStrategyStub();
strategy.isAuthenticated.mockReturnValue(false);
strategy.isAuthenticated.mockResolvedValue(false);
const processor = new AuthProcessor(strategy, fakeClient);

await processor.authenticateIfNeeded();
Expand All @@ -108,7 +142,7 @@ describe("AuthProcessor", () => {

it("coalesces concurrent first-time callers into exactly ONE strategy.authenticate invocation (single-flight)", async () => {
const strategy = createStrategyStub();
strategy.isAuthenticated.mockReturnValue(false);
strategy.isAuthenticated.mockResolvedValue(false);
strategy.authenticate.mockImplementation(async () => {
// 50ms delay guarantees that all concurrent callers land while
// the first call is still in flight, so @DeduplicateInflight must
Expand All @@ -132,7 +166,7 @@ describe("AuthProcessor", () => {

it("clears the inflight entry after the awaited promise resolves", async () => {
const strategy = createStrategyStub();
strategy.isAuthenticated.mockReturnValue(false);
strategy.isAuthenticated.mockResolvedValue(false);
const processor = new AuthProcessor(strategy, fakeClient);

await processor.authenticateIfNeeded();
Expand Down Expand Up @@ -170,11 +204,11 @@ describe("AuthProcessor", () => {
});

describe("clearAuth()", () => {
it("calls strategy.invalidate() exactly once", () => {
it("calls strategy.invalidate() exactly once", async () => {
const strategy = createStrategyStub();
const processor = new AuthProcessor(strategy, fakeClient);

processor.clearAuth();
await processor.clearAuth();

expect(strategy.invalidate).toHaveBeenCalledTimes(1);
});
Expand All @@ -184,8 +218,8 @@ describe("AuthProcessor", () => {
// Model a strategy whose `invalidate()` flips `isAuthenticated()` to false,
// mirroring the contract documented on AuthStrategy.invalidate.
let authenticated = true;
strategy.isAuthenticated.mockImplementation(() => authenticated);
strategy.invalidate.mockImplementation(() => {
strategy.isAuthenticated.mockImplementation(async () => authenticated);
strategy.invalidate.mockImplementation(async () => {
authenticated = false;
});
strategy.authenticate.mockImplementation(async () => {
Expand All @@ -198,15 +232,15 @@ describe("AuthProcessor", () => {
expect(strategy.authenticate).not.toHaveBeenCalled();

// Invalidate the session — strategy now reports unauthenticated.
processor.clearAuth();
await processor.clearAuth();
expect(strategy.invalidate).toHaveBeenCalledTimes(1);
expect(processor.isAuthenticated()).toBe(false);
await expect(processor.isAuthenticated()).resolves.toBe(false);

// Next pre-flight must trigger a fresh handshake forwarded with the client.
await processor.authenticateIfNeeded();
expect(strategy.authenticate).toHaveBeenCalledTimes(1);
expect(strategy.authenticate).toHaveBeenCalledWith(fakeClient);
expect(processor.isAuthenticated()).toBe(true);
await expect(processor.isAuthenticated()).resolves.toBe(true);
});
});
});
58 changes: 56 additions & 2 deletions src/auth/__tests__/auth-rest.client.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ function createRestClientStub(response: AxiosResponse = { data: 'ok' } as AxiosR
interface AuthStrategyStub {
authenticateIfNeeded: jest.Mock<Promise<void>, []>
extendRequest: jest.Mock<AxiosRequestConfig, [AxiosRequestConfig]>
clearAuth: jest.Mock<void, []>
clearAuth: jest.Mock<Promise<void>, []>
}

/**
Expand All @@ -87,7 +87,7 @@ function createAuthStrategyStub(): AuthStrategyStub {
},
}),
),
clearAuth: jest.fn(),
clearAuth: jest.fn().mockResolvedValue(undefined),
}
}

Expand Down Expand Up @@ -216,6 +216,60 @@ describe('AuthRestClient', () => {
expect(authStrategy.authenticateIfNeeded).toHaveBeenCalledTimes(2)
expect(authStrategy.clearAuth).toHaveBeenCalledTimes(1)
})

it('awaits clearAuth before invoking authenticateIfNeeded on the 401 recovery path', async () => {
// Pins the awaited-ordering invariant on the 401 recovery branch:
// `AuthRestClient.dispatch` MUST `await processor.clearAuth()` BEFORE
// calling `authenticateIfNeeded` for the re-handshake. A regression that
// drops the `await` would still type-check (both return Promise<void>)
// and could pass call-count assertions because nothing else pins the
// resolution timing — so this test gates the ordering with a deferred
// Promise that only resolves when the test explicitly releases it.
const { client, restClient, authStrategy } = buildSut()

restClient.get
.mockRejectedValueOnce(makeAxiosError(401))
.mockResolvedValueOnce({ data: 'ok' } as AxiosResponse)

// Manually-resolved deferred so `clearAuth` stays pending until the
// test releases it. The pre-flight authenticateIfNeeded fires once
// BEFORE the 401, so we snapshot that baseline call count and assert
// the recovery handshake has not yet been triggered.
let resolveClearAuth!: () => void
const clearAuthDeferred = new Promise<void>((resolve) => {
resolveClearAuth = resolve
})
authStrategy.clearAuth.mockImplementationOnce(() => clearAuthDeferred)

const dispatchPromise = client.get('/x')

// Drain the microtask + macrotask queues so the dispatcher reaches
// `clearAuth` and suspends on the deferred. A `setImmediate` boundary
// is required because the pre-flight `authenticateIfNeeded` await, the
// first `restClient.get` rejection, and the error-branch `await` form
// a chain of microtasks that all need to flush before `clearAuth` is
// actually invoked.
await new Promise<void>(resolve => setImmediate(resolve))

// Pre-flight handshake ran once; recovery handshake MUST NOT have
// fired while `clearAuth` is still pending. A dropped `await` on
// `clearAuth` would have allowed authenticateIfNeeded to run already.
expect(authStrategy.clearAuth).toHaveBeenCalledTimes(1)
expect(authStrategy.authenticateIfNeeded).toHaveBeenCalledTimes(1)

// Release the deferred — the dispatcher should now proceed to the
// recovery handshake and the second underlying RestClient call.
resolveClearAuth()

const response = await dispatchPromise

expect(response.data).toBe('ok')
// Recovery handshake fired exactly once AFTER clearAuth resolved,
// bringing the total authenticateIfNeeded invocations to two.
expect(authStrategy.authenticateIfNeeded).toHaveBeenCalledTimes(2)
expect(authStrategy.clearAuth).toHaveBeenCalledTimes(1)
expect(restClient.get).toHaveBeenCalledTimes(2)
})
})

describe('500 error path', () => {
Expand Down
12 changes: 6 additions & 6 deletions src/auth/__tests__/auth-rest.module.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,9 +68,9 @@ class StrategyWithDeps implements AuthStrategy {
constructor(@Inject(SENTINEL_TOKEN) public readonly sentinel: string) {}

async authenticate(_client: RestClient): Promise<void> {}
isAuthenticated(): boolean { return true }
async isAuthenticated(): Promise<boolean> { return true }
extendRequest(config: AxiosRequestConfig): AxiosRequestConfig { return config }
invalidate(): void {}
async invalidate(): Promise<void> {}
}

// Wraps the sentinel in its own NestJS module so it can be imported into
Expand Down Expand Up @@ -123,15 +123,15 @@ class StubAuthStrategy implements AuthStrategy {
// no-op; the module spec does not exercise auth flows
}

isAuthenticated(): boolean {
async isAuthenticated(): Promise<boolean> {
return true
}

extendRequest(config: AxiosRequestConfig): AxiosRequestConfig {
return config
}

invalidate(): void {
async invalidate(): Promise<void> {
// no-op; the module spec does not exercise invalidation
}
}
Expand All @@ -147,7 +147,7 @@ class StubAuthStrategy implements AuthStrategy {
class StampingAuthStrategy implements AuthStrategy {
async authenticate(_client: RestClient): Promise<void> {}

isAuthenticated(): boolean {
async isAuthenticated(): Promise<boolean> {
return true
}

Expand All @@ -158,7 +158,7 @@ class StampingAuthStrategy implements AuthStrategy {
}
}

invalidate(): void {}
async invalidate(): Promise<void> {}
}

const successResponse: AxiosResponse = {
Expand Down
Loading
Loading