From b9fb636f0b299f1d1b05ca6466d4c82df526ffce Mon Sep 17 00:00:00 2001 From: Antoine Clausse Date: Fri, 17 Jan 2025 09:06:05 +0100 Subject: [PATCH] [web] Remove `promises` exports from Controller modules (#22242) * Remove promises object from CollaboratorsInviteController.mjs * Define functions at root * Remove mentions of undefined `revokeInviteForUser` * Remove unused `doLogout` * Remove promises object from UserController.js * Remove unused `makeChangePreview` * Remove promises object from SubscriptionController.js (`getRecommendedCurrency` and `getLatamCountryBannerDetails`) * Remove promises object from CollabratecController.mjs * Remove promises object from SSOController.mjs * Remove promises object from ReferencesApiController.mjs * Remove promises object from MetricsEmailController.mjs * Remove promises object from InstitutionHubsController.mjs * Remove promises object from DocumentUpdaterController.mjs * Remove promises object from SubscriptionAdminController.mjs * Fixup unit tests * Add expects that controllers don't error * Promisify `ensureAffiliationMiddleware` GitOrigin-RevId: 311c8afa7d5c8e4f051408d305b6b4147a020edc --- .../CollaboratorsInviteController.mjs | 675 +++++++++--------- .../DocumentUpdaterController.mjs | 3 - .../Subscription/SubscriptionController.js | 8 +- .../app/src/Features/User/UserController.js | 16 +- services/web/app/src/router.mjs | 2 +- services/web/scripts/ensure_affiliations.mjs | 2 +- .../CollaboratorsInviteControllerTests.mjs | 298 ++++---- .../DocumentUpdaterControllerTests.mjs | 11 +- .../test/unit/src/User/UserControllerTests.js | 12 +- 9 files changed, 487 insertions(+), 540 deletions(-) diff --git a/services/web/app/src/Features/Collaborators/CollaboratorsInviteController.mjs b/services/web/app/src/Features/Collaborators/CollaboratorsInviteController.mjs index 241ed2cb75..f0be9e7063 100644 --- a/services/web/app/src/Features/Collaborators/CollaboratorsInviteController.mjs +++ b/services/web/app/src/Features/Collaborators/CollaboratorsInviteController.mjs @@ -1,4 +1,3 @@ -import { callbackify } from 'node:util' import ProjectGetter from '../Project/ProjectGetter.js' import LimitationsManager from '../Subscription/LimitationsManager.js' import UserGetter from '../User/UserGetter.js' @@ -34,336 +33,218 @@ const rateLimiter = new RateLimiter('invite-to-project-by-user-id', { duration: 60 * 30, }) -const CollaboratorsInviteController = { - async getAllInvites(req, res) { - const projectId = req.params.Project_id - logger.debug({ projectId }, 'getting all active invites for project') - const invites = - await CollaboratorsInviteGetter.promises.getAllInvites(projectId) - res.json({ invites }) - }, +async function getAllInvites(req, res) { + const projectId = req.params.Project_id + logger.debug({ projectId }, 'getting all active invites for project') + const invites = + await CollaboratorsInviteGetter.promises.getAllInvites(projectId) + res.json({ invites }) +} - async _checkShouldInviteEmail(email) { - if (Settings.restrictInvitesToExistingAccounts === true) { - logger.debug({ email }, 'checking if user exists with this email') - const user = await UserGetter.promises.getUserByAnyEmail(email, { - _id: 1, - }) - const userExists = user?._id != null - return userExists - } else { - return true - } - }, - - async _checkRateLimit(userId) { - let collabLimit = - await LimitationsManager.promises.allowedNumberOfCollaboratorsForUser( - userId - ) - - if (collabLimit == null || collabLimit === 0) { - collabLimit = 1 - } else if (collabLimit < 0 || collabLimit > 20) { - collabLimit = 20 - } - - // Consume enough points to hit the rate limit at 10 * collabLimit - const maxRequests = 10 * collabLimit - const points = Math.floor(RATE_LIMIT_POINTS / maxRequests) - try { - await rateLimiter.consume(userId, points, { method: 'userId' }) - } catch (err) { - if (err instanceof Error) { - throw err - } else { - return false - } - } - return true - }, - - async inviteToProject(req, res) { - const projectId = req.params.Project_id - let { email, privileges } = req.body - const sendingUser = SessionManager.getSessionUser(req.session) - const sendingUserId = sendingUser._id - req.logger.addFields({ email, sendingUserId }) - - if (email === sendingUser.email) { - logger.debug( - { projectId, email, sendingUserId }, - 'cannot invite yourself to project' - ) - return res.json({ invite: null, error: 'cannot_invite_self' }) - } - - logger.debug({ projectId, email, sendingUserId }, 'inviting to project') - - const project = await ProjectGetter.promises.getProject(projectId, { - owner_ref: 1, +async function _checkShouldInviteEmail(email) { + if (Settings.restrictInvitesToExistingAccounts === true) { + logger.debug({ email }, 'checking if user exists with this email') + const user = await UserGetter.promises.getUserByAnyEmail(email, { + _id: 1, }) - const linkSharingChanges = - await SplitTestHandler.promises.getAssignmentForUser( - project.owner_ref, - 'link-sharing-warning' - ) + const userExists = user?._id != null + return userExists + } else { + return true + } +} - let allowed = false - if (linkSharingChanges?.variant === 'active') { - // if link-sharing-warning is active, can always invite read-only collaborators - if (privileges === PrivilegeLevels.READ_ONLY) { - allowed = true - } else { - allowed = await LimitationsManager.promises.canAddXEditCollaborators( - projectId, - 1 - ) - } +async function _checkRateLimit(userId) { + let collabLimit = + await LimitationsManager.promises.allowedNumberOfCollaboratorsForUser( + userId + ) + + if (collabLimit == null || collabLimit === 0) { + collabLimit = 1 + } else if (collabLimit < 0 || collabLimit > 20) { + collabLimit = 20 + } + + // Consume enough points to hit the rate limit at 10 * collabLimit + const maxRequests = 10 * collabLimit + const points = Math.floor(RATE_LIMIT_POINTS / maxRequests) + try { + await rateLimiter.consume(userId, points, { method: 'userId' }) + } catch (err) { + if (err instanceof Error) { + throw err } else { - allowed = await LimitationsManager.promises.canAddXCollaborators( + return false + } + } + return true +} + +async function inviteToProject(req, res) { + const projectId = req.params.Project_id + let { email, privileges } = req.body + const sendingUser = SessionManager.getSessionUser(req.session) + const sendingUserId = sendingUser._id + req.logger.addFields({ email, sendingUserId }) + + if (email === sendingUser.email) { + logger.debug( + { projectId, email, sendingUserId }, + 'cannot invite yourself to project' + ) + return res.json({ invite: null, error: 'cannot_invite_self' }) + } + + logger.debug({ projectId, email, sendingUserId }, 'inviting to project') + + const project = await ProjectGetter.promises.getProject(projectId, { + owner_ref: 1, + }) + const linkSharingChanges = + await SplitTestHandler.promises.getAssignmentForUser( + project.owner_ref, + 'link-sharing-warning' + ) + + let allowed = false + if (linkSharingChanges?.variant === 'active') { + // if link-sharing-warning is active, can always invite read-only collaborators + if (privileges === PrivilegeLevels.READ_ONLY) { + allowed = true + } else { + allowed = await LimitationsManager.promises.canAddXEditCollaborators( projectId, 1 ) } - - if (!allowed) { - logger.debug( - { projectId, email, sendingUserId }, - 'not allowed to invite more users to project' - ) - return res.json({ invite: null }) - } - - email = EmailHelper.parseEmail(email, true) - if (email == null || email === '') { - logger.debug({ projectId, email, sendingUserId }, 'invalid email address') - return res.status(400).json({ errorReason: 'invalid_email' }) - } - - const underRateLimit = - await CollaboratorsInviteController._checkRateLimit(sendingUserId) - if (!underRateLimit) { - return res.sendStatus(429) - } - - const shouldAllowInvite = - await CollaboratorsInviteController._checkShouldInviteEmail(email) - if (!shouldAllowInvite) { - logger.debug( - { email, projectId, sendingUserId }, - 'not allowed to send an invite to this email address' - ) - return res.json({ - invite: null, - error: 'cannot_invite_non_user', - }) - } - - const invite = await CollaboratorsInviteHandler.promises.inviteToProject( + } else { + allowed = await LimitationsManager.promises.canAddXCollaborators( projectId, - sendingUser, - email, - privileges + 1 ) + } + if (!allowed) { + logger.debug( + { projectId, email, sendingUserId }, + 'not allowed to invite more users to project' + ) + return res.json({ invite: null }) + } + + email = EmailHelper.parseEmail(email, true) + if (email == null || email === '') { + logger.debug({ projectId, email, sendingUserId }, 'invalid email address') + return res.status(400).json({ errorReason: 'invalid_email' }) + } + + const underRateLimit = + await CollaboratorsInviteController._checkRateLimit(sendingUserId) + if (!underRateLimit) { + return res.sendStatus(429) + } + + const shouldAllowInvite = + await CollaboratorsInviteController._checkShouldInviteEmail(email) + if (!shouldAllowInvite) { + logger.debug( + { email, projectId, sendingUserId }, + 'not allowed to send an invite to this email address' + ) + return res.json({ + invite: null, + error: 'cannot_invite_non_user', + }) + } + + const invite = await CollaboratorsInviteHandler.promises.inviteToProject( + projectId, + sendingUser, + email, + privileges + ) + + ProjectAuditLogHandler.addEntryInBackground( + projectId, + 'send-invite', + sendingUserId, + req.ip, + { + inviteId: invite._id, + privileges, + } + ) + + logger.debug({ projectId, email, sendingUserId }, 'invite created') + + EditorRealTimeController.emitToRoom(projectId, 'project:membership:changed', { + invites: true, + }) + res.json({ invite }) +} +async function revokeInvite(req, res) { + const projectId = req.params.Project_id + const inviteId = req.params.invite_id + const user = SessionManager.getSessionUser(req.session) + + logger.debug({ projectId, inviteId }, 'revoking invite') + + const invite = await CollaboratorsInviteHandler.promises.revokeInvite( + projectId, + inviteId + ) + + if (invite != null) { ProjectAuditLogHandler.addEntryInBackground( projectId, - 'send-invite', - sendingUserId, + 'revoke-invite', + user._id, req.ip, { inviteId: invite._id, - privileges, + privileges: invite.privileges, } ) - - logger.debug({ projectId, email, sendingUserId }, 'invite created') - EditorRealTimeController.emitToRoom( projectId, 'project:membership:changed', { invites: true } ) - res.json({ invite }) - }, - async revokeInvite(req, res) { - const projectId = req.params.Project_id - const inviteId = req.params.invite_id - const user = SessionManager.getSessionUser(req.session) + } - logger.debug({ projectId, inviteId }, 'revoking invite') + res.sendStatus(204) +} - const invite = await CollaboratorsInviteHandler.promises.revokeInvite( +async function generateNewInvite(req, res) { + const projectId = req.params.Project_id + const inviteId = req.params.invite_id + const user = SessionManager.getSessionUser(req.session) + + logger.debug({ projectId, inviteId }, 'resending invite') + const sendingUser = SessionManager.getSessionUser(req.session) + const underRateLimit = await CollaboratorsInviteController._checkRateLimit( + sendingUser._id + ) + if (!underRateLimit) { + return res.sendStatus(429) + } + + const invite = await CollaboratorsInviteHandler.promises.generateNewInvite( + projectId, + sendingUser, + inviteId + ) + + EditorRealTimeController.emitToRoom(projectId, 'project:membership:changed', { + invites: true, + }) + + if (invite != null) { + ProjectAuditLogHandler.addEntryInBackground( projectId, - inviteId - ) - - if (invite != null) { - ProjectAuditLogHandler.addEntryInBackground( - projectId, - 'revoke-invite', - user._id, - req.ip, - { - inviteId: invite._id, - privileges: invite.privileges, - } - ) - EditorRealTimeController.emitToRoom( - projectId, - 'project:membership:changed', - { invites: true } - ) - } - - res.sendStatus(204) - }, - - async generateNewInvite(req, res) { - const projectId = req.params.Project_id - const inviteId = req.params.invite_id - const user = SessionManager.getSessionUser(req.session) - - logger.debug({ projectId, inviteId }, 'resending invite') - const sendingUser = SessionManager.getSessionUser(req.session) - const underRateLimit = await CollaboratorsInviteController._checkRateLimit( - sendingUser._id - ) - if (!underRateLimit) { - return res.sendStatus(429) - } - - const invite = await CollaboratorsInviteHandler.promises.generateNewInvite( - projectId, - sendingUser, - inviteId - ) - - EditorRealTimeController.emitToRoom( - projectId, - 'project:membership:changed', - { invites: true } - ) - - if (invite != null) { - ProjectAuditLogHandler.addEntryInBackground( - projectId, - 'resend-invite', - user._id, - req.ip, - { - inviteId: invite._id, - privileges: invite.privileges, - } - ) - - res.sendStatus(201) - } else { - res.sendStatus(404) - } - }, - - async viewInvite(req, res) { - const projectId = req.params.Project_id - const { token } = req.params - const _renderInvalidPage = function () { - res.status(404) - logger.debug({ projectId }, 'invite not valid, rendering not-valid page') - res.render('project/invite/not-valid', { title: 'Invalid Invite' }) - } - - // check if the user is already a member of the project - const currentUser = SessionManager.getSessionUser(req.session) - if (currentUser) { - const isMember = - await CollaboratorsGetter.promises.isUserInvitedMemberOfProject( - currentUser._id, - projectId - ) - if (isMember) { - logger.debug( - { projectId, userId: currentUser._id }, - 'user is already a member of this project, redirecting' - ) - return res.redirect(`/project/${projectId}`) - } - } - - // get the invite - const invite = await CollaboratorsInviteGetter.promises.getInviteByToken( - projectId, - token - ) - - // check if invite is gone, or otherwise non-existent - if (invite == null) { - logger.debug({ projectId }, 'no invite found for this token') - return _renderInvalidPage() - } - - // check the user who sent the invite exists - const owner = await UserGetter.promises.getUser( - { _id: invite.sendingUserId }, - { email: 1, first_name: 1, last_name: 1 } - ) - if (owner == null) { - logger.debug({ projectId }, 'no project owner found') - return _renderInvalidPage() - } - - // fetch the project name - const project = await ProjectGetter.promises.getProject(projectId, { - name: 1, - }) - if (project == null) { - logger.debug({ projectId }, 'no project found') - return _renderInvalidPage() - } - - if (!currentUser) { - req.session.sharedProjectData = { - project_name: project.name, - user_first_name: owner.first_name, - } - AuthenticationController.setRedirectInSession(req) - return res.redirect('/register') - } - - // cleanup if set for register page - delete req.session.sharedProjectData - - // finally render the invite - res.render('project/invite/show', { - invite, - token, - project, - owner, - title: 'Project Invite', - }) - }, - - async acceptInvite(req, res) { - const { Project_id: projectId, token } = req.params - const currentUser = SessionManager.getSessionUser(req.session) - logger.debug( - { projectId, userId: currentUser._id }, - 'got request to accept invite' - ) - - const invite = await CollaboratorsInviteGetter.promises.getInviteByToken( - projectId, - token - ) - - if (invite == null) { - throw new Errors.NotFoundError('no matching invite found') - } - - await ProjectAuditLogHandler.promises.addEntry( - projectId, - 'accept-invite', - currentUser._id, + 'resend-invite', + user._id, req.ip, { inviteId: invite._id, @@ -371,48 +252,154 @@ const CollaboratorsInviteController = { } ) - await CollaboratorsInviteHandler.promises.acceptInvite( - invite, - projectId, - currentUser - ) + res.sendStatus(201) + } else { + res.sendStatus(404) + } +} - await EditorRealTimeController.emitToRoom( - projectId, - 'project:membership:changed', - { invites: true, members: true } - ) - AnalyticsManager.recordEventForUserInBackground( - currentUser._id, - 'project-invite-accept', - { - projectId, - } - ) +async function viewInvite(req, res) { + const projectId = req.params.Project_id + const { token } = req.params + const _renderInvalidPage = function () { + res.status(404) + logger.debug({ projectId }, 'invite not valid, rendering not-valid page') + res.render('project/invite/not-valid', { title: 'Invalid Invite' }) + } - if (req.xhr) { - res.sendStatus(204) // Done async via project page notification - } else { - res.redirect(`/project/${projectId}`) + // check if the user is already a member of the project + const currentUser = SessionManager.getSessionUser(req.session) + if (currentUser) { + const isMember = + await CollaboratorsGetter.promises.isUserInvitedMemberOfProject( + currentUser._id, + projectId + ) + if (isMember) { + logger.debug( + { projectId, userId: currentUser._id }, + 'user is already a member of this project, redirecting' + ) + return res.redirect(`/project/${projectId}`) } - }, + } + + // get the invite + const invite = await CollaboratorsInviteGetter.promises.getInviteByToken( + projectId, + token + ) + + // check if invite is gone, or otherwise non-existent + if (invite == null) { + logger.debug({ projectId }, 'no invite found for this token') + return _renderInvalidPage() + } + + // check the user who sent the invite exists + const owner = await UserGetter.promises.getUser( + { _id: invite.sendingUserId }, + { email: 1, first_name: 1, last_name: 1 } + ) + if (owner == null) { + logger.debug({ projectId }, 'no project owner found') + return _renderInvalidPage() + } + + // fetch the project name + const project = await ProjectGetter.promises.getProject(projectId, { + name: 1, + }) + if (project == null) { + logger.debug({ projectId }, 'no project found') + return _renderInvalidPage() + } + + if (!currentUser) { + req.session.sharedProjectData = { + project_name: project.name, + user_first_name: owner.first_name, + } + AuthenticationController.setRedirectInSession(req) + return res.redirect('/register') + } + + // cleanup if set for register page + delete req.session.sharedProjectData + + // finally render the invite + res.render('project/invite/show', { + invite, + token, + project, + owner, + title: 'Project Invite', + }) } -export default { - promises: CollaboratorsInviteController, - getAllInvites: expressify(CollaboratorsInviteController.getAllInvites), - inviteToProject: expressify(CollaboratorsInviteController.inviteToProject), - revokeInvite: expressify(CollaboratorsInviteController.revokeInvite), - revokeInviteForUser: expressify( - CollaboratorsInviteController.revokeInviteForUser - ), - generateNewInvite: expressify( - CollaboratorsInviteController.generateNewInvite - ), - viewInvite: expressify(CollaboratorsInviteController.viewInvite), - acceptInvite: expressify(CollaboratorsInviteController.acceptInvite), - _checkShouldInviteEmail: callbackify( - CollaboratorsInviteController._checkShouldInviteEmail - ), - _checkRateLimit: callbackify(CollaboratorsInviteController._checkRateLimit), +async function acceptInvite(req, res) { + const { Project_id: projectId, token } = req.params + const currentUser = SessionManager.getSessionUser(req.session) + logger.debug( + { projectId, userId: currentUser._id }, + 'got request to accept invite' + ) + + const invite = await CollaboratorsInviteGetter.promises.getInviteByToken( + projectId, + token + ) + + if (invite == null) { + throw new Errors.NotFoundError('no matching invite found') + } + + await ProjectAuditLogHandler.promises.addEntry( + projectId, + 'accept-invite', + currentUser._id, + req.ip, + { + inviteId: invite._id, + privileges: invite.privileges, + } + ) + + await CollaboratorsInviteHandler.promises.acceptInvite( + invite, + projectId, + currentUser + ) + + await EditorRealTimeController.emitToRoom( + projectId, + 'project:membership:changed', + { invites: true, members: true } + ) + AnalyticsManager.recordEventForUserInBackground( + currentUser._id, + 'project-invite-accept', + { + projectId, + } + ) + + if (req.xhr) { + res.sendStatus(204) // Done async via project page notification + } else { + res.redirect(`/project/${projectId}`) + } } + +const CollaboratorsInviteController = { + getAllInvites: expressify(getAllInvites), + inviteToProject: expressify(inviteToProject), + revokeInvite: expressify(revokeInvite), + generateNewInvite: expressify(generateNewInvite), + viewInvite: expressify(viewInvite), + acceptInvite: expressify(acceptInvite), + _checkShouldInviteEmail, + _checkRateLimit, +} + +export default CollaboratorsInviteController diff --git a/services/web/app/src/Features/DocumentUpdater/DocumentUpdaterController.mjs b/services/web/app/src/Features/DocumentUpdater/DocumentUpdaterController.mjs index 80f42e26c6..d02b5a71f0 100644 --- a/services/web/app/src/Features/DocumentUpdater/DocumentUpdaterController.mjs +++ b/services/web/app/src/Features/DocumentUpdater/DocumentUpdaterController.mjs @@ -44,7 +44,4 @@ async function getDoc(req, res) { export default { getDoc: expressify(getDoc), - promises: { - getDoc, - }, } diff --git a/services/web/app/src/Features/Subscription/SubscriptionController.js b/services/web/app/src/Features/Subscription/SubscriptionController.js index bbf3802606..987acd7fe5 100644 --- a/services/web/app/src/Features/Subscription/SubscriptionController.js +++ b/services/web/app/src/Features/Subscription/SubscriptionController.js @@ -544,7 +544,7 @@ async function redirectToHostedPage(req, res) { res.redirect(url) } -async function _getRecommendedCurrency(req, res) { +async function getRecommendedCurrency(req, res) { const userId = SessionManager.getLoggedInUserId(req.session) let ip = req.ip if ( @@ -683,8 +683,6 @@ module.exports = { purchaseAddon, removeAddon, makeChangePreview, - promises: { - getRecommendedCurrency: _getRecommendedCurrency, - getLatamCountryBannerDetails, - }, + getRecommendedCurrency, + getLatamCountryBannerDetails, } diff --git a/services/web/app/src/Features/User/UserController.js b/services/web/app/src/Features/User/UserController.js index 3c1818979f..79347a03b7 100644 --- a/services/web/app/src/Features/User/UserController.js +++ b/services/web/app/src/Features/User/UserController.js @@ -16,7 +16,7 @@ const HttpErrorHandler = require('../Errors/HttpErrorHandler') const OError = require('@overleaf/o-error') const EmailHandler = require('../Email/EmailHandler') const UrlHelper = require('../Helpers/UrlHelper') -const { promisify, callbackify } = require('util') +const { promisify } = require('util') const { expressify } = require('@overleaf/promise-utils') const { acceptsJson, @@ -212,11 +212,7 @@ async function ensureAffiliationMiddleware(req, res, next) { return next() } } - try { - await ensureAffiliation(user) - } catch (error) { - return next(error) - } + await ensureAffiliation(user) return next() } @@ -505,13 +501,9 @@ module.exports = { subscribe: expressify(subscribe), unsubscribe: expressify(unsubscribe), updateUserSettings: expressify(updateUserSettings), - doLogout: callbackify(doLogout), logout: expressify(logout), expireDeletedUser: expressify(expireDeletedUser), expireDeletedUsersAfterDuration: expressify(expireDeletedUsersAfterDuration), - promises: { - doLogout, - ensureAffiliation, - ensureAffiliationMiddleware, - }, + ensureAffiliationMiddleware: expressify(ensureAffiliationMiddleware), + ensureAffiliation, } diff --git a/services/web/app/src/router.mjs b/services/web/app/src/router.mjs index da3ac2958b..44175b455a 100644 --- a/services/web/app/src/router.mjs +++ b/services/web/app/src/router.mjs @@ -314,7 +314,7 @@ async function initialize(webRouter, privateApiRouter, publicApiRouter) { '/user/emails', AuthenticationController.requireLogin(), PermissionsController.useCapabilities(), - UserController.promises.ensureAffiliationMiddleware, + UserController.ensureAffiliationMiddleware, UserEmailsController.list ) webRouter.get( diff --git a/services/web/scripts/ensure_affiliations.mjs b/services/web/scripts/ensure_affiliations.mjs index f6376614e7..98d9fd8b04 100644 --- a/services/web/scripts/ensure_affiliations.mjs +++ b/services/web/scripts/ensure_affiliations.mjs @@ -15,7 +15,7 @@ const query = { async function _handleEnsureAffiliation(user) { try { - await UserController.promises.ensureAffiliation(user) + await UserController.ensureAffiliation(user) console.log(`✔ ${user._id}`) success.push(user._id) } catch (error) { diff --git a/services/web/test/unit/src/Collaborators/CollaboratorsInviteControllerTests.mjs b/services/web/test/unit/src/Collaborators/CollaboratorsInviteControllerTests.mjs index 1ce63d4793..b806d5c773 100644 --- a/services/web/test/unit/src/Collaborators/CollaboratorsInviteControllerTests.mjs +++ b/services/web/test/unit/src/Collaborators/CollaboratorsInviteControllerTests.mjs @@ -238,17 +238,17 @@ describe('CollaboratorsInviteController', function () { }) describe('when all goes well', function (done) { - beforeEach(function (done) { - this.CollaboratorsInviteController.promises._checkShouldInviteEmail = - sinon.stub().resolves(true) - this.CollaboratorsInviteController.promises._checkRateLimit = sinon + beforeEach(async function () { + this.CollaboratorsInviteController._checkShouldInviteEmail = sinon .stub() .resolves(true) - this.res.callback = () => done() - this.CollaboratorsInviteController.inviteToProject( + this.CollaboratorsInviteController._checkRateLimit = sinon + .stub() + .resolves(true) + + await this.CollaboratorsInviteController.inviteToProject( this.req, - this.res, - done + this.res ) }) @@ -269,10 +269,11 @@ describe('CollaboratorsInviteController', function () { }) it('should have called _checkShouldInviteEmail', function () { - this.CollaboratorsInviteController.promises._checkShouldInviteEmail.callCount.should.equal( + this.CollaboratorsInviteController._checkShouldInviteEmail.callCount.should.equal( 1 ) - this.CollaboratorsInviteController.promises._checkShouldInviteEmail + + this.CollaboratorsInviteController._checkShouldInviteEmail .calledWith(this.targetEmail) .should.equal(true) }) @@ -322,9 +323,10 @@ describe('CollaboratorsInviteController', function () { describe('readAndWrite collaborator', function () { beforeEach(function (done) { this.privileges = 'readAndWrite' - this.CollaboratorsInviteController.promises._checkShouldInviteEmail = - sinon.stub().resolves(true) - this.CollaboratorsInviteController.promises._checkRateLimit = sinon + this.CollaboratorsInviteController._checkShouldInviteEmail = sinon + .stub() + .resolves(true) + this.CollaboratorsInviteController._checkRateLimit = sinon .stub() .resolves(true) this.res.callback = () => done() @@ -343,10 +345,10 @@ describe('CollaboratorsInviteController', function () { }) it('should not have called _checkShouldInviteEmail', function () { - this.CollaboratorsInviteController.promises._checkShouldInviteEmail.callCount.should.equal( + this.CollaboratorsInviteController._checkShouldInviteEmail.callCount.should.equal( 0 ) - this.CollaboratorsInviteController.promises._checkShouldInviteEmail + this.CollaboratorsInviteController._checkShouldInviteEmail .calledWith(this.currentUser, this.targetEmail) .should.equal(false) }) @@ -364,9 +366,10 @@ describe('CollaboratorsInviteController', function () { email: this.targetEmail, privileges: (this.privileges = 'readOnly'), } - this.CollaboratorsInviteController.promises._checkShouldInviteEmail = - sinon.stub().resolves(true) - this.CollaboratorsInviteController.promises._checkRateLimit = sinon + this.CollaboratorsInviteController._checkShouldInviteEmail = sinon + .stub() + .resolves(true) + this.CollaboratorsInviteController._checkRateLimit = sinon .stub() .resolves(true) this.res.callback = () => done() @@ -391,10 +394,10 @@ describe('CollaboratorsInviteController', function () { }) it('should have called _checkShouldInviteEmail', function () { - this.CollaboratorsInviteController.promises._checkShouldInviteEmail.callCount.should.equal( + this.CollaboratorsInviteController._checkShouldInviteEmail.callCount.should.equal( 1 ) - this.CollaboratorsInviteController.promises._checkShouldInviteEmail + this.CollaboratorsInviteController._checkShouldInviteEmail .calledWith(this.targetEmail) .should.equal(true) }) @@ -438,9 +441,10 @@ describe('CollaboratorsInviteController', function () { describe('when all goes well', function (done) { beforeEach(function (done) { - this.CollaboratorsInviteController.promises._checkShouldInviteEmail = - sinon.stub().resolves(true) - this.CollaboratorsInviteController.promises._checkRateLimit = sinon + this.CollaboratorsInviteController._checkShouldInviteEmail = sinon + .stub() + .resolves(true) + this.CollaboratorsInviteController._checkRateLimit = sinon .stub() .resolves(true) this.res.callback = () => done() @@ -468,10 +472,10 @@ describe('CollaboratorsInviteController', function () { }) it('should have called _checkShouldInviteEmail', function () { - this.CollaboratorsInviteController.promises._checkShouldInviteEmail.callCount.should.equal( + this.CollaboratorsInviteController._checkShouldInviteEmail.callCount.should.equal( 1 ) - this.CollaboratorsInviteController.promises._checkShouldInviteEmail + this.CollaboratorsInviteController._checkShouldInviteEmail .calledWith(this.targetEmail) .should.equal(true) }) @@ -513,9 +517,10 @@ describe('CollaboratorsInviteController', function () { describe('when the user is not allowed to add more collaborators', function () { beforeEach(function (done) { - this.CollaboratorsInviteController.promises._checkShouldInviteEmail = - sinon.stub().resolves(true) - this.CollaboratorsInviteController.promises._checkRateLimit = sinon + this.CollaboratorsInviteController._checkShouldInviteEmail = sinon + .stub() + .resolves(true) + this.CollaboratorsInviteController._checkRateLimit = sinon .stub() .resolves(true) this.LimitationsManager.promises.canAddXCollaborators.resolves(false) @@ -533,10 +538,10 @@ describe('CollaboratorsInviteController', function () { }) it('should not have called _checkShouldInviteEmail', function () { - this.CollaboratorsInviteController.promises._checkShouldInviteEmail.callCount.should.equal( + this.CollaboratorsInviteController._checkShouldInviteEmail.callCount.should.equal( 0 ) - this.CollaboratorsInviteController.promises._checkShouldInviteEmail + this.CollaboratorsInviteController._checkShouldInviteEmail .calledWith(this.currentUser, this.targetEmail) .should.equal(false) }) @@ -550,9 +555,10 @@ describe('CollaboratorsInviteController', function () { describe('when canAddXCollaborators produces an error', function () { beforeEach(function (done) { - this.CollaboratorsInviteController.promises._checkShouldInviteEmail = - sinon.stub().resolves(true) - this.CollaboratorsInviteController.promises._checkRateLimit = sinon + this.CollaboratorsInviteController._checkShouldInviteEmail = sinon + .stub() + .resolves(true) + this.CollaboratorsInviteController._checkRateLimit = sinon .stub() .resolves(true) this.LimitationsManager.promises.canAddXCollaborators.rejects( @@ -572,10 +578,10 @@ describe('CollaboratorsInviteController', function () { }) it('should not have called _checkShouldInviteEmail', function () { - this.CollaboratorsInviteController.promises._checkShouldInviteEmail.callCount.should.equal( + this.CollaboratorsInviteController._checkShouldInviteEmail.callCount.should.equal( 0 ) - this.CollaboratorsInviteController.promises._checkShouldInviteEmail + this.CollaboratorsInviteController._checkShouldInviteEmail .calledWith(this.currentUser, this.targetEmail) .should.equal(false) }) @@ -589,9 +595,10 @@ describe('CollaboratorsInviteController', function () { describe('when inviteToProject produces an error', function () { beforeEach(function (done) { - this.CollaboratorsInviteController.promises._checkShouldInviteEmail = - sinon.stub().resolves(true) - this.CollaboratorsInviteController.promises._checkRateLimit = sinon + this.CollaboratorsInviteController._checkShouldInviteEmail = sinon + .stub() + .resolves(true) + this.CollaboratorsInviteController._checkRateLimit = sinon .stub() .resolves(true) this.CollaboratorsInviteHandler.promises.inviteToProject.rejects( @@ -620,10 +627,10 @@ describe('CollaboratorsInviteController', function () { }) it('should have called _checkShouldInviteEmail', function () { - this.CollaboratorsInviteController.promises._checkShouldInviteEmail.callCount.should.equal( + this.CollaboratorsInviteController._checkShouldInviteEmail.callCount.should.equal( 1 ) - this.CollaboratorsInviteController.promises._checkShouldInviteEmail + this.CollaboratorsInviteController._checkShouldInviteEmail .calledWith(this.targetEmail) .should.equal(true) }) @@ -645,9 +652,10 @@ describe('CollaboratorsInviteController', function () { describe('when _checkShouldInviteEmail disallows the invite', function () { beforeEach(function (done) { - this.CollaboratorsInviteController.promises._checkShouldInviteEmail = - sinon.stub().resolves(false) - this.CollaboratorsInviteController.promises._checkRateLimit = sinon + this.CollaboratorsInviteController._checkShouldInviteEmail = sinon + .stub() + .resolves(false) + this.CollaboratorsInviteController._checkRateLimit = sinon .stub() .resolves(true) this.res.callback = () => done() @@ -667,10 +675,10 @@ describe('CollaboratorsInviteController', function () { }) it('should have called _checkShouldInviteEmail', function () { - this.CollaboratorsInviteController.promises._checkShouldInviteEmail.callCount.should.equal( + this.CollaboratorsInviteController._checkShouldInviteEmail.callCount.should.equal( 1 ) - this.CollaboratorsInviteController.promises._checkShouldInviteEmail + this.CollaboratorsInviteController._checkShouldInviteEmail .calledWith(this.targetEmail) .should.equal(true) }) @@ -684,9 +692,10 @@ describe('CollaboratorsInviteController', function () { describe('when _checkShouldInviteEmail produces an error', function () { beforeEach(function (done) { - this.CollaboratorsInviteController.promises._checkShouldInviteEmail = - sinon.stub().rejects(new Error('woops')) - this.CollaboratorsInviteController.promises._checkRateLimit = sinon + this.CollaboratorsInviteController._checkShouldInviteEmail = sinon + .stub() + .rejects(new Error('woops')) + this.CollaboratorsInviteController._checkRateLimit = sinon .stub() .resolves(true) this.next.callsFake(() => done()) @@ -703,10 +712,10 @@ describe('CollaboratorsInviteController', function () { }) it('should have called _checkShouldInviteEmail', function () { - this.CollaboratorsInviteController.promises._checkShouldInviteEmail.callCount.should.equal( + this.CollaboratorsInviteController._checkShouldInviteEmail.callCount.should.equal( 1 ) - this.CollaboratorsInviteController.promises._checkShouldInviteEmail + this.CollaboratorsInviteController._checkShouldInviteEmail .calledWith(this.targetEmail) .should.equal(true) }) @@ -721,9 +730,10 @@ describe('CollaboratorsInviteController', function () { describe('when the user invites themselves to the project', function () { beforeEach(function () { this.req.body.email = this.currentUser.email - this.CollaboratorsInviteController.promises._checkShouldInviteEmail = - sinon.stub().resolves(true) - this.CollaboratorsInviteController.promises._checkRateLimit = sinon + this.CollaboratorsInviteController._checkShouldInviteEmail = sinon + .stub() + .resolves(true) + this.CollaboratorsInviteController._checkRateLimit = sinon .stub() .resolves(true) this.CollaboratorsInviteController.inviteToProject( @@ -748,7 +758,7 @@ describe('CollaboratorsInviteController', function () { }) it('should not have called _checkShouldInviteEmail', function () { - this.CollaboratorsInviteController.promises._checkShouldInviteEmail.callCount.should.equal( + this.CollaboratorsInviteController._checkShouldInviteEmail.callCount.should.equal( 0 ) }) @@ -765,14 +775,14 @@ describe('CollaboratorsInviteController', function () { }) describe('when _checkRateLimit returns false', function () { - beforeEach(function (done) { - this.CollaboratorsInviteController.promises._checkShouldInviteEmail = - sinon.stub().resolves(true) - this.CollaboratorsInviteController.promises._checkRateLimit = sinon + beforeEach(async function () { + this.CollaboratorsInviteController._checkShouldInviteEmail = sinon + .stub() + .resolves(true) + this.CollaboratorsInviteController._checkRateLimit = sinon .stub() .resolves(false) - this.res.callback = () => done() - this.CollaboratorsInviteController.inviteToProject( + await this.CollaboratorsInviteController.inviteToProject( this.req, this.res, this.next @@ -1281,7 +1291,7 @@ describe('CollaboratorsInviteController', function () { Project_id: this.projectId, invite_id: this.invite._id.toString(), } - this.CollaboratorsInviteController.promises._checkRateLimit = sinon + this.CollaboratorsInviteController._checkRateLimit = sinon .stub() .resolves(true) }) @@ -1316,7 +1326,7 @@ describe('CollaboratorsInviteController', function () { }) it('should check the rate limit', function () { - this.CollaboratorsInviteController.promises._checkRateLimit.callCount.should.equal( + this.CollaboratorsInviteController._checkRateLimit.callCount.should.equal( 1 ) }) @@ -1600,12 +1610,8 @@ describe('CollaboratorsInviteController', function () { describe('when we should be restricting to existing accounts', function () { beforeEach(function () { this.settings.restrictInvitesToExistingAccounts = true - this.call = callback => { - this.CollaboratorsInviteController._checkShouldInviteEmail( - this.email, - callback - ) - } + this.call = () => + this.CollaboratorsInviteController._checkShouldInviteEmail(this.email) }) describe('when user account is present', function () { @@ -1614,12 +1620,12 @@ describe('CollaboratorsInviteController', function () { this.UserGetter.promises.getUserByAnyEmail.resolves(this.user) }) - it('should callback with `true`', function (done) { - this.call((err, shouldAllow) => { - expect(err).to.equal(null) - expect(shouldAllow).to.equal(true) - done() - }) + it('should callback with `true`', async function () { + const shouldAllow = + await this.CollaboratorsInviteController._checkShouldInviteEmail( + this.email + ) + expect(shouldAllow).to.equal(true) }) }) @@ -1629,25 +1635,22 @@ describe('CollaboratorsInviteController', function () { this.UserGetter.promises.getUserByAnyEmail.resolves(this.user) }) - it('should callback with `false`', function (done) { - this.call((err, shouldAllow) => { - expect(err).to.equal(null) - expect(shouldAllow).to.equal(false) - done() - }) + it('should callback with `false`', async function () { + const shouldAllow = + await this.CollaboratorsInviteController._checkShouldInviteEmail( + this.email + ) + expect(shouldAllow).to.equal(false) }) - it('should have called getUser', function (done) { - this.call((err, shouldAllow) => { - if (err) { - return done(err) - } - this.UserGetter.promises.getUserByAnyEmail.callCount.should.equal(1) - this.UserGetter.promises.getUserByAnyEmail - .calledWith(this.email, { _id: 1 }) - .should.equal(true) - done() - }) + it('should have called getUser', async function () { + await this.CollaboratorsInviteController._checkShouldInviteEmail( + this.email + ) + this.UserGetter.promises.getUserByAnyEmail.callCount.should.equal(1) + this.UserGetter.promises.getUserByAnyEmail + .calledWith(this.email, { _id: 1 }) + .should.equal(true) }) }) @@ -1657,13 +1660,12 @@ describe('CollaboratorsInviteController', function () { this.UserGetter.promises.getUserByAnyEmail.rejects(new Error('woops')) }) - it('should callback with an error', function (done) { - this.call((err, shouldAllow) => { - expect(err).to.not.equal(null) - expect(err).to.be.instanceof(Error) - expect(shouldAllow).to.equal(undefined) - done() - }) + it('should callback with an error', async function () { + await expect( + this.CollaboratorsInviteController._checkShouldInviteEmail( + this.email + ) + ).to.be.rejected }) }) }) @@ -1678,90 +1680,60 @@ describe('CollaboratorsInviteController', function () { .resolves(17) }) - it('should callback with `true` when rate limit under', function (done) { - this.CollaboratorsInviteController._checkRateLimit( - this.currentUserId, - (err, result) => { - if (err) { - return done(err) - } - expect(this.rateLimiter.consume).to.have.been.calledWith( - this.currentUserId - ) - result.should.equal(true) - done() - } + it('should callback with `true` when rate limit under', async function () { + const result = await this.CollaboratorsInviteController._checkRateLimit( + this.currentUserId ) + expect(this.rateLimiter.consume).to.have.been.calledWith( + this.currentUserId + ) + result.should.equal(true) }) - it('should callback with `false` when rate limit hit', function (done) { + it('should callback with `false` when rate limit hit', async function () { this.rateLimiter.consume.rejects({ remainingPoints: 0 }) - this.CollaboratorsInviteController._checkRateLimit( + const result = await this.CollaboratorsInviteController._checkRateLimit( + this.currentUserId + ) + expect(this.rateLimiter.consume).to.have.been.calledWith( + this.currentUserId + ) + result.should.equal(false) + }) + + it('should allow 10x the collaborators', async function () { + await this.CollaboratorsInviteController._checkRateLimit( + this.currentUserId + ) + expect(this.rateLimiter.consume).to.have.been.calledWith( this.currentUserId, - (err, result) => { - if (err) { - return done(err) - } - expect(this.rateLimiter.consume).to.have.been.calledWith( - this.currentUserId - ) - result.should.equal(false) - done() - } + Math.floor(40000 / 170) ) }) - it('should allow 10x the collaborators', function (done) { - this.CollaboratorsInviteController._checkRateLimit( - this.currentUserId, - (err, result) => { - if (err) { - return done(err) - } - expect(this.rateLimiter.consume).to.have.been.calledWith( - this.currentUserId, - Math.floor(40000 / 170) - ) - done() - } - ) - }) - - it('should allow 200 requests when collaborators is -1', function (done) { + it('should allow 200 requests when collaborators is -1', async function () { this.LimitationsManager.promises.allowedNumberOfCollaboratorsForUser .withArgs(this.currentUserId) .resolves(-1) - this.CollaboratorsInviteController._checkRateLimit( + await this.CollaboratorsInviteController._checkRateLimit( + this.currentUserId + ) + expect(this.rateLimiter.consume).to.have.been.calledWith( this.currentUserId, - (err, result) => { - if (err) { - return done(err) - } - expect(this.rateLimiter.consume).to.have.been.calledWith( - this.currentUserId, - Math.floor(40000 / 200) - ) - done() - } + Math.floor(40000 / 200) ) }) - it('should allow 10 requests when user has no collaborators set', function (done) { + it('should allow 10 requests when user has no collaborators set', async function () { this.LimitationsManager.promises.allowedNumberOfCollaboratorsForUser .withArgs(this.currentUserId) .resolves(null) - this.CollaboratorsInviteController._checkRateLimit( + await this.CollaboratorsInviteController._checkRateLimit( + this.currentUserId + ) + expect(this.rateLimiter.consume).to.have.been.calledWith( this.currentUserId, - (err, result) => { - if (err) { - return done(err) - } - expect(this.rateLimiter.consume).to.have.been.calledWith( - this.currentUserId, - Math.floor(40000 / 10) - ) - done() - } + Math.floor(40000 / 10) ) }) }) diff --git a/services/web/test/unit/src/DocumentUpdater/DocumentUpdaterControllerTests.mjs b/services/web/test/unit/src/DocumentUpdater/DocumentUpdaterControllerTests.mjs index d78b01e467..6a783d452e 100644 --- a/services/web/test/unit/src/DocumentUpdater/DocumentUpdaterControllerTests.mjs +++ b/services/web/test/unit/src/DocumentUpdater/DocumentUpdaterControllerTests.mjs @@ -38,6 +38,7 @@ describe('DocumentUpdaterController', function () { } this.lines = ['test', '', 'testing'] this.res = new MockResponse() + this.next = sinon.stub() this.doc = { name: 'myfile.tex' } }) @@ -53,8 +54,7 @@ describe('DocumentUpdaterController', function () { }) it('should call the document updater handler with the project_id and doc_id', async function () { - await this.controller.promises.getDoc(this.req, this.res) - + await this.controller.getDoc(this.req, this.res, this.next) expect( this.DocumentUpdaterHandler.promises.getDocument ).to.have.been.calledOnceWith( @@ -65,13 +65,14 @@ describe('DocumentUpdaterController', function () { }) it('should return the content', async function () { - await this.controller.promises.getDoc(this.req, this.res) + await this.controller.getDoc(this.req, this.res) + expect(this.next).to.not.have.been.called expect(this.res.statusCode).to.equal(200) expect(this.res.body).to.equal('test\n\ntesting') }) it('should find the doc in the project', async function () { - await this.controller.promises.getDoc(this.req, this.res) + await this.controller.getDoc(this.req, this.res) expect( this.ProjectLocator.promises.findElement ).to.have.been.calledOnceWith({ @@ -82,7 +83,7 @@ describe('DocumentUpdaterController', function () { }) it('should set the Content-Disposition header', async function () { - await this.controller.promises.getDoc(this.req, this.res) + await this.controller.getDoc(this.req, this.res) expect(this.res.setContentDisposition).to.have.been.calledWith( 'attachment', { filename: this.doc.name } diff --git a/services/web/test/unit/src/User/UserControllerTests.js b/services/web/test/unit/src/User/UserControllerTests.js index 874220f3cd..717d136a09 100644 --- a/services/web/test/unit/src/User/UserControllerTests.js +++ b/services/web/test/unit/src/User/UserControllerTests.js @@ -916,7 +916,7 @@ describe('UserController', function () { describe('ensureAffiliationMiddleware', function () { describe('without affiliations feature', function () { beforeEach(async function () { - await this.UserController.promises.ensureAffiliationMiddleware( + await this.UserController.ensureAffiliationMiddleware( this.req, this.res, this.next @@ -938,7 +938,7 @@ describe('UserController', function () { describe('without ensureAffiliation query parameter', function () { beforeEach(async function () { this.Features.hasFeature.withArgs('affiliations').returns(true) - await this.UserController.promises.ensureAffiliationMiddleware( + await this.UserController.ensureAffiliationMiddleware( this.req, this.res, this.next @@ -968,7 +968,7 @@ describe('UserController', function () { ] this.Features.hasFeature.withArgs('affiliations').returns(true) this.req.query.ensureAffiliation = true - await this.UserController.promises.ensureAffiliationMiddleware( + await this.UserController.ensureAffiliationMiddleware( this.req, this.res, this.next @@ -1005,7 +1005,7 @@ describe('UserController', function () { this.Features.hasFeature.withArgs('affiliations').returns(true) this.req.query.ensureAffiliation = true this.req.assertPermission = sinon.stub() - await this.UserController.promises.ensureAffiliationMiddleware( + await this.UserController.ensureAffiliationMiddleware( this.req, this.res, this.next @@ -1047,7 +1047,7 @@ describe('UserController', function () { this.Features.hasFeature.withArgs('affiliations').returns(true) this.req.query.ensureAffiliation = true this.req.assertPermission = sinon.stub() - await this.UserController.promises.ensureAffiliationMiddleware( + await this.UserController.ensureAffiliationMiddleware( this.req, this.res, this.next @@ -1089,7 +1089,7 @@ describe('UserController', function () { this.Features.hasFeature.withArgs('affiliations').returns(true) this.req.query.ensureAffiliation = true this.req.assertPermission = sinon.stub() - await this.UserController.promises.ensureAffiliationMiddleware( + await this.UserController.ensureAffiliationMiddleware( this.req, this.res, this.next