Skip to content

Commit

Permalink
fix(security): return authenticator message if set
Browse files Browse the repository at this point in the history 
Sets the first failure to the first invalid result encountered.
Previously only set if the result hadn't a status of 401, which
resulted in a generic error message.

fix #38
  • Loading branch information
@niallmccullagh
niallmccullagh committed Oct 1, 2018
1 parent 177bf28 commit 7d1e7fc
Show file tree
Hide file tree
Showing 3 changed files with 213 additions and 32 deletions.
37 changes: 37 additions & 0 deletions docs/OAS3 Security.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -196,3 +196,40 @@ If a user authenticated using `basicAuth`, then the controller would have
access to the object returned by the authenticator via `context.security.basicAuth`.
Simliarly, if the request used oauth, then `context.security.oauth` would be
populated with the result of the oauth authenticator.
### Using Multiple Authentication Types
Some REST APIs support several authentication types. The security section lets you combine the security requirements
using logical OR and AND to achieve the desired result. security uses the following logic:
```yaml
security: # A OR B
- A
- B
```
* The request will authenticate if **either** `A` or `B` return a success.
* `A` will run first then `B`
* The authentication process will return the result of the first successful authenticator.
* If no authenticators succeed then the result of the first invalid authenticator will be returned.
```yaml
security: # A AND B
- A
B
```
* The request will authenticate only if **both** `A` and `B` return a success.
* `A` will run first then `B`
* The authentication process will return the result of both the successful authenticators.
* If no authenticators succeed then the result of the first invalid authenticator will be returned.
```yaml
security: # (A AND B) OR (C AND D)
- A
B
- C
D
```
* The request will authenticate only if (`A` and `B`) OR (`C` AND `D`) return a success
7 changes: 2 additions & 5 deletions src/oas3/Operation.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -368,7 +368,6 @@ export default class Operation {
// No auth required
return {};
}

let firstFailure: AuthenticationFailure | undefined;
const challenges: {[schemeName: string]: string | undefined} = {};
let result: Dictionary<AuthenticationSuccess> | undefined;
Expand All @@ -395,12 +394,10 @@ export default class Operation {
// No luck with this security requirement.
if(failure.status === 401 && failure.challenge) {
challenges[securityRequirementResult.failedSchemeName!] = failure.challenge;
} else if(failure.status !== 401 && !firstFailure) {
firstFailure = failure;
}

if(securityRequirementResult.type === 'invalid') {
// Hard failure - don't try anything else.
break;
firstFailure = firstFailure || failure;
}
} else {
/* istanbul ignore this */
Expand Down
201 changes: 174 additions & 27 deletions test/oas3/OperationTest.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import FakeExegesisContext from '../fixtures/FakeExegesisContext';
chai.use(chaiAsPromised);
const {expect} = chai;

const BASIC_AUTH_SUCCESS_RESULT = { type: "success", user: 'benbria' };
const DEFAULT_OAUTH_RESULT = {type: 'success', user: 'benbria', scopes: ['admin']};

function makeOperation(
Expand Down Expand Up @@ -56,7 +57,6 @@ describe('oas3 Operation', function() {
this.operation = {
responses: {200: {description: "ok"}},
security: [
{basicAuth: []},
{oauth: ['admin']}
]
};
Expand All @@ -69,7 +69,6 @@ describe('oas3 Operation', function() {
const operation: Operation = makeOperation('get', this.operation);

expect(operation.securityRequirements).to.eql([
{basicAuth: []},
{oauth: ['admin']}
]);
});
Expand All @@ -91,7 +90,6 @@ describe('oas3 Operation', function() {
});

expect(operation.securityRequirements).to.eql([
{basicAuth: []},
{oauth: ['admin']}
]);
});
Expand All @@ -118,7 +116,6 @@ describe('oas3 Operation', function() {
it('should fail to authenticate an incoming request if no credentials are provided', async function() {
const options = {
authenticators: {
basicAuth() {return undefined;},
oauth() {return undefined;}
}
};
Expand All @@ -127,26 +124,25 @@ describe('oas3 Operation', function() {
await operation.authenticate(this.exegesisContext);
expect(this.exegesisContext.res.statusCode).to.equal(401);
expect(this.exegesisContext.res.body).to.eql({
message: 'Must authenticate using one of the following schemes: basicAuth, oauth.'
message: 'Must authenticate using one of the following schemes: oauth.'
});
expect(this.exegesisContext.res.headers['www-authenticate']).to.eql(['Basic', 'Bearer']);
expect(this.exegesisContext.res.headers['www-authenticate']).to.eql(['Bearer']);
});

it('should fail to authenticate an incoming request if one authenticator hard-fails', async function() {
it('should set response message to failed authenticator message if set', async function() {
const options = {
authenticators: {
basicAuth() {return {type: 'invalid'};},
oauth() {return {type: 'success'};}
oauth() {return { type: 'invalid', message: 'Bearer token expired'};}
}
};

const operation: Operation = makeOperation('get', this.operation, {options});
await operation.authenticate(this.exegesisContext);
expect(this.exegesisContext.res.statusCode).to.equal(401);
expect(this.exegesisContext.res.body).to.eql({
message: 'Must authenticate using one of the following schemes: basicAuth, oauth.'
message: 'Bearer token expired'
});
expect(this.exegesisContext.res.headers['www-authenticate']).to.eql(['Basic', 'Bearer']);
expect(this.exegesisContext.res.headers['www-authenticate']).to.eql(['Bearer']);
});

it('should fail to auth an incoming request if the user does not have the correct scopes', async function() {
Expand All @@ -161,25 +157,9 @@ describe('oas3 Operation', function() {
});
});

it('should fail to authenticate if user matches one security scheme but not the other', async function() {
this.operation.security = [{
basicAuth: [],
oauth: ['admin']
}];
const operation: Operation = makeOperation('get', this.operation);

await operation.authenticate(this.exegesisContext);
expect(this.exegesisContext.res.statusCode).to.equal(401);
expect(this.exegesisContext.res.body).to.eql({
message: 'Must authenticate using one of the following schemes: (basicAuth + oauth).'
});
expect(this.exegesisContext.res.headers['www-authenticate']).to.eql(['Basic', 'Bearer']);
});

it('should always authenticate a request with no security requirements', async function() {
const options = {
authenticators: {
basicAuth() {return undefined;},
oauth() {return undefined;}
}
};
Expand All @@ -190,6 +170,173 @@ describe('oas3 Operation', function() {
expect(authenticated).to.eql({});
});

describe('Security schemes combined via OR', async function() {
it('should return result of first successful authenticator', async function() {
const options = {
authenticators: {
basicAuth() {
return BASIC_AUTH_SUCCESS_RESULT;
},
oauth() {
return DEFAULT_OAUTH_RESULT;
}
}
};

const op: oas3.OperationObject = {
responses: {200: {description: "ok"}},
security: [
{basicAuth: []},
{oauth: ['admin']},
]
};

const operation: Operation = makeOperation('get', op, {options});
const authenticated = await operation.authenticate(this.exegesisContext);
expect(authenticated).to.exist;
expect(authenticated).to.eql({
basicAuth: BASIC_AUTH_SUCCESS_RESULT
});
});

it('should authenticate an incoming request if one authenticator hard-fails', async function() {
const options = {
authenticators: {
basicAuth() {
return {type: 'invalid'};
},
oauth() {
return DEFAULT_OAUTH_RESULT;
}
}
};

const op: oas3.OperationObject = {
responses: {200: {description: "ok"}},
security: [
{basicAuth: []},
{oauth: ['admin']},
]
};

const operation: Operation = makeOperation('get', op, {options});
const authenticated = await operation.authenticate(this.exegesisContext);
expect(authenticated).to.exist;
expect(authenticated).to.eql({
oauth: DEFAULT_OAUTH_RESULT
});
});

it('should set message from first authenticator if it all authenticators hard-fail', async function() {
const options = {
authenticators: {
basicAuth() {
return {type: 'invalid', message:'Invalid details'};
},
oauth() {
return {type: 'invalid', message:'Token expired'};
}
}
};

const op: oas3.OperationObject = {
responses: {200: {description: "ok"}},
security: [
{basicAuth: []},
{oauth: ['admin']},
]
};

const operation: Operation = makeOperation('get', op, {options});
const authenticated = await operation.authenticate(this.exegesisContext);
expect(authenticated).to.not.exist;
expect(this.exegesisContext.res.statusCode).to.equal(401);
expect(this.exegesisContext.res.body).to.eql({
message:'Invalid details'
});
expect(this.exegesisContext.res.headers['www-authenticate']).to.eql(['Basic', 'Bearer']);
});
});

describe('Security schemes combined via AND', async function() {
it('should authenticate an incoming request if all authenticators succeed', async function() {
const options = {
authenticators: {
basicAuth() {return BASIC_AUTH_SUCCESS_RESULT;},
oauth() {return DEFAULT_OAUTH_RESULT;}
}
};

const op :oas3.OperationObject= {
responses: {200: {description: "ok"}},
security: [
{oauth: [], basicAuth: [],},
]
};

const operation: Operation = makeOperation('get', op, {options});
const authenticated = await operation.authenticate(this.exegesisContext);
expect(authenticated).to.exist;
expect(authenticated).to.eql({
basicAuth: BASIC_AUTH_SUCCESS_RESULT,
oauth: DEFAULT_OAUTH_RESULT
});
});

it('should not authenticate an incoming request if one of the authenticators fail', async function() {
const options = {
authenticators: {
basicAuth() {return BASIC_AUTH_SUCCESS_RESULT;},
oauth() {return { type: 'invalid' };}
}
};

const op :oas3.OperationObject= {
responses: {200: {description: "ok"}},
security: [
{oauth: [], basicAuth: [],},
]
};

const operation: Operation = makeOperation('get', op, {options});
const authenticated = await operation.authenticate(this.exegesisContext);
expect(authenticated).to.not.exist;
expect(this.exegesisContext.res.statusCode).to.equal(401);
expect(this.exegesisContext.res.body).to.eql({
message: 'Must authenticate using one of the following schemes: (oauth + basicAuth).'
});
expect(this.exegesisContext.res.headers['www-authenticate']).to.eql(['Bearer', 'Basic']);
});

it('should set message from first authenticator if it hard-fails', async function() {
const options = {
authenticators: {
basicAuth() {
return {type: 'invalid', message:'Invalid details'};
},
oauth() {
return {type: 'invalid', message:'Token expired'};
}
}
};

const op: oas3.OperationObject = {
responses: {200: {description: "ok"}},
security: [
{oauth: [], basicAuth: [],},
]
};

const operation: Operation = makeOperation('get', op, {options});
const authenticated = await operation.authenticate(this.exegesisContext);
expect(authenticated).to.not.exist;
expect(this.exegesisContext.res.statusCode).to.equal(401);
expect(this.exegesisContext.res.body).to.eql({
message:'Token expired'
});
expect(this.exegesisContext.res.headers['www-authenticate']).to.eql(['Bearer', 'Basic']);
});
});
});

describe('body', function() {
Expand Down

0 comments on commit 7d1e7fc

Please sign in to comment.