Model is not found when a field other than id specified for authorization

[Mecit]

Hello. I'm trying to fetch a model by a field that's not id to check if the authenticated user is has the permission to view it. Consider the following scenario:

The database has the tables named users and sites and a user can view a site only if they belong to the site. So, the database structure is roughly as follows:

users
----------
id
name
site_id
*other fields*

sites
----------
id
domain
*other fields*

My authorization check defined in AuthServiceProvider.php is as follows:

        Gate::define('view site', function ($user, Site $site) {
            return $user->site_id === $site->id;
        });

So, when I define the query like site(id: ID! @eq): Site @find @can(ability: "view site" find: "id"), everything works just fine, but when it's site(domain: String! @eq): Site @find @can(ability: "view site" find: "domain"), I get an error saying "No query results for model [App\Site] example".

Does find work only with primary keys? The api reference in the docs goes like:

"""
  The name of the argument that is used to find a specific model
  instance against which the permissions should be checked.
"""
  find: String

[spawnia]

As of now, only primary keys work: https://github.com/nuwave/lighthouse/blob/50a6b714c40766ac308dc7e25d5b64291963ce3f/src/Schema/Directives/CanDirective.php#L147

The documentation is unclear about this, so we should at least fix that. (Update: I just did.)

I see how it might be valuable to allow for more flexible search mechanisms, but wonder how we could do that. The reason for the current design was that it works nicely with Mutations, e.g.:

type Mutation {
  updatePost(
    id: ID!
    content: String!
  ): Post @can(ability: "update post", find: "id")
}

[Mecit]

Wouldn't something like $enhancedBuilder->where($find, $findValue)->firstOrFail(); work?

[spawnia]

Wouldn't something like $enhancedBuilder->where($find, $findValue)->firstOrFail(); work?

Not necessarily, as find references the name of the field argument, which might be different from the actual column name.

We might simply do the same as in other directives such as @all or @find and apply the argument filters. I am a bit worried if that might be problematic for security somehow, will have to think that through.

[Jofairden]

I actually have a usecase for this as well, needing to check if a certain ID from a column (not ID!) is present in another table. Specifically a customer's plan and if they have access to the feature (the query if you will), and it kind of looks as follows: $hasFeature = in_array($feature->id, $user->plan->datafeatures()->pluck('datafeatures.id')->all()); However, there's another ID field on the datafeatures that I need to check somewhere else, but I can't get the ID, I'm assuming it's related to this issue.

[benkingcode]

I'm testing out Lighthouse and I think this might be a hard blocker for me to adopt it... I've got a multi-tenant app split up by a "workspace" model that uses a human-friendly slug in the URL, and has auto-incrementing IDs. In order to be able to use the @can guard, it sounds like I'll need to change the primary key to the slug?

[spawnia]

@dbbk if the functionality of a built-in directive does not fit you, there is always the option of creating a custom field directive: https://lighthouse-php.com/master/custom-directives/field-directives.html

You can even override the name @can if you want to keep it the same, although I recommend picking a distinct name.

[benkingcode]

@spawnia Oh I'll check that out, thanks!

Nonetheless, it feels like this would be a common enough problem to fix in the core - although I'm still new to GraphQL so I may be wrong about how to model it (still trying to figure out how the special ID scalar works in particular), but I could see human-readable slugs that differ from the model's primaryKey being used in a lot of apps.

[spawnia]

As I said in my comment above:

We might simply do the same as in other directives such as @all or @find and apply the argument filters. I am a bit worried if that might be problematic for security somehow, will have to think that through.

That would work out of the box for queries that are supposed to find a certain model through argument filters, those should have filters such as @eq already.

Mutations, such as @update, don't have those yet; but I currently don't see why they could not just use the same filters.

[dmouse]

I'm using this configuration and also its not working, debuging the protected function modelsToCheck the args variable is empty

type Content {
   user: User!@belongsTo @can(ability: "ownerOrAdmin", model: "App\\Models\\Content", find: "id")
}

extend type Query {
    content(
        id: ID @eq
    ): Content @find
}

the result is Got no key to find a model at the expected input path: id.

[br1ge]

Try this: site(domain: String! @eq): Site @find @can(ability: "view site", query: true)