diff --git a/src/main/ac-auth.ts b/src/main/ac-auth.ts index 2f67c3f..853b2c1 100644 --- a/src/main/ac-auth.ts +++ b/src/main/ac-auth.ts @@ -1,9 +1,4 @@ import { ClientError } from './exception.js'; -import { AuthToken } from './services/auth-context.js'; - -export interface AcAuthSpec { - jwtContext?: AcJwtContext; -} export interface AcJwtContext { organisation_id?: string; @@ -50,20 +45,16 @@ export interface AcJobAccessToken { clientName: string; } -export class AcAuth implements AuthToken { +export class AcAuth { - actor: AcActor | null = null; + actor: AcActor; - constructor(spec: AcAuthSpec = {}) { - this.actor = spec.jwtContext == null ? null : this.parseActor(spec.jwtContext); - } - - isValid(): boolean { - return this.actor != null; + constructor(jwtContext: AcJwtContext) { + this.actor = this.parseActor(jwtContext); } getOrganisationId(): string | null { - return this.actor?.organisationId ?? null; + return this.actor.organisationId ?? null; } requireOrganisationId(): string { @@ -75,10 +66,10 @@ export class AcAuth implements AuthToken { } getClientId(): string | null { - if (this.actor?.type === 'Client') { + if (this.actor.type === 'Client') { return this.actor.id; } - if (this.actor?.type === 'ServiceAccount' && this.actor.clientId) { + if (this.actor.type === 'ServiceAccount' && this.actor.clientId) { return this.actor.clientId; } return null; @@ -96,11 +87,15 @@ export class AcAuth implements AuthToken { protected parseActor(jwtContext: AcJwtContext) { // Note: order matters - return this.parseServiceAccount(jwtContext) ?? + const actor = this.parseServiceAccount(jwtContext) ?? this.parseJobAccessToken(jwtContext) ?? this.parseClient(jwtContext) ?? - this.parseUser(jwtContext) ?? - null; + this.parseUser(jwtContext); + if (actor == null) { + // TODO find what AcAuthProvider throws when isAuthenticated check is not met + throw new InvalidJwtTokenError('Could not parse actor from JWT payload'); + } + return actor; } protected parseServiceAccount(jwtContext: AcJwtContext): AcServiceAccount | null { @@ -168,3 +163,9 @@ export class AccessForbidden extends ClientError { override status = 403; } + +export class InvalidJwtTokenError extends ClientError { + + override status = 401; + +} diff --git a/src/main/services/ac-auth-provider.ts b/src/main/services/ac-auth-provider.ts index d70c55c..7ec8ab3 100644 --- a/src/main/services/ac-auth-provider.ts +++ b/src/main/services/ac-auth-provider.ts @@ -47,11 +47,9 @@ export class AcAuthProvider extends AuthProvider { organisation_id: organisationIdHeader, ...payload.context }; - return new AcAuth({ - jwtContext: data, - }); - } catch (err) { - this.logger.warn(`Authentication from token failed`, { details: err }); + return new AcAuth(data); + } catch (error) { + this.logger.warn(`Authentication from token failed`, { error }); throw new AuthenticationError(); } } diff --git a/src/main/services/auth-context.ts b/src/main/services/auth-context.ts index 67f3d14..8cd64e7 100644 --- a/src/main/services/auth-context.ts +++ b/src/main/services/auth-context.ts @@ -1,15 +1,11 @@ import { ClientError } from '@nodescript/errors'; -export interface AuthToken { - isValid(): boolean; -} - -export class AuthContext { +export class AuthContext { constructor(private authToken: T) {} isAuthenticated() { - return this.authToken != null && this.authToken.isValid(); + return this.authToken != null; } checkAuthenticated(): void { diff --git a/src/main/services/auth-provider.ts b/src/main/services/auth-provider.ts index b48e623..b21f3bd 100644 --- a/src/main/services/auth-provider.ts +++ b/src/main/services/auth-provider.ts @@ -1,8 +1,8 @@ -import { AuthContext, AuthToken } from './auth-context.js'; +import { AuthContext } from './auth-context.js'; export type AuthHeaders = Record; -export abstract class AuthProvider { +export abstract class AuthProvider { abstract provide(headers?: AuthHeaders): Promise>; diff --git a/src/test/integration/ac-auth-mocking.test.ts b/src/test/integration/ac-auth-mocking.test.ts index f345a3e..be6074e 100644 --- a/src/test/integration/ac-auth-mocking.test.ts +++ b/src/test/integration/ac-auth-mocking.test.ts @@ -25,13 +25,12 @@ describe('Mocking AcAuth', () => { const mesh = super.createGlobalScope(); mesh.constant(AuthProvider, { async provide() { - return new AuthContext(new AcAuth({ - jwtContext: { - organisation_id: 'foo', - service_account_id: 'service-account-worker', - service_account_name: 'Bot', - } - })); + const token = new AcAuth({ + organisation_id: 'foo', + service_account_id: 'service-account-worker', + service_account_name: 'Bot', + }); + return new AuthContext(token); } }); return mesh; diff --git a/src/test/specs/services/ac-auth.test.ts b/src/test/specs/services/ac-auth.test.ts index ad80ec6..8a0d091 100644 --- a/src/test/specs/services/ac-auth.test.ts +++ b/src/test/specs/services/ac-auth.test.ts @@ -276,14 +276,17 @@ describe('AcAuthProvider', () => { }); context('missing some info', () => { - it('does not return User actor when organisation_id is missing', async () => { - jwt.context = { - user_id: 'some-user-id', - user_name: 'some-user-name' - }; - const auth = await authProvider.provide(headers); - const user = auth.getAuthToken()?.actor; - assert.ok(user == null); + it('throws 401 when organisation_id is missing', async () => { + try { + jwt.context = { + user_id: 'some-user-id', + user_name: 'some-user-name' + }; + await authProvider.provide(headers); + throw new Error('UnexpectedSuccess'); + } catch (error: any) { + assert.strictEqual(error.status, 401); + } }); }); }); @@ -307,25 +310,31 @@ describe('AcAuthProvider', () => { }); context('missing required data', () => { - it('does not return actor when client_id is missing', async () => { - jwt.context = { - job_id: 'some-job-id', - organisation_id: 'ubio-organisation-id', - }; - const auth = await authProvider.provide(headers); - const jobAccessToken = auth.getAuthToken()?.actor; - assert.ok(jobAccessToken == null); + it('throws 401 when client_id is missing', async () => { + try { + jwt.context = { + job_id: 'some-job-id', + organisation_id: 'ubio-organisation-id', + }; + await authProvider.provide(headers); + throw new Error('UnexpectedSuccess'); + } catch (error: any) { + assert.strictEqual(error.status, 401); + } }); - it('does not return actor when organisation_id is missing', async () => { - jwt.context = { - job_id: 'some-job-id', - client_id: 'some-client-id', - client_name: 'Travel Aggregator', - }; - const auth = await authProvider.provide(headers); - const jobAccessToken = auth.getAuthToken()?.actor; - assert.ok(jobAccessToken == null); + it('401 when organisation_id is missing', async () => { + try { + jwt.context = { + job_id: 'some-job-id', + client_id: 'some-client-id', + client_name: 'Travel Aggregator', + }; + await authProvider.provide(headers); + throw new Error('UnexpectedSuccess'); + } catch (error: any) { + assert.strictEqual(error.status, 401); + } }); }); });