nuwave / nuwave/lighthouse

@canResolved acts different with @hasMany and @find

Open
#2,714 12 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

discussion
Dominant language
PHP
Stars
3.5k
Forks
468
Avg merge
3h 9m
Merged PRs (30d)
2

Description

Describe the bug

If you define a field as nullable and the resolved value is null or an empty collection you get different behaviour.

if it's a collection @canResolved will not call $authroize because it will receive an empty array ([]) and Utils::applyEach which calls $authorize($model); will be skipped because foreach([]) doesn't do anything.

But if the resolved value is null, it will call $authroize as Utils::applyEach will simply call $callback($valueOrValues); and not be skilled. calling $authroize on null will always result in an Auth error as the Gate code will not be able to find the model class and so doesn't know what Policy to check.

me {
    field(id: ID @eq): Field @find @canResolved(ability: "view")
    collection: [Field!] @hasMany @canResolved(ability: "view")
}

Expected behavior/Solution

Consistency...

the hard part is deciding which one is correct.

  1. if the field is nullable then the resolved value is null, and you can't check permissions, should that be a rejection by default? (that is what Gate would do)

  2. or do you say, well, the value is null, so it doesn't matter (but then does this leak information, you know something doesn't exist and you were not checked for access beforehand)

If the second option, how does one allow a nullable field that checks permission to access the field, maybe if null a second ability option should be provided, such as viewAny

@canResolved(ability: "view", abilityOnNull: "viewAny")

This would require the model class to be determined/known to be able to find the correct Policy, so?

@canResolved(ability: "view", abilityOnNull: "viewAny", modelClassonNull: "\\App\\Models\\Field") ??

ugly.

Steps to reproduce

me @auth {
    field(id: ID @eq): Field @find @canResolved(ability: "view")
    collection: [Field!] @hasMany @canResolved(ability: "view")
}
  me {
    field {
      column
    }
    collection {
      column
    }
  }

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start at the @canResolved directive and Utils::applyEach, then reproduce the nullable @find and empty @hasMany queries shown. Resolve whether null and empty collections should authorize consistently, including the policy behavior, and verify the chosen outcome against both examples.

Written by the indexing model from the issue text.

Assessment

Tech stack
graphql, laravel, php
Domain
api, authorization, backend
Issue type
Bug
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.