-
Notifications
You must be signed in to change notification settings - Fork 29
feat: HMAC authentication system #364
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
Conversation
KonradStaniec
left a comment
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.
Currently it also works without providing an HMAC key bypassing the security measure. At what point do we error and make this mandatory?
I would say on main net. Although this would be weird, as we would need to release new version with errors 🤔
When I was thinking about it, I though we should always have option to disable it, but have:
- default as enabled
- docs suggesting that it should be left enabled
Then is users disables it, it it on them. Wdyt ?
In general pr looks good to me 👍
|
Will do the secrets integration with cloud providers in a follow up PRs. |
gitferry
left a comment
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.
Great work! Some minor comments:
Lazar955
left a comment
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.
nice work!
The following is an implementation of the proposed HMAC authentication system from https://github.com/babylonlabs-io/pm/pull/261. A couple of questions I still have on my mind are: