Merge pull request #8897 from overleaf/ta-token-access-anonymous-redirect

Redirect Early on Anonymous Write Token Access Attempts

GitOrigin-RevId: 55e1839c3171a0a6a677ecca2f6bec87aad802bd

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

@@ -113,6 +113,28 @@ async function checkAndGetProjectOrResponseAction(
   res,
   next
 ) {
+  const isAnonymousUser = !userId
+  if (
+    isAnonymousUser &&
+    tokenType === TokenAccessHandler.TOKEN_TYPES.READ_AND_WRITE &&
+    !TokenAccessHandler.ANONYMOUS_READ_AND_WRITE_ENABLED
+  ) {
+    logger.warn('[TokenAccess] deny anonymous read-and-write token access')
+    AuthenticationController.setRedirectInSession(
+      req,
+      TokenAccessHandler.makeTokenUrl(token)
+    )
+    return [
+      null,
+      () => {
+        res.json({
+          redirect: '/restricted',
+          anonWriteAccessDenied: true,
+        })
+      },
+    ]
+  }
+
   // Try to get the project, and/or an alternative action to take.
   // Returns a tuple of [project, action]
   const project = await TokenAccessHandler.promises.getProjectByToken(
@@ -138,7 +160,6 @@ async function checkAndGetProjectOrResponseAction(
   }
 
   const projectId = project._id
-  const isAnonymousUser = !userId
   const tokenAccessEnabled =
     TokenAccessHandler.tokenAccessEnabledForProject(project)
   if (isAnonymousUser && tokenAccessEnabled) {
@@ -156,23 +177,10 @@ async function checkAndGetProjectOrResponseAction(
           },
         ]
       } else {
-        logger.warn(
+        // anonymous read-and-write token access should have been denied already
-          { projectId },
+        throw new Error(
-          '[TokenAccess] deny anonymous read-and-write token access'
+          'unreachable: anonymous read-and-write token access bug'
         )
-        AuthenticationController.setRedirectInSession(
-          req,
-          TokenAccessHandler.makeTokenUrl(token)
-        )
-        return [
-          null,
-          () => {
-            res.json({
-              redirect: '/restricted',
-              anonWriteAccessDenied: true,
-            })
-          },
-        ]
       }
     } else if (tokenType === TokenAccessHandler.TOKEN_TYPES.READ_ONLY) {
       logger.debug({ projectId }, 'granting read-only anonymous access')

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

@@ -993,6 +993,41 @@ describe('TokenAccess', function () {
           done
         )
       })
+
+      it('should require login if project does not exist', function (done) {
+        async.series(
+          [
+            // delete project
+            cb => {
+              this.owner.deleteProject(this.projectId, cb)
+            },
+            cb =>
+              tryReadAndWriteTokenAccess(
+                this.anon,
+                this.tokens.readAndWrite,
+                (response, body) => {
+                  expect(response.statusCode).to.equal(200)
+                },
+                (response, body) => {
+                  expect(response.statusCode).to.equal(200)
+                  expect(body).to.deep.equal({
+                    redirect: '/restricted',
+                    anonWriteAccessDenied: true,
+                  })
+                },
+                cb
+              ),
+            cb =>
+              this.anon.login((err, response, body) => {
+                expect(err).to.not.exist
+                expect(response.statusCode).to.equal(200)
+                expect(body.redir).to.equal(`/${this.tokens.readAndWrite}`)
+                cb()
+              }),
+          ],
+          done
+        )
+      })
     })
   } else {
     describe('anonymous read-and-write token, enabled', function () {
@@ -1118,6 +1153,35 @@ describe('TokenAccess', function () {
           )
         })
       })
+
+      it('should 404 if project does not exist', function (done) {
+        async.series(
+          [
+            // delete project
+            cb => {
+              this.owner.deleteProject(this.projectId, cb)
+            },
+            cb =>
+              tryReadAndWriteTokenAccess(
+                this.anon,
+                this.tokens.readAndWrite,
+                (response, body) => {
+                  expect(response.statusCode).to.equal(200)
+                },
+                (response, body) => {
+                  expect(response.statusCode).to.equal(200)
+                  expect(body).to.deep.equal({
+                    v1Import: {
+                      status: 'mustLogin',
+                    },
+                  })
+                },
+                cb
+              ),
+          ],
+          done
+        )
+      })
     })
   }