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, id }) => {
-            const labels = { [labelName]: id };
+        resources.map(async resource => {
+            const labels = { [labelName]: resource };
             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 { translateAndAuthorize } = require('../vault');
+const { authenticateRequest, vault } = require('../vault');
 
 const oasOptions = {
     controllers: path.join(__dirname, './API/'),
@@ -44,17 +44,6 @@ 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) {
@@ -81,7 +70,15 @@ 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
@@ -114,7 +111,7 @@ async function authV4Middleware(request, response, params) {
     let authorizedResources;
 
     try {
-        [passed, authorizedResources] = await translateAndAuthorize(request, action, params.level, requestedResources);
+        [passed, authorizedResources] = await authenticateRequest(request, action, params.level, requestedResources);
     } catch (error) {
         request.logger.error('error during authentication', { error });
         throw errors.InternalError;
@@ -125,14 +122,17 @@ async function authV4Middleware(request, response, params) {
         throw errors.AccessDenied;
     }
 
-    switch (request.ctx.operationId) {
-    case 'listMetrics':
-        params.body[params.level] = authorizedResources;
-        break;
+    if (params.level === 'accounts') {
+        request.logger.debug('converting account ids to canonical ids');
+        authorizedResources = await vault.getCanonicalIds(
+            authorizedResources,
+            request.logger.logger,
+        );
+    }
 
-    default:
-        [params.resource] = authorizedResources;
-        break;
+    // authorizedResources is only defined on non-account credentials
+    if (request.ctx.operationId === 'listMetrics' && authorizedResources !== undefined) {
+        params.body[params.level] = authorizedResources;
     }
 }
 

libV2/vault.js

@@ -2,7 +2,6 @@ const assert = require('assert');
 const { auth, policies } = require('arsenal');
 const vaultclient = require('vaultclient');
 const config = require('./config');
-const errors = require('./errors');
 
 /**
 @class Vault
@@ -84,14 +83,7 @@ class Vault {
                         reject(err);
                         return;
                     }
-                    if (!res.message || !res.message.body) {
-                        reject(errors.InternalError);
-                        return;
-                    }
-                    resolve(res.message.body.map(acc => ({
-                        resource: acc.accountId,
-                        id: acc.canonicalId,
-                    })));
+                    resolve(res);
                 }));
     }
 }
@@ -99,15 +91,6 @@ 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,
@@ -131,11 +114,10 @@ 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 {
-                    authorizedResources = res.reduce(
-                        (authed, result) => {
+                    const authorizedResources = (res || [])
+                        .reduce((authed, result) => {
                             if (result.isAllowed) {
                                 // result.arn should be of format:
                                 // arn:scality:utapi:::resourcetype/resource
@@ -146,32 +128,24 @@ 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,16 +38,14 @@ 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({}, req, resp);
+            middleware.errorMiddleware({}, null, resp);
             assert.strictEqual(resp._status, 500);
             assert.deepStrictEqual(resp._body, {
                 error: {
@@ -58,7 +56,7 @@ describe('Test middleware', () => {
         });
 
         it('should set the correct info from an error', () => {
-            middleware.errorMiddleware({ code: 123, message: 'Hello World!', utapiError: true }, req, resp);
+            middleware.errorMiddleware({ code: 123, message: 'Hello World!', utapiError: true }, null, resp);
             assert.deepStrictEqual(resp._body, {
                 error: {
                     code: '123',
@@ -68,7 +66,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!' }, req, resp);
+            middleware.errorMiddleware({ code: 123, message: 'Hello World!' }, null, resp);
             assert.deepStrictEqual(resp._body, {
                 error: {
                     code: '123',
@@ -76,16 +74,5 @@ 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,10 +7,6 @@ class ExpressResponseStub {
         this._redirect = null;
     }
 
-    get statusCode() {
-        return this._status;
-    }
-
     status(code) {
         this._status = code;
         return this;