Compare commits

..

4 Commits

Author SHA1 Message Date
Taylor McKinnon d5abb103ec fixup 2020-11-10 17:03:50 -08:00
Taylor McKinnon 860a9300d9 Merge branch 'bugfix/S3C-3514_add_missing_logline' into test/7.8.0.1 2020-11-10 11:26:55 -08:00
Taylor McKinnon 6870bc91ca bf(S3C-3516): Rework authz and accountId conversion 2020-11-09 16:25:03 -08:00
Taylor McKinnon 64c2f7307a bf(S3C-3514): Add missing call to responseLoggerMiddleware in errorMiddleware 2020-11-09 15:46:21 -08:00
5 changed files with 78 additions and 35 deletions

View File

@ -26,8 +26,8 @@ async function listMetric(ctx, params) {
// A separate request will be made to warp 10 per requested resource
const results = await Promise.all(
resources.map(async resource => {
const labels = { [labelName]: resource };
resources.map(async ({ resource, id }) => {
const labels = { [labelName]: id };
const options = {
params: {
start,

View File

@ -5,7 +5,7 @@ const { ipCheck } = require('arsenal');
const config = require('../config');
const { logger, buildRequestLogger } = require('../utils');
const errors = require('../errors');
const { authenticateRequest, vault } = require('../vault');
const { translateAndAuthorize } = require('../vault');
const oasOptions = {
controllers: path.join(__dirname, './API/'),
@ -44,6 +44,17 @@ function loggerMiddleware(req, res, next) {
return next();
}
function responseLoggerMiddleware(req, res, next) {
const info = {
httpCode: res.statusCode,
httpMessage: res.statusMessage,
};
req.logger.end('finished handling request', info);
if (next !== undefined) {
next();
}
}
// next is purposely not called as all error responses are handled here
// eslint-disable-next-line no-unused-vars
function errorMiddleware(err, req, res, next) {
@ -70,15 +81,7 @@ function errorMiddleware(err, req, res, next) {
message,
},
});
}
function responseLoggerMiddleware(req, res, next) {
const info = {
httpCode: res.statusCode,
httpMessage: res.statusMessage,
};
req.logger.end('finished handling request', info);
return next();
responseLoggerMiddleware(req, res);
}
// eslint-disable-next-line no-unused-vars
@ -111,7 +114,7 @@ async function authV4Middleware(request, response, params) {
let authorizedResources;
try {
[passed, authorizedResources] = await authenticateRequest(request, action, params.level, requestedResources);
[passed, authorizedResources] = await translateAndAuthorize(request, action, params.level, requestedResources);
} catch (error) {
request.logger.error('error during authentication', { error });
throw errors.InternalError;
@ -122,17 +125,14 @@ async function authV4Middleware(request, response, params) {
throw errors.AccessDenied;
}
if (params.level === 'accounts') {
request.logger.debug('converting account ids to canonical ids');
authorizedResources = await vault.getCanonicalIds(
authorizedResources,
request.logger.logger,
);
}
// authorizedResources is only defined on non-account credentials
if (request.ctx.operationId === 'listMetrics' && authorizedResources !== undefined) {
switch (request.ctx.operationId) {
case 'listMetrics':
params.body[params.level] = authorizedResources;
break;
default:
[params.resource] = authorizedResources;
break;
}
}

View File

@ -2,6 +2,7 @@ const assert = require('assert');
const { auth, policies } = require('arsenal');
const vaultclient = require('vaultclient');
const config = require('./config');
const errors = require('./errors');
/**
@class Vault
@ -83,7 +84,14 @@ class Vault {
reject(err);
return;
}
resolve(res);
if (!res.message || !res.message.body) {
reject(errors.InternalError);
return;
}
resolve(res.message.body.map(acc => ({
resource: acc.accountId,
id: acc.canonicalId,
})));
}));
}
}
@ -91,6 +99,15 @@ class Vault {
const vault = new Vault(config);
auth.setHandler(vault);
async function translateResourceIds(level, resources, log) {
if (level === 'accounts') {
const res = await vault.getCanonicalIds(resources, log);
return res;
}
return resources.map(resource => ({ resource, id: resource }));
}
async function authenticateRequest(request, action, level, resources) {
const policyContext = new policies.RequestContext(
request.headers,
@ -114,10 +131,11 @@ async function authenticateRequest(request, action, level, resources) {
return;
}
// Will only have res if request is from a user rather than an account
let authorizedResources = resources;
if (res) {
try {
const authorizedResources = (res || [])
.reduce((authed, result) => {
authorizedResources = res.reduce(
(authed, result) => {
if (result.isAllowed) {
// result.arn should be of format:
// arn:scality:utapi:::resourcetype/resource
@ -128,24 +146,32 @@ async function authenticateRequest(request, action, level, resources) {
request.logger.trace('access granted for resource', { resource });
}
return authed;
}, []);
resolve([
authorizedResources.length !== 0,
authorizedResources,
]);
}, [],
);
} catch (err) {
reject(err);
}
} else {
request.logger.trace('granted access to all resources');
resolve([true]);
}
resolve([
authorizedResources.length !== 0,
authorizedResources,
]);
}, 's3', [policyContext]);
});
}
async function translateAndAuthorize(request, action, level, resources) {
const [authed, authorizedResources] = await authenticateRequest(request, action, level, resources);
const translated = await translateResourceIds(level, authorizedResources, request.logger.logger);
return [authed, translated];
}
module.exports = {
authenticateRequest,
translateAndAuthorize,
Vault,
vault,
};

View File

@ -38,14 +38,16 @@ describe('Test middleware', () => {
});
describe('test errorMiddleware', () => {
let req;
let resp;
beforeEach(() => {
req = templateRequest();
resp = new ExpressResponseStub();
});
it('should set a default code and message', () => {
middleware.errorMiddleware({}, null, resp);
middleware.errorMiddleware({}, req, resp);
assert.strictEqual(resp._status, 500);
assert.deepStrictEqual(resp._body, {
error: {
@ -56,7 +58,7 @@ describe('Test middleware', () => {
});
it('should set the correct info from an error', () => {
middleware.errorMiddleware({ code: 123, message: 'Hello World!', utapiError: true }, null, resp);
middleware.errorMiddleware({ code: 123, message: 'Hello World!', utapiError: true }, req, resp);
assert.deepStrictEqual(resp._body, {
error: {
code: '123',
@ -66,7 +68,7 @@ describe('Test middleware', () => {
});
it("should replace an error's message if it's internal and not in development mode", () => {
middleware.errorMiddleware({ code: 123, message: 'Hello World!' }, null, resp);
middleware.errorMiddleware({ code: 123, message: 'Hello World!' }, req, resp);
assert.deepStrictEqual(resp._body, {
error: {
code: '123',
@ -74,5 +76,16 @@ describe('Test middleware', () => {
},
});
});
it('should call responseLoggerMiddleware after response', () => {
const spy = sinon.spy();
req.logger.end = spy;
resp.statusMessage = 'Hello World!';
middleware.errorMiddleware({ code: 123 }, req, resp);
assert(spy.calledOnceWith('finished handling request', {
httpCode: 123,
httpMessage: 'Hello World!',
}));
});
});
});

View File

@ -7,6 +7,10 @@ class ExpressResponseStub {
this._redirect = null;
}
get statusCode() {
return this._status;
}
status(code) {
this._status = code;
return this;