From eb829d9ded678c07fee805992d2ceb7fd1571bc0 Mon Sep 17 00:00:00 2001 From: izzy lyseggen Date: Tue, 20 Jul 2021 16:39:22 +0100 Subject: [PATCH] fix(server): webhook permissions fixes --- .../webhooks/graph/resolvers/webhooks.js | 12 +++++++++- .../webhooks/graph/schemas/webhooks.graphql | 16 +++++++++----- .../modules/webhooks/tests/webhooks.spec.js | 22 +++++++++++-------- 3 files changed, 35 insertions(+), 15 deletions(-) diff --git a/packages/server/modules/webhooks/graph/resolvers/webhooks.js b/packages/server/modules/webhooks/graph/resolvers/webhooks.js index 2c118b25a..943c9a954 100644 --- a/packages/server/modules/webhooks/graph/resolvers/webhooks.js +++ b/packages/server/modules/webhooks/graph/resolvers/webhooks.js @@ -1,9 +1,13 @@ +const appRoot = require( 'app-root-path' ) +const { authorizeResolver } = require( `${appRoot}/modules/shared` ) const { createWebhook, getWebhook, updateWebhook, deleteWebhook, getStreamWebhooks, getLastWebhookEvents, getWebhookEventsCount } = require( '../../services/webhooks' ) module.exports = { Stream: { async webhooks( parent, args, context, info ) { + await authorizeResolver( context.userId, parent.id, 'stream:owner' ) + if ( args.id ) { let wh = await getWebhook( { id: args.id } ) let items = wh ? [ wh ] : [] @@ -26,17 +30,23 @@ module.exports = { Mutation: { async webhookCreate( parent, args, context, info ) { + await authorizeResolver( context.userId, args.webhook.streamId, 'stream:owner' ) + let id = await createWebhook( { streamId: args.webhook.streamId, url: args.webhook.url, description: args.webhook.description, secret: args.webhook.secret, enabled: args.webhook.enabled !== false, triggers: args.webhook.triggers } ) return id }, async webhookUpdate( parent, args, context, info ) { + await authorizeResolver( context.userId, args.webhook.streamId, 'stream:owner' ) + let updated = await updateWebhook( { id: args.webhook.id, url: args.webhook.url, description: args.webhook.description, secret: args.webhook.secret, enabled: args.webhook.enabled !== false, triggers: args.webhook.triggers } ) return !!updated }, async webhookDelete( parent, args, context, info ) { - let deleted = await deleteWebhook( { id: args.id } ) + await authorizeResolver( context.userId, args.webhook.streamId, 'stream:owner' ) + + let deleted = await deleteWebhook( { id: args.webhook.id } ) return !!deleted } diff --git a/packages/server/modules/webhooks/graph/schemas/webhooks.graphql b/packages/server/modules/webhooks/graph/schemas/webhooks.graphql index befdf2e5d..8ec9e4f12 100644 --- a/packages/server/modules/webhooks/graph/schemas/webhooks.graphql +++ b/packages/server/modules/webhooks/graph/schemas/webhooks.graphql @@ -1,6 +1,6 @@ extend type Stream { webhooks(id: String): WebhookCollection - @hasRole(role: "stream:owner") + @hasRole(role: "server:user") @hasScope(scope: "streams:write") } @@ -9,21 +9,21 @@ extend type Mutation { Creates a new webhook on a stream """ webhookCreate(webhook: WebhookCreateInput!): String! - @hasRole(role: "stream:owner") + @hasRole(role: "server:user") @hasScope(scope: "streams:write") """ Updates an existing webhook """ webhookUpdate(webhook: WebhookUpdateInput!): String! - @hasRole(role: "stream:owner") + @hasRole(role: "server:user") @hasScope(scope: "streams:write") """ Deletes an existing webhook """ - webhookDelete(id: String!): String! - @hasRole(role: "stream:owner") + webhookDelete(webhook: WebhookDeleteInput!): String! + @hasRole(role: "server:user") @hasScope(scope: "streams:write") } @@ -53,6 +53,7 @@ input WebhookCreateInput { input WebhookUpdateInput { id: String! + streamId: String! url: String description: String secret: String @@ -60,6 +61,11 @@ input WebhookUpdateInput { triggers: [String] } +input WebhookDeleteInput { + id: String! + streamId: String! +} + type WebhookEventCollection{ totalCount: Int items: [WebhookEvent] diff --git a/packages/server/modules/webhooks/tests/webhooks.spec.js b/packages/server/modules/webhooks/tests/webhooks.spec.js index bda81c7ae..21473dee1 100644 --- a/packages/server/modules/webhooks/tests/webhooks.spec.js +++ b/packages/server/modules/webhooks/tests/webhooks.spec.js @@ -133,18 +133,18 @@ describe( 'Webhooks @webhooks', () => { before( async () => { userTwo.id = await createUser( userTwo ) - streamTwo.ownerId = userOne.id + streamTwo.ownerId = userTwo.id streamTwo.id = await createStream( streamTwo ) webhookTwo.streamId = streamTwo.id userOne.token = `Bearer ${( await createPersonalAccessToken( userOne.id, 'userOne test token', [ 'streams:read', 'streams:write' ] ) )}` userTwo.token = `Bearer ${( await createPersonalAccessToken( userTwo.id, 'userTwo test token', [ 'streams:read', 'streams:write' ] ) )}` - await grantPermissionsStream( { streamId: streamTwo.id, userId: userTwo.id, role: 'stream:contributor' } ) + await grantPermissionsStream( { streamId: streamTwo.id, userId: userOne.id, role: 'stream:contributor' } ) } ) it( 'Should create a webhook', async () => { - const res = await sendRequest( userOne.token, { query: 'mutation createWebhook($webhook: WebhookCreateInput!) { webhookCreate( webhook: $webhook ) }', variables: { webhook: webhookTwo } } ) + const res = await sendRequest( userTwo.token, { query: 'mutation createWebhook($webhook: WebhookCreateInput!) { webhookCreate( webhook: $webhook ) }', variables: { webhook: webhookTwo } } ) expect( noErrors( res ) ) expect( res.body.data.webhookCreate ).to.not.be.null webhookTwo.id = res.body.data.webhookCreate @@ -152,7 +152,7 @@ describe( 'Webhooks @webhooks', () => { it( 'Should get stream webhooks and the previous events', async () => { await dispatchStreamEvent( { streamId: streamTwo.id, event: 'commit_create', eventPayload: 'payload321' } ) - const res = await sendRequest( userOne.token, { query: `query { + const res = await sendRequest( userTwo.token, { query: `query { stream(id: "${streamTwo.id}") { webhooks { totalCount items { id url enabled history { totalCount items { status statusInfo payload } } } @@ -169,7 +169,8 @@ describe( 'Webhooks @webhooks', () => { } ) it( 'Should update a webhook', async () => { - const res = await sendRequest( userOne.token, { query: `mutation { webhookUpdate(webhook: { id: "${webhookTwo.id}", description: "updated webhook", enabled: false }) + const res = await sendRequest( userTwo.token, { + query: `mutation { webhookUpdate(webhook: { id: "${webhookTwo.id}", streamId: "${streamTwo.id}", description: "updated webhook", enabled: false }) }` } ) let webhook = await getWebhook( { id: webhookTwo.id } ) expect( noErrors( res ) ) @@ -179,8 +180,8 @@ describe( 'Webhooks @webhooks', () => { } ) it( 'Should delete a webhook', async () => { - const res = await sendRequest( userOne.token, { - query: `mutation { webhookDelete(id: "${webhookTwo.id}") }` + const res = await sendRequest( userTwo.token, { + query: `mutation { webhookDelete(webhook: { id: "${webhookTwo.id}", streamId: "${streamTwo.id}" } ) }` } ) expect( noErrors( res ) ) expect( res.body.data.webhookDelete ).to.equal( 'true' ) @@ -188,13 +189,16 @@ describe( 'Webhooks @webhooks', () => { it( 'Should *not* create a webhook if user is not a stream owner', async () => { delete webhookTwo.id - const res = await sendRequest( userTwo.token, { query: 'mutation createWebhook($webhook: WebhookCreateInput!) { webhookCreate( webhook: $webhook ) }', variables: { webhook: webhookTwo } } ) + const res = await sendRequest( userOne.token, { + query: 'mutation createWebhook($webhook: WebhookCreateInput!) { webhookCreate( webhook: $webhook ) }', + variables: { webhook: webhookTwo } + } ) expect( res.body.errors ).to.exist expect( res.body.errors[ 0 ].extensions.code ).to.equal( 'FORBIDDEN' ) } ) it( 'Should *not* get a webhook if the user is not a stream owner', async () => { - const res = await sendRequest( userTwo.token, { query: `query { + const res = await sendRequest( userOne.token, { query: `query { stream(id: "${streamTwo.id}") { webhooks { totalCount items { id url enabled } } } }` } ) expect( res.body.errors ).to.exist