From 2fa8a21754b24064c0e24bc6eb3c6bdcc28af9ae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gerg=C5=91=20Jedlicska?= Date: Thu, 27 Jul 2023 13:30:32 +0200 Subject: [PATCH] feat(server): implement switch user role to guest --- .../modules/core/graph/resolvers/users.js | 25 +++-- .../server/modules/core/services/users.js | 27 ++---- .../server/modules/core/tests/graph.spec.js | 8 +- .../server/modules/core/tests/users.spec.js | 4 +- .../modules/core/tests/usersAdmin.spec.js | 91 +++++++++++++------ .../modules/shared/helpers/envHelper.ts | 2 +- 6 files changed, 91 insertions(+), 66 deletions(-) diff --git a/packages/server/modules/core/graph/resolvers/users.js b/packages/server/modules/core/graph/resolvers/users.js index 631522358..2824fd222 100644 --- a/packages/server/modules/core/graph/resolvers/users.js +++ b/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) diff --git a/packages/server/modules/core/services/users.js b/packages/server/modules/core/services/users.js index ded009b0c..4eeae7bf3 100644 --- a/packages/server/modules/core/services/users.js +++ b/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 }) } } diff --git a/packages/server/modules/core/tests/graph.spec.js b/packages/server/modules/core/tests/graph.spec.js index 90c0b5a0a..fb23c2e1e 100644 --- a/packages/server/modules/core/tests/graph.spec.js +++ b/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 () => { diff --git a/packages/server/modules/core/tests/users.spec.js b/packages/server/modules/core/tests/users.spec.js index e49fad519..a984bd748 100644 --- a/packages/server/modules/core/tests/users.spec.js +++ b/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) diff --git a/packages/server/modules/core/tests/usersAdmin.spec.js b/packages/server/modules/core/tests/usersAdmin.spec.js index 082961a03..a8239e894 100644 --- a/packages/server/modules/core/tests/usersAdmin.spec.js +++ b/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' + } +} diff --git a/packages/server/modules/shared/helpers/envHelper.ts b/packages/server/modules/shared/helpers/envHelper.ts index 91afa4199..94a5edc20 100644 --- a/packages/server/modules/shared/helpers/envHelper.ts +++ b/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') }