Escape-Technologies / graphql-armor

🛡️ The missing GraphQL security security layer for Apollo GraphQL and Yoga / Envelop servers 🛡️
https://escape.tech/graphql-armor/docs/getting-started
MIT License
497 stars 28 forks source link

OnAccept/OnReject called multiple times with the same context but different n #677

Closed Nepoxx closed 2 months ago

Nepoxx commented 4 months ago

This is most likely not a bug and stems from my lack of understanding of how ValidationContext works, but if I have the following query:

query A {
    foo(name: "", limit: 99999) {
        users {
            groups {
                users {
                    groups {
                        users {
                            id
                        }
                    }
                }
            }
        }
    }
}
query B {
    bar(name: "", limit: 1000000) {
        users {
            id
        }
    }
}

My goal is to log the following when I receive it:

Query A has depth: 7
Query B has depth: 3

I tried the following:

  const armor = new ApolloArmor({
    maxDepth: {
      enabled: true,
      n: 10,
      propagateOnRejection: false,
      onAccept: [onAcceptHandler],
    },
  });

  function onAcceptHandler(ctx,{n}) {
      const { definitions } = ctx.getDocument();
      const operationName = definitions[0].name?.value || definitions[0].name;
      const operationType = definitions[0].operation;
      this.log.debug(`Query ${operationName} has depth: ${n}`);
    }

This prints:

Query A has depth: 7
Query A has depth: 3

I can see that Query B is found in definitions[1], but I don't know how to associate n to the correct operation. Would it make sense to pass operation to handlers here? https://github.com/Escape-Technologies/graphql-armor/blob/a54c8dda401cfa5a46e563d3adbd7e665d3f4edf/packages/plugins/max-depth/src/index.ts#L49

Cheers

nullswan commented 3 months ago

Hey @Nepoxx,

Definition is something else, I think it refers to https://www.apollographql.com/docs/react/data/document-transforms/#document-caching

Let me know, but this is rather Apollo related.

Nepoxx commented 3 months ago

Well I'm wondering if the callback should be augmented with the definition because it's not clear how to associate n with the operation in multi-query requests.

I'm coming from this library: https://www.npmjs.com/package/graphql-depth-limit its callback has an association between operations and depth https://www.npmjs.com/package/graphql-depth-limit?activeTab=code

So with my example above, my callback would be called with the following object:

{
  "A": 7,
  "B": 3
}

I guess this boils down to asking how I can associate n in the callbacks using graphql-armor

github-actions[bot] commented 2 months ago

This issue is stale because it has been open for 30 days with no activity.

github-actions[bot] commented 2 months ago

This issue was closed because it has been inactive for 14 days since being marked as stale.