fix: Add cacheing CMM examples #187
Conversation
resolves aws#23 Add examples for both node and browser. Refactor the browser examples to be tested with karma. Update the browser html so that it still works.
| * Just because you can decrypt something does not mean it is what you expect. | ||
| * For example, if you are are only expecting data from 'us-west-2', | ||
| * the origin can identify a malicious actor. | ||
| * See: https://docs.aws.amazon.com/encryption-sdk/latest/developer-guide/concepts.html#encryption-context |
There was a problem hiding this comment.
Better: The following doc describes the use of the encryption context in data key caching.
https://docs.aws.amazon.com/encryption-sdk/latest/developer-guide/data-caching-details.html#caching-encryption-context
There was a problem hiding this comment.
That link is not about cacheing, it is about encryption context. i.e. why you should include/use it.
Edit: But I see what you are saying. Added link
| * Use any method you like to get credentials into the browser. | ||
| * See kms.webpack.config | ||
| */ | ||
| declare const credentials: {accessKeyId: string, secretAccessKey:string, sessionToken:string } |
There was a problem hiding this comment.
Do you need to add the sessionToken to all browser examples, or just this one?
| import { toBase64 } from '@aws-sdk/util-base64-browser' | ||
|
|
||
| /* This is injected by webpack. | ||
| * The webpack.DefinePlugin will replace the values when bundling. |
There was a problem hiding this comment.
Update comment, this is no longer accurate.
| */ | ||
| const maxMessagesEncrypted = 100 | ||
|
|
||
| const cmm = new WebCryptoCachingMaterialsManager({ |
There was a problem hiding this comment.
Other ESDKs call this variable cachingCMM to make it very clear what it is.
| * This value is optional, | ||
| * but you should configure the lowest value possible. | ||
| */ | ||
| const maxMessagesEncrypted = 100 |
There was a problem hiding this comment.
Other samples set this to 10, not 100.
| */ | ||
|
|
||
| /* This is a simple example of using a KMS Keyring | ||
| * to encrypt and decrypt using the AWS Encryption SDK for Javascript in a browser. |
There was a problem hiding this comment.
Nope. This is the caching example.
| * This can be configure by passing a `proactiveFrequency` | ||
| * as the second paramter to however often you want to check in milliseconds. | ||
| */ | ||
| const cache = getLocalCryptographicMaterialsCache(100) |
There was a problem hiding this comment.
A capacity value of 100 is very high. The typical value is 10. Also, I would make this a variable so it's meaning is clear.
const maxSize = 10
const cache = getLocalCryptographicMaterialsCache(maxSize)
There was a problem hiding this comment.
will update in both places but I will use capacity
| * you must provide all of the plaintext to the encrypt function. | ||
| * Therefore, the encrypt function in the browser knows the length of the plaintext | ||
| * and does not accept a plaintextLength option. | ||
| */ |
There was a problem hiding this comment.
Ryan, is there a reason to explain why you can't provide a value? This function doesn't have this parameter. I would just omit lines 148-153. (Feel free to overrule me if I've misinterpreted this.)
There was a problem hiding this comment.
If you provide a value and it does not match, this just makes for confusion.
Co-Authored-By: June Blender <[email protected]>
Co-Authored-By: June Blender <[email protected]>
Co-Authored-By: June Blender <[email protected]>
Co-Authored-By: June Blender <[email protected]>
Co-Authored-By: June Blender <[email protected]>
Co-Authored-By: June Blender <[email protected]>
Co-Authored-By: June Blender <[email protected]>
Co-Authored-By: June Blender <[email protected]>
Co-Authored-By: June Blender <[email protected]>
Co-Authored-By: June Blender <[email protected]>
to avoid issues with cmm being called caching material manager
modules/client-node/Readme.md
Outdated
| @@ -61,10 +61,10 @@ const cleartext = 'asdf' | |||
| * the Encryption SDK returns an "encrypted message" that includes the ciphertext, | |||
There was a problem hiding this comment.
Same as above. Again, not required.
| /* You need to specify a name | ||
| * and a namespace for raw encryption key providers. | ||
| * The name and namespace that you use in the decryption keyring *must* be an exact, | ||
| * The name and namespace that you use in the decryption keyring *must* be an exact, case-sensitive match. | ||
| * *case-sensitive* match for the name and namespace in the encryption keyring. | ||
| */ |
There was a problem hiding this comment.
Sorry, I didn't see the line following. We now have a duplicate. I would revert the change.
| * Access to kms:Encrypt is required for every CMK in keyIds. | ||
| * You might list several keys in different AWS Regions. | ||
| * This allows you to decrypt the data in any of the represented Regions. | ||
| * In this example, I am using the same CMK. |
There was a problem hiding this comment.
This should match the text in the browser example.
| const keyring = new KmsKeyringNode({ generatorKeyId, keyIds }) | ||
|
|
||
| /* Create a cache to hold the material. | ||
| * In this case we use the local cache provided by the Encryption SDK. |
There was a problem hiding this comment.
Please make all text in the node example match the corresponding edited text in the browser example.
Co-Authored-By: June Blender <[email protected]>
Co-Authored-By: June Blender <[email protected]>
Co-Authored-By: June Blender <[email protected]>
Co-Authored-By: June Blender <[email protected]>
Co-Authored-By: June Blender <[email protected]>
Co-Authored-By: June Blender <[email protected]>
Co-Authored-By: June Blender <[email protected]>
Co-Authored-By: June Blender <[email protected]>
Co-Authored-By: June Blender <[email protected]>
Co-Authored-By: June Blender <[email protected]>
Co-Authored-By: June Blender <[email protected]>
resolves #23
Add examples for both node and browser.
Refactor the browser examples to be tested with karma.
Update the browser html so that it still works.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
Check any applicable: