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

feat: Use custom hash.Hash in hmac.HMACStrategy #663

Merged
merged 3 commits into from
Apr 15, 2022

Conversation

mash
Copy link
Contributor

@mash mash commented Apr 14, 2022

Add Hash func() hash.Hash field in hmac.HMACStrategy to allow using a custom hash.Hash function. When nil, the default sha512.New512_256 is used.

Related Issue or Design Document

#654

Checklist

  • I have read the contributing guidelines
    and signed the CLA.
  • I have referenced an issue containing the design document if my change
    introduces a new feature.
  • I have read the security policy.
  • I confirm that this pull request does not address a security
    vulnerability. If this pull request addresses a security vulnerability, I
    confirm that I got green light (please contact
    [email protected]) from the maintainers to push
    the changes.
  • I have added tests that prove my fix is effective or that my feature
    works.
  • I have added necessary documentation within the code base (if
    appropriate).

Further comments

@mash mash requested a review from aeneasr as a code owner April 14, 2022 14:30
@CLAassistant
Copy link

CLAassistant commented Apr 14, 2022

CLA assistant check
All committers have signed the CLA.

@mash mash changed the title Use custom hash.Hash in hmac.HMACStrategy feat: Use custom hash.Hash in hmac.HMACStrategy Apr 14, 2022
Copy link
Member

@aeneasr aeneasr left a comment

Choose a reason for hiding this comment

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

Awesome, thank you for your contribution! This looks pretty good and I have some ideas how to improve it further :)

@@ -131,3 +132,17 @@ func TestValidateWithRotatedKeyInvalid(t *testing.T) {

require.EqualError(t, new(HMACStrategy).Validate(token), "a secret for signing HMAC-SHA512/256 is expected to be defined, but none were")
}

func TestCustomHMAC(t *testing.T) {
Copy link
Member

Choose a reason for hiding this comment

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

Could you please also add a success case? :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, sure.

Copy link
Member

@aeneasr aeneasr left a comment

Choose a reason for hiding this comment

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

Awesome, thank you! 🎉 Your contribution makes Ory better :)

@aeneasr aeneasr merged commit d09a8c3 into ory:master Apr 15, 2022
@mash
Copy link
Contributor Author

mash commented Apr 15, 2022

Happy to be able to contribute.

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.

3 participants