Pass query/mutation context into the field privacy detection for better access control

[torunar]

Summary

Currently privacy in the field specification can access only the query/mutation arguments.
This creates difficulties in the applications where access to the specific field is based on the current query/mutation context, and the context itself is the robust object which contains user, site area (back office / client area), etc.

This results in quirky workarounds like registering context globally in the app and accessing it via facades.
This PR is fixes this issue by passing query/mutation context to the privacy handlers, so this:

'privacy' => static function (array $args) {
    $ctx = app(Context::class);
    return $ctx->isAdministrator();
},

becomes this:

'privacy' => static function (array $args, $ctx) {
    return $ctx->isAdministrator();
},

---

Type of change:

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• This change requires a documentation update
• Misc. change (internal, infrastructure, maintenance, etc.)

Checklist:

• Existing tests have been adapted and/or new tests have been added
• Add a CHANGELOG.md entry
• Update the README.md
• Code style has been fixed via composer fix-style

[mfn]

Just a quick feedback:

• makes sense to me!
• might be a breaking change though? (function signature change)
  "no a problem" though, we can just bump the major version and document it properly

=> will review later

[mfn]

So far I can already say: please add a test!

Also, a changelog entry as I think this is the way forward.

Are there any other arguments we might want to consider passing (like we do to the resolver)?

[torunar]

might be a breaking change though? (function signature change)

Existing privacy handlers will be compatible: calling function with more parameters that defined in its signature won't produce an error.

You are right, it breaks compatibility for the privacy policies defined in the class, this looks like a change for the major version then.

[torunar]

Hi @mfn,

I've added tests, changelog entries and marked this PR as the one with breaking changes. Thank you for the feedback, I was so occupied with closure-based privacy handlers, I've completely missed the class-based ones.

Are there any other arguments we might want to consider passing (like we do to the resolver)?

I currently don't see use cases where we need anything but query arguments and its context to decide whether a field is accessible in the context of the current operation. All the extra can be passed via the class privacy handler constructor and auto-wired here:

graphql-laravel/src/Support/SelectFields.php

Line 335 in 468e593

$instance = app($privacyClass);

[torunar]

@mfn

May I ask for your help with the code analysis checks?
When I run composer phpstan locally, I get the following errors:

 ------ -----------------------------------------------------------------------
  Line   src/Support/SelectFields.php
 ------ -----------------------------------------------------------------------
         Ignored error pattern #^Parameter \#1 \$callback of function
         call_user_func expects callable\(\)\: mixed, array\(\*NEVER\*,
         'fire'\) given\.$# in path
         /Users/mschekotov/srv/src/graphql-laravel/src/Support/SelectFields.ph
         p was not matched in reported errors.
         Ignored error pattern #^Parameter \#1 \$callback of function
         call_user_func expects callable\(\)\: mixed, array\(\*NEVER\*,
         mixed\) given\.$# in path
         /Users/mschekotov/srv/src/graphql-laravel/src/Support/SelectFields.ph
         p was not matched in reported errors.
  203    Parameter #1 $function of function call_user_func expects callable():
         mixed, array(*NEVER*, mixed) given.
 ------ -----------------------------------------------------------------------

 ------ ----------------------------------------------------------------------
  Line   src/Support/Type.php
 ------ ----------------------------------------------------------------------
         Ignored error pattern #^Parameter \#1 \$callback of function
         call_user_func_array expects callable\(\)\: mixed,
         array\(\$this\(Rebing\\GraphQL\\Support\\Type\), string\) given\.$#
         in path
         /Users/mschekotov/srv/src/graphql-laravel/src/Support/Type.php was
         not matched in reported errors.
  68     Parameter #1 $function of function call_user_func_array expects
         callable(): mixed, array($this(Rebing\GraphQL\Support\Type), string)
         given.
 ------ ----------------------------------------------------------------------

 ------ ------------------------------------------------------------------
  Line   src/routes.php
 ------ ------------------------------------------------------------------
         Ignored error pattern #^Parameter \#2 \$callback of function
         array_filter expects \(callable\(mixed, mixed\)\: bool\)\|null,
         Closure\(array, string\)\: bool\|null given\.$# in path
         /Users/mschekotov/srv/src/graphql-laravel/src/routes.php was not
         matched in reported errors.
  40     Parameter #2 $callback of function array_filter expects
         callable(mixed, mixed): bool, Closure(array, string): bool|null
         given.
 ------ ------------------------------------------------------------------

 ------ ------------------------------------------------------------------------
  Line   tests/Database/SelectFields/ValidateFieldTests/PrivacyQueryContext.php
 ------ ------------------------------------------------------------------------
  15     Method
         Rebing\GraphQL\Tests\Database\SelectFields\ValidateFieldTests\Privacy
         QueryContext::validate() has parameter $queryArgs with no value type
         specified in iterable type array.
         💡 See:
         https://phpstan.org/blog/solving-phpstan-no-value-type-specified-in-i
         terable-type
 ------ ------------------------------------------------------------------------

But when the same action is performed via GitHub, it produces only 2 errors:

 ------ ----------------------------------------------------------------------- 
  Line   src/Support/SelectFields.php                                           
 ------ ----------------------------------------------------------------------- 
         Ignored error pattern #^Parameter \#1 \$callback of function           
         call_user_func expects callable\(\)\: mixed, array\(\*NEVER\*,         
         'fire'\) given\.$# in path                                             
         /home/runner/work/graphql-laravel/graphql-laravel/src/Support/SelectF  
         ields.php was not matched in reported errors.                          
 ------ ----------------------------------------------------------------------- 

 ------ ------------------------------------------------------------------------ 
  Line   tests/Database/SelectFields/ValidateFieldTests/PrivacyQueryContext.php  
 ------ ------------------------------------------------------------------------ 
  15     Method                                                                  
         Rebing\GraphQL\Tests\Database\SelectFields\ValidateFieldTests\Privacy   
         QueryContext::validate() has parameter $queryArgs with no value type    
         specified in iterable type array.                                       
         💡 See:                                                                  
         https://phpstan.org/blog/solving-phpstan-no-value-type-specified-in-i   
         terable-type                                                            
 ------ ------------------------------------------------------------------------ 

[mfn]

But when the same action is performed via GitHub, it produces only 2 errors:

Ah yeah, super annoying: because of the difference of the PHP version I think. Github Action is using 8.0, see

graphql-laravel/.github/workflows/analysis.yml

Line 31 in 4a12c13

php-version: 8.0

But: don't worry, I'm not rejecting anyones work due to this ;) Once I've time for the review, I'll take care of it!

[mfn]

LGTM, thanks for the PR!

I fixed the phpstan baseline and some other small things since I was already on it.

[mfn]

PS: that was really as solid PR for a first time contribution, thank you 🙏