-
Notifications
You must be signed in to change notification settings - Fork 6
Added encrytion method #24
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
base: master
Are you sure you want to change the base?
Conversation
CharithW
commented
Jan 8, 2018
- a task list item
- list syntax required
- normal formatting, @mentions, #1234 refs
- incomplete
- completed
| { | ||
| public class Encrypt | ||
| { | ||
| readonly string PasswordHash ="SampleHash"; |
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.
Inject the Secrets to the class via .net DI framework. Refer the code written Decrypt.cs
| { | ||
| byte[] plainTextBytes = Encoding.UTF8.GetBytes(plainText); | ||
|
|
||
| byte[] keyBytes = new Rfc2898DeriveBytes(PasswordHash, Encoding.ASCII.GetBytes(SaltKey)).GetBytes(256 / 8); |
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.
Format the document.
| byte[] plainTextBytes = Encoding.UTF8.GetBytes(plainText); | ||
|
|
||
| byte[] keyBytes = new Rfc2898DeriveBytes(PasswordHash, Encoding.ASCII.GetBytes(SaltKey)).GetBytes(256 / 8); | ||
| var symmetricKey = new RijndaelManaged() { Mode = CipherMode.CBC, Padding = PaddingMode.Zeros }; |
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.
Use the corresponding types instead of var.
| cipherTextBytes = memoryStream.ToArray(); | ||
| cryptoStream.Close(); | ||
| } | ||
| memoryStream.Close(); |
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.
memoryStream closes automatically once the execution has done within the using statement. Not need to specifically close.
| } | ||
| } | ||
|
|
||
| static SymmetricAlgorithm CreateAlgorithm() |
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.
Consider changing the accessibility of the method and available as an instance method instead of static. You can change this to private SymmetricAlgorithm CreateAlgorithm() instead.
OR
reuse the same logic written on Decrypt.cs
| } | ||
| memoryStream.Close(); | ||
| } | ||
| string encrypted_text = Convert.ToBase64String(cipherTextBytes); |
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.
Consider returning the converted string instead of assigning to a local variable and returning the variable instance.
|
Noted and will add these changes |
| { | ||
| byte[] plainTextBytes = Encoding.UTF8.GetBytes(plainText); | ||
|
|
||
| byte[] keyBytes = new Rfc2898DeriveBytes(PasswordHash, Encoding.ASCII.GetBytes(SaltKey)).GetBytes(256 / 8); |
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.
I think the line is bit off indented? Applicable to several places.
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.
Just noticed that @kosalanuwan already pointed it out. Ignore my comment.