From ac83dd9bb34645067dd954063ed0f0f7f45e371f Mon Sep 17 00:00:00 2001
From: June Kelly <june.kelly@overleaf.com>
Date: Wed, 1 Feb 2023 11:06:14 +0000
Subject: [PATCH] Merge pull request #11353 from
 overleaf/jk-remove-deprecated-public-access-levels

[web] Disallow deprecated access levels from being set

GitOrigin-RevId: cf276e849692be210a2dc7d896820579efc46952
---
 .../Authorization/PublicAccessLevels.js       | 11 ++++++++++
 .../Features/Project/ProjectDetailsHandler.js |  9 +--------
 .../test/acceptance/src/AuthorizationTests.js |  8 ++++++++
 .../web/test/acceptance/src/helpers/User.js   | 20 +++++++------------
 .../src/Project/ProjectDetailsHandlerTests.js |  2 +-
 5 files changed, 28 insertions(+), 22 deletions(-)

diff --git a/services/web/app/src/Features/Authorization/PublicAccessLevels.js b/services/web/app/src/Features/Authorization/PublicAccessLevels.js
index c7619ff638..285acd19e0 100644
--- a/services/web/app/src/Features/Authorization/PublicAccessLevels.js
+++ b/services/web/app/src/Features/Authorization/PublicAccessLevels.js
@@ -1,3 +1,14 @@
+/**
+ * Note:
+ * It used to be that `project.publicAccessLevel` could be set to `private`,
+ * `readOnly` or `readAndWrite`, the latter of which made the project publicly
+ * accessible.
+ *
+ * This system was replaced with "link sharing", therafter the valid values are
+ * `private` or `tokenBased`. While it is no longer possible to set
+ * `publicAccessLevel` to the legacy values, there are projects in the system
+ * that already have those values set.
+ */
 module.exports = {
   READ_ONLY: 'readOnly', // LEGACY
   READ_AND_WRITE: 'readAndWrite', // LEGACY
diff --git a/services/web/app/src/Features/Project/ProjectDetailsHandler.js b/services/web/app/src/Features/Project/ProjectDetailsHandler.js
index 539bc6d742..90c5cca7ee 100644
--- a/services/web/app/src/Features/Project/ProjectDetailsHandler.js
+++ b/services/web/app/src/Features/Project/ProjectDetailsHandler.js
@@ -181,18 +181,11 @@ function fixProjectName(name) {
 }
 
 async function setPublicAccessLevel(projectId, newAccessLevel) {
-  // DEPRECATED: `READ_ONLY` and `READ_AND_WRITE` are still valid in, but should no longer
-  // be passed here. Remove after token-based access has been live for a while
   if (
     projectId != null &&
     newAccessLevel != null &&
     _.include(
-      [
-        PublicAccessLevels.READ_ONLY,
-        PublicAccessLevels.READ_AND_WRITE,
-        PublicAccessLevels.PRIVATE,
-        PublicAccessLevels.TOKEN_BASED,
-      ],
+      [PublicAccessLevels.PRIVATE, PublicAccessLevels.TOKEN_BASED],
       newAccessLevel
     )
   ) {
diff --git a/services/web/test/acceptance/src/AuthorizationTests.js b/services/web/test/acceptance/src/AuthorizationTests.js
index f700909f80..885dbabde5 100644
--- a/services/web/test/acceptance/src/AuthorizationTests.js
+++ b/services/web/test/acceptance/src/AuthorizationTests.js
@@ -633,6 +633,10 @@ describe('Authorization', function () {
   })
 
   describe('public read-write project', function () {
+    /**
+     * Note: this is a test for the legacy "public access" feature.
+     * See documentation comment in `Authorization/PublicAccessLevels`
+     * */
     beforeEach(function (done) {
       this.owner.createProject('public-rw-project', (error, projectId) => {
         if (error != null) {
@@ -693,6 +697,10 @@ describe('Authorization', function () {
   })
 
   describe('public read-only project', function () {
+    /**
+     * Note: this is a test for the legacy "public access" feature.
+     * See documentation comment in `Authorization/PublicAccessLevels`
+     * */
     beforeEach(function (done) {
       this.owner.createProject('public-ro-project', (error, projectId) => {
         if (error != null) {
diff --git a/services/web/test/acceptance/src/helpers/User.js b/services/web/test/acceptance/src/helpers/User.js
index 817053ed6f..c3af73f1f8 100644
--- a/services/web/test/acceptance/src/helpers/User.js
+++ b/services/web/test/acceptance/src/helpers/User.js
@@ -624,19 +624,13 @@ class User {
   }
 
   makePublic(projectId, level, callback) {
-    this.request.post(
-      {
-        url: `/project/${projectId}/settings/admin`,
-        json: {
-          publicAccessLevel: level,
-        },
-      },
-      (error, response, body) => {
-        if (error != null) {
-          return callback(error)
-        }
-        callback(null)
-      }
+    // A fudge, to get around the fact that `readOnly` and `readAndWrite` are now disallowed
+    // via the API, but we still need to test the behaviour of projects with these values set.
+    db.projects.updateOne(
+      { _id: ObjectId(projectId) },
+      // NOTE: Yes, there is a typo in the db schema.
+      { $set: { publicAccesLevel: level } },
+      callback
     )
   }
 
diff --git a/services/web/test/unit/src/Project/ProjectDetailsHandlerTests.js b/services/web/test/unit/src/Project/ProjectDetailsHandlerTests.js
index 0edf69eb3c..fe4d2885ca 100644
--- a/services/web/test/unit/src/Project/ProjectDetailsHandlerTests.js
+++ b/services/web/test/unit/src/Project/ProjectDetailsHandlerTests.js
@@ -443,7 +443,7 @@ describe('ProjectDetailsHandler', function () {
 
   describe('setPublicAccessLevel', function () {
     beforeEach(function () {
-      this.accessLevel = 'readOnly'
+      this.accessLevel = 'tokenBased'
     })
 
     it('should update the project with the new level', async function () {