feat(server): implement switch user role to guest

packages/server/modules/core/graph/resolvers/users.js

@@ -6,10 +6,8 @@ const {
   getUserRole,
   deleteUser,
   searchUsers,
-  makeUserAdmin,
-  unmakeUserAdmin,
-  archiveUser
-} = require('../../services/users')
+  changeUserRole
+} = require('@/modules/core/services/users')
 const { updateUserAndNotify } = require('@/modules/core/services/users/management')
 const { saveActivity } = require('@/modules/activitystream/services')
 const { ActionTypes } = require('@/modules/activitystream/helpers/types')
@@ -21,6 +19,7 @@ const {
 const { Roles, Scopes } = require('@speckle/shared')
 const { markOnboardingComplete } = require('@/modules/core/repositories/users')
 const { UsersMeta } = require('@/modules/core/dbSchema')
+const { guestServerRoleEnabled } = require('@/modules/shared/helpers/envHelper')
 
 /** @type {import('@/modules/core/graph/generated/graphql').Resolvers} */
 module.exports = {
@@ -130,24 +129,22 @@ module.exports = {
     }
   },
   Mutation: {
-    async userUpdate(parent, args, context) {
+    async userUpdate(_parent, args, context) {
       await validateServerRole(context, Roles.Server.User)
       await updateUserAndNotify(context.userId, args.user)
       return true
     },
 
-    async userRoleChange(parent, args) {
-      const roleChangers = {
-        [Roles.Server.Admin]: makeUserAdmin,
-        [Roles.Server.User]: unmakeUserAdmin,
-        [Roles.Server.ArchivedUser]: archiveUser
-      }
-      const roleChanger = roleChangers[args.userRoleInput.role]
-      await roleChanger({ userId: args.userRoleInput.id })
+    async userRoleChange(_parent, args) {
+      await changeUserRole({
+        role: args.userRoleInput.role,
+        userId: args.userRoleInput.id,
+        guestRoleEnabled: guestServerRoleEnabled()
+      })
       return true
     },
 
-    async adminDeleteUser(parent, args, context) {
+    async adminDeleteUser(_parent, args, context) {
       await validateServerRole(context, Roles.Server.Admin)
       const user = await getUserByEmail({ email: args.userConfirmation.email })
       await deleteUser(user.id)

packages/server/modules/core/services/users.js

@@ -28,7 +28,7 @@ const {
 } = require('@/modules/core/errors/userinput')
 const { Roles } = require('@speckle/shared')
 
-const changeUserRole = async ({ userId, role }) =>
+const _changeUserRole = async ({ userId, role }) =>
   await Acl().where({ userId }).update({ role })
 
 const countAdminUsers = async () => {
@@ -264,25 +264,18 @@ module.exports = {
     return users
   },
 
-  async makeUserAdmin({ userId }) {
-    await changeUserRole({ userId, role: Roles.Server.Admin })
-  },
-
-  async unmakeUserAdmin({ userId }) {
-    // dont delete last admin role
-    await _ensureAtleastOneAdminRemains(userId)
-    await changeUserRole({ userId, role: Roles.Server.User })
-  },
-
-  async archiveUser({ userId }) {
-    // dont change last admin to archived
-    await _ensureAtleastOneAdminRemains(userId)
-    await changeUserRole({ userId, role: Roles.Server.ArchivedUser })
-  },
-
   async countUsers(searchQuery = null) {
     const query = getUsersBaseQuery(searchQuery)
     const [userCount] = await query.count()
     return parseInt(userCount.count)
+  },
+
+  async changeUserRole({ userId, role, guestRoleEnabled = false }) {
+    if (!Object.values(Roles.Server).includes(role))
+      throw new UserInputError(`Invalid role specified: ${role}`)
+    if (!guestRoleEnabled && role === Roles.Server.Guest)
+      throw new UserInputError('Guest role is not enabled')
+    if (role !== Roles.Server.Admin) await _ensureAtleastOneAdminRemains(userId)
+    await _changeUserRole({ userId, role })
   }
 }

packages/server/modules/core/tests/graph.spec.js

@@ -5,7 +5,11 @@ const request = require('supertest')
 const { beforeEachContext, initializeTestServer } = require(`@/test/hooks`)
 const { generateManyObjects } = require(`@/test/helpers`)
 
-const { createUser, getUsers, archiveUser } = require('../services/users')
+const {
+  createUser,
+  getUsers,
+  changeUserRole
+} = require('@/modules/core/services/users')
 const { createPersonalAccessToken } = require('../services/tokens')
 const {
   addOrUpdateStreamCollaborator,
@@ -1706,7 +1710,7 @@ describe('GraphQL API Core @core-api', () => {
           Scopes.Users.Invite
         ]
       )}`
-      await archiveUser({ userId: archivedUser.id })
+      await changeUserRole({ userId: archivedUser.id, role: Roles.Server.ArchivedUser })
     })
 
     it('Should be able to read public streams', async () => {

packages/server/modules/core/tests/users.spec.js

@@ -7,7 +7,7 @@ const assert = require('assert')
 const knex = require('@/db/knex')
 
 const {
-  archiveUser,
+  changeUserRole,
   createUser,
   findOrCreateUser,
   getUser,
@@ -293,7 +293,7 @@ describe('Actors & Tokens @user-services', () => {
         password: 'nanananananaaaa'
       })
 
-      await archiveUser({ userId: toBeArchivedId })
+      await changeUserRole({ userId: toBeArchivedId, role: Roles.Server.ArchivedUser })
 
       let { users } = await searchUsers('Library', 20, null)
       expect(users).to.have.lengthOf(1)

packages/server/modules/core/tests/usersAdmin.spec.js

@@ -6,12 +6,12 @@ const {
   getUsers,
   countUsers,
   deleteUser,
-  getUserRole,
-  unmakeUserAdmin,
-  makeUserAdmin
-} = require('../services/users')
+  changeUserRole,
+  getUserRole
+} = require('@/modules/core/services/users')
 const { beforeEachContext } = require('@/test/hooks')
 const { Roles } = require('@speckle/shared')
+const cryptoRandomString = require('crypto-random-string')
 
 describe('User admin @user-services', () => {
   const myTestActor = {
@@ -53,14 +53,6 @@ describe('User admin @user-services', () => {
   })
 
   it('Get users query limit is sanitized to upper limit', async () => {
-    const createNewDroid = (number) => {
-      return {
-        name: `${number}`,
-        email: `${number}@droidarmy.com`,
-        password: 'sn3aky-1337-b1m'
-      }
-    }
-
     const userInputs = Array(250)
       .fill()
       .map((v, i) => createNewDroid(i))
@@ -90,27 +82,66 @@ describe('User admin @user-services', () => {
     expect(await countUsers('droid')).to.equal(250)
   })
 
-  it('Change user role modifies role', async () => {
-    const [user] = await getUsers(1, 10)
+  describe('changeUserRole', () => {
+    it('throws for invalid role value', async () => {
+      const role = 'shadow:lurker'
+      try {
+        await changeUserRole({ userId: myTestActor.id, role })
+        assert.fail('This should have failed')
+      } catch (err) {
+        expect(err.message).to.equal(`Invalid role specified: ${role}`)
+      }
+    })
+    it('throws if guest role not enabled, but trying to change user role to guest', async () => {
+      const role = Roles.Server.Guest
+      try {
+        await changeUserRole({ userId: myTestActor.id, role })
+        assert.fail('This should have failed')
+      } catch (err) {
+        expect(err.message).to.equal('Guest role is not enabled')
+      }
+    })
+    it('modifies role', async () => {
+      const userId = await createUser(
+        createNewDroid(cryptoRandomString({ length: 13 }))
+      )
 
-    const oldRole = await getUserRole(user.id)
-    expect(oldRole).to.equal(Roles.Server.User)
+      const oldRole = await getUserRole(userId)
+      expect(oldRole).to.equal(Roles.Server.User)
 
-    await makeUserAdmin({ userId: user.id })
-    let newRole = await getUserRole(user.id)
-    expect(newRole).to.equal(Roles.Server.Admin)
+      await changeUserRole({ userId, role: Roles.Server.Admin })
+      let newRole = await getUserRole(userId)
+      expect(newRole).to.equal(Roles.Server.Admin)
 
-    await unmakeUserAdmin({ userId: user.id })
-    newRole = await getUserRole(user.id)
-    expect(newRole).to.equal(Roles.Server.User)
-  })
+      await changeUserRole({ userId, role: Roles.Server.User })
+      newRole = await getUserRole(userId)
+      expect(newRole).to.equal(Roles.Server.User)
 
-  it('Ensure at least one admin remains in the server', async () => {
-    try {
-      await unmakeUserAdmin({ userId: myTestActor.id, role: Roles.Server.Admin })
-      assert.fail('This should have failed')
-    } catch (err) {
-      expect(err.message).to.equal('Cannot remove the last admin role from the server')
-    }
+      await changeUserRole({
+        userId,
+        role: Roles.Server.Guest,
+        guestRoleEnabled: true
+      })
+      newRole = await getUserRole(userId)
+      expect(newRole).to.equal(Roles.Server.Guest)
+    })
+    it('Ensures at least one admin remains in the server', async () => {
+      try {
+        await changeUserRole({ userId: myTestActor.id, role: Roles.Server.User })
+        assert.fail('This should have failed')
+      } catch (err) {
+        expect(err.message).to.equal(
+          'Cannot remove the last admin role from the server'
+        )
+      }
+    })
   })
 })
+
+const createNewDroid = (number) => {
+  return {
+    name: `${number}`,
+    email: `${number}@droidarmy.com`,
+    password: 'sn3aky-1337-b1m'
+  }
+}

packages/server/modules/shared/helpers/envHelper.ts

@@ -188,6 +188,6 @@ export function ignoreMissingMigrations() {
   return ['1', 'true'].includes(process.env.IGNORE_MISSING_MIRATIONS || 'false')
 }
 
-export function enableServerGuests() {
+export function guestServerRoleEnabled() {
   return ['1', 'true'].includes(process.env.ENABLE_SERVER_GUESTS || 'false')
 }