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

GDPR #404

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

GDPR #404

wants to merge 1 commit into from

Conversation

unixslayer
Copy link
Member

No description provided.

@unixslayer unixslayer self-assigned this Oct 30, 2024
@lifinsky
Copy link
Contributor

@unixslayer It should be a part of State / ES aggregates functionality. The need to encrypt data using symmetric or asymmetric encryption is quite a popular task

@lifinsky
Copy link
Contributor

The automatic encryption/decryption process can be applied to command or event attributes using PHP attributes (more abstract than PersonalData)

@unixslayer
Copy link
Member Author

@unixslayer It should be a part of State / ES aggregates functionality. The need to encrypt data using symmetric or asymmetric encryption is quite a popular task

@lifinsky at first we are focusing on ES aggregates as State are persisted differently and may require different approach.

The automatic encryption/decryption process can be applied to command or event attributes using PHP attributes (more abstract than PersonalData)

Can you elaborate more? I think I don't follow. Example may be welcome.

@jlabedo
Copy link
Contributor

jlabedo commented Oct 31, 2024

I am not a big fan of "GDPR" for naming the package: I'm willing to bet that in x years a new law will be called something like PROTECT (Personal Rights Over Transparency and Ethical Consent Transmission) or something else and GDPR would not have any meaning anymore :) And I guess there is this sort of regulation in the US also.
What do you think of PersonalDataProtection or DataProtection or PIIModule ?

@unixslayer
Copy link
Member Author

@jlabedo DataProtection sounds most neutral I guess.

@lifinsky
Copy link
Contributor

Encryption/decryption process can be applied to any sensitive data. Payment, card data, anything. It can also be part of the tokenization process or PCI DSS

@lifinsky
Copy link
Contributor

How about making it part of the event sourcing configuration or providing the ability to use for any DTO/VO attributes? PHP has nice feature SensitiveParameter. SensitiveData as naming idea.

@lifinsky
Copy link
Contributor

At the moment, in a similar problem, my team concentrated the process of encryption and decryption in VO from an abstract class

@lifinsky
Copy link
Contributor

/**
 * @template T of Stringable
 *
 * @SuppressWarnings(PHPMD.LongVariable)
 */
abstract class AbstractEncrypted
{
    /**
     * @param T $originalObject
     */
    private mixed $decryptedObject;

    final private function __construct(private readonly string $encrypted) {}

    public static function createFromEncrypted(string $encrypted, SensitiveDataDecryptorInterface $sensitiveDataDecryptor): static
    {
        $encryptedObject = new static($encrypted);
        $encryptedObject->assertIsValid($sensitiveDataDecryptor);

        return $encryptedObject;
    }

    public static function createFromEncryptedSkippingValidation(string $encrypted): static
    {
        return new static($encrypted);
    }

    public function getEncryptedString(): string
    {
        return $this->encrypted;
    }

    /**
     * @param self<T> $other
     */
    public function equals(self $other): bool
    {
        return $this->encrypted === $other->encrypted;
    }

    /**
     * @return T
     */
    public function getDecryptedObject(SensitiveDataDecryptorInterface $sensitiveDataDecryptor): mixed
    {
        if (!isset($this->decryptedObject)) {
            $this->initializeDecryptedObjectFromEncryptedData($sensitiveDataDecryptor);
        }

        return $this->decryptedObject;
    }

    /**
     * @return T
     */
    abstract protected function createDecryptedObject(string $decryptedString): mixed;

    /**
     * @SuppressWarnings(PHPMD.UnusedPrivateMethod)
     */
    private function assertIsValid(SensitiveDataDecryptorInterface $sensitiveDataDecryptor): void
    {
        $this->initializeDecryptedObjectFromEncryptedData($sensitiveDataDecryptor);
    }

    private function initializeDecryptedObjectFromEncryptedData(SensitiveDataDecryptorInterface $sensitiveDataDecryptor): void
    {
        $this->decryptedObject = $this->createDecryptedObject($sensitiveDataDecryptor->decrypt($this->encrypted));
    }
}

@lifinsky
Copy link
Contributor

We also use Ecotone converters:
For example:

class CardNumberConverter
{
    #[Converter]
    public static function convertToString(#[SensitiveParameter] CardNumber $cardNumber): string
    {
        return (string) $cardNumber;
    }

    #[Converter]
    public static function convertFromStringSkippingValidation(#[SensitiveParameter] string $cardNumber): CardNumber
    {
        return CardNumber::createSkippingValidation($cardNumber);
    }

    public static function convertFromString(#[SensitiveParameter] string $cardNumber): CardNumber
    {
        return CardNumber::create($cardNumber);
    }
}

It adds a lot of boilerplate code, but allows validation of distributed events with sensitive data.

@lifinsky
Copy link
Contributor

The depth and complexity of the problem lies in the fact that it is sometimes necessary to decrypt the data for ES projection. Because reading data from event sourcing aggregates is slow.

@@ -39,6 +39,7 @@
"packages/PdoEventSourcing/src",
"packages/PdoEventSourcing/src"
],
"Ecotone\\GDPR\\": "packages/GDPR/src",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DataProtection as package name sounds good :)

ModuleReferenceSearchService $moduleReferenceSearchService,
InterfaceToCallRegistry $interfaceToCallRegistry
): void {
if (! ExtensionObjectResolver::contains(PersonalDataEncryptionConfiguration::class, $extensionObjects)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ModuleReferenceSearchService $moduleReferenceSearchService,
InterfaceToCallRegistry $interfaceToCallRegistry
): void {
if (! ExtensionObjectResolver::contains(PersonalDataEncryptionConfiguration::class, $extensionObjects)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What we do with in other modules is that if you install the package, it enables given features by default.
So installing a package is agreement to enable given behaviour.

I think we could do that the same here.
Like if I install it, then it's enough to place attribute on the Event to start using it.
Auto-Configuration ftw :)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dgafka I thought about it and noticed it makes no sense for auto-configuration when it comes to encryption. Even if Ecotone knows what should be encrypted, it is useless without knowing how. Although after giving it a second thought I think that it may be good idea for Ecotone to manage keys by default.

{
}

public static function createWithDefaults(): self
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could state for which EventStore references it's enables

Suggested change
public static function createWithDefaults(): self
public static function createWithDefaults(array $eventStoreReferences = [EventStore::class]): self

Then if someone want to switch, he simply create and pass different reference

@dgafka
Copy link
Member

dgafka commented Nov 2, 2024

How about making it part of the event sourcing configuration or providing the ability to use for any DTO/VO attributes? PHP has nice feature SensitiveParameter. SensitiveData as naming idea.

I guess the encryption using Converters would make it work out of the box for all Messages types (queries/command/events).
And that make sense that, as if Message lands in DLQ or Message Queue, it would be better to have personal data encrpyted there too.

That would actually be huge advantage, because there is none such solution in PHP, that does that for everything.
Yet we would still need to allow for customization of what is encrypted anyway. Like first call application level converter and then use the encryption one.

@unixslayer any thoughts on this?

@unixslayer
Copy link
Member Author

unixslayer commented Nov 2, 2024

@dgafka This draft did what I planned - it gave me some additional context to think about ;)

  1. Encryption can be attached to any message whether this is command, query or event (published or persisted) therefore configuration should allow where (message bus/event store) and how to attach data protection.
  2. Although, attaching encryption itself to stored event and message on a bus will require different pointcut.
  3. The goal is still to provide simple way defining what should be protected and how. Using proper attribute on message class or any of its attributes is still on the table. How will be covered by service context configuration.
  4. Also, whether it is a scalar value or VO to be encrypted it makes no difference. The later can be normalized to JSON using JMS Module. So even for complex objects I should be able to create converter that will be used before actual encryption.
  5. There can be multiple different options on how encryption may, or should be configured. Single and global configuration is one option but I think most likely and for security reasons I'd rather encrypt stored data in different way than encrypting distributed messages.
  6. Data protection is not only about encryption. Forgettable payload is also more compliant with GDPR directive than encryption.

I'm already working on this. Further implementation should be ready very soon ;)

@dgafka
Copy link
Member

dgafka commented Nov 3, 2024

@unixslayer cool :)
And thanks @lifinsky for giving your use-case scenarios :)

So @unixslayer can we say, that on run-time we will be dealing with decrypted messages (because deserialized), yet after they will be stored in event-store, queue, dlq they will be encrypted (because serialized)?

@unixslayer
Copy link
Member Author

@dgafka that is correct.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants