From f53ab2cb30485ba5ed756774be2dc24e89a127e3 Mon Sep 17 00:00:00 2001
From: Paul Fitzpatrick <paulfitz@alum.mit.edu>
Date: Fri, 20 Aug 2021 15:23:27 -0400
Subject: [PATCH] (core) forbids edits when "view as" user is a viewer and
 access rules are permissive

Summary:
Currently, if access rules are set to allow edits unconditionally,
and an owner does "View As" a user who is a viewer only, they will
be allowed to make edits.  This catches that condition and adds a
test.

Test Plan: added test

Reviewers: georgegevoian

Reviewed By: georgegevoian

Differential Revision: https://phab.getgrist.com/D2991
---
 app/server/lib/GranularAccess.ts | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/app/server/lib/GranularAccess.ts b/app/server/lib/GranularAccess.ts
index ab69cb51..f6912369 100644
--- a/app/server/lib/GranularAccess.ts
+++ b/app/server/lib/GranularAccess.ts
@@ -15,7 +15,7 @@ import { AclMatchInput, InfoEditor, InfoView } from 'app/common/GranularAccessCl
 import { UserInfo } from 'app/common/GranularAccessClause';
 import { isCensored } from 'app/common/gristTypes';
 import { getSetMapValue, isObject, pruneArray } from 'app/common/gutil';
-import { canView, Role } from 'app/common/roles';
+import { canEdit, canView, Role } from 'app/common/roles';
 import { FullUser } from 'app/common/UserAPI';
 import { HomeDBManager } from 'app/gen-server/lib/HomeDBManager';
 import { compileAclFormula } from 'app/server/lib/ACLFormula';
@@ -232,6 +232,16 @@ export class GranularAccess implements GranularAccessForBundle {
     if (this._activeBundle.hasDeliberateRuleChange && !await this.isOwner(docSession)) {
       throw new ErrorWithCode('ACL_DENY', 'Only owners can modify access rules');
     }
+    // Normally, viewer requests would never reach this point, but they can happen
+    // using the "view as" functionality where user is an owner wanting to preview the
+    // access level of another.  And again, the default access rules would normally
+    // forbid edit access to a viewer - but that can be overridden.
+    // An alternative to this check would be to sandwich user-defined access rules
+    // between some defaults.  Currently the defaults have lower priority than
+    // user-defined access rules.
+    if (!canEdit(await this._getNominalAccess(docSession))) {
+      throw new ErrorWithCode('ACL_DENY', 'Only owners or editors can modify documents');
+    }
     if (this._ruler.haveRules()) {
       await Promise.all(
         docActions.map((action, actionIdx) => {