Merge pull request #4191 from overleaf/sk-token-v1-fix

TokenAccess: Fix handling of deleted projects
GitOrigin-RevId: a602da335567d36b5e674ada69c1e1ab4a909d4a

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

@@ -53,6 +53,8 @@ async function _handleV1Project(token, userId) {
       token,
       userId
     )
+    // This should not happen anymore, but it does show
+    // a nice "contact support" message, so it can stay
     if (!docInfo) {
       return { v1Import: { status: 'cannotImport' } }
     }

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

@@ -1,7 +1,6 @@
 const { Project } = require('../../models/Project')
 const PublicAccessLevels = require('../Authorization/PublicAccessLevels')
 const PrivilegeLevels = require('../Authorization/PrivilegeLevels')
-const UserGetter = require('../User/UserGetter')
 const { ObjectId } = require('mongodb')
 const Settings = require('settings-sharelatex')
 const logger = require('logger-sharelatex')
@@ -279,23 +278,12 @@ const TokenAccessHandler = {
         exported: false,
       })
     }
-    UserGetter.getUser(v2UserId, { overleaf: 1 }, function (err, user) {
+    const v1Url = `/api/v1/sharelatex/docs/${token}/info`
+    V1Api.request({ url: v1Url }, function (err, response, body) {
       if (err != null) {
         return callback(err)
       }
-      const v1UserId = user.overleaf != null ? user.overleaf.id : undefined
-      if (!v1UserId) {
-        return callback(null, null)
-      }
-      V1Api.request(
-        { url: `/api/v1/sharelatex/users/${v1UserId}/docs/${token}/info` },
-        function (err, response, body) {
-          if (err != null) {
-            return callback(err)
-          }
-          callback(null, body)
-        }
-      )
+      callback(null, body)
     })
   },
 }

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

@@ -50,7 +50,9 @@ block content
 				.container
 					.row
 						.col-sm-8.col-sm-offset-2
-							h1.text-center Move project to Overleaf v2
+							h1.text-center
+								span(ng-if="v1ImportData.status != 'mustLogin'") Overleaf v1 Project
+								span(ng-if="v1ImportData.status == 'mustLogin'") Please Log In
 							img.v2-import__img(
 								src="/img/v1-import/v2-editor.png"
 								alt="The new V2 editor."
@@ -60,7 +62,7 @@ block content
 								h2.text-center
 									| Cannot Access Overleaf v1 Project
 								p.text-center.row-spaced-small
-									| The project you are attempting to access must be imported to Overleaf v2 before it can be accessed. Please contact the project owner or
+									| Please contact the project owner or
 									|
 									a(href="/contact") contact support
 									|
@@ -68,14 +70,12 @@ block content
 
 							div(ng-if="v1ImportData.status == 'mustLogin'")
 								p.text-center.row-spaced-small
-									| This project has not yet been moved into the new version of
-									| Overleaf. You will need to log in and move it in order to
-									| continue working on it.
+									| You will need to log in to access this project.
 
 								.row-spaced.text-center
 									a.btn.btn-primary(
 										href="/login?redir={{ currentPath() }}"
-									) Log In To Move Project
+									) Log In To Access Project
 
 							div(ng-if="v1ImportData.status == 'canDownloadZip'")
 								p.text-center.row-spaced.small
@@ -98,4 +98,3 @@ block append foot-scripts
 				$('.loading-screen-brand').css('height', '20%')
 			}, 500);
 		});
-

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

@@ -1211,6 +1211,77 @@ describe('TokenAccess', function () {
     })
   })
 
+  describe('deleted project', function () {
+    beforeEach(function (done) {
+      settings.overleaf = { host: 'http://localhost:5000' }
+      this.owner.createProject(
+        `delete-test${Math.random()}`,
+        (err, projectId) => {
+          if (err != null) {
+            return done(err)
+          }
+          this.projectId = projectId
+          this.owner.makeTokenBased(this.projectId, err => {
+            if (err != null) {
+              return done(err)
+            }
+            this.owner.getProject(this.projectId, (err, project) => {
+              if (err != null) {
+                return done(err)
+              }
+              this.tokens = project.tokens
+              done()
+            })
+          })
+        }
+      )
+    })
+
+    afterEach(function () {
+      delete settings.overleaf
+    })
+
+    it('should 404', function (done) {
+      async.series(
+        [
+          // delete project
+          cb => {
+            this.owner.deleteProject(this.projectId, cb)
+          },
+          cb => {
+            // use read-only token
+            tryReadOnlyTokenAccess(
+              this.other1,
+              this.tokens.readOnly,
+              (response, body) => {
+                expect(response.statusCode).to.equal(200)
+              },
+              (response, body) => {
+                expect(response.statusCode).to.equal(404)
+              },
+              cb
+            )
+          },
+          cb => {
+            // use read-write token
+            tryReadAndWriteTokenAccess(
+              this.other1,
+              this.tokens.readAndWrite,
+              (response, body) => {
+                expect(response.statusCode).to.equal(200)
+              },
+              (response, body) => {
+                expect(response.statusCode).to.equal(404)
+              },
+              cb
+            )
+          },
+        ],
+        done
+      )
+    })
+  })
+
   describe('unimported v1 project', function () {
     beforeEach(function () {
       settings.overleaf = { host: 'http://localhost:5000' }
@@ -1220,8 +1291,15 @@ describe('TokenAccess', function () {
       delete settings.overleaf
     })
 
-    it('should show error page for read and write token', function (done) {
+    it('should show download option for read and write token', function (done) {
       const unimportedV1Token = '123abcdefabcdef'
+      const docInfo = {
+        exists: true,
+        exported: false,
+        has_owner: true,
+        name: 'test',
+      }
+      MockV1Api.setDocInfo(unimportedV1Token, docInfo)
       tryReadAndWriteTokenAccess(
         this.owner,
         unimportedV1Token,
@@ -1230,14 +1308,28 @@ describe('TokenAccess', function () {
         },
         (response, body) => {
           expect(response.statusCode).to.equal(200)
-          expect(body).to.deep.equal({ v1Import: { status: 'cannotImport' } })
+          expect(body).to.deep.equal({
+            v1Import: {
+              hasOwner: true,
+              name: 'test',
+              projectId: unimportedV1Token,
+              status: 'canDownloadZip',
+            },
+          })
         },
         done
       )
     })
 
-    it('should show error page for read only token to v1', function (done) {
+    it('should show download option for read only token to v1', function (done) {
       const unimportedV1Token = 'aaaaaabbbbbb'
+      const docInfo = {
+        exists: true,
+        exported: false,
+        has_owner: true,
+        name: 'test',
+      }
+      MockV1Api.setDocInfo(unimportedV1Token, docInfo)
       tryReadOnlyTokenAccess(
         this.owner,
         unimportedV1Token,
@@ -1246,7 +1338,14 @@ describe('TokenAccess', function () {
         },
         (response, body) => {
           expect(response.statusCode).to.equal(200)
-          expect(body).to.deep.equal({ v1Import: { status: 'cannotImport' } })
+          expect(body).to.deep.equal({
+            v1Import: {
+              hasOwner: true,
+              name: 'test',
+              projectId: unimportedV1Token,
+              status: 'canDownloadZip',
+            },
+          })
         },
         done
       )

services/web/test/acceptance/src/mocks/MockV1Api.js

@@ -353,6 +353,14 @@ class MockV1Api extends AbstractMockApi {
       }
     )
 
+    this.app.get('/api/v1/sharelatex/docs/:token/info', (req, res) => {
+      const info = this.getDocInfo(req.params.token) || {
+        exists: false,
+        exported: false,
+      }
+      res.json(info)
+    })
+
     this.app.get(
       '/api/v1/sharelatex/docs/read_token/:token/exists',
       (req, res) => {

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

@@ -38,7 +38,6 @@ describe('TokenAccessHandler', function () {
         mongodb: { ObjectId },
         '../../models/Project': { Project: (this.Project = {}) },
         'settings-sharelatex': (this.settings = {}),
-        '../User/UserGetter': (this.UserGetter = {}),
         '../V1/V1Api': (this.V1Api = {
           request: sinon.stub(),
         }),
@@ -616,7 +615,6 @@ describe('TokenAccessHandler', function () {
 
   describe('getV1DocInfo', function () {
     beforeEach(function () {
-      this.v2UserId = 123
       return (this.callback = sinon.stub())
     })
 
@@ -645,47 +643,8 @@ describe('TokenAccessHandler', function () {
         return (this.settings.apis = { v1: 'v1' })
       })
 
-      describe('on UserGetter.getUser success', function () {
-        beforeEach(function () {
-          this.UserGetter.getUser = sinon.stub().yields(null, {
-            overleaf: { id: 1 },
-          })
-          return this.TokenAccessHandler.getV1DocInfo(
-            this.token,
-            this.v2UserId,
-            this.callback
-          )
-        })
-
-        it('should get user', function () {
-          return expect(
-            this.UserGetter.getUser.calledWith(this.v2UserId)
-          ).to.equal(true)
-        })
-      })
-
-      describe('on UserGetter.getUser error', function () {
-        beforeEach(function () {
-          this.error = new Error('failed to get user')
-          this.UserGetter.getUser = sinon.stub().yields(this.error)
-          return this.TokenAccessHandler.getV1DocInfo(
-            this.token,
-            this.v2UserId,
-            this.callback
-          )
-        })
-
-        it('should callback with error', function () {
-          return expect(this.callback.calledWith(this.error)).to.equal(true)
-        })
-      })
-
       describe('on V1Api.request success', function () {
         beforeEach(function () {
-          this.v1UserId = 1
-          this.UserGetter.getUser = sinon.stub().yields(null, {
-            overleaf: { id: this.v1UserId },
-          })
           this.V1Api.request = sinon
             .stub()
             .callsArgWith(1, null, null, 'mock-data')
@@ -699,7 +658,7 @@ describe('TokenAccessHandler', function () {
         it('should return response body', function () {
           expect(
             this.V1Api.request.calledWith({
-              url: `/api/v1/sharelatex/users/${this.v1UserId}/docs/${this.token}/info`,
+              url: `/api/v1/sharelatex/docs/${this.token}/info`,
             })
           ).to.equal(true)
           return expect(this.callback.calledWith(null, 'mock-data')).to.equal(
@@ -710,9 +669,6 @@ describe('TokenAccessHandler', function () {
 
       describe('on V1Api.request error', function () {
         beforeEach(function () {
-          this.UserGetter.getUser = sinon.stub().yields(null, {
-            overleaf: { id: 1 },
-          })
           this.V1Api.request = sinon.stub().callsArgWith(1, 'error')
           return this.TokenAccessHandler.getV1DocInfo(
             this.token,