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

PHPLIB-986: Split keyAltName example and link key management docs #995

Merged
merged 4 commits into from
Oct 24, 2022

Conversation

alcaeus
Copy link
Member

@alcaeus alcaeus commented Oct 18, 2022

PHPLIB-986

This PR splits the tutorial section on key alternate names in two parts: creating the key and using it. This is done to better explain the disconnect of creating an encryption key at one time (e.g. on deployment) and using it at a different time (i.e. when inserting data). While we could show ways how to serialise a MongoDB\BSON\Binary object and include that via the configuration, I believe the easier way is using an alternate name for this, as it's just a string as opposed to binary data.

In addition I've linked the server's Encryption Key Management Documentation as it contains more information about how to manage encryption keys, including examples how to dynamically choose an encryption key when inserting data.

@alcaeus alcaeus requested review from jmikola and comandeo October 18, 2022 12:36
@alcaeus alcaeus self-assigned this Oct 18, 2022
@alcaeus
Copy link
Member Author

alcaeus commented Oct 18, 2022

After discussing this with @comandeo, we decided that it's better to restructure the tutorial to first show key creation and talk about key management. The current tutorials would then follow this first section, always referencing the key created in the first section. While this makes the examples less copy-pasteable, it does represent real-world usage better than the current examples.

Copy link
Member

@jmikola jmikola left a comment

Choose a reason for hiding this comment

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

Some grammar suggestions but apart from those this LGTM.

@@ -202,6 +202,15 @@ specified when creating the key. The following example creates an encryption key
with an alternative name, which could be done when deploying the application.
The software then encrypts data by referencing the key by its alternative name.

Creating the encryption key
Copy link
Member

Choose a reason for hiding this comment

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

Title capitalization:

Suggested change
Creating the encryption key
Creating the Encryption Key

and create a new data key. You can pass multiple alternate names for this key
and later reference the key by these alternate names instead of the key ID.
Creating a new data encryption key would typically be done on initial deployment,
but depending on your use-case you may want to use more than one encryption key.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
but depending on your use-case you may want to use more than one encryption key.
but depending on your use case you may want to use more than one encryption key.

I think this is more commonly written without hyphenation.

@@ -222,10 +231,35 @@ The software then encrypts data by referencing the key by its alternative name.
$client = new Client();
$clientEncryption = $client->createClientEncryption($clientEncryptionOpts);

// Create an encryption key with an alternative name. This could be done when
// deploying the application
// Create an encryption key with an alternate name.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
// Create an encryption key with an alternate name.
// Create an encryption key with an alternate name

We typically omit periods for single-sentence comments

$keyId = $clientEncryption->createDataKey('local', ['keyAltNames' => ['altname']]);

Using an encryption key by alternate name
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Using an encryption key by alternate name
Using an Encryption Key by Alternate Name

FYI: https://capitalizemytitle.com/ is a helpful tool for this (there are a bunch)

@alcaeus alcaeus requested review from jmikola and comandeo October 19, 2022 12:16
@alcaeus alcaeus force-pushed the phplib-986-csfle-docs branch from 8ab140f to 670d3af Compare October 19, 2022 12:17
@alcaeus
Copy link
Member Author

alcaeus commented Oct 19, 2022

@jmikola thank you for your review, I applied your suggestions. I've extracted the key creation part to the beginning of the tutorial and updated the examples to account for this, so I'd appreciate another look.

Copy link
Member

@jmikola jmikola left a comment

Choose a reason for hiding this comment

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

Comment revisions but LGTM otherwise.

Co-authored-by: Jeremy Mikola <jmikola@gmail.com>
@alcaeus alcaeus removed the request for review from comandeo October 24, 2022 06:54
@alcaeus alcaeus merged commit 7b3eca3 into mongodb:master Oct 24, 2022
@alcaeus alcaeus deleted the phplib-986-csfle-docs branch October 24, 2022 06:54
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.

None yet

2 participants