Merge pull request #4947 from overleaf/em-project-rename-for-owners-only

Prevent collaborators from renaming a project

GitOrigin-RevId: 94d12e25592fea55b84427aeae78f7bb2a544a58

services/web/app/src/Features/Authorization/AuthorizationManager.js

@@ -1,3 +1,5 @@
+const { callbackify } = require('util')
+const { ObjectId } = require('mongodb')
 const CollaboratorsGetter = require('../Collaborators/CollaboratorsGetter')
 const CollaboratorsHandler = require('../Collaborators/CollaboratorsHandler')
 const ProjectGetter = require('../Project/ProjectGetter')
@@ -6,290 +8,237 @@ const PrivilegeLevels = require('./PrivilegeLevels')
 const TokenAccessHandler = require('../TokenAccess/TokenAccessHandler')
 const PublicAccessLevels = require('./PublicAccessLevels')
 const Errors = require('../Errors/Errors')
-const { ObjectId } = require('mongodb')
-const { promisifyAll } = require('../../util/promises')
 
-const AuthorizationManager = {
+function isRestrictedUser(userId, privilegeLevel, isTokenMember) {
-  isRestrictedUser(userId, privilegeLevel, isTokenMember) {
   if (privilegeLevel === PrivilegeLevels.NONE) {
     return true
   }
   return (
     privilegeLevel === PrivilegeLevels.READ_ONLY && (isTokenMember || !userId)
   )
-  },
+}
 
-  isRestrictedUserForProject(userId, projectId, token, callback) {
+async function isRestrictedUserForProject(userId, projectId, token) {
-    AuthorizationManager.getPrivilegeLevelForProject(
+  const privilegeLevel = await getPrivilegeLevelForProject(
     userId,
     projectId,
-      token,
+    token
-      (err, privilegeLevel) => {
+  )
-        if (err) {
+  const isTokenMember = await CollaboratorsHandler.promises.userIsTokenMember(
-          return callback(err)
-        }
-        CollaboratorsHandler.userIsTokenMember(
     userId,
-          projectId,
+    projectId
-          (err, isTokenMember) => {
-            if (err) {
-              return callback(err)
-            }
-            callback(
-              null,
-              AuthorizationManager.isRestrictedUser(
-                userId,
-                privilegeLevel,
-                isTokenMember
   )
-            )
+  return isRestrictedUser(userId, privilegeLevel, isTokenMember)
-          }
+}
-        )
-      }
-    )
-  },
 
-  getPublicAccessLevel(projectId, callback) {
+async function getPublicAccessLevel(projectId) {
   if (!ObjectId.isValid(projectId)) {
-      return callback(new Error('invalid project id'))
+    throw new Error('invalid project id')
   }
+
   // Note, the Project property in the DB is `publicAccesLevel`, without the second `s`
-    ProjectGetter.getProject(
+  const project = await ProjectGetter.promises.getProject(projectId, {
-      projectId,
+    publicAccesLevel: 1,
-      { publicAccesLevel: 1 },
+  })
-      function (error, project) {
-        if (error) {
-          return callback(error)
-        }
   if (!project) {
-          return callback(
+    throw new Errors.NotFoundError(`no project found with id ${projectId}`)
-            new Errors.NotFoundError(`no project found with id ${projectId}`)
-          )
   }
-        callback(null, project.publicAccesLevel)
+  return project.publicAccesLevel
-      }
+}
-    )
-  },
 
-  // Get the privilege level that the user has for the project
+/**
-  // Returns:
+ * Get the privilege level that the user has for the project.
-  //	* privilegeLevel: "owner", "readAndWrite", of "readOnly" if the user has
+ *
-  //	  access. false if the user does not have access
+ * @param userId - The id of the user that wants to access the project.
-  //   * becausePublic: true if the access level is only because the project is public.
+ * @param projectId - The id of the project to be accessed.
-  //   * becauseSiteAdmin: true if access level is only because user is admin
+ * @param {Object} opts
-  getPrivilegeLevelForProject(userId, projectId, token, callback) {
+ * @param {boolean} opts.ignoreSiteAdmin - Do not consider whether the user is
+ *     a site admin.
+ * @param {boolean} opts.ignorePublicAccess - Do not consider the project is
+ *     publicly accessible.
+ *
+ * @returns {string|boolean} The privilege level. One of "owner",
+ *     "readAndWrite", "readOnly" or false.
+ */
+async function getPrivilegeLevelForProject(
+  userId,
+  projectId,
+  token,
+  opts = {}
+) {
   if (userId) {
-      AuthorizationManager.getPrivilegeLevelForProjectWithUser(
+    return getPrivilegeLevelForProjectWithUser(userId, projectId, token, opts)
-        userId,
-        projectId,
-        token,
-        callback
-      )
   } else {
-      AuthorizationManager.getPrivilegeLevelForProjectWithoutUser(
+    return getPrivilegeLevelForProjectWithoutUser(projectId, token, opts)
-        projectId,
-        token,
-        callback
-      )
   }
-  },
+}
 
-  // User is present, get their privilege level from database
+// User is present, get their privilege level from database
-  getPrivilegeLevelForProjectWithUser(userId, projectId, token, callback) {
+async function getPrivilegeLevelForProjectWithUser(
-    CollaboratorsGetter.getMemberIdPrivilegeLevel(
   userId,
   projectId,
-      function (error, privilegeLevel) {
+  token,
-        if (error) {
+  opts = {}
-          return callback(error)
+) {
-        }
+  const privilegeLevel = await CollaboratorsGetter.promises.getMemberIdPrivilegeLevel(
+    userId,
+    projectId
+  )
   if (privilegeLevel && privilegeLevel !== PrivilegeLevels.NONE) {
     // The user has direct access
-          return callback(null, privilegeLevel, false, false)
+    return privilegeLevel
-        }
-        AuthorizationManager.isUserSiteAdmin(userId, function (error, isAdmin) {
-          if (error) {
-            return callback(error)
   }
+
+  if (!opts.ignoreSiteAdmin) {
+    const isAdmin = await isUserSiteAdmin(userId)
     if (isAdmin) {
-            return callback(null, PrivilegeLevels.OWNER, false, true)
+      return PrivilegeLevels.OWNER
     }
+  }
+
+  if (!opts.ignorePublicAccess) {
     // Legacy public-access system
     // User is present (not anonymous), but does not have direct access
-          AuthorizationManager.getPublicAccessLevel(
+    const publicAccessLevel = await getPublicAccessLevel(projectId)
-            projectId,
-            function (err, publicAccessLevel) {
-              if (err) {
-                return callback(err)
-              }
     if (publicAccessLevel === PublicAccessLevels.READ_ONLY) {
-                return callback(null, PrivilegeLevels.READ_ONLY, true, false)
+      return PrivilegeLevels.READ_ONLY
     }
     if (publicAccessLevel === PublicAccessLevels.READ_AND_WRITE) {
-                return callback(
+      return PrivilegeLevels.READ_AND_WRITE
-                  null,
-                  PrivilegeLevels.READ_AND_WRITE,
-                  true,
-                  false
-                )
     }
-              callback(null, PrivilegeLevels.NONE, false, false)
   }
-          )
-        })
-      }
-    )
-  },
 
-  // User is Anonymous, Try Token-based access
+  return PrivilegeLevels.NONE
-  getPrivilegeLevelForProjectWithoutUser(projectId, token, callback) {
+}
-    AuthorizationManager.getPublicAccessLevel(
+
+// User is Anonymous, Try Token-based access
+async function getPrivilegeLevelForProjectWithoutUser(
   projectId,
-      function (err, publicAccessLevel) {
+  token,
-        if (err) {
+  opts = {}
-          return callback(err)
+) {
-        }
+  const publicAccessLevel = await getPublicAccessLevel(projectId)
+  if (!opts.ignorePublicAccess) {
     if (publicAccessLevel === PublicAccessLevels.READ_ONLY) {
       // Legacy public read-only access for anonymous user
-          return callback(null, PrivilegeLevels.READ_ONLY, true, false)
+      return PrivilegeLevels.READ_ONLY
     }
     if (publicAccessLevel === PublicAccessLevels.READ_AND_WRITE) {
       // Legacy public read-write access for anonymous user
-          return callback(null, PrivilegeLevels.READ_AND_WRITE, true, false)
+      return PrivilegeLevels.READ_AND_WRITE
+    }
   }
   if (publicAccessLevel === PublicAccessLevels.TOKEN_BASED) {
-          return AuthorizationManager.getPrivilegeLevelForProjectWithToken(
+    return getPrivilegeLevelForProjectWithToken(projectId, token)
-            projectId,
-            token,
-            callback
-          )
   }
-        // Deny anonymous user access
-        callback(null, PrivilegeLevels.NONE, false, false)
-      }
-    )
-  },
 
-  getPrivilegeLevelForProjectWithToken(projectId, token, callback) {
+  // Deny anonymous user access
+  return PrivilegeLevels.NONE
+}
+
+async function getPrivilegeLevelForProjectWithToken(projectId, token) {
   // Anonymous users can have read-only access to token-based projects,
   // while read-write access must be logged in,
   // unless the `enableAnonymousReadAndWriteSharing` setting is enabled
-    TokenAccessHandler.validateTokenForAnonymousAccess(
+  const {
+    isValidReadAndWrite,
+    isValidReadOnly,
+  } = await TokenAccessHandler.promises.validateTokenForAnonymousAccess(
     projectId,
-      token,
+    token
-      function (err, isValidReadAndWrite, isValidReadOnly) {
+  )
-        if (err) {
-          return callback(err)
-        }
   if (isValidReadOnly) {
     // Grant anonymous user read-only access
-          return callback(null, PrivilegeLevels.READ_ONLY, false, false)
+    return PrivilegeLevels.READ_ONLY
   }
   if (isValidReadAndWrite) {
     // Grant anonymous user read-and-write access
-          return callback(null, PrivilegeLevels.READ_AND_WRITE, false, false)
+    return PrivilegeLevels.READ_AND_WRITE
   }
   // Deny anonymous access
-        callback(null, PrivilegeLevels.NONE, false, false)
+  return PrivilegeLevels.NONE
-      }
+}
-    )
-  },
 
-  canUserReadProject(userId, projectId, token, callback) {
+async function canUserReadProject(userId, projectId, token) {
-    AuthorizationManager.getPrivilegeLevelForProject(
+  const privilegeLevel = await getPrivilegeLevelForProject(
     userId,
     projectId,
-      token,
+    token
-      function (error, privilegeLevel) {
+  )
-        if (error) {
+  return [
-          return callback(error)
-        }
-        callback(
-          null,
-          [
     PrivilegeLevels.OWNER,
     PrivilegeLevels.READ_AND_WRITE,
     PrivilegeLevels.READ_ONLY,
   ].includes(privilegeLevel)
-        )
-      }
-    )
-  },
-
-  canUserWriteProjectContent(userId, projectId, token, callback) {
-    AuthorizationManager.getPrivilegeLevelForProject(
-      userId,
-      projectId,
-      token,
-      function (error, privilegeLevel) {
-        if (error) {
-          return callback(error)
-        }
-        callback(
-          null,
-          [PrivilegeLevels.OWNER, PrivilegeLevels.READ_AND_WRITE].includes(
-            privilegeLevel
-          )
-        )
-      }
-    )
-  },
-
-  canUserWriteProjectSettings(userId, projectId, token, callback) {
-    AuthorizationManager.getPrivilegeLevelForProject(
-      userId,
-      projectId,
-      token,
-      function (error, privilegeLevel, becausePublic) {
-        if (error) {
-          return callback(error)
-        }
-        if (privilegeLevel === PrivilegeLevels.OWNER) {
-          return callback(null, true)
-        }
-        if (
-          privilegeLevel === PrivilegeLevels.READ_AND_WRITE &&
-          !becausePublic
-        ) {
-          return callback(null, true)
-        }
-        callback(null, false)
-      }
-    )
-  },
-
-  canUserAdminProject(userId, projectId, token, callback) {
-    AuthorizationManager.getPrivilegeLevelForProject(
-      userId,
-      projectId,
-      token,
-      function (error, privilegeLevel, becausePublic, becauseSiteAdmin) {
-        if (error) {
-          return callback(error)
-        }
-        callback(
-          null,
-          privilegeLevel === PrivilegeLevels.OWNER,
-          becauseSiteAdmin
-        )
-      }
-    )
-  },
-
-  isUserSiteAdmin(userId, callback) {
-    if (!userId) {
-      return callback(null, false)
-    }
-    User.findOne({ _id: userId }, { isAdmin: 1 }, function (error, user) {
-      if (error) {
-        return callback(error)
-      }
-      callback(null, (user && user.isAdmin) === true)
-    })
-  },
 }
 
-module.exports = AuthorizationManager
+async function canUserWriteProjectContent(userId, projectId, token) {
-module.exports.promises = promisifyAll(AuthorizationManager, {
+  const privilegeLevel = await getPrivilegeLevelForProject(
-  without: 'isRestrictedUser',
+    userId,
-})
+    projectId,
+    token
+  )
+  return [PrivilegeLevels.OWNER, PrivilegeLevels.READ_AND_WRITE].includes(
+    privilegeLevel
+  )
+}
+
+async function canUserWriteProjectSettings(userId, projectId, token) {
+  const privilegeLevel = await getPrivilegeLevelForProject(
+    userId,
+    projectId,
+    token,
+    { ignorePublicAccess: true }
+  )
+  return [PrivilegeLevels.OWNER, PrivilegeLevels.READ_AND_WRITE].includes(
+    privilegeLevel
+  )
+}
+
+async function canUserRenameProject(userId, projectId, token) {
+  const privilegeLevel = await getPrivilegeLevelForProject(
+    userId,
+    projectId,
+    token
+  )
+  return privilegeLevel === PrivilegeLevels.OWNER
+}
+
+async function canUserAdminProject(userId, projectId, token) {
+  const privilegeLevel = await getPrivilegeLevelForProject(
+    userId,
+    projectId,
+    token
+  )
+  return privilegeLevel === PrivilegeLevels.OWNER
+}
+
+async function isUserSiteAdmin(userId) {
+  if (!userId) {
+    return false
+  }
+  const user = await User.findOne({ _id: userId }, { isAdmin: 1 }).exec()
+  return user != null && user.isAdmin === true
+}
+
+module.exports = {
+  canUserReadProject: callbackify(canUserReadProject),
+  canUserWriteProjectContent: callbackify(canUserWriteProjectContent),
+  canUserWriteProjectSettings: callbackify(canUserWriteProjectSettings),
+  canUserRenameProject: callbackify(canUserRenameProject),
+  canUserAdminProject: callbackify(canUserAdminProject),
+  getPrivilegeLevelForProject: callbackify(getPrivilegeLevelForProject),
+  isRestrictedUser,
+  isRestrictedUserForProject: callbackify(isRestrictedUserForProject),
+  isUserSiteAdmin: callbackify(isUserSiteAdmin),
+  promises: {
+    canUserReadProject,
+    canUserWriteProjectContent,
+    canUserWriteProjectSettings,
+    canUserRenameProject,
+    canUserAdminProject,
+    getPrivilegeLevelForProject,
+    isRestrictedUserForProject,
+    isUserSiteAdmin,
+  },
+}

services/web/app/src/Features/Authorization/AuthorizationMiddleware.js

@@ -1,6 +1,4 @@
-let AuthorizationMiddleware
 const AuthorizationManager = require('./AuthorizationManager')
-const async = require('async')
 const logger = require('logger-sharelatex')
 const { ObjectId } = require('mongodb')
 const Errors = require('../Errors/Errors')
@@ -8,152 +6,97 @@ const HttpErrorHandler = require('../Errors/HttpErrorHandler')
 const AuthenticationController = require('../Authentication/AuthenticationController')
 const SessionManager = require('../Authentication/SessionManager')
 const TokenAccessHandler = require('../TokenAccess/TokenAccessHandler')
+const { expressify } = require('../../util/promises')
 
-module.exports = AuthorizationMiddleware = {
+async function ensureUserCanReadMultipleProjects(req, res, next) {
-  ensureUserCanReadMultipleProjects(req, res, next) {
   const projectIds = (req.query.project_ids || '').split(',')
-    AuthorizationMiddleware._getUserId(req, function (error, userId) {
+  const userId = _getUserId(req)
-      if (error) {
+  for (const projectId of projectIds) {
-        return next(error)
-      }
-      // Remove the projects we have access to. Note rejectSeries doesn't use
-      // errors in callbacks
-      async.rejectSeries(
-        projectIds,
-        function (projectId, cb) {
     const token = TokenAccessHandler.getRequestToken(req, projectId)
-          AuthorizationManager.canUserReadProject(
+    const canRead = await AuthorizationManager.promises.canUserReadProject(
       userId,
       projectId,
-            token,
+      token
-            function (error, canRead) {
-              if (error) {
-                return next(error)
-              }
-              cb(canRead)
-            }
     )
-        },
+    if (!canRead) {
-        function (unauthorizedProjectIds) {
+      return _redirectToRestricted(req, res, next)
-          if (unauthorizedProjectIds.length > 0) {
+    }
-            return AuthorizationMiddleware.redirectToRestricted(req, res, next)
   }
   next()
-        }
+}
-      )
-    })
-  },
 
-  blockRestrictedUserFromProject(req, res, next) {
+async function blockRestrictedUserFromProject(req, res, next) {
-    AuthorizationMiddleware._getUserAndProjectId(
+  const projectId = _getProjectId(req)
-      req,
+  const userId = _getUserId(req)
-      function (error, userId, projectId) {
-        if (error) {
-          return next(error)
-        }
   const token = TokenAccessHandler.getRequestToken(req, projectId)
-        AuthorizationManager.isRestrictedUserForProject(
+  const isRestrictedUser = await AuthorizationManager.promises.isRestrictedUserForProject(
     userId,
     projectId,
-          token,
+    token
-          (err, isRestrictedUser) => {
+  )
-            if (err) {
-              return next(err)
-            }
   if (isRestrictedUser) {
-              return res.sendStatus(403)
+    return HttpErrorHandler.forbidden(req, res)
   }
   next()
-          }
+}
-        )
-      }
-    )
-  },
 
-  ensureUserCanReadProject(req, res, next) {
+async function ensureUserCanReadProject(req, res, next) {
-    AuthorizationMiddleware._getUserAndProjectId(
+  const projectId = _getProjectId(req)
-      req,
+  const userId = _getUserId(req)
-      function (error, userId, projectId) {
-        if (error) {
-          return next(error)
-        }
   const token = TokenAccessHandler.getRequestToken(req, projectId)
-        AuthorizationManager.canUserReadProject(
+  const canRead = await AuthorizationManager.promises.canUserReadProject(
     userId,
     projectId,
-          token,
+    token
-          function (error, canRead) {
+  )
-            if (error) {
-              return next(error)
-            }
   if (canRead) {
-              logger.log(
+    logger.log({ userId, projectId }, 'allowing user read access to project')
-                { userId, projectId },
-                'allowing user read access to project'
-              )
     return next()
   }
-            logger.log(
+  logger.log({ userId, projectId }, 'denying user read access to project')
-              { userId, projectId },
-              'denying user read access to project'
-            )
   HttpErrorHandler.forbidden(req, res)
-          }
+}
-        )
-      }
-    )
-  },
 
-  ensureUserCanWriteProjectSettings(req, res, next) {
+async function ensureUserCanWriteProjectSettings(req, res, next) {
-    AuthorizationMiddleware._getUserAndProjectId(
+  const projectId = _getProjectId(req)
-      req,
+  const userId = _getUserId(req)
-      function (error, userId, projectId) {
-        if (error) {
-          return next(error)
-        }
   const token = TokenAccessHandler.getRequestToken(req, projectId)
-        AuthorizationManager.canUserWriteProjectSettings(
+
+  if (req.body.name != null) {
+    const canRename = await AuthorizationManager.promises.canUserRenameProject(
       userId,
       projectId,
-          token,
+      token
-          function (error, canWrite) {
-            if (error) {
-              return next(error)
-            }
-            if (canWrite) {
-              logger.log(
-                { userId, projectId },
-                'allowing user write access to project settings'
     )
-              return next()
+    if (!canRename) {
+      return HttpErrorHandler.forbidden(req, res)
     }
-            logger.log(
-              { userId, projectId },
-              'denying user write access to project settings'
-            )
-            HttpErrorHandler.forbidden(req, res)
   }
-        )
-      }
-    )
-  },
 
-  ensureUserCanWriteProjectContent(req, res, next) {
+  const otherParams = Object.keys(req.body).filter(x => x !== 'name')
-    AuthorizationMiddleware._getUserAndProjectId(
+  if (otherParams.length > 0) {
-      req,
+    const canWrite = await AuthorizationManager.promises.canUserWriteProjectSettings(
-      function (error, userId, projectId) {
-        if (error) {
-          return next(error)
-        }
-        const token = TokenAccessHandler.getRequestToken(req, projectId)
-        AuthorizationManager.canUserWriteProjectContent(
       userId,
       projectId,
-          token,
+      token
-          function (error, canWrite) {
+    )
-            if (error) {
+    if (!canWrite) {
-              return next(error)
+      return HttpErrorHandler.forbidden(req, res)
     }
+  }
+
+  next()
+}
+
+async function ensureUserCanWriteProjectContent(req, res, next) {
+  const projectId = _getProjectId(req)
+  const userId = _getUserId(req)
+  const token = TokenAccessHandler.getRequestToken(req, projectId)
+  const canWrite = await AuthorizationManager.promises.canUserWriteProjectContent(
+    userId,
+    projectId,
+    token
+  )
   if (canWrite) {
     logger.log(
       { userId, projectId },
@@ -166,99 +109,61 @@ module.exports = AuthorizationMiddleware = {
     'denying user write access to project settings'
   )
   HttpErrorHandler.forbidden(req, res)
-          }
+}
-        )
-      }
-    )
-  },
 
-  ensureUserCanAdminProject(req, res, next) {
+async function ensureUserCanAdminProject(req, res, next) {
-    AuthorizationMiddleware._getUserAndProjectId(
+  const projectId = _getProjectId(req)
-      req,
+  const userId = _getUserId(req)
-      function (error, userId, projectId) {
-        if (error) {
-          return next(error)
-        }
   const token = TokenAccessHandler.getRequestToken(req, projectId)
-        AuthorizationManager.canUserAdminProject(
+  const canAdmin = await AuthorizationManager.promises.canUserAdminProject(
     userId,
     projectId,
-          token,
+    token
-          function (error, canAdmin) {
-            if (error) {
-              return next(error)
-            }
-            if (canAdmin) {
-              logger.log(
-                { userId, projectId },
-                'allowing user admin access to project'
   )
+  if (canAdmin) {
+    logger.log({ userId, projectId }, 'allowing user admin access to project')
     return next()
   }
-            logger.log(
+  logger.log({ userId, projectId }, 'denying user admin access to project')
-              { userId, projectId },
-              'denying user admin access to project'
-            )
   HttpErrorHandler.forbidden(req, res)
-          }
+}
-        )
-      }
-    )
-  },
 
-  ensureUserIsSiteAdmin(req, res, next) {
+async function ensureUserIsSiteAdmin(req, res, next) {
-    AuthorizationMiddleware._getUserId(req, function (error, userId) {
+  const userId = _getUserId(req)
-      if (error) {
+  const isAdmin = await AuthorizationManager.promises.isUserSiteAdmin(userId)
-        return next(error)
-      }
-      AuthorizationManager.isUserSiteAdmin(userId, function (error, isAdmin) {
-        if (error) {
-          return next(error)
-        }
   if (isAdmin) {
     logger.log({ userId }, 'allowing user admin access to site')
     return next()
   }
   logger.log({ userId }, 'denying user admin access to site')
-        AuthorizationMiddleware.redirectToRestricted(req, res, next)
+  _redirectToRestricted(req, res, next)
-      })
+}
-    })
-  },
 
-  _getUserAndProjectId(req, callback) {
+function _getProjectId(req) {
   const projectId = req.params.project_id || req.params.Project_id
   if (!projectId) {
-      return callback(new Error('Expected project_id in request parameters'))
+    throw new Error('Expected project_id in request parameters')
   }
   if (!ObjectId.isValid(projectId)) {
-      return callback(
+    throw new Errors.NotFoundError(`invalid projectId: ${projectId}`)
-        new Errors.NotFoundError(`invalid projectId: ${projectId}`)
-      )
   }
-    AuthorizationMiddleware._getUserId(req, function (error, userId) {
+  return projectId
-      if (error) {
+}
-        return callback(error)
-      }
-      callback(null, userId, projectId)
-    })
-  },
 
-  _getUserId(req, callback) {
+function _getUserId(req) {
-    const userId =
+  return (
     SessionManager.getLoggedInUserId(req.session) ||
     (req.oauth_user && req.oauth_user._id) ||
     null
-    callback(null, userId)
-  },
-
-  redirectToRestricted(req, res, next) {
-    // TODO: move this to throwing ForbiddenError
-    res.redirect(
-      `/restricted?from=${encodeURIComponent(res.locals.currentUrl)}`
   )
-  },
+}
 
-  restricted(req, res, next) {
+function _redirectToRestricted(req, res, next) {
+  // TODO: move this to throwing ForbiddenError
+  res.redirect(`/restricted?from=${encodeURIComponent(res.locals.currentUrl)}`)
+}
+
+function restricted(req, res, next) {
   if (SessionManager.isUserLoggedIn(req.session)) {
     return res.render('user/restricted', { title: 'restricted' })
   }
@@ -268,5 +173,21 @@ module.exports = AuthorizationMiddleware = {
     AuthenticationController.setRedirectInSession(req, from)
   }
   res.redirect('/login')
-  },
+}
+
+module.exports = {
+  ensureUserCanReadMultipleProjects: expressify(
+    ensureUserCanReadMultipleProjects
+  ),
+  blockRestrictedUserFromProject: expressify(blockRestrictedUserFromProject),
+  ensureUserCanReadProject: expressify(ensureUserCanReadProject),
+  ensureUserCanWriteProjectSettings: expressify(
+    ensureUserCanWriteProjectSettings
+  ),
+  ensureUserCanWriteProjectContent: expressify(
+    ensureUserCanWriteProjectContent
+  ),
+  ensureUserCanAdminProject: expressify(ensureUserCanAdminProject),
+  ensureUserIsSiteAdmin: expressify(ensureUserIsSiteAdmin),
+  restricted,
 }

services/web/app/src/Features/TokenAccess/TokenAccessHandler.js

@@ -302,8 +302,10 @@ TokenAccessHandler.promises = promisifyAll(TokenAccessHandler, {
     'grantSessionTokenAccess',
     'getRequestToken',
     'protectTokens',
-    'validateTokenForAnonymousAccess',
   ],
+  multiResult: {
+    validateTokenForAnonymousAccess: ['isValidReadAndWrite', 'isValidReadOnly'],
+  },
 })
 
 module.exports = TokenAccessHandler

services/web/app/src/router.js

@@ -316,6 +316,7 @@ function initialize(webRouter, privateApiRouter, publicApiRouter) {
   )
   webRouter.post(
     '/project/:Project_id/settings',
+    validate({ body: Joi.object() }),
     AuthorizationMiddleware.ensureUserCanWriteProjectSettings,
     ProjectController.updateProjectSettings
   )

services/web/test/acceptance/src/AuthorizationTests.js

@@ -33,6 +33,24 @@ function tryReadAccess(user, projectId, test, callback) {
   )
 }
 
+function tryRenameProjectAccess(user, projectId, test, callback) {
+  user.request.post(
+    {
+      uri: `/project/${projectId}/settings`,
+      json: {
+        name: 'new name',
+      },
+    },
+    (error, response, body) => {
+      if (error != null) {
+        return callback(error)
+      }
+      test(response, body)
+      callback()
+    }
+  )
+}
+
 function trySettingsWriteAccess(user, projectId, test, callback) {
   async.series(
     [
@@ -166,6 +184,17 @@ function expectContentWriteAccess(user, projectId, callback) {
   )
 }
 
+function expectRenameProjectAccess(user, projectId, callback) {
+  tryRenameProjectAccess(
+    user,
+    projectId,
+    (response, body) => {
+      expect(response.statusCode).to.be.oneOf([200, 204])
+    },
+    callback
+  )
+}
+
 function expectSettingsWriteAccess(user, projectId, callback) {
   trySettingsWriteAccess(
     user,
@@ -184,7 +213,7 @@ function expectAdminAccess(user, projectId, callback) {
   )
 }
 
-function expectNoReadAccess(user, projectId, options, callback) {
+function expectNoReadAccess(user, projectId, callback) {
   async.series(
     [
       cb =>
@@ -214,7 +243,7 @@ function expectNoContentWriteAccess(user, projectId, callback) {
   )
 }
 
-function expectNoSettingsWriteAccess(user, projectId, options, callback) {
+function expectNoSettingsWriteAccess(user, projectId, callback) {
   trySettingsWriteAccess(
     user,
     projectId,
@@ -223,6 +252,15 @@ function expectNoSettingsWriteAccess(user, projectId, options, callback) {
   )
 }
 
+function expectNoRenameProjectAccess(user, projectId, callback) {
+  tryRenameProjectAccess(
+    user,
+    projectId,
+    expectErrorResponse.restricted.json,
+    callback
+  )
+}
+
 function expectNoAdminAccess(user, projectId, callback) {
   tryAdminAccess(
     user,
@@ -325,6 +363,10 @@ describe('Authorization', function () {
       expectSettingsWriteAccess(this.owner, this.projectId, done)
     })
 
+    it('should allow the owner to rename the project', function (done) {
+      expectRenameProjectAccess(this.owner, this.projectId, done)
+    })
+
     it('should allow the owner admin access to it', function (done) {
       expectAdminAccess(this.owner, this.projectId, done)
     })
@@ -334,12 +376,7 @@ describe('Authorization', function () {
     })
 
     it('should not allow another user read access to the project', function (done) {
-      expectNoReadAccess(
+      expectNoReadAccess(this.other1, this.projectId, done)
-        this.other1,
-        this.projectId,
-        { redirect_to: '/restricted' },
-        done
-      )
     })
 
     it('should not allow another user write access to its content', function (done) {
@@ -347,12 +384,11 @@ describe('Authorization', function () {
     })
 
     it('should not allow another user write access to its settings', function (done) {
-      expectNoSettingsWriteAccess(
+      expectNoSettingsWriteAccess(this.other1, this.projectId, done)
-        this.other1,
+    })
-        this.projectId,
+
-        { redirect_to: '/restricted' },
+    it('should not allow another user to rename the project', function (done) {
-        done
+      expectNoRenameProjectAccess(this.other1, this.projectId, done)
-      )
     })
 
     it('should not allow another user admin access to it', function (done) {
@@ -364,12 +400,7 @@ describe('Authorization', function () {
     })
 
     it('should not allow anonymous user read access to it', function (done) {
-      expectNoReadAccess(
+      expectNoReadAccess(this.anon, this.projectId, done)
-        this.anon,
-        this.projectId,
-        { redirect_to: '/restricted' },
-        done
-      )
     })
 
     it('should not allow anonymous user write access to its content', function (done) {
@@ -377,12 +408,11 @@ describe('Authorization', function () {
     })
 
     it('should not allow anonymous user write access to its settings', function (done) {
-      expectNoSettingsWriteAccess(
+      expectNoSettingsWriteAccess(this.anon, this.projectId, done)
-        this.anon,
+    })
-        this.projectId,
+
-        { redirect_to: '/restricted' },
+    it('should not allow anonymous user to rename the project', function (done) {
-        done
+      expectNoRenameProjectAccess(this.anon, this.projectId, done)
-      )
     })
 
     it('should not allow anonymous user admin access to it', function (done) {
@@ -405,6 +435,10 @@ describe('Authorization', function () {
       expectSettingsWriteAccess(this.site_admin, this.projectId, done)
     })
 
+    it('should allow site admin users to rename the project', function (done) {
+      expectRenameProjectAccess(this.site_admin, this.projectId, done)
+    })
+
     it('should allow site admin users admin access to it', function (done) {
       expectAdminAccess(this.site_admin, this.projectId, done)
     })
@@ -456,12 +490,11 @@ describe('Authorization', function () {
     })
 
     it('should not allow the read-only user write access to its settings', function (done) {
-      expectNoSettingsWriteAccess(
+      expectNoSettingsWriteAccess(this.ro_user, this.projectId, done)
-        this.ro_user,
+    })
-        this.projectId,
+
-        { redirect_to: '/restricted' },
+    it('should not allow the read-only user to rename the project', function (done) {
-        done
+      expectNoRenameProjectAccess(this.ro_user, this.projectId, done)
-      )
     })
 
     it('should not allow the read-only user admin access to it', function (done) {
@@ -480,6 +513,10 @@ describe('Authorization', function () {
       expectSettingsWriteAccess(this.rw_user, this.projectId, done)
     })
 
+    it('should not allow the read-write user to rename the project', function (done) {
+      expectNoRenameProjectAccess(this.rw_user, this.projectId, done)
+    })
+
     it('should not allow the read-write user admin access to it', function (done) {
       expectNoAdminAccess(this.rw_user, this.projectId, done)
     })
@@ -513,12 +550,11 @@ describe('Authorization', function () {
     })
 
     it('should not allow a user write access to its settings', function (done) {
-      expectNoSettingsWriteAccess(
+      expectNoSettingsWriteAccess(this.other1, this.projectId, done)
-        this.other1,
+    })
-        this.projectId,
+
-        { redirect_to: '/restricted' },
+    it('should not allow a user to rename the project', function (done) {
-        done
+      expectNoRenameProjectAccess(this.other1, this.projectId, done)
-      )
     })
 
     it('should not allow a user admin access to it', function (done) {
@@ -538,12 +574,11 @@ describe('Authorization', function () {
     })
 
     it('should not allow an anonymous user write access to its settings', function (done) {
-      expectNoSettingsWriteAccess(
+      expectNoSettingsWriteAccess(this.anon, this.projectId, done)
-        this.anon,
+    })
-        this.projectId,
+
-        { redirect_to: '/restricted' },
+    it('should not allow an anonymous user to rename the project', function (done) {
-        done
+      expectNoRenameProjectAccess(this.anon, this.projectId, done)
-      )
     })
 
     it('should not allow an anonymous user admin access to it', function (done) {
@@ -571,12 +606,11 @@ describe('Authorization', function () {
     })
 
     it('should not allow a user write access to its settings', function (done) {
-      expectNoSettingsWriteAccess(
+      expectNoSettingsWriteAccess(this.other1, this.projectId, done)
-        this.other1,
+    })
-        this.projectId,
+
-        { redirect_to: '/restricted' },
+    it('should not allow a user to rename the project', function (done) {
-        done
+      expectNoRenameProjectAccess(this.other1, this.projectId, done)
-      )
     })
 
     it('should not allow a user admin access to it', function (done) {
@@ -597,12 +631,11 @@ describe('Authorization', function () {
     })
 
     it('should not allow an anonymous user write access to its settings', function (done) {
-      expectNoSettingsWriteAccess(
+      expectNoSettingsWriteAccess(this.anon, this.projectId, done)
-        this.anon,
+    })
-        this.projectId,
+
-        { redirect_to: '/restricted' },
+    it('should not allow an anonymous user to rename the project', function (done) {
-        done
+      expectNoRenameProjectAccess(this.anon, this.projectId, done)
-      )
     })
 
     it('should not allow an anonymous user admin access to it', function (done) {

services/web/test/unit/src/Authorization/AuthorizationMiddlewareTests.js

@@ -1,6 +1,7 @@
 const sinon = require('sinon')
 const { expect } = require('chai')
 const SandboxedModule = require('sandboxed-module')
+const { ObjectId } = require('mongodb')
 const Errors = require('../../../../app/src/Features/Errors/Errors.js')
 
 const MODULE_PATH =
@@ -8,31 +9,34 @@ const MODULE_PATH =
 
 describe('AuthorizationMiddleware', function () {
   beforeEach(function () {
-    this.userId = 'user-id-123'
+    this.userId = new ObjectId().toString()
-    this.project_id = 'project-id-123'
+    this.project_id = new ObjectId().toString()
     this.token = 'some-token'
     this.AuthenticationController = {}
     this.SessionManager = {
       getLoggedInUserId: sinon.stub().returns(this.userId),
       isUserLoggedIn: sinon.stub().returns(true),
     }
-    this.AuthorizationManager = {}
+    this.AuthorizationManager = {
+      promises: {
+        canUserReadProject: sinon.stub(),
+        canUserWriteProjectSettings: sinon.stub(),
+        canUserWriteProjectContent: sinon.stub(),
+        canUserAdminProject: sinon.stub(),
+        canUserRenameProject: sinon.stub(),
+        isUserSiteAdmin: sinon.stub(),
+        isRestrictedUserForProject: sinon.stub(),
+      },
+    }
     this.HttpErrorHandler = {
       forbidden: sinon.stub(),
     }
     this.TokenAccessHandler = {
       getRequestToken: sinon.stub().returns(this.token),
     }
-    this.ObjectId = {
-      isValid: sinon.stub().withArgs(this.project_id).returns(true),
-    }
-    this.AuthorizationManager = {}
     this.AuthorizationMiddleware = SandboxedModule.require(MODULE_PATH, {
       requires: {
         './AuthorizationManager': this.AuthorizationManager,
-        mongodb: {
-          ObjectId: this.ObjectId,
-        },
         '../Errors/HttpErrorHandler': this.HttpErrorHandler,
         '../Authentication/AuthenticationController': this
           .AuthenticationController,
@@ -40,542 +44,355 @@ describe('AuthorizationMiddleware', function () {
         '../TokenAccess/TokenAccessHandler': this.TokenAccessHandler,
       },
     })
-    this.req = {}
+    this.req = {
-    this.res = {}
+      params: {
+        project_id: this.project_id,
+      },
+      body: {},
+    }
+    this.res = {
+      redirect: sinon.stub(),
+      locals: {
+        currentUrl: '/current/url',
+      },
+    }
     this.next = sinon.stub()
   })
 
-  describe('_getUserId', function () {
+  describe('ensureCanReadProject', function () {
-    beforeEach(function () {
+    testMiddleware('ensureUserCanReadProject', 'canUserReadProject')
-      this.req = {}
   })
 
-    it('should get the user from session', function (done) {
+  describe('ensureUserCanWriteProjectContent', function () {
-      this.SessionManager.getLoggedInUserId = sinon.stub().returns('1234')
+    testMiddleware(
-      this.AuthorizationMiddleware._getUserId(this.req, (err, userId) => {
+      'ensureUserCanWriteProjectContent',
-        expect(err).to.not.exist
+      'canUserWriteProjectContent'
-        expect(userId).to.equal('1234')
-        done()
-      })
-    })
-
-    it('should get oauth_user from request', function (done) {
-      this.SessionManager.getLoggedInUserId = sinon.stub().returns(null)
-      this.req.oauth_user = { _id: '5678' }
-      this.AuthorizationMiddleware._getUserId(this.req, (err, userId) => {
-        expect(err).to.not.exist
-        expect(userId).to.equal('5678')
-        done()
-      })
-    })
-
-    it('should fall back to null', function (done) {
-      this.SessionManager.getLoggedInUserId = sinon.stub().returns(null)
-      this.req.oauth_user = undefined
-      this.AuthorizationMiddleware._getUserId(this.req, (err, userId) => {
-        expect(err).to.not.exist
-        expect(userId).to.equal(null)
-        done()
-      })
-    })
-  })
-
-  const METHODS_TO_TEST = {
-    ensureUserCanReadProject: 'canUserReadProject',
-    ensureUserCanWriteProjectSettings: 'canUserWriteProjectSettings',
-    ensureUserCanWriteProjectContent: 'canUserWriteProjectContent',
-  }
-  Object.entries(METHODS_TO_TEST).forEach(
-    ([middlewareMethod, managerMethod]) => {
-      describe(middlewareMethod, function () {
-        beforeEach(function () {
-          this.req.params = { project_id: this.project_id }
-          this.AuthorizationManager[managerMethod] = sinon.stub()
-          this.AuthorizationMiddleware.redirectToRestricted = sinon.stub()
-        })
-
-        describe('with missing project_id', function () {
-          beforeEach(function () {
-            this.req.params = {}
-          })
-
-          it('should return an error to next', function () {
-            this.AuthorizationMiddleware[middlewareMethod](
-              this.req,
-              this.res,
-              this.next
     )
-            this.next
-              .calledWith(sinon.match.instanceOf(Error))
-              .should.equal(true)
-          })
   })
 
-        describe('with logged in user', function () {
+  describe('ensureUserCanWriteProjectSettings', function () {
+    describe('when renaming a project', function () {
       beforeEach(function () {
-            this.SessionManager.getLoggedInUserId.returns(this.userId)
+        this.req.body.name = 'new project name'
       })
 
-          describe('when user has permission', function () {
+      testMiddleware(
-            beforeEach(function () {
+        'ensureUserCanWriteProjectSettings',
-              this.AuthorizationManager[managerMethod]
+        'canUserRenameProject'
-                .withArgs(this.userId, this.project_id, this.token)
-                .yields(null, true)
-            })
-
-            it('should return next', function () {
-              this.AuthorizationMiddleware[middlewareMethod](
-                this.req,
-                this.res,
-                this.next
       )
-              this.next.called.should.equal(true)
-            })
     })
 
-          describe("when user doesn't have permission", function () {
+    describe('when setting another parameter', function () {
       beforeEach(function () {
-              this.AuthorizationManager[managerMethod]
+        this.req.body.compiler = 'texlive-2017'
-                .withArgs(this.userId, this.project_id, this.token)
-                .yields(null, false)
       })
 
-            it('should raise a 403', function () {
+      testMiddleware(
-              this.AuthorizationMiddleware[middlewareMethod](
+        'ensureUserCanWriteProjectSettings',
-                this.req,
+        'canUserWriteProjectSettings'
-                this.res,
-                this.next
-              )
-              this.next.called.should.equal(false)
-              this.HttpErrorHandler.forbidden
-                .calledWith(this.req, this.res)
-                .should.equal(true)
-            })
-          })
-        })
-
-        describe('with anonymous user', function () {
-          describe('when user has permission', function () {
-            beforeEach(function () {
-              this.SessionManager.getLoggedInUserId.returns(null)
-              this.AuthorizationManager[managerMethod]
-                .withArgs(null, this.project_id, this.token)
-                .yields(null, true)
-            })
-
-            it('should return next', function () {
-              this.AuthorizationMiddleware[middlewareMethod](
-                this.req,
-                this.res,
-                this.next
-              )
-              this.next.called.should.equal(true)
-            })
-          })
-
-          describe("when user doesn't have permission", function () {
-            beforeEach(function () {
-              this.SessionManager.getLoggedInUserId.returns(null)
-              this.AuthorizationManager[managerMethod]
-                .withArgs(null, this.project_id, this.token)
-                .yields(null, false)
-            })
-
-            it('should redirect to redirectToRestricted', function () {
-              this.AuthorizationMiddleware[middlewareMethod](
-                this.req,
-                this.res,
-                this.next
-              )
-              this.next.called.should.equal(false)
-              this.HttpErrorHandler.forbidden
-                .calledWith(this.req, this.res)
-                .should.equal(true)
-            })
-          })
-        })
-
-        describe('with malformed project id', function () {
-          beforeEach(function () {
-            this.req.params = { project_id: 'blah' }
-            this.ObjectId.isValid = sinon.stub().returns(false)
-          })
-
-          it('should return a not found error', function (done) {
-            this.AuthorizationMiddleware[middlewareMethod](
-              this.req,
-              this.res,
-              error => {
-                error.should.be.instanceof(Errors.NotFoundError)
-                return done()
-              }
       )
     })
   })
-      })
-    }
-  )
 
   describe('ensureUserCanAdminProject', function () {
-    beforeEach(function () {
+    testMiddleware('ensureUserCanAdminProject', 'canUserAdminProject')
-      this.req.params = { project_id: this.project_id }
-      this.AuthorizationManager.canUserAdminProject = sinon.stub()
-      this.AuthorizationMiddleware.redirectToRestricted = sinon.stub()
-    })
-
-    describe('with missing project_id', function () {
-      beforeEach(function () {
-        this.req.params = {}
-      })
-
-      it('should return an error to next', function () {
-        this.AuthorizationMiddleware.ensureUserCanAdminProject(
-          this.req,
-          this.res,
-          this.next
-        )
-        this.next.calledWith(sinon.match.instanceOf(Error)).should.equal(true)
-      })
-    })
-
-    describe('with logged in user', function () {
-      beforeEach(function () {
-        this.SessionManager.getLoggedInUserId.returns(this.userId)
-      })
-
-      describe('when user has permission', function () {
-        beforeEach(function () {
-          this.AuthorizationManager.canUserAdminProject
-            .withArgs(this.userId, this.project_id, this.token)
-            .yields(null, true)
-        })
-
-        it('should return next', function () {
-          this.AuthorizationMiddleware.ensureUserCanAdminProject(
-            this.req,
-            this.res,
-            this.next
-          )
-          this.next.called.should.equal(true)
-        })
-      })
-
-      describe("when user doesn't have permission", function () {
-        beforeEach(function () {
-          this.AuthorizationManager.canUserAdminProject
-            .withArgs(this.userId, this.project_id, this.token)
-            .yields(null, false)
-        })
-
-        it('should invoke HTTP forbidden error handler', function (done) {
-          this.HttpErrorHandler.forbidden = sinon.spy(() => done())
-          this.AuthorizationMiddleware.ensureUserCanAdminProject(
-            this.req,
-            this.res
-          )
-        })
-      })
-    })
-
-    describe('with anonymous user', function () {
-      describe('when user has permission', function () {
-        beforeEach(function () {
-          this.SessionManager.getLoggedInUserId.returns(null)
-          this.AuthorizationManager.canUserAdminProject
-            .withArgs(null, this.project_id, this.token)
-            .yields(null, true)
-        })
-
-        it('should return next', function () {
-          this.AuthorizationMiddleware.ensureUserCanAdminProject(
-            this.req,
-            this.res,
-            this.next
-          )
-          this.next.called.should.equal(true)
-        })
-      })
-
-      describe("when user doesn't have permission", function () {
-        beforeEach(function () {
-          this.SessionManager.getLoggedInUserId.returns(null)
-          this.AuthorizationManager.canUserAdminProject
-            .withArgs(null, this.project_id, this.token)
-            .yields(null, false)
-        })
-
-        it('should invoke HTTP forbidden error handler', function (done) {
-          this.HttpErrorHandler.forbidden = sinon.spy(() => done())
-          this.AuthorizationMiddleware.ensureUserCanAdminProject(
-            this.req,
-            this.res
-          )
-        })
-      })
-    })
-
-    describe('with malformed project id', function () {
-      beforeEach(function () {
-        this.req.params = { project_id: 'blah' }
-        this.ObjectId.isValid = sinon.stub().returns(false)
-      })
-
-      it('should return a not found error', function (done) {
-        this.AuthorizationMiddleware.ensureUserCanAdminProject(
-          this.req,
-          this.res,
-          error => {
-            error.should.be.instanceof(Errors.NotFoundError)
-            return done()
-          }
-        )
-      })
-    })
   })
 
   describe('ensureUserIsSiteAdmin', function () {
-    beforeEach(function () {
-      this.AuthorizationManager.isUserSiteAdmin = sinon.stub()
-      this.AuthorizationMiddleware.redirectToRestricted = sinon.stub()
-    })
-
     describe('with logged in user', function () {
-      beforeEach(function () {
-        this.SessionManager.getLoggedInUserId.returns(this.userId)
-      })
-
       describe('when user has permission', function () {
-        beforeEach(function () {
+        setupSiteAdmin(true)
-          this.AuthorizationManager.isUserSiteAdmin
+        invokeMiddleware('ensureUserIsSiteAdmin')
-            .withArgs(this.userId)
+        expectNext()
-            .yields(null, true)
-        })
-
-        it('should return next', function () {
-          this.AuthorizationMiddleware.ensureUserIsSiteAdmin(
-            this.req,
-            this.res,
-            this.next
-          )
-          this.next.called.should.equal(true)
-        })
       })
 
       describe("when user doesn't have permission", function () {
-        beforeEach(function () {
+        setupSiteAdmin(false)
-          this.AuthorizationManager.isUserSiteAdmin
+        invokeMiddleware('ensureUserIsSiteAdmin')
-            .withArgs(this.userId)
+        expectRedirectToRestricted()
-            .yields(null, false)
+      })
     })
 
-        it('should redirect to redirectToRestricted', function () {
+    describe('with oauth user', function () {
-          this.AuthorizationMiddleware.ensureUserIsSiteAdmin(
+      setupOAuthUser()
-            this.req,
+
-            this.res,
+      describe('when user has permission', function () {
-            this.next
+        setupSiteAdmin(true)
-          )
+        invokeMiddleware('ensureUserIsSiteAdmin')
-          this.next.called.should.equal(false)
+        expectNext()
-          this.AuthorizationMiddleware.redirectToRestricted
-            .calledWith(this.req, this.res, this.next)
-            .should.equal(true)
       })
+
+      describe("when user doesn't have permission", function () {
+        setupSiteAdmin(false)
+        invokeMiddleware('ensureUserIsSiteAdmin')
+        expectRedirectToRestricted()
       })
     })
 
     describe('with anonymous user', function () {
-      describe('when user has permission', function () {
+      setupAnonymousUser()
-        beforeEach(function () {
+      invokeMiddleware('ensureUserIsSiteAdmin')
-          this.SessionManager.getLoggedInUserId.returns(null)
+      expectRedirectToRestricted()
-          this.AuthorizationManager.isUserSiteAdmin
-            .withArgs(null)
-            .yields(null, true)
-        })
-
-        it('should return next', function () {
-          this.AuthorizationMiddleware.ensureUserIsSiteAdmin(
-            this.req,
-            this.res,
-            this.next
-          )
-          this.next.called.should.equal(true)
-        })
-      })
-
-      describe("when user doesn't have permission", function () {
-        beforeEach(function () {
-          this.SessionManager.getLoggedInUserId.returns(null)
-          this.AuthorizationManager.isUserSiteAdmin
-            .withArgs(null)
-            .yields(null, false)
-        })
-
-        it('should redirect to redirectToRestricted', function () {
-          this.AuthorizationMiddleware.ensureUserIsSiteAdmin(
-            this.req,
-            this.res,
-            this.next
-          )
-          this.next.called.should.equal(false)
-          this.AuthorizationMiddleware.redirectToRestricted
-            .calledWith(this.req, this.res, this.next)
-            .should.equal(true)
-        })
-      })
     })
   })
 
   describe('blockRestrictedUserFromProject', function () {
-    beforeEach(function () {
+    describe('for a restricted user', function () {
-      this.AuthorizationMiddleware._getUserAndProjectId = sinon
+      setupPermission('isRestrictedUserForProject', true)
-        .stub()
+      invokeMiddleware('blockRestrictedUserFromProject')
-        .callsArgWith(1, null, this.userId, this.project_id)
+      expectForbidden()
     })
 
-    it('should issue a 401 response for a restricted user', function (done) {
+    describe('for a regular user', function (done) {
-      this.AuthorizationManager.isRestrictedUserForProject = sinon
+      setupPermission('isRestrictedUserForProject', false)
-        .stub()
+      invokeMiddleware('blockRestrictedUserFromProject')
-        .callsArgWith(3, null, true)
+      expectNext()
-      this.req = {}
-      this.next = sinon.stub()
-      this.res.sendStatus = status => {
-        expect(status).to.equal(403)
-        expect(
-          this.AuthorizationManager.isRestrictedUserForProject.called
-        ).to.equal(true)
-        expect(this.next.called).to.equal(false)
-        done()
-      }
-      this.AuthorizationMiddleware.blockRestrictedUserFromProject(
-        this.req,
-        this.res,
-        this.next
-      )
-    })
-
-    it('should pass through for a regular user', function (done) {
-      this.AuthorizationManager.isRestrictedUserForProject = sinon
-        .stub()
-        .callsArgWith(3, null, false)
-      this.req = {}
-      this.res.sendStatus = sinon.stub()
-      this.next = status => {
-        expect(
-          this.AuthorizationManager.isRestrictedUserForProject.called
-        ).to.equal(true)
-        expect(this.res.sendStatus.called).to.equal(false)
-        done()
-      }
-      this.AuthorizationMiddleware.blockRestrictedUserFromProject(
-        this.req,
-        this.res,
-        this.next
-      )
     })
   })
 
   describe('ensureUserCanReadMultipleProjects', function () {
     beforeEach(function () {
-      this.AuthorizationManager.canUserReadProject = sinon.stub()
-      this.AuthorizationMiddleware.redirectToRestricted = sinon.stub()
       this.req.query = { project_ids: 'project1,project2' }
     })
 
     describe('with logged in user', function () {
-      beforeEach(function () {
-        this.SessionManager.getLoggedInUserId.returns(this.userId)
-      })
-
       describe('when user has permission to access all projects', function () {
         beforeEach(function () {
-          this.AuthorizationManager.canUserReadProject
+          this.AuthorizationManager.promises.canUserReadProject
             .withArgs(this.userId, 'project1', this.token)
-            .yields(null, true)
+            .resolves(true)
-          this.AuthorizationManager.canUserReadProject
+          this.AuthorizationManager.promises.canUserReadProject
             .withArgs(this.userId, 'project2', this.token)
-            .yields(null, true)
+            .resolves(true)
         })
 
-        it('should return next', function () {
+        invokeMiddleware('ensureUserCanReadMultipleProjects')
-          this.AuthorizationMiddleware.ensureUserCanReadMultipleProjects(
+        expectNext()
-            this.req,
-            this.res,
-            this.next
-          )
-          this.next.called.should.equal(true)
-        })
       })
 
       describe("when user doesn't have permission to access one of the projects", function () {
         beforeEach(function () {
-          this.AuthorizationManager.canUserReadProject
+          this.AuthorizationManager.promises.canUserReadProject
             .withArgs(this.userId, 'project1', this.token)
-            .yields(null, true)
+            .resolves(true)
-          this.AuthorizationManager.canUserReadProject
+          this.AuthorizationManager.promises.canUserReadProject
             .withArgs(this.userId, 'project2', this.token)
-            .yields(null, false)
+            .resolves(false)
         })
 
-        it('should redirect to redirectToRestricted', function () {
+        invokeMiddleware('ensureUserCanReadMultipleProjects')
-          this.AuthorizationMiddleware.ensureUserCanReadMultipleProjects(
+        expectRedirectToRestricted()
-            this.req,
-            this.res,
-            this.next
-          )
-          this.next.called.should.equal(false)
-          this.AuthorizationMiddleware.redirectToRestricted
-            .calledWith(this.req, this.res, this.next)
-            .should.equal(true)
       })
     })
+
+    describe('with oauth user', function () {
+      setupOAuthUser()
+
+      beforeEach(function () {
+        this.AuthorizationManager.promises.canUserReadProject
+          .withArgs(this.userId, 'project1', this.token)
+          .resolves(true)
+        this.AuthorizationManager.promises.canUserReadProject
+          .withArgs(this.userId, 'project2', this.token)
+          .resolves(true)
+      })
+
+      invokeMiddleware('ensureUserCanReadMultipleProjects')
+      expectNext()
     })
 
     describe('with anonymous user', function () {
+      setupAnonymousUser()
+
       describe('when user has permission', function () {
         describe('when user has permission to access all projects', function () {
           beforeEach(function () {
-            this.SessionManager.getLoggedInUserId.returns(null)
+            this.AuthorizationManager.promises.canUserReadProject
-            this.AuthorizationManager.canUserReadProject
               .withArgs(null, 'project1', this.token)
-              .yields(null, true)
+              .resolves(true)
-            this.AuthorizationManager.canUserReadProject
+            this.AuthorizationManager.promises.canUserReadProject
               .withArgs(null, 'project2', this.token)
-              .yields(null, true)
+              .resolves(true)
           })
 
-          it('should return next', function () {
+          invokeMiddleware('ensureUserCanReadMultipleProjects')
-            this.AuthorizationMiddleware.ensureUserCanReadMultipleProjects(
+          expectNext()
-              this.req,
-              this.res,
-              this.next
-            )
-            this.next.called.should.equal(true)
-          })
         })
 
         describe("when user doesn't have permission to access one of the projects", function () {
           beforeEach(function () {
-            this.SessionManager.getLoggedInUserId.returns(null)
+            this.AuthorizationManager.promises.canUserReadProject
-            this.AuthorizationManager.canUserReadProject
               .withArgs(null, 'project1', this.token)
-              .yields(null, true)
+              .resolves(true)
-            this.AuthorizationManager.canUserReadProject
+            this.AuthorizationManager.promises.canUserReadProject
               .withArgs(null, 'project2', this.token)
-              .yields(null, false)
+              .resolves(false)
           })
 
-          it('should redirect to redirectToRestricted', function () {
+          invokeMiddleware('ensureUserCanReadMultipleProjects')
-            this.AuthorizationMiddleware.ensureUserCanReadMultipleProjects(
+          expectRedirectToRestricted()
-              this.req,
-              this.res,
-              this.next
-            )
-            this.next.called.should.equal(false)
-            this.AuthorizationMiddleware.redirectToRestricted
-              .calledWith(this.req, this.res, this.next)
-              .should.equal(true)
-          })
         })
       })
     })
   })
 })
+
+function testMiddleware(middleware, permission) {
+  describe(middleware, function () {
+    describe('with missing project_id', function () {
+      setupMissingProjectId()
+      invokeMiddleware(middleware)
+      expectError()
+    })
+
+    describe('with logged in user', function () {
+      describe('when user has permission', function () {
+        setupPermission(permission, true)
+        invokeMiddleware(middleware)
+        expectNext()
+      })
+
+      describe("when user doesn't have permission", function () {
+        setupPermission(permission, false)
+        invokeMiddleware(middleware)
+        expectForbidden()
+      })
+    })
+
+    describe('with oauth user', function () {
+      setupOAuthUser()
+
+      describe('when user has permission', function () {
+        setupPermission(permission, true)
+        invokeMiddleware(middleware)
+        expectNext()
+      })
+
+      describe("when user doesn't have permission", function () {
+        setupPermission(permission, false)
+        invokeMiddleware(middleware)
+        expectForbidden()
+      })
+    })
+
+    describe('with anonymous user', function () {
+      setupAnonymousUser()
+
+      describe('when user has permission', function () {
+        setupAnonymousPermission(permission, true)
+        invokeMiddleware(middleware)
+        expectNext()
+      })
+
+      describe("when user doesn't have permission", function () {
+        setupAnonymousPermission(permission, false)
+        invokeMiddleware(middleware)
+        expectForbidden()
+      })
+    })
+
+    describe('with malformed project id', function () {
+      setupMalformedProjectId()
+      invokeMiddleware(middleware)
+      expectNotFound()
+    })
+  })
+}
+
+function setupAnonymousUser() {
+  beforeEach('set up anonymous user', function () {
+    this.SessionManager.getLoggedInUserId.returns(null)
+    this.SessionManager.isUserLoggedIn.returns(false)
+  })
+}
+
+function setupOAuthUser() {
+  beforeEach('set up oauth user', function () {
+    this.SessionManager.getLoggedInUserId.returns(null)
+    this.req.oauth_user = { _id: this.userId }
+  })
+}
+
+function setupPermission(permission, value) {
+  beforeEach(`set permission ${permission} to ${value}`, function () {
+    this.AuthorizationManager.promises[permission]
+      .withArgs(this.userId, this.project_id, this.token)
+      .resolves(value)
+  })
+}
+
+function setupAnonymousPermission(permission, value) {
+  beforeEach(`set anonymous permission ${permission} to ${value}`, function () {
+    this.AuthorizationManager.promises[permission]
+      .withArgs(null, this.project_id, this.token)
+      .resolves(value)
+  })
+}
+
+function setupSiteAdmin(value) {
+  beforeEach(`set site admin to ${value}`, function () {
+    this.AuthorizationManager.promises.isUserSiteAdmin
+      .withArgs(this.userId)
+      .resolves(value)
+  })
+}
+
+function setupMissingProjectId() {
+  beforeEach('set up missing project id', function () {
+    delete this.req.params.project_id
+  })
+}
+
+function setupMalformedProjectId() {
+  beforeEach('set up malformed project id', function () {
+    this.req.params = { project_id: 'bad-project-id' }
+  })
+}
+
+function invokeMiddleware(method) {
+  beforeEach(`invoke ${method}`, function (done) {
+    this.next.callsFake(() => done())
+    this.HttpErrorHandler.forbidden.callsFake(() => done())
+    this.res.redirect.callsFake(() => done())
+    this.AuthorizationMiddleware[method](this.req, this.res, this.next)
+  })
+}
+
+function expectNext() {
+  it('calls the next middleware', function () {
+    expect(this.next).to.have.been.calledWithExactly()
+  })
+}
+
+function expectError() {
+  it('calls the error middleware', function () {
+    expect(this.next).to.have.been.calledWith(sinon.match.instanceOf(Error))
+  })
+}
+
+function expectNotFound() {
+  it('raises a 404', function () {
+    expect(this.next).to.have.been.calledWith(
+      sinon.match.instanceOf(Errors.NotFoundError)
+    )
+  })
+}
+
+function expectForbidden() {
+  it('raises a 403', function () {
+    expect(this.HttpErrorHandler.forbidden).to.have.been.calledWith(
+      this.req,
+      this.res
+    )
+    expect(this.next).not.to.have.been.called
+  })
+}
+
+function expectRedirectToRestricted() {
+  it('redirects to restricted', function () {
+    expect(this.res.redirect).to.have.been.calledWith(
+      '/restricted?from=%2Fcurrent%2Furl'
+    )
+    expect(this.next).not.to.have.been.called
+  })
+}