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

Improve subscriptions with authorize filter #284

Open
jakubnavratil opened this issue Aug 7, 2024 · 0 comments
Open

Improve subscriptions with authorize filter #284

jakubnavratil opened this issue Aug 7, 2024 · 0 comments
Labels
enhancement New feature or request

Comments

@jakubnavratil
Copy link

Problem
Using DTO authorizer, I'm able to query filtered collections based on user role.
Role Admin sees everything but role User sees only his own records.
Now when both subscribe to updates and Admin updates record, User does NOT get notified and vice versa, because they have different authorize filters.

Original solutions for that was done in d2f857f
It introduced separate PubSub event using original eventName and authorizeFilter

export function getSubscriptionEventName<T>(eventName: string, authorizeFilter?: Filter<T>): string {

So Admin will publish event eventName-A on update and is subscribed to the same event, but User will publish and subscribe to eventName-B.
That way implementation doesn't leak data to user without proper role BUT not everyone gets notified even if they should.

Solution
This can be solved by using existing applyFilter on subscription iterator inside subscription resolver method.

How this could look?
Assume this helper exists:

async function* filterAsync<T>(
  iterator: AsyncIterator<T>,
  predicate: (item: T) => boolean | Promise<boolean>,
): AsyncIterator<T> {
  while (true) {
    const { value, done } = await iterator.next();
    if (done) break;

    if (await predicate(value)) {
      yield value;
    }
  }
}

Now we can filter results from pubSub.asyncIterator(eventName).
This is how implementation of updateOneSubscription could look:

updatedOneSubscription(
  @Args({ type: () => UOSA }) input?: any,
  @AuthorizerFilter({ operationGroup: OperationGroup.UPDATE, many: false })
  authorizeFilter?: Filter<DTO>,
): AsyncIterator<UpdatedEvent<DTO>> {
  if (!enableOneSubscriptions || !this.pubSub) {
    throw new Error(`Unable to subscribe to ${updateOneEvent}`);
  }
  const iter = this.pubSub.asyncIterator<UpdatedEvent<DTO>>(updateOneEvent);
  if (authorizeFilter == null) {
    return iter;
  }

  return filterAsync(iter, (payload) =>
    applyFilter(payload[updateOneEvent], authorizeFilter),
  );
}

async publishUpdatedOneEvent(
  dto: DTO,
  authorizeFilter?: Filter<DTO>,
): Promise<void> {
  if (this.pubSub) {
    await this.pubSub.publish(updateOneEvent, { [updateOneEvent]: dto });
  }
}

My current solution
I'm currently monkey-patching these methods with custom decorator (cause overriding all methods with proper types is just too much, I'm already overriding some).

Would you be interested in PR?
It would very much helped me, if this could be included in library. From my perspective, this is how subscription should work.
I'll be willing to make PR if there is more interest in this. But my hope is it would be merged if possible, not forgotten :)

@jakubnavratil jakubnavratil added the enhancement New feature or request label Aug 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

1 participant