diff --git a/services/web/app/src/Features/Collaborators/CollaboratorsInviteController.mjs b/services/web/app/src/Features/Collaborators/CollaboratorsInviteController.mjs index a42b3473ea..5162421564 100644 --- a/services/web/app/src/Features/Collaborators/CollaboratorsInviteController.mjs +++ b/services/web/app/src/Features/Collaborators/CollaboratorsInviteController.mjs @@ -22,6 +22,7 @@ import PrivilegeLevels, { } from '../Authorization/PrivilegeLevels.mjs' import SplitTestHandler from '../SplitTests/SplitTestHandler.mjs' import SubscriptionGroupHandler from '../Subscription/SubscriptionGroupHandler.mjs' +import SubscriptionLocator from '../Subscription/SubscriptionLocator.mjs' // This rate limiter allows a different number of requests depending on the // number of callaborators a user is allowed. This is implemented by providing @@ -554,10 +555,30 @@ async function updateSharingLink(req, res) { const privileges = body.privileges const subscriptionId = body.subscriptionId + const currentUser = SessionManager.getSessionUser(req.session) + + if (subscriptionId) { + const subscriptions = + await SubscriptionLocator.promises.getUserActiveProfessionalGroupSubscriptions( + currentUser._id, + { _id: 1 } + ) + const canShareWithSubscription = subscriptions.some( + subscription => subscription._id.toString() === subscriptionId.toString() + ) + + if (!canShareWithSubscription) { + logger.debug( + { projectId, subscriptionId, userId: currentUser._id }, + 'cannot create a group sharing link for a non-professional or non-member subscription' + ) + return res.status(403).json({ errorReason: 'subscription_not_eligible' }) + } + } + let invite = await CollaboratorsInviteGetter.promises.getSharingLinkInvite(projectId) - const currentUser = SessionManager.getSessionUser(req.session) if (invite === null) { invite = await CollaboratorsInviteHandler.promises.createSharingLinkInvite( projectId, diff --git a/services/web/app/src/Features/Project/ProjectController.mjs b/services/web/app/src/Features/Project/ProjectController.mjs index 87c7e5bbd6..8658129a8f 100644 --- a/services/web/app/src/Features/Project/ProjectController.mjs +++ b/services/web/app/src/Features/Project/ProjectController.mjs @@ -574,19 +574,22 @@ const _ProjectController = { ) ).isMember)() : false, - activeGroupSubscriptions: - SubscriptionLocator.promises.getUserActiveGroupSubscriptions(userId, { - _id: 1, - teamName: 1, - sharingPermissions: 1, - }), + activeProfessionalGroupSubscriptions: + SubscriptionLocator.promises.getUserActiveProfessionalGroupSubscriptions( + userId, + { + _id: 1, + teamName: 1, + sharingPermissions: 1, + } + ), }) const { project, userValues, userIsMemberOfGroupSubscription, - activeGroupSubscriptions, + activeProfessionalGroupSubscriptions, } = responses await Promise.all([ @@ -976,7 +979,7 @@ const _ProjectController = { ), isMemberOfGroupSubscription: userIsMemberOfGroupSubscription, hasInstitutionLicence: userHasInstitutionLicence, - activeGroupSubscriptions, + activeProfessionalGroupSubscriptions, }, initialLoadingScreenTheme, userSettings, diff --git a/services/web/app/src/Features/Subscription/SubscriptionLocator.mjs b/services/web/app/src/Features/Subscription/SubscriptionLocator.mjs index 1374906c1b..11904b1843 100644 --- a/services/web/app/src/Features/Subscription/SubscriptionLocator.mjs +++ b/services/web/app/src/Features/Subscription/SubscriptionLocator.mjs @@ -7,6 +7,7 @@ import logger from '@overleaf/logger' import { AI_ADD_ON_CODE, isStandaloneAiAddOnPlanCode } from './AiHelper.mjs' import './GroupPlansData.mjs' // make sure dynamic group plans are loaded import Features from '../../infrastructure/Features.mjs' +import { isProfessionalGroupPlan } from './PlansHelper.mjs' const SubscriptionLocator = { async getUsersSubscription(userOrId) { @@ -180,11 +181,14 @@ const SubscriptionLocator = { } }, - async getUserActiveGroupSubscriptions(userOrId, projection = {}) { + async getUserActiveProfessionalGroupSubscriptions(userOrId, projection = {}) { if (!Features.hasFeature('saas')) return [] const userId = SubscriptionLocator._getUserId(userOrId) - return await Subscription.find( + + if (!userId) return [] + + const activeGroupSubscriptions = await Subscription.find( { groupPlan: true, $and: [ @@ -197,8 +201,16 @@ const SubscriptionLocator = { }, ], }, - projection + { + ...projection, + groupPlan: 1, + planCode: 1, + } ).exec() + + return activeGroupSubscriptions.filter(subscription => + isProfessionalGroupPlan(subscription) + ) }, async getUserSubscriptionStatus(userId) { diff --git a/services/web/frontend/js/features/share-project-modal/components/project-access.tsx b/services/web/frontend/js/features/share-project-modal/components/project-access.tsx index af2857c396..82c89166d7 100644 --- a/services/web/frontend/js/features/share-project-modal/components/project-access.tsx +++ b/services/web/frontend/js/features/share-project-modal/components/project-access.tsx @@ -31,6 +31,7 @@ import { ExcludeStrict } from '@ol-types/utils' import getMeta from '@/utils/meta' import { useFeatureFlag } from '@/shared/context/split-test-context' import { sendMB } from '@/infrastructure/event-tracking' +import { debugConsole } from '@/utils/debugging' type ProjectAccessProps = { setIsInvitedPeopleScreen: React.Dispatch> @@ -51,7 +52,7 @@ function ProjectAccess({ null ) const { isProjectOwner } = useEditorContext() - const { activeGroupSubscriptions } = getMeta('ol-user') + const { activeProfessionalGroupSubscriptions } = getMeta('ol-user') const groupSharingEnabled = useFeatureFlag('group-link-sharing') const { @@ -92,16 +93,18 @@ function ProjectAccess({ } return data - }).then(data => { - setSharingLinkData(data) - setProjectAccess(newAccess) - setSuccessActionMessage(t('access_updated')) - sendMB('sharing-link-set-permissions', { - project_id: projectId, - access_level: newAccess.split('.')[0], - ...reqBody, - }) }) + .then(data => { + setSharingLinkData(data) + setProjectAccess(newAccess) + setSuccessActionMessage(t('access_updated')) + sendMB('sharing-link-set-permissions', { + project_id: projectId, + access_level: newAccess.split('.')[0], + ...reqBody, + }) + }) + .catch(debugConsole.error) } const onAccessSelect = (eventKey: ProjectAccessType) => { @@ -125,27 +128,31 @@ function ProjectAccess({ privileges: eventKey, subscriptionId: sharingLinkData?.subscriptionId, }) - ).then(data => { - setSharingLinkData(data) - setSuccessActionMessage(t('access_updated')) - sendMB('sharing-link-set-permissions', { - project_id: projectId, - access_level: projectAccess?.split('.')[0], - privileges: eventKey, - subscriptionId: data.subscriptionId, + ) + .then(data => { + setSharingLinkData(data) + setSuccessActionMessage(t('access_updated')) + sendMB('sharing-link-set-permissions', { + project_id: projectId, + access_level: projectAccess?.split('.')[0], + privileges: eventKey, + subscriptionId: data.subscriptionId, + }) }) - }) + .catch(debugConsole.error) } const getGroupLinkText = (id?: string) => { if ( !id || - !activeGroupSubscriptions || - activeGroupSubscriptions.length === 0 + !activeProfessionalGroupSubscriptions || + activeProfessionalGroupSubscriptions.length === 0 ) { return '' } - const subscription = activeGroupSubscriptions.find(sub => sub._id === id) + const subscription = activeProfessionalGroupSubscriptions.find( + sub => sub._id === id + ) if (subscription?.teamName) { return t('anyone_in_x_with_the_link', { groupName: subscription.teamName, @@ -252,8 +259,8 @@ function ProjectAccess({ {groupSharingEnabled && - activeGroupSubscriptions && - activeGroupSubscriptions.map(subscription => ( + activeProfessionalGroupSubscriptions && + activeProfessionalGroupSubscriptions.map(subscription => ( }) => { - setError( - error.data?.errorReason || - error.data?.error || - 'generic_something_went_wrong' - ) - }) - - promise.finally(() => { - setInFlight(false) - }) - - return promise + return request() + .catch((error: { data?: Record }) => { + setError( + error.data?.errorReason || + error.data?.error || + 'generic_something_went_wrong' + ) + throw error + }) + .finally(() => { + setInFlight(false) + }) }, []) if (!project) { diff --git a/services/web/test/acceptance/src/ProjectInviteTests.mjs b/services/web/test/acceptance/src/ProjectInviteTests.mjs index 619468adc3..b8180ef2f4 100644 --- a/services/web/test/acceptance/src/ProjectInviteTests.mjs +++ b/services/web/test/acceptance/src/ProjectInviteTests.mjs @@ -1086,6 +1086,9 @@ describe('ProjectInviteTests', function () { const subscription = new Subscription({ adminId: this.sendingUser._id, memberIds: [this.sendingUser._id], + groupPlan: true, + planCode: 'group_professional', + paymentProvider: { state: 'active' }, }) let sharingLink Async.series( diff --git a/services/web/test/frontend/features/share-project-modal/components/share-project-modal.test.tsx b/services/web/test/frontend/features/share-project-modal/components/share-project-modal.test.tsx index e6aea59207..77f2fccd90 100644 --- a/services/web/test/frontend/features/share-project-modal/components/share-project-modal.test.tsx +++ b/services/web/test/frontend/features/share-project-modal/components/share-project-modal.test.tsx @@ -1102,7 +1102,7 @@ describe('', function () { user: { id: USER_ID, email: USER_EMAIL, - activeGroupSubscriptions: [{ _id: 'sub-123' }], + activeProfessionalGroupSubscriptions: [{ _id: 'sub-123' }], }, }) diff --git a/services/web/test/frontend/helpers/editor-providers.tsx b/services/web/test/frontend/helpers/editor-providers.tsx index ceaa0b0f3c..c5e337f3b5 100644 --- a/services/web/test/frontend/helpers/editor-providers.tsx +++ b/services/web/test/frontend/helpers/editor-providers.tsx @@ -75,7 +75,7 @@ export type EditorProvidersProps = { | 'id' | 'email' | 'signUpDate' - | 'activeGroupSubscriptions' + | 'activeProfessionalGroupSubscriptions' | 'isProfessionalGroupPlan' > projectId?: string diff --git a/services/web/test/unit/src/Collaborators/CollaboratorsInviteController.test.mjs b/services/web/test/unit/src/Collaborators/CollaboratorsInviteController.test.mjs index b8d965b9d3..4a4a6a7052 100644 --- a/services/web/test/unit/src/Collaborators/CollaboratorsInviteController.test.mjs +++ b/services/web/test/unit/src/Collaborators/CollaboratorsInviteController.test.mjs @@ -149,6 +149,12 @@ describe('CollaboratorsInviteController', function () { }, } + ctx.SubscriptionLocator = { + promises: { + getUserActiveProfessionalGroupSubscriptions: sinon.stub().resolves([]), + }, + } + ctx.SplitTestHandler = { promises: { getAssignment: sinon.stub().resolves({ variant: 'default' }), @@ -257,6 +263,13 @@ describe('CollaboratorsInviteController', function () { }) ) + vi.doMock( + '../../../../app/src/Features/Subscription/SubscriptionLocator.mjs', + () => ({ + default: ctx.SubscriptionLocator, + }) + ) + ctx.CollaboratorsInviteController = (await import(MODULE_PATH)).default ctx.res = new MockResponse(vi) @@ -1916,6 +1929,53 @@ describe('CollaboratorsInviteController', function () { expect(ctx.invite.save).to.have.been.calledOnce expect(ctx.res.json).toHaveBeenCalledTimes(1) }) + + describe('with a subscriptionId', function () { + beforeEach(function (ctx) { + ctx.subscriptionId = new ObjectId() + ctx.req.body.subscriptionId = ctx.subscriptionId.toString() + }) + + it('creates the invite when the user is in a matching professional group subscription', async function (ctx) { + await new Promise(resolve => { + ctx.SubscriptionLocator.promises.getUserActiveProfessionalGroupSubscriptions.resolves( + [{ _id: ctx.subscriptionId }] + ) + ctx.CollaboratorsInviteGetter.promises.getSharingLinkInvite.resolves( + null + ) + ctx.CollaboratorsInviteHandler.promises.createSharingLinkInvite.resolves( + ctx.invite + ) + ctx.res.callback = () => resolve() + ctx.CollaboratorsInviteController.updateSharingLink(ctx.req, ctx.res) + }) + + ctx.CollaboratorsInviteHandler.promises.createSharingLinkInvite.should.have.been.calledWith( + ctx.projectId, + 'readOnly', + ctx.subscriptionId.toString() + ) + }) + + it('responds with a 403 JSON error when the subscription is not a professional group the user belongs to', async function (ctx) { + await new Promise(resolve => { + ctx.SubscriptionLocator.promises.getUserActiveProfessionalGroupSubscriptions.resolves( + [] + ) + ctx.res.callback = () => resolve() + ctx.CollaboratorsInviteController.updateSharingLink(ctx.req, ctx.res) + }) + + expect(ctx.res.statusCode).to.equal(403) + expect(ctx.res.json).toHaveBeenCalledWith({ + errorReason: 'subscription_not_eligible', + }) + ctx.CollaboratorsInviteHandler.promises.createSharingLinkInvite.called.should.equal( + false + ) + }) + }) }) describe('validateSharingLink', function () { diff --git a/services/web/test/unit/src/Project/ProjectController.test.mjs b/services/web/test/unit/src/Project/ProjectController.test.mjs index 2d44612d85..adbb2d68d0 100644 --- a/services/web/test/unit/src/Project/ProjectController.test.mjs +++ b/services/web/test/unit/src/Project/ProjectController.test.mjs @@ -71,7 +71,7 @@ describe('ProjectController', function () { ctx.SubscriptionLocator = { promises: { getUsersSubscription: sinon.stub().resolves(), - getUserActiveGroupSubscriptions: sinon.stub().resolves(), + getUserActiveProfessionalGroupSubscriptions: sinon.stub().resolves(), }, } ctx.SubscriptionController = { diff --git a/services/web/test/unit/src/Subscription/SubscriptionLocator.test.mjs b/services/web/test/unit/src/Subscription/SubscriptionLocator.test.mjs index 1a576025ae..be4ced9a0f 100644 --- a/services/web/test/unit/src/Subscription/SubscriptionLocator.test.mjs +++ b/services/web/test/unit/src/Subscription/SubscriptionLocator.test.mjs @@ -48,6 +48,12 @@ describe('Subscription Locator Tests', function () { SSOConfig: ctx.SSOConfig, })) + ctx.isProfessionalGroupPlan = sinon.stub() + vi.doMock('../../../../app/src/Features/Subscription/PlansHelper', () => ({ + isProfessionalGroupPlan: subscription => + ctx.isProfessionalGroupPlan(subscription), + })) + ctx.SubscriptionLocator = (await import(modulePath)).default }) @@ -193,4 +199,58 @@ describe('Subscription Locator Tests', function () { expect(subscriptionStatus).to.deep.equal({ personal: true, group: true }) }) }) + + describe('getUserActiveProfessionalGroupSubscriptions', function () { + beforeEach(function (ctx) { + ctx.standardSubscription = { + _id: 'standard-sub', + groupPlan: true, + planCode: 'group_standard', + } + ctx.professionalSubscription = { + _id: 'professional-sub', + groupPlan: true, + planCode: 'group_professional', + } + ctx.Subscription.find.returns({ + exec: sinon + .stub() + .resolves([ctx.standardSubscription, ctx.professionalSubscription]), + }) + ctx.isProfessionalGroupPlan.callsFake( + subscription => subscription === ctx.professionalSubscription + ) + }) + + it('returns only the professional group subscriptions', async function (ctx) { + const result = + await ctx.SubscriptionLocator.promises.getUserActiveProfessionalGroupSubscriptions( + ctx.user._id + ) + expect(result).to.deep.equal([ctx.professionalSubscription]) + }) + + it('always requests `planCode` and `groupPlan` so professional status can be determined', async function (ctx) { + await ctx.SubscriptionLocator.promises.getUserActiveProfessionalGroupSubscriptions( + ctx.user._id, + { _id: 1, teamName: 1 } + ) + const projection = ctx.Subscription.find.lastCall.args[1] + expect(projection).to.include({ + _id: 1, + teamName: 1, + planCode: 1, + groupPlan: 1, + }) + }) + + it('returns an empty list without querying when no userId is provided', async function (ctx) { + const result = + await ctx.SubscriptionLocator.promises.getUserActiveProfessionalGroupSubscriptions( + undefined + ) + expect(result).to.deep.equal([]) + expect(ctx.Subscription.find.called).to.equal(false) + }) + }) }) diff --git a/services/web/types/user.ts b/services/web/types/user.ts index fbeb968e2c..14e498d434 100644 --- a/services/web/types/user.ts +++ b/services/web/types/user.ts @@ -65,7 +65,7 @@ export type User = { isMemberOfGroupSubscription?: boolean isProfessionalGroupPlan?: boolean hasInstitutionLicence?: boolean - activeGroupSubscriptions?: { + activeProfessionalGroupSubscriptions?: { _id: string teamName?: string sharingPermissions?: SharingPermissions