feat: Read-only users (#1955)

* Introduce isViewer field

* Update policies

* Make users read-only feature

* Remove not demoting current user validation

* Update tests

* Catch the unhandled promise rejection

* Hide unnecessary ui elements for read-only user

* Update app/scenes/Settings/People.js

Co-authored-by: Tom Moor <tom.moor@gmail.com>

* Remove redundant logic for admin only policies

* Use can logic

* Update snapshot

* Remove lint error

* Update snapshot

* Minor fix

* Update app/menus/UserMenu.js

Co-authored-by: Tom Moor <tom.moor@gmail.com>

* Update server/api/users.js

Co-authored-by: Tom Moor <tom.moor@gmail.com>

* Update app/components/DocumentListItem.js

Co-authored-by: Tom Moor <tom.moor@gmail.com>

* Update app/stores/UsersStore.js

Co-authored-by: Tom Moor <tom.moor@gmail.com>

* Use useCurrentTeam hook in functional component

* Update translation

* Update ternary

* Remove punctuation

* Move the functions to User model

* Update share policy and shareMenu

* Rename makeAdmin to promote

* Create updateCounts function and Rank enum

* Update tests

* Remove enum

* Use async await, remove enum and create computed accessor

* Remove unused variable

* Fix lint issues

* Hide templates

* Create shared/types and use rank type from it

* Delete shared/utils/rank type file

Co-authored-by: Tom Moor <tom.moor@gmail.com>

server/api/__snapshots__/users.test.js.snap

@@ -9,6 +9,7 @@ Object {
     "id": "46fde1d4-0050-428f-9f0b-0bf77f4bdf61",
     "isAdmin": false,
     "isSuspended": false,
+    "isViewer": false,
     "language": "en_US",
     "lastActiveAt": null,
     "name": "User 1",
@@ -19,7 +20,7 @@ Object {
       "abilities": Object {
         "activate": true,
         "delete": true,
-        "demote": false,
+        "demote": true,
         "promote": true,
         "read": true,
         "suspend": true,
@@ -59,6 +60,7 @@ Object {
     "id": "46fde1d4-0050-428f-9f0b-0bf77f4bdf61",
     "isAdmin": false,
     "isSuspended": false,
+    "isViewer": false,
     "language": "en_US",
     "lastActiveAt": null,
     "name": "User 1",
@@ -69,7 +71,73 @@ Object {
       "abilities": Object {
         "activate": true,
         "delete": true,
-        "demote": false,
+        "demote": true,
+        "promote": true,
+        "read": true,
+        "suspend": true,
+        "update": false,
+      },
+      "id": "46fde1d4-0050-428f-9f0b-0bf77f4bdf61",
+    },
+  ],
+  "status": 200,
+}
+`;
+
+exports[`#users.demote should demote an admin to viewer 1`] = `
+Object {
+  "data": Object {
+    "avatarUrl": "https://tiley.herokuapp.com/avatar/111d68d06e2d317b5a59c2c6c5bad808/U.png",
+    "createdAt": "2018-01-02T00:00:00.000Z",
+    "email": "user1@example.com",
+    "id": "46fde1d4-0050-428f-9f0b-0bf77f4bdf61",
+    "isAdmin": false,
+    "isSuspended": false,
+    "isViewer": true,
+    "language": "en_US",
+    "lastActiveAt": null,
+    "name": "User 1",
+  },
+  "ok": true,
+  "policies": Array [
+    Object {
+      "abilities": Object {
+        "activate": true,
+        "delete": true,
+        "demote": true,
+        "promote": true,
+        "read": true,
+        "suspend": true,
+        "update": false,
+      },
+      "id": "46fde1d4-0050-428f-9f0b-0bf77f4bdf61",
+    },
+  ],
+  "status": 200,
+}
+`;
+
+exports[`#users.demote should demote an admin to member 1`] = `
+Object {
+  "data": Object {
+    "avatarUrl": "https://tiley.herokuapp.com/avatar/111d68d06e2d317b5a59c2c6c5bad808/U.png",
+    "createdAt": "2018-01-02T00:00:00.000Z",
+    "email": "user1@example.com",
+    "id": "46fde1d4-0050-428f-9f0b-0bf77f4bdf61",
+    "isAdmin": false,
+    "isSuspended": false,
+    "isViewer": false,
+    "language": "en_US",
+    "lastActiveAt": null,
+    "name": "User 1",
+  },
+  "ok": true,
+  "policies": Array [
+    Object {
+      "abilities": Object {
+        "activate": true,
+        "delete": true,
+        "demote": true,
         "promote": true,
         "read": true,
         "suspend": true,
@@ -109,6 +177,7 @@ Object {
     "id": "46fde1d4-0050-428f-9f0b-0bf77f4bdf61",
     "isAdmin": true,
     "isSuspended": false,
+    "isViewer": false,
     "language": "en_US",
     "lastActiveAt": null,
     "name": "User 1",
@@ -168,6 +237,7 @@ Object {
     "id": "46fde1d4-0050-428f-9f0b-0bf77f4bdf61",
     "isAdmin": false,
     "isSuspended": true,
+    "isViewer": false,
     "language": "en_US",
     "lastActiveAt": null,
     "name": "User 1",

server/api/users.js

@@ -116,8 +116,7 @@ router.post("users.promote", auth(), async (ctx) => {
   const user = await User.findByPk(userId);
   authorize(actor, "promote", user);
 
-  const team = await Team.findByPk(teamId);
-  await team.addAdmin(user);
+  await user.promote();
 
   await Event.create({
     name: "users.promote",
@@ -137,14 +136,18 @@ router.post("users.promote", auth(), async (ctx) => {
 router.post("users.demote", auth(), async (ctx) => {
   const userId = ctx.body.id;
   const teamId = ctx.state.user.teamId;
+  let { to } = ctx.body;
+
   const actor = ctx.state.user;
   ctx.assertPresent(userId, "id is required");
 
+  to = to === "Viewer" ? "Viewer" : "Member";
+
   const user = await User.findByPk(userId);
+
   authorize(actor, "demote", user);
 
-  const team = await Team.findByPk(teamId);
-  await team.removeAdmin(user);
+  await user.demote(teamId, to);
 
   await Event.create({
     name: "users.demote",
@@ -190,8 +193,7 @@ router.post("users.activate", auth(), async (ctx) => {
   const user = await User.findByPk(userId);
   authorize(actor, "activate", user);
 
-  const team = await Team.findByPk(teamId);
-  await team.activateUser(user, actor);
+  await user.activate();
 
   await Event.create({
     name: "users.activate",

server/api/users.test.js

@@ -264,6 +264,40 @@ describe("#users.demote", () => {
     expect(body).toMatchSnapshot();
   });
 
+  it("should demote an admin to viewer", async () => {
+    const { admin, user } = await seed();
+    await user.update({ isAdmin: true }); // Make another admin
+
+    const res = await server.post("/api/users.demote", {
+      body: {
+        token: admin.getJwtToken(),
+        id: user.id,
+        to: "Viewer",
+      },
+    });
+    const body = await res.json();
+
+    expect(res.status).toEqual(200);
+    expect(body).toMatchSnapshot();
+  });
+
+  it("should demote an admin to member", async () => {
+    const { admin, user } = await seed();
+    await user.update({ isAdmin: true }); // Make another admin
+
+    const res = await server.post("/api/users.demote", {
+      body: {
+        token: admin.getJwtToken(),
+        id: user.id,
+        to: "Member",
+      },
+    });
+    const body = await res.json();
+
+    expect(res.status).toEqual(200);
+    expect(body).toMatchSnapshot();
+  });
+
   it("should not demote admins if only one available", async () => {
     const admin = await buildAdmin();
 

server/migrations/20210314173941-isViewer.js

@@ -0,0 +1,15 @@
+'use strict';
+
+module.exports = {
+  up: async (queryInterface, Sequelize) => {
+     await queryInterface.addColumn("users", "isViewer", {
+      type: Sequelize.BOOLEAN,
+      defaultValue: false,
+      allowNull: false,
+    });
+  },
+
+  down: async (queryInterface, Sequelize) => {
+    await queryInterface.removeColumn("users", "isViewer");
+  }
+};

server/models/Team.js

@@ -8,14 +8,12 @@ import {
   stripSubdomain,
   RESERVED_SUBDOMAINS,
 } from "../../shared/utils/domains";
-import { ValidationError } from "../errors";
 import { DataTypes, sequelize, Op } from "../sequelize";
 import { generateAvatarUrl } from "../utils/avatars";
 import { publicS3Endpoint, uploadToS3FromUrl } from "../utils/s3";
 
 import Collection from "./Collection";
 import Document from "./Document";
-import User from "./User";
 
 const readFile = util.promisify(fs.readFile);
 
@@ -194,35 +192,6 @@ Team.prototype.provisionFirstCollection = async function (userId) {
   }
 };
 
-Team.prototype.addAdmin = async function (user: User) {
-  return user.update({ isAdmin: true });
-};
-
-Team.prototype.removeAdmin = async function (user: User) {
-  const res = await User.findAndCountAll({
-    where: {
-      teamId: this.id,
-      isAdmin: true,
-      id: {
-        [Op.ne]: user.id,
-      },
-    },
-    limit: 1,
-  });
-  if (res.count >= 1) {
-    return user.update({ isAdmin: false });
-  } else {
-    throw new ValidationError("At least one admin is required");
-  }
-};
-
-Team.prototype.activateUser = async function (user: User, admin: User) {
-  return user.update({
-    suspendedById: null,
-    suspendedAt: null,
-  });
-};
-
 Team.prototype.collectionIds = async function (paranoid: boolean = true) {
   let models = await Collection.findAll({
     attributes: ["id"],

server/models/User.js

@@ -7,7 +7,7 @@ import uuid from "uuid";
 import { languages } from "../../shared/i18n";
 import { ValidationError } from "../errors";
 import { sendEmail } from "../mailer";
-import { DataTypes, sequelize, encryptedFields } from "../sequelize";
+import { DataTypes, sequelize, encryptedFields, Op } from "../sequelize";
 import { DEFAULT_AVATAR_HOST } from "../utils/avatars";
 import { publicS3Endpoint, uploadToS3FromUrl } from "../utils/s3";
 import { Star, Team, Collection, NotificationSetting, ApiKey } from ".";
@@ -25,6 +25,11 @@ const User = sequelize.define(
     name: DataTypes.STRING,
     avatarUrl: { type: DataTypes.STRING, allowNull: true },
     isAdmin: DataTypes.BOOLEAN,
+    isViewer: {
+      type: DataTypes.BOOLEAN,
+      defaultValue: false,
+      allowNull: false,
+    },
     service: { type: DataTypes.STRING, allowNull: true },
     serviceId: { type: DataTypes.STRING, allowNull: true, unique: true },
     jwtSecret: encryptedFields().vault("jwtSecret"),
@@ -277,6 +282,7 @@ User.getCounts = async function (teamId: string) {
     SELECT 
       COUNT(CASE WHEN "suspendedAt" IS NOT NULL THEN 1 END) as "suspendedCount",
       COUNT(CASE WHEN "isAdmin" = true THEN 1 END) as "adminCount",
+      COUNT(CASE WHEN "isViewer" = true THEN 1 END) as "viewerCount",
       COUNT(CASE WHEN "lastActiveAt" IS NULL THEN 1 END) as "invitedCount",
       COUNT(CASE WHEN "suspendedAt" IS NULL AND "lastActiveAt" IS NOT NULL THEN 1 END) as "activeCount",
       COUNT(*) as count
@@ -295,10 +301,48 @@ User.getCounts = async function (teamId: string) {
   return {
     active: parseInt(counts.activeCount),
     admins: parseInt(counts.adminCount),
+    viewers: parseInt(counts.viewerCount),
     all: parseInt(counts.count),
     invited: parseInt(counts.invitedCount),
     suspended: parseInt(counts.suspendedCount),
   };
 };
 
+User.prototype.demote = async function (
+  teamId: string,
+  to: "Member" | "Viewer"
+) {
+  const res = await User.findAndCountAll({
+    where: {
+      teamId,
+      isAdmin: true,
+      id: {
+        [Op.ne]: this.id,
+      },
+    },
+    limit: 1,
+  });
+
+  if (res.count >= 1) {
+    if (to === "Member") {
+      return this.update({ isAdmin: false, isViewer: false });
+    } else if (to === "Viewer") {
+      return this.update({ isAdmin: false, isViewer: true });
+    }
+  } else {
+    throw new ValidationError("At least one admin is required");
+  }
+};
+
+User.prototype.promote = async function () {
+  return this.update({ isAdmin: true, isViewer: false });
+};
+
+User.prototype.activate = async function () {
+  return this.update({
+    suspendedById: null,
+    suspendedAt: null,
+  });
+};
+
 export default User;

server/policies/apiKey.js

@@ -5,13 +5,11 @@ import policy from "./policy";
 const { allow } = policy;
 
 allow(User, "createApiKey", Team, (user, team) => {
-  if (!team || user.teamId !== team.id) return false;
+  if (!team || user.isViewer || user.teamId !== team.id) return false;
   return true;
 });
 
-allow(
-  User,
-  ["read", "update", "delete"],
-  ApiKey,
-  (user, apiKey) => user && user.id === apiKey.userId
-);
+allow(User, ["read", "update", "delete"], ApiKey, (user, apiKey) => {
+  if (user.isViewer) return false;
+  return user && user.id === apiKey.userId;
+});

server/policies/attachment.js

@@ -5,11 +5,19 @@ import policy from "./policy";
 const { allow } = policy;
 
 allow(User, "createAttachment", Team, (user, team) => {
-  if (!team || user.teamId !== team.id) return false;
+  if (!team || user.isViewer || user.teamId !== team.id) return false;
   return true;
 });
 
-allow(User, ["read", "delete"], Attachment, (actor, attachment) => {
+allow(User, "read", Attachment, (actor, attachment) => {
+  if (!attachment || attachment.teamId !== actor.teamId) return false;
+  if (actor.isAdmin) return true;
+  if (actor.id === attachment.userId) return true;
+  return false;
+});
+
+allow(User, "delete", Attachment, (actor, attachment) => {
+  if (actor.isViewer) return false;
   if (!attachment || attachment.teamId !== actor.teamId) return false;
   if (actor.isAdmin) return true;
   if (actor.id === attachment.userId) return true;

server/policies/collection.js

@@ -8,7 +8,7 @@ import policy from "./policy";
 const { allow } = policy;
 
 allow(User, "createCollection", Team, (user, team) => {
-  if (!team || user.teamId !== team.id) return false;
+  if (!team || user.isViewer || user.teamId !== team.id) return false;
   return true;
 });
 
@@ -48,6 +48,7 @@ allow(User, ["read", "export"], Collection, (user, collection) => {
 });
 
 allow(User, "share", Collection, (user, collection) => {
+  if (user.isViewer) return false;
   if (!collection || user.teamId !== collection.teamId) return false;
   if (!collection.sharing) return false;
 
@@ -71,6 +72,7 @@ allow(User, "share", Collection, (user, collection) => {
 });
 
 allow(User, ["publish", "update"], Collection, (user, collection) => {
+  if (user.isViewer) return false;
   if (!collection || user.teamId !== collection.teamId) return false;
 
   if (collection.permission !== "read_write") {
@@ -93,6 +95,7 @@ allow(User, ["publish", "update"], Collection, (user, collection) => {
 });
 
 allow(User, "delete", Collection, (user, collection) => {
+  if (user.isViewer) return false;
   if (!collection || user.teamId !== collection.teamId) return false;
 
   if (collection.permission !== "read_write") {

server/policies/document.js

@@ -6,7 +6,7 @@ import policy from "./policy";
 const { allow, cannot } = policy;
 
 allow(User, "createDocument", Team, (user, team) => {
-  if (!team || user.teamId !== team.id) return false;
+  if (!team || user.isViewer || user.teamId !== team.id) return false;
   return true;
 });
 
@@ -102,6 +102,7 @@ allow(User, ["pin", "unpin"], Document, (user, document) => {
 
 allow(User, "delete", Document, (user, document) => {
   // unpublished drafts can always be deleted
+  if (user.isViewer) return false;
   if (
     !document.deletedAt &&
     !document.publishedAt &&
@@ -121,6 +122,7 @@ allow(User, "delete", Document, (user, document) => {
 });
 
 allow(User, "restore", Document, (user, document) => {
+  if (user.isViewer) return false;
   if (!document.deletedAt) return false;
   return user.teamId === document.teamId;
 });

server/policies/group.js

@@ -6,7 +6,7 @@ import policy from "./policy";
 const { allow } = policy;
 
 allow(User, "createGroup", Team, (actor, team) => {
-  if (!team || actor.teamId !== team.id) return false;
+  if (!team || actor.isViewer || actor.teamId !== team.id) return false;
   if (actor.isAdmin) return true;
   throw new AdminRequiredError();
 });
@@ -21,7 +21,7 @@ allow(User, "read", Group, (actor, group) => {
 });
 
 allow(User, ["update", "delete"], Group, (actor, group) => {
-  if (!group || actor.teamId !== group.teamId) return false;
+  if (!group || actor.isViewer || actor.teamId !== group.teamId) return false;
   if (actor.isAdmin) return true;
   throw new AdminRequiredError();
 });

server/policies/integration.js

@@ -6,7 +6,7 @@ import policy from "./policy";
 const { allow } = policy;
 
 allow(User, "createIntegration", Team, (actor, team) => {
-  if (!team || actor.teamId !== team.id) return false;
+  if (!team || actor.isViewer || actor.teamId !== team.id) return false;
   if (actor.isAdmin) return true;
   throw new AdminRequiredError();
 });
@@ -19,6 +19,7 @@ allow(
 );
 
 allow(User, ["update", "delete"], Integration, (user, integration) => {
+  if (user.isViewer) return false;
   if (!integration || user.teamId !== integration.teamId) return false;
   if (user.isAdmin) return true;
   throw new AdminRequiredError();

server/policies/share.js

@@ -5,14 +5,17 @@ import policy from "./policy";
 
 const { allow } = policy;
 
-allow(
-  User,
-  ["read", "update"],
-  Share,
-  (user, share) => user.teamId === share.teamId
-);
+allow(User, "read", Share, (user, share) => {
+  return user.teamId === share.teamId;
+});
+
+allow(User, "update", Share, (user, share) => {
+  if (user.isViewer) return false;
+  return user.teamId === share.teamId;
+});
 
 allow(User, "revoke", Share, (user, share) => {
+  if (user.isViewer) return false;
   if (!share || user.teamId !== share.teamId) return false;
   if (user.id === share.userId) return true;
   if (user.isAdmin) return true;

server/policies/team.js

@@ -7,11 +7,11 @@ const { allow } = policy;
 allow(User, "read", Team, (user, team) => team && user.teamId === team.id);
 
 allow(User, "share", Team, (user, team) => {
-  if (!team || user.teamId !== team.id) return false;
+  if (!team || user.isViewer || user.teamId !== team.id) return false;
   return team.sharing;
 });
 
 allow(User, ["update", "export", "manage"], Team, (user, team) => {
-  if (!team || user.teamId !== team.id) return false;
+  if (!team || user.isViewer || user.teamId !== team.id) return false;
   return user.isAdmin;
 });

server/policies/user.js

@@ -46,7 +46,7 @@ allow(User, "promote", User, (actor, user) => {
 
 allow(User, "demote", User, (actor, user) => {
   if (!user || user.teamId !== actor.teamId) return false;
-  if (!user.isAdmin || user.isSuspended) return false;
+  if (user.isSuspended) return false;
   if (actor.isAdmin) return true;
   throw new AdminRequiredError();
 });

server/presenters/__snapshots__/user.test.js.snap

@@ -7,6 +7,7 @@ Object {
   "id": "123",
   "isAdmin": undefined,
   "isSuspended": undefined,
+  "isViewer": undefined,
   "language": "en_US",
   "lastActiveAt": undefined,
   "name": "Test User",
@@ -20,6 +21,7 @@ Object {
   "id": "123",
   "isAdmin": undefined,
   "isSuspended": undefined,
+  "isViewer": undefined,
   "language": "en_US",
   "lastActiveAt": undefined,
   "name": "Test User",

server/presenters/user.js

@@ -12,6 +12,7 @@ type UserPresentation = {
   email?: string,
   isAdmin: boolean,
   isSuspended: boolean,
+  isViewer: boolean,
   language: string,
 };
 
@@ -22,6 +23,7 @@ export default (user: User, options: Options = {}): ?UserPresentation => {
   userData.lastActiveAt = user.lastActiveAt;
   userData.name = user.name;
   userData.isAdmin = user.isAdmin;
+  userData.isViewer = user.isViewer;
   userData.isSuspended = user.isSuspended;
   userData.avatarUrl = user.avatarUrl;
   userData.language = user.language || process.env.DEFAULT_LANGUAGE || "en_US";