Skip to content

Commit

Permalink
throw boomified aggregate error, prevent logging of server errors, li…
Browse files Browse the repository at this point in the history
…nt test and example files
  • Loading branch information
jscheffner committed Nov 4, 2019
1 parent c98c24d commit faf305e
Show file tree
Hide file tree
Showing 8 changed files with 100 additions and 36 deletions.
10 changes: 9 additions & 1 deletion .eslintrc.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,5 +5,13 @@
"error",
{ "allow": ["_schemes"] }
]
}
},
"overrides": [
{
"files": ["examples/**/*.js"],
"rules": {
"import/no-unresolved": "off"
}
}
]
}
15 changes: 15 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -63,3 +63,18 @@ server.auth.strategy('any', 'any', {
server.auth.default('any');
```
## Debugging
Sometimes, the messages exposed to the end users are not enough to find the root of a problem, specifically if a strategy fails because of a server-side error.
To access the complete errors, including their stack, you can register an event listener:
```js
server.events.on({ name: 'request', channels: 'internal' }, (request, event, tags) => {
if (tags.auth && tags.error) {
console.log(event.error);
}
});
```
In this, debugging `hapi-auth-any` isn't different from debugging other strategies. However, in order to preserve the errors that caused `hapi-auth-any` to fail, it throws an [AggregateError](https://github.com/sindresorhus/aggregate-error), decorated with some [boom](https://github.com/hapijs/boom)-specific properties. `AggregateErrors` are iterable, which is how you can access the individual errors.
18 changes: 9 additions & 9 deletions examples/basic.js
Original file line number Diff line number Diff line change
@@ -1,23 +1,23 @@
const hapi = require('@hapi/hapi');
const anyAuth = require('hapi-auth-any');
const basic = require('@hapi/basic')
const basic = require('@hapi/basic');

const init = async () => {
const server = hapi.server();
await server.initialize();

await server.register([
anyAuth,
basic
basic,
]);
server.auth.strategy('simple', 'basic', { /* content omitted */ });

server.auth.strategy('simple', 'basic', { /* content omitted */ });
server.auth.strategy('not-so-simple', 'basic', { /* content omitted */ });

server.auth.strategy('any', 'any', {
strategies: ['simple', 'not-so-simple']
strategies: ['simple', 'not-so-simple'],
});
server.auth.default('any');
}
};

init()
init();
38 changes: 19 additions & 19 deletions examples/hapi-auth-keycloak-wrapper.js
Original file line number Diff line number Diff line change
@@ -1,51 +1,51 @@
/**
* Example of how to use hapi-auth-any to wrap [hapi-auth-keycloak](https://github.com/felixheck/hapi-auth-keycloak), so it is configurable with multiple realmURLs
*
*
* It accepts all the options that `hapi-auth-keycloak` does, plus a `realms` option.
*
*
* Example:
*
*
* ```javaScript
* {
* realms: [
* {
* name: 'foo',
* url: 'https://example.com/auth/realms/foo'
* url: 'https://example.com/auth/realms/foo'
* },
* {
* name: 'bar',
* url: 'https://example.com/auth/realms/bar'
* url: 'https://example.com/auth/realms/bar'
* }
* ],
* clientId: 'foobar'
* }
* ```
*/

const hapiKeycloak = require('hapi-auth-keycloak')
const anyAuth = require('hapi-auth-any')
const hapiKeycloak = require('hapi-auth-keycloak');
const anyAuth = require('hapi-auth-any');

const register = async (server, opts) => {
const { realms, ...hapiKeycloakOptions } = opts
const { realms, ...hapiKeycloakOptions } = opts;

await server.register([
{
plugin: hapiKeycloak,
options: hapiKeycloakOptions
options: hapiKeycloakOptions,
},
anyAuth
])
anyAuth,
]);

const strategies = realms.map(({ name, url }) => {
server.auth.strategy(name, 'keycloak-jwt', { name, realmUrl: url })
return name
})
server.auth.strategy(name, 'keycloak-jwt', { name, realmUrl: url });
return name;
});

server.auth.strategy('keycloak', 'any', { strategies })
server.auth.default('keycloak')
}
server.auth.strategy('keycloak', 'any', { strategies });
server.auth.default('keycloak');
};

module.exports = {
register,
name: 'auth'
}
name: 'auth',
};
9 changes: 7 additions & 2 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,10 @@ const test = (server, request) => async (strategy) => {
const [error, auth] = await to(server.auth.test(strategy, request));

if (error) {
throw new Error(`Strategy ${strategy}: ${error.message}`);
const preparedError = boom.boomify(error, { decorate: { strategy } });
preparedError.message = `Strategy ${strategy}: ${preparedError.message}`;
preparedError.output.payload.message = `Strategy ${strategy}: ${preparedError.output.payload.message}`;
throw preparedError;
}

return auth;
Expand All @@ -21,7 +24,9 @@ const authenticate = (server, strategies) => async (request, h) => {
const [errors, auth] = await to(pAny(strategies.map(test(server, request))));

if (errors) {
throw boom.unauthorized([...errors].map(({ message }) => message).join(', '));
const preparedAggregateError = boom.boomify(errors, { statusCode: 401 });
preparedAggregateError.output.payload.message = [...errors].map(({ output }) => output.payload.message).join(', ');
throw preparedAggregateError;
}

return h.authenticated(auth);
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
"main": "index.js",
"scripts": {
"test": "nyc ava",
"lint": "eslint test index.js",
"lint": "eslint test index.js test/**/*.js examples/**/*.js",
"format": "npm run lint -- --fix",
"coverage": "nyc report --reporter=text-lcov | coveralls"
},
Expand Down
40 changes: 38 additions & 2 deletions test/index.spec.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
const test = require('ava');
const { AggregateError } = require('p-any');
const { isBoom } = require('@hapi/boom');
const { setup: setupAll, setupServerAndPlugin, request } = require('./utils');

test('register plugin and create strategy', async (t) => {
Expand All @@ -14,11 +16,28 @@ test('fail if all fail', async (t) => {
t.is(result.message, 'Strategy strategy-0: Missing Credentials, Strategy strategy-1: Missing Credentials, Strategy strategy-2: Missing Credentials');
});

test('provide causes of failing strategies', async (t) => {
const server = await setupAll([false, false, false, false]);
const listener = new Promise((resolve) => {
server.events.on({ name: 'request', channels: 'internal' }, (req, event) => {
resolve(event.error);
});
});

request(server);
const err = await listener;

t.log(err);
t.true(err instanceof AggregateError);
t.true(isBoom(err));
t.is([...err].length, 4);
t.true([...err].every(isBoom));
});

test('succeed if all succeed', async (t) => {
const server = await setupAll([true, true, true, true]);
const { statusCode, result } = await request(server);
const { statusCode } = await request(server);

t.log(result);
t.is(statusCode, 200);
});

Expand All @@ -35,3 +54,20 @@ test('succeed if some succeeds', async (t) => {

t.is(statusCode, 200);
});

test('do not expose message of server errors', async (t) => {
const server = await setupServerAndPlugin();
server.auth.scheme('error', () => ({ authenticate: () => { throw new Error('Test Error'); } }));
server.auth.strategy('error', 'error');
server.auth.strategy('any', 'any', { strategies: ['error'] });
server.auth.default('any');

const { statusCode, result } = await request(server);
t.log(result);
t.is(statusCode, 401);
t.deepEqual(result, {
error: 'Unauthorized',
message: 'Strategy error: An internal server error occurred',
statusCode: 401,
});
});
4 changes: 2 additions & 2 deletions test/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,8 @@ const setupServerAndPlugin = async () => {
method: 'GET',
path: '/',
options: {
handler(_, h) {
return h.response();
handler() {
return 'authenticated';
},
},
});
Expand Down

0 comments on commit faf305e

Please sign in to comment.