Skip to content

Commit 6de2b36

Browse files
william-romCopilot
andauthored
feat: add https validation and tls config option (#33)
* feat: add https validation and tls config option * chore: typo Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
1 parent ce3cddc commit 6de2b36

File tree

2 files changed

+80
-10
lines changed

2 files changed

+80
-10
lines changed

keyfetch.go

Lines changed: 69 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package jwt
22

33
import (
44
"context"
5+
"crypto/tls"
56
"encoding/json"
67
"fmt"
78
"io"
@@ -45,8 +46,10 @@ type JWKSFetcherOpts struct {
4546
httpClientIdleConnTimeout time.Duration
4647
httpClientMaxIdleCon int
4748
logger *slog.Logger
48-
maxResponseSize int64 // Maximum size of JWKS response in bytes
49-
maxKeysCount int // Maximum number of keys allowed in JWKS
49+
maxResponseSize int64 // Maximum size of JWKS response in bytes
50+
maxKeysCount int // Maximum number of keys allowed in JWKS
51+
tlsConfig *tls.Config // TLS configuration for HTTPS connections
52+
requireHTTPS bool // Require HTTPS for JWKS URLs (true by default for security)
5053
}
5154

5255
// Where the keyfetcher will fetch its public keys from.
@@ -65,6 +68,16 @@ type Generic struct {
6568
DiscoveryURL string
6669
}
6770

71+
// defaultTLSConfig returns a secure TLS configuration with modern standards.
72+
func defaultTLSConfig() *tls.Config {
73+
return &tls.Config{
74+
MinVersion: tls.VersionTLS12, // Require TLS 1.2 minimum
75+
InsecureSkipVerify: false, // Always verify certificates
76+
// Cipher suites intentionally not set - Go's defaults are secure and
77+
// automatically updated with each release to use the best available options
78+
}
79+
}
80+
6881
// NewJWKSFetcher creates a new JWKSFetcher from a keySource.
6982
func NewJWKSFetcher(source keySource, options ...Option) (*JWKSFetcher, error) {
7083
// Set default fetcher opts
@@ -77,6 +90,8 @@ func NewJWKSFetcher(source keySource, options ...Option) (*JWKSFetcher, error) {
7790
logger: slog.Default(),
7891
maxResponseSize: defaultMaxResponseSize,
7992
maxKeysCount: defaultMaxKeysCount,
93+
tlsConfig: defaultTLSConfig(), // Use secure defaults
94+
requireHTTPS: true, // Require HTTPS by default for security
8095
}
8196

8297
// Apply options set by user
@@ -93,6 +108,7 @@ func NewJWKSFetcher(source keySource, options ...Option) (*JWKSFetcher, error) {
93108
MaxIdleConns: opts.httpClientMaxIdleCon,
94109
IdleConnTimeout: opts.httpClientIdleConnTimeout,
95110
TLSHandshakeTimeout: opts.tlsHandshakeTimeout,
111+
TLSClientConfig: opts.tlsConfig,
96112
},
97113
}
98114

@@ -101,9 +117,9 @@ func NewJWKSFetcher(source keySource, options ...Option) (*JWKSFetcher, error) {
101117
return nil, fmt.Errorf("failed to set discovery url: %w", err)
102118
}
103119

104-
jwksURL, err := fetchJWKSURL(context.Background(), discoveryURL, httpClient)
120+
jwksURL, err := fetchJWKSURL(context.Background(), discoveryURL, httpClient, opts.requireHTTPS)
105121
if err != nil {
106-
return nil, fmt.Errorf("failed to fetch JWKS URL from discoveryURR '%s': %w", discoveryURL, err)
122+
return nil, fmt.Errorf("failed to fetch JWKS URL from discoveryURL '%s': %w", discoveryURL, err)
107123
}
108124

109125
return &JWKSFetcher{
@@ -208,11 +224,22 @@ func (f *JWKSFetcher) fetchRemoteJWKS(ctx context.Context, jwksURL string) (JWKS
208224
}
209225

210226
// Gets the JWKS URL from the OIDC discovery document.
211-
func fetchJWKSURL(ctx context.Context, discoveryURL string, client *http.Client) (string, error) {
227+
func fetchJWKSURL(ctx context.Context, discoveryURL string, client *http.Client, requireHTTPS bool) (string, error) {
212228
if discoveryURL == "" {
213229
return "", fmt.Errorf("discovery url can not be empty")
214230
}
215231

232+
// Validate discovery URL uses HTTPS if required
233+
if requireHTTPS {
234+
discoveryParsed, err := url.Parse(discoveryURL)
235+
if err != nil {
236+
return "", fmt.Errorf("failed to parse discovery URL: %w", err)
237+
}
238+
if discoveryParsed.Scheme != "https" {
239+
return "", fmt.Errorf("discovery URL must use HTTPS, got scheme: %s (use WithRequireHTTPS(false) to allow HTTP in secure environments)", discoveryParsed.Scheme)
240+
}
241+
}
242+
216243
req, err := http.NewRequestWithContext(ctx, http.MethodGet, discoveryURL, nil)
217244
if err != nil {
218245
return "", fmt.Errorf("failed to create OIDC discovery request: %w", err)
@@ -237,6 +264,17 @@ func fetchJWKSURL(ctx context.Context, discoveryURL string, client *http.Client)
237264
return "", fmt.Errorf("jwks_uri not found in discovery doc from %s", discoveryURL)
238265
}
239266

267+
// Validate JWKS URL uses HTTPS if required
268+
if requireHTTPS {
269+
jwksParsed, err := url.Parse(discoveryDoc.JwksURI)
270+
if err != nil {
271+
return "", fmt.Errorf("failed to parse JWKS URL from discovery document: %w", err)
272+
}
273+
if jwksParsed.Scheme != "https" {
274+
return "", fmt.Errorf("JWKS URL must use HTTPS for security, got: %s (use WithRequireHTTPS(false) to allow HTTP in secure environments)", discoveryDoc.JwksURI)
275+
}
276+
}
277+
240278
return discoveryDoc.JwksURI, nil
241279
}
242280

@@ -253,11 +291,13 @@ func (g Generic) getDiscoveryEndpoint() (string, error) {
253291
return "", fmt.Errorf("discovery url cannot be empty")
254292
}
255293

294+
// Validate it's a valid URL
256295
_, err := url.ParseRequestURI(g.DiscoveryURL)
257296
if err != nil {
258297
return "", fmt.Errorf("invalid DiscoveryURL: %w", err)
259298
}
260299

300+
// HTTPS validation is now done in fetchJWKSURL based on requireHTTPS option
261301
return g.DiscoveryURL, nil
262302
}
263303

@@ -351,3 +391,27 @@ func WithMaxKeysCount(count int) Option {
351391
return nil
352392
}
353393
}
394+
395+
// WithTLSConfig sets a custom TLS configuration for JWKS fetching.
396+
// If not set, a secure default configuration with TLS 1.2+ and strong cipher suites is used.
397+
// Use this option if you need specific TLS settings for your environment.
398+
func WithTLSConfig(tlsConfig *tls.Config) Option {
399+
return func(o *JWKSFetcherOpts) error {
400+
if tlsConfig == nil {
401+
return fmt.Errorf("WithTLSConfig: tlsConfig cannot be nil")
402+
}
403+
o.tlsConfig = tlsConfig
404+
return nil
405+
}
406+
}
407+
408+
// WithRequireHTTPS controls whether HTTPS is required for JWKS URLs.
409+
// By default, HTTPS is required for security (true).
410+
// Set to false only in secure environments like airgapped networks or internal systems
411+
// where TLS termination happens at a different layer.
412+
func WithRequireHTTPS(require bool) Option {
413+
return func(o *JWKSFetcherOpts) error {
414+
o.requireHTTPS = require
415+
return nil
416+
}
417+
}

keyfetch_test.go

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,8 @@ func TestStartInitialFetchSync(t *testing.T) {
109109
defer server.Close()
110110

111111
// Create fetcher with the test server URL as discovery endpoint
112-
fetcher, err := NewJWKSFetcher(Generic{DiscoveryURL: server.URL})
112+
// Use WithRequireHTTPS(false) for local testing with HTTP
113+
fetcher, err := NewJWKSFetcher(Generic{DiscoveryURL: server.URL}, WithRequireHTTPS(false))
113114
require.NoError(t, err, "failed to create JWKS fetcher")
114115

115116
ctx := context.Background()
@@ -139,7 +140,8 @@ func TestStartInitialFetchError(t *testing.T) {
139140
defer server.Close()
140141

141142
// Create fetcher with the test server URL as discovery endpoint
142-
fetcher, err := NewJWKSFetcher(Generic{DiscoveryURL: server.URL})
143+
// Use WithRequireHTTPS(false) for local testing with HTTP
144+
fetcher, err := NewJWKSFetcher(Generic{DiscoveryURL: server.URL}, WithRequireHTTPS(false))
143145
require.NoError(t, err, "failed to create JWKS fetcher")
144146

145147
ctx := context.Background()
@@ -199,7 +201,7 @@ func TestNoRaceConditionOnStartup(t *testing.T) {
199201
defer server.Close()
200202

201203
// Create fetcher
202-
fetcher, err := NewJWKSFetcher(Generic{DiscoveryURL: server.URL})
204+
fetcher, err := NewJWKSFetcher(Generic{DiscoveryURL: server.URL}, WithRequireHTTPS(false))
203205
require.NoError(t, err, "failed to create JWKS fetcher")
204206

205207
ctx := context.Background()
@@ -267,6 +269,7 @@ func TestBackgroundSyncContinuesAfterStart(t *testing.T) {
267269
fetcher, err := NewJWKSFetcher(
268270
Generic{DiscoveryURL: server.URL},
269271
WithFetchInterval(testFetchInterval),
272+
WithRequireHTTPS(false), // Allow HTTP for testing
270273
)
271274
require.NoError(t, err, "failed to create JWKS fetcher")
272275

@@ -354,6 +357,7 @@ func TestMaxResponseSize(t *testing.T) {
354357
fetcher, err := NewJWKSFetcher(
355358
Generic{DiscoveryURL: server.URL + "/.well-known/openid-configuration"},
356359
WithMaxResponseSize(100), // Very small limit
360+
WithRequireHTTPS(false), // Allow HTTP for testing
357361
)
358362
require.NoError(t, err)
359363

@@ -405,7 +409,8 @@ func TestMaxKeysCount(t *testing.T) {
405409
// Create fetcher with small max keys count
406410
fetcher, err := NewJWKSFetcher(
407411
Generic{DiscoveryURL: server.URL + "/.well-known/openid-configuration"},
408-
WithMaxKeysCount(10), // Allow only 10 keys
412+
WithMaxKeysCount(10), // Allow only 10 keys
413+
WithRequireHTTPS(false), // Allow HTTP for testing
409414
)
410415
require.NoError(t, err)
411416

@@ -502,9 +507,10 @@ func TestJWKSLimits(t *testing.T) {
502507
defer server.Close()
503508

504509
// Create fetcher with specified options
510+
allOptions := append(tt.options, WithRequireHTTPS(false)) //nolint:gocritic // intentionally creating new slice
505511
fetcher, err := NewJWKSFetcher(
506512
Generic{DiscoveryURL: server.URL + "/.well-known/openid-configuration"},
507-
tt.options...,
513+
allOptions...,
508514
)
509515
require.NoError(t, err)
510516

0 commit comments

Comments
 (0)