From f8f475a65d2a6555a81d1f69779dd8a0f9926ae5 Mon Sep 17 00:00:00 2001 From: Andrew Nesbitt Date: Mon, 13 Jul 2026 16:17:52 -0700 Subject: [PATCH 1/2] upstream: apply authentication through shared transport --- config.example.yaml | 3 +- docs/architecture.md | 2 + docs/configuration.md | 6 +- internal/config/config.go | 51 +++- internal/config/config_test.go | 42 +++ internal/httpclient/transport.go | 418 ++++++++++++++++++++++++++ internal/httpclient/transport_test.go | 151 ++++++++++ internal/server/server.go | 32 +- 8 files changed, 689 insertions(+), 16 deletions(-) create mode 100644 internal/httpclient/transport.go create mode 100644 internal/httpclient/transport_test.go diff --git a/config.example.yaml b/config.example.yaml index d1ed9a1..2124e48 100644 --- a/config.example.yaml +++ b/config.example.yaml @@ -96,7 +96,8 @@ upstream: cargo_download: "https://static.crates.io/crates" # Authentication for upstream registries - # Keys are URL prefixes matched against request URLs. + # Keys are absolute URL scopes. Scheme, host, effective port, and path + # segment boundaries must match; the longest matching scope wins. # Values can reference environment variables using ${VAR_NAME} syntax. # # Supported auth types: diff --git a/docs/architecture.md b/docs/architecture.md index f04d548..cf8b0e2 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -240,6 +240,8 @@ Fetches artifacts from upstream registries. - Exponential backoff retry on 429 (rate limit) and 5xx errors - Returns streaming reader (doesn't load into memory) - Configurable user-agent +- Shares an authentication-aware transport with metadata requests so URL-scoped credentials apply consistently +- Discovers and caches scoped OCI Bearer tokens from registry challenges **Resolver:** - Determines download URL for a package/version diff --git a/docs/configuration.md b/docs/configuration.md index dcdec24..02d426f 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -123,7 +123,9 @@ upstream: ## Authentication -Configure authentication for private upstream registries. Auth is matched by URL prefix, and credentials can reference environment variables using `${VAR_NAME}` syntax. +Configure authentication for private upstream registries. The same authentication-aware client is used for metadata and artifact downloads, and credentials can reference environment variables using `${VAR_NAME}` syntax. + +OCI registries that return a Bearer challenge from a `/v2/{repository}/…` endpoint are handled automatically. The proxy discovers the token realm from `WWW-Authenticate`, applies any configured credentials for the token URL, and reuses the scoped token until shortly before it expires. ### Bearer Token @@ -172,7 +174,7 @@ upstream: ### URL Matching -Auth configs are matched by URL prefix. The longest matching prefix wins, so you can configure different credentials for different paths: +Auth keys must be absolute URLs. Matching compares the scheme, host, effective port, and path-segment prefix, preventing credentials for `registry.example.com` from being sent to a lookalike host such as `registry.example.com.evil.test`. The longest matching scope wins, so you can configure different credentials for different paths: ```yaml upstream: diff --git a/internal/config/config.go b/internal/config/config.go index 2d275c7..a54d50c 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -274,23 +274,29 @@ type UpstreamConfig struct { CargoDownload string `json:"cargo_download" yaml:"cargo_download"` // Auth configures authentication for upstream registries. - // Keys are URL prefixes that are matched against request URLs. + // Keys are absolute URL scopes matched by scheme, host, effective port, + // and path-segment prefix. // Example: "https://npm.pkg.github.com" matches all requests to that host. Auth map[string]AuthConfig `json:"auth" yaml:"auth"` } // AuthForURL returns the auth config that matches the given URL. -// Matches are based on URL prefix - the longest matching prefix wins. +// The longest matching URL scope wins. func (u *UpstreamConfig) AuthForURL(url string) *AuthConfig { if u.Auth == nil { return nil } + target, err := parseAuthURL(url) + if err != nil { + return nil + } var bestMatch *AuthConfig var bestLen int for pattern, auth := range u.Auth { - if strings.HasPrefix(url, pattern) && len(pattern) > bestLen { + configured, err := parseAuthURL(pattern) + if err == nil && authURLMatches(configured, target) && len(pattern) > bestLen { a := auth // copy to avoid loop variable capture bestMatch = &a bestLen = len(pattern) @@ -300,6 +306,45 @@ func (u *UpstreamConfig) AuthForURL(url string) *AuthConfig { return bestMatch } +func parseAuthURL(value string) (*url.URL, error) { + parsed, err := url.Parse(value) + if err != nil || !parsed.IsAbs() || parsed.Hostname() == "" || parsed.Opaque != "" { + return nil, fmt.Errorf("invalid authentication URL") + } + return parsed, nil +} + +func authURLMatches(configured, target *url.URL) bool { + if !strings.EqualFold(configured.Scheme, target.Scheme) || + !strings.EqualFold(configured.Hostname(), target.Hostname()) || + authURLPort(configured) != authURLPort(target) { + return false + } + if configured.RawQuery != "" && configured.RawQuery != target.RawQuery { + return false + } + + configuredPath := strings.TrimSuffix(configured.EscapedPath(), "/") + if configuredPath == "" { + return true + } + targetPath := strings.TrimSuffix(target.EscapedPath(), "/") + return targetPath == configuredPath || strings.HasPrefix(targetPath, configuredPath+"/") +} + +func authURLPort(value *url.URL) string { + if port := value.Port(); port != "" { + return port + } + if strings.EqualFold(value.Scheme, "https") { + return "443" + } + if strings.EqualFold(value.Scheme, "http") { + return "80" + } + return "" +} + // AuthConfig configures authentication for an upstream registry. type AuthConfig struct { // Type is the authentication type: "bearer", "basic", or "header". diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 9c34023..63520a2 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -760,3 +760,45 @@ func TestDatabaseConfigString(t *testing.T) { } } } + +func TestUpstreamAuthForURLMatchesURLComponents(t *testing.T) { + registryAuth := AuthConfig{Type: "bearer", Token: "registry-token"} + privateAuth := AuthConfig{Type: "bearer", Token: "private-token"} + config := UpstreamConfig{Auth: map[string]AuthConfig{ + "https://registry.example.com": registryAuth, + "https://registry.example.com/private": privateAuth, + }} + + tests := []struct { + name string + url string + wantToken string + }{ + {name: "registry root", url: "https://registry.example.com/package", wantToken: "registry-token"}, + {name: "host is case insensitive", url: "https://REGISTRY.EXAMPLE.COM/package", wantToken: "registry-token"}, + {name: "longest path match", url: "https://registry.example.com/private/package", wantToken: "private-token"}, + {name: "exact path match", url: "https://registry.example.com/private", wantToken: "private-token"}, + {name: "path segment boundary", url: "https://registry.example.com/private-other/package", wantToken: "registry-token"}, + {name: "lookalike host rejected", url: "https://registry.example.com.evil.test/package"}, + {name: "different scheme rejected", url: "http://registry.example.com/package"}, + {name: "different port rejected", url: "https://registry.example.com:8443/package"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + auth := config.AuthForURL(tt.url) + if tt.wantToken == "" { + if auth != nil { + t.Fatalf("AuthForURL() = %+v, want nil", auth) + } + return + } + if auth == nil { + t.Fatal("AuthForURL() = nil, want authentication") + } + if auth.Token != tt.wantToken { + t.Errorf("token = %q, want %q", auth.Token, tt.wantToken) + } + }) + } +} diff --git a/internal/httpclient/transport.go b/internal/httpclient/transport.go new file mode 100644 index 0000000..1cdb457 --- /dev/null +++ b/internal/httpclient/transport.go @@ -0,0 +1,418 @@ +// Package httpclient provides authentication-aware HTTP transports for upstream requests. +package httpclient + +import ( + "context" + "encoding/json" + "fmt" + "io" + "net/http" + "net/url" + "strings" + "sync" + "time" +) + +const ( + defaultTokenLifetime = 60 * time.Second + tokenExpirySkew = 5 * time.Second + maxTokenResponseSize = 1 << 20 + shortTokenSkewDivisor = 10 +) + +// AuthFunc returns a configured authentication header for a URL. +type AuthFunc func(url string) (headerName, headerValue string) + +// Transport adds configured authentication and follows OCI Bearer challenges. +type Transport struct { + base http.RoundTripper + authForURL AuthFunc + + mu sync.Mutex + tokens map[string]cachedToken + challenges map[string]bearerChallenge +} + +type cachedToken struct { + value string + expiresAt time.Time +} + +type bearerChallenge struct { + realm string + service string + scopes []string +} + +type tokenResponse struct { + Token string `json:"token"` + AccessToken string `json:"access_token"` + ExpiresIn int64 `json:"expires_in"` + IssuedAt string `json:"issued_at"` +} + +// NewTransport creates an authentication-aware transport around base. +func NewTransport(base http.RoundTripper, authForURL AuthFunc) *Transport { + if base == nil { + base = http.DefaultTransport + } + return &Transport{ + base: base, + authForURL: authForURL, + tokens: make(map[string]cachedToken), + challenges: make(map[string]bearerChallenge), + } +} + +// RoundTrip implements http.RoundTripper. +func (t *Transport) RoundTrip(req *http.Request) (*http.Response, error) { + outbound := cloneRequest(req) + t.applyAuthentication(outbound, req.Header.Get("Authorization") != "") + + resp, err := t.base.RoundTrip(outbound) + if err != nil || resp.StatusCode != http.StatusUnauthorized { + return resp, err + } + if registryProtectionSpace(req.URL) == "" { + return resp, nil + } + + challenge, ok := parseBearerChallenge(resp.Header.Values("WWW-Authenticate")) + if !ok || !canReplay(req) { + return resp, nil + } + + drainAndClose(resp.Body) + token, err := t.token(req.Context(), challenge) + if err != nil { + return nil, fmt.Errorf("registry authentication: %w", err) + } + t.rememberChallenge(req.URL, challenge) + + retry, err := cloneRequestForRetry(req) + if err != nil { + return nil, err + } + t.applyConfiguredAuthentication(retry) + retry.Header.Set("Authorization", "Bearer "+token) + return t.base.RoundTrip(retry) +} + +func (t *Transport) applyAuthentication(req *http.Request, hasExplicitAuthorization bool) { + t.applyConfiguredAuthentication(req) + if hasExplicitAuthorization { + return + } + if token := t.cachedTokenForRequest(req.URL); token != "" { + req.Header.Set("Authorization", "Bearer "+token) + } +} + +func (t *Transport) applyConfiguredAuthentication(req *http.Request) { + if t.authForURL == nil { + return + } + name, value := t.authForURL(req.URL.String()) + if name != "" && value != "" && req.Header.Get(name) == "" { + req.Header.Set(name, value) + } +} + +func (t *Transport) token(ctx context.Context, challenge bearerChallenge) (string, error) { + key := challenge.key() + if token := t.cachedToken(key); token != "" { + return token, nil + } + + token, expiresAt, err := t.fetchToken(ctx, challenge) + if err != nil { + return "", err + } + + t.mu.Lock() + t.tokens[key] = cachedToken{value: token, expiresAt: expiresAt} + t.mu.Unlock() + return token, nil +} + +func (t *Transport) fetchToken(ctx context.Context, challenge bearerChallenge) (string, time.Time, error) { + tokenURL, err := url.Parse(challenge.realm) + if err != nil || !tokenURL.IsAbs() || (tokenURL.Scheme != "https" && tokenURL.Scheme != "http") { + return "", time.Time{}, fmt.Errorf("invalid token realm %q", challenge.realm) + } + + query := tokenURL.Query() + if challenge.service != "" { + query.Set("service", challenge.service) + } + for _, scope := range challenge.scopes { + query.Add("scope", scope) + } + query.Set("client_id", "git-pkgs-proxy") + tokenURL.RawQuery = query.Encode() + + req, err := http.NewRequestWithContext(ctx, http.MethodGet, tokenURL.String(), nil) + if err != nil { + return "", time.Time{}, err + } + + client := &http.Client{Transport: configuredTransport{parent: t}} + resp, err := client.Do(req) + if err != nil { + return "", time.Time{}, fmt.Errorf("requesting token: %w", err) + } + defer func() { _ = resp.Body.Close() }() + + if resp.StatusCode < http.StatusOK || resp.StatusCode >= http.StatusMultipleChoices { + body, _ := io.ReadAll(io.LimitReader(resp.Body, maxTokenResponseSize)) + return "", time.Time{}, fmt.Errorf("token service returned %d: %s", resp.StatusCode, strings.TrimSpace(string(body))) + } + + var payload tokenResponse + if err := json.NewDecoder(io.LimitReader(resp.Body, maxTokenResponseSize)).Decode(&payload); err != nil { + return "", time.Time{}, fmt.Errorf("decoding token response: %w", err) + } + token := payload.Token + if token == "" { + token = payload.AccessToken + } + if token == "" { + return "", time.Time{}, fmt.Errorf("token response did not contain a token") + } + + issuedAt := time.Now() + if payload.IssuedAt != "" { + if parsed, parseErr := time.Parse(time.RFC3339, payload.IssuedAt); parseErr == nil { + issuedAt = parsed + } + } + lifetime := time.Duration(payload.ExpiresIn) * time.Second + if lifetime <= 0 { + lifetime = defaultTokenLifetime + } + expiresAt := issuedAt.Add(lifetime).Add(-expirySkew(lifetime)) + return token, expiresAt, nil +} + +type configuredTransport struct { + parent *Transport +} + +func (t configuredTransport) RoundTrip(req *http.Request) (*http.Response, error) { + outbound := cloneRequest(req) + t.parent.applyConfiguredAuthentication(outbound) + return t.parent.base.RoundTrip(outbound) +} + +func (t *Transport) cachedTokenForRequest(requestURL *url.URL) string { + space := registryProtectionSpace(requestURL) + if space == "" { + return "" + } + + t.mu.Lock() + challenge, ok := t.challenges[space] + t.mu.Unlock() + if !ok { + return "" + } + return t.cachedToken(challenge.key()) +} + +func (t *Transport) cachedToken(key string) string { + now := time.Now() + t.mu.Lock() + defer t.mu.Unlock() + + token, ok := t.tokens[key] + if !ok { + return "" + } + if !now.Before(token.expiresAt) { + delete(t.tokens, key) + return "" + } + return token.value +} + +func (t *Transport) rememberChallenge(requestURL *url.URL, challenge bearerChallenge) { + space := registryProtectionSpace(requestURL) + if space == "" { + return + } + t.mu.Lock() + t.challenges[space] = challenge + t.mu.Unlock() +} + +func (c bearerChallenge) key() string { + return c.realm + "\x00" + c.service + "\x00" + strings.Join(c.scopes, "\x00") +} + +func registryProtectionSpace(u *url.URL) string { + const registryPrefix = "/v2/" + if u == nil || !strings.HasPrefix(u.Path, registryPrefix) { + return "" + } + rest := strings.TrimPrefix(u.Path, registryPrefix) + end := len(rest) + for _, marker := range []string{"/blobs/", "/manifests/", "/tags/", "/referrers/"} { + if index := strings.Index(rest, marker); index >= 0 && index < end { + end = index + } + } + if end == len(rest) || end == 0 { + return "" + } + return u.Scheme + "://" + u.Host + registryPrefix + rest[:end] +} + +func parseBearerChallenge(values []string) (bearerChallenge, bool) { + for _, value := range values { + params, ok := bearerParameters(value) + if !ok || params["realm"] == "" { + continue + } + challenge := bearerChallenge{ + realm: params["realm"], + service: params["service"], + } + if scope := params["scope"]; scope != "" { + challenge.scopes = append(challenge.scopes, scope) + } + return challenge, true + } + return bearerChallenge{}, false +} + +func bearerParameters(value string) (map[string]string, bool) { + start := findAuthScheme(value, "Bearer") + if start < 0 { + return nil, false + } + rest := value[start+len("Bearer"):] + params := make(map[string]string) + for { + rest = strings.TrimLeft(rest, " \t,") + if rest == "" { + break + } + + keyEnd := strings.IndexAny(rest, "= \t,") + if keyEnd <= 0 { + break + } + key := strings.ToLower(rest[:keyEnd]) + rest = strings.TrimLeft(rest[keyEnd:], " \t") + if rest == "" || rest[0] != '=' { + break + } + rest = strings.TrimLeft(rest[1:], " \t") + + parsed, remaining, ok := parseAuthValue(rest) + if !ok { + return nil, false + } + params[key] = parsed + rest = remaining + } + return params, true +} + +func findAuthScheme(value, scheme string) int { + inQuote := false + escaped := false + for index := 0; index+len(scheme) <= len(value); index++ { + char := value[index] + if escaped { + escaped = false + continue + } + if char == '\\' && inQuote { + escaped = true + continue + } + if char == '"' { + inQuote = !inQuote + continue + } + if inQuote || !strings.EqualFold(value[index:index+len(scheme)], scheme) { + continue + } + beforeOK := index == 0 || value[index-1] == ',' || value[index-1] == ' ' || value[index-1] == '\t' + after := index + len(scheme) + afterOK := after < len(value) && (value[after] == ' ' || value[after] == '\t') + if beforeOK && afterOK { + return index + } + } + return -1 +} + +func parseAuthValue(value string) (parsed, remaining string, ok bool) { + if value == "" { + return "", "", false + } + if value[0] != '"' { + end := strings.IndexAny(value, " \t,") + if end < 0 { + return value, "", true + } + return value[:end], value[end:], end > 0 + } + + var builder strings.Builder + escaped := false + for index := 1; index < len(value); index++ { + char := value[index] + if escaped { + builder.WriteByte(char) + escaped = false + continue + } + if char == '\\' { + escaped = true + continue + } + if char == '"' { + return builder.String(), value[index+1:], true + } + builder.WriteByte(char) + } + return "", "", false +} + +func cloneRequest(req *http.Request) *http.Request { + clone := req.Clone(req.Context()) + clone.Header = req.Header.Clone() + return clone +} + +func canReplay(req *http.Request) bool { + return req.Body == nil || req.GetBody != nil +} + +func cloneRequestForRetry(req *http.Request) (*http.Request, error) { + clone := cloneRequest(req) + if req.Body == nil { + return clone, nil + } + body, err := req.GetBody() + if err != nil { + return nil, fmt.Errorf("replaying authenticated request: %w", err) + } + clone.Body = body + return clone, nil +} + +func expirySkew(lifetime time.Duration) time.Duration { + if lifetime < tokenExpirySkew*2 { + return lifetime / shortTokenSkewDivisor + } + return tokenExpirySkew +} + +func drainAndClose(body io.ReadCloser) { + _, _ = io.Copy(io.Discard, io.LimitReader(body, maxTokenResponseSize)) + _ = body.Close() +} diff --git a/internal/httpclient/transport_test.go b/internal/httpclient/transport_test.go new file mode 100644 index 0000000..027a378 --- /dev/null +++ b/internal/httpclient/transport_test.go @@ -0,0 +1,151 @@ +package httpclient + +import ( + "io" + "net/http" + "net/http/httptest" + "strings" + "testing" +) + +func TestTransportFollowsBearerChallengeAndCachesToken(t *testing.T) { + var registryRequests int + var tokenRequests int + var server *httptest.Server + + server = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/token": + tokenRequests++ + if got := r.URL.Query().Get("service"); got != "registry.test" { + t.Errorf("service = %q, want %q", got, "registry.test") + } + if got := r.URL.Query().Get("scope"); got != "repository:library/test:pull" { + t.Errorf("scope = %q, want %q", got, "repository:library/test:pull") + } + w.Header().Set("Content-Type", "application/json") + _, _ = io.WriteString(w, `{"token":"registry-token","expires_in":3600}`) + case "/v2/library/test/blobs/sha256:first", "/v2/library/test/blobs/sha256:second": + registryRequests++ + if r.Header.Get("Authorization") != "Bearer registry-token" { + w.Header().Set("WWW-Authenticate", `Bearer realm="`+server.URL+`/token",service="registry.test",scope="repository:library/test:pull"`) + http.Error(w, "authentication required", http.StatusUnauthorized) + return + } + _, _ = io.WriteString(w, "blob") + default: + http.NotFound(w, r) + } + })) + defer server.Close() + + client := &http.Client{Transport: NewTransport(http.DefaultTransport, nil)} + for _, digest := range []string{"sha256:first", "sha256:second"} { + resp, err := client.Get(server.URL + "/v2/library/test/blobs/" + digest) + if err != nil { + t.Fatalf("GET %s: %v", digest, err) + } + body, readErr := io.ReadAll(resp.Body) + _ = resp.Body.Close() + if readErr != nil { + t.Fatalf("read %s response: %v", digest, readErr) + } + if resp.StatusCode != http.StatusOK { + t.Fatalf("GET %s status = %d, want %d", digest, resp.StatusCode, http.StatusOK) + } + if string(body) != "blob" { + t.Errorf("GET %s body = %q, want %q", digest, body, "blob") + } + } + + if tokenRequests != 1 { + t.Errorf("token requests = %d, want 1", tokenRequests) + } + if registryRequests != 3 { + t.Errorf("registry requests = %d, want 3", registryRequests) + } +} + +func TestTransportAddsConfiguredAuthentication(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if got := r.Header.Get("X-Registry-Token"); got != "configured-token" { + t.Errorf("X-Registry-Token = %q, want %q", got, "configured-token") + } + w.WriteHeader(http.StatusNoContent) + })) + defer server.Close() + + authForURL := func(url string) (string, string) { + if strings.HasPrefix(url, server.URL) { + return "X-Registry-Token", "configured-token" + } + return "", "" + } + client := &http.Client{Transport: NewTransport(http.DefaultTransport, authForURL)} + + resp, err := client.Get(server.URL + "/metadata") + if err != nil { + t.Fatalf("GET metadata: %v", err) + } + _ = resp.Body.Close() + if resp.StatusCode != http.StatusNoContent { + t.Errorf("status = %d, want %d", resp.StatusCode, http.StatusNoContent) + } +} + +func TestTransportPreservesExplicitAuthentication(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if got := r.Header.Get("Authorization"); got != "Bearer explicit-token" { + t.Errorf("Authorization = %q, want %q", got, "Bearer explicit-token") + } + w.WriteHeader(http.StatusNoContent) + })) + defer server.Close() + + authForURL := func(string) (string, string) { + return "Authorization", "Bearer configured-token" + } + client := &http.Client{Transport: NewTransport(http.DefaultTransport, authForURL)} + req, err := http.NewRequest(http.MethodGet, server.URL+"/artifact", nil) + if err != nil { + t.Fatal(err) + } + req.Header.Set("Authorization", "Bearer explicit-token") + + resp, err := client.Do(req) + if err != nil { + t.Fatalf("GET artifact: %v", err) + } + _ = resp.Body.Close() + if resp.StatusCode != http.StatusNoContent { + t.Errorf("status = %d, want %d", resp.StatusCode, http.StatusNoContent) + } +} + +func TestTransportDoesNotFollowBearerChallengeOutsideOCIRegistry(t *testing.T) { + tokenRequests := 0 + var server *httptest.Server + server = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path == "/token" { + tokenRequests++ + _, _ = io.WriteString(w, `{"token":"unexpected"}`) + return + } + w.Header().Set("WWW-Authenticate", `Bearer realm="`+server.URL+`/token"`) + http.Error(w, "authentication required", http.StatusUnauthorized) + })) + defer server.Close() + + client := &http.Client{Transport: NewTransport(http.DefaultTransport, nil)} + resp, err := client.Get(server.URL + "/api/packages") + if err != nil { + t.Fatalf("GET API: %v", err) + } + _ = resp.Body.Close() + if resp.StatusCode != http.StatusUnauthorized { + t.Errorf("status = %d, want %d", resp.StatusCode, http.StatusUnauthorized) + } + if tokenRequests != 0 { + t.Errorf("token requests = %d, want 0", tokenRequests) + } +} diff --git a/internal/server/server.go b/internal/server/server.go index 5aac4db..b763278 100644 --- a/internal/server/server.go +++ b/internal/server/server.go @@ -58,17 +58,19 @@ import ( "strings" "time" + "github.com/git-pkgs/cooldown" swaggerdoc "github.com/git-pkgs/proxy/docs/swagger" "github.com/git-pkgs/proxy/internal/config" - "github.com/git-pkgs/cooldown" "github.com/git-pkgs/proxy/internal/database" "github.com/git-pkgs/proxy/internal/enrichment" "github.com/git-pkgs/proxy/internal/handler" + upstreamhttp "github.com/git-pkgs/proxy/internal/httpclient" "github.com/git-pkgs/proxy/internal/metrics" "github.com/git-pkgs/proxy/internal/mirror" "github.com/git-pkgs/proxy/internal/storage" "github.com/git-pkgs/purl" "github.com/git-pkgs/registries/fetch" + "github.com/git-pkgs/registries/safehttp" "github.com/git-pkgs/spdx" "github.com/go-chi/chi/v5" "github.com/go-chi/chi/v5/middleware" @@ -84,12 +86,12 @@ const ( // Server is the main proxy server. type Server struct { - cfg *config.Config - db *database.DB - storage storage.Storage - logger *slog.Logger - http *http.Server - templates *Templates + cfg *config.Config + db *database.DB + storage storage.Storage + logger *slog.Logger + http *http.Server + templates *Templates cancel context.CancelFunc healthCache *healthCache } @@ -156,8 +158,18 @@ func New(cfg *config.Config, logger *slog.Logger) (*Server, error) { // Start starts the HTTP server. func (s *Server) Start() error { - // Create shared components with circuit breaker - baseFetcher := fetch.NewFetcher(fetch.WithAuthFunc(s.authForURL)) + // Use one authentication-aware transport for metadata and artifacts so + // configured credentials and cached OCI challenges apply consistently. + safeClient := safehttp.New(nil, safehttp.Options{}) + authTransport := upstreamhttp.NewTransport(safeClient.Transport, upstreamhttp.AuthFunc(s.authForURL)) + metadataClient := *safeClient + metadataClient.Timeout = s.cfg.ParseHTTPTimeout() + metadataClient.Transport = authTransport + artifactClient := metadataClient + artifactClient.Timeout = serverWriteTimeout + + // Create shared components with circuit breaker. + baseFetcher := fetch.NewFetcher(fetch.WithHTTPClient(&artifactClient)) fetcher := fetch.NewCircuitBreakerFetcher(baseFetcher) resolver := fetch.NewResolver() cd := &cooldown.Config{ @@ -166,7 +178,7 @@ func (s *Server) Start() error { Packages: s.cfg.Cooldown.Packages, } proxy := handler.NewProxy(s.db, s.storage, fetcher, resolver, s.logger) - proxy.HTTPClient.Timeout = s.cfg.ParseHTTPTimeout() + proxy.HTTPClient = &metadataClient proxy.Cooldown = cd proxy.CacheMetadata = s.cfg.CacheMetadata proxy.MetadataTTL = s.cfg.ParseMetadataTTL() From 2717f216a445be29d68b9b5a644eaa46ccdf03aa Mon Sep 17 00:00:00 2001 From: Andrew Nesbitt Date: Mon, 13 Jul 2026 17:18:26 -0700 Subject: [PATCH 2/2] Address upstream authentication review findings --- internal/config/config.go | 20 +++++- internal/config/config_test.go | 29 ++++++++ internal/httpclient/transport.go | 23 ++++-- internal/httpclient/transport_test.go | 100 ++++++++++++++++++++++++++ 4 files changed, 165 insertions(+), 7 deletions(-) diff --git a/internal/config/config.go b/internal/config/config.go index a54d50c..cd43831 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -306,6 +306,16 @@ func (u *UpstreamConfig) AuthForURL(url string) *AuthConfig { return bestMatch } +// Validate checks upstream authentication URL scopes. +func (u *UpstreamConfig) Validate() error { + for pattern := range u.Auth { + if _, err := parseAuthURL(pattern); err != nil { + return fmt.Errorf("invalid upstream.auth URL %q: %w", pattern, err) + } + } + return nil +} + func parseAuthURL(value string) (*url.URL, error) { parsed, err := url.Parse(value) if err != nil || !parsed.IsAbs() || parsed.Hostname() == "" || parsed.Opaque != "" { @@ -625,15 +635,19 @@ func (c *Config) Validate() error { return err } - if err := c.Health.Validate(); err != nil { + return c.validateComponents() +} + +func (c *Config) validateComponents() error { + if err := c.Upstream.Validate(); err != nil { return err } - if err := c.Gradle.BuildCache.Validate(); err != nil { + if err := c.Health.Validate(); err != nil { return err } - return nil + return c.Gradle.BuildCache.Validate() } // Validate checks the /health configuration. An unset interval is allowed diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 63520a2..e615f55 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -3,6 +3,7 @@ package config import ( "os" "path/filepath" + "strings" "testing" "time" ) @@ -802,3 +803,31 @@ func TestUpstreamAuthForURLMatchesURLComponents(t *testing.T) { }) } } + +func TestValidateUpstreamAuthURLs(t *testing.T) { + t.Run("valid absolute URL", func(t *testing.T) { + cfg := Default() + cfg.Upstream.Auth = map[string]AuthConfig{ + "https://registry.example.com/private": {Type: "bearer", Token: "token"}, + } + + if err := cfg.Validate(); err != nil { + t.Fatalf("Validate() error = %v", err) + } + }) + + t.Run("invalid URL", func(t *testing.T) { + cfg := Default() + cfg.Upstream.Auth = map[string]AuthConfig{ + "registry.example.com": {Type: "bearer", Token: "token"}, + } + + err := cfg.Validate() + if err == nil { + t.Fatal("Validate() error = nil, want invalid upstream.auth URL error") + } + if !strings.Contains(err.Error(), "upstream.auth") || !strings.Contains(err.Error(), "registry.example.com") { + t.Errorf("Validate() error = %q, want field and URL", err) + } + }) +} diff --git a/internal/httpclient/transport.go b/internal/httpclient/transport.go index 1cdb457..d96c827 100644 --- a/internal/httpclient/transport.go +++ b/internal/httpclient/transport.go @@ -66,13 +66,17 @@ func NewTransport(base http.RoundTripper, authForURL AuthFunc) *Transport { // RoundTrip implements http.RoundTripper. func (t *Transport) RoundTrip(req *http.Request) (*http.Response, error) { + hasExplicitAuthorization := req.Header.Get("Authorization") != "" outbound := cloneRequest(req) - t.applyAuthentication(outbound, req.Header.Get("Authorization") != "") + t.applyAuthentication(outbound, hasExplicitAuthorization) resp, err := t.base.RoundTrip(outbound) if err != nil || resp.StatusCode != http.StatusUnauthorized { return resp, err } + if hasExplicitAuthorization { + return resp, nil + } if registryProtectionSpace(req.URL) == "" { return resp, nil } @@ -129,12 +133,23 @@ func (t *Transport) token(ctx context.Context, challenge bearerChallenge) (strin return "", err } - t.mu.Lock() - t.tokens[key] = cachedToken{value: token, expiresAt: expiresAt} - t.mu.Unlock() + t.cacheToken(key, cachedToken{value: token, expiresAt: expiresAt}) return token, nil } +func (t *Transport) cacheToken(key string, token cachedToken) { + now := time.Now() + t.mu.Lock() + defer t.mu.Unlock() + + for cachedKey, cached := range t.tokens { + if !now.Before(cached.expiresAt) { + delete(t.tokens, cachedKey) + } + } + t.tokens[key] = token +} + func (t *Transport) fetchToken(ctx context.Context, challenge bearerChallenge) (string, time.Time, error) { tokenURL, err := url.Parse(challenge.realm) if err != nil || !tokenURL.IsAbs() || (tokenURL.Scheme != "https" && tokenURL.Scheme != "http") { diff --git a/internal/httpclient/transport_test.go b/internal/httpclient/transport_test.go index 027a378..bd1f266 100644 --- a/internal/httpclient/transport_test.go +++ b/internal/httpclient/transport_test.go @@ -1,11 +1,13 @@ package httpclient import ( + "context" "io" "net/http" "net/http/httptest" "strings" "testing" + "time" ) func TestTransportFollowsBearerChallengeAndCachesToken(t *testing.T) { @@ -122,6 +124,104 @@ func TestTransportPreservesExplicitAuthentication(t *testing.T) { } } +func TestTransportDoesNotReplaceExplicitAuthenticationAfterBearerChallenge(t *testing.T) { + var registryRequests int + var tokenRequests int + var server *httptest.Server + + server = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/token": + tokenRequests++ + _, _ = io.WriteString(w, `{"token":"registry-token"}`) + case "/v2/library/test/blobs/sha256:test": + registryRequests++ + if got := r.Header.Get("Authorization"); got != "Bearer explicit-token" { + t.Errorf("Authorization = %q, want %q", got, "Bearer explicit-token") + } + w.Header().Set("WWW-Authenticate", `Bearer realm="`+server.URL+`/token"`) + http.Error(w, "authentication required", http.StatusUnauthorized) + default: + http.NotFound(w, r) + } + })) + defer server.Close() + + client := &http.Client{Transport: NewTransport(http.DefaultTransport, nil)} + req, err := http.NewRequest(http.MethodGet, server.URL+"/v2/library/test/blobs/sha256:test", nil) + if err != nil { + t.Fatal(err) + } + req.Header.Set("Authorization", "Bearer explicit-token") + + resp, err := client.Do(req) + if err != nil { + t.Fatalf("GET blob: %v", err) + } + _ = resp.Body.Close() + if resp.StatusCode != http.StatusUnauthorized { + t.Errorf("status = %d, want %d", resp.StatusCode, http.StatusUnauthorized) + } + if registryRequests != 1 { + t.Errorf("registry requests = %d, want 1", registryRequests) + } + if tokenRequests != 0 { + t.Errorf("token requests = %d, want 0", tokenRequests) + } +} + +func TestTransportDoesNotForwardConfiguredAuthenticationOnTokenRedirect(t *testing.T) { + destination := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if got := r.Header.Get("X-Registry-Token"); got != "" { + t.Errorf("redirected X-Registry-Token = %q, want empty", got) + } + _, _ = io.WriteString(w, `{"token":"registry-token"}`) + })) + defer destination.Close() + + source := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if got := r.Header.Get("X-Registry-Token"); got != "configured-token" { + t.Errorf("source X-Registry-Token = %q, want %q", got, "configured-token") + } + http.Redirect(w, r, destination.URL+"/token", http.StatusFound) + })) + defer source.Close() + + authForURL := func(rawURL string) (string, string) { + if strings.HasPrefix(rawURL, source.URL) { + return "X-Registry-Token", "configured-token" + } + return "", "" + } + transport := NewTransport(http.DefaultTransport, authForURL) + token, _, err := transport.fetchToken(context.Background(), bearerChallenge{realm: source.URL + "/token"}) + if err != nil { + t.Fatalf("fetchToken: %v", err) + } + if token != "registry-token" { + t.Errorf("token = %q, want %q", token, "registry-token") + } +} + +func TestTransportPrunesExpiredTokens(t *testing.T) { + transport := NewTransport(http.DefaultTransport, nil) + transport.tokens["expired-unused"] = cachedToken{ + value: "expired-token", + expiresAt: time.Now().Add(-time.Minute), + } + transport.cacheToken("current", cachedToken{ + value: "current-token", + expiresAt: time.Now().Add(time.Minute), + }) + + if got := transport.cachedToken("current"); got != "current-token" { + t.Errorf("cachedToken(current) = %q, want %q", got, "current-token") + } + if _, ok := transport.tokens["expired-unused"]; ok { + t.Error("expired unused token was not pruned") + } +} + func TestTransportDoesNotFollowBearerChallengeOutsideOCIRegistry(t *testing.T) { tokenRequests := 0 var server *httptest.Server