Skip to content

Commit

Permalink
chore: add more test coverage
Browse files Browse the repository at this point in the history 
add more test coverage, throw non-http error when strategy not found
  • Loading branch information
@emonddr
emonddr committed Apr 24, 2019
1 parent 6a196cf commit 85b9ad8
Show file tree
Hide file tree
Showing 6 changed files with 111 additions and 15 deletions.
52 changes: 46 additions & 6 deletions packages/authentication/src/__tests__/acceptance/basic-auth-extension.acceptance.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import {anOpenApiSpec} from '@loopback/openapi-spec-builder';
import {api, get} from '@loopback/openapi-v3';
import {
FindRoute,
HttpErrors,
InvokeMethod,
ParseParams,
Reject,
Expand All @@ -27,6 +28,7 @@ import {
AuthenticationComponent,
UserProfile,
} from '../..';
import {AuthenticationStrategyNotFoundError} from '../../types';
import {BasicAuthenticationStrategyBindings, USER_REPO} from '../fixtures/keys';
import {BasicAuthenticationUserService} from '../fixtures/services/basic-auth-user-service';
import {BasicAuthenticationStrategy} from '../fixtures/strategies/basic-strategy';
Expand Down Expand Up @@ -73,7 +75,7 @@ describe('Basic Authentication', () => {
const client = whenIMakeRequestTo(server);

//not passing in 'Authorization' header
await client.get('/whoAmI').expect(404);
await client.get('/whoAmI').expect(401);
});

it(`returns error for missing 'Basic ' portion of Authorization header value`, async () => {
Expand All @@ -86,7 +88,7 @@ describe('Basic Authentication', () => {
await client
.get('/whoAmI')
.set('Authorization', 'NotB@s1c ' + hash)
.expect(404);
.expect(401);
});

it(`returns error for missing ':' in decrypted Authorization header credentials value`, async () => {
Expand All @@ -99,7 +101,7 @@ describe('Basic Authentication', () => {
await client
.get('/whoAmI')
.set('Authorization', 'Basic ' + hash)
.expect(404);
.expect(401);
});

it(`returns error for too many parts in decrypted Authorization header credentials value`, async () => {
Expand All @@ -114,7 +116,7 @@ describe('Basic Authentication', () => {
await client
.get('/whoAmI')
.set('Authorization', 'Basic ' + hash)
.expect(404);
.expect(401);
});

it('allows anonymous requests to methods with no decorator', async () => {
Expand Down Expand Up @@ -172,6 +174,21 @@ describe('Basic Authentication', () => {
app.controller(MyController);
}

it('returns error for unknown authentication strategy', async () => {
class InfoController {
@get('/status')
@authenticate('doesnotexist')
status() {
return {running: true};
}
}

app.controller(InfoController);
await whenIMakeRequestTo(server)
.get('/status')
.expect(401);
});

function givenAuthenticatedSequence() {
class MySequence implements SequenceHandler {
constructor(
Expand All @@ -190,8 +207,31 @@ describe('Basic Authentication', () => {
const {request, response} = context;
const route = this.findRoute(request);

// Authenticate
await this.authenticateRequest(request);
//
// The authentication action utilizes a strategy resolver to find
// an authentication strategy by name, and then it calls
// strategy.authenticate(request).
//
// The strategy resolver throws a non-http error if it cannot
// resolve the strategy. It is necessary to catch this error
// and rethrow it as in http error (in our REST application example)
//
// Errors thrown by the strategy implementations are http errors
// (in our REST application example). We simply rethrow them.
//
try {
//call authentication action
await this.authenticateRequest(request);
} catch (e) {
// strategy not found error
if (e instanceof AuthenticationStrategyNotFoundError) {
throw new HttpErrors.Unauthorized(e.message);
} //if
else {
// strategy error
throw e;
} //endif
} //catch

// Authentication successful, proceed to invoke controller
const args = await this.parseParams(request, route);
Expand Down
44 changes: 42 additions & 2 deletions packages/authentication/src/__tests__/acceptance/jwt-auth-extension.acceptance.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import {addExtension, Application} from '@loopback/core';
import {get} from '@loopback/openapi-v3';
import {
FindRoute,
HttpErrors,
InvokeMethod,
ParseParams,
Reject,
Expand All @@ -25,6 +26,7 @@ import {
AuthenticationComponent,
UserProfile,
} from '../..';
import {AuthenticationStrategyNotFoundError} from '../../types';
import {JWTAuthenticationStrategyBindings, USER_REPO} from '../fixtures/keys';
import {JWTService} from '../fixtures/services/jwt-service';
import {JWTAuthenticationStrategy} from '../fixtures/strategies/jwt-strategy';
Expand Down Expand Up @@ -260,6 +262,21 @@ describe('JWT Authentication', () => {
.expect(200, {running: true});
});

it('returns error for unknown authentication strategy', async () => {
class InfoController {
@get('/status')
@authenticate('doesnotexist')
status() {
return {running: true};
}
}

app.controller(InfoController);
await whenIMakeRequestTo(server)
.get('/status')
.expect(401);
});

async function givenAServer() {
app = new Application();
app.component(AuthenticationComponent);
Expand All @@ -285,8 +302,31 @@ describe('JWT Authentication', () => {
const {request, response} = context;
const route = this.findRoute(request);

// Authenticate
await this.authenticateRequest(request);
//
// The authentication action utilizes a strategy resolver to find
// an authentication strategy by name, and then it calls
// strategy.authenticate(request).
//
// The strategy resolver throws a non-http error if it cannot
// resolve the strategy. It is necessary to catch this error
// and rethrow it as in http error (in our REST application example)
//
// Errors thrown by the strategy implementations are http errors
// (in our REST application example). We simply rethrow them.
//
try {
//call authentication action
await this.authenticateRequest(request);
} catch (e) {
// strategy not found error
if (e instanceof AuthenticationStrategyNotFoundError) {
throw new HttpErrors.Unauthorized(e.message);
} //if
else {
// strategy error
throw e;
} //endif
} //catch

// Authentication successful, proceed to invoke controller
const args = await this.parseParams(request, route);
Expand Down
6 changes: 3 additions & 3 deletions packages/authentication/src/__tests__/fixtures/strategies/basic-strategy.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,15 +35,15 @@ export class BasicAuthenticationStrategy implements AuthenticationStrategy {
extractCredentals(request: Request): BasicAuthenticationStrategyCredentials {
if (!request.headers.authorization) {
//throw an error
throw new HttpErrors['NotFound'](`Authorization header not found.`);
throw new HttpErrors.Unauthorized(`Authorization header not found.`);
} //if

// for example : Basic Z2l6bW9AZ21haWwuY29tOnBhc3N3b3Jk
let auth_header_value = request.headers.authorization;

if (!auth_header_value.startsWith('Basic')) {
//throw an error
throw new HttpErrors['NotFound'](
throw new HttpErrors.Unauthorized(
`Authorization header is not of type 'Basic'.`,
);
} //if
Expand All @@ -63,7 +63,7 @@ export class BasicAuthenticationStrategy implements AuthenticationStrategy {

if (decryptedParts.length !== 2) {
//throw an error
throw new HttpErrors['NotFound'](
throw new HttpErrors.Unauthorized(
`Authorization header 'Basic' value does not contain two parts separated by ':'.`,
);
} //if
Expand Down
4 changes: 2 additions & 2 deletions packages/authentication/src/__tests__/fixtures/strategies/jwt-strategy.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ export class JWTAuthenticationStrategy implements AuthenticationStrategy {
extractCredentals(request: Request): string {
if (!request.headers.access_token) {
//throw an error
throw new HttpErrors['NotFound'](`'access_token' header not found.`);
throw new HttpErrors.Unauthorized(`'access_token' header not found.`);
} //if

let token: string;
Expand All @@ -37,7 +37,7 @@ export class JWTAuthenticationStrategy implements AuthenticationStrategy {
token = request.headers.access_token;
} else {
//throw an error
throw new HttpErrors['NotFound'](
throw new HttpErrors.Unauthorized(
`'access_token' header of type 'string' not found.`,
);
}
Expand Down
10 changes: 8 additions & 2 deletions packages/authentication/src/providers/auth-strategy.provider.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,10 @@ import {
} from '@loopback/core';
import {AuthenticationMetadata} from '../decorators/authenticate.decorator';
import {AuthenticationBindings} from '../keys';
import {AuthenticationStrategy} from '../types';
import {
AuthenticationStrategy,
AuthenticationStrategyNotFoundError,
} from '../types';

//this needs to be transient, e.g. for request level context.
@extensionPoint(
Expand All @@ -37,7 +40,10 @@ export class AuthenticationStrategyProvider
if (strategy) {
return strategy;
} else {
throw new Error(`The strategy '${name}' is not available.`);
// important not to throw a non-protocol-specific error here
throw new AuthenticationStrategyNotFoundError(
`The strategy '${name}' is not available.`,
);
}
});
}
Expand Down
10 changes: 10 additions & 0 deletions packages/authentication/src/types.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,3 +51,13 @@ export interface AuthenticationStrategy {
*/
authenticate(request: Request): Promise<UserProfile | undefined>;
}

/**
* The error thrown when the authentication strategy resolver
* cannot find the specified authentication strategy by name.
*/
export class AuthenticationStrategyNotFoundError extends Error {
constructor(error: string) {
super(error);
}
}

0 comments on commit 85b9ad8

Please sign in to comment.