Merge pull request #15838 from overleaf/jel-link-sharing-redirect-hash

[web] Save link sharing URL hash as part of redirect

GitOrigin-RevId: 7d067852863b93e3246e5132511031005e333810

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

@@ -111,6 +111,7 @@ async function checkAndGetProjectOrResponseAction(
   tokenType,
   token,
   userId,
+  tokenHashPrefix,
   req,
   res,
   next
@@ -122,10 +123,14 @@ async function checkAndGetProjectOrResponseAction(
     !TokenAccessHandler.ANONYMOUS_READ_AND_WRITE_ENABLED
   ) {
     logger.warn('[TokenAccess] deny anonymous read-and-write token access')
-    AuthenticationController.setRedirectInSession(
-      req,
-      TokenAccessHandler.makeTokenUrl(token)
-    )
+
+    let projectUrlWithToken = TokenAccessHandler.makeTokenUrl(token)
+
+    if (tokenHashPrefix && tokenHashPrefix.startsWith('#')) {
+      projectUrlWithToken += `${tokenHashPrefix}`
+    }
+
+    AuthenticationController.setRedirectInSession(req, projectUrlWithToken)
     return [
       null,
       () => {
@@ -245,6 +250,7 @@ async function grantTokenAccessReadAndWrite(req, res, next) {
       tokenType,
       token,
       userId,
+      tokenHashPrefix,
       req,
       res,
       next
@@ -323,6 +329,7 @@ async function grantTokenAccessReadOnly(req, res, next) {
       tokenType,
       token,
       userId,
+      tokenHashPrefix,
       req,
       res,
       next

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

@@ -310,10 +310,7 @@ const TokenAccessHandler = {
       }
     }
 
-    if (
-      hashPrefixStatus === 'mismatch' ||
-      hashPrefixStatus === 'mismatch-v1-format'
-    ) {
+    if (hashPrefixStatus === 'mismatch') {
       logger.info(
         {
           tokenHashPrefix,

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

@@ -1247,6 +1247,73 @@ describe('TokenAccess', function () {
           done
         )
       })
+
+      it('should save URL hash in redirect', function (done) {
+        const urlFragment = '#123456'
+        const tokenWithUrlFragment = `${this.tokens.readAndWrite}${urlFragment}`
+
+        async.series(
+          [
+            cb =>
+              tryEditorAccess(
+                this.anon,
+                this.projectId,
+                expectErrorResponse.restricted.html,
+                cb
+              ),
+            cb =>
+              this.anon.request.get(
+                tokenWithUrlFragment,
+                (err, response, body) => {
+                  if (err) {
+                    return cb(err)
+                  }
+                  expect(response.statusCode).to.equal(200)
+
+                  this.anon.request.post(
+                    `${this.tokens.readAndWrite}/grant`,
+                    {
+                      json: {
+                        token: this.tokens.readAndWrite,
+                        tokenHashPrefix: urlFragment,
+                      },
+                    },
+                    (err, response, body) => {
+                      if (err) {
+                        return cb(err)
+                      }
+                      expect(response.statusCode).to.equal(200)
+                      expect(body).to.deep.equal({
+                        redirect: '/restricted',
+                        anonWriteAccessDenied: true,
+                      })
+                      cb()
+                    }
+                  )
+                }
+              ),
+            cb =>
+              tryAnonContentAccess(
+                this.anon,
+                this.projectId,
+                this.tokens.readAndWrite,
+                (response, body) => {
+                  expect(response.statusCode).to.equal(403)
+                  expect(body).to.equal('Forbidden')
+                },
+                cb
+              ),
+            cb =>
+              this.anon.login((err, response, body) => {
+                expect(err).to.not.exist
+                expect(response.statusCode).to.equal(200)
+                expect(body.redir).to.equal(`/${tokenWithUrlFragment}`)
+                cb()
+              }),
+          ],
+          done
+        )
+      })
     })
   } else {
     describe('anonymous read-and-write token, enabled', function () {

services/web/test/unit/src/TokenAccess/TokenAccessControllerTests.js

@@ -86,7 +86,7 @@ describe('TokenAccessController', function () {
     describe('normal case', function () {
       beforeEach(function (done) {
         this.req.params = { token: this.token }
-        this.req.body = { confirmedByUser: true, tokenHashPrefix: 'prefix' }
+        this.req.body = { confirmedByUser: true, tokenHashPrefix: '#prefix' }
         this.res.callback = done
         this.TokenAccessController.grantTokenAccessReadAndWrite(
           this.req,
@@ -118,7 +118,7 @@ describe('TokenAccessController', function () {
           this.TokenAccessHandler.checkTokenHashPrefix
         ).to.have.been.calledWith(
           this.token,
-          'prefix',
+          '#prefix',
           'readAndWrite',
           this.user._id,
           { projectId: this.project._id, action: 'continue' }
@@ -222,7 +222,7 @@ describe('TokenAccessController', function () {
       beforeEach(function () {
         this.SessionManager.getLoggedInUserId.returns(null)
         this.req.params = { token: this.token }
-        this.req.body = { tokenHashPrefix: 'prefix' }
+        this.req.body = { tokenHashPrefix: '#prefix' }
       })
       describe('ANONYMOUS_READ_AND_WRITE_ENABLED is undefined', function () {
         beforeEach(function (done) {
@@ -246,7 +246,7 @@ describe('TokenAccessController', function () {
             this.TokenAccessHandler.checkTokenHashPrefix
           ).to.have.been.calledWith(
             this.token,
-            'prefix',
+            '#prefix',
             'readAndWrite',
             null,
             {
@@ -254,6 +254,12 @@ describe('TokenAccessController', function () {
             }
           )
         })
+
+        it('saves redirect URL with URL fragment', function () {
+          expect(
+            this.AuthenticationController.setRedirectInSession.lastCall.args[1]
+          ).to.equal('/#prefix')
+        })
       })
 
       describe('ANONYMOUS_READ_AND_WRITE_ENABLED is true', function () {
@@ -280,7 +286,7 @@ describe('TokenAccessController', function () {
             this.TokenAccessHandler.checkTokenHashPrefix
           ).to.have.been.calledWith(
             this.token,
-            'prefix',
+            '#prefix',
             'readAndWrite',
             null,
             {
@@ -304,7 +310,7 @@ describe('TokenAccessController', function () {
             has_owner: true,
           })
           this.req.params = { token: this.token }
-          this.req.body = { tokenHashPrefix: 'prefix' }
+          this.req.body = { tokenHashPrefix: '#prefix' }
           this.res.callback = done
           this.TokenAccessController.grantTokenAccessReadAndWrite(
             this.req,
@@ -328,7 +334,7 @@ describe('TokenAccessController', function () {
             this.TokenAccessHandler.checkTokenHashPrefix
           ).to.have.been.calledWith(
             this.token,
-            'prefix',
+            '#prefix',
             'readAndWrite',
             this.user._id,
             {
@@ -345,7 +351,7 @@ describe('TokenAccessController', function () {
             exists: false,
           })
           this.req.params = { token: this.token }
-          this.req.body = { tokenHashPrefix: 'prefix' }
+          this.req.body = { tokenHashPrefix: '#prefix' }
           this.res.callback = done
           this.TokenAccessController.grantTokenAccessReadAndWrite(
             this.req,
@@ -361,7 +367,7 @@ describe('TokenAccessController', function () {
             this.TokenAccessHandler.checkTokenHashPrefix
           ).to.have.been.calledWith(
             this.token,
-            'prefix',
+            '#prefix',
             'readAndWrite',
             this.user._id,
             {
@@ -376,7 +382,7 @@ describe('TokenAccessController', function () {
       beforeEach(function () {
         this.TokenAccessHandler.promises.getProjectByToken.resolves(undefined)
         this.req.params = { token: this.token }
-        this.req.body = { tokenHashPrefix: 'prefix' }
+        this.req.body = { tokenHashPrefix: '#prefix' }
       })
       it('passes Errors.NotFoundError to next when project not found and still checks token hash', function (done) {
         this.TokenAccessController.grantTokenAccessReadAndWrite(
@@ -389,7 +395,7 @@ describe('TokenAccessController', function () {
               this.TokenAccessHandler.checkTokenHashPrefix
             ).to.have.been.calledWith(
               this.token,
-              'prefix',
+              '#prefix',
               'readAndWrite',
               this.user._id,
               {
@@ -406,7 +412,7 @@ describe('TokenAccessController', function () {
     it('passes Errors.NotFoundError to next when token access is not enabled but still checks token hash', function (done) {
       this.TokenAccessHandler.tokenAccessEnabledForProject.returns(false)
       this.req.params = { token: this.token }
-      this.req.body = { tokenHashPrefix: 'prefix' }
+      this.req.body = { tokenHashPrefix: '#prefix' }
       this.TokenAccessController.grantTokenAccessReadAndWrite(
         this.req,
         this.res,
@@ -417,7 +423,7 @@ describe('TokenAccessController', function () {
             this.TokenAccessHandler.checkTokenHashPrefix
           ).to.have.been.calledWith(
             this.token,
-            'prefix',
+            '#prefix',
             'readAndWrite',
             this.user._id,
             {
@@ -434,7 +440,7 @@ describe('TokenAccessController', function () {
     it('returns 400 when not using a read write token', function () {
       this.TokenAccessHandler.isReadAndWriteToken.returns(false)
       this.req.params = { token: this.token }
-      this.req.body = { tokenHashPrefix: 'prefix' }
+      this.req.body = { tokenHashPrefix: '#prefix' }
       this.TokenAccessController.grantTokenAccessReadAndWrite(
         this.req,
         this.res
@@ -447,7 +453,7 @@ describe('TokenAccessController', function () {
     describe('normal case', function () {
       beforeEach(function (done) {
         this.req.params = { token: this.token }
-        this.req.body = { confirmedByUser: true, tokenHashPrefix: 'prefix' }
+        this.req.body = { confirmedByUser: true, tokenHashPrefix: '#prefix' }
         this.res.callback = done
         this.TokenAccessController.grantTokenAccessReadOnly(
           this.req,
@@ -479,7 +485,7 @@ describe('TokenAccessController', function () {
           this.TokenAccessHandler.checkTokenHashPrefix
         ).to.have.been.calledWith(
           this.token,
-          'prefix',
+          '#prefix',
           'readOnly',
           this.user._id,
           { projectId: this.project._id, action: 'continue' }
@@ -521,7 +527,7 @@ describe('TokenAccessController', function () {
     it('returns 400 when not using a read only token', function () {
       this.TokenAccessHandler.isReadOnlyToken.returns(false)
       this.req.params = { token: this.token }
-      this.req.body = { tokenHashPrefix: 'prefix' }
+      this.req.body = { tokenHashPrefix: '#prefix' }
       this.TokenAccessController.grantTokenAccessReadOnly(this.req, this.res)
       expect(this.res.sendStatus).to.have.been.calledWith(400)
     })
@@ -587,7 +593,7 @@ describe('TokenAccessController', function () {
     it('passes Errors.NotFoundError to next when token access is not enabled but still checks token hash', function (done) {
       this.TokenAccessHandler.tokenAccessEnabledForProject.returns(false)
       this.req.params = { token: this.token }
-      this.req.body = { tokenHashPrefix: 'prefix' }
+      this.req.body = { tokenHashPrefix: '#prefix' }
       this.TokenAccessController.grantTokenAccessReadOnly(
         this.req,
         this.res,
@@ -598,7 +604,7 @@ describe('TokenAccessController', function () {
             this.TokenAccessHandler.checkTokenHashPrefix
           ).to.have.been.calledWith(
             this.token,
-            'prefix',
+            '#prefix',
             'readOnly',
             this.user._id,
             {

services/web/test/unit/src/TokenAccess/TokenAccessHandlerTests.js

@@ -755,10 +755,10 @@ describe('TokenAccessHandler', function () {
         }
       )
     })
+
     it('sends "mismatch-v1-format" for suspected v1 URLs with 7 numbers in URL fragment', function () {
       const token = '4112142489ddsbkrdzhxrq'
       const prefix = '%2F1234567%2F'
-
       this.TokenAccessHandler.checkTokenHashPrefix(
         token,
         `#${prefix}`,
@@ -766,7 +766,6 @@ describe('TokenAccessHandler', function () {
         userId,
         { projectId }
       )
-
       expect(this.Metrics.inc).to.have.been.calledWith(
         'link-sharing.hash-check',
         {
@@ -774,22 +773,10 @@ describe('TokenAccessHandler', function () {
           status: 'mismatch-v1-format',
         }
       )
-
-      expect(this.logger.info).to.have.been.calledWith(
-        {
-          tokenHashPrefix: prefix,
-          hashPrefixStatus: 'mismatch-v1-format',
-          userId,
-          projectId,
-          type: 'readAndWrite',
-        },
-        'mismatched token hash prefix'
-      )
     })
     it('sends "mismatch-v1-format" for suspected v1 URLs with 8 numbers in URL fragment', function () {
       const token = '4112142489ddsbkrdzhxrq'
       const prefix = '%2F12345678%2F'
-
       this.TokenAccessHandler.checkTokenHashPrefix(
         token,
         `#${prefix}`,
@@ -797,7 +784,6 @@ describe('TokenAccessHandler', function () {
         userId,
         { projectId }
       )
-
       expect(this.Metrics.inc).to.have.been.calledWith(
         'link-sharing.hash-check',
         {
@@ -805,17 +791,6 @@ describe('TokenAccessHandler', function () {
           status: 'mismatch-v1-format',
         }
       )
-
-      expect(this.logger.info).to.have.been.calledWith(
-        {
-          tokenHashPrefix: prefix,
-          hashPrefixStatus: 'mismatch-v1-format',
-          userId,
-          projectId,
-          type: 'readAndWrite',
-        },
-        'mismatched token hash prefix'
-      )
     })
   })
 })