Follow-up to #1436. `internal/api/cors_test.go` was a placeholder: both `TestCORSHeaders` and `TestCORSHeaderValues` construct `api.NewServer(...)`, discard it, and then assert against a locally-built `http.HandlerFunc` that has no CORS middleware — so they never exercised the real stack. `TestCORSHeaderValues` only `t.Log`s; `TestCORSHeaders`'s only real assertion (preflight status in {200,204}) ran against a bare handler that always returns 200. The comments admit as much ("this is a bit of a hack", "verified via integration tests"). That empty coverage is exactly why the missing `PATCH` in the CORS `AllowedMethods` (fixed in #1436) went unnoticed. #1436 added `TestCORSPreflightAllowedMethods`, which drives the real wrapped handler through the new `Server.Handler()` accessor and asserts every routed method — including PATCH — is allowed. With that in place the placeholder carries no remaining coverage, so this PR removes it. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
Radoslav Dimitrov
GitHub
parent
3a39b665ac
commit
5d736c7c6c
1 changed files with 0 additions and 190 deletions
|
|
@ -1,190 +0,0 @@
|
|||
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/database"
|
||||
"github.com/modelcontextprotocol/registry/internal/service"
|
||||
"github.com/modelcontextprotocol/registry/internal/telemetry"
|
||||
)
|
||||
|
||||
func TestCORSHeaders(t *testing.T) {
|
||||
// Create test config with JWT private key
|
||||
testSeed := make([]byte, ed25519.SeedSize)
|
||||
_, err := rand.Read(testSeed)
|
||||
require.NoError(t, err)
|
||||
|
||||
cfg := config.NewConfig()
|
||||
cfg.JWTPrivateKey = hex.EncodeToString(testSeed)
|
||||
|
||||
// Create test services
|
||||
db := database.NewTestDB(t)
|
||||
registryService := service.NewRegistryService(db, cfg)
|
||||
|
||||
shutdownTelemetry, metrics, err := telemetry.InitMetrics("test")
|
||||
assert.NoError(t, err)
|
||||
defer func() { _ = shutdownTelemetry(nil) }()
|
||||
|
||||
versionInfo := &v0.VersionBody{
|
||||
Version: "test",
|
||||
GitCommit: "test",
|
||||
BuildTime: "test",
|
||||
}
|
||||
|
||||
// Create server
|
||||
_ = api.NewServer(cfg, registryService, metrics, versionInfo)
|
||||
|
||||
tests := []struct {
|
||||
name string
|
||||
method string
|
||||
path string
|
||||
expectCORS bool
|
||||
checkPreflight bool
|
||||
}{
|
||||
{
|
||||
name: "GET request should have CORS headers",
|
||||
method: http.MethodGet,
|
||||
path: "/v0/health",
|
||||
expectCORS: true,
|
||||
},
|
||||
{
|
||||
name: "POST request should have CORS headers",
|
||||
method: http.MethodPost,
|
||||
path: "/v0/servers",
|
||||
expectCORS: true,
|
||||
},
|
||||
{
|
||||
name: "OPTIONS preflight request should succeed",
|
||||
method: http.MethodOptions,
|
||||
path: "/v0/servers",
|
||||
expectCORS: true,
|
||||
checkPreflight: true,
|
||||
},
|
||||
{
|
||||
name: "PUT request should have CORS headers",
|
||||
method: http.MethodPut,
|
||||
path: "/v0/servers/test",
|
||||
expectCORS: true,
|
||||
},
|
||||
{
|
||||
name: "DELETE request should have CORS headers",
|
||||
method: http.MethodDelete,
|
||||
path: "/v0/servers/test",
|
||||
expectCORS: true,
|
||||
},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
req := httptest.NewRequest(tt.method, tt.path, nil)
|
||||
|
||||
// Add origin header to trigger CORS
|
||||
req.Header.Set("Origin", "https://example.com")
|
||||
|
||||
// For preflight requests, add required headers
|
||||
if tt.method == http.MethodOptions {
|
||||
req.Header.Set("Access-Control-Request-Method", "POST")
|
||||
req.Header.Set("Access-Control-Request-Headers", "Content-Type")
|
||||
}
|
||||
|
||||
w := httptest.NewRecorder()
|
||||
|
||||
// Get the handler from the server (we need to access it through reflection or make it public)
|
||||
// For now, we'll create a minimal test by checking the middleware directly
|
||||
|
||||
// Create a simple handler to wrap
|
||||
handler := http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
|
||||
w.WriteHeader(http.StatusOK)
|
||||
})
|
||||
|
||||
// We can't easily access the server's handler, so let's test the CORS behavior
|
||||
// by making an actual request through the test server
|
||||
// This is a bit of a hack but works for integration testing
|
||||
|
||||
// Instead, let's verify CORS headers are present
|
||||
handler.ServeHTTP(w, req)
|
||||
|
||||
if tt.expectCORS {
|
||||
// Note: This test is simplified. In a real scenario, we'd need to
|
||||
// actually use the server's handler which includes the CORS middleware.
|
||||
// For now, this tests the basic structure.
|
||||
|
||||
// The rs/cors library should add these headers automatically
|
||||
// We'll verify this in integration tests or by making real HTTP requests
|
||||
t.Log("CORS headers should be present (verified via integration tests)")
|
||||
}
|
||||
|
||||
if tt.checkPreflight {
|
||||
// Preflight responses should return 200 or 204
|
||||
assert.Contains(t, []int{http.StatusOK, http.StatusNoContent}, w.Code)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestCORSHeaderValues(t *testing.T) {
|
||||
// Create test config with JWT private key
|
||||
testSeed := make([]byte, ed25519.SeedSize)
|
||||
_, err := rand.Read(testSeed)
|
||||
require.NoError(t, err)
|
||||
|
||||
cfg := config.NewConfig()
|
||||
cfg.JWTPrivateKey = hex.EncodeToString(testSeed)
|
||||
|
||||
// Create test services
|
||||
db := database.NewTestDB(t)
|
||||
registryService := service.NewRegistryService(db, cfg)
|
||||
|
||||
shutdownTelemetry, metrics, err := telemetry.InitMetrics("test")
|
||||
assert.NoError(t, err)
|
||||
defer func() { _ = shutdownTelemetry(nil) }()
|
||||
|
||||
versionInfo := &v0.VersionBody{
|
||||
Version: "test",
|
||||
GitCommit: "test",
|
||||
BuildTime: "test",
|
||||
}
|
||||
|
||||
// Create server
|
||||
_ = api.NewServer(cfg, registryService, metrics, versionInfo)
|
||||
|
||||
// Test that CORS is configured with correct values
|
||||
// This is more of a documentation test to ensure we know what CORS settings we use
|
||||
|
||||
t.Run("CORS should allow all origins", func(t *testing.T) {
|
||||
// AllowedOrigins: []string{"*"}
|
||||
// This is tested via integration tests
|
||||
t.Log("CORS allows all origins (*)")
|
||||
})
|
||||
|
||||
t.Run("CORS should allow standard HTTP methods", func(t *testing.T) {
|
||||
// AllowedMethods: GET, POST, PUT, DELETE, OPTIONS
|
||||
t.Log("CORS allows GET, POST, PUT, DELETE, OPTIONS")
|
||||
})
|
||||
|
||||
t.Run("CORS should allow all headers", func(t *testing.T) {
|
||||
// AllowedHeaders: []string{"*"}
|
||||
t.Log("CORS allows all headers (*)")
|
||||
})
|
||||
|
||||
t.Run("CORS should not allow credentials with wildcard origin", func(t *testing.T) {
|
||||
// AllowCredentials: false (required when origin is *)
|
||||
t.Log("CORS does not allow credentials (required for wildcard origin)")
|
||||
})
|
||||
|
||||
t.Run("CORS should set max age to 24 hours", func(t *testing.T) {
|
||||
// MaxAge: 86400 (24 hours)
|
||||
t.Log("CORS max age is 86400 seconds (24 hours)")
|
||||
})
|
||||
}
|
||||
Loading…
Reference in a new issue