Improve auth errors (#258)

* Restructure auth errors.

* First pass of error message improvements.

* Remove old AuthorizationError.

* Error tweaks and tests.

* Populate token details in error logs.

* Further tweaks.

* Changeset.

Author: rkistner

.changeset/clever-kangaroos-thank.md

@@ -0,0 +1,10 @@
+---
+'@powersync/service-module-postgres': minor
+'@powersync/service-rsocket-router': minor
+'@powersync/service-errors': minor
+'@powersync/service-core': minor
+'@powersync/lib-services-framework': minor
+'@powersync/service-image': minor
+---
+
+Improve authentication error messages and logs

libs/lib-services/src/router/endpoint.ts

@@ -16,7 +16,10 @@ export const executeEndpoint = async <I, O, C, P extends EndpointHandlerPayload<
   }
   const authorizer_response = await endpoint.authorize?.(payload);
   if (authorizer_response && !authorizer_response.authorized) {
-    throw new errors.AuthorizationError(authorizer_response.errors);
+    if (authorizer_response.error == null) {
+      throw new errors.AuthorizationError(errors.ErrorCode.PSYNC_S2101, 'Authorization failed');
+    }
+    throw authorizer_response.error;
   }
 
   return endpoint.handler(payload);

libs/lib-services/src/router/router-definitions.ts

@@ -1,3 +1,4 @@
+import { ServiceError } from '@powersync/service-errors';
 import { MicroValidator } from '../schema/definitions.js';
 
 /**
@@ -22,7 +23,7 @@ export type AuthorizationResponse =
     }
   | {
       authorized: false;
-      errors?: any[];
+      error?: ServiceError | undefined;
     };
 
 /**

modules/module-postgres/src/auth/SupabaseKeyCollector.ts

@@ -1,17 +1,18 @@
 import * as lib_postgres from '@powersync/lib-service-postgres';
-import { auth } from '@powersync/service-core';
+import { auth, KeyResult } from '@powersync/service-core';
 import * as pgwire from '@powersync/service-jpgwire';
 import * as jose from 'jose';
 
 import * as types from '../types/types.js';
+import { AuthorizationError, ErrorCode } from '@powersync/lib-services-framework';
 
 /**
  * Fetches key from the Supabase database.
  *
  * Unfortunately, despite the JWTs containing a kid, we have no way to lookup that kid
  * before receiving a valid token.
  *
- * @deprecated Supabase is removing support for "app.settings.jwt_secret".
+ * @deprecated Supabase is removing support for "app.settings.jwt_secret". This is likely to not function anymore, except in some self-hosted setups.
  */
 export class SupabaseKeyCollector implements auth.KeyCollector {
   private pool: pgwire.PgClient;
@@ -35,7 +36,7 @@ export class SupabaseKeyCollector implements auth.KeyCollector {
     return this.pool.end();
   }
 
-  async getKeys() {
+  async getKeys(): Promise<KeyResult> {
     let row: { jwt_secret: string };
     try {
       const rows = pgwire.pgwireRows(
@@ -44,7 +45,10 @@ export class SupabaseKeyCollector implements auth.KeyCollector {
       row = rows[0] as any;
     } catch (e) {
       if (e.message?.includes('unrecognized configuration parameter')) {
-        throw new jose.errors.JOSEError(`Generate a new JWT secret on Supabase. Cause: ${e.message}`);
+        throw new AuthorizationError(
+          ErrorCode.PSYNC_S2201,
+          'No JWT secret found in Supabase database. Manually configure the secret.'
+        );
       } else {
         throw e;
       }
@@ -53,7 +57,12 @@ export class SupabaseKeyCollector implements auth.KeyCollector {
     if (secret == null) {
       return {
         keys: [],
-        errors: [new jose.errors.JWKSNoMatchingKey()]
+        errors: [
+          new AuthorizationError(
+            ErrorCode.PSYNC_S2201,
+            'No JWT secret found in Supabase database. Manually configure the secret.'
+          )
+        ]
       };
     } else {
       const key: jose.JWK = {

packages/rsocket-router/src/router/ReactiveSocketRouter.ts

@@ -3,7 +3,7 @@
  * to expose reactive websocket stream in an interface similar to
  * other Journey micro routers.
  */
-import { errors, logger } from '@powersync/lib-services-framework';
+import { ErrorCode, errors, logger } from '@powersync/lib-services-framework';
 import * as http from 'http';
 import { Payload, RSocketServer } from 'rsocket-core';
 import * as ws from 'ws';
@@ -72,7 +72,7 @@ export class ReactiveSocketRouter<C> {
           // Throwing an exception in this context will be returned to the client side request
           if (!payload.metadata) {
             // Meta data is required for endpoint handler path matching
-            throw new errors.AuthorizationError('No context meta data provided');
+            throw new errors.AuthorizationError(ErrorCode.PSYNC_S2101, 'No context meta data provided');
           }
 
           const context = await params.contextProvider(payload.metadata!);
@@ -166,7 +166,9 @@ export async function handleReactiveStream<Context>(
       responder
     });
     if (!isAuthorized.authorized) {
-      return exitWithError(new errors.AuthorizationError(isAuthorized.errors));
+      return exitWithError(
+        isAuthorized.error ?? new errors.AuthorizationError(ErrorCode.PSYNC_S2101, 'Authorization failed')
+      );
     }
   }
 

packages/service-core/src/auth/CachedKeyCollector.ts

@@ -3,6 +3,8 @@ import timers from 'timers/promises';
 import { KeySpec } from './KeySpec.js';
 import { LeakyBucket } from './LeakyBucket.js';
 import { KeyCollector, KeyResult } from './KeyCollector.js';
+import { AuthorizationError } from '@powersync/lib-services-framework';
+import { mapAuthConfigError } from './utils.js';
 
 /**
  * Manages caching and refreshing for a key collector.
@@ -39,7 +41,7 @@ export class CachedKeyCollector implements KeyCollector {
    */
   private keyExpiry = 3600000;
 
-  private currentErrors: jose.errors.JOSEError[] = [];
+  private currentErrors: AuthorizationError[] = [];
   /**
    * Indicates a "fatal" error that should be retried.
    */
@@ -103,11 +105,7 @@ export class CachedKeyCollector implements KeyCollector {
     } catch (e) {
       this.error = true;
       // No result - keep previous keys
-      if (e instanceof jose.errors.JOSEError) {
-        this.currentErrors = [e];
-      } else {
-        this.currentErrors = [new jose.errors.JOSEError(e.message ?? 'Failed to fetch keys')];
-      }
+      this.currentErrors = [mapAuthConfigError(e)];
     }
   }
 

packages/service-core/src/auth/CompoundKeyCollector.ts

@@ -1,6 +1,7 @@
 import * as jose from 'jose';
 import { KeySpec } from './KeySpec.js';
 import { KeyCollector, KeyResult } from './KeyCollector.js';
+import { AuthorizationError } from '@powersync/lib-services-framework';
 
 export class CompoundKeyCollector implements KeyCollector {
   private collectors: KeyCollector[];
@@ -15,7 +16,7 @@ export class CompoundKeyCollector implements KeyCollector {
 
   async getKeys(): Promise<KeyResult> {
     let keys: KeySpec[] = [];
-    let errors: jose.errors.JOSEError[] = [];
+    let errors: AuthorizationError[] = [];
     const promises = this.collectors.map((collector) =>
       collector.getKeys().then((result) => {
         keys.push(...result.keys);

packages/service-core/src/auth/KeyCollector.ts

@@ -1,4 +1,4 @@
-import * as jose from 'jose';
+import { AuthorizationError } from '@powersync/lib-services-framework';
 import { KeySpec } from './KeySpec.js';
 
 export interface KeyCollector {
@@ -22,6 +22,6 @@ export interface KeyCollector {
 }
 
 export interface KeyResult {
-  errors: jose.errors.JOSEError[];
+  errors: AuthorizationError[];
   keys: KeySpec[];
 }

packages/service-core/src/auth/KeyStore.ts

@@ -1,9 +1,10 @@
-import { logger } from '@powersync/lib-services-framework';
+import { logger, errors, AuthorizationError, ErrorCode } from '@powersync/lib-services-framework';
 import * as jose from 'jose';
 import secs from '../util/secs.js';
 import { JwtPayload } from './JwtPayload.js';
 import { KeyCollector } from './KeyCollector.js';
 import { KeyOptions, KeySpec, SUPPORTED_ALGORITHMS } from './KeySpec.js';
+import { mapAuthError } from './utils.js';
 
 /**
  * KeyStore to get keys and verify tokens.
@@ -49,7 +50,8 @@ export class KeyStore<Collector extends KeyCollector = KeyCollector> {
       clockTolerance: 60,
       // More specific algorithm checking is done when selecting the key to use.
       algorithms: SUPPORTED_ALGORITHMS,
-      requiredClaims: ['aud', 'sub', 'iat', 'exp']
+      // 'aud' presence is checked below, so we can add more details to the error message.
+      requiredClaims: ['sub', 'iat', 'exp']
     });
 
     let audiences = options.defaultAudiences;
@@ -60,16 +62,24 @@ export class KeyStore<Collector extends KeyCollector = KeyCollector> {
 
     const tokenPayload = result.payload;
 
-    let aud = tokenPayload.aud!;
-    if (!Array.isArray(aud)) {
+    let aud = tokenPayload.aud;
+    if (aud == null) {
+      throw new AuthorizationError(ErrorCode.PSYNC_S2105, `JWT payload is missing a required claim "aud"`, {
+        configurationDetails: `Current configuration allows these audience values: ${JSON.stringify(audiences)}`
+      });
+    } else if (!Array.isArray(aud)) {
       aud = [aud];
     }
     if (
       !aud.some((a) => {
         return audiences.includes(a);
       })
     ) {
-      throw new jose.errors.JWTClaimValidationFailed('unexpected "aud" claim value', 'aud', 'check_failed');
+      throw new AuthorizationError(
+        ErrorCode.PSYNC_S2105,
+        `Unexpected "aud" claim value: ${JSON.stringify(tokenPayload.aud)}`,
+        { configurationDetails: `Current configuration allows these audience values: ${JSON.stringify(audiences)}` }
+      );
     }
 
     const tokenDuration = tokenPayload.exp! - tokenPayload.iat!;
@@ -78,29 +88,36 @@ export class KeyStore<Collector extends KeyCollector = KeyCollector> {
     // is too far into the future.
     const maxAge = keyOptions.maxLifetimeSeconds ?? secs(options.maxAge);
     if (tokenDuration > maxAge) {
-      throw new jose.errors.JWTInvalid(`Token must expire in a maximum of ${maxAge} seconds, got ${tokenDuration}`);
+      throw new AuthorizationError(
+        ErrorCode.PSYNC_S2104,
+        `Token must expire in a maximum of ${maxAge} seconds, got ${tokenDuration}s`
+      );
     }
 
     const parameters = tokenPayload.parameters;
     if (parameters != null && (Array.isArray(parameters) || typeof parameters != 'object')) {
-      throw new jose.errors.JWTInvalid('parameters must be an object');
+      throw new AuthorizationError(ErrorCode.PSYNC_S2101, `Payload parameters must be an object`);
     }
 
     return tokenPayload as JwtPayload;
   }
 
   private async verifyInternal(token: string, options: jose.JWTVerifyOptions) {
     let keyOptions: KeyOptions | undefined = undefined;
-    const result = await jose.jwtVerify(
-      token,
-      async (header) => {
-        let key = await this.getCachedKey(token, header);
-        keyOptions = key.options;
-        return key.key;
-      },
-      options
-    );
-    return { result, keyOptions: keyOptions! };
+    try {
+      const result = await jose.jwtVerify(
+        token,
+        async (header) => {
+          let key = await this.getCachedKey(token, header);
+          keyOptions = key.options;
+          return key.key;
+        },
+        options
+      );
+      return { result, keyOptions: keyOptions! };
+    } catch (e) {
+      throw mapAuthError(e, token);
+    }
   }
 
   private async getCachedKey(token: string, header: jose.JWTHeaderParameters): Promise<KeySpec> {
@@ -112,7 +129,10 @@ export class KeyStore<Collector extends KeyCollector = KeyCollector> {
       for (let key of keys) {
         if (key.kid == kid) {
           if (!key.matchesAlgorithm(header.alg)) {
-            throw new jose.errors.JOSEAlgNotAllowed(`Unexpected token algorithm ${header.alg}`);
+            throw new AuthorizationError(ErrorCode.PSYNC_S2101, `Unexpected token algorithm ${header.alg}`, {
+              configurationDetails: `Key kid: ${key.source.kid}, alg: ${key.source.alg}, kty: ${key.source.kty}`
+              // Token details automatically populated elsewhere
+            });
           }
           return key;
         }
@@ -145,8 +165,13 @@ export class KeyStore<Collector extends KeyCollector = KeyCollector> {
         logger.error(`Failed to refresh keys`, e);
       });
 
-      throw new jose.errors.JOSEError(
-        'Could not find an appropriate key in the keystore. The key is missing or no key matched the token KID'
+      throw new AuthorizationError(
+        ErrorCode.PSYNC_S2101,
+        'Could not find an appropriate key in the keystore. The key is missing or no key matched the token KID',
+        {
+          configurationDetails: `Known kid values: ${keys.map((key) => key.kid ?? '*').join(', ')}`
+          // tokenDetails automatically populated later
+        }
       );
     }
   }

packages/service-core/src/auth/RemoteJWKSCollector.ts

@@ -4,6 +4,7 @@ import * as jose from 'jose';
 import fetch from 'node-fetch';
 
 import {
+  AuthorizationError,
   ErrorCode,
   LookupOptions,
   makeHostnameLookupFunction,
@@ -46,36 +47,58 @@ export class RemoteJWKSCollector implements KeyCollector {
     this.agent = this.resolveAgent();
   }
 
-  async getKeys(): Promise<KeyResult> {
+  private async getJwksData(): Promise<any> {
     const abortController = new AbortController();
     const timeout = setTimeout(() => {
       abortController.abort();
     }, 30_000);
 
-    const res = await fetch(this.url, {
-      method: 'GET',
-      headers: {
-        Accept: 'application/json'
-      },
-      signal: abortController.signal,
-      agent: this.agent
-    });
-
-    if (!res.ok) {
-      throw new jose.errors.JWKSInvalid(`JWKS request failed with ${res.statusText}`);
-    }
+    try {
+      const res = await fetch(this.url, {
+        method: 'GET',
+        headers: {
+          Accept: 'application/json'
+        },
+        signal: abortController.signal,
+        agent: this.agent
+      });
+
+      if (!res.ok) {
+        throw new AuthorizationError(ErrorCode.PSYNC_S2204, `JWKS request failed with ${res.statusText}`, {
+          configurationDetails: `JWKS URL: ${this.url}`
+        });
+      }
 
-    const data = (await res.json()) as any;
+      return (await res.json()) as any;
+    } catch (e) {
+      throw new AuthorizationError(ErrorCode.PSYNC_S2204, `JWKS request failed`, {
+        configurationDetails: `JWKS URL: ${this.url}`,
+        // This covers most cases of FetchError
+        // `cause: e` could lose the error message
+        cause: { message: e.message, code: e.code }
+      });
+    } finally {
+      clearTimeout(timeout);
+    }
+  }
 
-    clearTimeout(timeout);
+  async getKeys(): Promise<KeyResult> {
+    const data = await this.getJwksData();
 
     // https://github.com/panva/jose/blob/358e864a0cccf1e0f9928a959f91f18f3f06a7de/src/jwks/local.ts#L36
     if (
       data.keys == null ||
       !Array.isArray(data.keys) ||
       !(data.keys as any[]).every((key) => typeof key == 'object' && !Array.isArray(key))
     ) {
-      return { keys: [], errors: [new jose.errors.JWKSInvalid(`No keys in found in JWKS response`)] };
+      return {
+        keys: [],
+        errors: [
+          new AuthorizationError(ErrorCode.PSYNC_S2204, `Invalid JWKS response`, {
+            configurationDetails: `JWKS URL: ${this.url}. Response:\n${JSON.stringify(data, null, 2)}`
+          })
+        ]
+      };
     }
 
     let keys: KeySpec[] = [];