feat(encryption): Add previous keys fallback feature

[patel-vansh]

Description
This PR adds a new feature to the encryption service. Currently encryption service only supports single key decryption. This feature adds a feature to enable fallback keys which can be used if data can't be decrypted by the main key.

For more detailed use cases and motivation, please take a look at this forum post.

For equivalent Laravel implementation, please look at this page.

Note:
This PR is not complete yet. I need help in testing as well as user guide update according to this new change. I've done basic logic implementation and hence I am creating this PR to let others have a look at the change and suggest some changes. Testing and User guide updation is still remaining.

Checklist:

• Securely signed commits
• Component(s) with PHPDoc blocks, only if necessary or adds value (without duplication)
• Unit testing, with >80% coverage
• User guide updated
• Conforms to style guide

[paulbalandan]

The encryption config can take in values from the .env file and having array properties can be hard to be populated. I suggest this takes a string of comma separated app keys instead.

[patel-vansh]

We can do that. So, this would be changed to comma separated string, right? So, we will have to do like, convert hex2bin or base64_decode on the fly when needed, or maybe, convert all previous keys and implode them as comma separated keys?

Which approach would be more better?

[patel-vansh]

I've currently set the second approach as for now (while initializing in the BaseConfig, do the parsing stuff and then implode as comma-separated string for future use), but we can use the other approach as well.

[patel-vansh]

I don't know why this static analysis action is failing. The $result variable is initialized to false, maybe that would be the case.
But I don't think the problem is that its initializd to false, there must be something that I'm missing.

Thanks for help.

[michalsn]

Nice start. Some thoughts: I think we should only fall back to the previous keys if the $params do not contain a key. If a key is provided in the $params, it suggests we are dealing with a custom encryption key, and the previous keys are meant to work only with the key from the config file, which would be overridden.

I would explore the option to wrap decrypt() methods content into a callback.

[michalsn]

Can't we just make it string|array?

[patel-vansh]

So, you mean like anyone can pass both comma-seperated string or array of keys and both should work, like that?

[michalsn]

Yes. For working with .env files, the string syntax will be preferred. Arrays will still be allowed, but they must follow the usual array rules - specifically, keys must be explicitly declared for values seeded from the .env file. This will enable us to transform strings into arrays safely.

[michalsn]

This is a very bad idea. We can't safely implode encryption keys because once they are decoded to raw bytes, we lose a reliable way to explode them again without corrupting the keys.

[patel-vansh]

Ok. So, I think the other way is, keep this as it is, then, when needed, parse those keys on the fly. Would that work? But for that, we need this parseEncryptionKeys function to be accessible from both handlers.

[michalsn]

We can parse encryption keys as we do now. Just assign an array, not a string (don't use implode). It will be allowed if we declare $previousKeys in the config as string|array.

$keysArray  = is_string($this->{$property}) ? array_map(trim(...), explode(',', $this->{$property})) : $this->{$property};
$parsedKeys = [];

foreach ($keysArray as $key) {
    $parsedKeys[] = $this->parseEncryptionKey($key);
}

$this->{$property} = $parsedKeys;

[michalsn]

I find this hard to read in its current form. Wrapping the logic in a callback method might make it clearer and easier to maintain, especially since the existing method logic won't change.