From 2d022aff124adfee4a723361d26e2ccbda99db99 Mon Sep 17 00:00:00 2001 From: Antoine Clausse Date: Wed, 23 Jul 2025 14:36:37 +0200 Subject: [PATCH] [web] Replace token-link validation on reconfirm affiliation notifications (#27250) * Update Reconfirm Affiliation (Pug) in Portal page Change the notification so it directs users to the settings page instead of triggering an email confirmation with token-link * Update Reconfirm Affiliation (React) in Settings page Update the component so it sends the 6-digits code and opens the modal to enter it * Update Reconfirm Affiliation (React) in Projects page Update the component so it sends the 6-digits code and opens the modal to enter it * `cleanup_unused_locales` * `bin/run web npm run extract-translations` * Update tests * Minor updates in tests * Update Pug notifications with the notification mixin * De-center the "reconfirmed notification" * Update "Learn more" links to "Learn more about institutional email reconfirmation" * Update tests GitOrigin-RevId: cb65623e209217614786eec56f7f5d28b9e8cec5 --- .../web/app/views/_mixins/notification.pug | 1 + .../reconfirm_affiliation-marketing.pug | 75 +++----- .../web/frontend/extracted-translations.json | 4 +- .../affiliation/reconfirm-affiliation.tsx | 123 +++---------- .../components/emails/reconfirmation-info.tsx | 171 +++++------------- .../reconfirmation-info-prompt-text.tsx | 2 +- services/web/locales/en.json | 4 +- .../components/notifications.test.tsx | 16 +- .../emails/reconfirmation-info.test.tsx | 20 +- 9 files changed, 130 insertions(+), 286 deletions(-) diff --git a/services/web/app/views/_mixins/notification.pug b/services/web/app/views/_mixins/notification.pug index 482dd540c5..807e13ab60 100644 --- a/services/web/app/views/_mixins/notification.pug +++ b/services/web/app/views/_mixins/notification.pug @@ -24,6 +24,7 @@ mixin notification(options) p b #{title} | !{content} + block //- TODO: handle action //- if action //- .notification-cta diff --git a/services/web/app/views/_mixins/reconfirm_affiliation-marketing.pug b/services/web/app/views/_mixins/reconfirm_affiliation-marketing.pug index f54dd5d4ba..cc3fe756f2 100644 --- a/services/web/app/views/_mixins/reconfirm_affiliation-marketing.pug +++ b/services/web/app/views/_mixins/reconfirm_affiliation-marketing.pug @@ -1,55 +1,34 @@ +include ./notification + mixin reconfirmAffiliationNotification-marketing(userEmail, location) - form(data-ol-async-form action='/user/emails/send-reconfirmation') - input(name='_csrf' type='hidden' value=csrfToken) - input(name='email' type='hidden' value=userEmail.email) - +formMessages + - var ssoEnabled = userEmail.affiliation && userEmail.affiliation.institution && userEmail.affiliation.institution.ssoEnabled + if ssoEnabled + - var institutionId = userEmail.affiliation && userEmail.affiliation.institution && userEmail.affiliation.institution.id + a.btn-reconfirm.btn.btn-sm.btn-info( + data-ol-slow-link + href=`${settings.saml.ukamf.initPath}?university_id=${institutionId}&reconfirm=${location}` + ) + span(data-ol-inflight='idle') #{translate("confirm_affiliation")} + span(hidden data-ol-inflight='pending') #{translate("pending")}… - .reconfirm-notification - div(data-ol-not-sent style='width: 100%') - i.fa.fa-warning + else + a.btn-reconfirm.btn.btn-sm.btn-info(data-ol-slow-link href='/user/settings') + span(data-ol-inflight='idle') #{translate("go_to_account_settings")} + span(hidden data-ol-inflight='pending') #{translate("redirecting")}… - - var ssoEnabled = userEmail.affiliation && userEmail.affiliation.institution && userEmail.affiliation.institution.ssoEnabled - if ssoEnabled - - var institutionId = userEmail.affiliation && userEmail.affiliation.institution && userEmail.affiliation.institution.id - a.btn-reconfirm.btn.btn-sm.btn-info( - data-ol-slow-link - href=`${settings.saml.ukamf.initPath}?university_id=${institutionId}&reconfirm=${location}` - ) - span(data-ol-inflight='idle') #{translate("confirm_affiliation")} - span(hidden data-ol-inflight='pending') #{translate("pending")}… + | !{translate("are_you_still_at", { institutionName: userEmail.affiliation.institution.name }, ['strong'])}  - else - button.btn-reconfirm.btn.btn-sm.btn-info( - type='submit' - data-ol-disabled-inflight - ) - span(data-ol-inflight='idle') #{translate("confirm_affiliation")} - span(hidden data-ol-inflight='pending') #{translate("pending")}… + if location == '/user/settings' + | !{translate('please_reconfirm_institutional_email', {}, [{ name: 'span' }])} + if userEmail.default + span  #{translate('need_to_add_new_primary_before_remove')} + else + | !{translate("please_reconfirm_institutional_email_2")} - | !{translate("are_you_still_at", {institutionName: userEmail.affiliation.institution.name}, ['strong'])}  - - if location == '/user/settings' - | !{translate('please_reconfirm_institutional_email', {}, [{ name: 'span' }])} - if userEmail.default - span  #{translate('need_to_add_new_primary_before_remove')} - else - | !{translate("please_reconfirm_institutional_email", {}, [{name: 'a', attrs: {href: '/user/settings?remove=' + userEmail.email}}])} - - |   - a(href='/learn/how-to/Institutional_Email_Reconfirmation' target='_blank') #{translate("learn_more")} - - div(hidden data-ol-sent) - | !{translate("please_check_your_inbox_to_confirm", {institutionName: userEmail.affiliation.institution.name}, ['strong'])} - |   - button.btn-inline-link(type='submit' data-ol-disabled-inflight) - span(data-ol-inflight='idle') #{translate('resend_confirmation_email')} - span(hidden data-ol-inflight='pending') #{translate("pending")}… + |   + a(href='/learn/how-to/Institutional_Email_Reconfirmation' target='_blank') #{translate("learn_more_about_email_reconfirmation")} mixin reconfirmedAffiliationNotification-marketing(userEmail) - .alert.alert-info - .reconfirm-notification - div(style='width: 100%') - //- extra div for flex styling - | !{translate("your_affiliation_is_confirmed", {institutionName: userEmail.affiliation.institution.name}, ['strong'])} - | - | #{translate('thank_you_exclamation')} + | !{translate("your_affiliation_is_confirmed", {institutionName: userEmail.affiliation.institution.name}, ['strong'])} + | + | #{translate('thank_you_exclamation')} diff --git a/services/web/frontend/extracted-translations.json b/services/web/frontend/extracted-translations.json index e76317d33f..4f34862de9 100644 --- a/services/web/frontend/extracted-translations.json +++ b/services/web/frontend/extracted-translations.json @@ -930,6 +930,7 @@ "learn_more": "", "learn_more_about": "", "learn_more_about_account": "", + "learn_more_about_email_reconfirmation": "", "learn_more_about_link_sharing": "", "learn_more_about_managed_users": "", "learn_more_about_other_causes_of_compile_timeouts": "", @@ -1253,7 +1254,6 @@ "please_ask_the_project_owner_to_upgrade_more_collaborators": "", "please_ask_the_project_owner_to_upgrade_to_track_changes": "", "please_change_primary_to_remove": "", - "please_check_your_inbox_to_confirm": "", "please_compile_pdf_before_download": "", "please_compile_pdf_before_word_count": "", "please_confirm_primary_email_or_edit": "", @@ -1440,7 +1440,6 @@ "republish": "", "resend": "", "resend_confirmation_code": "", - "resend_confirmation_email": "", "resend_group_invite": "", "resend_link_sso": "", "resend_managed_user_invite": "", @@ -1561,7 +1560,6 @@ "send_first_message": "", "send_message": "", "send_request": "", - "sending": "", "server_error": "", "server_pro_license_entitlement_line_1": "", "server_pro_license_entitlement_line_2": "", diff --git a/services/web/frontend/js/features/project-list/components/notifications/groups/affiliation/reconfirm-affiliation.tsx b/services/web/frontend/js/features/project-list/components/notifications/groups/affiliation/reconfirm-affiliation.tsx index 56a27843be..1156e6ba1f 100644 --- a/services/web/frontend/js/features/project-list/components/notifications/groups/affiliation/reconfirm-affiliation.tsx +++ b/services/web/frontend/js/features/project-list/components/notifications/groups/affiliation/reconfirm-affiliation.tsx @@ -1,17 +1,12 @@ -import { useState, useEffect } from 'react' +import { useState } from 'react' import { useTranslation, Trans } from 'react-i18next' import getMeta from '../../../../../../utils/meta' -import useAsync from '../../../../../../shared/hooks/use-async' -import { - FetchError, - postJSON, -} from '../../../../../../infrastructure/fetch-json' import { UserEmailData } from '../../../../../../../../types/user-email' import { Institution } from '../../../../../../../../types/institution' import { useLocation } from '../../../../../../shared/hooks/use-location' -import { debugConsole } from '@/utils/debugging' import OLButton from '@/features/ui/components/ol/ol-button' import Notification from '@/features/project-list/components/notifications/notification' +import ResendConfirmationCodeModal from '@/features/settings/components/emails/resend-confirmation-code-modal' type ReconfirmAffiliationProps = { email: UserEmailData['email'] @@ -24,78 +19,13 @@ function ReconfirmAffiliation({ }: ReconfirmAffiliationProps) { const { t } = useTranslation() const { samlInitPath } = getMeta('ol-ExposedSettings') - const { error, isLoading, isError, isSuccess, runAsync } = useAsync() - const [hasSent, setHasSent] = useState(false) + const [isSuccess, setIsSuccess] = useState(false) + const [isLoading, setIsLoading] = useState(false) const [isPending, setIsPending] = useState(false) const ssoEnabled = institution.ssoEnabled const location = useLocation() - useEffect(() => { - if (isSuccess) { - setHasSent(true) - } - }, [isSuccess]) - - const handleRequestReconfirmation = () => { - if (ssoEnabled) { - setIsPending(true) - location.assign( - `${samlInitPath}?university_id=${institution.id}&reconfirm=/project` - ) - } else { - runAsync( - postJSON('/user/emails/send-reconfirmation', { - body: { email }, - }) - ).catch(debugConsole.error) - } - } - - const rateLimited = - error && error instanceof FetchError && error.response?.status === 429 - - if (hasSent) { - return ( - - ]} // eslint-disable-line react/jsx-key - values={{ institutionName: institution.name }} - shouldUnescape - tOptions={{ interpolation: { escapeValue: true } }} - /> -   - {isError && ( - <> -
-
- {rateLimited - ? t('too_many_requests') - : t('generic_something_went_wrong')} -
- - )} - - } - action={ - - {t('resend_confirmation_email')} - - } - /> - ) - } - + if (isSuccess) return null return ( - {t('learn_more')} + {t('learn_more_about_email_reconfirmation')} - {isError && ( - <> -
-
- {rateLimited - ? t('too_many_requests') - : t('generic_something_went_wrong')} -
- - )} } action={ - - {t('confirm_affiliation')} - + ssoEnabled ? ( + { + setIsPending(true) + location.assign( + `${samlInitPath}?university_id=${institution.id}&reconfirm=/project` + ) + }} + > + {t('confirm_affiliation')} + + ) : ( + setIsSuccess(true)} + triggerVariant="secondary" + /> + ) } /> ) diff --git a/services/web/frontend/js/features/settings/components/emails/reconfirmation-info.tsx b/services/web/frontend/js/features/settings/components/emails/reconfirmation-info.tsx index 08423946a6..6f469b696b 100644 --- a/services/web/frontend/js/features/settings/components/emails/reconfirmation-info.tsx +++ b/services/web/frontend/js/features/settings/components/emails/reconfirmation-info.tsx @@ -1,4 +1,4 @@ -import { useState, useEffect, useLayoutEffect } from 'react' +import { useState } from 'react' import { UserEmailData } from '../../../../../../types/user-email' import getMeta from '../../../../utils/meta' import ReconfirmationInfoSuccess from './reconfirmation-info/reconfirmation-info-success' @@ -7,14 +7,11 @@ import OLRow from '@/features/ui/components/ol/ol-row' import OLCol from '@/features/ui/components/ol/ol-col' import OLNotification from '@/features/ui/components/ol/ol-notification' import { useUserEmailsContext } from '@/features/settings/context/user-email-context' -import { FetchError, postJSON } from '@/infrastructure/fetch-json' -import { debugConsole } from '@/utils/debugging' import { ssoAvailableForInstitution } from '@/features/settings/utils/sso' -import { Trans, useTranslation } from 'react-i18next' -import useAsync from '@/shared/hooks/use-async' +import { useTranslation } from 'react-i18next' import { useLocation } from '@/shared/hooks/use-location' import OLButton from '@/features/ui/components/ol/ol-button' -import LoadingSpinner from '@/shared/components/loading-spinner' +import ResendConfirmationCodeModal from '@/features/settings/components/emails/resend-confirmation-code-modal' type ReconfirmationInfoProps = { userEmailData: UserEmailData @@ -22,50 +19,21 @@ type ReconfirmationInfoProps = { function ReconfirmationInfo({ userEmailData }: ReconfirmationInfoProps) { const reconfirmedViaSAML = getMeta('ol-reconfirmedViaSAML') - + const affiliation = userEmailData.affiliation const { t } = useTranslation() const { samlInitPath } = getMeta('ol-ExposedSettings') - const { error, isLoading, isError, isSuccess, runAsync } = useAsync() - const { state, setLoading: setUserEmailsContextLoading } = - useUserEmailsContext() - const [hasSent, setHasSent] = useState(false) + const { + state, + setLoading: setUserEmailsContextLoading, + getEmails, + } = useUserEmailsContext() const [isPending, setIsPending] = useState(false) const location = useLocation() const ssoAvailable = Boolean( - ssoAvailableForInstitution(userEmailData.affiliation?.institution ?? null) + ssoAvailableForInstitution(affiliation?.institution ?? null) ) - const handleRequestReconfirmation = () => { - if (userEmailData.affiliation?.institution && ssoAvailable) { - setIsPending(true) - location.assign( - `${samlInitPath}?university_id=${userEmailData.affiliation.institution.id}&reconfirm=/user/settings` - ) - } else { - runAsync( - postJSON('/user/emails/send-reconfirmation', { - body: { - email: userEmailData.email, - }, - }) - ).catch(debugConsole.error) - } - } - - useEffect(() => { - setUserEmailsContextLoading(isLoading) - }, [setUserEmailsContextLoading, isLoading]) - - useLayoutEffect(() => { - if (isSuccess) { - setHasSent(true) - } - }, [isSuccess]) - - const rateLimited = - isError && error instanceof FetchError && error.response?.status === 429 - - if (!userEmailData.affiliation) { + if (!affiliation) { return null } @@ -80,7 +48,7 @@ function ReconfirmationInfo({ userEmailData }: ReconfirmationInfoProps) { type="info" content={ } /> @@ -89,86 +57,45 @@ function ReconfirmationInfo({ userEmailData }: ReconfirmationInfoProps) { ) } - if (userEmailData.affiliation.inReconfirmNotificationPeriod) { - return ( - - - - {hasSent ? ( - ] - } - /> - ) : ( - - )} -
- {isError && ( -
- {rateLimited - ? t('too_many_requests') - : t('generic_something_went_wrong')} -
- )} - - } - action={ - hasSent ? ( - <> - {isLoading ? ( - <> - - - ) : ( - - {t('resend_confirmation_email')} - - )} - - ) : ( - - {isLoading ? ( - <> - - - ) : ( - t('confirm_affiliation') - )} - - ) - } - /> - - - ) + if (!affiliation.inReconfirmNotificationPeriod) { + return null } - return null + return ( + + } + action={ + affiliation?.institution && ssoAvailable ? ( + { + setIsPending(true) + location.assign( + `${samlInitPath}?university_id=${affiliation.institution.id}&reconfirm=/user/settings` + ) + }} + > + {t('confirm_affiliation')} + + ) : ( + + ) + } + /> + ) } export default ReconfirmationInfo diff --git a/services/web/frontend/js/features/settings/components/emails/reconfirmation-info/reconfirmation-info-prompt-text.tsx b/services/web/frontend/js/features/settings/components/emails/reconfirmation-info/reconfirmation-info-prompt-text.tsx index 898649504d..20a9705e60 100644 --- a/services/web/frontend/js/features/settings/components/emails/reconfirmation-info/reconfirmation-info-prompt-text.tsx +++ b/services/web/frontend/js/features/settings/components/emails/reconfirmation-info/reconfirmation-info-prompt-text.tsx @@ -37,7 +37,7 @@ function ReconfirmationInfoPromptText({ href="/learn/how-to/Institutional_Email_Reconfirmation" target="_blank" > - {t('learn_more')} + {t('learn_more_about_email_reconfirmation')}
{primary ? {t('need_to_add_new_primary_before_remove')} : null} diff --git a/services/web/locales/en.json b/services/web/locales/en.json index 823b19f70f..2a7cce6cee 100644 --- a/services/web/locales/en.json +++ b/services/web/locales/en.json @@ -1207,6 +1207,7 @@ "learn_more": "Learn more", "learn_more_about": "Learn more about __appName__", "learn_more_about_account": "<0>Learn more about managing your __appName__ account.", + "learn_more_about_email_reconfirmation": "Learn more about institutional email reconfirmation.", "learn_more_about_emails": "<0>Learn more about managing your __appName__ emails.", "learn_more_about_link_sharing": "Learn more about Link Sharing", "learn_more_about_managed_users": "Learn more about Managed Users.", @@ -1648,7 +1649,6 @@ "please_ask_the_project_owner_to_upgrade_more_collaborators": "Please ask the project owner to upgrade their plan to allow more collaborators.", "please_ask_the_project_owner_to_upgrade_to_track_changes": "Please ask the project owner to upgrade to use track changes", "please_change_primary_to_remove": "Please change your primary email in order to remove", - "please_check_your_inbox_to_confirm": "Please check your email inbox to confirm your <0>__institutionName__ affiliation.", "please_compile_pdf_before_download": "Please compile your project before downloading the PDF", "please_compile_pdf_before_word_count": "Please compile your project before performing a word count", "please_confirm_email": "Please confirm your email __emailAddress__ by clicking on the link in the confirmation email ", @@ -1665,6 +1665,7 @@ "please_provide_a_subject": "Please provide a subject", "please_provide_a_valid_email_address": "Please provide a valid email address", "please_reconfirm_institutional_email": "Please take a moment to confirm your institutional email address or <0>remove it from your account.", + "please_reconfirm_institutional_email_2": "Please take a moment to confirm your institutional email address or remove it from your account.", "please_reconfirm_your_affiliation_before_making_this_primary": "Please confirm your affiliation before making this the primary.", "please_refresh": "Please refresh the page to continue.", "please_request_a_new_password_reset_email_and_follow_the_link": "Please request a new password reset email and follow the link", @@ -1879,7 +1880,6 @@ "required": "Required", "resend": "Resend", "resend_confirmation_code": "Resend confirmation code", - "resend_confirmation_email": "Resend confirmation email", "resend_group_invite": "Resend group invite", "resend_link_sso": "Resend SSO invite", "resend_managed_user_invite": "Resend managed user invite", diff --git a/services/web/test/frontend/features/project-list/components/notifications.test.tsx b/services/web/test/frontend/features/project-list/components/notifications.test.tsx index 8813566a2e..9d1564fefd 100644 --- a/services/web/test/frontend/features/project-list/components/notifications.test.tsx +++ b/services/web/test/frontend/features/project-list/components/notifications.test.tsx @@ -842,24 +842,22 @@ describe('', function () { ) const sendReconfirmationMock = fetchMock.post( - '/user/emails/send-reconfirmation', + '/user/emails/send-confirmation-code', 200 ) fireEvent.click( - screen.getByRole('button', { name: /confirm affiliation/i }) + screen.getByRole('button', { name: 'Send confirmation code' }) ) await waitForElementToBeRemoved(() => screen.getByText(/loading/i)) - screen.getByText(/check your email inbox to confirm/i) - expect(screen.queryByRole('button', { name: /confirm affiliation/i })).to - .be.null - expect(screen.queryByRole('link', { name: /remove it/i })).to.be.null - expect(screen.queryByRole('link', { name: /learn more/i })).to.be.null + screen.getByText(/Enter the 6-digit code sent to foo@overleaf.com/i) expect(sendReconfirmationMock.callHistory.called()).to.be.true fireEvent.click( - screen.getByRole('button', { name: /resend confirmation email/i }) + screen.getByRole('button', { name: /resend confirmation code/i }) + ) + await waitForElementToBeRemoved(() => + screen.getByText('Resending confirmation code') ) - await waitForElementToBeRemoved(() => screen.getByText('Sending…')) expect(sendReconfirmationMock.callHistory.calls()).to.have.lengthOf(2) }) diff --git a/services/web/test/frontend/features/settings/components/emails/reconfirmation-info.test.tsx b/services/web/test/frontend/features/settings/components/emails/reconfirmation-info.test.tsx index 801b864cd4..716c36536c 100644 --- a/services/web/test/frontend/features/settings/components/emails/reconfirmation-info.test.tsx +++ b/services/web/test/frontend/features/settings/components/emails/reconfirmation-info.test.tsx @@ -76,7 +76,9 @@ describe('', function () { screen.getByText( /Please take a moment to confirm your institutional email address/ ) - screen.getByRole('link', { name: 'Learn more' }) + screen.getByRole('link', { + name: 'Learn more about institutional email reconfirmation.', + }) expect(screen.queryByText(/add a new primary email address/)).to.not.exist }) @@ -121,13 +123,13 @@ describe('', function () { Object.assign(getMeta('ol-ExposedSettings'), { hasSamlFeature: false, }) - fetchMock.post('/user/emails/send-reconfirmation', 200) + fetchMock.post('/user/emails/send-confirmation-code', 200) }) it('sends and resends confirmation email', async function () { renderReconfirmationInfo(inReconfirmUserData) const confirmButton = (await screen.findByRole('button', { - name: 'Confirm affiliation', + name: 'Send confirmation code', })) as HTMLButtonElement await waitFor(() => { @@ -141,12 +143,14 @@ describe('', function () { expect(fetchMock.callHistory.called()).to.be.true // the confirmation text should now be displayed - await screen.findByText(/Please check your email inbox to confirm/) + await screen.findByLabelText( + /Enter the 6-digit code sent to sso-prof@sso-university\.edu/ + ) // try the resend button fetchMock.clearHistory() const resendButton = await screen.findByRole('button', { - name: 'Resend confirmation email', + name: /Resend confirmation code/, }) fireEvent.click(resendButton) @@ -154,9 +158,11 @@ describe('', function () { // commented out as it's already gone by this point // await screen.findByText(/Sending/) expect(fetchMock.callHistory.called()).to.be.true - await waitForElementToBeRemoved(() => screen.getByText('Sending…')) + await waitForElementToBeRemoved(() => + screen.getByText('Resending confirmation code') + ) await screen.findByRole('button', { - name: 'Resend confirmation email', + name: 'Resend confirmation code', }) }) })