From 7f5c9f681be053f5d14ee7a97955e4475c9bd6e5 Mon Sep 17 00:00:00 2001 From: rdlabo Date: Sun, 26 Jul 2026 09:45:13 +0900 Subject: [PATCH] fix: classify authentication access denial --- projects/kit/README.md | 18 ++--- .../src/lib/http/kit-http.interceptor.spec.ts | 80 +++++++++++++++++++ .../kit/src/lib/http/kit-http.interceptor.ts | 41 +++++++++- 3 files changed, 127 insertions(+), 12 deletions(-) diff --git a/projects/kit/README.md b/projects/kit/README.md index d447b70..b675540 100644 --- a/projects/kit/README.md +++ b/projects/kit/README.md @@ -416,9 +416,7 @@ provideKitAuth(() => ({ exchange: async (ctx) => { const remote = await auth.exchangeCredential(ctx); const authSubject = auth.currentSubject(); - return remote && authSubject - ? { ...remote, authSubject } - : false; + return remote && authSubject ? { ...remote, authSubject } : false; }, currentAuthSubject: () => auth.currentSubject(), isUnavailableError: isOfflineFallbackError, @@ -647,6 +645,9 @@ export const appConfig: ApplicationConfig = { // Required for the new offline auth boundary. Kept opt-in so existing applications retain // their current interceptor behavior until they wire KitAuthAccessService. enforceAuthAccessMode: true, + // Optional. Use this only when the API distinguishes invalid identity (401) from + // authenticated resource permission (403). Omission keeps the legacy 401/403 revoke policy. + isAuthAccessDenial: (_req, error) => error.status === 401, getAuthHeaders: async (req) => ({ Authorization: `Bearer ${await auth.getToken()}`, }), @@ -686,8 +687,9 @@ because that also skips authentication headers and error handling, and do not gl **Error dispatch** (after retries, in `catchError`): -1. With `enforceAuthAccessMode`, `401` / `403` → revoke access, notify the matching hook, and reject - without consulting `offlineFallback` +1. With `enforceAuthAccessMode`, `401` / `403` never consults `offlineFallback`. The matching hook is + notified and the error is rejected. Shared access is revoked only when `isAuthAccessDenial` + returns `true`; omitting it preserves the legacy behavior that revokes on both 401 and 403. 2. Otherwise, `offlineFallback` non-null → return fallback observable (no further hooks called) 3. `401` → `onUnauthorized` · `403` → `onForbidden` 4. `0` (connected) → `onNetworkError` · `429` → `onRateLimited(retryAfter?)` · `502/503/504` → `onServerBusy(status, retryAfter?)` @@ -963,11 +965,7 @@ export class AuthService { // request and pass it as expectedUser; the kit checks object identity after `before` completes // and immediately before invoking Firebase signOut. signOutDeniedSession(expectedUser: User) { - return kitSignOut( - this.#auth, - { before: () => this.clearDeniedSessionTransport() }, - { expectedUser }, - ); + return kitSignOut(this.#auth, { before: () => this.clearDeniedSessionTransport() }, { expectedUser }); } } ``` diff --git a/projects/kit/src/lib/http/kit-http.interceptor.spec.ts b/projects/kit/src/lib/http/kit-http.interceptor.spec.ts index 7149194..3bb1f28 100644 --- a/projects/kit/src/lib/http/kit-http.interceptor.spec.ts +++ b/projects/kit/src/lib/http/kit-http.interceptor.spec.ts @@ -261,6 +261,86 @@ describe('kitAuthInterceptor', () => { expect(config.onAuthError).toHaveBeenCalledWith(baseReq, denial); expect(next).not.toHaveBeenCalled(); }); + + it('keeps remote access and reports only onAuthError when a header 403 is not an access denial', async () => { + const denial = { status: 403 }; + const config = makeConfig({ + enforceAuthAccessMode: true, + getAuthHeaders: vi.fn().mockRejectedValue(denial), + isAuthAccessDenial: vi.fn(() => false), + onAuthError: vi.fn(), + }); + setupInterceptor(config); + const access = TestBed.inject(KitAuthAccessService); + access.grantRemote(); + const next = vi.fn(); + + await expect(firstValueFrom(runInterceptor(baseReq, next))).rejects.toBe(denial); + + expect(access.mode).toBe('remote'); + expect(config.onAuthError).toHaveBeenCalledWith(baseReq, denial); + expect(config.onUnauthorized).not.toHaveBeenCalled(); + expect(config.onForbidden).not.toHaveBeenCalled(); + expect(next).not.toHaveBeenCalled(); + }); + + it('403 calls onForbidden without revoking remote access when isAuthAccessDenial accepts only 401', async () => { + const fallbackResponse = new HttpResponse({ status: 200, body: 'cached' }); + const error403 = new HttpErrorResponse({ status: 403 }); + const config = makeConfig({ + enforceAuthAccessMode: true, + offlineFallback: vi.fn().mockReturnValue(of(fallbackResponse)), + isAuthAccessDenial: vi.fn((_req, error) => (error as HttpErrorResponse).status === 401), + }); + setupInterceptor(config); + const access = TestBed.inject(KitAuthAccessService); + access.grantRemote(); + const next = vi.fn().mockReturnValue(throwError(() => error403)); + + await expect(firstValueFrom(runInterceptor(baseReq, next))).rejects.toBe(error403); + + expect(access.mode).toBe('remote'); + expect(config.onForbidden).toHaveBeenCalledOnce(); + expect(config.onUnauthorized).not.toHaveBeenCalled(); + expect(config.offlineFallback).not.toHaveBeenCalled(); + expect(config.isAuthAccessDenial).toHaveBeenCalledWith(baseReq, error403); + }); + + it('401 still revokes remote access when isAuthAccessDenial accepts only 401', async () => { + const error401 = new HttpErrorResponse({ status: 401 }); + const config = makeConfig({ + enforceAuthAccessMode: true, + isAuthAccessDenial: vi.fn((_req, error) => (error as HttpErrorResponse).status === 401), + }); + setupInterceptor(config); + const access = TestBed.inject(KitAuthAccessService); + access.grantRemote(); + const next = vi.fn().mockReturnValue(throwError(() => error401)); + + await expect(firstValueFrom(runInterceptor(baseReq, next))).rejects.toBe(error401); + + expect(access.mode).toBe('none'); + expect(config.onUnauthorized).toHaveBeenCalledOnce(); + expect(config.isAuthAccessDenial).toHaveBeenCalledWith(baseReq, error401); + }); + + it('omitted isAuthAccessDenial keeps revoking remote access for both 401 and 403', async () => { + for (const status of [401, 403] as const) { + TestBed.resetTestingModule(); + const hook = status === 401 ? 'onUnauthorized' : 'onForbidden'; + const config = makeConfig({ enforceAuthAccessMode: true }); + setupInterceptor(config); + const access = TestBed.inject(KitAuthAccessService); + access.grantRemote(); + const error = new HttpErrorResponse({ status }); + const next = vi.fn().mockReturnValue(throwError(() => error)); + + await expect(firstValueFrom(runInterceptor(baseReq, next))).rejects.toBe(error); + + expect(access.mode).toBe('none'); + expect(config[hook]).toHaveBeenCalledOnce(); + } + }); }); // ---- 401 handling --------------------------------------------------------- diff --git a/projects/kit/src/lib/http/kit-http.interceptor.ts b/projects/kit/src/lib/http/kit-http.interceptor.ts index 75aba80..b2c3d96 100644 --- a/projects/kit/src/lib/http/kit-http.interceptor.ts +++ b/projects/kit/src/lib/http/kit-http.interceptor.ts @@ -270,6 +270,28 @@ export interface KitHttpConfig { * @param error - The error thrown by `getAuthHeaders`. */ onAuthError?(request: HttpRequest, error: unknown): void; + /** + * Classify whether an auth-related failure should revoke shared remote access. + * + * @remarks + * Consulted only when {@link KitHttpConfig.enforceAuthAccessMode} is enabled and the failure is + * an explicit authentication denial ({@link isExplicitAuthDenial | HTTP 401/403} or the same + * status shape from {@link KitHttpConfig.getAuthHeaders}). Return `true` to call + * {@link KitAuthAccessService.clear}; return `false` to keep the current access mode (for example + * when a `403` is a resource permission error rather than a lost session). When omitted, both + * `401` and `403` revoke access — the historical default. + * + * Regardless of the return value, explicit `401`/`403` responses never reach + * {@link KitHttpConfig.offlineFallback}; the matching response hook + * ({@link KitHttpConfig.onUnauthorized} / {@link KitHttpConfig.onForbidden}) still runs and the + * error is rethrown. A rejection from `getAuthHeaders` has no response, so it calls only + * {@link KitHttpConfig.onAuthError}. + * + * @param request - The outgoing request that failed (or whose headers could not be produced). + * @param error - The `HttpErrorResponse` or header-production rejection. + * @returns `true` when shared remote access should be cleared. + */ + isAuthAccessDenial?(request: HttpRequest, error: unknown): boolean; } /** @@ -311,6 +333,19 @@ export const KIT_HTTP_CONFIG = new InjectionToken('@rdlabo/ionic- export const provideKitHttp = (configFactory: () => KitHttpConfig): EnvironmentProviders => makeEnvironmentProviders([{ provide: KIT_HTTP_CONFIG, useFactory: configFactory }]); +/** + * True when shared remote access should be revoked for an auth-related failure. + * + * @remarks + * Only meaningful when {@link KitHttpConfig.enforceAuthAccessMode} is enabled and + * {@link isExplicitAuthDenial} already matched. When {@link KitHttpConfig.isAuthAccessDenial} is + * omitted, every explicit denial revokes access (legacy default). + * + * @internal + */ +const shouldRevokeAuthAccess = (config: KitHttpConfig, req: HttpRequest, error: unknown): boolean => + config.isAuthAccessDenial?.(req, error) ?? isExplicitAuthDenial(error); + /** * Classify a final (post-retry) error and invoke the matching {@link KitHttpConfig} hook. * @@ -402,7 +437,9 @@ export const kitAuthInterceptor: HttpInterceptorFn = (request, next) => { return from(Promise.resolve(config.getAuthHeaders(request))).pipe( catchError((headerError: unknown) => { // getAuthHeaders failed → the request is never sent; classify it instead of failing silently. - if (config.enforceAuthAccessMode && isExplicitAuthDenial(headerError)) access.clear(); + if (config.enforceAuthAccessMode && isExplicitAuthDenial(headerError) && shouldRevokeAuthAccess(config, request, headerError)) { + access.clear(); + } config.onAuthError?.(request, headerError); return throwError(() => headerError); }), @@ -453,7 +490,7 @@ export const kitAuthInterceptor: HttpInterceptorFn = (request, next) => { }), catchError((error: HttpErrorResponse) => { if (config.enforceAuthAccessMode && isExplicitAuthDenial(error)) { - access.clear(); + if (shouldRevokeAuthAccess(config, req, error)) access.clear(); dispatchError(config, req, error); return throwError(() => error); }