Merge pull request #8571 from overleaf/ta-token-access-page

Require User Interaction on Token Access Page

GitOrigin-RevId: 2f4c00ba75ebd6bd87d3e770ec8223d736344f5b

services/web/app/src/Features/Authorization/AuthorizationManager.js

@@ -69,7 +69,7 @@ async function getPrivilegeLevelForProject(
   opts = {}
 ) {
   if (userId) {
-    return getPrivilegeLevelForProjectWithUser(userId, projectId, token, opts)
+    return getPrivilegeLevelForProjectWithUser(userId, projectId, opts)
   } else {
     return getPrivilegeLevelForProjectWithoutUser(projectId, token, opts)
   }
@@ -79,7 +79,6 @@ async function getPrivilegeLevelForProject(
 async function getPrivilegeLevelForProjectWithUser(
   userId,
   projectId,
-  token,
   opts = {}
 ) {
   const privilegeLevel =

services/web/app/src/Features/TokenAccess/TokenAccessController.js

@@ -19,20 +19,17 @@ const orderedPrivilegeLevels = [
   PrivilegeLevels.OWNER,
 ]
 
-async function _userAlreadyHasHigherPrivilege(
+async function _userAlreadyHasHigherPrivilege(userId, projectId, tokenType) {
-  userId,
-  projectId,
-  token,
-  tokenType
-) {
   if (!Object.values(TokenAccessHandler.TOKEN_TYPES).includes(tokenType)) {
     throw new Error('bad token type')
   }
+  if (!userId) {
+    return false
+  }
   const privilegeLevel =
     await AuthorizationManager.promises.getPrivilegeLevelForProject(
       userId,
-      projectId,
+      projectId
-      token
     )
   return (
     orderedPrivilegeLevels.indexOf(privilegeLevel) >=
@@ -196,7 +193,6 @@ async function checkAndGetProjectOrResponseAction(
   const userHasPrivilege = await _userAlreadyHasHigherPrivilege(
     userId,
     projectId,
-    token,
     tokenType
   )
   if (userHasPrivilege) {
@@ -220,6 +216,7 @@ async function checkAndGetProjectOrResponseAction(
 
 async function grantTokenAccessReadAndWrite(req, res, next) {
   const { token } = req.params
+  const { confirmedByUser } = req.body
   const userId = SessionManager.getLoggedInUserId(req.session)
   if (!TokenAccessHandler.isReadAndWriteToken(token)) {
     return res.sendStatus(400)
@@ -240,6 +237,13 @@ async function grantTokenAccessReadAndWrite(req, res, next) {
     if (!project) {
       return next(new Errors.NotFoundError())
     }
+    if (!confirmedByUser) {
+      return res.json({
+        requireAccept: {
+          projectName: project.name,
+        },
+      })
+    }
     await TokenAccessHandler.promises.addReadAndWriteUserToProject(
       userId,
       project._id
@@ -261,6 +265,7 @@ async function grantTokenAccessReadAndWrite(req, res, next) {
 
 async function grantTokenAccessReadOnly(req, res, next) {
   const { token } = req.params
+  const { confirmedByUser } = req.body
   const userId = SessionManager.getLoggedInUserId(req.session)
   if (!TokenAccessHandler.isReadOnlyToken(token)) {
     return res.sendStatus(400)
@@ -286,6 +291,13 @@ async function grantTokenAccessReadOnly(req, res, next) {
     if (!project) {
       return next(new Errors.NotFoundError())
     }
+    if (!confirmedByUser) {
+      return res.json({
+        requireAccept: {
+          projectName: project.name,
+        },
+      })
+    }
     await TokenAccessHandler.promises.addReadOnlyUserToProject(
       userId,
       project._id

services/web/app/views/project/token/access.pug

@@ -91,6 +91,31 @@ block content
 									a.btn.btn-primary(ng-href="{{ buildZipDownloadPath(v1ImportData.projectId) }}")
 										| Download project zip file
 
+			.loading-screen(
+				ng-show="mode == 'requireAccept'"
+			)
+				.container
+					.row
+						.col-md-8.col-md-offset-2
+							.card
+								.page-header.text-centered
+									h1 #{translate("invited_to_join")}
+										br
+										em {{ getProjectName() }}
+								.row.text-center
+									.col-md-12
+										p
+											if user
+												| #{translate("accepting_invite_as")}
+												|
+												em #{user.email}
+								.row.text-center
+									.col-md-12
+											button.btn.btn-lg.btn-primary(
+												type='submit'
+												ng-click="postConfirmedByUser()"
+											) #{translate("join_project")}
+
 
 block append foot-scripts
 	script(type="text/javascript", nonce=scriptNonce).

services/web/frontend/js/features/form-helpers/hydrate-form.js

@@ -44,8 +44,8 @@ function formSubmitHelper(formEl) {
       formEl.dispatchEvent(new Event('sent'))
 
       // Handle redirects
-      if (data.redir) {
+      if (data.redir || data.redirect) {
-        window.location = data.redir
+        window.location = data.redir || data.redirect
         return
       }
 

services/web/frontend/js/main/token-access.js

@@ -3,9 +3,10 @@ App.controller(
   'TokenAccessPageController',
   ($scope, $http, $location, localStorage) => {
     window.S = $scope
-    $scope.mode = 'accessAttempt' // 'accessAttempt' | 'v1Import'
+    $scope.mode = 'accessAttempt' // 'accessAttempt' | 'v1Import' | 'requireAccept'
 
     $scope.v1ImportData = null
+    $scope.requireAccept = null
 
     $scope.accessInFlight = false
     $scope.accessSuccess = false
@@ -20,14 +21,20 @@ App.controller(
     }
 
     $scope.getProjectName = () => {
-      if (!$scope.v1ImportData || !$scope.v1ImportData.name) {
+      if ($scope.v1ImportData?.name) {
-        return 'This project'
-      } else {
         return $scope.v1ImportData.name
+      } else if ($scope.requireAccept?.projectName) {
+        return $scope.requireAccept.projectName
+      } else {
+        return 'This project'
       }
     }
 
-    $scope.post = () => {
+    $scope.postConfirmedByUser = () => {
+      $scope.post(true)
+    }
+
+    $scope.post = (confirmedByUser = false) => {
       $scope.mode = 'accessAttempt'
       const textData = $('#overleaf-token-access-data').text()
       const parsedData = JSON.parse(textData)
@@ -39,6 +46,7 @@ App.controller(
         url: postUrl,
         data: {
           _csrf: csrfToken,
+          confirmedByUser,
         },
       }).then(
         function successCallback(response) {
@@ -59,6 +67,9 @@ App.controller(
           } else if (data.v1Import) {
             $scope.mode = 'v1Import'
             $scope.v1ImportData = data.v1Import
+          } else if (data.requireAccept) {
+            $scope.mode = 'requireAccept'
+            $scope.requireAccept = data.requireAccept
           } else {
             console.warn(
               'invalid data from server in success response',

services/web/locales/en.json

@@ -1030,6 +1030,7 @@
   "login_here": "Login here",
   "set_new_password": "Set new password",
   "user_wants_you_to_see_project": "__username__ would like you to join __projectname__",
+  "invited_to_join": "You have been invited to join",
   "join_sl_to_view_project": "Join __appName__ to view this project",
   "register_to_edit_template": "Please register to edit the __templateName__ template",
   "already_have_sl_account": "Already have an __appName__ account?",

services/web/test/acceptance/src/TokenAccessTests.js

@@ -96,6 +96,70 @@ const _doTryTokenAccess = (
   })
 }
 
+const tryReadOnlyTokenAccept = (
+  user,
+  token,
+  testPageLoad,
+  testFormPost,
+  callback
+) => {
+  _doTryTokenAccept(
+    `/read/${token}`,
+    user,
+    token,
+    testPageLoad,
+    testFormPost,
+    callback
+  )
+}
+
+const tryReadAndWriteTokenAccept = (
+  user,
+  token,
+  testPageLoad,
+  testFormPost,
+  callback
+) => {
+  _doTryTokenAccept(
+    `/${token}`,
+    user,
+    token,
+    testPageLoad,
+    testFormPost,
+    callback
+  )
+}
+
+const _doTryTokenAccept = (
+  url,
+  user,
+  token,
+  testPageLoad,
+  testFormPost,
+  callback
+) => {
+  user.request.get(url, (err, response, body) => {
+    if (err) {
+      return callback(err)
+    }
+    testPageLoad(response, body)
+    if (!testFormPost) {
+      return callback()
+    }
+    user.request.post(
+      `${url}/grant`,
+      { json: { token, confirmedByUser: true } },
+      (err, response, body) => {
+        if (err) {
+          return callback(err)
+        }
+        testFormPost(response, body)
+        callback()
+      }
+    )
+  })
+}
+
 const tryContentAccess = (user, projcetId, test, callback) => {
   // The real-time service calls this end point to determine the user's
   // permissions.
@@ -233,27 +297,25 @@ describe('TokenAccess', function () {
 
   describe('read-only token', function () {
     beforeEach(function (done) {
-      this.owner.createProject(
+      this.projectName = `token-ro-test${Math.random()}`
-        `token-ro-test${Math.random()}`,
+      this.owner.createProject(this.projectName, (err, projectId) => {
-        (err, projectId) => {
+        if (err != null) {
+          return done(err)
+        }
+        this.projectId = projectId
+        this.owner.makeTokenBased(this.projectId, err => {
           if (err != null) {
             return done(err)
           }
-          this.projectId = projectId
+          this.owner.getProject(this.projectId, (err, project) => {
-          this.owner.makeTokenBased(this.projectId, err => {
             if (err != null) {
               return done(err)
             }
-            this.owner.getProject(this.projectId, (err, project) => {
+            this.tokens = project.tokens
-              if (err != null) {
+            done()
-                return done(err)
-              }
-              this.tokens = project.tokens
-              done()
-            })
           })
-        }
+        })
-      )
+      })
     })
 
     it('allow the user read-only access to the project', function (done) {
@@ -269,8 +331,34 @@ describe('TokenAccess', function () {
             )
           },
           cb => {
-            // use token
+            // try token
             tryReadOnlyTokenAccess(
+              this.other1,
+              this.tokens.readOnly,
+              (response, body) => {
+                expect(response.statusCode).to.equal(200)
+              },
+              (response, body) => {
+                expect(response.statusCode).to.equal(200)
+                expect(body.requireAccept.projectName).to.equal(
+                  this.projectName
+                )
+              },
+              cb
+            )
+          },
+          cb => {
+            // deny access before token is accepted
+            tryEditorAccess(
+              this.other1,
+              this.projectId,
+              expectErrorResponse.restricted.html,
+              cb
+            )
+          },
+          cb => {
+            // accept token
+            tryReadOnlyTokenAccept(
               this.other1,
               this.tokens.readOnly,
               (response, body) => {
@@ -543,27 +631,25 @@ describe('TokenAccess', function () {
 
   describe('read-and-write token', function () {
     beforeEach(function (done) {
-      this.owner.createProject(
+      this.projectName = `token-rw-test${Math.random()}`
-        `token-rw-test${Math.random()}`,
+      this.owner.createProject(this.projectName, (err, projectId) => {
-        (err, projectId) => {
+        if (err != null) {
+          return done(err)
+        }
+        this.projectId = projectId
+        this.owner.makeTokenBased(this.projectId, err => {
           if (err != null) {
             return done(err)
           }
-          this.projectId = projectId
+          this.owner.getProject(this.projectId, (err, project) => {
-          this.owner.makeTokenBased(this.projectId, err => {
             if (err != null) {
               return done(err)
             }
-            this.owner.getProject(this.projectId, (err, project) => {
+            this.tokens = project.tokens
-              if (err != null) {
+            done()
-                return done(err)
-              }
-              this.tokens = project.tokens
-              done()
-            })
           })
-        }
+        })
-      )
+      })
     })
 
     it('should allow the user to access project via read-and-write token url', function (done) {
@@ -577,8 +663,33 @@ describe('TokenAccess', function () {
               expectErrorResponse.restricted.html,
               cb
             ),
+          // try token
           cb =>
             tryReadAndWriteTokenAccess(
+              this.other1,
+              this.tokens.readAndWrite,
+              (response, body) => {
+                expect(response.statusCode).to.equal(200)
+              },
+              (response, body) => {
+                expect(response.statusCode).to.equal(200)
+                expect(body.requireAccept.projectName).to.equal(
+                  this.projectName
+                )
+              },
+              cb
+            ),
+          // deny access before the token is accepted
+          cb =>
+            tryEditorAccess(
+              this.other1,
+              this.projectId,
+              expectErrorResponse.restricted.html,
+              cb
+            ),
+          // accept token
+          cb =>
+            tryReadAndWriteTokenAccept(
               this.other1,
               this.tokens.readAndWrite,
               (response, body) => {
@@ -661,7 +772,7 @@ describe('TokenAccess', function () {
               ),
             cb => {
               // use read-only token
-              tryReadOnlyTokenAccess(
+              tryReadOnlyTokenAccept(
                 this.other1,
                 this.tokens.readOnly,
                 (response, body) => {
@@ -707,7 +818,7 @@ describe('TokenAccess', function () {
             // Then switch to read-write token
             //
             cb =>
-              tryReadAndWriteTokenAccess(
+              tryReadAndWriteTokenAccept(
                 this.other1,
                 this.tokens.readAndWrite,
                 (response, body) => {