xorgxx / doctrine-encryptor-bundle

This bundle provides Encrypt/Decrypt data sensible in a Db system in your application. Its main goal is to make it simple for you to manage encrypt & decrypt sensible data into Db! This bundle is to refresh the old bundle [DoctrineEncryptBundle]
MIT License
4 stars 0 forks source link

Entity not updating onFlush if null #1

Open Taity-mini opened 4 days ago

Taity-mini commented 4 days ago

Hi there, just testing out this bundle for encrypting sensitive information in my project's database.

Symfony version: 6.4.9 (LTS) PHP version: 8.3.7

I noticed one of our entities couldn't update when the encrypted setting was turned on. Two fields are set to be encrypted ( #[neoxEncryptor(build: "in")]) on this entity using the halite encryption options

Got this error:

#message: "EntityManager#persist() expects parameter 1 to be an entity object, NULL given."
  #code: 0
  #file: "./vendor/doctrine/orm/src/ORMInvalidArgumentException.php"
  #line: 190
  trace: {
    ./vendor/doctrine/orm/src/ORMInvalidArgumentException.php:190 { …}
    ./vendor/doctrine/orm/src/EntityManager.php:666 { …}
    ./vendor/xorgxx/doctrine-encryptor-bundle/src/EventSubscriber/DoctrineEncryptorSubscriber.php:151 { …}
    ./vendor/symfony/doctrine-bridge/ContainerAwareEventManager.php:63 { …}
    ./vendor/doctrine/orm/src/UnitOfWork.php:3810 { …}
    ./vendor/doctrine/orm/src/UnitOfWork.php:425 { …}
    ./vendor/doctrine/orm/src/EntityManager.php:403 { …}

As a quick fix, I added this line after line 145 and the entity was able to persist successfully

if(is_null($neoxEncryptorEntity)){
                       continue;
}

Any assistance on the above error would be greatly appreciated. Great work on the bundle so far and looking forward to future updates :)

xorgxx commented 3 days ago

Hi @Taity-mini thank, so can you provide more code entity class ? so i can reproduce this issue. Quick fix as you provide is to

 if(is_null($neoxEncryptorEntity)){
                       continue;
}

but i need to see and check the exact issue context.

Taity-mini commented 3 days ago

Hi @xorgxx ,

Thanks for the quick response.

The entity class is for Xero API tokens and a separate entity is used for tenants. The primary tenant relationship is for end-users to switch which tenant is used to send/receive invoices from.

The issue occurs when updating the XeroDetails, for new access and refresh tokens (this runs from the command line).

If you require any additional details then please let me know. XeroDetails.php

<?php

namespace App\Entity;

use App\Repository\XeroDetailsRepository;
use DateTime;
use DateTimeInterface;
use Doctrine\Common\Collections\ArrayCollection;
use Doctrine\Common\Collections\Collection;
use Doctrine\DBAL\Types\Types;
use Doctrine\ORM\Mapping as ORM;
use DoctrineEncryptor\DoctrineEncryptorBundle\Attribute\NeoxEncryptor;

#[ORM\Entity(repositoryClass: XeroDetailsRepository::class)]
#[ORM\Table(name: 'xero_details')]
#[ORM\HasLifecycleCallbacks]
class XeroDetails 
{
    #[ORM\Id]
    #[ORM\GeneratedValue]
    #[ORM\Column]
    private ?int $id = null;

    #[ORM\Column(length: 255)]
    private ?string $userId = null;

    #[ORM\Column(type: 'text')]
    #[neoxEncryptor(build: "in")]
    private ?string $accessToken = null;

    #[ORM\Column(length: 1000)]
    #[neoxEncryptor(build: "in")]
    private ?string $refreshToken = null;

    #[ORM\Column(type: Types::DATETIME_MUTABLE)]
    private ?DateTimeInterface $accessTokenExpiry = null;

    #[ORM\Column(type: Types::DATETIME_MUTABLE)]
    private ?DateTimeInterface $refreshTokenExpiry = null;

    #[ORM\Column(type: Types::DATETIME_MUTABLE)]
    private ?DateTimeInterface $createdAt = null;

    #[ORM\Column(type: Types::DATETIME_MUTABLE, nullable: true)]
    private ?DateTimeInterface $updatedAt = null;

    /**
     * @var Collection<int, XeroConnectionDetails>
     */
    #[ORM\OneToMany(mappedBy: 'user', targetEntity: XeroConnectionDetails::class, cascade: ['persist', 'remove'])]
    private Collection $tenants;

    #[ORM\OneToOne(inversedBy: 'xeroDetails', cascade: ['persist', 'remove'])]
    #[ORM\JoinColumn(nullable: true)]
    private ?XeroConnectionDetails $primaryTenant = null;

    public function __construct()
    {
        $this->tenants = new ArrayCollection();
    }

    public function getId(): ?int
    {
        return $this->id;
    }

    public function getUserId(): ?string
    {
        return $this->userId;
    }

    public function setUserId(string $userId): static
    {
        $this->userId = $userId;

        return $this;
    }

    public function getAccessToken(): ?string
    {
        return $this->accessToken;
    }

    public function setAccessToken(?string $accessToken): void
    {
        $this->accessToken = $accessToken;
    }

    public function getRefreshToken(): ?string
    {
        return $this->refreshToken;
    }

    public function setRefreshToken(string $refreshToken): static
    {
        $this->refreshToken = $refreshToken;

        return $this;
    }

    public function getAccessTokenExpiry(): ?DateTimeInterface
    {
        return $this->accessTokenExpiry;
    }

    public function setAccessTokenExpiry(DateTimeInterface $accessTokenExpiry): static
    {
        $this->accessTokenExpiry = $accessTokenExpiry;

        return $this;
    }

    public function getRefreshTokenExpiry(): ?DateTimeInterface
    {
        return $this->refreshTokenExpiry;
    }

    public function setRefreshTokenExpiry(DateTimeInterface $refreshTokenExpiry): static
    {
        $this->refreshTokenExpiry = $refreshTokenExpiry;

        return $this;
    }

    public function getCreatedAt(): ?DateTimeInterface
    {
        return $this->createdAt;
    }

    public function setCreatedAt(DateTimeInterface $createdAt): static
    {
        $this->createdAt = $createdAt;

        return $this;
    }

    public function getUpdatedAt(): ?DateTimeInterface
    {
        return $this->updatedAt;
    }

    public function setUpdatedAt(?DateTimeInterface $updatedAt): static
    {
        $this->updatedAt = $updatedAt;

        return $this;
    }

    #[ORM\PrePersist]
    #[ORM\PreUpdate]
    public function updatedTimestamps(): void
    {
        $this->setUpdatedAt(new DateTime('now'));
        if ($this->getCreatedAt() == null) {
            $this->setCreatedAt(new DateTime('now'));
        }
    }

    /**
     * @return Collection<int, XeroConnectionDetails>
     */
    public function getTenants(): Collection
    {
        return $this->tenants;
    }

    public function addTenant(XeroConnectionDetails $tenant): static
    {
        if (!$this->tenants->contains($tenant)) {
            $this->tenants->add($tenant);
            $tenant->setUserId($this);
        }

        return $this;
    }

    public function removeTenant(XeroConnectionDetails $tenant): static
    {
        if ($this->tenants->removeElement($tenant)) {
            // set the owning side to null (unless already changed)
            if ($tenant->getUserId() === $this) {
                $tenant->setUserId(null);
            }
        }

        return $this;
    }

    public function getPrimaryTenant(): ?XeroConnectionDetails
    {
        return $this->primaryTenant;
    }

    public function setPrimaryTenant(XeroConnectionDetails $primaryTenant): static
    {
        $this->primaryTenant = $primaryTenant;

        return $this;
    }
}

XeroConnectionDetails.php

<?php

namespace App\Entity;

use App\Repository\XeroConnectionDetailsRepository;
use DateTime;
use Doctrine\DBAL\Types\Types;
use Doctrine\ORM\Mapping as ORM;

#[ORM\Entity(repositoryClass: XeroConnectionDetailsRepository::class)]
#[ORM\Table(name: 'xero_connection_details')]
#[ORM\HasLifecycleCallbacks]
class XeroConnectionDetails
{
    #[ORM\Id]
    #[ORM\GeneratedValue]
    #[ORM\Column]
    private ?int $id = null;

    #[ORM\ManyToOne(inversedBy: 'tenants')]
    #[ORM\JoinColumn(nullable: false)]
    private ?XeroDetails $user = null;

    #[ORM\Column(length: 255)]
    private ?string $name = null;

    #[ORM\Column(length: 255)]
    private ?string $tenantId = null;

    #[ORM\Column(type: Types::DATETIME_MUTABLE)]
    private ?\DateTimeInterface $createdAt = null;

    #[ORM\Column(type: Types::DATETIME_MUTABLE, nullable: true)]
    private ?\DateTimeInterface $updatedAt = null;

    #[ORM\OneToOne(mappedBy: 'primaryTenant', cascade: ['persist', 'remove'])]
    private ?XeroDetails $xeroDetails = null;

    public function getId(): ?int
    {
        return $this->id;
    }

    public function getUser(): ?XeroDetails
    {
        return $this->user;
    }

    public function setUser(?XeroDetails $user): void
    {
        $this->user = $user;
    }

    public function getName(): ?string
    {
        return $this->name;
    }

    public function setName(string $name): static
    {
        $this->name = $name;

        return $this;
    }

    public function getTenantId(): ?string
    {
        return $this->tenantId;
    }

    public function setTenantId(string $tenantId): static
    {
        $this->tenantId = $tenantId;

        return $this;
    }

    public function getCreatedAt(): ?\DateTimeInterface
    {
        return $this->createdAt;
    }

    public function setCreatedAt(\DateTimeInterface $createdAt): static
    {
        $this->createdAt = $createdAt;

        return $this;
    }

    public function getUpdatedAt(): ?\DateTimeInterface
    {
        return $this->updatedAt;
    }

    public function setUpdatedAt(?\DateTimeInterface $updatedAt): static
    {
        $this->updatedAt = $updatedAt;

        return $this;
    }

    #[ORM\PrePersist]
    #[ORM\PreUpdate]
    public function updatedTimestamps(): void
    {
        $this->setUpdatedAt(new DateTime('now'));
        if ($this->getCreatedAt() == null) {
            $this->setCreatedAt(new DateTime('now'));
        }
    }

    public function getXeroDetails(): ?XeroDetails
    {
        return $this->xeroDetails;
    }

    public function setXeroDetails(XeroDetails $xeroDetails): static
    {
        // set the owning side of the relation if necessary
        if ($xeroDetails->getPrimaryTenant() !== $this) {
            $xeroDetails->setPrimaryTenant($this);
        }

        $this->xeroDetails = $xeroDetails;

        return $this;
    }

    public function isPrimaryTenant(): bool
    {
        if($this->xeroDetails !== null){
            return true;
        }
        return false;

    }
}
xorgxx commented 3 days ago

Thanks! So I think I have found it: you mentioned that the entity can be null.

   #[ORM\OneToOne(inversedBy: 'xeroDetails', cascade: ['persist', 'remove'])]
    #[ORM\JoinColumn(nullable: true)] <------- HERE joint fix DoctrineEncryptor to provide null on joint --------
    private ?XeroConnectionDetails $primaryTenant = null;

And of course, I did not implement this! So I will fix this issue. VERY SOON !!!

Taity-mini commented 3 days ago

Perfect, thanks a lot! 👍

Yes, I had to make that entity relationship NULL initially, as the Xero Details row is created first then the tenant connection afterwards.

Taity-mini commented 3 days ago

While I remember, the Symfony profiler flags these as deprecations, if you are planning a new release version: image

xorgxx commented 2 days ago

I have looked at the code and fixed this issue!

Normally, an API REST token is created for a short duration, like 3 hours to 1 day, and then it is automatically destroyed for security purposes. This makes me wonder why we would want to "encrypt" this type of data [TOKEN] ? This bundle is more intended for encrypting sensitive data such as names, emails, credit card numbers, phone numbers, etc.

The PR will be updated during week 28. Please let me know if you are still experiencing any issues.

Regarding depreciation, it will take more time !!

Regards.

Taity-mini commented 11 hours ago

Thanks for the quick fix! I will test it out once the new release version is updated.

Ah I see, yeah there will be other parts of the system with sensitive/personal information that will also be encrypted. One of the API tokens is valid for 60 days, so I decided to try this bundle to secure it.

No worries, I just thought I'd highlight those depreciations 😄

xorgxx commented 9 hours ago

Ok. I haven't tried all functionalities using the API, but it should do the job the same way.