From 260fdf13071ecf00819069292e084093458a008a Mon Sep 17 00:00:00 2001 From: Thomas Date: Wed, 26 Jun 2024 12:16:07 +0200 Subject: [PATCH] Add read write token join interstitial variation for link sharing changes (#19060) * Add read write join interstitial variation for link sharing changes GitOrigin-RevId: 41661f43f4ab0f18f6ada5bec0b6af2407f65f07 --- .../TokenAccess/TokenAccessController.js | 85 ++++++++++++---- .../app/views/project/token/access-react.pug | 3 +- .../web/frontend/extracted-translations.json | 4 + .../components/require-accept-screen.tsx | 55 +++++++++++ .../components/token-access-root.tsx | 17 ++++ .../web/frontend/stylesheets/app/invite.less | 10 ++ services/web/locales/en.json | 4 + .../TokenAccess/TokenAccessControllerTests.js | 96 +++++++++++++++++++ 8 files changed, 254 insertions(+), 20 deletions(-) diff --git a/services/web/app/src/Features/TokenAccess/TokenAccessController.js b/services/web/app/src/Features/TokenAccess/TokenAccessController.js index 8174ca797e..3735c4e6b4 100644 --- a/services/web/app/src/Features/TokenAccess/TokenAccessController.js +++ b/services/web/app/src/Features/TokenAccess/TokenAccessController.js @@ -12,6 +12,9 @@ const { handleAdminDomainRedirect, } = require('../Authorization/AuthorizationMiddleware') const ProjectAuditLogHandler = require('../Project/ProjectAuditLogHandler') +const SplitTestHandler = require('../SplitTests/SplitTestHandler') +const CollaboratorsHandler = require('../Collaborators/CollaboratorsHandler') +const EditorRealTimeController = require('../Editor/EditorRealTimeController') const orderedPrivilegeLevels = [ PrivilegeLevels.NONE, @@ -271,33 +274,77 @@ async function grantTokenAccessReadAndWrite(req, res, next) { return next(new Errors.NotFoundError()) } - if (!confirmedByUser) { - return res.json({ - requireAccept: { - projectName: project.name, - }, - }) - } + const linkSharingChanges = + await SplitTestHandler.promises.getAssignmentForUser( + project.owner_ref, + 'link-sharing-warning' + ) + + if (linkSharingChanges?.variant === 'active') { + if (!confirmedByUser) { + return res.json({ + requireAccept: { + linkSharingChanges: true, + projectName: project.name, + }, + }) + } - if (!project.tokenAccessReadAndWrite_refs.some(id => id.equals(userId))) { await ProjectAuditLogHandler.promises.addEntry( project._id, - 'join-via-token', + 'accept-via-link-sharing', userId, req.ip, { privileges: 'readAndWrite' } ) + // Currently does not enforce the collaborator limit (warning phase) + await CollaboratorsHandler.promises.addUserIdToProject( + project._id, + undefined, + userId, + PrivilegeLevels.READ_AND_WRITE + ) + // Does not remove any pending invite or the invite notification + // Should be a noop if the user is already a member, + // and would redirect transparently into the project. + EditorRealTimeController.emitToRoom( + project._id, + 'project:membership:changed', + { members: true } + ) + + return res.json({ + redirect: `/project/${project._id}`, + }) + } else { + if (!confirmedByUser) { + return res.json({ + requireAccept: { + projectName: project.name, + }, + }) + } + + if (!project.tokenAccessReadAndWrite_refs.some(id => id.equals(userId))) { + await ProjectAuditLogHandler.promises.addEntry( + project._id, + 'join-via-token', + userId, + req.ip, + { privileges: 'readAndWrite' } + ) + } + + await TokenAccessHandler.promises.addReadAndWriteUserToProject( + userId, + project._id + ) + + return res.json({ + redirect: `/project/${project._id}`, + tokenAccessGranted: tokenType, + }) } - - await TokenAccessHandler.promises.addReadAndWriteUserToProject( - userId, - project._id - ) - - return res.json({ - redirect: `/project/${project._id}`, - tokenAccessGranted: tokenType, - }) } catch (err) { return next( OError.tag( diff --git a/services/web/app/views/project/token/access-react.pug b/services/web/app/views/project/token/access-react.pug index 157d806f60..83e9f79b61 100644 --- a/services/web/app/views/project/token/access-react.pug +++ b/services/web/app/views/project/token/access-react.pug @@ -13,4 +13,5 @@ block append meta meta(name="ol-user" data-type="json" content=user) block content - div#token-access-page + .content.content-alt#main-content + div#token-access-page diff --git a/services/web/frontend/extracted-translations.json b/services/web/frontend/extracted-translations.json index 85cdf0aace..1ef95c39e7 100644 --- a/services/web/frontend/extracted-translations.json +++ b/services/web/frontend/extracted-translations.json @@ -99,6 +99,7 @@ "are_you_getting_an_undefined_control_sequence_error": "", "are_you_still_at": "", "are_you_sure": "", + "as_email": "", "ask_proj_owner_to_unlink_from_current_github": "", "ask_proj_owner_to_upgrade_for_full_history": "", "ask_proj_owner_to_upgrade_for_references_search": "", @@ -873,6 +874,7 @@ "off": "", "official": "", "ok": "", + "ok_join_project": "", "on": "", "on_free_plan_upgrade_to_access_features": "", "one_step_away_from_professional_features": "", @@ -1667,6 +1669,7 @@ "your_git_access_info_bullet_5": "", "your_git_access_tokens": "", "your_message_to_collaborators": "", + "your_name_and_email_address_will_be_visible_to_the_project_owner_and_other_editors": "", "your_new_plan": "", "your_password_was_detected": "", "your_plan": "", @@ -1683,6 +1686,7 @@ "youre_about_to_enable_single_sign_on": "", "youre_about_to_enable_single_sign_on_sso_only": "", "youre_already_setup_for_sso": "", + "youre_joining": "", "youre_on_free_trial_which_ends_on": "", "youre_signed_in_as_logout": "", "youve_unlinked_all_users": "", diff --git a/services/web/frontend/js/features/token-access/components/require-accept-screen.tsx b/services/web/frontend/js/features/token-access/components/require-accept-screen.tsx index 1bcdcdcc4d..005de55c77 100644 --- a/services/web/frontend/js/features/token-access/components/require-accept-screen.tsx +++ b/services/web/frontend/js/features/token-access/components/require-accept-screen.tsx @@ -4,6 +4,7 @@ import getMeta from '@/utils/meta' export type RequireAcceptData = { projectName?: string + linkSharingChanges: boolean } export const RequireAcceptScreen: FC<{ @@ -13,6 +14,60 @@ export const RequireAcceptScreen: FC<{ const { t } = useTranslation() const user = getMeta('ol-user') + if (requireAcceptData.linkSharingChanges) { + return ( +
+
+
+
+
+
+

+ {t('youre_joining')} +
+ + + {requireAcceptData.projectName || 'This project'} + + + {user && ( + <> +
+ {t('as_email', { email: user.email })} + + )} +

+
+
+ +
+
+

+ {t( + 'your_name_and_email_address_will_be_visible_to_the_project_owner_and_other_editors' + )} +

+
+
+ +
+
+ +
+
+
+
+
+
+ ) + } + return (
diff --git a/services/web/frontend/js/features/token-access/components/token-access-root.tsx b/services/web/frontend/js/features/token-access/components/token-access-root.tsx index faef02760f..6cef2f3be2 100644 --- a/services/web/frontend/js/features/token-access/components/token-access-root.tsx +++ b/services/web/frontend/js/features/token-access/components/token-access-root.tsx @@ -87,6 +87,23 @@ function TokenAccessRoot() { return null } + // We don't want the full-size div and back link(?) on + // the new page, but we do this so the original page + // doesn't change. When tearing down we can clean up + // the DOM in the main return + if ( + mode === 'requireAccept' && + requireAcceptData && + requireAcceptData.linkSharingChanges + ) { + return ( + + ) + } + return (
diff --git a/services/web/frontend/stylesheets/app/invite.less b/services/web/frontend/stylesheets/app/invite.less index a628fe21fa..d7f4af2cd7 100644 --- a/services/web/frontend/stylesheets/app/invite.less +++ b/services/web/frontend/stylesheets/app/invite.less @@ -17,3 +17,13 @@ } margin-bottom: 30px; } + +.link-sharing-invite { + font-family: 'Noto Sans', sans-serif; +} + +.link-sharing-invite-header { + // heading-lg + font-size: var(--font-size-07); + line-height: var(--line-height-06); +} diff --git a/services/web/locales/en.json b/services/web/locales/en.json index 1fe4f43c83..d283bafb13 100644 --- a/services/web/locales/en.json +++ b/services/web/locales/en.json @@ -143,6 +143,7 @@ "article": "Article", "articles": "Articles", "as_a_member_of_sso_required": "As a member of __institutionName__, you must log in to __appName__ through your institution.", + "as_email": "as __email__", "ascending": "Ascending", "ask_proj_owner_to_unlink_from_current_github": "Ask the owner of the project (<0>__projectOwnerEmail__) to unlink the project from the current GitHub repository and create a connection to a different repository.", "ask_proj_owner_to_upgrade_for_full_history": "Please ask the project owner to upgrade to access this project’s full history.", @@ -1283,6 +1284,7 @@ "off": "Off", "official": "Official", "ok": "OK", + "ok_join_project": "OK, join project", "on": "On", "on_free_plan_upgrade_to_access_features": "You are on the __appName__ Free plan. Upgrade to access these <0>Premium Features", "one_collaborator": "Only one collaborator", @@ -2328,6 +2330,7 @@ "your_git_access_info_bullet_5": "Previously generated tokens will be shown here.", "your_git_access_tokens": "Your Git authentication tokens", "your_message_to_collaborators": "Send a message to your collaborators", + "your_name_and_email_address_will_be_visible_to_the_project_owner_and_other_editors": "Your name and email address will be visible to the project owner and other editors.", "your_new_plan": "Your new plan", "your_password_has_been_successfully_changed": "Your password has been successfully changed", "your_password_was_detected": "Your password is on a <0>public list of known compromised passwords. Keep your account safe by changing your password now.", @@ -2347,6 +2350,7 @@ "youre_about_to_enable_single_sign_on": "You’re about to enable single sign-on (SSO). Before you do this, you should ensure you’re confident the SSO configuration is correct and all your group members have managed user accounts.", "youre_about_to_enable_single_sign_on_sso_only": "You’re about to enable single sign-on (SSO). Before you do this, you should ensure you’re confident the SSO configuration is correct.", "youre_already_setup_for_sso": "You’re already set up for SSO", + "youre_joining": "You’re joining", "youre_on_free_trial_which_ends_on": "You’re on a free trial which ends on <0>__date__.", "youre_signed_in_as_logout": "You’re signed in as <0>__email__. <1>Log out.", "youre_signed_up": "You’re signed up", diff --git a/services/web/test/unit/src/TokenAccess/TokenAccessControllerTests.js b/services/web/test/unit/src/TokenAccess/TokenAccessControllerTests.js index b731142949..176cfab903 100644 --- a/services/web/test/unit/src/TokenAccess/TokenAccessControllerTests.js +++ b/services/web/test/unit/src/TokenAccess/TokenAccessControllerTests.js @@ -15,6 +15,7 @@ describe('TokenAccessController', function () { this.user = { _id: new ObjectId() } this.project = { _id: new ObjectId(), + name: 'test', tokenAccessReadAndWrite_refs: [], tokenAccessReadOnly_refs: [], } @@ -69,9 +70,18 @@ describe('TokenAccessController', function () { this.SplitTestHandler = { promises: { getAssignment: sinon.stub().resolves({ variant: 'default' }), + getAssignmentForUser: sinon.stub().resolves({ variant: 'default' }), }, } + this.CollaboratorsHandler = { + promises: { + addUserIdToProject: sinon.stub().resolves(), + }, + } + + this.EditorRealTimeController = { emitToRoom: sinon.stub() } + this.TokenAccessController = SandboxedModule.require(MODULE_PATH, { requires: { '@overleaf/settings': this.Settings, @@ -85,6 +95,8 @@ describe('TokenAccessController', function () { '../Project/ProjectAuditLogHandler': this.ProjectAuditLogHandler, '../SplitTests/SplitTestHandler': this.SplitTestHandler, '../Errors/Errors': (this.Errors = { NotFoundError: sinon.stub() }), + '../Collaborators/CollaboratorsHandler': this.CollaboratorsHandler, + '../Editor/EditorRealTimeController': this.EditorRealTimeController, }, }) }) @@ -133,6 +145,90 @@ describe('TokenAccessController', function () { }) }) + describe('when project owner in link-sharing-warning split test', function () { + beforeEach(function () { + this.SplitTestHandler.promises.getAssignmentForUser.resolves({ + variant: 'active', + }) + }) + + it('tells the ui to show the link-sharing-warning variant', async function () { + this.req.params = { token: this.token } + this.req.body = { tokenHashPrefix: '#prefix' } + await this.TokenAccessController.grantTokenAccessReadAndWrite( + this.req, + { + json: content => { + expect(content).to.deep.equal({ + requireAccept: { + linkSharingChanges: true, + projectName: this.project.name, + }, + }) + }, + } + ) + }) + + describe('normal case', function () { + beforeEach(function (done) { + this.req.params = { token: this.token } + this.req.body = { confirmedByUser: true, tokenHashPrefix: '#prefix' } + this.res.callback = done + this.TokenAccessController.grantTokenAccessReadAndWrite( + this.req, + this.res, + done + ) + }) + + it('adds the user as a read and write invited member', function () { + expect( + this.CollaboratorsHandler.promises.addUserIdToProject + ).to.have.been.calledWith( + this.project._id, + undefined, + this.user._id, + PrivilegeLevels.READ_AND_WRITE + ) + }) + + it('writes a project audit log', function () { + expect( + this.ProjectAuditLogHandler.promises.addEntry + ).to.have.been.calledWith( + this.project._id, + 'accept-via-link-sharing', + this.user._id, + this.req.ip, + { privileges: 'readAndWrite' } + ) + }) + + it('emits a project membership changed event', function () { + expect( + this.EditorRealTimeController.emitToRoom + ).to.have.been.calledWith( + this.project._id, + 'project:membership:changed', + { members: true } + ) + }) + + it('checks token hash', function () { + expect( + this.TokenAccessHandler.checkTokenHashPrefix + ).to.have.been.calledWith( + this.token, + '#prefix', + 'readAndWrite', + this.user._id, + { projectId: this.project._id, action: 'continue' } + ) + }) + }) + }) + describe('when the access was already granted', function () { beforeEach(function (done) { this.project.tokenAccessReadAndWrite_refs.push(this.user._id)