Przeglądaj źródła

[real-time] ensure tc is enabled when reviewer role is used (#22245)

* Support for adding reviewer role

* show reviewer in track changes user list

* added "review" in assertClientCanViewProject

* test if reviewer can read project

* added collaboratorsGetter tests

* eit toggle-track-changes when track changes changes

* [real-time] ensure tc is enabled when reviewer role is used

* use assertClientCanReviewProjectAndDoc, refactor _assertClientCanApplyUpdate

GitOrigin-RevId: 158bd1ff0d4b4977da950134f8ad8b3740855290
Domagoj Kriskovic 1 rok temu
rodzic
commit
5a447aa349

+ 17 - 0
services/real-time/app/js/AuthorizationManager.js

@@ -18,6 +18,14 @@ module.exports = AuthorizationManager = {
     )
   },
 
+  assertClientCanReviewProject(client, callback) {
+    AuthorizationManager._assertClientHasPrivilegeLevel(
+      client,
+      ['readAndWrite', 'owner', 'review'],
+      callback
+    )
+  },
+
   _assertClientHasPrivilegeLevel(client, allowedLevels, callback) {
     if (allowedLevels.includes(client.ol_context.privilege_level)) {
       callback(null)
@@ -44,6 +52,15 @@ module.exports = AuthorizationManager = {
     })
   },
 
+  assertClientCanReviewProjectAndDoc(client, docId, callback) {
+    AuthorizationManager.assertClientCanReviewProject(client, function (error) {
+      if (error) {
+        return callback(error)
+      }
+      AuthorizationManager._assertClientCanAccessDoc(client, docId, callback)
+    })
+  },
+
   _assertClientCanAccessDoc(client, docId, callback) {
     if (client.ol_context[`doc:${docId}`] === 'allowed') {
       callback(null)

+ 19 - 20
services/real-time/app/js/WebsocketController.js

@@ -622,26 +622,25 @@ module.exports = WebsocketController = {
   },
 
   _assertClientCanApplyUpdate(client, docId, update, callback) {
-    AuthorizationManager.assertClientCanEditProjectAndDoc(
-      client,
-      docId,
-      function (error) {
-        if (
-          error &&
-          error.message === 'not authorized' &&
-          WebsocketController._isCommentUpdate(update)
-        ) {
-          // This might be a comment op, which we only need read-only priveleges for
-          AuthorizationManager.assertClientCanViewProjectAndDoc(
-            client,
-            docId,
-            callback
-          )
-          return
-        }
-        callback(error)
-      }
-    )
+    if (WebsocketController._isCommentUpdate(update)) {
+      return AuthorizationManager.assertClientCanViewProjectAndDoc(
+        client,
+        docId,
+        callback
+      )
+    } else if (update.meta?.tc) {
+      return AuthorizationManager.assertClientCanReviewProjectAndDoc(
+        client,
+        docId,
+        callback
+      )
+    } else {
+      return AuthorizationManager.assertClientCanEditProjectAndDoc(
+        client,
+        docId,
+        callback
+      )
+    }
   },
 
   _isCommentUpdate(update) {

+ 100 - 1
services/real-time/test/unit/js/AuthorizationManagerTests.js

@@ -228,7 +228,7 @@ describe('AuthorizationManager', function () {
     })
   })
 
-  return describe('assertClientCanEditProjectAndDoc', function () {
+  describe('assertClientCanEditProjectAndDoc', function () {
     beforeEach(function () {
       this.doc_id = '12345'
       this.callback = sinon.stub()
@@ -326,4 +326,103 @@ describe('AuthorizationManager', function () {
       })
     })
   })
+
+  return describe('assertClientCanReviewProjectAndDoc', function () {
+    beforeEach(function () {
+      this.doc_id = '12345'
+      this.callback = sinon.stub()
+      return (this.client.ol_context = {})
+    })
+
+    describe('when not authorised at the project level', function () {
+      beforeEach(function () {
+        return (this.client.ol_context.privilege_level = 'readOnly')
+      })
+
+      it('should not allow access', function () {
+        return this.AuthorizationManager.assertClientCanReviewProjectAndDoc(
+          this.client,
+          this.doc_id,
+          err => err.message.should.equal('not authorized')
+        )
+      })
+
+      return describe('even when authorised at the doc level', function () {
+        beforeEach(function (done) {
+          return this.AuthorizationManager.addAccessToDoc(
+            this.client,
+            this.doc_id,
+            done
+          )
+        })
+
+        return it('should not allow access', function () {
+          return this.AuthorizationManager.assertClientCanReviewProjectAndDoc(
+            this.client,
+            this.doc_id,
+            err => err.message.should.equal('not authorized')
+          )
+        })
+      })
+    })
+
+    return describe('when authorised at the project level', function () {
+      beforeEach(function () {
+        return (this.client.ol_context.privilege_level = 'review')
+      })
+
+      describe('and not authorised at the document level', function () {
+        return it('should not allow access', function () {
+          return this.AuthorizationManager.assertClientCanReviewProjectAndDoc(
+            this.client,
+            this.doc_id,
+            err => err.message.should.equal('not authorized')
+          )
+        })
+      })
+
+      describe('and authorised at the document level', function () {
+        beforeEach(function (done) {
+          return this.AuthorizationManager.addAccessToDoc(
+            this.client,
+            this.doc_id,
+            done
+          )
+        })
+
+        return it('should allow access', function () {
+          this.AuthorizationManager.assertClientCanReviewProjectAndDoc(
+            this.client,
+            this.doc_id,
+            this.callback
+          )
+          return this.callback.calledWith(null).should.equal(true)
+        })
+      })
+
+      return describe('when document authorisation is added and then removed', function () {
+        beforeEach(function (done) {
+          return this.AuthorizationManager.addAccessToDoc(
+            this.client,
+            this.doc_id,
+            () => {
+              return this.AuthorizationManager.removeAccessToDoc(
+                this.client,
+                this.doc_id,
+                done
+              )
+            }
+          )
+        })
+
+        return it('should deny access', function () {
+          return this.AuthorizationManager.assertClientCanReviewProjectAndDoc(
+            this.client,
+            this.doc_id,
+            err => err.message.should.equal('not authorized')
+          )
+        })
+      })
+    })
+  })
 })

+ 43 - 1
services/real-time/test/unit/js/WebsocketControllerTests.js

@@ -1578,6 +1578,8 @@ describe('WebsocketController', function () {
       } // comments may still be in an edit op
       this.comment_update = { op: [{ c: 'bar', p: 132 }] }
       this.AuthorizationManager.assertClientCanEditProjectAndDoc = sinon.stub()
+      this.AuthorizationManager.assertClientCanReviewProjectAndDoc =
+        sinon.stub()
       return (this.AuthorizationManager.assertClientCanViewProjectAndDoc =
         sinon.stub())
     })
@@ -1633,7 +1635,7 @@ describe('WebsocketController', function () {
       })
     })
 
-    return describe('with a totally unauthorized client', function () {
+    describe('with a totally unauthorized client', function () {
       return it('should return an error', function (done) {
         this.AuthorizationManager.assertClientCanEditProjectAndDoc.yields(
           new Error('not authorized')
@@ -1652,5 +1654,45 @@ describe('WebsocketController', function () {
         )
       })
     })
+
+    describe('with a review client', function () {
+      it('op with tc should succeed', function (done) {
+        this.AuthorizationManager.assertClientCanEditProjectAndDoc.yields(
+          new Error('not authorized')
+        )
+        this.AuthorizationManager.assertClientCanViewProjectAndDoc.yields(null)
+        this.AuthorizationManager.assertClientCanReviewProjectAndDoc.yields(
+          null
+        )
+        return this.WebsocketController._assertClientCanApplyUpdate(
+          this.client,
+          this.doc_id,
+          { op: [{ p: 10, i: 'a' }], meta: { tc: '123456' } },
+          error => {
+            expect(error).to.be.null
+            return done()
+          }
+        )
+      })
+
+      return it('op without tc should fail', function (done) {
+        this.AuthorizationManager.assertClientCanEditProjectAndDoc.yields(
+          new Error('not authorized')
+        )
+        this.AuthorizationManager.assertClientCanViewProjectAndDoc.yields(null)
+        this.AuthorizationManager.assertClientCanReviewProjectAndDoc.yields(
+          null
+        )
+        return this.WebsocketController._assertClientCanApplyUpdate(
+          this.client,
+          this.doc_id,
+          { op: [{ p: 10, i: 'a' }] },
+          error => {
+            expect(error.message).to.equal('not authorized')
+            return done()
+          }
+        )
+      })
+    })
   })
 })