From 51223315e41644a94832ee8154996e9f25f87927 Mon Sep 17 00:00:00 2001 From: Miguel Serrano Date: Fri, 26 May 2023 12:56:15 +0200 Subject: [PATCH] Merge pull request #13164 from overleaf/msm-email-limit [web] limit user email addresses to 10 GitOrigin-RevId: 038214cc921d86a407391e6c82fa9cd16a7f9646 --- .../src/Features/User/UserEmailsController.js | 10 ++++++- .../src/Features/User/UserPagesController.js | 1 + services/web/app/views/user/settings.pug | 1 + services/web/config/settings.defaults.js | 2 ++ .../web/frontend/extracted-translations.json | 1 + .../settings/components/emails/add-email.tsx | 21 +++++++++++-- .../settings/context/user-email-context.tsx | 5 ++++ .../stories/settings/emails.stories.js | 8 +++++ .../stories/settings/helpers/emails.js | 12 ++++++++ services/web/locales/en.json | 1 + .../emails-section-add-new-email.test.tsx | 18 +++++++++++ .../web/test/frontend/helpers/with-markup.ts | 30 +++++++++++++++++++ .../src/User/UserEmailsControllerTests.js | 28 ++++++++++++++++- 13 files changed, 133 insertions(+), 5 deletions(-) create mode 100644 services/web/test/frontend/helpers/with-markup.ts diff --git a/services/web/app/src/Features/User/UserEmailsController.js b/services/web/app/src/Features/User/UserEmailsController.js index f1c2acd2b2..9b4d5454e1 100644 --- a/services/web/app/src/Features/User/UserEmailsController.js +++ b/services/web/app/src/Features/User/UserEmailsController.js @@ -1,3 +1,4 @@ +const Settings = require('@overleaf/settings') const logger = require('@overleaf/logger') const SessionManager = require('../Authentication/SessionManager') const UserGetter = require('./UserGetter') @@ -32,7 +33,14 @@ async function add(req, res, next) { if (!email) { return res.sendStatus(422) } - const user = await UserGetter.promises.getUser(userId, { email: 1 }) + const user = await UserGetter.promises.getUser(userId, { + email: 1, + 'emails.email': 1, + }) + + if (user.emails.length >= Settings.emailAddressLimit) { + return res.status(422).json({ message: 'secondary email limit exceeded' }) + } const affiliationOptions = { university: req.body.university, diff --git a/services/web/app/src/Features/User/UserPagesController.js b/services/web/app/src/Features/User/UserPagesController.js index 059b2c9a0c..db91812cb4 100644 --- a/services/web/app/src/Features/User/UserPagesController.js +++ b/services/web/app/src/Features/User/UserPagesController.js @@ -125,6 +125,7 @@ async function settingsPage(req, res) { projectSyncSuccessMessage, showPersonalAccessToken, personalAccessTokens, + emailAddressLimit: Settings.emailAddressLimit, }) } diff --git a/services/web/app/views/user/settings.pug b/services/web/app/views/user/settings.pug index caf2b69fc6..a6c265c05e 100644 --- a/services/web/app/views/user/settings.pug +++ b/services/web/app/views/user/settings.pug @@ -24,6 +24,7 @@ block append meta meta(name="ol-projectSyncSuccessMessage", content=projectSyncSuccessMessage) meta(name="ol-showPersonalAccessToken", data-type="boolean" content=showPersonalAccessToken) meta(name="ol-personalAccessTokens", data-type="json" content=personalAccessTokens) + meta(name="ol-emailAddressLimit", data-type="json", content=emailAddressLimit) block content main.content.content-alt#settings-page-root diff --git a/services/web/config/settings.defaults.js b/services/web/config/settings.defaults.js index c5617b8868..272cbcc35e 100644 --- a/services/web/config/settings.defaults.js +++ b/services/web/config/settings.defaults.js @@ -684,6 +684,8 @@ module.exports = { emailConfirmationDisabled: process.env.EMAIL_CONFIRMATION_DISABLED === 'true' || false, + emailAddressLimit: intFromEnv('EMAIL_ADDRESS_LIMIT', 10), + enabledServices: (process.env.ENABLED_SERVICES || 'web,api') .split(',') .map(s => s.trim()), diff --git a/services/web/frontend/extracted-translations.json b/services/web/frontend/extracted-translations.json index 46dd78d365..3e41611f5e 100644 --- a/services/web/frontend/extracted-translations.json +++ b/services/web/frontend/extracted-translations.json @@ -257,6 +257,7 @@ "educational_discount_for_groups_of_x_or_more": "", "educational_percent_discount_applied": "", "email": "", + "email_limit_reached": "", "email_or_password_wrong_try_again": "", "emails_and_affiliations_explanation": "", "emails_and_affiliations_title": "", diff --git a/services/web/frontend/js/features/settings/components/emails/add-email.tsx b/services/web/frontend/js/features/settings/components/emails/add-email.tsx index 6cccca4dea..298582e7e4 100644 --- a/services/web/frontend/js/features/settings/components/emails/add-email.tsx +++ b/services/web/frontend/js/features/settings/components/emails/add-email.tsx @@ -1,5 +1,5 @@ import { useState, useEffect } from 'react' -import { useTranslation } from 'react-i18next' +import { useTranslation, Trans } from 'react-i18next' import { Col } from 'react-bootstrap' import Cell from './cell' import Layout from './add-email/layout' @@ -15,6 +15,7 @@ import { postJSON } from '../../../../infrastructure/fetch-json' import { University } from '../../../../../../types/university' import { CountryCode } from '../../data/countries-list' import { isValidEmail } from '../../../../shared/utils/email' +import getMeta from '../../../../utils/meta' function AddEmail() { const { t } = useTranslation() @@ -38,6 +39,8 @@ function AddEmail() { getEmails, } = useUserEmailsContext() + const emailAddressLimit = getMeta('ol-emailAddressLimit', 10) + useEffect(() => { setUserEmailsContextLoading(isLoading) }, [setUserEmailsContextLoading, isLoading]) @@ -98,9 +101,21 @@ function AddEmail() { if (!isFormVisible) { return ( - + - + {state.data.emailCount >= emailAddressLimit ? ( + + ]} // eslint-disable-line react/jsx-key + /> + + ) : ( + + )} diff --git a/services/web/frontend/js/features/settings/context/user-email-context.tsx b/services/web/frontend/js/features/settings/context/user-email-context.tsx index f34f8aeefa..6d470d271e 100644 --- a/services/web/frontend/js/features/settings/context/user-email-context.tsx +++ b/services/web/frontend/js/features/settings/context/user-email-context.tsx @@ -65,6 +65,7 @@ export type State = { isLoading: boolean data: { byId: NormalizedObject + emailCount: number linkedInstitutionIds: NonNullable[] emailAffiliationBeingEdited: Nullable } @@ -82,6 +83,7 @@ const setData = (state: State, action: ActionSetData) => { const normalized = normalize(action.payload, { idAttribute: 'email', }) + const emailCount = action.payload.length const byId = normalized || {} const linkedInstitutionIds = action.payload .filter(email => Boolean(email.samlProviderId)) @@ -94,6 +96,7 @@ const setData = (state: State, action: ActionSetData) => { data: { ...initialState.data, byId, + emailCount, linkedInstitutionIds, }, } @@ -132,6 +135,7 @@ const deleteEmailAction = (state: State, action: ActionDeleteEmail) => { ...state, data: { ...state.data, + emailCount: state.data.emailCount - 1, byId, }, } @@ -191,6 +195,7 @@ const initialState: State = { isLoading: false, data: { byId: {}, + emailCount: 0, linkedInstitutionIds: [], emailAffiliationBeingEdited: null, }, diff --git a/services/web/frontend/stories/settings/emails.stories.js b/services/web/frontend/stories/settings/emails.stories.js index 9621ad6349..bccc1f0014 100644 --- a/services/web/frontend/stories/settings/emails.stories.js +++ b/services/web/frontend/stories/settings/emails.stories.js @@ -6,6 +6,7 @@ import { defaultSetupMocks, reconfirmationSetupMocks, errorsMocks, + emailLimitSetupMocks, } from './helpers/emails' export const EmailsList = args => { @@ -15,6 +16,13 @@ export const EmailsList = args => { return } +export const EmailLimitList = args => { + useFetchMock(emailLimitSetupMocks) + setDefaultMeta() + + return +} + export const ReconfirmationEmailsList = args => { useFetchMock(reconfirmationSetupMocks) setReconfirmationMeta() diff --git a/services/web/frontend/stories/settings/helpers/emails.js b/services/web/frontend/stories/settings/helpers/emails.js index 2356f9d66b..af165e671d 100644 --- a/services/web/frontend/stories/settings/helpers/emails.js +++ b/services/web/frontend/stories/settings/helpers/emails.js @@ -176,6 +176,18 @@ export function reconfirmationSetupMocks(fetchMock) { }) } +export function emailLimitSetupMocks(fetchMock) { + const userData = [] + for (let i = 0; i < 10; i++) { + userData.push({ email: `example${i}@overleaf.com` }) + } + defaultSetupMocks(fetchMock) + fetchMock.get(/\/user\/emails/, userData, { + delay: MOCK_DELAY, + overwriteRoutes: true, + }) +} + export function errorsMocks(fetchMock) { fetchMock .get(/\/user\/emails/, fakeUsersData, { delay: MOCK_DELAY }) diff --git a/services/web/locales/en.json b/services/web/locales/en.json index 98dd9b4a59..53c008664b 100644 --- a/services/web/locales/en.json +++ b/services/web/locales/en.json @@ -430,6 +430,7 @@ "email_already_registered_secondary": "This email is already registered as a secondary email", "email_already_registered_sso": "This email is already registered. Please log in to your account another way and link your account to the new provider via your account settings.", "email_does_not_belong_to_university": "We don’t recognize that domain as being affiliated with your university. Please contact us to add the affiliation.", + "email_limit_reached": "You can have a maximum of <0>__emailAddressLimit__ email addresses on this account. To add another email address, please delete an existing one.", "email_link_expired": "Email link expired, please request a new one.", "email_or_password_wrong_try_again": "Your email or password is incorrect. Please try again.", "email_or_password_wrong_try_again_or_reset": "Your email or password is incorrect. Please try again, or <0>set or reset your password.", diff --git a/services/web/test/frontend/features/settings/components/emails/emails-section-add-new-email.test.tsx b/services/web/test/frontend/features/settings/components/emails/emails-section-add-new-email.test.tsx index 6e5390ad36..337de5e4ff 100644 --- a/services/web/test/frontend/features/settings/components/emails/emails-section-add-new-email.test.tsx +++ b/services/web/test/frontend/features/settings/components/emails/emails-section-add-new-email.test.tsx @@ -10,6 +10,7 @@ import { expect } from 'chai' import fetchMock from 'fetch-mock' import { UserEmailData } from '../../../../../../types/user-email' import { Affiliation } from '../../../../../../types/affiliation' +import withMarkup from '../../../../helpers/with-markup' const userEmailData: UserEmailData & { affiliation: Affiliation } = { affiliation: { @@ -149,6 +150,23 @@ describe('', function () { screen.getByRole('button', { name: /add new email/i }) }) + it('prevent users from adding new emails when the limit is reached', async function () { + const emails = [] + for (let i = 0; i < 10; i++) { + emails.push({ email: `bar${i}@overleaf.com` }) + } + fetchMock.get('/user/emails?ensureAffiliation=true', emails) + render() + + const findByTextWithMarkup = withMarkup(screen.findByText) + await findByTextWithMarkup( + 'You can have a maximum of 10 email addresses on this account. To add another email address, please delete an existing one.' + ) + + expect(screen.queryByRole('button', { name: /add another email/i })).to.not + .exist + }) + it('adds new email address', async function () { fetchMock.get('/user/emails?ensureAffiliation=true', []) render() diff --git a/services/web/test/frontend/helpers/with-markup.ts b/services/web/test/frontend/helpers/with-markup.ts new file mode 100644 index 0000000000..e92aa60364 --- /dev/null +++ b/services/web/test/frontend/helpers/with-markup.ts @@ -0,0 +1,30 @@ +import { MatcherFunction } from '@testing-library/react' + +type Query = (f: MatcherFunction) => Element | Promise + +/* + Utility function to run testing-library queries over nodes that contain html tags, as in + `

this includes some bold text

`. + + Usage: + const getByTextWithMarkup = withMarkup(screen.getByText) + getByTextWithMarkup('this includes some bold text') + + const findByTextWithMarkup = withMarkup(screen.findByText) + await findByTextWithMarkup('this includes some bold text') + */ +const withMarkup = + (query: Query) => + (text: string): Element | Promise => + query((content: string, node: Element | null) => { + if (!node) { + return false + } + const hasText = (node: Element) => node.textContent === text + const childrenDontHaveText = Array.from(node.children).every( + child => !hasText(child as HTMLElement) + ) + return hasText(node) && childrenDontHaveText + }) + +export default withMarkup diff --git a/services/web/test/unit/src/User/UserEmailsControllerTests.js b/services/web/test/unit/src/User/UserEmailsControllerTests.js index 2542a65b6b..fad68cd277 100644 --- a/services/web/test/unit/src/User/UserEmailsControllerTests.js +++ b/services/web/test/unit/src/User/UserEmailsControllerTests.js @@ -14,7 +14,11 @@ describe('UserEmailsController', function () { this.req.sessionID = Math.random().toString() this.res = new MockResponse() this.next = sinon.stub() - this.user = { _id: 'mock-user-id', email: 'example@overleaf.com' } + this.user = { + _id: 'mock-user-id', + email: 'example@overleaf.com', + emails: {}, + } this.UserGetter = { getUserFullEmails: sinon.stub(), @@ -234,6 +238,28 @@ describe('UserEmailsController', function () { }) this.UserEmailsController.add(this.req, this.res, this.next) }) + + it('should fail to add new emails when the limit has been reached', function (done) { + this.user.emails = [] + for (let i = 0; i < 10; i++) { + this.user.emails.push({ email: `example${i}@overleaf.com` }) + } + this.UserEmailsController.add( + this.req, + { + status: code => { + expect(code).to.equal(422) + return { + json: error => { + expect(error.message).to.equal('secondary email limit exceeded') + done() + }, + } + }, + }, + this.next + ) + }) }) describe('remove', function () {