Send CORS headers only from the image routes (closes #98)
check / check (push) Successful in 4m34s
check / check (push) Successful in 4m34s
The CORS middleware with the `access_control_allow_origin` origin wrapped every route from the router root, so the login and URL generator pages and `/metrics` sent `Access-Control-Allow-Origin` too. It now wraps only `/v1/image/` and `/v1/e/`, which form a `/v1` subrouter so that a browser's preflight `OPTIONS` request still gets its answer; the maintenance mode group moved inside it unchanged. Every other route sends no CORS headers. A new test checks both image routes, a preflight included, and the login and URL generator pages. `README.md` and `config.example.yml` say the setting covers the image routes only. Unverified by test: `/metrics`, whose middleware registers with the process-wide Prometheus registry. Model: opus-5-5
This commit was merged in pull request #162.
This commit is contained in:
@@ -0,0 +1,64 @@
|
||||
package server
|
||||
|
||||
import (
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"testing"
|
||||
)
|
||||
|
||||
// TestCORSOnlyOnImageRoutes verifies that the image routes answer with the
|
||||
// configured access_control_allow_origin, a preflight request included, and
|
||||
// that the login and URL generator pages send no Access-Control-Allow-Origin,
|
||||
// so no other site can read them. /metrics is left out: its middleware
|
||||
// registers with the process-wide Prometheus registry, which only one test
|
||||
// in this package can do.
|
||||
func TestCORSOnlyOnImageRoutes(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
const appOrigin = "https://app.example.com"
|
||||
|
||||
s := newTestServer(t)
|
||||
s.config.AccessControlAllowOrigin = appOrigin
|
||||
s.SetupRoutes()
|
||||
|
||||
requests := []struct {
|
||||
method string
|
||||
path string
|
||||
want string
|
||||
}{
|
||||
{http.MethodGet, unsignedImagePath, appOrigin},
|
||||
{http.MethodHead, unsignedImagePath, appOrigin},
|
||||
{http.MethodOptions, unsignedImagePath, appOrigin},
|
||||
{http.MethodGet, encryptedImagePath, appOrigin},
|
||||
{http.MethodGet, "/", ""},
|
||||
{http.MethodOptions, "/", ""},
|
||||
{http.MethodPost, "/generate", ""},
|
||||
{http.MethodGet, "/logout", ""},
|
||||
}
|
||||
|
||||
for _, tc := range requests {
|
||||
t.Run(tc.method+" "+tc.path, func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
req := httptest.NewRequestWithContext(
|
||||
t.Context(), tc.method, tc.path, nil)
|
||||
req.Header.Set("Origin", appOrigin)
|
||||
|
||||
// An OPTIONS request naming the method it asks about is the
|
||||
// preflight a browser sends before some cross-origin requests.
|
||||
if tc.method == http.MethodOptions {
|
||||
req.Header.Set("Access-Control-Request-Method", http.MethodGet)
|
||||
}
|
||||
|
||||
rec := httptest.NewRecorder()
|
||||
s.ServeHTTP(rec, req)
|
||||
t.Logf("status %d", rec.Code)
|
||||
|
||||
got := rec.Header().Get("Access-Control-Allow-Origin")
|
||||
if got != tc.want {
|
||||
t.Errorf("Access-Control-Allow-Origin = %q, want %q",
|
||||
got, tc.want)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
@@ -13,6 +13,10 @@ import (
|
||||
// unsignedImagePath is an image URL that carries no signature.
|
||||
const unsignedImagePath = "/v1/image/cdn.example.com/cat.jpg/100x100.jpeg"
|
||||
|
||||
// encryptedImagePath is an encrypted image URL whose token cannot be
|
||||
// decrypted.
|
||||
const encryptedImagePath = "/v1/e/token/cat.jpg"
|
||||
|
||||
// TestMaintenanceModeRefusesImageRequests verifies that while maintenance
|
||||
// mode is on, both image routes answer 503 Service Unavailable with a
|
||||
// Retry-After header and the JSON error body the image handlers send.
|
||||
@@ -28,7 +32,7 @@ func TestMaintenanceModeRefusesImageRequests(t *testing.T) {
|
||||
}{
|
||||
{http.MethodGet, unsignedImagePath},
|
||||
{http.MethodHead, unsignedImagePath},
|
||||
{http.MethodGet, "/v1/e/token/cat.jpg"},
|
||||
{http.MethodGet, encryptedImagePath},
|
||||
}
|
||||
|
||||
for _, tc := range requests {
|
||||
@@ -95,7 +99,7 @@ func TestImageRequestsServedWithoutMaintenanceMode(t *testing.T) {
|
||||
}{
|
||||
{http.MethodGet, unsignedImagePath, http.StatusUnauthorized},
|
||||
{http.MethodHead, unsignedImagePath, http.StatusUnauthorized},
|
||||
{http.MethodGet, "/v1/e/token/cat.jpg", http.StatusBadRequest},
|
||||
{http.MethodGet, encryptedImagePath, http.StatusBadRequest},
|
||||
}
|
||||
|
||||
for _, tc := range requests {
|
||||
|
||||
+23
-15
@@ -38,7 +38,6 @@ func (s *Server) SetupRoutes() {
|
||||
s.router.Use(s.mw.Metrics())
|
||||
}
|
||||
|
||||
s.router.Use(s.mw.CORS())
|
||||
s.router.Use(middleware.Timeout(s.config.DownstreamTimeout))
|
||||
|
||||
if s.sentryEnabled {
|
||||
@@ -74,22 +73,31 @@ func (s *Server) SetupRoutes() {
|
||||
|
||||
s.router.Get("/logout", s.h.HandleLogout())
|
||||
|
||||
// Image routes, refused while maintenance mode is on. Only these: the
|
||||
// image's Docker HEALTHCHECK requests the health check, a 503 there
|
||||
// would make the container unhealthy, and upaas marks a deploy failed
|
||||
// when its container is unhealthy.
|
||||
s.router.Group(func(r chi.Router) {
|
||||
r.Use(s.refuseDuringMaintenance)
|
||||
// Image routes, the only ones that send CORS headers, as pages on other
|
||||
// sites read them. They are a subrouter rather than a group: a group's
|
||||
// middleware runs only for a request that matches one of its routes,
|
||||
// and a browser's preflight OPTIONS request matches none, so the CORS
|
||||
// middleware could not answer it.
|
||||
s.router.Route("/v1", func(r chi.Router) {
|
||||
r.Use(s.mw.CORS())
|
||||
|
||||
// Main image proxy route
|
||||
// /v1/image/<host>/<path>/<width>x<height>.<format>
|
||||
r.Get("/v1/image/*", s.h.HandleImage())
|
||||
r.Head("/v1/image/*", s.h.HandleImage())
|
||||
// Refused while maintenance mode is on. Only these: the image's
|
||||
// Docker HEALTHCHECK requests the health check, a 503 there would
|
||||
// make the container unhealthy, and upaas marks a deploy failed
|
||||
// when its container is unhealthy.
|
||||
r.Group(func(r chi.Router) {
|
||||
r.Use(s.refuseDuringMaintenance)
|
||||
|
||||
// Encrypted image URL route
|
||||
// The trailing filename (e.g., /img.jpg) is ignored but helps
|
||||
// browsers with content type
|
||||
r.Get("/v1/e/{token}/*", s.h.HandleImageEnc())
|
||||
// Main image proxy route
|
||||
// /v1/image/<host>/<path>/<width>x<height>.<format>
|
||||
r.Get("/image/*", s.h.HandleImage())
|
||||
r.Head("/image/*", s.h.HandleImage())
|
||||
|
||||
// Encrypted image URL route
|
||||
// The trailing filename (e.g., /img.jpg) is ignored but helps
|
||||
// browsers with content type
|
||||
r.Get("/e/{token}/*", s.h.HandleImageEnc())
|
||||
})
|
||||
})
|
||||
|
||||
// Metrics endpoint with auth
|
||||
|
||||
Reference in New Issue
Block a user