Skip to content

Commit

Permalink
Error instead of dropping unauthorized headers (#201)
Browse files Browse the repository at this point in the history
* fix: returns an HTTP 401 if authorization headers are required, but are not there or incorrect rather than silently ignoring those headers
  • Loading branch information
aschmahmann authored Nov 25, 2024
1 parent e692866 commit 9b0b7f4
Show file tree
Hide file tree
Showing 4 changed files with 11 additions and 21 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ The following emojis are used to highlight certain changes:

- boxo [v0.24.3](https://github.com/ipfs/boxo/releases/tag/v0.24.3)
- go-libp2p-kad-dht [v0.28.1](https://github.com/libp2p/go-libp2p-kad-dht/releases/tag/v0.28.1)
- passing headers that require authorization but are not authorized now results in an HTTP 401 instead of ignoring those headers

### Removed

Expand Down
6 changes: 3 additions & 3 deletions docs/headers.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ See [`RAINBOW_TRACING_AUTH`](./environment-variables.md#rainbow_tracing_auth)

Optional. Clients may use this header to return a additional vendor-specific trace identification information across different distributed tracing systems.

Currently ignored unless `Authorization` matches [`RAINBOW_TRACING_AUTH`](./environment-variables.md#rainbow_tracing_auth).
Will error unless `Authorization` matches [`RAINBOW_TRACING_AUTH`](./environment-variables.md#rainbow_tracing_auth).

> [!TIP]
> `Traceparent` value format can be found in [W3C Trace Context Specification](https://www.w3.org/TR/trace-context-1/#trace-context-http-headers-format).
Expand All @@ -19,7 +19,7 @@ Currently ignored unless `Authorization` matches [`RAINBOW_TRACING_AUTH`](./envi

Optional. Clients may use this header to return a additional vendor-specific trace identification information in addition to `Traceparent`.

Currently ignored unless `Authorization` matches [`RAINBOW_TRACING_AUTH`](./environment-variables.md#rainbow_tracing_auth).
Will error unless `Authorization` matches [`RAINBOW_TRACING_AUTH`](./environment-variables.md#rainbow_tracing_auth).

> [!TIP]
> `Tracestate` value format can be found in [W3C Trace Context Specification](https://www.w3.org/TR/trace-context-1/#trace-context-http-headers-format).
Expand All @@ -28,6 +28,6 @@ Currently ignored unless `Authorization` matches [`RAINBOW_TRACING_AUTH`](./envi

If the value is `true` the associated request will skip the local block cache and leverage a separate in-memory block cache for the request.

This header is not respected unless the request has a valid `Authorization` header
Will error unless the request has a valid `Authorization` header

See [`RAINBOW_TRACING_AUTH`](./environment-variables.md#rainbow_tracing_auth)
14 changes: 4 additions & 10 deletions handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -158,15 +158,12 @@ func TestNoBlockcacheHeader(t *testing.T) {
req, err := http.NewRequest(http.MethodGet, url, nil)
require.NoError(t, err)

// Authorization missing, expect NoBlockcacheHeader to be ignored
// Authorization missing, expect NoBlockcacheHeader to result in an error
req.Header.Set(NoBlockcacheHeader, "true")

res, err := http.DefaultClient.Do(req)
assert.NoError(t, err)
assert.Equal(t, http.StatusOK, res.StatusCode)
responseBody, err := io.ReadAll(res.Body)
assert.NoError(t, err)
assert.Equal(t, content, responseBody)
assert.Equal(t, http.StatusUnauthorized, res.StatusCode)
})

t.Run("Skipping the cache only works when RAINBOW_TRACING_AUTH is set", func(t *testing.T) {
Expand All @@ -185,14 +182,11 @@ func TestNoBlockcacheHeader(t *testing.T) {
req, err := http.NewRequest(http.MethodGet, url, nil)
require.NoError(t, err)

// Authorization missing, expect NoBlockcacheHeader to be ignored
// Authorization missing, expect NoBlockcacheHeader to result in an error
req.Header.Set(NoBlockcacheHeader, "true")

res, err := http.DefaultClient.Do(req)
assert.NoError(t, err)
assert.Equal(t, http.StatusOK, res.StatusCode)
responseBody, err := io.ReadAll(res.Body)
assert.NoError(t, err)
assert.Equal(t, content, responseBody)
assert.Equal(t, http.StatusUnauthorized, res.StatusCode)
})
}
11 changes: 3 additions & 8 deletions handlers.go
Original file line number Diff line number Diff line change
Expand Up @@ -363,14 +363,9 @@ func withTracingAndDebug(next http.Handler, authToken string) http.Handler {
return http.HandlerFunc(func(writer http.ResponseWriter, request *http.Request) {
// Disable tracing/debug headers if auth token missing or invalid
if authToken == "" || request.Header.Get("Authorization") != authToken {
if request.Header.Get("Traceparent") != "" {
request.Header.Del("Traceparent")
}
if request.Header.Get("Tracestate") != "" {
request.Header.Del("Tracestate")
}
if request.Header.Get(NoBlockcacheHeader) != "" {
request.Header.Del(NoBlockcacheHeader)
if request.Header.Get("Traceparent") != "" || request.Header.Get("Tracestate") != "" || request.Header.Get(NoBlockcacheHeader) != "" {
http.Error(writer, "The request is not accompanied by a valid authorization header", http.StatusUnauthorized)
return
}
}

Expand Down

0 comments on commit 9b0b7f4

Please sign in to comment.