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

Added HMAC authentication mode in Hash generators #555

Merged
merged 6 commits into from
Jun 6, 2022

Conversation

L1nu5
Copy link
Contributor

@L1nu5 L1nu5 commented Jun 3, 2022

Pull request type

Please check the type of change your PR introduces:

  • Bugfix
  • Feature
  • UI change (please include screenshot!)
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • Documentation content changes
  • Internationalization and localization
  • Other (please describe):

What is the current behavior?

Currently, we do not have HMAC generation feature in the tool. Please refer: 516

Issue Number: 516

What is the new behavior?

I'm using MacAlgorithmProvider currently for this feature. I also noticed that SHA 384 hash output is missing, I could implement that as well but I'm not sure why it was not added on the Hash generator screen.

Here's how it'll look like with the fix:

  1. Normal mode

Normal_mode

  1. HMAC mode

Hmac_mode

I'm currently using hardcoded strings for the toggle switch and the secret key textbox title. I just wanted to make sure that the current strings are good enough in this PR review, before I initiate a translation on Crowdin.

Other information

Quality check

Before creating this PR, have you:

  • Followed the code style guideline as described in CONTRIBUTING.md
  • Verified that the change work in Release build configuration
  • Checked all unit tests pass

@veler veler linked an issue Jun 4, 2022 that may be closed by this pull request
@veler
Copy link
Collaborator

veler commented Jun 5, 2022

Hi,
Thank you for offering this change. I like the idea overall :D
One suggestion / question though: how do you feel about not having a HMAC Mode option, and simply applying the HMAC encryption when Secret Key isn't empty?

@L1nu5
Copy link
Contributor Author

L1nu5 commented Jun 6, 2022

@veler I had initially implemented it that way but thought that the Secret Key text field might need some concise hint/tooltip on leaving it empty if the user only needs Hash instead of HMAC. With toggle switch, it feels slightly easier to operate without needing to explain with a tooltip/hint. I can definitely change it back without needing to use a toggle switch though and it will drastically reduce the overall LoC for the change, please let me know your thoughts on this :)

@veler
Copy link
Collaborator

veler commented Jun 6, 2022

@veler I had initially implemented it that way but thought that the Secret Key text field might need some concise hint/tooltip on leaving it empty if the user only needs Hash instead of HMAC. With toggle switch, it feels slightly easier to operate without needing to explain with a tooltip/hint. I can definitely change it back without needing to use a toggle switch though and it will drastically reduce the overall LoC for the change, please let me know your thoughts on this :)

That is fair :-) Let's keep it the way it is then !

@@ -42,6 +42,17 @@
<ComboBoxItem Tag="Base64" Content="{x:Bind ViewModel.Strings.OutputBase64}"/>
</ComboBox>
</controls:ExpandableSettingControl>
<controls:ExpandableSettingControl
Title="HMAC Mode">
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please globalize this string by adding a new entry in https://github.com/veler/DevToys/blob/main/src/dev/impl/DevToys/Strings/en-US/HashGenerator.resw . Then open https://github.com/veler/DevToys/blob/main/src/dev/impl/DevToys/LanguageManager.tt in Visual Studio, press Ctrl+S (even if there's no changes). This will re-regenerate https://github.com/veler/DevToys/blob/main/src/dev/impl/DevToys/LanguageManager.cs automatically and you will then be able to use the resource in the XAML.

@@ -50,6 +61,13 @@
AcceptsReturn="True"
Text="{x:Bind ViewModel.Input, Mode=TwoWay, UpdateSourceTrigger=PropertyChanged}"/>

<controls:CustomTextBox x:Name="SecretKeyInput"
Header="Secret Key (HMAC only)"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same thing here. Also, perhaps we can remove (HMAC only) since this field only appear when HMAC mode is enabled.

string? hash = "";
IBuffer? buffer = null;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Copy link
Collaborator

@veler veler left a comment

Choose a reason for hiding this comment

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

It looks all good ! :D Thank you so much for this contribution!!

@veler veler merged commit 1d80b49 into DevToys-app:main Jun 6, 2022
@L1nu5
Copy link
Contributor Author

L1nu5 commented Jun 6, 2022

It looks all good ! :D Thank you so much for this contribution!!

No problem, I'm really happy to contribute :)

veler pushed a commit that referenced this pull request Mar 31, 2023
* Added first prototype of hmac feature

Todo: localized text and an icon for the HMAC mode toggle switch and secret key text box label

* Added an icon for HMAC

* Fixed compilation errors after merging from main

* Added unit tests, globalized strings
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.

HMAC-SHA256 Generator Tool
2 participants