Skip to content

Commit f1431b6

Browse files
authored
Improve auth failure diagnostics (#295)
* Expand on key details. * Improve JWKS timeout handling. * Add changeset. * Always log JWKS errors.
1 parent 61f48cd commit f1431b6

File tree

5 files changed

+52
-7
lines changed

5 files changed

+52
-7
lines changed

.changeset/wild-llamas-melt.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@powersync/service-core': patch
3+
---
4+
5+
Improve diagnostics in logs for JWKS timeouts.

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

Lines changed: 25 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import timers from 'timers/promises';
33
import { KeySpec } from './KeySpec.js';
44
import { LeakyBucket } from './LeakyBucket.js';
55
import { KeyCollector, KeyResult } from './KeyCollector.js';
6-
import { AuthorizationError } from '@powersync/lib-services-framework';
6+
import { AuthorizationError, ErrorCode, logger } from '@powersync/lib-services-framework';
77
import { mapAuthConfigError } from './utils.js';
88

99
/**
@@ -70,8 +70,21 @@ export class CachedKeyCollector implements KeyCollector {
7070
// e.g. in the case of waiting for error retries.
7171
// In the case of very slow requests, we don't wait for it to complete, but the
7272
// request can still complete in the background.
73-
const timeout = timers.setTimeout(3000);
74-
await Promise.race([this.refreshPromise, timeout]);
73+
const WAIT_TIMEOUT_SECONDS = 3;
74+
const timeout = timers.setTimeout(WAIT_TIMEOUT_SECONDS * 1000).then(() => {
75+
throw new AuthorizationError(ErrorCode.PSYNC_S2204, `JWKS request failed`, {
76+
cause: { message: `Key request timed out in ${WAIT_TIMEOUT_SECONDS}s`, name: 'AbortError' }
77+
});
78+
});
79+
try {
80+
await Promise.race([this.refreshPromise, timeout]);
81+
} catch (e) {
82+
if (e instanceof AuthorizationError) {
83+
return { keys: this.currentKeys, errors: [...this.currentErrors, e] };
84+
} else {
85+
throw e;
86+
}
87+
}
7588
}
7689

7790
return { keys: this.currentKeys, errors: this.currentErrors };
@@ -102,7 +115,16 @@ export class CachedKeyCollector implements KeyCollector {
102115
this.currentErrors = errors;
103116
this.keyTimestamp = Date.now();
104117
this.error = false;
118+
119+
// Due to caching and background refresh behavior, errors are not always propagated to the request handler,
120+
// so we log them here.
121+
for (let error of errors) {
122+
logger.error(`Soft key refresh error`, error);
123+
}
105124
} catch (e) {
125+
// Due to caching and background refresh behavior, errors are not always propagated to the request handler,
126+
// so we log them here.
127+
logger.error(`Hard key refresh error`, e);
106128
this.error = true;
107129
// No result - keep previous keys
108130
this.currentErrors = [mapAuthConfigError(e)];

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

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,20 @@ export class KeySpec {
4040
return this.source.kid;
4141
}
4242

43+
get description(): string {
44+
let details: string[] = [];
45+
details.push(`kid: ${this.kid ?? '*'}`);
46+
details.push(`kty: ${this.source.kty}`);
47+
if (this.source.alg != null) {
48+
details.push(`alg: ${this.source.alg}`);
49+
}
50+
if (this.options.requiresAudience != null) {
51+
details.push(`aud: ${this.options.requiresAudience.join(', ')}`);
52+
}
53+
54+
return `<${details.filter((x) => x != null).join(', ')}>`;
55+
}
56+
4357
matchesAlgorithm(jwtAlg: string): boolean {
4458
if (this.source.alg) {
4559
return jwtAlg === this.source.alg;

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { logger, errors, AuthorizationError, ErrorCode } from '@powersync/lib-services-framework';
1+
import { AuthorizationError, ErrorCode, logger } from '@powersync/lib-services-framework';
22
import * as jose from 'jose';
33
import secs from '../util/secs.js';
44
import { JwtPayload } from './JwtPayload.js';
@@ -169,7 +169,7 @@ export class KeyStore<Collector extends KeyCollector = KeyCollector> {
169169
ErrorCode.PSYNC_S2101,
170170
'Could not find an appropriate key in the keystore. The key is missing or no key matched the token KID',
171171
{
172-
configurationDetails: `Known kid values: ${keys.map((key) => key.kid ?? '*').join(', ')}`
172+
configurationDetails: `Known keys: ${keys.map((key) => key.description).join(', ')}`
173173
// tokenDetails automatically populated later
174174
}
175175
);

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

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,9 +49,10 @@ export class RemoteJWKSCollector implements KeyCollector {
4949

5050
private async getJwksData(): Promise<any> {
5151
const abortController = new AbortController();
52+
const REQUEST_TIMEOUT_SECONDS = 30;
5253
const timeout = setTimeout(() => {
5354
abortController.abort();
54-
}, 30_000);
55+
}, REQUEST_TIMEOUT_SECONDS * 1000);
5556

5657
try {
5758
const res = await fetch(this.url, {
@@ -71,11 +72,14 @@ export class RemoteJWKSCollector implements KeyCollector {
7172

7273
return (await res.json()) as any;
7374
} catch (e) {
75+
if (e instanceof Error && e.name === 'AbortError') {
76+
e = { message: `Request timed out in ${REQUEST_TIMEOUT_SECONDS}s`, name: 'AbortError' };
77+
}
7478
throw new AuthorizationError(ErrorCode.PSYNC_S2204, `JWKS request failed`, {
7579
configurationDetails: `JWKS URL: ${this.url}`,
7680
// This covers most cases of FetchError
7781
// `cause: e` could lose the error message
78-
cause: { message: e.message, code: e.code }
82+
cause: { message: e.message, code: e.code, name: e.name }
7983
});
8084
} finally {
8185
clearTimeout(timeout);

0 commit comments

Comments
 (0)