bypass linking sharing admin redirect for internal projects (#19314)

* disable linking sharing admin redirect

* address comments

* remove ignoreSiteAdmin

* load admin domains from settings

* add acceptance test

* more tests

* fix tests and restore admin domain

* use adminDomains as array

GitOrigin-RevId: 5acb62e1b6ada0aaeceab6db6a6635f82e30833f

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

@@ -12,7 +12,7 @@ const {
 } = require('../Helpers/AdminAuthorizationHelper')
 const { getSafeAdminDomainRedirect } = require('../Helpers/UrlHelper')
 
-function handleAdminDomainRedirect(req, res) {
+function _handleAdminDomainRedirect(req, res) {
   if (canRedirectToAdminDomain(SessionManager.getSessionUser(req.session))) {
     logger.warn({ req }, 'redirecting admin user to admin domain')
     res.redirect(getSafeAdminDomainRedirect(req.originalUrl))
@@ -150,7 +150,7 @@ async function ensureUserIsSiteAdmin(req, res, next) {
     logger.debug({ userId }, 'allowing user admin access to site')
     return next()
   }
-  if (handleAdminDomainRedirect(req, res)) return
+  if (_handleAdminDomainRedirect(req, res)) return
   logger.debug({ userId }, 'denying user admin access to site')
   _redirectToRestricted(req, res, next)
 }
@@ -205,6 +205,5 @@ module.exports = {
   ),
   ensureUserCanAdminProject: expressify(ensureUserCanAdminProject),
   ensureUserIsSiteAdmin: expressify(ensureUserIsSiteAdmin),
-  handleAdminDomainRedirect,
   restricted,
 }

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

@@ -8,9 +8,6 @@ const OError = require('@overleaf/o-error')
 const { expressify } = require('@overleaf/promise-utils')
 const AuthorizationManager = require('../Authorization/AuthorizationManager')
 const PrivilegeLevels = require('../Authorization/PrivilegeLevels')
-const {
-  handleAdminDomainRedirect,
-} = require('../Authorization/AuthorizationMiddleware')
 const ProjectAuditLogHandler = require('../Project/ProjectAuditLogHandler')
 const SplitTestHandler = require('../SplitTests/SplitTestHandler')
 const CollaboratorsHandler = require('../Collaborators/CollaboratorsHandler')
@@ -19,6 +16,12 @@ const CollaboratorsGetter = require('../Collaborators/CollaboratorsGetter')
 const ProjectGetter = require('../Project/ProjectGetter')
 const AsyncFormHelper = require('../Helpers/AsyncFormHelper')
 const AnalyticsManager = require('../Analytics/AnalyticsManager')
+const {
+  canRedirectToAdminDomain,
+} = require('../Helpers/AdminAuthorizationHelper')
+const { getSafeAdminDomainRedirect } = require('../Helpers/UrlHelper')
+const UserGetter = require('../User/UserGetter')
+const Settings = require('@overleaf/settings')
 
 const orderedPrivilegeLevels = [
   PrivilegeLevels.NONE,
@@ -86,15 +89,20 @@ async function _handleV1Project(token, userId) {
   }
 }
 
+async function _isOverleafStaff(userId) {
+  const emails = await UserGetter.promises.getUserConfirmedEmails(userId)
+  const adminDomains = Settings.adminDomains ?? []
+  return emails.some(email =>
+    adminDomains.some(adminDomain => email.email.endsWith(`@${adminDomain}`))
+  )
+}
+
 async function tokenAccessPage(req, res, next) {
   const { token } = req.params
   if (!TokenAccessHandler.isValidToken(token)) {
     return next(new Errors.NotFoundError())
   }
-  if (handleAdminDomainRedirect(req, res)) {
-    // Admin users do not join the project, but view it on the admin domain.
-    return
-  }
+
   try {
     if (TokenAccessHandler.isReadOnlyToken(token)) {
       const docPublishedInfo =
@@ -231,6 +239,37 @@ async function checkAndGetProjectOrResponseAction(
       { projectId, action: 'user already has higher or same privilege' },
     ]
   }
+
+  // Handle admin redirect
+  // If the project owner is an internal staff (using @overleaf.com email),
+  // the admin will join the project "for real".
+  // If the project owner is a external user
+  // the admin will be redirect to admin domain to view the project.
+  if (canRedirectToAdminDomain(SessionManager.getSessionUser(req.session))) {
+    const isProjectOwnerOverleafStaff = await _isOverleafStaff(
+      project.owner_ref
+    )
+    if (isProjectOwnerOverleafStaff) {
+      logger.warn(
+        { projectId, userId },
+        'letting admin user join staff project'
+      )
+    } else {
+      let projectUrlWithToken = TokenAccessHandler.makeTokenUrl(token)
+      if (tokenHashPrefix && tokenHashPrefix.startsWith('#')) {
+        projectUrlWithToken += `${tokenHashPrefix}`
+      }
+      return [
+        null,
+        () =>
+          res.json({
+            redirect: getSafeAdminDomainRedirect(projectUrlWithToken),
+          }),
+        { projectId, action: 'redirect admin user to admin domain' },
+      ]
+    }
+  }
+
   if (!tokenAccessEnabled) {
     return [
       null,

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

@@ -1,6 +1,9 @@
 const Settings = require('@overleaf/settings')
 const { expect } = require('chai')
 const User = require('./helpers/User').promises
+const {
+  getSafeAdminDomainRedirect,
+} = require('../../../app/src/Features/Helpers/UrlHelper')
 
 describe('AdminPrivilegeAvailable', function () {
   let adminUser, otherUser
@@ -22,7 +25,10 @@ describe('AdminPrivilegeAvailable', function () {
   })
 
   beforeEach('create other user and project', async function () {
-    otherUser = new User()
+    otherUser = new User({
+      email: 'test@non-staff.com',
+      confirmedAt: new Date(),
+    })
     await otherUser.login()
 
     otherUsersProjectId = await otherUser.createProject('other users project')
@@ -66,6 +72,16 @@ describe('AdminPrivilegeAvailable', function () {
     it('should display token access page for regular user', async function () {
       await displayTokenAccessPage(otherUser)
     })
+    it('should redirect a token grant request to project page', async function () {
+      const { response } = await adminUser.doRequest('POST', {
+        url: `${otherUsersProjectTokenAccessURL}/grant`,
+        json: {
+          confirmedByUser: true,
+        },
+      })
+      expect(response.statusCode).to.equal(200)
+      expect(response.body.redirect).to.equal(`/project/${otherUsersProjectId}`)
+    })
   })
 
   describe('adminPrivilegeAvailable=false', function () {
@@ -78,18 +94,49 @@ describe('AdminPrivilegeAvailable', function () {
     it('should block the admin from non-owned project', async function () {
       expect(await hasAccess(otherUsersProjectId)).to.equal(false)
     })
-    it('should redirect a token access request to admin panel', async function () {
-      const { response } = await adminUser.doRequest(
-        'GET',
-        otherUsersProjectTokenAccessURL
-      )
-      expect(response.statusCode).to.equal(302)
-      expect(response.headers.location).to.equal(
-        Settings.adminUrl + otherUsersProjectTokenAccessURL
-      )
+    it('should display token access page for admin', async function () {
+      displayTokenAccessPage(adminUser)
     })
     it('should display token access page for regular user', async function () {
       await displayTokenAccessPage(otherUser)
     })
+    it('should redirect a token grant request to admin panel if belongs to non-staff', async function () {
+      const { response } = await adminUser.doRequest('POST', {
+        url: `${otherUsersProjectTokenAccessURL}/grant`,
+        json: {
+          confirmedByUser: true,
+        },
+      })
+      expect(response.statusCode).to.equal(200)
+      expect(response.body.redirect).to.equal(
+        getSafeAdminDomainRedirect(otherUsersProjectTokenAccessURL)
+      )
+    })
+
+    it('should redirect a token grant request to project page if belongs to staff', async function () {
+      const staff = new User({
+        email: `test@${Settings.adminDomains[0]}`,
+        confirmedAt: new Date(),
+      })
+      await staff.ensureUserExists()
+      await staff.ensureAdmin()
+      await staff.login()
+
+      const staffProjectId = await staff.createProject('staff user project')
+      await staff.makeTokenBased(staffProjectId)
+      const {
+        tokens: { readOnly: readOnlyTokenAdmin },
+      } = await staff.getProject(staffProjectId)
+      const staffProjectTokenAccessURL = `/read/${readOnlyTokenAdmin}`
+
+      const { response } = await adminUser.doRequest('POST', {
+        url: `${staffProjectTokenAccessURL}/grant`,
+        json: {
+          confirmedByUser: true,
+        },
+      })
+      expect(response.statusCode).to.equal(200)
+      expect(response.body.redirect).to.equal(`/project/${staffProjectId}`)
+    })
   })
 })

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

@@ -23,7 +23,12 @@ describe('TokenAccessController', function () {
     this.res = new MockResponse()
     this.next = sinon.stub().returns()
 
-    this.Settings = {}
+    this.Settings = {
+      siteUrl: 'https://www.dev-overleaf.com',
+      adminPrivilegeAvailable: false,
+      adminUrl: 'https://admin.dev-overleaf.com',
+      adminDomains: ['overleaf.com'],
+    }
     this.TokenAccessHandler = {
       TOKEN_TYPES: {
         READ_ONLY: 'readOnly',
@@ -48,6 +53,7 @@ describe('TokenAccessController', function () {
 
     this.SessionManager = {
       getLoggedInUserId: sinon.stub().returns(this.user._id),
+      getSessionUser: sinon.stub().returns(this.user._id),
     }
 
     this.AuthenticationController = {
@@ -104,6 +110,18 @@ describe('TokenAccessController', function () {
       recordEventForSession: sinon.stub(),
     }
 
+    this.UserGetter = {
+      promises: {
+        getUser: sinon.stub().callsFake(async (userId, filter) => {
+          if (userId === this.userId) {
+            return this.user
+          } else {
+            return null
+          }
+        }),
+      },
+    }
+
     this.TokenAccessController = SandboxedModule.require(MODULE_PATH, {
       requires: {
         '@overleaf/settings': this.Settings,
@@ -125,6 +143,7 @@ describe('TokenAccessController', function () {
           redirect: sinon.stub(),
         }),
         '../Analytics/AnalyticsManager': this.AnalyticsManager,
+        '../User/UserGetter': this.UserGetter,
       },
     })
   })
@@ -540,6 +559,58 @@ describe('TokenAccessController', function () {
       })
     })
 
+    describe('when user is admin', function () {
+      const admin = { _id: new ObjectId(), isAdmin: true }
+      beforeEach(function () {
+        this.SessionManager.getLoggedInUserId.returns(admin._id)
+        this.SessionManager.getSessionUser.returns(admin)
+        this.req.params = { token: this.token }
+        this.req.body = { confirmedByUser: true, tokenHashPrefix: '#prefix' }
+      })
+
+      it('redirects if project owner is non-admin', function () {
+        this.UserGetter.promises.getUserConfirmedEmails = sinon
+          .stub()
+          .resolves([{ email: 'test@not-overleaf.com' }])
+        this.res.callback = () => {
+          expect(this.res.json).to.have.been.calledWith({
+            redirect: `${this.Settings.adminUrl}/#prefix`,
+          })
+        }
+        this.TokenAccessController.grantTokenAccessReadAndWrite(
+          this.req,
+          this.res
+        )
+      })
+
+      it('grants access if project owner is an internal staff', function () {
+        const internalStaff = { _id: new ObjectId(), isAdmin: true }
+        const projectFromInternalStaff = {
+          _id: new ObjectId(),
+          name: 'test',
+          tokenAccessReadAndWrite_refs: [],
+          tokenAccessReadOnly_refs: [],
+          owner_ref: internalStaff._id,
+        }
+        this.UserGetter.promises.getUser = sinon.stub().resolves(internalStaff)
+        this.UserGetter.promises.getUserConfirmedEmails = sinon
+          .stub()
+          .resolves([{ email: 'test@overleaf.com' }])
+        this.TokenAccessHandler.promises.getProjectByToken = sinon
+          .stub()
+          .resolves(projectFromInternalStaff)
+        this.res.callback = () => {
+          expect(
+            this.TokenAccessHandler.promises.addReadAndWriteUserToProject
+          ).to.have.been.calledWith(admin._id, projectFromInternalStaff._id)
+        }
+        this.TokenAccessController.grantTokenAccessReadAndWrite(
+          this.req,
+          this.res
+        )
+      })
+    })
+
     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 }