Skip to content

Commit 4307b00

Browse files
committed
Refactor auth access config. Closes #2992. Closes #2998
1 parent baf7b9c commit 4307b00

File tree

3 files changed

+173
-57
lines changed

3 files changed

+173
-57
lines changed

lib/auth.js

Lines changed: 73 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -117,15 +117,25 @@ internals.Auth.prototype._setupRoute = function (options, path) {
117117

118118
options.mode = options.mode || 'required';
119119

120-
if (options.scope) {
121-
if (typeof options.scope === 'string') {
122-
options.scope = [options.scope];
123-
}
120+
if (options.entity !== undefined ||
121+
options.scope !== undefined) {
122+
123+
options.access = [{ entity: options.entity, scope: options.scope }];
124+
delete options.entity;
125+
delete options.scope;
126+
}
124127

125-
for (let i = 0; i < options.scope.length; ++i) {
126-
if (/{([^}]+)}/.test(options.scope[i])) {
127-
options.hasScopeParameters = true;
128-
break;
128+
if (options.access) {
129+
for (let i = 0; i < options.access.length; ++i) {
130+
const access = options.access[i];
131+
if (access.scope) { // Avoid processing the defaults
132+
access.hasScopeParameters = false;
133+
for (let j = 0; j < access.scope.length; ++j) {
134+
if (/{([^}]+)}/.test(access.scope[j])) {
135+
access.hasScopeParameters = true;
136+
break;
137+
}
138+
}
129139
}
130140
}
131141
}
@@ -135,14 +145,15 @@ internals.Auth.prototype._setupRoute = function (options, path) {
135145
}
136146

137147
let hasAuthenticatePayload = false;
138-
options.strategies.forEach((name) => {
139-
148+
for (let i = 0; i < options.strategies.length; ++i) {
149+
const name = options.strategies[i];
140150
const strategy = this._strategies[name];
141151
Hoek.assert(strategy, 'Unknown authentication strategy', name, 'in', path);
152+
142153
Hoek.assert(strategy.methods.payload || options.payload !== 'required', 'Payload validation can only be required when all strategies support it in', path);
143154
hasAuthenticatePayload = hasAuthenticatePayload || strategy.methods.payload;
144155
Hoek.assert(!strategy.methods.options.payload || options.payload === undefined || options.payload === 'required', 'Cannot set authentication payload to', options.payload, 'when a strategy requires payload validation in', path);
145-
});
156+
}
146157

147158
Hoek.assert(!options.payload || hasAuthenticatePayload, 'Payload authentication requires at least one strategy with payload support in', path);
148159

@@ -363,60 +374,73 @@ internals.Authenticator = class {
363374
request.auth.credentials = credentials;
364375
request.auth.artifacts = result.artifacts;
365376

366-
// Check scope
377+
const authenticated = () => {
367378

368-
if (config.scope) {
369-
let scopes = config.scope;
370-
if (config.hasScopeParameters) {
371-
scopes = [];
372-
const context = { params: request.params, query: request.query };
373-
for (let i = 0; i < config.scope.length; ++i) {
374-
scopes[i] = Hoek.reachTemplate(context, config.scope[i]);
375-
}
376-
}
379+
request._log(['auth', name]);
380+
request.auth.isAuthenticated = true;
381+
return next();
382+
};
377383

378-
if (!credentials.scope ||
379-
(typeof credentials.scope === 'string' ? scopes.indexOf(credentials.scope) === -1 : !Hoek.intersect(scopes, credentials.scope).length)) {
384+
// Check access rules
380385

381-
request._log(['auth', 'scope', 'error', name], { got: credentials.scope, need: scopes });
382-
return next(Boom.forbidden('Insufficient scope, expected any of: ' + scopes));
383-
}
386+
if (!config.access) {
387+
return authenticated();
384388
}
385389

386-
// Check entity
390+
const requestEntity = (credentials.user ? 'user' : 'app');
387391

388-
const entity = config.entity || 'any';
392+
const scopeErrors = [];
393+
for (let i = 0; i < config.access.length; ++i) {
394+
const access = config.access[i];
389395

390-
// Entity: 'any'
396+
// Check entity
391397

392-
if (entity === 'any') {
393-
request._log(['auth', name]);
394-
request.auth.isAuthenticated = true;
395-
return next();
396-
}
398+
const entity = access.entity;
399+
if (entity &&
400+
entity !== 'any' &&
401+
entity !== requestEntity) {
402+
403+
continue;
404+
}
397405

398-
// Entity: 'user'
406+
// Check scope
399407

400-
if (entity === 'user') {
401-
if (!credentials.user) {
402-
request._log(['auth', 'entity', 'user', 'error', name]);
403-
return next(Boom.forbidden('Application credentials cannot be used on a user endpoint'));
408+
let scope = access.scope;
409+
if (scope) {
410+
if (access.hasScopeParameters) {
411+
scope = [];
412+
const context = { params: request.params, query: request.query };
413+
for (let j = 0; j < access.scope.length; ++j) {
414+
scope[j] = Hoek.reachTemplate(context, access.scope[j]);
415+
}
416+
}
417+
418+
if (!credentials.scope ||
419+
(typeof credentials.scope === 'string' ? scope.indexOf(credentials.scope) === -1 : !Hoek.intersect(scope, credentials.scope).length)) {
420+
421+
scopeErrors.push(scope);
422+
continue;
423+
}
404424
}
405425

406-
request._log(['auth', name]);
407-
request.auth.isAuthenticated = true;
408-
return next();
426+
return authenticated();
409427
}
410428

411-
// Entity: 'app'
429+
// Scope error
412430

413-
if (credentials.user) {
414-
request._log(['auth', 'entity', 'app', 'error', name]);
415-
return next(Boom.forbidden('User credentials cannot be used on an application endpoint'));
431+
if (scopeErrors.length) {
432+
request._log(['auth', 'scope', 'error', name], { got: credentials.scope, need: scopeErrors });
433+
return next(Boom.forbidden('Insufficient scope'));
416434
}
417435

418-
request._log(['auth', name]);
419-
request.auth.isAuthenticated = true;
420-
return next();
436+
// Entity error
437+
438+
if (requestEntity === 'user') {
439+
request._log(['auth', 'entity', 'user', 'error', name]);
440+
return next(Boom.forbidden('Application credentials cannot be used on a user endpoint'));
441+
}
442+
443+
request._log(['auth', 'entity', 'app', 'error', name]);
444+
return next(Boom.forbidden('User credentials cannot be used on an application endpoint'));
421445
}
422446
};

lib/schema.js

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -31,24 +31,26 @@ internals.cache = Joi.object({
3131
}).unknown();
3232

3333

34-
internals.strategy = Joi.object({
34+
internals.access = Joi.object({
3535
entity: Joi.string().valid('user', 'app', 'any'),
36-
payload: [
37-
Joi.string().valid('required', 'optional'),
38-
Joi.boolean()
39-
],
40-
scope: [false, Joi.string(), Joi.array()]
36+
scope: [false, Joi.array().items(Joi.string()).single()]
4137
});
4238

4339

4440
internals.auth = Joi.alternatives([
4541
Joi.string(),
46-
internals.strategy.keys({
42+
internals.access.keys({
4743
mode: Joi.string().valid('required', 'optional', 'try'),
4844
strategy: Joi.string(),
49-
strategies: Joi.array().items(Joi.string(), internals.strategy.keys({ name: Joi.string().required() })).min(1)
45+
strategies: Joi.array().items(Joi.string()).min(1),
46+
access: Joi.array().items(internals.access.or('entity', 'scope')).single().min(1),
47+
payload: [
48+
Joi.string().valid('required', 'optional'),
49+
Joi.boolean()
50+
]
5051
})
5152
.without('strategy', 'strategies')
53+
.without('access', ['scope', 'entity'])
5254
]);
5355

5456

test/auth.js

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1074,6 +1074,96 @@ describe('authentication', () => {
10741074
});
10751075
});
10761076

1077+
it('matches scope (access single)', (done) => {
1078+
1079+
const handler = function (request, reply) {
1080+
1081+
return reply(request.auth.credentials.user);
1082+
};
1083+
1084+
const server = new Hapi.Server();
1085+
server.connection();
1086+
server.auth.scheme('custom', internals.implementation);
1087+
server.auth.strategy('default', 'custom', true, { users: { steve: { scope: ['one'] } } });
1088+
server.route({
1089+
method: 'GET',
1090+
path: '/',
1091+
config: {
1092+
handler: handler,
1093+
auth: {
1094+
access: {
1095+
scope: 'one'
1096+
}
1097+
}
1098+
}
1099+
});
1100+
1101+
server.inject({ url: '/', headers: { authorization: 'Custom steve' } }, (res) => {
1102+
1103+
expect(res.statusCode).to.equal(200);
1104+
done();
1105+
});
1106+
});
1107+
1108+
it('matches scope (access array)', (done) => {
1109+
1110+
const handler = function (request, reply) {
1111+
1112+
return reply(request.auth.credentials.user);
1113+
};
1114+
1115+
const server = new Hapi.Server();
1116+
server.connection();
1117+
server.auth.scheme('custom', internals.implementation);
1118+
server.auth.strategy('default', 'custom', true, { users: { steve: { scope: ['one'] } } });
1119+
server.route({
1120+
method: 'GET',
1121+
path: '/',
1122+
config: {
1123+
handler: handler,
1124+
auth: {
1125+
access: [
1126+
{ scope: 'other' },
1127+
{ scope: 'one' }
1128+
]
1129+
}
1130+
}
1131+
});
1132+
1133+
server.inject({ url: '/', headers: { authorization: 'Custom steve' } }, (res) => {
1134+
1135+
expect(res.statusCode).to.equal(200);
1136+
done();
1137+
});
1138+
});
1139+
1140+
it('matches any entity', (done) => {
1141+
1142+
const server = new Hapi.Server();
1143+
server.connection();
1144+
server.auth.scheme('custom', internals.implementation);
1145+
server.auth.strategy('default', 'custom', true, { users: { steve: { user: 'steve' } } });
1146+
server.route({
1147+
method: 'GET',
1148+
path: '/',
1149+
config: {
1150+
handler: function (request, reply) {
1151+
1152+
return reply(request.auth.credentials.user);
1153+
},
1154+
auth: {
1155+
entity: 'any'
1156+
}
1157+
}
1158+
});
1159+
1160+
server.inject({ url: '/', headers: { authorization: 'Custom steve' } }, (res) => {
1161+
1162+
expect(res.statusCode).to.equal(200);
1163+
done();
1164+
});
1165+
});
1166+
10771167
it('matches user entity', (done) => {
10781168

10791169
const server = new Hapi.Server();

0 commit comments

Comments
 (0)