fix(api): allow PATCH in CORS so browsers can call the status endpoints (#1436)
The status endpoints (PATCH /v0/servers/{name}/status and the
per-version one) are served with PATCH, but the CORS middleware's
AllowedMethods lists GET/POST/PUT/DELETE/OPTIONS and never PATCH. So a
cross-origin browser preflight for those endpoints comes back without
the allow headers and the browser blocks the actual request. One-line
fix: add PATCH to the allowed methods.
While here, the existing CORS test turned out to be a placeholder that
never reaches the real wrapped handler (its own comments say so), which
is why this went unnoticed. Added a small Handler() accessor and a
proper preflight test that asserts each routed method including PATCH is
allowed.
This commit is contained in:
parent
c36b521bf1
commit
3a39b665ac
2 changed files with 69 additions and 0 deletions
62
internal/api/cors_preflight_test.go
Normal file
62
internal/api/cors_preflight_test.go
Normal file
|
|
@ -0,0 +1,62 @@
|
||||||
|
package api_test
|
||||||
|
|
||||||
|
import (
|
||||||
|
"crypto/ed25519"
|
||||||
|
"crypto/rand"
|
||||||
|
"encoding/hex"
|
||||||
|
"net/http"
|
||||||
|
"net/http/httptest"
|
||||||
|
"testing"
|
||||||
|
|
||||||
|
"github.com/stretchr/testify/assert"
|
||||||
|
"github.com/stretchr/testify/require"
|
||||||
|
|
||||||
|
"github.com/modelcontextprotocol/registry/internal/api"
|
||||||
|
v0 "github.com/modelcontextprotocol/registry/internal/api/handlers/v0"
|
||||||
|
"github.com/modelcontextprotocol/registry/internal/config"
|
||||||
|
"github.com/modelcontextprotocol/registry/internal/telemetry"
|
||||||
|
)
|
||||||
|
|
||||||
|
// TestCORSPreflightAllowedMethods checks that the CORS layer permits every HTTP
|
||||||
|
// method the API actually routes. The status endpoints
|
||||||
|
// (PATCH /v0/servers/{name}/status and .../versions/{version}/status) are served
|
||||||
|
// with PATCH, so a cross-origin browser preflight for PATCH has to be allowed or
|
||||||
|
// the browser refuses to send the real request. Guards against a routed method
|
||||||
|
// being missing from AllowedMethods.
|
||||||
|
func TestCORSPreflightAllowedMethods(t *testing.T) {
|
||||||
|
seed := make([]byte, ed25519.SeedSize)
|
||||||
|
_, err := rand.Read(seed)
|
||||||
|
require.NoError(t, err)
|
||||||
|
|
||||||
|
cfg := config.NewConfig()
|
||||||
|
cfg.JWTPrivateKey = hex.EncodeToString(seed)
|
||||||
|
|
||||||
|
shutdownTelemetry, metrics, err := telemetry.InitMetrics("test")
|
||||||
|
require.NoError(t, err)
|
||||||
|
defer func() { _ = shutdownTelemetry(nil) }()
|
||||||
|
|
||||||
|
// registryService is nil on purpose: a CORS preflight is answered by the
|
||||||
|
// middleware before any route handler runs, so business logic is never hit.
|
||||||
|
srv := api.NewServer(cfg, nil, metrics, &v0.VersionBody{Version: "test"})
|
||||||
|
|
||||||
|
for _, method := range []string{
|
||||||
|
http.MethodGet,
|
||||||
|
http.MethodPost,
|
||||||
|
http.MethodPut,
|
||||||
|
http.MethodPatch,
|
||||||
|
http.MethodDelete,
|
||||||
|
} {
|
||||||
|
t.Run(method, func(t *testing.T) {
|
||||||
|
req := httptest.NewRequest(http.MethodOptions, "/v0/servers/example/status", nil)
|
||||||
|
req.Header.Set("Origin", "https://example.com")
|
||||||
|
req.Header.Set("Access-Control-Request-Method", method)
|
||||||
|
|
||||||
|
w := httptest.NewRecorder()
|
||||||
|
srv.Handler().ServeHTTP(w, req)
|
||||||
|
|
||||||
|
allow := w.Header().Get("Access-Control-Allow-Methods")
|
||||||
|
assert.Contains(t, allow, method,
|
||||||
|
"CORS preflight for %s should be allowed, got Access-Control-Allow-Methods=%q", method, allow)
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
@ -109,6 +109,7 @@ func NewServer(cfg *config.Config, registryService service.RegistryService, metr
|
||||||
http.MethodGet,
|
http.MethodGet,
|
||||||
http.MethodPost,
|
http.MethodPost,
|
||||||
http.MethodPut,
|
http.MethodPut,
|
||||||
|
http.MethodPatch,
|
||||||
http.MethodDelete,
|
http.MethodDelete,
|
||||||
http.MethodOptions,
|
http.MethodOptions,
|
||||||
},
|
},
|
||||||
|
|
@ -147,6 +148,12 @@ func NewServer(cfg *config.Config, registryService service.RegistryService, metr
|
||||||
return server
|
return server
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Handler returns the fully wrapped HTTP handler (middleware stack plus routes).
|
||||||
|
// Exposed so tests can exercise the middleware, e.g. CORS, without binding a port.
|
||||||
|
func (s *Server) Handler() http.Handler {
|
||||||
|
return s.server.Handler
|
||||||
|
}
|
||||||
|
|
||||||
// Start begins listening for incoming HTTP requests
|
// Start begins listening for incoming HTTP requests
|
||||||
func (s *Server) Start() error {
|
func (s *Server) Start() error {
|
||||||
log.Printf("HTTP server starting on %s", s.config.ServerAddress)
|
log.Printf("HTTP server starting on %s", s.config.ServerAddress)
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue