Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Subscriptions based on the userId from authorizer #71

Open
tkohout opened this issue Feb 25, 2020 · 9 comments
Open

Subscriptions based on the userId from authorizer #71

tkohout opened this issue Feb 25, 2020 · 9 comments
Labels
question Further information is requested

Comments

@tkohout
Copy link

tkohout commented Feb 25, 2020

Hello,
thanks for this library! I've been trying to setup simple subscriptions based on your example server app.

I managed to setup the authorizer on websocket, and retrieve user in the context function. With something like this:

context: async (context) => {
   // When called from DynamoDB event stream
    if (context.event?.requestContext == undefined) {
      return {
        pubSub
      }
    }

    let principalId = context.event.requestContext.authorizer!.principalId
    
    const user = await User.findByPk(principalId)

    if (!user) {
      throw new AuthenticationError('User not authorized');
      return
    }

    return {
      me: user,
      pubSub
    };
  }

And the subscription would be something like this:

hasUnseenViewNotifications: {
            resolve: (rootValue: ViewNotificationSeenChanged) => {
              return rootValue.unseen;
            },
            subscribe: (rootValue, args, context, info) => {
                const result = (context.pubSub as PubSub).subscribe(Subscriptions.viewNotificationSeenChanged.toString())
                
                return withFilter(result, (rootValue: ViewNotificationSeenChanged, args) => {
                    return rootValue.userId == context.me.id
                })(rootValue, args, context, info)
            }
        },

I assume the subscribe function is called from the dynamodb event stream so I am not getting requestContext and the userId with it. How to get it inside of the subscribe function?

I came upon this issue: #70 which might be what I am looking for, but I am confused how this would go all together.

Could you point me in the right direction?

Thanks
Tomas

@michalkvasnicak
Copy link
Owner

@tkohout Sorry for late response. Basically if you want to store something in context, you should use onConnect event, that returns an object that can be JSON serialised. Then this data are stored on your connection.data so they should be available in the GraphQL context as ctx.foo.

See:

https://github.com/michalkvasnicak/aws-lambda-graphql/blob/master/packages/aws-lambda-graphql/src/__tests__/Server.test.ts#L396

https://github.com/michalkvasnicak/aws-lambda-graphql/blob/master/packages/aws-lambda-graphql/src/__tests__/Server.test.ts#L443

@michalkvasnicak
Copy link
Owner

Please, let me know if my advice helped you.

@tkohout
Copy link
Author

tkohout commented Feb 26, 2020

Thanks @michalkvasnicak,
yes, that set me in the right direction. I got my usecase working now. For anybody who would find this useful, I did:

subscriptions: {
    onConnect: async (messagePayload, connection, event, context) => {

      let principalId: string
      //serverless-offline does not pass around authorizer info for websockets
      if (process.env.IS_OFFLINE) {
        principalId = "default-user-id"
      } else {
        principalId = event.requestContext?.authorizer?.principalId
      }

      return {
        userId: principalId
      }
    }

and then access it from resolver directly on context:

unseenViewNotificationsCount: {
            resolve: (rootValue: ViewNotificationSeenChanged) => {
              return rootValue.count;
            },
            subscribe: (rootValue, args, context, info) => {
                const result = (context.pubSub as PubSub).subscribe(Subscription.viewNotificationSeenChanged.toString())
                
                return withFilter(result, (rootValue: ViewNotificationSeenChanged, args) => {
                    return rootValue.userId == context.userId
                })(rootValue, args, context, info)
            }
        }

One gotcha would be that if you want to pass around pubSub using context (as per docs) you have to return it for the dynamodb stream as well:

context: async (context) => {
    //This is how I check it's a dynamodb stream, maybe there's a better way?
    if (context.event?.requestContext == undefined) {
      return {
        pubSub
      }
    }

    //If you don't return early this will fail and subscriptions won't work 
    const user = await userForRequest(context.event.requestContext.authorizer!.principalId)

    return {
      me: user,
      pubSub
    };
}

Perhaps authorization could be part of the example? It's a bit hard to setup and I think most of the time the websocket will need to be protected.

@michalkvasnicak michalkvasnicak added the question Further information is requested label Mar 6, 2020
@michalkvasnicak
Copy link
Owner

@tkohout thank you very much for this valuable info. An example is really good idea, would you have some free time to prepare some really simple solution in a PR?

Also about

One gotcha would be that if you want to pass around pubSub using context (as per docs) you have to return it for the dynamodb stream as well:

Yeah this one maybe should be a part of example too.

@IslamWahid
Copy link
Contributor

@michalkvasnicak @tkohout thanks for this info it's really helpful and would be nice to add this to the readme or the code example

@IslamWahid
Copy link
Contributor

@michalkvasnicak @tkohout can you guys please explain the benefits of passing the pubSub through the context instead of just creating a new object and use it when you need it?

@michalkvasnicak
Copy link
Owner

@IslamWahid for me GraphQL context can serve as a Dependency injection container that contains all the services, etc. So instead of importing the pubSub or other services that you need, you can expose them in a context and access them anywhere.

@IslamWahid
Copy link
Contributor

@michalkvasnicak I'm thinking about it performance-wise as it gives me more flexibility to just import it whenever and wherever I need to publish an event instead of injecting it, but just wanted to ask if you're doing this for performance reasons.

@michalkvasnicak
Copy link
Owner

@IslamWahid for me it's just about convenience. It's easier to change type declaration for context and add/remove few things than add/remove multiple import statements from N files.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

3 participants