From a2bb36e43ec68008a0a4ffc9e5c6096297e4e6f9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gerg=C5=91=20Jedlicska?= Date: Wed, 26 Jul 2023 14:08:47 +0200 Subject: [PATCH] refactor(server scopes): use constants for streams owner role --- packages/server/modules/comments/services/index.js | 3 ++- packages/server/modules/core/graph/resolvers/streams.js | 8 ++++---- packages/server/modules/core/repositories/streams.ts | 2 +- packages/server/modules/core/services/users.js | 4 ++-- packages/server/modules/core/tests/generic.spec.js | 2 +- packages/server/modules/core/tests/graph.spec.js | 2 +- packages/server/modules/core/tests/users.spec.js | 4 ++-- .../server/modules/webhooks/graph/resolvers/webhooks.js | 9 +++++---- 8 files changed, 18 insertions(+), 16 deletions(-) diff --git a/packages/server/modules/comments/services/index.js b/packages/server/modules/comments/services/index.js index 1fc117ab7..93231162b 100644 --- a/packages/server/modules/comments/services/index.js +++ b/packages/server/modules/comments/services/index.js @@ -13,6 +13,7 @@ const { markCommentViewed } = require('@/modules/comments/repositories/comments') const { clamp } = require('lodash') +const { Roles } = require('@speckle/shared') const Comments = () => knex('comments') const CommentLinks = () => knex('comment_links') @@ -221,7 +222,7 @@ module.exports = { .first() if (comment.authorId !== userId) { - if (!aclEntry || aclEntry.role !== 'stream:owner') + if (!aclEntry || aclEntry.role !== Roles.Stream.Owner) throw new ForbiddenError("You don't have permission to archive the comment") } diff --git a/packages/server/modules/core/graph/resolvers/streams.js b/packages/server/modules/core/graph/resolvers/streams.js index 1261afff4..932884436 100644 --- a/packages/server/modules/core/graph/resolvers/streams.js +++ b/packages/server/modules/core/graph/resolvers/streams.js @@ -221,13 +221,13 @@ module.exports = { }, async streamUpdate(parent, args, context) { - await authorizeResolver(context.userId, args.stream.id, 'stream:owner') + await authorizeResolver(context.userId, args.stream.id, Roles.Stream.Owner) await updateStreamAndNotify(args.stream, context.userId) return true }, async streamDelete(parent, args, context, info) { - await authorizeResolver(context.userId, args.id, 'stream:owner') + await authorizeResolver(context.userId, args.id, Roles.Stream.Owner) return await _deleteStream(parent, args, context, info) }, @@ -246,7 +246,7 @@ module.exports = { await authorizeResolver( context.userId, args.permissionParams.streamId, - 'stream:owner' + Roles.Stream.Owner ) const result = await updateStreamRoleAndNotify( @@ -260,7 +260,7 @@ module.exports = { await authorizeResolver( context.userId, args.permissionParams.streamId, - 'stream:owner' + Roles.Stream.Owner ) const result = await updateStreamRoleAndNotify( diff --git a/packages/server/modules/core/repositories/streams.ts b/packages/server/modules/core/repositories/streams.ts index 482ef00b8..458a3d478 100644 --- a/packages/server/modules/core/repositories/streams.ts +++ b/packages/server/modules/core/repositories/streams.ts @@ -930,7 +930,7 @@ export async function revokeStreamPermissions(params: { .select('*') .first() - if (aclEntry?.role === 'stream:owner') { + if (aclEntry?.role === Roles.Stream.Owner) { const [countObj] = await StreamAcl.knex() .where({ resourceId: streamId, diff --git a/packages/server/modules/core/services/users.js b/packages/server/modules/core/services/users.js index f6f7e5efe..f96c68c2e 100644 --- a/packages/server/modules/core/services/users.js +++ b/packages/server/modules/core/services/users.js @@ -227,9 +227,9 @@ module.exports = { ( -- Get streams ids on which the user is owner SELECT "resourceId" FROM stream_acl - WHERE role = 'stream:owner' AND "userId" = ? + WHERE role = ${Roles.Stream.Owner} AND "userId" = ? ) AS us ON acl."resourceId" = us."resourceId" - WHERE acl.role = 'stream:owner' + WHERE acl.role = ${Roles.Stream.Owner} GROUP BY (acl."resourceId") ) AS soc WHERE cnt = 1 diff --git a/packages/server/modules/core/tests/generic.spec.js b/packages/server/modules/core/tests/generic.spec.js index 2c4bb95f6..75f2a246e 100644 --- a/packages/server/modules/core/tests/generic.spec.js +++ b/packages/server/modules/core/tests/generic.spec.js @@ -154,7 +154,7 @@ describe('Generic AuthN & AuthZ controller tests', () => { myStream.id, 'stream:contributor' ) - expect(role).to.equal('stream:owner') + expect(role).to.equal(Roles.Stream.Owner) }) it('should get the passed in role for server:admins if override enabled', async () => { diff --git a/packages/server/modules/core/tests/graph.spec.js b/packages/server/modules/core/tests/graph.spec.js index ee239d02e..d7adb25fa 100644 --- a/packages/server/modules/core/tests/graph.spec.js +++ b/packages/server/modules/core/tests/graph.spec.js @@ -1277,7 +1277,7 @@ describe('GraphQL API Core @core-api', () => { expect(stream.name).to.equal('TS1 (u A) Private UPDATED') expect(stream.collaborators).to.have.lengthOf(2) expect(stream.collaborators[0].role).to.equal('stream:contributor') - expect(stream.collaborators[1].role).to.equal('stream:owner') + expect(stream.collaborators[1].role).to.equal(Roles.Stream.Owner) }) it('Should retrieve a public stream even if not authenticated', async () => { diff --git a/packages/server/modules/core/tests/users.spec.js b/packages/server/modules/core/tests/users.spec.js index 9d3b96377..e49fad519 100644 --- a/packages/server/modules/core/tests/users.spec.js +++ b/packages/server/modules/core/tests/users.spec.js @@ -41,7 +41,7 @@ const { const { createObject } = require('../services/objects') const { beforeEachContext } = require('@/test/hooks') -const { Scopes } = require('@speckle/shared') +const { Scopes, Roles } = require('@speckle/shared') describe('Actors & Tokens @user-services', () => { const myTestActor = { @@ -194,7 +194,7 @@ describe('Actors & Tokens @user-services', () => { await grantPermissionsStream({ streamId: multiOwnerStream.id, userId: myTestActor.id, - role: 'stream:owner' + role: Roles.Stream.Owner }) // create a branch for ballmer on the multiowner stream diff --git a/packages/server/modules/webhooks/graph/resolvers/webhooks.js b/packages/server/modules/webhooks/graph/resolvers/webhooks.js index 90186ac36..faaee85bd 100644 --- a/packages/server/modules/webhooks/graph/resolvers/webhooks.js +++ b/packages/server/modules/webhooks/graph/resolvers/webhooks.js @@ -10,11 +10,12 @@ const { getLastWebhookEvents, getWebhookEventsCount } = require('../../services/webhooks') +const { Roles } = require('@speckle/shared') module.exports = { Stream: { async webhooks(parent, args, context) { - await authorizeResolver(context.userId, parent.id, 'stream:owner') + await authorizeResolver(context.userId, parent.id, Roles.Stream.Owner) if (args.id) { const wh = await getWebhook({ id: args.id }) @@ -41,7 +42,7 @@ module.exports = { Mutation: { async webhookCreate(parent, args, context) { - await authorizeResolver(context.userId, args.webhook.streamId, 'stream:owner') + await authorizeResolver(context.userId, args.webhook.streamId, Roles.Stream.Owner) const id = await createWebhook({ streamId: args.webhook.streamId, @@ -55,7 +56,7 @@ module.exports = { return id }, async webhookUpdate(parent, args, context) { - await authorizeResolver(context.userId, args.webhook.streamId, 'stream:owner') + await authorizeResolver(context.userId, args.webhook.streamId, Roles.Stream.Owner) const wh = await getWebhook({ id: args.webhook.id }) if (args.webhook.streamId !== wh.streamId) @@ -75,7 +76,7 @@ module.exports = { return !!updated }, async webhookDelete(parent, args, context) { - await authorizeResolver(context.userId, args.webhook.streamId, 'stream:owner') + await authorizeResolver(context.userId, args.webhook.streamId, Roles.Stream.Owner) const wh = await getWebhook({ id: args.webhook.id }) if (args.webhook.streamId !== wh.streamId)