nestjs / graphql

GraphQL (TypeScript) module for Nest framework (node.js) 🍷
https://docs.nestjs.com/graphql/quick-start
MIT License
1.46k stars 396 forks source link

complexity configuration option not working with federated graphs #1513

Closed DonnyVerduijn closed 2 weeks ago

DonnyVerduijn commented 3 years ago

I'm submitting a...


[ ] Regression 
[x] Bug report
[ ] Feature request
[ ] Documentation issue or request
[ ] Support request => Please do not submit support request here, instead post your question on Stack Overflow.

Current behavior

Currently, when using federated graphs, the complexity option property does not limit the allowed query depth, when there is a circular reference between two object types (when fields are added by resolvers that are decorated with the ResolveField decorator). ​

Expected behavior

The complexity configuration options property should work with federated graphs and resolvers that are decorated with the ResolveField decorator, when they recursively resolve through circular references between two object types. The complexity of queries still need to be limited on a per resolver basis.

Minimal reproduction of the problem with instructions

https://github.com/hardyscc/nestjs-cqrs-starter ./apps/service-account/src/account/resolver/user.resolver.ts

@Resolver(() => User)
export class UserResolver {
  constructor(private readonly accountService: AccountService) {}

  @ResolveField(() => [Account], { complexity: 1})
  accounts(@Parent() user: User) {
    return this.accountService.findByUserId(user.id);
  }
}

This still allows the accounts field to be resolved from the user field inside an account type.

What is the motivation / use case for changing the behavior?

Circular referencing can pose security risks that can be exploited by malicious queries

Environment


Nest version: 7.6.15
​
For Tooling issues:
- Node version: 14
- Platform:  WSL Ubuntu 18
kamilmysliwiec commented 3 years ago

Would you like to create a PR for this issue?

DonnyVerduijn commented 3 years ago

https://github.com/DonnyVerduijn/nestjs-cqrs-starter Should be straightforward to reproduce.

kamilmysliwiec commented 3 years ago

Would you like to create a PR that addresses this issue?

DonnyVerduijn commented 3 years ago

I'll try to take a look at this today.

bneigher commented 2 years ago

👀

ahmadxgani commented 1 year ago

:eyes:

choco14t commented 1 year ago

Hey, minimal reproduction is not installed graphql-query-complexity. So complexity is not working, but expected behavior. I created minimal example repository. It doesn't use federation, but will be work too.

https://github.com/choco14t/nest-graphql-complexity

choco14t commented 1 year ago

@kamilmysliwiec I tested complexity with federated graph. Complexity extension works fine. testing repo: https://github.com/choco14t/example-gateway-logging/tree/check-complexity

Set default complexity 1 per field, user.posts complexity 5. Return complexity 11 when executed query below.

query QueryPosts {
  posts {
    id
    title
    user {
      id
      posts { # set complexity: 5
        id
      }
    }
  }
}

2023-02-26_062214