Skip to content

Commit 71afcc3

Browse files
committed
backend: auth: Extract ConfigureTLSContext from headlamp.go
1 parent 6149f3d commit 71afcc3

File tree

4 files changed

+106
-101
lines changed

4 files changed

+106
-101
lines changed

backend/cmd/headlamp.go

Lines changed: 2 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@ import (
2121
"compress/gzip"
2222
"context"
2323
"crypto/tls"
24-
"crypto/x509"
2524
"encoding/base64"
2625
"encoding/json"
2726
"errors"
@@ -631,7 +630,7 @@ func createHeadlampHandler(config *HeadlampConfig) http.Handler {
631630
return
632631
}
633632

634-
ctx = configureTLSContext(ctx, oidcAuthConfig.SkipTLSVerify, oidcAuthConfig.CACert)
633+
ctx = auth.ConfigureTLSContext(ctx, oidcAuthConfig.SkipTLSVerify, oidcAuthConfig.CACert)
635634

636635
if config.oidcValidatorIdpIssuerURL != "" {
637636
ctx = oidc.InsecureIssuerURLContext(ctx, config.oidcValidatorIdpIssuerURL)
@@ -805,47 +804,12 @@ func createHeadlampHandler(config *HeadlampConfig) http.Handler {
805804
return r
806805
}
807806

808-
// configureTLSContext configures TLS settings for the HTTP client in the context.
809-
// If skipTLSVerify is true, TLS verification will be skipped.
810-
// If caCert is provided, it will be added to the certificate pool for TLS verification.
811-
func configureTLSContext(ctx context.Context, skipTLSVerify *bool, caCert *string) context.Context {
812-
if skipTLSVerify != nil && *skipTLSVerify {
813-
tlsSkipTransport := &http.Transport{
814-
// the gosec linter is disabled here because we are explicitly requesting to skip TLS verification.
815-
TLSClientConfig: &tls.Config{InsecureSkipVerify: true}, //nolint:gosec
816-
}
817-
ctx = oidc.ClientContext(ctx, &http.Client{Transport: tlsSkipTransport})
818-
}
819-
820-
if caCert != nil {
821-
caCertPool := x509.NewCertPool()
822-
if !caCertPool.AppendCertsFromPEM([]byte(*caCert)) {
823-
// Log error but continue with original context
824-
logger.Log(logger.LevelError, nil,
825-
errors.New("failed to append ca cert to pool"), "couldn't add custom cert to context")
826-
return ctx
827-
}
828-
829-
// the gosec linter is disabled because gosec promotes using a minVersion of TLS 1.2 or higher.
830-
// since we are using a custom CA cert configured by the user, we are not forcing a minVersion.
831-
customTransport := &http.Transport{
832-
TLSClientConfig: &tls.Config{ //nolint:gosec
833-
RootCAs: caCertPool,
834-
},
835-
}
836-
837-
ctx = oidc.ClientContext(ctx, &http.Client{Transport: customTransport})
838-
}
839-
840-
return ctx
841-
}
842-
843807
func refreshAndCacheNewToken(oidcAuthConfig *kubeconfig.OidcConfig,
844808
cache cache.Cache[interface{}],
845809
tokenType, token, issuerURL string,
846810
) (*oauth2.Token, error) {
847811
ctx := context.Background()
848-
ctx = configureTLSContext(ctx, oidcAuthConfig.SkipTLSVerify, oidcAuthConfig.CACert)
812+
ctx = auth.ConfigureTLSContext(ctx, oidcAuthConfig.SkipTLSVerify, oidcAuthConfig.CACert)
849813

850814
// get provider
851815
provider, err := oidc.NewProvider(ctx, issuerURL)

backend/cmd/headlamp_test.go

Lines changed: 0 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@ import (
2323
"crypto/x509"
2424
"encoding/base64"
2525
"encoding/json"
26-
"encoding/pem"
2726
"fmt"
2827
"io"
2928
"net/http"
@@ -1290,68 +1289,6 @@ func TestProcessTokenProtocol(t *testing.T) {
12901289
}
12911290
}
12921291

1293-
// TestConfigureTLSContext_NoConfig tests when both skipTLSVerify and caCert are not set.
1294-
func TestConfigureTLSContext_NoConfig(t *testing.T) {
1295-
baseCtx := context.Background()
1296-
resultCtx := configureTLSContext(baseCtx, nil, nil)
1297-
1298-
// Context should remain unchanged when no TLS configuration is provided
1299-
assert.Equal(t, baseCtx, resultCtx, "Context should remain unchanged when no TLS configuration is provided")
1300-
}
1301-
1302-
/*
1303-
TestConfigureTLSContext_SkipTLS tests when skipTLSVerify is set to true.
1304-
The OIDC library would use this context to make requests
1305-
We can't directly extract the client, but we can verify the behavior
1306-
by checking that the context was modified (indicating TLS config was applied).
1307-
*/
1308-
func TestConfigureTLSContext_SkipTLS(t *testing.T) {
1309-
// Create a test server that requires TLS
1310-
server := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
1311-
w.WriteHeader(http.StatusOK)
1312-
_, err := w.Write([]byte("TLS connection successful"))
1313-
require.NoError(t, err)
1314-
}))
1315-
defer server.Close()
1316-
1317-
baseCtx := context.Background()
1318-
skipTLSVerify := true
1319-
resultCtx := configureTLSContext(baseCtx, &skipTLSVerify, nil)
1320-
1321-
// Context should be modified when skipTLSVerify is true
1322-
assert.NotEqual(t, baseCtx, resultCtx, "Context should be modified when skipTLSVerify is true")
1323-
1324-
// Test that the configured context can make TLS requests with skip verification
1325-
// This verifies that the TLS configuration was actually applied
1326-
_, err := http.NewRequestWithContext(resultCtx, "GET", server.URL, nil)
1327-
require.NoError(t, err)
1328-
}
1329-
1330-
// TestConfigureTLSContext_CACert tests when caCert is provided.
1331-
func TestConfigureTLSContext_CACert(t *testing.T) {
1332-
// Read the pre-generated CA certificate from testdata
1333-
caCertBytes, err := os.ReadFile("headlamp_testdata/ca.crt")
1334-
require.NoError(t, err)
1335-
1336-
// Test the configureTLSContext function with the CA certificate
1337-
baseCtx := context.Background()
1338-
caCert := string(caCertBytes)
1339-
resultCtx := configureTLSContext(baseCtx, nil, &caCert)
1340-
1341-
// Context should be modified when caCert is provided
1342-
assert.NotEqual(t, baseCtx, resultCtx, "Context should be modified when caCert is provided")
1343-
1344-
// Verify that the CA certificate was parsed correctly by checking if it's valid PEM
1345-
block, _ := pem.Decode([]byte(caCert))
1346-
assert.NotNil(t, block, "CA certificate should be valid PEM format")
1347-
assert.Equal(t, "CERTIFICATE", block.Type, "CA certificate should be of type CERTIFICATE")
1348-
1349-
// Parse the CA certificate to verify it's valid
1350-
caCertParsed, err := x509.ParseCertificate(block.Bytes)
1351-
require.NoError(t, err)
1352-
assert.True(t, caCertParsed.IsCA, "Generated certificate should be a CA certificate")
1353-
}
1354-
13551292
// newFakeK8sServer create a mock k8s server for testing purpose,
13561293
// this would help to test Caching Machanism without making request
13571294
// to the k8s server.

backend/pkg/auth/auth.go

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@ package auth
1818

1919
import (
2020
"context"
21+
"crypto/tls"
22+
"crypto/x509"
2123
"encoding/base64"
2224
"encoding/json"
2325
"errors"
@@ -27,6 +29,7 @@ import (
2729
"strings"
2830
"time"
2931

32+
"github.com/coreos/go-oidc/v3/oidc"
3033
"github.com/kubernetes-sigs/headlamp/backend/pkg/cache"
3134
"github.com/kubernetes-sigs/headlamp/backend/pkg/logger"
3235
"golang.org/x/oauth2"
@@ -201,3 +204,39 @@ func GetNewToken(clientID, clientSecret string, cache cache.Cache[interface{}],
201204

202205
return newToken, nil
203206
}
207+
208+
// ConfigureTLSContext configures TLS settings for the HTTP client in the context.
209+
// When skipTLSVerify is true, a client that skips verification is installed.
210+
// When caCert is provided, a client with that CA pool is installed and takes precedence,
211+
// re-enabling verification while trusting the supplied certificate bundle.
212+
func ConfigureTLSContext(ctx context.Context, skipTLSVerify *bool, caCert *string) context.Context {
213+
if skipTLSVerify != nil && *skipTLSVerify {
214+
tlsSkipTransport := &http.Transport{
215+
// the gosec linter is disabled here because we are explicitly requesting to skip TLS verification.
216+
TLSClientConfig: &tls.Config{InsecureSkipVerify: true}, //nolint:gosec
217+
}
218+
ctx = oidc.ClientContext(ctx, &http.Client{Transport: tlsSkipTransport})
219+
}
220+
221+
if caCert != nil {
222+
caCertPool := x509.NewCertPool()
223+
if !caCertPool.AppendCertsFromPEM([]byte(*caCert)) {
224+
// Log error but continue with original context
225+
logger.Log(logger.LevelError, nil,
226+
errors.New("failed to append ca cert to pool"), "couldn't add custom cert to context")
227+
return ctx
228+
}
229+
230+
// the gosec linter is disabled because gosec promotes using a minVersion of TLS 1.2 or higher.
231+
// since we are using a custom CA cert configured by the user, we are not forcing a minVersion.
232+
customTransport := &http.Transport{
233+
TLSClientConfig: &tls.Config{ //nolint:gosec
234+
RootCAs: caCertPool,
235+
},
236+
}
237+
238+
ctx = oidc.ClientContext(ctx, &http.Client{Transport: customTransport})
239+
}
240+
241+
return ctx
242+
}

backend/pkg/auth/auth_test.go

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,18 +18,23 @@ package auth_test
1818

1919
import (
2020
"context"
21+
"crypto/x509"
2122
"encoding/base64"
2223
"encoding/json"
24+
"encoding/pem"
2325
"errors"
2426
"net/http"
2527
"net/http/httptest"
28+
"os"
2629
"reflect"
2730
"strings"
2831
"testing"
2932
"time"
3033

3134
"github.com/kubernetes-sigs/headlamp/backend/pkg/auth"
3235
"github.com/kubernetes-sigs/headlamp/backend/pkg/cache"
36+
"github.com/stretchr/testify/assert"
37+
"github.com/stretchr/testify/require"
3338
"golang.org/x/oauth2"
3439
)
3540

@@ -683,3 +688,63 @@ func TestGetNewToken_CacheUpdateErrors(t *testing.T) {
683688
})
684689
}
685690
}
691+
692+
// TestConfigureTLSContext_NoConfig tests when both skipTLSVerify and caCert are not set.
693+
func TestConfigureTLSContext_NoConfig(t *testing.T) {
694+
baseCtx := context.Background()
695+
resultCtx := auth.ConfigureTLSContext(baseCtx, nil, nil)
696+
697+
// Context should remain unchanged when no TLS configuration is provided
698+
assert.Equal(t, baseCtx, resultCtx, "Context should remain unchanged when no TLS configuration is provided")
699+
}
700+
701+
// TestConfigureTLSContext_SkipTLS tests when skipTLSVerify is set to true.
702+
// The OIDC library would use this context to make requests
703+
// We can't directly extract the client, but we can verify the behavior
704+
// by checking that the context was modified (indicating TLS config was applied).
705+
func TestConfigureTLSContext_SkipTLS(t *testing.T) {
706+
// Create a test server that requires TLS
707+
server := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
708+
w.WriteHeader(http.StatusOK)
709+
_, err := w.Write([]byte("TLS connection successful"))
710+
require.NoError(t, err)
711+
}))
712+
defer server.Close()
713+
714+
baseCtx := context.Background()
715+
skipTLSVerify := true
716+
resultCtx := auth.ConfigureTLSContext(baseCtx, &skipTLSVerify, nil)
717+
718+
// Context should be modified when skipTLSVerify is true
719+
assert.NotEqual(t, baseCtx, resultCtx, "Context should be modified when skipTLSVerify is true")
720+
721+
// Test that the configured context can make TLS requests with skip verification
722+
// This verifies that the TLS configuration was actually applied
723+
_, err := http.NewRequestWithContext(resultCtx, "GET", server.URL, nil)
724+
require.NoError(t, err)
725+
}
726+
727+
// TestConfigureTLSContext_CACert tests when caCert is provided.
728+
func TestConfigureTLSContext_CACert(t *testing.T) {
729+
// Read the pre-generated CA certificate from testdata
730+
caCertBytes, err := os.ReadFile("../../cmd/headlamp_testdata/ca.crt")
731+
require.NoError(t, err)
732+
733+
// Test the configureTLSContext function with the CA certificate
734+
baseCtx := context.Background()
735+
caCert := string(caCertBytes)
736+
resultCtx := auth.ConfigureTLSContext(baseCtx, nil, &caCert)
737+
738+
// Context should be modified when caCert is provided
739+
assert.NotEqual(t, baseCtx, resultCtx, "Context should be modified when caCert is provided")
740+
741+
// Verify that the CA certificate was parsed correctly by checking if it's valid PEM
742+
block, _ := pem.Decode([]byte(caCert))
743+
assert.NotNil(t, block, "CA certificate should be valid PEM format")
744+
assert.Equal(t, "CERTIFICATE", block.Type, "CA certificate should be of type CERTIFICATE")
745+
746+
// Parse the CA certificate to verify it's valid
747+
caCertParsed, err := x509.ParseCertificate(block.Bytes)
748+
require.NoError(t, err)
749+
assert.True(t, caCertParsed.IsCA, "Generated certificate should be a CA certificate")
750+
}

0 commit comments

Comments
 (0)