Skip to content

Commit af32700

Browse files
authored
fix: Service utilize httputil.SafeHttpClient (#1926)
### Proposed Changes This PR follows #1910 by updating `service` to utilize the new `httputil` helper for constructing a client which wont follow redirects, and has sensible timeouts. To quote that description: > This change switches us from maintaining a tls config which we then on-demand initialize an http.Client with to instead maintain and reuse an http.Client instance. This enables us to utilize the connection pooling which occurs within the http.Transport to reduce ssl handshakes and thus reduce latency. > > In addition this change provides us a central place to configure out http.Client (httputil). Allowing us to easily set configuration options to reduce the security risks of using an unconfigured http.Client. Notably timeouts to reduce DoS risks, and control around following redirects to prevent blind SSRF's. The prior auth API was marked as deprecated. There is no remaining use within this repo, so it may be able to be removed. Merging this should fully resolve #1891. ### Checklist - [X] I have added or updated unit tests - [ ] I have added or updated integration tests (if appropriate) - [ ] I have added or updated documentation ### Testing Instructions
1 parent 9e289c9 commit af32700

File tree

4 files changed

+18
-12
lines changed

4 files changed

+18
-12
lines changed

sdk/auth/token_adding_interceptor.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,8 @@ const (
2525
JTILength = 14
2626
)
2727

28+
// Deprecated: NewTokenAddingInterceptor is deprecated, use NewTokenAddingInterceptorWithClient instead. A http client
29+
// can be constructed using httputil.SafeHTTPClientWithTLSConfig, but should be reused as much as possible.
2830
func NewTokenAddingInterceptor(t AccessTokenSource, c *tls.Config) TokenAddingInterceptor {
2931
return NewTokenAddingInterceptorWithClient(t, httputil.SafeHTTPClientWithTLSConfig(c))
3032
}

sdk/auth/token_adding_interceptor_test.go

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import (
1818
"github.com/lestrrat-go/jwx/v2/jws"
1919
"github.com/lestrrat-go/jwx/v2/jwt"
2020
"github.com/opentdf/platform/protocol/go/kas"
21+
"github.com/opentdf/platform/sdk/httputil"
2122
"github.com/stretchr/testify/assert"
2223
"github.com/stretchr/testify/require"
2324
"google.golang.org/grpc"
@@ -42,9 +43,9 @@ func TestAddingTokensToOutgoingRequest(t *testing.T) {
4243
accessToken: "thisisafakeaccesstoken",
4344
}
4445
server := FakeAccessServiceServer{}
45-
oo := NewTokenAddingInterceptor(&ts, &tls.Config{
46+
oo := NewTokenAddingInterceptorWithClient(&ts, httputil.SafeHTTPClientWithTLSConfig(&tls.Config{
4647
MinVersion: tls.VersionTLS12,
47-
})
48+
}))
4849

4950
client, stop := runServer(&server, oo)
5051
defer stop()
@@ -97,9 +98,9 @@ func TestAddingTokensToOutgoingRequest(t *testing.T) {
9798
func Test_InvalidCredentials_DoesNotSendMessage(t *testing.T) {
9899
ts := FakeTokenSource{key: nil, accessToken: ""}
99100
server := FakeAccessServiceServer{}
100-
oo := NewTokenAddingInterceptor(&ts, &tls.Config{
101+
oo := NewTokenAddingInterceptorWithClient(&ts, httputil.SafeHTTPClientWithTLSConfig(&tls.Config{
101102
MinVersion: tls.VersionTLS12,
102-
})
103+
}))
103104

104105
client, stop := runServer(&server, oo)
105106
defer stop()

service/internal/auth/authn_test.go

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ import (
2929
"github.com/opentdf/platform/protocol/go/kas"
3030
"github.com/opentdf/platform/protocol/go/kas/kasconnect"
3131
sdkauth "github.com/opentdf/platform/sdk/auth"
32+
"github.com/opentdf/platform/sdk/httputil"
3233
"github.com/opentdf/platform/service/internal/server/memhttp"
3334
"github.com/opentdf/platform/service/logger"
3435
ctxAuth "github.com/opentdf/platform/service/pkg/auth"
@@ -458,12 +459,12 @@ func (s *AuthSuite) TestDPoPEndToEnd_GRPC() {
458459
server := memhttp.New(mux)
459460
defer server.Close()
460461

461-
addingInterceptor := sdkauth.NewTokenAddingInterceptor(&FakeTokenSource{
462+
addingInterceptor := sdkauth.NewTokenAddingInterceptorWithClient(&FakeTokenSource{
462463
key: dpopKey,
463464
accessToken: string(signedTok),
464-
}, &tls.Config{
465+
}, httputil.SafeHTTPClientWithTLSConfig(&tls.Config{
465466
MinVersion: tls.VersionTLS12,
466-
})
467+
}))
467468

468469
conn, _ := grpc.NewClient("passthrough://bufconn", grpc.WithContextDialer(func(ctx context.Context, _ string) (net.Conn, error) {
469470
return server.Listener.DialContext(ctx, "tcp", "http://localhost:8080")
@@ -519,19 +520,19 @@ func (s *AuthSuite) TestDPoPEndToEnd_HTTP() {
519520

520521
req, err := http.NewRequest(http.MethodGet, server.URL+"/attributes", nil)
521522

522-
addingInterceptor := sdkauth.NewTokenAddingInterceptor(&FakeTokenSource{
523+
addingInterceptor := sdkauth.NewTokenAddingInterceptorWithClient(&FakeTokenSource{
523524
key: dpopKey,
524525
accessToken: string(signedTok),
525-
}, &tls.Config{
526+
}, httputil.SafeHTTPClientWithTLSConfig(&tls.Config{
526527
MinVersion: tls.VersionTLS12,
527-
})
528+
}))
528529
s.Require().NoError(err)
529530
req.Header.Set("Authorization", fmt.Sprintf("Bearer %s", signedTok))
530531
dpopTok, err := addingInterceptor.GetDPoPToken(server.URL+"/attributes", "GET", string(signedTok))
531532
s.Require().NoError(err)
532533
req.Header.Set("DPoP", dpopTok)
533534

534-
client := http.Client{}
535+
client := httputil.SafeHTTPClient() // use safe client to help validate the client
535536
_, err = client.Do(req)
536537
s.Require().NoError(err)
537538
var dpopKeyFromRequest jwk.Key

service/pkg/server/start.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import (
1717
"github.com/opentdf/platform/sdk"
1818
sdkauth "github.com/opentdf/platform/sdk/auth"
1919
"github.com/opentdf/platform/sdk/auth/oauth"
20+
"github.com/opentdf/platform/sdk/httputil"
2021
"github.com/opentdf/platform/service/internal/auth"
2122
"github.com/opentdf/platform/service/internal/config"
2223
"github.com/opentdf/platform/service/internal/server"
@@ -231,7 +232,8 @@ func Start(f ...StartOptions) error {
231232
return fmt.Errorf("error creating ERS tokensource: %w", err)
232233
}
233234

234-
interceptor := sdkauth.NewTokenAddingInterceptor(ts, tlsConfig)
235+
interceptor := sdkauth.NewTokenAddingInterceptorWithClient(ts,
236+
httputil.SafeHTTPClientWithTLSConfig(tlsConfig))
235237

236238
ersDialOptions = append(ersDialOptions, grpc.WithChainUnaryInterceptor(interceptor.AddCredentials))
237239
}

0 commit comments

Comments
 (0)