-
Notifications
You must be signed in to change notification settings - Fork 786
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
2002 config encryption in UI #2050
Conversation
import Utf8 from 'crypto-js/enc-utf8'; | ||
|
||
export function encryptText(content: string, password: string): string { | ||
return AES.encrypt(content, password).toString(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it safe to convert this to a string? It will have characters that cannot be represented, which is problematic if we're going to insert this into a JSON string. If we want to return a string, we might consider base64 encoding.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
AES.encrypt(content, password)
already returns base64, maybe the .toString();
is not required, but that's how it's shown in the documentation, I guess it doesn't bother me and explains that our end result is a string
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Password based encryptor is required for configuration encryption
Previously config was not getting submitted before exporting. This could cause a misunderstanding where user exports a different configuration to the one he sees on the screen
New format of {metadata: {encrypted: true}, contents: {...}} will simplify the logic of configuration import since we'll know if it's encrypted beforehand
8c53bcc
to
37152c2
Compare
Codecov Report
@@ Coverage Diff @@
## develop #2050 +/- ##
========================================
Coverage 57.52% 57.52%
========================================
Files 478 478
Lines 12888 12888
========================================
Hits 7414 7414
Misses 5474 5474 Continue to review full report at Codecov.
|
Statement if showPassword then showPassword is redundant
tryImport better reflects what the function is doing
What does this PR do?
Fixes #2002
Add any further explanations here.
PR Checklist
Testing Checklist