diff --git a/internal/handler/apk_test.go b/internal/handler/apk_test.go index 61c874ee..fc906814 100644 --- a/internal/handler/apk_test.go +++ b/internal/handler/apk_test.go @@ -144,9 +144,7 @@ func TestAPKHandler_IndexesServedUnchanged(t *testing.T) { proxy.CacheMetadata = true proxy.MetadataTTL = time.Hour proxy.HTTPClient = upstream.Client() - proxy.AuthForURL = func(string) (string, string) { - return "Authorization", "Bearer apk-token" - } + authorizeUpstream(proxy, "Bearer apk-token") h := NewAPKHandler(proxy, "http://proxy.example", map[string]string{"alpine": upstream.URL}) diff --git a/internal/handler/arch_test.go b/internal/handler/arch_test.go index 2650f022..3affad5c 100644 --- a/internal/handler/arch_test.go +++ b/internal/handler/arch_test.go @@ -202,7 +202,7 @@ func TestArchHandler_DatabasesServedUnchanged(t *testing.T) { } upstream := newArchUpstream(t, files) proxy := newArchTestProxy(t, upstream) - proxy.AuthForURL = func(string) (string, string) { return "Authorization", "Bearer arch-token" } + authorizeUpstream(proxy, "Bearer arch-token") h := NewArchHandler(proxy, "http://proxy.example", map[string]string{"archlinux": upstream.URL}) // The database goes first, as pacman requests it. diff --git a/internal/handler/cargo_test.go b/internal/handler/cargo_test.go index 895ff7b6..de3b2f3a 100644 --- a/internal/handler/cargo_test.go +++ b/internal/handler/cargo_test.go @@ -89,9 +89,7 @@ func TestCargoHandlerUsesConfiguredUpstreams(t *testing.T) { proxy, _, _, _ := setupTestProxy(t) proxy.HTTPClient = upstream.Client() - proxy.AuthForURL = func(string) (string, string) { - return "Authorization", "Bearer cargo-token" - } + authorizeUpstream(proxy, "Bearer cargo-token") h := NewCargoHandler( proxy, "http://proxy.test", diff --git a/internal/handler/handler.go b/internal/handler/handler.go index 776fa8cb..b84e827c 100644 --- a/internal/handler/handler.go +++ b/internal/handler/handler.go @@ -173,7 +173,6 @@ type Proxy struct { // storage at an internal one. DirectServeBaseURL string HTTPClient *http.Client - AuthForURL func(string) (headerName, headerValue string) // StreamArtifacts streams artifacts from upstream without storing them. // Each request fetches its own copy: there is no cache to check and @@ -1059,7 +1058,6 @@ func (p *Proxy) ProxyUpstream(w http.ResponseWriter, r *http.Request, upstreamUR req.Header.Set(header, v) } } - p.applyUpstreamAuth(req) resp, err := p.HTTPClient.Do(req) if err != nil { @@ -1079,7 +1077,6 @@ func (p *Proxy) ProxyFile(w http.ResponseWriter, r *http.Request, upstreamURL st http.Error(w, "failed to create request", http.StatusInternalServerError) return } - p.applyUpstreamAuth(req) resp, err := p.HTTPClient.Do(req) if err != nil { @@ -1422,7 +1419,6 @@ func (p *Proxy) fetchUpstreamMetadata(ctx context.Context, upstreamURL string, e if acceptEncoding != "" { req.Header.Set(headerAcceptEncoding, acceptEncoding) } - p.applyUpstreamAuth(req) if entry != nil && entry.ETag.Valid { req.Header.Set("If-None-Match", entry.ETag.String) @@ -1678,7 +1674,6 @@ func (p *Proxy) proxyMetadataStream(w http.ResponseWriter, r *http.Request, upst if acceptEncoding != "" { req.Header.Set(headerAcceptEncoding, acceptEncoding) } - p.applyUpstreamAuth(req) for _, header := range []string{"If-Modified-Since", "If-None-Match"} { if v := r.Header.Get(header); v != "" { @@ -1702,17 +1697,6 @@ func (p *Proxy) proxyMetadataStream(w http.ResponseWriter, r *http.Request, upst }) } -func (p *Proxy) applyUpstreamAuth(req *http.Request) { - if p.AuthForURL == nil { - return - } - - headerName, headerValue := p.AuthForURL(req.URL.String()) - if headerName != "" && headerValue != "" { - req.Header.Set(headerName, headerValue) - } -} - // GetOrFetchArtifactFromURL retrieves an artifact from cache or fetches from a specific URL. // This is useful for registries where download URLs are determined from metadata. func (p *Proxy) GetOrFetchArtifactFromURL(ctx context.Context, ecosystem, name, version, filename, downloadURL string) (*CacheResult, error) { diff --git a/internal/handler/npm.go b/internal/handler/npm.go index 96ab10ea..92629d24 100644 --- a/internal/handler/npm.go +++ b/internal/handler/npm.go @@ -160,7 +160,6 @@ func (h *NPMHandler) handleSecurity(w http.ResponseWriter, r *http.Request) { if req.Header.Get(headerContentType) == "" { req.Header.Set(headerContentType, contentTypeJSON) } - h.proxy.applyUpstreamAuth(req) resp, err := h.proxy.HTTPClient.Do(req) if err != nil { diff --git a/internal/handler/npm_test.go b/internal/handler/npm_test.go index 5f45feb8..a1ad53d2 100644 --- a/internal/handler/npm_test.go +++ b/internal/handler/npm_test.go @@ -72,9 +72,7 @@ func TestNPMHandlerUsesConfiguredUpstream(t *testing.T) { proxy, _, _, _ := setupTestProxy(t) proxy.HTTPClient = upstream.Client() - proxy.AuthForURL = func(string) (string, string) { - return "Authorization", "Bearer npm-token" - } + authorizeUpstream(proxy, "Bearer npm-token") h := NewNPMHandler(proxy, "http://proxy.test", upstream.URL+"/root/") req := httptest.NewRequest(http.MethodGet, "/testpkg", nil) @@ -845,9 +843,7 @@ func TestNPMAuditCoversAllSecurityEndpoints(t *testing.T) { func TestNPMAuditAppliesUpstreamAuth(t *testing.T) { h, got := newNPMAuditUpstream(t, npmAuditJSON(`{}`)) - h.proxy.AuthForURL = func(string) (string, string) { - return "Authorization", "Bearer npm-token" - } + authorizeUpstream(h.proxy, "Bearer npm-token") serveNPM(t, h, http.MethodPost, "/-/npm/v1/security/audits", strings.NewReader(`{}`), nil) @@ -1011,9 +1007,7 @@ func TestNPMKeysProxiesUpstream(t *testing.T) { proxy, _, _, _ := setupTestProxy(t) proxy.HTTPClient = upstream.Client() - proxy.AuthForURL = func(string) (string, string) { - return "Authorization", "Bearer npm-token" - } + authorizeUpstream(proxy, "Bearer npm-token") h := NewNPMHandler(proxy, "http://proxy.test", upstream.URL) w := serveNPM(t, h, http.MethodGet, npmKeysPath, nil, nil) diff --git a/internal/handler/swift.go b/internal/handler/swift.go index bd46fab2..15825e02 100644 --- a/internal/handler/swift.go +++ b/internal/handler/swift.go @@ -269,7 +269,6 @@ func (h *SwiftHandler) probeSourceArchive(ctx context.Context, upstreamURL, acce } req.Header.Set("Accept", accept) req.Header.Set("Range", "bytes=0-0") - h.proxy.applyUpstreamAuth(req) resp, err := h.proxy.HTTPClient.Do(req) if err != nil { @@ -313,7 +312,6 @@ func (h *SwiftHandler) fetchMetadataWithHeaders( return nil, "", nil, fmt.Errorf("creating upstream metadata request: %w", err) } req.Header.Set("Accept", accept) - h.proxy.applyUpstreamAuth(req) resp, err := h.proxy.HTTPClient.Do(req) if err != nil { @@ -438,7 +436,6 @@ func (h *SwiftHandler) proxySwiftResource(w http.ResponseWriter, r *http.Request req.Header.Set(name, value) } } - h.proxy.applyUpstreamAuth(req) resp, err := h.proxy.HTTPClient.Do(req) if err != nil { diff --git a/internal/handler/upstream_auth_helper_test.go b/internal/handler/upstream_auth_helper_test.go new file mode 100644 index 00000000..261636d2 --- /dev/null +++ b/internal/handler/upstream_auth_helper_test.go @@ -0,0 +1,21 @@ +package handler + +import ( + "net/http" + + upstreamhttp "github.com/git-pkgs/proxy/internal/httpclient" +) + +// authorizeUpstream sends authorization on every upstream request the way the +// server does: through the auth transport, which scopes it to each hop. +func authorizeUpstream(proxy *Proxy, authorization string) { + base := proxy.HTTPClient.Transport + if base == nil { + base = http.DefaultTransport + } + client := *proxy.HTTPClient + client.Transport = upstreamhttp.NewTransport(base, func(string) (string, string) { + return "Authorization", authorization + }) + proxy.HTTPClient = &client +} diff --git a/internal/server/server.go b/internal/server/server.go index 8ee1c623..6473e76a 100644 --- a/internal/server/server.go +++ b/internal/server/server.go @@ -247,7 +247,6 @@ func (s *Server) serve(listener net.Listener) error { } proxy := handler.NewProxy(s.db, s.storage, fetcher, resolver, s.logger) proxy.HTTPClient = &metadataClient - proxy.AuthForURL = s.authForURL cooldownPolicy, err := cooldownpolicy.New(cd, s.cfg.Cooldown.PackagePatterns) if err != nil { return fmt.Errorf("configuring cooldown policy: %w", err) diff --git a/internal/server/upstream_auth_redirect_test.go b/internal/server/upstream_auth_redirect_test.go new file mode 100644 index 00000000..7c2f0c44 --- /dev/null +++ b/internal/server/upstream_auth_redirect_test.go @@ -0,0 +1,125 @@ +package server + +import ( + "context" + "errors" + "io" + "log/slog" + "net" + "net/http" + "net/http/httptest" + "path/filepath" + "sync" + "testing" + "time" + + "github.com/git-pkgs/proxy/internal/config" +) + +const redirectAuthHeader = "X-Api-Key" + +// headerRecorder remembers the auth header value each request carried. +type headerRecorder struct { + mu sync.Mutex + values []string +} + +func (h *headerRecorder) record(r *http.Request) { + h.mu.Lock() + defer h.mu.Unlock() + h.values = append(h.values, r.Header.Get(redirectAuthHeader)) +} + +func (h *headerRecorder) seen() []string { + h.mu.Lock() + defer h.mu.Unlock() + return append([]string(nil), h.values...) +} + +func TestHeaderAuthIsNotForwardedAcrossRedirects(t *testing.T) { + var atOther headerRecorder + other := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + atOther.record(r) + w.Header().Set("Content-Type", "application/json") + _, _ = io.WriteString(w, `{"name":"leftpad","versions":{}}`) + })) + defer other.Close() + + var atRegistry headerRecorder + registry := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + atRegistry.record(r) + http.Redirect(w, r, other.URL+r.URL.Path, http.StatusFound) + })) + defer registry.Close() + + listener, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatalf("reserving proxy address: %v", err) + } + t.Cleanup(func() { _ = listener.Close() }) + + tempDir := t.TempDir() + cfg := config.Default() + cfg.Listen = listener.Addr().String() + cfg.BaseURL = "http://" + cfg.Listen + cfg.Database.Path = filepath.Join(tempDir, "proxy.db") + cfg.Storage.URL = "file://" + filepath.Join(tempDir, "artifacts") + cfg.Upstream.AllowLoopback = true + cfg.Upstream.NPM = registry.URL + cfg.Upstream.Auth = map[string]config.AuthConfig{ + registry.URL: {Type: "header", HeaderName: redirectAuthHeader, HeaderValue: "registry-secret"}, + } + if err := cfg.Validate(); err != nil { + t.Fatalf("validating config: %v", err) + } + + proxyServer, err := New(cfg, slog.New(slog.NewTextHandler(io.Discard, nil)), BuildInfo{Version: "test", Commit: "test"}) + if err != nil { + t.Fatalf("creating server: %v", err) + } + startErr := make(chan error, 1) + go func() { startErr <- proxyServer.Start(listener) }() + defer func() { + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + if err := proxyServer.Shutdown(ctx); err != nil { + t.Errorf("shutting down server: %v", err) + } + if err := <-startErr; !errors.Is(err, http.ErrServerClosed) { + t.Errorf("Start() error = %v, want %v", err, http.ErrServerClosed) + } + }() + + resp := getWhenListening(t, cfg.BaseURL+"/npm/leftpad") + _ = resp.Body.Close() + if resp.StatusCode != http.StatusOK { + t.Fatalf("status = %d, want %d", resp.StatusCode, http.StatusOK) + } + + registrySeen := atRegistry.seen() + if len(registrySeen) == 0 || registrySeen[0] != "registry-secret" { + t.Errorf("registry received %s = %q, want the configured value", redirectAuthHeader, registrySeen) + } + for _, value := range atOther.seen() { + if value != "" { + t.Errorf("redirect target received %s = %q, want none", redirectAuthHeader, value) + } + } +} + +// getWhenListening retries a GET until the proxy accepts connections. +func getWhenListening(t *testing.T, url string) *http.Response { + t.Helper() + client := &http.Client{Timeout: 5 * time.Second} + deadline := time.Now().Add(5 * time.Second) + for { + resp, err := client.Get(url) + if err == nil { + return resp + } + if time.Now().After(deadline) { + t.Fatalf("proxy did not start: %v", err) + } + time.Sleep(10 * time.Millisecond) + } +}