Merge branch 'main' into feature/forms-779-devcontainer

app/app.js

@@ -31,7 +31,7 @@ app.use(express.urlencoded({ extended: true }));
 // pulls the IP address from the headers, rather than use the proxy IP address.
 // This gives the correct IP address in the logs and for the rate limiting.
 // See https://express-rate-limit.github.io/ERR_ERL_UNEXPECTED_X_FORWARDED_FOR
-// app.set('trust proxy', 1);
+app.set('trust proxy', 1);
 
 // Skip if running tests
 if (process.env.NODE_ENV !== 'test') {

app/src/forms/auth/middleware/userAccess.js

@@ -22,7 +22,7 @@ const setUser = async (req, _res, next) => {
     // ex. /forms/:formId=ABC/version?formId=123
     // the ABC in the url will be used... so don't do that.
     const params = { ...req.query, ...req.params };
-    req.currentUser = await service.login(token, params);
+    req.currentUser = await service.login(token, params, req.chefsLoadForms);
     next();
   } catch (error) {
     next(error);
@@ -60,15 +60,45 @@ const currentUserTemp = async (req, res, next) => {
     if (!ok) {
       return new Problem(403, { detail: 'Authorization token is invalid.' }).send(res);
     }
+  }
+
+  // Temporarily set a flag to prevent expensive database calls. We will
+  // eventually move all routes to working in this way, and then the extra
+  // downstream logic can be removed.
+  req.chefsLoadForms = false;
+
+  return setUser(req, res, next);
+};
+
+// To deal with performance problems, we are going to move away from setting the
+// req.currentUser.forms and req.currentUser.deletedForms data. The code using
+// those will eventually be removed from this function.
+const _getForm = async (currentUser, formId) => {
+  let forms;
+  if (currentUser.forms) {
+    forms = currentUser.forms;
+  } else {
+    forms = await service.getUserForms(currentUser, { active: true, formId: formId });
+  }
+
+  let form = forms.find((f) => f.formId === formId);
 
-    return setUser(req, res, next);
+  if (!form) {
+    let deletedForms;
+    if (currentUser.deletedForms) {
+      deletedForms = currentUser.deletedForms;
+    } else {
+      deletedForms = await service.getUserForms(currentUser, { active: false, formId: formId });
+    }
+
+    form = deletedForms.find((f) => f.formId === formId);
   }
 
-  next();
+  return form;
 };
 
 const hasFormPermissions = (permissions) => {
-  return (req, res, next) => {
+  return async (req, res, next) => {
     // Skip permission checks if requesting as API entity
     if (req.apiUser) {
       return next();
@@ -84,16 +114,10 @@ const hasFormPermissions = (permissions) => {
       // No form provided to this route that secures based on form... that's a problem!
       return new Problem(401, { detail: 'Form Id not found on request.' }).send(res);
     }
-    let form = req.currentUser.forms.find((f) => f.formId === formId);
+    let form = await _getForm(req.currentUser, formId);
     if (!form) {
-      // check deleted... (this allows 404 on other queries later)
-      if (req.currentUser.deletedForms) {
-        form = req.currentUser.deletedForms.find((f) => f.formId === formId);
-      }
-      if (!form) {
-        // cannot find the form... guess we don't have access... FAIL!
-        return new Problem(401, { detail: 'Current user has no access to form.' }).send(res);
-      }
+      // cannot find the form... guess we don't have access... FAIL!
+      return new Problem(401, { detail: 'Current user has no access to form.' }).send(res);
     }
 
     if (!Array.isArray(permissions)) {
@@ -136,7 +160,14 @@ const hasSubmissionPermissions = (permissions) => {
 
       // Does the user have permissions for this submission due to their FORM permissions
       if (req.currentUser) {
-        let formFromCurrentUser = req.currentUser.forms.find((f) => f.formId === submissionForm.form.id);
+        let forms;
+        if (req.currentUser.forms) {
+          forms = req.currentUser.forms;
+        } else {
+          forms = await service.getUserForms(req.currentUser, { active: true, formId: submissionForm.form.id });
+        }
+
+        let formFromCurrentUser = forms.find((f) => f.formId === submissionForm.form.id);
         if (formFromCurrentUser) {
           // Do they have the submission permissions being requested on this FORM
           const intersection = permissions.filter((p) => {

app/src/forms/auth/service.js

@@ -201,12 +201,18 @@ const service = {
     };
   },
 
-  login: async (token, params = {}) => {
+  login: async (token, params = {}, loadForms = true) => {
     const userInfo = service.parseToken(token);
     const user = await service.getUserId(userInfo);
+
+    if (!loadForms) {
+      return { ...user };
+    }
+
     const forms = await service.getUserForms(user, params); // get forms for user (filtered by params)...
     params.active = false;
     const deletedForms = await service.getUserForms(user, params); // get forms for user (filtered by params)...
+
     return { ...user, forms: forms, deletedForms: deletedForms };
   },
 

app/src/forms/form/routes.js

@@ -1,14 +1,14 @@
 const config = require('config');
 const routes = require('express').Router();
 const apiAccess = require('../auth/middleware/apiAccess');
-const { currentUser, hasFormPermissions } = require('../auth/middleware/userAccess');
+const { currentUserTemp, hasFormPermissions } = require('../auth/middleware/userAccess');
 const P = require('../common/constants').Permissions;
 const rateLimiter = require('../common/middleware').apiKeyRateLimiter;
 
 const keycloak = require('../../components/keycloak');
 const controller = require('./controller');
 
-routes.use(currentUser);
+routes.use(currentUserTemp);
 
 routes.get('/', keycloak.protect(`${config.get('server.keycloak.clientId')}:admin`), async (req, res, next) => {
   await controller.listForms(req, res, next);

app/src/forms/submission/routes.js

@@ -3,18 +3,15 @@ const routes = require('express').Router();
 
 const controller = require('./controller');
 const P = require('../common/constants').Permissions;
-const { currentUser, currentUserTemp, hasSubmissionPermissions, filterMultipleSubmissions } = require('../auth/middleware/userAccess');
+const { currentUserTemp, hasSubmissionPermissions, filterMultipleSubmissions } = require('../auth/middleware/userAccess');
 const rateLimiter = require('../common/middleware').apiKeyRateLimiter;
 
-// This endpoint is the one most used by API Key users. Try it with a temporary
-// version of currentUser that only creates req.currentUser for Bearer tokens.
-// This saves a couple of big database calls that are not needed for API Keys.
-routes.get('/:formSubmissionId', currentUserTemp, rateLimiter, apiAccess, hasSubmissionPermissions(P.SUBMISSION_READ), async (req, res, next) => {
+routes.use(currentUserTemp);
+
+routes.get('/:formSubmissionId', rateLimiter, apiAccess, hasSubmissionPermissions(P.SUBMISSION_READ), async (req, res, next) => {
   await controller.read(req, res, next);
 });
 
-routes.use(currentUser);
-
 routes.put('/:formSubmissionId', hasSubmissionPermissions(P.SUBMISSION_UPDATE), async (req, res, next) => {
   await controller.update(req, res, next);
 });