Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Security annotation use results in array_combine() error #339

Open
sanderlissenburg opened this issue Apr 28, 2021 · 6 comments
Open

Security annotation use results in array_combine() error #339

sanderlissenburg opened this issue Apr 28, 2021 · 6 comments
Labels
help wanted Extra attention is needed

Comments

@sanderlissenburg
Copy link

sanderlissenburg commented Apr 28, 2021

I'm trying to use a custom method for the security test as documented under "Accessing the current object".

https://graphqlite.thecodingmachine.io/docs/fine-grained-security#accessing-the-current-object

But this results in the following error.

array_combine(): Both parameters should have an equal number of elements

For example.

/**
 * @Type(class=LocationModel::class)
 */
final class LocationType
{
    /**
     * @Field
     * @Security("this.isAdmin(user)")
     * @param LocationModel $locationModel
     * @return string
     */
    public function name(LocationModel $locationModel): string
    {
        return $locationModel->name;
    }


    public function isAdmin(User $user): bool
    {
        return $user->getType() === 'admin';
    }
}
        #message: "array_combine(): Both parameters should have an equal number of elements"
        #code: 0
        #file: "/app/vendor/thecodingmachine/graphqlite/src/Middlewares/SecurityFieldMiddleware.php"
        #line: 132
        #severity: E_WARNING
        trace: {
          /app/vendor/thecodingmachine/graphqlite/src/Middlewares/SecurityFieldMiddleware.php:132 {
            Laminas\Stratigility\Middleware\ErrorHandler->Laminas\Stratigility\Middleware\{closure} …
            › $argsName = array_keys($parameters);
            › $argsByName = array_combine($argsName, $args);
            › Assert::isArray($argsByName);
          }
          Laminas\Stratigility\Middleware\ErrorHandler->Laminas\Stratigility\Middleware\{closure}() {}
          /app/vendor/thecodingmachine/graphqlite/src/Middlewares/SecurityFieldMiddleware.php:132 {
            TheCodingMachine\GraphQLite\Middlewares\SecurityFieldMiddleware->getVariables(array $args, array $parameters, ResolverInterface $callable): array …
            › $argsName = array_keys($parameters);
            › $argsByName = array_combine($argsName, $args);
            › Assert::isArray($argsByName);
            arguments: {
              $keys: []
              $values: array:1 [ …1]
            }
          }
          /app/vendor/thecodingmachine/graphqlite/src/Middlewares/SecurityFieldMiddleware.php:88 {
            TheCodingMachine\GraphQLite\Middlewares\SecurityFieldMiddleware->TheCodingMachine\GraphQLite\Middlewares\{closure} …
            › $queryFieldDescriptor->setResolver(function (...$args) use ($securityAnnotations, $resolver, $failWith, $parameters, $queryFieldDescriptor, $originalResolver) {
            ›     $variables = $this->getVariables($args, $parameters, $originalResolver);
            › 
            arguments: {
              $args: array:1 [ …1]
              $parameters: []
              $callable: TheCodingMachine\GraphQLite\Middlewares\ServiceResolver {#2401 …}
            }
          }
         ...

I'm using version 4.1.2.

@oojacoboo
Copy link
Collaborator

Please provide a stack-trace.

@sanderlissenburg
Copy link
Author

sanderlissenburg commented Apr 28, 2021

Not sure how to get a decent track trace because it's formatted to a JSON result by the Graphql middleware. With some var_dumping and die() I got this. Hope it's enough. I have added it to the initial question/comment.

@oojacoboo
Copy link
Collaborator

oojacoboo commented Apr 30, 2021

@sanderlissenburg without digging into this bit of the code further, I'm not entirely certain here. Can you verify that you have an active User session/object available?

We don't use this User "session management stuff" with GraphQLite, as I've stated my opinion on this before, I'm not a fan of this library getting tangled up in the User logic of an application. I, personally, would never advise it's use. IMO, all of your User logic should be managed outside of this library. The @Security annotation is great though, just get your User object within that method, or via a service that method leverages. Parameter injecting is just a really bad pattern IMO.

@sanderlissenburg
Copy link
Author

is_granted works, so I'm pretty sure the user object is available. But even when I don't inject any parameter in the method. I get this error. For example

final class Foo
{
    /**
     * @Field
     * @Security("this.foo()")
     * @param FooModel $fooModel
     * @return string
     */
    public function name(FooModel $fooModel): string
    {
        return $fooModel->bar;
    }

    public function foo(): bool
    {
        return false;
    }
}

@oojacoboo oojacoboo added the help wanted Extra attention is needed label Jun 15, 2021
@vyaaki
Copy link

vyaaki commented Sep 3, 2021

Same problem vendor/thecodingmachine/graphqlite/src/Middlewares/SecurityFieldMiddleware.php:132. Here array $parameters is empty, becouse of array_shift at vendor/thecodingmachine/graphqlite/src/FieldsBuilder.php:378, when injectSource class

@oojacoboo
Copy link
Collaborator

@vyaaki be great if you could submit a PR with tests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

3 participants