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/block proposal authorization password for v2/block_proposal endpoint #4471
Conversation
57c5a61
to
08b4a51
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## next #4471 +/- ##
==========================================
+ Coverage 83.23% 83.30% +0.07%
==========================================
Files 453 453
Lines 325584 325630 +46
Branches 318 318
==========================================
+ Hits 270993 271263 +270
+ Misses 54583 54359 -224
Partials 8 8
... and 29 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
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.
Thanks for hustling on this!
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.
🙌 lgtm!
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.
This isn't the right way to do HMAC. Also, we don't need it here since we're not even supporting HTTPS.
Can you instead just have the block-proposal endpoint check for an authorization header and treat the value as a bare password? Thanks!
Yep will do! |
08b4a51
to
96d2ca8
Compare
96d2ca8
to
1c61431
Compare
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.
lgtm!
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.
👏
@jferrant as a sanity check, can you run the regtest env with this PR's latest commit and make sure it produces at least one Nakamoto block? |
1c61431
to
57d8d1f
Compare
57d8d1f
to
f7aaa48
Compare
… endpoint Signed-off-by: Jacinta Ferrant <jacinta@trustmachines.co>
f7aaa48
to
c1a6f2f
Compare
Re-requesting reviews. I'm running |
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.
lgtm and have tested in regtest!
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.
lgtm!
Added config option to the signer and to the ConnectionOptions for adding a password. Just passing it around as a plaintext password in the authorization header.