Motivation

Currently the Encrypt service passes the key value to the encryption method on encrypt/decrypt.
Passing the entire Key entity would allow for more advanced integration between the method plugin and the Key entity.

For example:
An AES encryption method would be able to integrate with an AES encryption key that would hold a reference to the necessary IV (input vector) string.

Proposed solution

  • Modify the encrypt, decrypt & checkDependencies methods on EncryptionMethodInterface to receive \Drupal\key\KeyInterface objects instead of strings.
  • Modify the encrypt & decrypt methods in EncryptService class to pass the \Drupal\key\KeyInterface objects instead of strings to the EncryptionMethod encrypt/decrypt methods.
  • Modify the validate method in EncryptionProfile to pass the \Drupal\key\KeyInterface objects instead of strings to the checkDependencies method on EncryptionMethodInterface.
CommentFileSizeAuthor
#2 pass_key_entity_2749349_2.patch7.2 KBeyal shalev

Issue fork encrypt-2749349

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

Eyal Shalev created an issue. See original summary.

eyal shalev’s picture

eyal shalev
he/him
commented
Status: Active » Needs review
StatusFileSize
new pass_key_entity_2749349_2.patch7.2 KB

This is the patch I used to execute the proposed solution.

rlhawk’s picture

rlhawk
Primary language English
Location Seattle, Washington, United States
commented

I think this is an excellent idea. It will require coordination with modules that provide encryption methods, of course, but that should not be too bad. Making an API change such as this should be possible, since Encrypt 8.x-3.x is still in an alpha release state.

gambry’s picture

gambry
Primary language Italian
Location Milan
commented

I agree having access to the Key object will open to a range of possibilities. However I see even more powerful giving access to the EncryptionProfile entity, rather than the Key only.
From the EncryptionProfile a developer will be able to access the Key, any additional custom option the profile may have, etc.

This issue is really important, but I think the coordination mentioned in #3 is a big blocker.
Why don't we inject the EncryptionProfile argument to the EncryptionMethod constructor?
It will require a custom refactoring of the plugin manager, but the work shouldn't be massive and - unless plugin definitions don't override the construct - no coordination is needed.

ptmkenny made their first commit to this issue’s fork.

ptmkenny’s picture

ptmkenny commented
Status: Needs review » Needs work

3.x has been released, so this would have to be moved to 4.x. Also, the patch no longer applies to 3.x-dev, so marking "Needs work."

ptmkenny’s picture

ptmkenny commented
Status: Needs work » Postponed

We could make this API change in Encrypt 4.0.

ptmkenny’s picture

ptmkenny commented
Version: 8.x-3.x-dev » 4.x-dev
Status: Postponed » Active

ptmkenny changed the visibility of the branch 2749349-pass-the-key to hidden.

ptmkenny opened merge request !30

ptmkenny’s picture

ptmkenny commented
Status: Active » Fixed

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

  • ptmkenny committed 3be1e0cb on 4.x 
    feat: #2749349 Pass the Key entity to the encryption method on encrypt/...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.