[DRAFT] Proof of concept for @webex/plugin-encryption #4023
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
| @@ -188,7 +188,9 @@ const Encryption = WebexPlugin.extend({ | |||
| return Promise.reject(new Error('Cannot encrypt `scr` without first setting `loc`')); | |||
| } | |||
|
|
|||
| return this.getKey(key, options).then((k) => scr.toJWE(k.jwk)); | |||
| return this.getKey(key, options).then((k) => | |||
| SCR.fromJSON(scr).then((encScr) => encScr.toJWE(k.jwk)) | |||
There was a problem hiding this comment.
can you add some comments on what this line does and why do we need to always do fromJSON arent there cases where its not in that case what would the function return , i assume old value
| @@ -0,0 +1,5 @@ | |||
| interface IEncryption { | |||
There was a problem hiding this comment.
is it ok to add this type inline in the file , if it grows big we can move it out
| } | ||
|
|
||
| // eslint-disable-next-line require-jsdoc | ||
| public async testRun(): Promise<string> { |
There was a problem hiding this comment.
why is this function needed here ?
| * console.log(decryptedFile); | ||
| * ``` | ||
| */ | ||
| public async downloadAndDecryptFile(fileUri: string): Promise<File> { |
There was a problem hiding this comment.
i have not seen the ticket but shouldnt we expose the encrypt and decrypt text logic also . are we planning to do that later in future sprint ?
| // JavaScript modularization | ||
|
|
||
| /* istanbul ignore else */ | ||
| if (!global._babelPolyfill) { |
There was a problem hiding this comment.
webpack should automatically add it
and not sure if we need to call this encryption as it includes decryption also , need to find a better name
| require('@webex/common'); | ||
| require('@webex/internal-plugin-encryption'); | ||
| require('@webex/plugin-encryption'); | ||
| require('node-scr'); |
There was a problem hiding this comment.
dont think we need node-src and internal encryption and common as they are already sub dependecny of the plugin encryption
COMPLETES #SPARK-580073
This pull request addresses
< DESCRIBE THE CONTEXT OF THE ISSUE >
by making the following changes
< DESCRIBE YOUR CHANGES >
Change Type
The following scenarios were tested
< ENUMERATE TESTS PERFORMED, WHETHER MANUAL OR AUTOMATED >
I certified that
I have read and followed contributing guidelines
I discussed changes with code owners prior to submitting this pull request
I have not skipped any automated checks
All existing and new tests passed
I have updated the documentation accordingly
Make sure to have followed the contributing guidelines before submitting.