fixup

libV2/server/API/metrics/listMetrics.js

@@ -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 => {
+        resources.map(async ({ resource, id }) => {
-            const labels = { [labelName]: resource };
+            const labels = { [labelName]: id };
             const options = {
                 params: {
                     start,

libV2/server/middleware.js

@@ -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,
         },
     });
-}
+    responseLoggerMiddleware(req, res);
-
-function responseLoggerMiddleware(req, res, next) {
-    const info = {
-        httpCode: res.statusCode,
-        httpMessage: res.statusMessage,
-    };
-    req.logger.end('finished handling request', info);
-    return next();
 }
 
 // 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') {
+    switch (request.ctx.operationId) {
-        request.logger.debug('converting account ids to canonical ids');
+    case 'listMetrics':
-        authorizedResources = await vault.getCanonicalIds(
-            authorizedResources,
-            request.logger.logger,
-        );
-    }
-
-    // authorizedResources is only defined on non-account credentials
-    if (request.ctx.operationId === 'listMetrics' && authorizedResources !== undefined) {
         params.body[params.level] = authorizedResources;
+        break;
+
+    default:
+        [params.resource] = authorizedResources;
+        break;
     }
 }
 

libV2/vault.js

@@ -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 || [])
+                    authorizedResources = res.reduce(
-                        .reduce((authed, result) => {
+                        (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,
 };

tests/unit/v2/server/testMiddleware.js

@@ -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!',
+            }));
+        });
     });
 });

tests/utils/v2Data/request.js

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