From 6db1bdcf421c1126915cc97c5eb110d38edae05e Mon Sep 17 00:00:00 2001 From: izzy lyseggen Date: Fri, 21 Aug 2020 13:44:19 +0100 Subject: [PATCH] feat(subs): simplify stream create/delete subs as per discussion, remove `ownerId` input and only allow subscribing to your own stream creation / deletion --- modules/core/graph/resolvers/streams.js | 8 ++++---- modules/core/graph/schemas/streams.graphql | 16 ++++++++-------- modules/core/tests/graphSubs.spec.js | 16 ++++++++-------- 3 files changed, 20 insertions(+), 20 deletions(-) diff --git a/modules/core/graph/resolvers/streams.js b/modules/core/graph/resolvers/streams.js index 58f74a88b..ee0ebd50b 100644 --- a/modules/core/graph/resolvers/streams.js +++ b/modules/core/graph/resolvers/streams.js @@ -144,8 +144,8 @@ module.exports = { Subscription: { userStreamCreated: { subscribe: withFilter( () => pubsub.asyncIterator( [ USER_STREAM_CREATED ] ), - ( payload, variables ) => { - return payload.ownerId === variables.ownerId + ( payload, variables, context ) => { + return payload.ownerId === context.userId } ) }, streamUpdated: { @@ -157,8 +157,8 @@ module.exports = { }, userStreamDeleted: { subscribe: withFilter( () => pubsub.asyncIterator( [ USER_STREAM_DELETED ] ), - ( payload, variables ) => { - return payload.ownerId === variables.ownerId + ( payload, variables, context ) => { + return payload.ownerId === context.userId } ) }, streamPermissionGranted: { diff --git a/modules/core/graph/schemas/streams.graphql b/modules/core/graph/schemas/streams.graphql index a7237a5ec..b0eecdf2b 100644 --- a/modules/core/graph/schemas/streams.graphql +++ b/modules/core/graph/schemas/streams.graphql @@ -74,21 +74,20 @@ extend type Subscription { # # User bound subscriptions that operate on the stream collection of an user - # Example relevant view/usecase: a given user's profile page. + # Example relevant view/usecase: updates when working in GH or Dynamo # # Source: # - stream created mutation (target: stream creator) # - stream grant permissions (target: grantee id) # - # Payload: maybe just the streamId? as this event will trigger a new call to get the user's streams on the client side? + # Payload: streamId and sreamCreate input args #  - # TODO: scope check: if context.userId === variables.userId -> profile:read; otherwise users:read - # Q: make the arg optional; if not present -> default to context.user? + # As per discussion, removed `ownerId` input so this now just works for the current user """ Subscribes to new stream created event for a given user. """ - userStreamCreated( ownerId: String! ): JSONObject + userStreamCreated: JSONObject @hasRole(role: "server:user") @hasScope(scope: "profile:read") @@ -96,18 +95,19 @@ extend type Subscription { # - stream delete mutation (target: all stream users in acl) # - stream revoke permissions (target: grantee id) # - # Payload: maybe just the streamId? as this event will trigger a new call to get the user's streams on the client side? + # Payload: streamId """ Subscribes to stream deleted event for a given user. """ - userStreamDeleted( ownerId: String! ): JSONObject + userStreamDeleted: JSONObject @hasRole(role: "server:user") @hasScope(scope: "profile:read") """ Subscribes to stream updated event. """ - streamUpdated( streamId: String! ): JSONObject + streamUpdated( streamId: String ): JSONObject @hasRole(role: "server:user") + @hasScope(scope: "streams:read") """ Subscribes to stream permission granted event. """ diff --git a/modules/core/tests/graphSubs.spec.js b/modules/core/tests/graphSubs.spec.js index d27043d07..e43085480 100644 --- a/modules/core/tests/graphSubs.spec.js +++ b/modules/core/tests/graphSubs.spec.js @@ -81,7 +81,7 @@ describe( 'GraphQL API Subscriptions', ( ) => { userB.token = `Bearer ${( await createPersonalAccessToken( userB.id, 'test token user B', [ 'streams:read', 'streams:write', 'users:read', 'users:email', 'tokens:write', 'tokens:read', 'profile:read', 'profile:email' ] ) )}` userC.id = await createUser( userC ) - userC.token = `Bearer ${( await createPersonalAccessToken( userC.id, 'test token user B', [ 'streams:read', 'users:read', 'users:email' ] ) )}` + userC.token = `Bearer ${( await createPersonalAccessToken( userC.id, 'test token user B', [ 'streams:read', 'streams:write', 'users:read', 'users:email' ] ) )}` } ) after( async ( ) => { @@ -92,7 +92,7 @@ describe( 'GraphQL API Subscriptions', ( ) => { it( 'Should be notified when a stream is created', async ( ) => { let eventNum = 0 - const query = gql `subscription mySub { userStreamCreated ( ownerId: "${userA.id}" ) }` + const query = gql `subscription mySub { userStreamCreated }` const client = createSubscriptionObservable( wsAddr, userA.token, query ) const consumer = client.subscribe( eventData => { // console.log( 'Create subscription log' ) @@ -152,7 +152,7 @@ describe( 'GraphQL API Subscriptions', ( ) => { const sid2 = sc2.body.data.streamCreate let eventNum = 0 - const query = gql `subscription userStreamDeleted { userStreamDeleted( ownerId: "${userA.id}" ) }` + const query = gql `subscription userStreamDeleted { userStreamDeleted }` const client = createSubscriptionObservable( wsAddr, userA.token, query ) const consumer = client.subscribe( eventData => { expect( eventData.data.userStreamDeleted ).to.exist @@ -226,7 +226,7 @@ describe( 'GraphQL API Subscriptions', ( ) => { }) it( 'Should *not* be notified of stream creation if invalid token', async () => { - const query = gql`subscription mySub { userStreamCreated ( userId: "${userA.id}" ) }` + const query = gql`subscription mySub { userStreamCreated }` const client = createSubscriptionObservable( wsAddr, "faketoken123", query ) const consumer = client.subscribe( eventData => { expect( eventData.data ).to.not.exist @@ -242,7 +242,7 @@ describe( 'GraphQL API Subscriptions', ( ) => { } ) it( 'Should *not* be notified of another user stream created', async () => { - const query = gql`subscription mySub { userStreamCreated ( ownerId: "${userB.id}" ) }` + const query = gql`subscription mySub { userStreamCreated }` const client = createSubscriptionObservable(wsAddr, userB.token, query) const consumer = client.subscribe(eventData => { expect( eventData.data.userStreamCreated ).to.not.exist @@ -261,7 +261,7 @@ describe( 'GraphQL API Subscriptions', ( ) => { } ) it( 'Should *not* allow subscribing to stream creation without profile:read scope', async () => { - const query = gql`subscription mySub { userStreamCreated ( ownerId: "${userA.id}" ) }` + const query = gql`subscription mySub { userStreamCreated }` const client = createSubscriptionObservable(wsAddr, userC.token, query) const consumer = client.subscribe(eventData => { expect( eventData.data.userStreamCreated ).to.not.exist @@ -269,10 +269,10 @@ describe( 'GraphQL API Subscriptions', ( ) => { await sleep(500) - let sc1 = await sendRequest(userA.token, { query: `mutation { streamCreate(stream: { name: "Subs Test (u A) Private", description: "Hello World", isPublic:false } ) }` }) + let sc1 = await sendRequest(userC.token, { query: `mutation { streamCreate(stream: { name: "Subs Test (u A) Private", description: "Hello World", isPublic:false } ) }` }) expect(sc1.body.errors).to.not.exist - let sc2 = await sendRequest(userA.token, { query: `mutation { streamCreate(stream: { name: "Subs Test (u A) Private", description: "Hello World", isPublic:false } ) }` }) + let sc2 = await sendRequest(userC.token, { query: `mutation { streamCreate(stream: { name: "Subs Test (u A) Private", description: "Hello World", isPublic:false } ) }` }) expect(sc2.body.errors).to.not.exist await sleep(1000) // we need to wait up a second here