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

feat: Support Authentication in Realtime Subscriptions #8815

Merged
merged 3 commits into from
Jul 4, 2023

Conversation

dthyresson
Copy link
Contributor

@dthyresson dthyresson commented Jul 3, 2023

This PR implements validator directives on subscriptions like @requireAuth so one may enabled authentication on a Subscription just like one does for Queries and Mutation in RedwoodJS's GraphQL.

It also:

  • enables any validator directive on Subscriptions (but not transformer ones)
  • checks that subscriptions have the @skipAuth or @requireAuth directive (as do queries and mutations)
  • updates the subscription templates to include @requireAuth

For authorization (that it is I can Subscribe to newMessage -- authn -- but not listen to changes to room 227 -- authz -- one can do something like where you check for a user and the room and raise if no access rights:

const newMessage = {
  newMessage: {
    subscribe: (
      _,
      { roomId },
      {
        pubSub,
        currentUser,
      }: { pubSub: NewMessageChannelType; currentUser: Record<string, any> }
    ) => {
      if (currentUser?.id === '1234' && roomId === '227') {
        throw new ForbiddenError('no can do')
      }

      logger.debug({ roomId }, 'newMessage subscription')

      return pubSub.subscribe('newMessage', roomId)
    },
...

TODO:

  • docs
  • docs for directives and how Subscriptions cannot have transformer directives
  • see maybe if Subs can transformer directives -- otherwise see if need to warn better

@dthyresson dthyresson added the release:feature This PR introduces a new feature label Jul 3, 2023
@dthyresson dthyresson added this to the v6.0.0 milestone Jul 3, 2023
@dthyresson dthyresson self-assigned this Jul 3, 2023
@dthyresson dthyresson marked this pull request as ready for review July 3, 2023 18:43
@dthyresson dthyresson changed the title feat: WIP Support Authentication in Realtime Subscriptions feat: Support Authentication in Realtime Subscriptions Jul 3, 2023
@dthyresson dthyresson requested review from ajcwebdev and Josh-Walker-GM and removed request for ajcwebdev July 3, 2023 18:43
Copy link
Collaborator

@Josh-Walker-GM Josh-Walker-GM left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome that this was able to be done by using the existing directive!

Tested locally and it worked great and the changes look good too.

One question would be do you want to keep using skipAuth in your example live query (auction) and in the generator for live queries? We use requireAuth in our existing non-experimental generators. This is just a minor point though so feel free to go ahead without changing them.

Copy link
Contributor Author

Oh I should fix that. I probably leaked. I realized also when demonstrating to Danny that I should deconstruct the liveQueryStore from the comtext in the bid service

@Josh-Walker-GM
Copy link
Collaborator

I realized also when demonstrating to Danny that I should deconstruct the liveQueryStore from the comtext in the bid service

You mean get rid of the resolver level context as an argument to the function and use the global context? Then in the service you can still do something like const { liveQueryStore } = context?

@dthyresson
Copy link
Contributor Author

dthyresson commented Jul 4, 2023

you can still do something like const { liveQueryStore } = context?

Actually, now that I looked at it, I made it similar to the way pub sub is done in the subscription service.

Now the store is typed and knows what invalidate is:

export const bid = async (
  { input },
  { context }: { context: { liveQueryStore: LiveQueryStorageMechanism } }
) => {
  const { auctionId, amount } = input

  const index = auctions.findIndex((a) => a.id === auctionId)

  const bid = { amount }

  auctions[index].bids.push(bid)
  logger.debug({ auctionId, bid }, 'Added bid to auction')

  const key = `Auction:${auctionId}`
  context.liveQueryStore.invalidate(key)
...

I bet there is a better way to do that though.

@dthyresson
Copy link
Contributor Author

Oh I should fix that. I probably leaked. I realized also when demonstrating to Danny that I should deconstruct the liveQueryStore from the comtext in the bid service

Fixed!

@Josh-Walker-GM
Copy link
Collaborator

Josh-Walker-GM commented Jul 4, 2023

Would it be appropriate to put this on the RedwoodGraphQLContext? I think that would make it globally available and it would be typed? That would be doing the same as we do for currentUser

It would be a breaking change since users can already store anything they want on that and we'd be reserving a spot for the live query store.

Just a thought that might not be any good. I'd need to actually tinker with the code to see.

@dthyresson
Copy link
Contributor Author

Would it be appropriate to put this on the RedwoodGraphQLContext?

Hm. Not sure since both pubSub and liveQuerryStore only get added when useRedwoodRealtime extends that context with them -- and not all GQL contexts will have them.

@dthyresson dthyresson merged commit a471d26 into redwoodjs:main Jul 4, 2023
29 checks passed
jtoar pushed a commit that referenced this pull request Jul 4, 2023
This PR implements validator directives on subscriptions like
`@requireAuth` so one may enabled authentication on a Subscription just
like one does for Queries and Mutation in RedwoodJS's GraphQL.

It also:

* enables any validator directive on Subscriptions (but not transformer
ones)
* checks that subscriptions have the `@skipAuth` or `@requireAuth`
directive (as do queries and mutations)
* updates the subscription templates to include `@requireAuth`

For authorization (that it is I can Subscribe to newMessage -- authn --
but not listen to changes to room 227 -- authz -- one can do something
like where you check for a user and the room and raise if no access
rights:

```ts
const newMessage = {
  newMessage: {
    subscribe: (
      _,
      { roomId },
      {
        pubSub,
        currentUser,
      }: { pubSub: NewMessageChannelType; currentUser: Record<string, any> }
    ) => {
      if (currentUser?.id === '1234' && roomId === '227') {
        throw new ForbiddenError('no can do')
      }

      logger.debug({ roomId }, 'newMessage subscription')

      return pubSub.subscribe('newMessage', roomId)
    },
...
```


TODO:

* docs
* docs for directives and how Subscriptions cannot have transformer
directives
* see maybe if Subs can transformer directives -- otherwise see if need
to warn better
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release:feature This PR introduces a new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants