Skip to content

Commit

Permalink
Change inject auth to use object. Closes #3887
Browse files Browse the repository at this point in the history
  • Loading branch information
hueniverse committed Jan 6, 2019
1 parent 91d3b84 commit f02cdf1
Show file tree
Hide file tree
Showing 6 changed files with 69 additions and 46 deletions.
21 changes: 13 additions & 8 deletions API.md
Original file line number Diff line number Diff line change
Expand Up @@ -2108,14 +2108,19 @@ injections, with some additional options and response properties:
an object it will be converted to a string for you. Defaults to no payload. Note that payload
processing defaults to `'application/json'` if no 'Content-Type' header provided.

- `credentials` - (optional) an credentials object containing authentication information. The
`credentials` are used to bypass the default authentication strategies, and are validated
directly as if they were received via an authentication scheme. Defaults to no credentials.

- `artifacts` - (optional) an artifacts object containing authentication artifact information.
The `artifacts` are used to bypass the default authentication strategies, and are validated
directly as if they were received via an authentication scheme. Ignored if set without
`credentials`. Defaults to no artifacts.
- `auth` - (optional) an object containing parsed authentication credentials where:

- `strategy` - (required) the authentication strategy name matching the provided
credentials.

- `credentials` - (required) a credentials object containing authentication information.
The `credentials` are used to bypass the default authentication strategies, and are
validated directly as if they were received via an authentication scheme.

- `artifacts` - (optional) an artifacts object containing authentication artifact
information. The `artifacts` are used to bypass the default authentication strategies,
and are validated directly as if they were received via an authentication scheme.
Defaults to no artifacts.

- `app` - (optional) sets the initial value of `request.app`, defaults to `{}`.

Expand Down
18 changes: 6 additions & 12 deletions lib/auth.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ exports = module.exports = internals.Auth = class {

Hoek.assert(name, 'Authentication strategy must have a name');
Hoek.assert(typeof options === 'object', 'options must be an object');
Hoek.assert(name !== 'bypass', 'Cannot use reserved strategy name: bypass');
Hoek.assert(!this._strategies[name], 'Authentication strategy name already exists');
Hoek.assert(scheme, 'Authentication strategy', name, 'missing scheme');
Hoek.assert(this._schemes[scheme], 'Authentication strategy', name, 'uses unknown scheme:', scheme);
Expand Down Expand Up @@ -110,10 +109,6 @@ exports = module.exports = internals.Auth = class {
return;
}

if (auth.strategy === 'bypass') {
return;
}

const strategy = this._strategies[auth.strategy];
Hoek.assert(strategy, 'Unknown authentication strategy:', auth.strategy);

Expand Down Expand Up @@ -247,7 +242,7 @@ exports = module.exports = internals.Auth = class {
// Injection bypass

if (request.auth.credentials) {
internals.validate(null, { credentials: request.auth.credentials, artifacts: request.auth.artifacts }, 'bypass', config, request, errors);
internals.validate(null, { credentials: request.auth.credentials, artifacts: request.auth.artifacts }, request.auth.strategy, config, request, errors);
return;
}

Expand Down Expand Up @@ -364,14 +359,13 @@ exports = module.exports = internals.Auth = class {

static async payload(request) {

if (!request.auth.isAuthenticated ||
request.auth.strategy === 'bypass') {

if (!request.auth.isAuthenticated) {
return;
}

const auth = request._core.auth;
const strategy = auth._strategies[request.auth.strategy];
Hoek.assert(strategy, 'Unknown authentication strategy:', request.auth.strategy);

if (!strategy.methods.payload) {
return;
Expand Down Expand Up @@ -399,13 +393,13 @@ exports = module.exports = internals.Auth = class {
static async response(request) {

const auth = request._core.auth;
if (!request.auth.isAuthenticated ||
request.auth.strategy === 'bypass') {

if (!request.auth.isAuthenticated) {
return;
}

const strategy = auth._strategies[request.auth.strategy];
Hoek.assert(strategy, 'Unknown authentication strategy:', request.auth.strategy);

if (!strategy.methods.response) {
return;
}
Expand Down
6 changes: 3 additions & 3 deletions lib/request.js
Original file line number Diff line number Diff line change
Expand Up @@ -62,9 +62,9 @@ exports = module.exports = internals.Request = class {
this.auth = {
isAuthenticated: false,
isAuthorized: false,
credentials: options.credentials || null, // Special keys: 'app', 'user', 'scope'
artifacts: options.artifacts || null, // Scheme-specific artifacts
strategy: null,
credentials: options.auth ? options.auth.credentials : null, // Special keys: 'app', 'user', 'scope'
artifacts: options.auth && options.auth.artifacts || null, // Scheme-specific artifacts
strategy: options.auth ? options.auth.strategy : null,
mode: null,
error: null
};
Expand Down
16 changes: 11 additions & 5 deletions lib/server.js
Original file line number Diff line number Diff line change
Expand Up @@ -279,24 +279,30 @@ internals.Server = class {
}

if (!settings.authority ||
settings.credentials ||
settings.auth ||
settings.app ||
settings.plugins ||
settings.allowInternals !== undefined) { // Can be false

settings = Object.assign({}, settings); // options can be reused (shallow cloned)
delete settings.credentials;
delete settings.artifacts; // Cannot appear without credentials
delete settings.auth;
delete settings.app;
delete settings.plugins;
delete settings.allowInternals;

settings.authority = settings.authority || (this._core.info.host + ':' + this._core.info.port);
}

Hoek.assert(!options.credentials, 'options.credentials no longer supported (use options.auth)');

if (options.auth) {
Hoek.assert(typeof options.auth === 'object', 'options.auth must be an object');
Hoek.assert(options.auth.credentials, 'options.auth.credentials is missing');
Hoek.assert(options.auth.strategy, 'options.auth.strategy is missing');
}

const needle = this._core._dispatch({
credentials: options.credentials,
artifacts: options.artifacts,
auth: options.auth,
allowInternals: options.allowInternals,
app: options.app,
plugins: options.plugins
Expand Down
22 changes: 14 additions & 8 deletions test/auth.js
Original file line number Diff line number Diff line change
Expand Up @@ -419,7 +419,7 @@ describe('authentication', () => {

const doubleHandler = async (request) => {

const options = { url: '/2', credentials: request.auth.credentials };
const options = { url: '/2', auth: { credentials: request.auth.credentials, strategy: 'default' } };
const res = await server.inject(options);
return res.result;
};
Expand All @@ -441,7 +441,7 @@ describe('authentication', () => {

const doubleHandler = async (request) => {

const options = { url: '/2', credentials: request.auth.credentials, artifacts: '!' };
const options = { url: '/2', auth: { credentials: request.auth.credentials, artifacts: '!', strategy: 'default' } };
const res = await server.inject(options);
return res.result;
};
Expand Down Expand Up @@ -1272,8 +1272,11 @@ describe('authentication', () => {
const options = {
url: '/',
headers: { authorization: 'Custom steve' },
credentials: { foo: 'bar' },
artifacts: { bar: 'baz' }
auth: {
credentials: { foo: 'bar' },
artifacts: { bar: 'baz' },
strategy: 'default'
}
};

const res = await server.inject(options);
Expand Down Expand Up @@ -1375,11 +1378,14 @@ describe('authentication', () => {
const res3 = await server.inject({ url: '/', headers: { authorization: 'Custom unknown' } });
expect(res3.result.message).to.equal('Missing credentials');

const res4 = await server.inject({ url: '/', credentials: {} });
expect(res4.result).to.equal('ok');
const res4 = await server.inject({ url: '/', auth: { credentials: {}, strategy: 'default' } });
expect(res4.result.message).to.equal('Invalid');

const res5 = await server.inject({ url: '/', auth: { credentials: { user: 'steve' }, strategy: 'default' } });
expect(res5.result).to.equal('ok');

const res5 = await server.inject({ url: '/', headers: { authorization: 'Custom john' } });
expect(res5.result.message).to.equal('Invalid');
const res6 = await server.inject({ url: '/', headers: { authorization: 'Custom john' } });
expect(res6.result.message).to.equal('Invalid');
});

it('skips when verify unsupported', async () => {
Expand Down
32 changes: 22 additions & 10 deletions test/core.js
Original file line number Diff line number Diff line change
Expand Up @@ -984,12 +984,15 @@ describe('Core', () => {

const options = {
url: '/',
credentials: { foo: 'bar' }
auth: {
credentials: { foo: 'bar' },
strategy: 'test'
}
};

const res = await server.inject(options);
expect(res.statusCode).to.equal(200);
expect(options.credentials).to.exist();
expect(options.auth.credentials).to.exist();
});

it('sets credentials (with host header)', async () => {
Expand All @@ -999,15 +1002,18 @@ describe('Core', () => {

const options = {
url: '/',
credentials: { foo: 'bar' },
auth: {
credentials: { foo: 'bar' },
strategy: 'test'
},
headers: {
host: 'something'
}
};

const res = await server.inject(options);
expect(res.statusCode).to.equal(200);
expect(options.credentials).to.exist();
expect(options.auth.credentials).to.exist();
});

it('sets credentials (with authority)', async () => {
Expand All @@ -1017,14 +1023,17 @@ describe('Core', () => {

const options = {
url: '/',
credentials: { foo: 'bar' },
authority: 'something'
authority: 'something',
auth: {
credentials: { foo: 'bar' },
strategy: 'test'
}
};

const res = await server.inject(options);
expect(res.statusCode).to.equal(200);
expect(res.result).to.equal('something');
expect(options.credentials).to.exist();
expect(options.auth.credentials).to.exist();
});

it('sets authority', async () => {
Expand All @@ -1049,14 +1058,17 @@ describe('Core', () => {

const options = {
url: '/',
credentials: { foo: 'bar' },
artifacts: { bar: 'baz' }
auth: {
credentials: { foo: 'bar' },
artifacts: { bar: 'baz' },
strategy: 'test'
}
};

const res = await server.inject(options);
expect(res.statusCode).to.equal(200);
expect(res.result.bar).to.equal('baz');
expect(options.artifacts).to.exist();
expect(options.auth.artifacts).to.exist();
});

it('sets app settings', async () => {
Expand Down

0 comments on commit f02cdf1

Please sign in to comment.