Skip to content

Commit b0325c9

Browse files
rkistnerrkistner
committed
Error tweaks and tests.
1 parent 563a96e commit b0325c9

File tree

3 files changed

+39
-23
lines changed

3 files changed

+39
-23
lines changed

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

Lines changed: 15 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import secs from '../util/secs.js';
44
import { JwtPayload } from './JwtPayload.js';
55
import { KeyCollector } from './KeyCollector.js';
66
import { KeyOptions, KeySpec, SUPPORTED_ALGORITHMS } from './KeySpec.js';
7+
import { mapAuthError } from './utils.js';
78

89
/**
910
* KeyStore to get keys and verify tokens.
@@ -98,16 +99,20 @@ export class KeyStore<Collector extends KeyCollector = KeyCollector> {
9899

99100
private async verifyInternal(token: string, options: jose.JWTVerifyOptions) {
100101
let keyOptions: KeyOptions | undefined = undefined;
101-
const result = await jose.jwtVerify(
102-
token,
103-
async (header) => {
104-
let key = await this.getCachedKey(token, header);
105-
keyOptions = key.options;
106-
return key.key;
107-
},
108-
options
109-
);
110-
return { result, keyOptions: keyOptions! };
102+
try {
103+
const result = await jose.jwtVerify(
104+
token,
105+
async (header) => {
106+
let key = await this.getCachedKey(token, header);
107+
keyOptions = key.options;
108+
return key.key;
109+
},
110+
options
111+
);
112+
return { result, keyOptions: keyOptions! };
113+
} catch (e) {
114+
throw mapAuthError(e);
115+
}
111116
}
112117

113118
private async getCachedKey(token: string, header: jose.JWTHeaderParameters): Promise<KeySpec> {

‎packages/service-core/src/auth/utils.ts‎

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,20 @@ import * as jose from 'jose';
33

44
export function mapJoseError(error: jose.errors.JOSEError): AuthorizationError {
55
// TODO: improved message for exp issues, etc
6-
if (error.code === 'ERR_JWS_INVALID' || error.code === 'ERR_JWT_INVALID') {
6+
if (error.code === jose.errors.JWSInvalid.code || error.code === jose.errors.JWTInvalid.code) {
77
throw new AuthorizationError(ErrorCode.PSYNC_S2101, 'Token is not a well-formed JWT. Check the token format.', {
88
details: error.message
99
});
10+
} else if (error.code === jose.errors.JWTClaimValidationFailed.code) {
11+
// Example: missing required "sub" claim
12+
const claim = (error as jose.errors.JWTClaimValidationFailed).claim;
13+
throw new AuthorizationError(
14+
ErrorCode.PSYNC_S2101,
15+
`JWT payload is missing a required claim ${JSON.stringify(claim)}`,
16+
{
17+
cause: error
18+
}
19+
);
1020
}
1121
return new AuthorizationError(ErrorCode.PSYNC_S2101, error.message, { cause: error });
1222
}
@@ -21,7 +31,7 @@ export function mapAuthError(error: any): AuthorizationError {
2131
}
2232

2333
export function mapJoseConfigError(error: jose.errors.JOSEError): AuthorizationError {
24-
return new AuthorizationError(ErrorCode.PSYNC_S2201, error.message, { cause: error });
34+
return new AuthorizationError(ErrorCode.PSYNC_S2201, error.message ?? 'Authorization error', { cause: error });
2535
}
2636

2737
export function mapAuthConfigError(error: any): AuthorizationError {
@@ -30,5 +40,5 @@ export function mapAuthConfigError(error: any): AuthorizationError {
3040
} else if (error instanceof jose.errors.JOSEError) {
3141
return mapJoseConfigError(error);
3242
}
33-
return new AuthorizationError(ErrorCode.PSYNC_S2201, error.message, { cause: error });
43+
return new AuthorizationError(ErrorCode.PSYNC_S2201, error.message ?? 'Auth configuration error', { cause: error });
3444
}

‎packages/service-core/test/src/auth.test.ts‎

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -75,21 +75,21 @@ describe('JWT Auth', () => {
7575
defaultAudiences: ['other'],
7676
maxAge: '6m'
7777
})
78-
).rejects.toThrow('unexpected "aud" claim value');
78+
).rejects.toThrow('[PSYNC_S2105] Unexpected "aud" claim value: "tests"');
7979

8080
await expect(
8181
store.verifyJwt(signedJwt, {
8282
defaultAudiences: [],
8383
maxAge: '6m'
8484
})
85-
).rejects.toThrow('unexpected "aud" claim value');
85+
).rejects.toThrow('[PSYNC_S2105] Unexpected "aud" claim value: "tests"');
8686

8787
await expect(
8888
store.verifyJwt(signedJwt, {
8989
defaultAudiences: ['tests'],
9090
maxAge: '1m'
9191
})
92-
).rejects.toThrow('Token must expire in a maximum of');
92+
).rejects.toThrow('[PSYNC_S2104] Token must expire in a maximum of 60 seconds, got 300s');
9393

9494
const signedJwt2 = await new jose.SignJWT({})
9595
.setProtectedHeader({ alg: 'HS256', kid: 'k1' })
@@ -104,7 +104,7 @@ describe('JWT Auth', () => {
104104
defaultAudiences: ['tests'],
105105
maxAge: '5m'
106106
})
107-
).rejects.toThrow('missing required "sub" claim');
107+
).rejects.toThrow('[PSYNC_S2101] JWT payload is missing a required claim "sub"');
108108
});
109109

110110
test('Algorithm validation', async () => {
@@ -159,7 +159,7 @@ describe('JWT Auth', () => {
159159
maxAge: '6m'
160160
})
161161
).rejects.toThrow(
162-
'Could not find an appropriate key in the keystore. The key is missing or no key matched the token KID'
162+
'[PSYNC_S2101] Could not find an appropriate key in the keystore. The key is missing or no key matched the token KID'
163163
);
164164

165165
// Wrong kid
@@ -178,7 +178,7 @@ describe('JWT Auth', () => {
178178
maxAge: '6m'
179179
})
180180
).rejects.toThrow(
181-
'Could not find an appropriate key in the keystore. The key is missing or no key matched the token KID'
181+
'[PSYNC_S2101] Could not find an appropriate key in the keystore. The key is missing or no key matched the token KID'
182182
);
183183

184184
// No kid, matches sharedKey2
@@ -255,7 +255,7 @@ describe('JWT Auth', () => {
255255
defaultAudiences: ['tests'],
256256
maxAge: '6m'
257257
})
258-
).rejects.toThrow('unexpected "aud" claim value');
258+
).rejects.toThrow('[PSYNC_S2105] Unexpected "aud" claim value: "tests"');
259259

260260
const signedJwt3 = await new jose.SignJWT({})
261261
.setProtectedHeader({ alg: 'HS256', kid: 'k1' })
@@ -345,7 +345,7 @@ describe('JWT Auth', () => {
345345
expect(key.kid).toEqual(publicKeyRSA.kid!);
346346

347347
cached.addTimeForTests(301_000);
348-
currentResponse = Promise.reject('refresh failed');
348+
currentResponse = Promise.reject(new Error('refresh failed'));
349349

350350
// Uses the promise, refreshes in the background
351351
let response = await cached.getKeys();
@@ -356,15 +356,16 @@ describe('JWT Auth', () => {
356356
await cached.addTimeForTests(0);
357357
response = await cached.getKeys();
358358
// Still have the cached key, but also have the error
359+
console.log('e', response.errors[0]);
359360
expect(response.keys[0].kid).toEqual(publicKeyRSA.kid!);
360-
expect(response.errors[0].message).toMatch('Failed to fetch');
361+
expect(response.errors[0].message).toMatch('[PSYNC_S2201] refresh failed');
361362

362363
await cached.addTimeForTests(3601_000);
363364
response = await cached.getKeys();
364365

365366
// Now the keys have expired, and the request still fails
366367
expect(response.keys).toEqual([]);
367-
expect(response.errors[0].message).toMatch('Failed to fetch');
368+
expect(response.errors[0].message).toMatch('[PSYNC_S2201] refresh failed');
368369

369370
currentResponse = Promise.resolve({
370371
errors: [],

0 commit comments

Comments
 (0)