这是indexloc提供的服务,不要输入任何密码
Skip to content

Conversation

@Vvaradinov
Copy link
Contributor

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:

  1. Currently it also works without providing an HMAC key bypassing the security measure. At what point do we error and make this mandatory?
  2. I have created a secrets getter with the 3 cloud providers without implementation, which ones do we want to support ?

Copy link
Contributor

@KonradStaniec KonradStaniec left a 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 👍

@Vvaradinov
Copy link
Contributor Author

Will do the secrets integration with cloud providers in a follow up PRs.

Copy link
Member

@gitferry gitferry left a 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:

Copy link
Member

@Lazar955 Lazar955 left a comment

Choose a reason for hiding this comment

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

nice work!

@Vvaradinov Vvaradinov merged commit d19ccaf into main Mar 11, 2025
18 checks passed
@Vvaradinov Vvaradinov deleted the Vvaradinov/hmac-auth branch March 11, 2025 10:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants