feat(btcindexer): secure block ingestion with bearer auth#351
Merged
feat(btcindexer): secure block ingestion with bearer auth#351
Conversation
…nit tests Signed-off-by: sczembor <stanislaw.czembor@gmail.com>
Signed-off-by: sczembor <stanislaw.czembor@gmail.com>
Signed-off-by: sczembor <stanislaw.czembor@gmail.com>
Contributor
Reviewer's GuideAdds bearer-token based authentication to the block ingestor /bitcoin/blocks endpoint and its clients, updates HTTP method and path conventions, and wires the auth secret into the worker environment and Go client. Sequence diagram for authenticated PUT /bitcoin/blocks ingest flowsequenceDiagram
actor Relayer
participant Client as BtcIndexerClient
participant Worker as BlockIngestorWorker
participant Auth as AuthModule
participant Secret as RELAYER_AUTH_TOKEN
participant KV as BtcBlocks
participant Queue as BlockQueue
Relayer->>Client: putBlocks(blocks)
Client->>Client: PutBlocksReq.encode(blocks)
Client->>Worker: HTTP PUT /bitcoin/blocks
activate Worker
Worker->>Auth: isAuthorized(request, env)
activate Auth
Auth->>Secret: get()
Secret-->>Auth: secretValue
Auth-->>Worker: authorized or unauthorized
deactivate Auth
alt unauthorized
Worker-->>Client: 401 Unauthorized
else authorized
Worker->>Worker: PutBlocksReq.decode(body)
Worker->>KV: store blocks
Worker->>Queue: enqueue block events
Worker-->>Client: 2xx Success
end
deactivate Worker
Client-->>Relayer: return (void or error)
Class diagram for updated block ingestor clients and authclassDiagram
class BtcIndexerClient {
- string url
- string authToken
+ BtcIndexerClient(url string, authToken string)
+ putBlocks(blocks PutBlock[]) Promise~void~
}
class PutBlocksReqTS {
+ encode(blocks PutBlock[]) Uint8Array
+ decode(buffer ArrayBuffer) PutBlock[]
}
class Env {
+ BtcBlocks KVNamespace
+ RELAYER_AUTH_TOKEN SecretsStoreSecret
+ BtcIndexer Service
+ BlockQueue Queue
}
class AuthModule {
+ isAuthorized(request Request, env Env) Promise~boolean~
}
class GoClient {
- string baseUrl
- string authToken
- http.Client c
+ NewClient(workerUrl string, authToken string) GoClient
+ PutBlocks(putBlocks PutBlocksReq) *http.Response, error
}
class PutBlocksReqGo {
+ MsgpackEncode() ([]byte, error)
}
BtcIndexerClient --> PutBlocksReqTS : uses
GoClient --> PutBlocksReqGo : uses
AuthModule --> Env : reads
BtcIndexerClient --> Env : targets worker using env config
GoClient --> Env : targets worker using env config
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Contributor
Author
|
@sourcery-ai title |
sourcery-ai
bot
changed the title
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- The request path, method, and
Content-Typeare now duplicated across the TypeScript client, Go client, and worker handler; consider centralizing these API contract details in a shared definition to avoid future divergence. - In
isAuthorized, a misconfigured or missingRELAYER_AUTH_TOKENwill cause all requests to be rejected without any signal; consider surfacing this as a clear startup/runtime error or logging a configuration warning so it’s easier to diagnose.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The request path, method, and `Content-Type` are now duplicated across the TypeScript client, Go client, and worker handler; consider centralizing these API contract details in a shared definition to avoid future divergence.
- In `isAuthorized`, a misconfigured or missing `RELAYER_AUTH_TOKEN` will cause all requests to be rejected without any signal; consider surfacing this as a clear startup/runtime error or logging a configuration warning so it’s easier to diagnose.
## Individual Comments
### Comment 1
<location> `api/btcindexer/api_test.go:48` </location>
<code_context>
pb := PutBlock{Network: NetworkRegtest, Height: 156, Block: blockBz}
- c := NewClient("http://localhost:8787")
+ c := NewClient("http://localhost:8787", "")
resp, err := c.PutBlocks(PutBlocksReq{pb})
assert.NilError(t, err)
</code_context>
<issue_to_address>
**suggestion (testing):** Add tests to verify that `NewClient` with a non-empty auth token sets the `Authorization` header in `PutBlocks` requests.
This update only adapts to the new `NewClient` signature and still passes an empty token, so the new auth behavior isn’t actually tested. Please add a test (or table-driven tests) that builds a `Client` with both non-empty and empty `authToken` values, calls `PutBlocks`, and asserts that:
- `Authorization: Bearer <token>` is set when the token is non-empty
- The `Authorization` header is omitted when the token is empty
You can do this by injecting an `http.Client` with a custom `Transport` that captures the outgoing request instead of performing a real HTTP call.
Suggested implementation:
```golang
pb := PutBlock{Network: NetworkRegtest, Height: 156, Block: blockBz}
c := NewClient("http://localhost:8787", "")
resp, err := c.PutBlocks(PutBlocksReq{pb})
assert.NilError(t, err)
respBody, err := io.ReadAll(resp.Body)
}
func TestClientPutBlocksAuthorizationHeader(t *testing.T) {
tests := []struct {
name string
token string
wantAuth string
}{
{
name: "no auth token omits Authorization header",
token: "",
wantAuth: "",
},
{
name: "non-empty auth token sets Authorization header",
token: "my-secret-token",
wantAuth: "Bearer my-secret-token",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
var capturedReq *http.Request
// Custom RoundTripper that captures the request instead of doing a real HTTP call.
rt := roundTripperFunc(func(req *http.Request) (*http.Response, error) {
capturedReq = req
// Return a minimal successful HTTP response so PutBlocks can proceed.
return &http.Response{
StatusCode: http.StatusOK,
Body: io.NopCloser(strings.NewReader(`{}`)),
Header: make(http.Header),
Request: req,
}, nil
})
httpClient := &http.Client{
Transport: rt,
}
client := NewClient("http://localhost:8787", tt.token)
// Inject our custom HTTP client so we can capture the outgoing request.
client.httpClient = httpClient
_, err := client.PutBlocks(PutBlocksReq{})
assert.NilError(t, err)
if capturedReq == nil {
t.Fatalf("expected request to be captured")
}
gotAuth := capturedReq.Header.Get("Authorization")
if tt.wantAuth == "" {
if gotAuth != "" {
t.Fatalf("expected no Authorization header, got %q", gotAuth)
}
} else {
if gotAuth != tt.wantAuth {
t.Fatalf("expected Authorization header %q, got %q", tt.wantAuth, gotAuth)
}
}
})
}
```
1. Ensure the following helper type is defined in this file (at top-level, outside any function), to satisfy `http.RoundTripper`:
```go
type roundTripperFunc func(*http.Request) (*http.Response, error)
func (f roundTripperFunc) RoundTrip(req *http.Request) (*http.Response, error) {
return f(req)
}
```
2. Make sure the `Client` type used by `NewClient` exposes a way to inject a custom `*http.Client`. The test above assumes a field `httpClient`:
```go
type Client struct {
baseURL string
authToken string
httpClient *http.Client
}
```
If this field is not currently exported/available, either:
- Add it to `Client`, initialized in `NewClient` (e.g., `httpClient: http.DefaultClient`), and keep using it inside `PutBlocks`, or
- Provide a setter method (e.g., `func (c *Client) SetHTTPClient(hc *http.Client)`) and update the test to call that instead of assigning the field directly.
3. Confirm the following imports exist at the top of `api/btcindexer/api_test.go`:
```go
import (
"io"
"net/http"
"strings"
"testing"
"gotest.tools/v3/assert"
)
```
Adjust to match existing import style and any additional imports already present.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Signed-off-by: sczembor <stanislaw.czembor@gmail.com>
Signed-off-by: sczembor <stanislaw.czembor@gmail.com>
Signed-off-by: sczembor <stanislaw.czembor@gmail.com>
Signed-off-by: sczembor <stanislaw.czembor@gmail.com>
Signed-off-by: sczembor <stanislaw.czembor@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Closes: #339
Author Checklist
All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.
I have...
!to the type prefix if API or client breaking changeCHANGELOG.mdSummary by Sourcery
Secure the Bitcoin block ingestion endpoint with bearer-token authentication and align clients with the new authenticated /bitcoin/blocks API.
New Features:
Enhancements:
Tests: