api-platform / core

The server component of API Platform: hypermedia and GraphQL APIs in minutes
https://api-platform.com
MIT License
2.4k stars 859 forks source link

[GraphQL] Context Builder problem #3040

Closed NMathar closed 5 years ago

NMathar commented 5 years ago

Hi all,

i got a problem with the contextBuilder for GraphQL. I use the current master (2.5) of api-platform with php 7.2 on Ubuntu 18.04.3 LTS.

No one can read or update the "deleted" attribute of the domain entity. I get always

on collection query

{
  "errors": [
    {
      "message": "Cannot query field \"deleted\" on type \"DomainCollection\".",
      "extensions": {
        "category": "graphql"
      },
      "locations": [
        {
          "line": 7,
          "column": 5
        }
      ]
    }
  ]
}

or on update

{
  "errors": [
    {
      "message": "Variable \"$input\" got invalid value {\"id\":\"\\\/api\\\/domain\\\/1\",\"deleted\":true}; Field \"deleted\" is not defined by type updateDomainInput.",
      "extensions": {
        "category": "graphql"
      },
      "locations": [
        {
          "line": 1,
          "column": 11
        }
      ]
  }
]
}

Any ideas what i am doing wrong? It looks like some processes start first and throw these errors before contextBuilder can start and add some more context groups.

My files looks like this:

services:
     'App\Serializer\AdminContextBuilder':
        decorates: 'api_platform.graphql.serializer.context_builder'
        arguments: [ '@App\Serializer\AdminContextBuilder.inner' ]
        autoconfigure: false

Serializer/AdminContextBuilder.php

<?php
namespace App\Serializer;

use ApiPlatform\Core\GraphQl\Serializer\SerializerContextBuilderInterface;
use Symfony\Component\Security\Core\Authorization\AuthorizationCheckerInterface;

final class AdminContextBuilder implements SerializerContextBuilderInterface
{
    private $decorated;
    private $authorizationChecker;

    public function __construct(SerializerContextBuilderInterface $decorated, AuthorizationCheckerInterface $authorizationChecker)
    {
        $this->decorated = $decorated;
        $this->authorizationChecker = $authorizationChecker;
    }

    public function create(?string $resourceClass, string $operationName, array $resolverContext, bool $normalization): array
    {
        $context = $this->decorated->create($resourceClass, $operationName, $resolverContext, $normalization);

        if (isset($context['groups']) && $this->authorizationChecker->isGranted('ROLE_ADMIN')) {
            if ($normalization) {
                // Add admin:read for normalization.
                $context['groups'][] = 'admin:read';
            } else {
                // Add admin:write contexts for denormalization.
                $context['groups'][] = 'admin:write';
            }
        }
        return $context;
    }
}

Entity

<?php

namespace App\Entity;

use ApiPlatform\Core\Annotation\ApiProperty;
use ApiPlatform\Core\Annotation\ApiResource;
use Doctrine\ORM\Mapping as ORM;
use Symfony\Component\Serializer\Annotation\Groups;
use Symfony\Component\Validator\Constraints as Assert;

/**
 *  @ApiResource(
 *     normalizationContext={"groups"={"read"}},
 *     denormalizationContext={"groups"={"write"}}
 * )
 * @ORM\Entity()
 * @ORM\Table(name="domain")
 */
class Domain {
    /**
     * @ORM\Column(type="integer")
     * @ORM\Id
     * @ORM\GeneratedValue(strategy="AUTO")
     * @Groups({"read"})
     */
    private $id;

    /**
     * @ORM\Column(type="string", length=200)
     * @Assert\NotBlank()
     * @Groups({"read", "write"})
     */
    private $domain;

    /**
     * @ORM\Column(type="boolean", nullable=true)
     * @Groups({"admin:read","admin:write"})
     * @var bool $deleted
     */
    private $deleted = false;

/**
     * @return mixed
     */
    public function getId()
    {
        return $this->id;
    }

    /**
     * @param mixed $id
     */
    public function setId($id): void
    {
        $this->id = $id;
    }

    /**
     * @return mixed
     */
    public function getDomain()
    {
        return $this->domain;
    }

    /**
     * @param mixed $domain
     */
    public function setDomain($domain): void
    {
        $this->domain = $domain;
    }

/**
     * @return bool
     */
    public function isDeleted(): bool
    {
        return $this->deleted;
    }

    /**
     * @param bool $deleted
     */
    public function setDeleted(bool $deleted): void
    {
        $this->deleted = $deleted;
    }
}
alanpoulain commented 5 years ago

I think it's because the deleted field is not queryable / mutable when the schema is built. Maybe a solution would be to add the read / write groups and "remove" (or do not add) them in the context builder.

NMathar commented 5 years ago

Now i get this error on query and update :thinking:

"errors": [
    {
      "debugMessage": "Cannot return null for non-nullable field Domain._id.",
      "message": "Internal server error",
      "extensions": {
        "category": "internal"
      },
      "locations": [
        {
          "line": 4,
          "column": 7
        }
      ],

I added your suggestion like this:

    /**
     * @ORM\Column(type="boolean", nullable=true)
     * @Groups({"read", "admin:read", "write", "admin:write"})
     * @var bool $deleted
     */
    private $deleted = false;
            if ($normalization) {
                // Add admin:read for normalization.
                $pos = array_search('read',  $context['groups']);
                unset($context['groups'][$pos]);
                $context['groups'][] = 'admin:read';
            } else {
                // Add admin:write contexts for denormalization.
                $pos = array_search('write',  $context['groups']);
                unset($context['groups'][$pos]);
                $context['groups'][] = 'admin:write';
            }
alanpoulain commented 5 years ago

Because of the groups, the id is null whereas it shouldn't.

NMathar commented 5 years ago

Ok now its running but it has no effect every user can read and write "deleted". Thats not right :laughing: Are you sure that this use case is possible at the moment?

alanpoulain commented 5 years ago

Yes, I think it is. WDYM by "it has no effect"? You can mutate the field, but it will not change the underlying entity. And you can query it, but it will be null.

NMathar commented 5 years ago

It has no permission effect. The edit and query of the deleted attribute is callable for every user and not only for the administrator.

alanpoulain commented 5 years ago

It is callable because the schema is not dynamically built. It's a wanted behavior.

NMathar commented 5 years ago

ok think i dont get the functionality behind the ContextBuilder then.

I thought i can limit the access of some attributes with it and the attributes are then limited by a ROLE.

It is not working because only the normalizationContext groups and the denormalizationContext groups will be considered. The context groups cant limit access to some attributes because they were overridden by normalizationContext groups or denormalizationContext groups i am right?

Is there another way to archive that in graphql or are there any plans to implement that?

Sorry for wasting your time :(

alanpoulain commented 5 years ago

The GraphQL system in API Platform works in two "phases":

NMathar commented 5 years ago

ok now i get the functionality and i got a working process can you short prove it? The attributes are now null and will be not persisted into the database if the group was not granted.

Great work only my imagination of the functionality was wrong.

Serializer/AdminContextBuilder.php

<?php
namespace App\Serializer;

use ApiPlatform\Core\GraphQl\Serializer\SerializerContextBuilderInterface;
use Symfony\Component\Security\Core\Authorization\AuthorizationCheckerInterface;

final class AdminContextBuilder implements SerializerContextBuilderInterface
{
    private $decorated;
    private $authorizationChecker;

    public function __construct(SerializerContextBuilderInterface $decorated, AuthorizationCheckerInterface $authorizationChecker)
    {
        $this->decorated = $decorated;
        $this->authorizationChecker = $authorizationChecker;
    }

    public function create(?string $resourceClass, string $operationName, array $resolverContext, bool $normalization): array
    {
        $context = $this->decorated->create($resourceClass, $operationName, $resolverContext, $normalization);

        if (isset($context['groups'])) {
            if ($this->authorizationChecker->isGranted('ROLE_ADMIN')) {
                $normalization ? $context['groups'][] = 'admin:read' : $context['groups'][] = 'admin:write';
            }else{
                unset($context['groups']);
                $normalization ? $context['groups'][] = 'user:read' : $context['groups'][] = 'user:write';
            }
        }
        return $context;
    }
}

Entity

<?php

namespace App\Entity;

use ApiPlatform\Core\Annotation\ApiProperty;
use ApiPlatform\Core\Annotation\ApiResource;
use Doctrine\ORM\Mapping as ORM;
use Symfony\Component\Serializer\Annotation\Groups;
use Symfony\Component\Validator\Constraints as Assert;

/**
 *  @ApiResource(
 *     normalizationContext={"groups"={"user:read", "admin:read"}},
 *     denormalizationContext={"groups"={"user:write", "admin:write"}}
 * )
 * @ORM\Entity()
 * @ORM\Table(name="domain")
 */
class Domain {
/**
     * @ORM\Column(type="integer")
     * @ORM\Id
     * @ORM\GeneratedValue(strategy="AUTO")
     * @Groups({"user:read"})
     */
    private $id;

    /**
     * @ORM\Column(type="string", length=200)
     * @Assert\NotBlank()
     * @Groups({"user:read", "user:write"})
     */
    private $domain;

    /**
     * @ORM\Column(type="boolean", nullable=true)
     * @Groups({"admin:read", "admin:write"})
     * @var bool $deleted
     */
    private $deleted = false;

/**
     * @return mixed
     */
    public function getId()
    {
        return $this->id;
    }

    /**
     * @param mixed $id
     */
    public function setId($id): void
    {
        $this->id = $id;
    }

    /**
     * @return mixed
     */
    public function getDomain()
    {
        return $this->domain;
    }

    /**
     * @param mixed $domain
     */
    public function setDomain($domain): void
    {
        $this->domain = $domain;
    }

/**
     * @return bool
     */
    public function isDeleted(): bool
    {
        return $this->deleted;
    }

    /**
     * @param bool $deleted
     */
    public function setDeleted(bool $deleted): void
    {
        $this->deleted = $deleted;
    }
}
eradionov commented 2 years ago

Seems like this functionality is still not working Could it be removed from documentation in this case? https://api-platform.com/docs/core/graphql/#changing-the-serialization-context-dynamically