@canResolved acts different with @hasMany and @find

[SlyDave]

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
    }
  }

[SlyDave]

Same applies to @canModel but also breaks on collections, because if the resolver returns null the call to Gate will always reject.

[SlyDave]

How did this come about?

well, we have a field that may be null, but if it isn't, we need to check access against the model. It's valid for the/a user to not have one of these models, but if they do, we need to check they're permitted to access it via the model Policy.

[spawnia]

To clarify: In your example, the field with @hasMany is nullable. Are you talking about differences between handling an empty list or null there? I think it should most likely be non-nullable.

I think we could change @canResolved to return early if the resolved value is null. If hiding existence/non-existence is a concern, you can use @canRoot on the field additionally.

[SlyDave]

To clarify: In your example, the field with @hasmany is nullable. Are you talking about differences between handling an empty list or null there? I think it should most likely be non-nullable.

empty list, see below:

collection: [Field!] @hasMany @canResolved(ability: "view")

• collection = null == auth error ✅
• collection = [null] == schema error ✅
• collection = [null, 'valid-option'] == schema error ✅
• collection = [] == no auth error ❌

collection: [Field] @hasMany @canResolved(ability: "view")

• collection = null == auth error ✅
• collection = [null] == auth error ✅
• collection = [null, 'valid-option'] == auth error ✅
• collection = [] == no auth error ❌

In this instance, throwing auth error when null, and being nullable are not compatible, which is another problem we're facing, as we want allow nulls but auth if not null. we're likely going to write a custom Directive for that one, I suspect it's not a common use case.

I think we could change @canResolved to return early if the resolved value is null. If hiding existence/non-existence is a concern, you can use @canRoot on the field additionally.

I think throwing the auth error if the list is empty is the solution here, as you can't test the authorization if there is no resource to test, and Gate will by default throw an auth exception in that situation (as it does with null)

@canRoot works in this example as the field is on me, but we have other places we're doing similar things where root ends up being null. Otherwise, being able to say "visibility of this field is defined by canViewField on the root object" would be perfect - I may need to revisit...

[SlyDave]

regarding @canRoot I remember why we're not using that now.
It's legitimate for our resource to be null, let's say for example "Passport" a User may not have a passport. and we don't want the authorisation for the resource to throw an auth exception on a null resource. but if the resource does exist, we need to ensure the requesting user has permission (as it may not be me)

@canRoot and @canModal have the same issue as @canResolved if the resource is null, the gate check fails (as there is no resource it can't find the required Policy and thus returns unauthorised)

[k0ka]

I don't really get it.

if root is empty how can you get a property of that null object?

User { 
   name @canRoot("read")
}

@canRoot checks authorization on the User object. If the User object is null, the name won't be queried at all.

So the first thing you need to answer is "what object should be checked authorization against?". In your case it looks like you want to check if the actor has an access to the me object. And so you should use @canRoot.

[SlyDave]

1. collection: [Field!] @hasMany
   note the @hasmany, if you use @canRoot, you have no way to pass the object of the @hasmany to the Policy of the root, so you can't do a can root read object in HasMany, and if you use @canResolved, you can't get the Policy to test against if the resource is null because @canResolved wants the resolved object to find the Policy

2. root will be null if top level, so you can't use canRoot there either

3. "In your case it looks like you want to check if the actor has an access to the me object. And so you should use @canRoot."
   That is just for the example to show the inconsistency of the @canResolved function when used against an empty collection (which is what the issues is actually about). The actual use case could (and in our case) does have this applied in an object that can appear in me, or under other types too - think of a shared resource like "Address" where me { Address } and me { NextOfKin { Address } } are both valid, but so also is NextOfKin(userId: $userId) { Address } etc. -- so this is a general solution.

Hope that clears it up for you

[k0ka]

It doesn't made it much clearer. And some of your points are incorrect. However, let's stick to your main problem.

What do you want to achieve? As I understand, your simple schema is

me {
    collection: [Field] @canResolved(ability: "view")
}

The Field can be null. You want to check each Field in the collection and allow even if some of them are null.

It's Laravel restriction for the Policy usage - the object must not be null. However, you can define a global Gate manually, which would work for any object including null. Here is an example:

class AppServiceProvider extends ServiceProvider
{
    public function boot(): void
    {
        Gate::define('view', function (?User $user, $model) {
            return $model == null;
        });
    }
}

[SlyDave]

You're trying to solve for the wrong problem K0ka.

The ticket is quite clear that the behaviour of the @canResolved acts differently between empty collections (via @hasmany) and singular resolved values via @find. that's what the ticket is about. let's stick to that.

[spawnia]

We generally avoid nullable lists in our GraphQL schema, my company's coding guidelines say this:

Fields that return lists SHOULD be non-nullable and contain non-nullable elements.

For clients, it is easier to deal with the result of a field returning a list if:

• at least an empty list is returned instead of null
• the elements within this list are never null

type Foo {
- bars: [Bar]
+ bars: [Bar!]!
}

How is null semantically different from an empty list in case of your @hasMany field?

Given @canResolved acts on singular model instances, it makes sense to me that it does no validation for an empty list.

[SlyDave]

I'd argue that not returning an auth exception on an empty list when using @canResolved, exposes information. You now know there isn't a value. If there is a value it is authorised, and you either get an exception or you get the value.

Additionally, that behaviour differs to singular values. Where you either get an exception or you get the value. - So, it's another check that has to be put in place just for lists.

I agree with the generally avoiding advice and in this instance doesn't modify the outcome (it only changes use case 2 in the examples above, from an auth error to a schema error) and isn't enforced beyond policy (a good one at that), the code should likely deal with the situation in a consistent manner or become opinionated.

How is null semantically different from an empty list in case of your @hasmany field?

it isn't thus it should return the exception, no? as null will...

[k0ka]

I'd argue that not returning an auth exception on an empty list when using @canResolved, exposes information.

You can remove this exposure by either defining Gate globally, or by setting action to RETURN_VALUE and returnValue to `[]