Merge pull request #127 from bnema/fix/admin-path-collision
fix: remove /admin/ from proxy mux to stop intercepting app routes
2 files · +2 -97
M internal/app/run.go +2 -50
| @@ -1464,7 +1464,7 @@ func createHTTPHandlers(svc *services, cfg Config, log zerowrap.Logger) (http.Ha | |||
| 1464 | 1464 | proxyMux := http.NewServeMux() | |
| 1465 | 1465 | proxyMux.Handle("/", proxyWithMiddleware) | |
| 1466 | 1466 | ||
| 1467 | - | registerAdminRoutes(registryMux, proxyMux, svc, cfg, trustedNets, log) | |
| 1467 | + | registerAdminRoutes(registryMux, svc, cfg, trustedNets, log) | |
| 1468 | 1468 | ||
| 1469 | 1469 | return registryMux, proxyMux | |
| 1470 | 1470 | } | |
| @@ -1607,7 +1607,7 @@ func wrapRegistryForLocalMode(registryWithMiddleware http.Handler, cfg Config, l | |||
| 1607 | 1607 | return registryWithMiddleware | |
| 1608 | 1608 | } | |
| 1609 | 1609 | ||
| 1610 | - | func registerAdminRoutes(registryMux, proxyMux *http.ServeMux, svc *services, cfg Config, trustedNets []*net.IPNet, log zerowrap.Logger) { | |
| 1610 | + | func registerAdminRoutes(registryMux *http.ServeMux, svc *services, cfg Config, trustedNets []*net.IPNet, log zerowrap.Logger) { | |
| 1611 | 1611 | if svc.adminHandler == nil { | |
| 1612 | 1612 | return | |
| 1613 | 1613 | } | |
| @@ -1646,54 +1646,6 @@ func registerAdminRoutes(registryMux, proxyMux *http.ServeMux, svc *services, cf | |||
| 1646 | 1646 | "gordon.admin", | |
| 1647 | 1647 | ) | |
| 1648 | 1648 | registryMux.Handle("/admin/", loopbackOnly(adminWithMiddleware, log)) | |
| 1649 | - | proxyMux.Handle("/admin/", adminHostOnly(adminWithMiddleware, cfg.Server.RegistryDomain, log)) | |
| 1650 | - | } | |
| 1651 | - | ||
| 1652 | - | func adminHostOnly(next http.Handler, allowedHost string, log zerowrap.Logger) http.Handler { | |
| 1653 | - | canonicalAllowedHost := canonicalHost(allowedHost) | |
| 1654 | - | ||
| 1655 | - | return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | |
| 1656 | - | host := canonicalHost(r.Host) | |
| 1657 | - | if host == "" { | |
| 1658 | - | host = canonicalHost(r.URL.Host) | |
| 1659 | - | } | |
| 1660 | - | ||
| 1661 | - | if !isLoopbackHost(host) && (canonicalAllowedHost == "" || !strings.EqualFold(host, canonicalAllowedHost)) { | |
| 1662 | - | log.Warn(). | |
| 1663 | - | Str("path", r.URL.Path). | |
| 1664 | - | Str("host", r.Host). | |
| 1665 | - | Str("allowed_host", canonicalAllowedHost). | |
| 1666 | - | Msg("blocked admin request on unauthorized host") | |
| 1667 | - | http.NotFound(w, r) | |
| 1668 | - | return | |
| 1669 | - | } | |
| 1670 | - | ||
| 1671 | - | next.ServeHTTP(w, r) | |
| 1672 | - | }) | |
| 1673 | - | } | |
| 1674 | - | ||
| 1675 | - | func canonicalHost(rawHost string) string { | |
| 1676 | - | host := strings.TrimSpace(rawHost) | |
| 1677 | - | if host == "" { | |
| 1678 | - | return "" | |
| 1679 | - | } | |
| 1680 | - | ||
| 1681 | - | if splitHost, _, err := net.SplitHostPort(host); err == nil { | |
| 1682 | - | host = splitHost | |
| 1683 | - | } | |
| 1684 | - | ||
| 1685 | - | host = strings.TrimPrefix(host, "[") | |
| 1686 | - | host = strings.TrimSuffix(host, "]") | |
| 1687 | - | return strings.ToLower(host) | |
| 1688 | - | } | |
| 1689 | - | ||
| 1690 | - | func isLoopbackHost(host string) bool { | |
| 1691 | - | if host == "localhost" { | |
| 1692 | - | return true | |
| 1693 | - | } | |
| 1694 | - | ||
| 1695 | - | ip := net.ParseIP(host) | |
| 1696 | - | return ip != nil && ip.IsLoopback() | |
| 1697 | 1649 | } | |
| 1698 | 1650 | ||
| 1699 | 1651 | // runServers starts the HTTP servers and waits for shutdown. | |
| @@ -1464,7 +1464,7 @@ func createHTTPHandlers(svc *services, cfg Config, log zerowrap.Logger) (http.Ha | |||||
| 1464 | proxyMux := http.NewServeMux() | 1464 | proxyMux := http.NewServeMux() | ||
| 1465 | proxyMux.Handle("/", proxyWithMiddleware) | 1465 | proxyMux.Handle("/", proxyWithMiddleware) | ||
| 1466 | 1466 | ||||
| 1467 | - | registerAdminRoutes(registryMux, proxyMux, svc, cfg, trustedNets, log) | 1467 | + | registerAdminRoutes(registryMux, svc, cfg, trustedNets, log) |
| 1468 | 1468 | ||||
| 1469 | return registryMux, proxyMux | 1469 | return registryMux, proxyMux | ||
| 1470 | } | 1470 | } | ||
| @@ -1607,7 +1607,7 @@ func wrapRegistryForLocalMode(registryWithMiddleware http.Handler, cfg Config, l | |||||
| 1607 | return registryWithMiddleware | 1607 | return registryWithMiddleware | ||
| 1608 | } | 1608 | } | ||
| 1609 | 1609 | ||||
| 1610 | - | func registerAdminRoutes(registryMux, proxyMux *http.ServeMux, svc *services, cfg Config, trustedNets []*net.IPNet, log zerowrap.Logger) { | 1610 | + | func registerAdminRoutes(registryMux *http.ServeMux, svc *services, cfg Config, trustedNets []*net.IPNet, log zerowrap.Logger) { |
| 1611 | if svc.adminHandler == nil { | 1611 | if svc.adminHandler == nil { | ||
| 1612 | return | 1612 | return | ||
| 1613 | } | 1613 | } | ||
| @@ -1646,54 +1646,6 @@ func registerAdminRoutes(registryMux, proxyMux *http.ServeMux, svc *services, cf | |||||
| 1646 | "gordon.admin", | 1646 | "gordon.admin", | ||
| 1647 | ) | 1647 | ) | ||
| 1648 | registryMux.Handle("/admin/", loopbackOnly(adminWithMiddleware, log)) | 1648 | registryMux.Handle("/admin/", loopbackOnly(adminWithMiddleware, log)) | ||
| 1649 | - | proxyMux.Handle("/admin/", adminHostOnly(adminWithMiddleware, cfg.Server.RegistryDomain, log)) | |||
| 1650 | - | } | |||
| 1651 | - | ||||
| 1652 | - | func adminHostOnly(next http.Handler, allowedHost string, log zerowrap.Logger) http.Handler { | |||
| 1653 | - | canonicalAllowedHost := canonicalHost(allowedHost) | |||
| 1654 | - | ||||
| 1655 | - | return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | |||
| 1656 | - | host := canonicalHost(r.Host) | |||
| 1657 | - | if host == "" { | |||
| 1658 | - | host = canonicalHost(r.URL.Host) | |||
| 1659 | - | } | |||
| 1660 | - | ||||
| 1661 | - | if !isLoopbackHost(host) && (canonicalAllowedHost == "" || !strings.EqualFold(host, canonicalAllowedHost)) { | |||
| 1662 | - | log.Warn(). | |||
| 1663 | - | Str("path", r.URL.Path). | |||
| 1664 | - | Str("host", r.Host). | |||
| 1665 | - | Str("allowed_host", canonicalAllowedHost). | |||
| 1666 | - | Msg("blocked admin request on unauthorized host") | |||
| 1667 | - | http.NotFound(w, r) | |||
| 1668 | - | return | |||
| 1669 | - | } | |||
| 1670 | - | ||||
| 1671 | - | next.ServeHTTP(w, r) | |||
| 1672 | - | }) | |||
| 1673 | - | } | |||
| 1674 | - | ||||
| 1675 | - | func canonicalHost(rawHost string) string { | |||
| 1676 | - | host := strings.TrimSpace(rawHost) | |||
| 1677 | - | if host == "" { | |||
| 1678 | - | return "" | |||
| 1679 | - | } | |||
| 1680 | - | ||||
| 1681 | - | if splitHost, _, err := net.SplitHostPort(host); err == nil { | |||
| 1682 | - | host = splitHost | |||
| 1683 | - | } | |||
| 1684 | - | ||||
| 1685 | - | host = strings.TrimPrefix(host, "[") | |||
| 1686 | - | host = strings.TrimSuffix(host, "]") | |||
| 1687 | - | return strings.ToLower(host) | |||
| 1688 | - | } | |||
| 1689 | - | ||||
| 1690 | - | func isLoopbackHost(host string) bool { | |||
| 1691 | - | if host == "localhost" { | |||
| 1692 | - | return true | |||
| 1693 | - | } | |||
| 1694 | - | ||||
| 1695 | - | ip := net.ParseIP(host) | |||
| 1696 | - | return ip != nil && ip.IsLoopback() | |||
| 1697 | } | 1649 | } | ||
| 1698 | 1650 | ||||
| 1699 | // runServers starts the HTTP servers and waits for shutdown. | 1651 | // runServers starts the HTTP servers and waits for shutdown. | ||
M internal/app/run_http_handlers_test.go +0 -47
| @@ -67,53 +67,6 @@ func TestCreateHTTPHandlers_LocalMode_RestrictsRegistryToLoopback(t *testing.T) | |||
| 67 | 67 | } | |
| 68 | 68 | } | |
| 69 | 69 | ||
| 70 | - | func TestCreateHTTPHandlers_AuthEnabled_ExposesAdminOnProxyForRegistryHost(t *testing.T) { | |
| 71 | - | t.Parallel() | |
| 72 | - | ||
| 73 | - | cfg := Config{} | |
| 74 | - | cfg.Auth.Enabled = true | |
| 75 | - | cfg.Server.RegistryDomain = "gordon.local" | |
| 76 | - | ||
| 77 | - | svc := &services{adminHandler: &adminhttp.Handler{}} | |
| 78 | - | _, proxyHandler := createHTTPHandlers(svc, cfg, zerowrap.Default()) | |
| 79 | - | // services uses an adminHandler stub with authSvc left nil. In createHTTPHandlers, | |
| 80 | - | // registerAdminRoutes installs fail-closed middleware for that case, so admin | |
| 81 | - | // requests on the allowed host return 503 ServiceUnavailable. | |
| 82 | - | ||
| 83 | - | req := httptest.NewRequest(http.MethodGet, "/admin/status", nil) | |
| 84 | - | req.Host = "gordon.local" | |
| 85 | - | req.RemoteAddr = "192.0.2.30:12345" | |
| 86 | - | ||
| 87 | - | rec := httptest.NewRecorder() | |
| 88 | - | proxyHandler.ServeHTTP(rec, req) | |
| 89 | - | ||
| 90 | - | if rec.Code != http.StatusServiceUnavailable { | |
| 91 | - | t.Fatalf("expected status %d, got %d", http.StatusServiceUnavailable, rec.Code) | |
| 92 | - | } | |
| 93 | - | } | |
| 94 | - | ||
| 95 | - | func TestCreateHTTPHandlers_AuthEnabled_BlocksAdminOnNonRegistryHost(t *testing.T) { | |
| 96 | - | t.Parallel() | |
| 97 | - | ||
| 98 | - | cfg := Config{} | |
| 99 | - | cfg.Auth.Enabled = true | |
| 100 | - | cfg.Server.RegistryDomain = "gordon.local" | |
| 101 | - | ||
| 102 | - | svc := &services{adminHandler: &adminhttp.Handler{}} | |
| 103 | - | _, proxyHandler := createHTTPHandlers(svc, cfg, zerowrap.Default()) | |
| 104 | - | ||
| 105 | - | req := httptest.NewRequest(http.MethodGet, "/admin/status", nil) | |
| 106 | - | req.Host = "app.example.com" | |
| 107 | - | req.RemoteAddr = "192.0.2.30:12345" | |
| 108 | - | ||
| 109 | - | rec := httptest.NewRecorder() | |
| 110 | - | proxyHandler.ServeHTTP(rec, req) | |
| 111 | - | ||
| 112 | - | if rec.Code != http.StatusNotFound { | |
| 113 | - | t.Fatalf("expected status %d, got %d", http.StatusNotFound, rec.Code) | |
| 114 | - | } | |
| 115 | - | } | |
| 116 | - | ||
| 117 | 70 | func TestBuildRegistryCIDRAllowlistMiddleware_InvalidEntries_DenyAll(t *testing.T) { | |
| 118 | 71 | t.Parallel() | |
| 119 | 72 | ||
| @@ -67,53 +67,6 @@ func TestCreateHTTPHandlers_LocalMode_RestrictsRegistryToLoopback(t *testing.T) | |||||
| 67 | } | 67 | } | ||
| 68 | } | 68 | } | ||
| 69 | 69 | ||||
| 70 | - | func TestCreateHTTPHandlers_AuthEnabled_ExposesAdminOnProxyForRegistryHost(t *testing.T) { | |||
| 71 | - | t.Parallel() | |||
| 72 | - | ||||
| 73 | - | cfg := Config{} | |||
| 74 | - | cfg.Auth.Enabled = true | |||
| 75 | - | cfg.Server.RegistryDomain = "gordon.local" | |||
| 76 | - | ||||
| 77 | - | svc := &services{adminHandler: &adminhttp.Handler{}} | |||
| 78 | - | _, proxyHandler := createHTTPHandlers(svc, cfg, zerowrap.Default()) | |||
| 79 | - | // services uses an adminHandler stub with authSvc left nil. In createHTTPHandlers, | |||
| 80 | - | // registerAdminRoutes installs fail-closed middleware for that case, so admin | |||
| 81 | - | // requests on the allowed host return 503 ServiceUnavailable. | |||
| 82 | - | ||||
| 83 | - | req := httptest.NewRequest(http.MethodGet, "/admin/status", nil) | |||
| 84 | - | req.Host = "gordon.local" | |||
| 85 | - | req.RemoteAddr = "192.0.2.30:12345" | |||
| 86 | - | ||||
| 87 | - | rec := httptest.NewRecorder() | |||
| 88 | - | proxyHandler.ServeHTTP(rec, req) | |||
| 89 | - | ||||
| 90 | - | if rec.Code != http.StatusServiceUnavailable { | |||
| 91 | - | t.Fatalf("expected status %d, got %d", http.StatusServiceUnavailable, rec.Code) | |||
| 92 | - | } | |||
| 93 | - | } | |||
| 94 | - | ||||
| 95 | - | func TestCreateHTTPHandlers_AuthEnabled_BlocksAdminOnNonRegistryHost(t *testing.T) { | |||
| 96 | - | t.Parallel() | |||
| 97 | - | ||||
| 98 | - | cfg := Config{} | |||
| 99 | - | cfg.Auth.Enabled = true | |||
| 100 | - | cfg.Server.RegistryDomain = "gordon.local" | |||
| 101 | - | ||||
| 102 | - | svc := &services{adminHandler: &adminhttp.Handler{}} | |||
| 103 | - | _, proxyHandler := createHTTPHandlers(svc, cfg, zerowrap.Default()) | |||
| 104 | - | ||||
| 105 | - | req := httptest.NewRequest(http.MethodGet, "/admin/status", nil) | |||
| 106 | - | req.Host = "app.example.com" | |||
| 107 | - | req.RemoteAddr = "192.0.2.30:12345" | |||
| 108 | - | ||||
| 109 | - | rec := httptest.NewRecorder() | |||
| 110 | - | proxyHandler.ServeHTTP(rec, req) | |||
| 111 | - | ||||
| 112 | - | if rec.Code != http.StatusNotFound { | |||
| 113 | - | t.Fatalf("expected status %d, got %d", http.StatusNotFound, rec.Code) | |||
| 114 | - | } | |||
| 115 | - | } | |||
| 116 | - | ||||
| 117 | func TestBuildRegistryCIDRAllowlistMiddleware_InvalidEntries_DenyAll(t *testing.T) { | 70 | func TestBuildRegistryCIDRAllowlistMiddleware_InvalidEntries_DenyAll(t *testing.T) { | ||
| 118 | t.Parallel() | 71 | t.Parallel() | ||
| 119 | 72 | ||||