From 3610a7f4a234a8e81c9014009f626df08e2783f9 Mon Sep 17 00:00:00 2001
From: Saumya Pandey <sp160899@gmail.com>
Date: Wed, 20 Oct 2021 09:26:11 +0530
Subject: [PATCH] fix: Add default role option for new users (#2665)

* Add defaultUserRole on server

* Handle defaultUserRole on frontend

* Handle tests

* Handle user role in userCreator

* Minor improvments

* Fix prettier issue

* Undefined when isNewTeam is false

* Update app/scenes/Settings/Security.js

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

* Update app/scenes/Settings/Security.js

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

* Update app/scenes/Settings/Security.js

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

* Remove duplicate validation

* Update Team.js

* fix: Move note out of restricted width wrapper

* Move language setting to use 'note' prop

* Remove admin option

Co-authored-by: Tom Moor <tom.moor@gmail.com>
---
 app/components/InputSelect.js                 |  6 ++
 app/models/Team.js                            |  1 +
 app/scenes/Settings/Profile.js                | 29 ++++----
 app/scenes/Settings/Security.js               | 39 ++++++++--
 server/commands/accountProvisioner.js         |  2 +-
 server/commands/userCreator.js                | 10 ++-
 server/commands/userCreator.test.js           | 74 +++++++++++++++++++
 .../20211015170955-add-defaultUserRole.js     | 15 ++++
 server/models/Team.js                         | 11 +++
 server/presenters/team.js                     |  1 +
 server/routes/api/team.js                     |  4 +
 server/routes/api/team.test.js                | 17 +++++
 shared/i18n/locales/en_US/translation.json    |  3 +
 13 files changed, 189 insertions(+), 23 deletions(-)
 create mode 100644 server/migrations/20211015170955-add-defaultUserRole.js

diff --git a/app/components/InputSelect.js b/app/components/InputSelect.js
index 5198c800..b59367c4 100644
--- a/app/components/InputSelect.js
+++ b/app/components/InputSelect.js
@@ -12,6 +12,7 @@ import { VisuallyHidden } from "reakit/VisuallyHidden";
 import scrollIntoView from "smooth-scroll-into-view-if-needed";
 import styled, { css } from "styled-components";
 import Button, { Inner } from "components/Button";
+import HelpText from "components/HelpText";
 import { Position, Background, Backdrop } from "./ContextMenu";
 import { MenuAnchorCSS } from "./ContextMenu/MenuItem";
 import { LabelText } from "./Input";
@@ -30,6 +31,7 @@ export type Props = {
   labelHidden?: boolean,
   icon?: React.Node,
   options: Option[],
+  note?: React.Node,
   onChange: (string) => Promise<void> | void,
 };
 
@@ -49,6 +51,7 @@ const InputSelect = (props: Props) => {
     onChange,
     disabled,
     nude,
+    note,
     icon,
   } = props;
 
@@ -124,6 +127,7 @@ const InputSelect = (props: Props) => {
           ) : (
             wrappedLabel
           ))}
+
         <Select
           {...select}
           disabled={disabled}
@@ -201,6 +205,8 @@ const InputSelect = (props: Props) => {
           }}
         </SelectPopover>
       </Wrapper>
+      {note && <HelpText small>{note}</HelpText>}
+
       {(select.visible || select.animating) && <Backdrop />}
     </>
   );
diff --git a/app/models/Team.js b/app/models/Team.js
index b0865b8e..5d15e303 100644
--- a/app/models/Team.js
+++ b/app/models/Team.js
@@ -13,6 +13,7 @@ class Team extends BaseModel {
   subdomain: ?string;
   domain: ?string;
   url: string;
+  defaultUserRole: string;
 
   @computed
   get signinMethods(): string {
diff --git a/app/scenes/Settings/Profile.js b/app/scenes/Settings/Profile.js
index 7770ee5d..bbe9bacd 100644
--- a/app/scenes/Settings/Profile.js
+++ b/app/scenes/Settings/Profile.js
@@ -107,23 +107,22 @@ const Profile = () => {
           value={language}
           onChange={handleLanguageChange}
           ariaLabel={t("Language")}
+          note={
+            <Trans>
+              Please note that translations are currently in early access.
+              <br />
+              Community contributions are accepted though our{" "}
+              <a
+                href="https://translate.getoutline.com"
+                target="_blank"
+                rel="noreferrer"
+              >
+                translation portal
+              </a>
+            </Trans>
+          }
           short
         />
-        <HelpText small>
-          <Trans>
-            Please note that translations are currently in early access.
-            <br />
-            Community contributions are accepted though our{" "}
-            <a
-              href="https://translate.getoutline.com"
-              target="_blank"
-              rel="noreferrer"
-            >
-              translation portal
-            </a>
-          </Trans>
-          .
-        </HelpText>
         <Button type="submit" disabled={isSaving || !isValid}>
           {isSaving ? `${t("Saving")}…` : t("Save")}
         </Button>
diff --git a/app/scenes/Settings/Security.js b/app/scenes/Settings/Security.js
index fbc96084..b20bf306 100644
--- a/app/scenes/Settings/Security.js
+++ b/app/scenes/Settings/Security.js
@@ -8,6 +8,7 @@ import { useTranslation, Trans } from "react-i18next";
 import Checkbox from "components/Checkbox";
 import Heading from "components/Heading";
 import HelpText from "components/HelpText";
+import InputSelect from "components/InputSelect";
 import Scene from "components/Scene";
 import env from "env";
 import useCurrentTeam from "hooks/useCurrentTeam";
@@ -23,13 +24,20 @@ function Security() {
     sharing: team.sharing,
     documentEmbeds: team.documentEmbeds,
     guestSignin: team.guestSignin,
+    defaultUserRole: team.defaultUserRole,
   });
 
-  const showSuccessMessage = React.useCallback(
-    debounce(() => {
-      showToast(t("Settings saved"), { type: "success" });
-    }, 250),
-    [t, showToast]
+  const notes = {
+    member: t("New user accounts will be given member permissions by default"),
+    viewer: t("New user accounts will be given viewer permissions by default"),
+  };
+
+  const showSuccessMessage = React.useMemo(
+    () =>
+      debounce(() => {
+        showToast(t("Settings saved"), { type: "success" });
+      }, 250),
+    [showToast, t]
   );
 
   const handleChange = React.useCallback(
@@ -44,6 +52,15 @@ function Security() {
     [auth, data, showSuccessMessage]
   );
 
+  const handleDefaultRoleChange = async (newDefaultRole: string) => {
+    const newData = { ...data, defaultUserRole: newDefaultRole };
+    setData(newData);
+
+    await auth.updateTeam(newData);
+
+    showSuccessMessage();
+  };
+
   return (
     <Scene title={t("Security")} icon={<PadlockIcon color="currentColor" />}>
       <Heading>
@@ -86,6 +103,18 @@ function Security() {
           "Links to supported services are shown as rich embeds within your documents"
         )}
       />
+      <InputSelect
+        value={data.defaultUserRole}
+        label="Default role"
+        options={[
+          { label: t("Member"), value: "member" },
+          { label: t("Viewer"), value: "viewer" },
+        ]}
+        onChange={handleDefaultRoleChange}
+        ariaLabel={t("Default role")}
+        note={notes[data.defaultUserRole]}
+        short
+      />
     </Scene>
   );
 }
diff --git a/server/commands/accountProvisioner.js b/server/commands/accountProvisioner.js
index 55b08ee6..f8508623 100644
--- a/server/commands/accountProvisioner.js
+++ b/server/commands/accountProvisioner.js
@@ -76,7 +76,7 @@ export default async function accountProvisioner({
       name: userParams.name,
       email: userParams.email,
       username: userParams.username,
-      isAdmin: isNewTeam,
+      isAdmin: isNewTeam || undefined,
       avatarUrl: userParams.avatarUrl,
       teamId: team.id,
       ip,
diff --git a/server/commands/userCreator.js b/server/commands/userCreator.js
index 31fdeb37..159862ab 100644
--- a/server/commands/userCreator.js
+++ b/server/commands/userCreator.js
@@ -1,6 +1,6 @@
 // @flow
 import Sequelize from "sequelize";
-import { Event, User, UserAuthentication } from "../models";
+import { Event, Team, User, UserAuthentication } from "../models";
 import { sequelize } from "../sequelize";
 
 const Op = Sequelize.Op;
@@ -126,12 +126,18 @@ export default async function userCreator({
   let transaction = await sequelize.transaction();
 
   try {
+    const { defaultUserRole } = await Team.findByPk(teamId, {
+      attributes: ["defaultUserRole"],
+      transaction,
+    });
+
     const user = await User.create(
       {
         name,
         email,
         username,
-        isAdmin,
+        isAdmin: typeof isAdmin === "boolean" && isAdmin,
+        isViewer: isAdmin === true ? false : defaultUserRole === "viewer",
         teamId,
         avatarUrl,
         service: null,
diff --git a/server/commands/userCreator.test.js b/server/commands/userCreator.test.js
index 3d37191b..3c8cd122 100644
--- a/server/commands/userCreator.test.js
+++ b/server/commands/userCreator.test.js
@@ -122,9 +122,83 @@ describe("userCreator", () => {
     expect(authentication.scopes[0]).toEqual("read");
     expect(user.email).toEqual("test@example.com");
     expect(user.username).toEqual("tname");
+    expect(user.isAdmin).toEqual(false);
+    expect(user.isViewer).toEqual(false);
     expect(isNewUser).toEqual(true);
   });
 
+  it("should prefer isAdmin argument over defaultUserRole", async () => {
+    const team = await buildTeam({ defaultUserRole: "viewer" });
+    const authenticationProviders = await team.getAuthenticationProviders();
+    const authenticationProvider = authenticationProviders[0];
+
+    const result = await userCreator({
+      name: "Test Name",
+      email: "test@example.com",
+      username: "tname",
+      teamId: team.id,
+      isAdmin: true,
+      ip,
+      authentication: {
+        authenticationProviderId: authenticationProvider.id,
+        providerId: "fake-service-id",
+        accessToken: "123",
+        scopes: ["read"],
+      },
+    });
+
+    const { user } = result;
+
+    expect(user.isAdmin).toEqual(true);
+  });
+
+  it("should prefer defaultUserRole when isAdmin is undefined or false", async () => {
+    const team = await buildTeam({ defaultUserRole: "viewer" });
+    const authenticationProviders = await team.getAuthenticationProviders();
+    const authenticationProvider = authenticationProviders[0];
+
+    const result = await userCreator({
+      name: "Test Name",
+      email: "test@example.com",
+      username: "tname",
+      teamId: team.id,
+      ip,
+      authentication: {
+        authenticationProviderId: authenticationProvider.id,
+        providerId: "fake-service-id",
+        accessToken: "123",
+        scopes: ["read"],
+      },
+    });
+
+    const { user: tname } = result;
+
+    expect(tname.username).toEqual("tname");
+    expect(tname.isAdmin).toEqual(false);
+    expect(tname.isViewer).toEqual(true);
+
+    const tname2Result = await userCreator({
+      name: "Test2 Name",
+      email: "tes2@example.com",
+      username: "tname2",
+      teamId: team.id,
+      isAdmin: false,
+      ip,
+      authentication: {
+        authenticationProviderId: authenticationProvider.id,
+        providerId: "fake-service-id",
+        accessToken: "123",
+        scopes: ["read"],
+      },
+    });
+
+    const { user: tname2 } = tname2Result;
+
+    expect(tname2.username).toEqual("tname2");
+    expect(tname2.isAdmin).toEqual(false);
+    expect(tname2.isViewer).toEqual(true);
+  });
+
   it("should create a user from an invited user", async () => {
     const team = await buildTeam();
     const invite = await buildInvite({ teamId: team.id });
diff --git a/server/migrations/20211015170955-add-defaultUserRole.js b/server/migrations/20211015170955-add-defaultUserRole.js
new file mode 100644
index 00000000..67a4e73e
--- /dev/null
+++ b/server/migrations/20211015170955-add-defaultUserRole.js
@@ -0,0 +1,15 @@
+'use strict';
+
+module.exports = {
+  up: async (queryInterface, Sequelize) => {
+    await queryInterface.addColumn("teams", "defaultUserRole", {
+      type: Sequelize.STRING,
+      defaultValue: "member",
+      allowNull: false,
+    });
+  },
+
+  down: async (queryInterface, Sequelize) => {
+    await queryInterface.removeColumn("teams", "defaultUserRole");
+  }
+};
diff --git a/server/models/Team.js b/server/models/Team.js
index c796e3db..a87b4ded 100644
--- a/server/models/Team.js
+++ b/server/models/Team.js
@@ -75,6 +75,17 @@ const Team = sequelize.define(
       allowNull: false,
       defaultValue: false,
     },
+    defaultUserRole: {
+      type: DataTypes.STRING,
+      defaultValue: "member",
+      allowNull: false,
+      validate: {
+        isIn: {
+          args: [["viewer", "member"]],
+          msg: "Must be 'viewer' or 'member'",
+        },
+      },
+    },
   },
   {
     paranoid: true,
diff --git a/server/presenters/team.js b/server/presenters/team.js
index f90b81be..f78e5443 100644
--- a/server/presenters/team.js
+++ b/server/presenters/team.js
@@ -16,5 +16,6 @@ export default function present(team: Team) {
     subdomain: team.subdomain,
     domain: team.domain,
     url: team.url,
+    defaultUserRole: team.defaultUserRole,
   };
 }
diff --git a/server/routes/api/team.js b/server/routes/api/team.js
index eb433948..3ff5f7e5 100644
--- a/server/routes/api/team.js
+++ b/server/routes/api/team.js
@@ -18,6 +18,7 @@ router.post("team.update", auth(), async (ctx) => {
     guestSignin,
     documentEmbeds,
     collaborativeEditing,
+    defaultUserRole,
   } = ctx.body;
   const user = ctx.state.user;
   const team = await Team.findByPk(user.teamId);
@@ -35,6 +36,9 @@ router.post("team.update", auth(), async (ctx) => {
   if (collaborativeEditing !== undefined) {
     team.collaborativeEditing = collaborativeEditing;
   }
+  if (defaultUserRole !== undefined) {
+    team.defaultUserRole = defaultUserRole;
+  }
 
   const changes = team.changed();
   const data = {};
diff --git a/server/routes/api/team.test.js b/server/routes/api/team.test.js
index 6bc70a7c..28df9b66 100644
--- a/server/routes/api/team.test.js
+++ b/server/routes/api/team.test.js
@@ -21,6 +21,23 @@ describe("#team.update", () => {
     expect(body.data.name).toEqual("New name");
   });
 
+  it("should only allow member,viewer or admin as default role", async () => {
+    const { admin } = await seed();
+    const res = await server.post("/api/team.update", {
+      body: { token: admin.getJwtToken(), defaultUserRole: "New name" },
+    });
+
+    expect(res.status).toEqual(400);
+
+    const successRes = await server.post("/api/team.update", {
+      body: { token: admin.getJwtToken(), defaultUserRole: "viewer" },
+    });
+
+    const body = await successRes.json();
+    expect(successRes.status).toEqual(200);
+    expect(body.data.defaultUserRole).toBe("viewer");
+  });
+
   it("should allow identical team details", async () => {
     const { admin, team } = await seed();
     const res = await server.post("/api/team.update", {
diff --git a/shared/i18n/locales/en_US/translation.json b/shared/i18n/locales/en_US/translation.json
index a4b0357f..5a3e47fa 100644
--- a/shared/i18n/locales/en_US/translation.json
+++ b/shared/i18n/locales/en_US/translation.json
@@ -561,6 +561,8 @@
   "Delete Account": "Delete Account",
   "You may delete your account at any time, note that this is unrecoverable": "You may delete your account at any time, note that this is unrecoverable",
   "Delete account": "Delete account",
+  "New user accounts will be given member permissions by default": "New user accounts will be given member permissions by default",
+  "New user accounts will be given viewer permissions by default": "New user accounts will be given viewer permissions by default",
   "Settings that impact the access, security, and content of your knowledge base.": "Settings that impact the access, security, and content of your knowledge base.",
   "Allow email authentication": "Allow email authentication",
   "When enabled, users can sign-in using their email address": "When enabled, users can sign-in using their email address",
@@ -568,6 +570,7 @@
   "When enabled, documents can be shared publicly on the internet by any team member": "When enabled, documents can be shared publicly on the internet by any team member",
   "Rich service embeds": "Rich service embeds",
   "Links to supported services are shown as rich embeds within your documents": "Links to supported services are shown as rich embeds within your documents",
+  "Default role": "Default role",
   "Documents that have been shared are listed below. Anyone that has the public link can access a read-only version of the document until the link has been revoked.": "Documents that have been shared are listed below. Anyone that has the public link can access a read-only version of the document until the link has been revoked.",
   "Sharing is currently disabled.": "Sharing is currently disabled.",
   "You can globally enable and disable public document sharing in the <em>security settings</em>.": "You can globally enable and disable public document sharing in the <em>security settings</em>.",