Allow CORS origins from CORS_ALLOWED_ORIGINS
Echo compares allowed origins literally, so "https://*" and "http://*" never matched a real origin and no CORS headers were sent. The allowed origins now come from CORS_ALLOWED_ORIGINS (comma-separated, checked at startup). When it is empty, only same-origin requests work, which is enough for the bundled frontend. Credentials are no longer allowed since the API uses Bearer tokens. Browsers may now send Range and X-Queue-Id and read X-Queue-Id and Content-Range.
This commit is contained in:
1 parent
a705d7b476
commit
d31bf3551c
4 files changed
+85
-8
No files matched your search
@@ -5,6 +5,7 @@ import (
|
|||||||
"errors"
|
"errors"
|
||||||
"fmt"
|
"fmt"
|
||||||
"net"
|
"net"
|
||||||
|
"net/url"
|
||||||
"os"
|
"os"
|
||||||
"strconv"
|
"strconv"
|
||||||
"strings"
|
"strings"
|
||||||
@@ -38,6 +39,10 @@ type Config struct {
|
|||||||
// TrustedProxies are CIDR ranges whose X-Forwarded-For header is trusted when resolving client IPs.
|
// TrustedProxies are CIDR ranges whose X-Forwarded-For header is trusted when resolving client IPs.
|
||||||
TrustedProxies []*net.IPNet
|
TrustedProxies []*net.IPNet
|
||||||
|
|
||||||
|
// CORSAllowedOrigins are the browser origins, like https://music.example.com, that may call the API
|
||||||
|
// from another site. Empty means only same-origin requests, which is enough for the bundled frontend.
|
||||||
|
CORSAllowedOrigins []string
|
||||||
|
|
||||||
ReleasesURL string
|
ReleasesURL string
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -91,6 +96,11 @@ func load(getenv func(string) string) (Config, error) {
|
|||||||
errs = append(errs, fmt.Errorf("TRUSTED_PROXIES: %w", err))
|
errs = append(errs, fmt.Errorf("TRUSTED_PROXIES: %w", err))
|
||||||
}
|
}
|
||||||
|
|
||||||
|
cfg.CORSAllowedOrigins, err = parseOrigins(getenv("CORS_ALLOWED_ORIGINS"))
|
||||||
|
if err != nil {
|
||||||
|
errs = append(errs, fmt.Errorf("CORS_ALLOWED_ORIGINS: %w", err))
|
||||||
|
}
|
||||||
|
|
||||||
return cfg, errors.Join(errs...)
|
return cfg, errors.Join(errs...)
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -110,6 +120,27 @@ func parseCIDRs(value string) ([]*net.IPNet, error) {
|
|||||||
return nets, nil
|
return nets, nil
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// parseOrigins parses a comma-separated list of origins (scheme://host[:port], nothing else).
|
||||||
|
func parseOrigins(value string) ([]string, error) {
|
||||||
|
var origins []string
|
||||||
|
for part := range strings.SplitSeq(value, ",") {
|
||||||
|
part = strings.TrimSpace(part)
|
||||||
|
if part == "" {
|
||||||
|
continue
|
||||||
|
}
|
||||||
|
u, err := url.Parse(part)
|
||||||
|
if err != nil {
|
||||||
|
return nil, err
|
||||||
|
}
|
||||||
|
if (u.Scheme != "http" && u.Scheme != "https") || u.Host == "" || u.Path != "" || u.RawQuery != "" ||
|
||||||
|
u.Fragment != "" || u.User != nil || strings.Contains(u.Host, "*") {
|
||||||
|
return nil, fmt.Errorf("%q is not an origin like https://music.example.com", part)
|
||||||
|
}
|
||||||
|
origins = append(origins, u.Scheme+"://"+u.Host)
|
||||||
|
}
|
||||||
|
return origins, nil
|
||||||
|
}
|
||||||
|
|
||||||
func withDefault(value, fallback string) string {
|
func withDefault(value, fallback string) string {
|
||||||
if value == "" {
|
if value == "" {
|
||||||
return fallback
|
return fallback
|
||||||
|
|||||||
@@ -31,6 +31,7 @@ func TestLoadDefaults(t *testing.T) {
|
|||||||
assert.Equal(t, "songs", cfg.SoundCheckPath)
|
assert.Equal(t, "songs", cfg.SoundCheckPath)
|
||||||
assert.Equal(t, DefaultReleasesURL, cfg.ReleasesURL)
|
assert.Equal(t, DefaultReleasesURL, cfg.ReleasesURL)
|
||||||
assert.Empty(t, cfg.TrustedProxies)
|
assert.Empty(t, cfg.TrustedProxies)
|
||||||
|
assert.Empty(t, cfg.CORSAllowedOrigins)
|
||||||
}
|
}
|
||||||
|
|
||||||
func TestLoadMissingRequired(t *testing.T) {
|
func TestLoadMissingRequired(t *testing.T) {
|
||||||
@@ -56,6 +57,21 @@ func TestLoadTrustedProxies(t *testing.T) {
|
|||||||
assert.ErrorContains(t, err, "TRUSTED_PROXIES")
|
assert.ErrorContains(t, err, "TRUSTED_PROXIES")
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func TestLoadCORSAllowedOrigins(t *testing.T) {
|
||||||
|
values := validEnv()
|
||||||
|
values["CORS_ALLOWED_ORIGINS"] = "https://music.example.com, http://localhost:5173,"
|
||||||
|
cfg, err := load(env(values))
|
||||||
|
require.NoError(t, err)
|
||||||
|
assert.Equal(t, []string{"https://music.example.com", "http://localhost:5173"}, cfg.CORSAllowedOrigins)
|
||||||
|
|
||||||
|
for _, invalid := range []string{"music.example.com", "ftp://example.com", "https://example.com/app",
|
||||||
|
"https://*.example.com", "https://*", "*", "https://example.com?x=1", "https://user@example.com"} {
|
||||||
|
values["CORS_ALLOWED_ORIGINS"] = invalid
|
||||||
|
_, err = load(env(values))
|
||||||
|
assert.ErrorContains(t, err, "CORS_ALLOWED_ORIGINS", invalid)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
func TestLoadInvalidPort(t *testing.T) {
|
func TestLoadInvalidPort(t *testing.T) {
|
||||||
values := validEnv()
|
values := validEnv()
|
||||||
values["PORT"] = "abc"
|
values["PORT"] = "abc"
|
||||||
|
|||||||
@@ -40,14 +40,17 @@ func (s *Server) routes() (*echo.Echo, huma.API) {
|
|||||||
|
|
||||||
e.Use(logging.RequestLogger())
|
e.Use(logging.RequestLogger())
|
||||||
e.Use(echoMiddleware.Recover())
|
e.Use(echoMiddleware.Recover())
|
||||||
e.Use(echoMiddleware.CORSWithConfig(echoMiddleware.CORSConfig{
|
// Without allowed origins no CORS headers are sent, so only same-origin browser requests work.
|
||||||
AllowOrigins: []string{"https://*", "http://*"},
|
// The API uses Bearer tokens, not cookies, so credentials are not allowed.
|
||||||
AllowMethods: []string{"GET", "POST", "PUT", "DELETE", "OPTIONS", "PATCH"},
|
if len(s.cfg.CORSAllowedOrigins) > 0 {
|
||||||
AllowHeaders: []string{"Accept", "Authorization", "Content-Type", "X-CSRF-Token", middleware.QueueHeader},
|
e.Use(echoMiddleware.CORSWithConfig(echoMiddleware.CORSConfig{
|
||||||
ExposeHeaders: []string{middleware.QueueHeader},
|
AllowOrigins: s.cfg.CORSAllowedOrigins,
|
||||||
AllowCredentials: true,
|
AllowMethods: []string{"GET", "POST", "PUT", "DELETE", "OPTIONS", "PATCH"},
|
||||||
MaxAge: 300,
|
AllowHeaders: []string{"Accept", "Authorization", "Content-Type", "Range", middleware.QueueHeader},
|
||||||
}))
|
ExposeHeaders: []string{middleware.QueueHeader, "Content-Range", "Accept-Ranges"},
|
||||||
|
MaxAge: 300,
|
||||||
|
}))
|
||||||
|
}
|
||||||
|
|
||||||
// The API: /api/v1 is documented at /docs and /openapi.json.
|
// The API: /api/v1 is documented at /docs and /openapi.json.
|
||||||
a := newAPI(e)
|
a := newAPI(e)
|
||||||
|
|||||||
@@ -196,3 +196,30 @@ func TestTrustedProxies(t *testing.T) {
|
|||||||
assert.Equal(t, http.StatusTooManyRequests, e.doBody(http.MethodPost, "/api/v1/token", `{}`, "X-Forwarded-For", "10.0.0.1").Code)
|
assert.Equal(t, http.StatusTooManyRequests, e.doBody(http.MethodPost, "/api/v1/token", `{}`, "X-Forwarded-For", "10.0.0.1").Code)
|
||||||
assert.Equal(t, http.StatusOK, e.doBody(http.MethodPost, "/api/v1/token", `{}`, "X-Forwarded-For", "10.0.0.2").Code)
|
assert.Equal(t, http.StatusOK, e.doBody(http.MethodPost, "/api/v1/token", `{}`, "X-Forwarded-For", "10.0.0.2").Code)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func TestCORSAllowedOrigins(t *testing.T) {
|
||||||
|
e := newTestEnv(t, func(c *config.Config) { c.CORSAllowedOrigins = []string{"https://music.example.com"} })
|
||||||
|
|
||||||
|
rec := e.do(http.MethodGet, "/api/v1/music/list", "Origin", "https://music.example.com")
|
||||||
|
assert.Equal(t, http.StatusOK, rec.Code)
|
||||||
|
assert.Equal(t, "https://music.example.com", rec.Header().Get("Access-Control-Allow-Origin"))
|
||||||
|
assert.Contains(t, rec.Header().Get("Access-Control-Expose-Headers"), "X-Queue-Id")
|
||||||
|
assert.Empty(t, rec.Header().Get("Access-Control-Allow-Credentials"))
|
||||||
|
|
||||||
|
preflight := e.do(http.MethodOptions, "/api/v1/music/list", "Origin", "https://music.example.com",
|
||||||
|
"Access-Control-Request-Method", "GET", "Access-Control-Request-Headers", "authorization,x-queue-id")
|
||||||
|
assert.Equal(t, http.StatusNoContent, preflight.Code)
|
||||||
|
assert.Equal(t, "https://music.example.com", preflight.Header().Get("Access-Control-Allow-Origin"))
|
||||||
|
assert.Contains(t, preflight.Header().Get("Access-Control-Allow-Headers"), "X-Queue-Id")
|
||||||
|
assert.Contains(t, preflight.Header().Get("Access-Control-Allow-Headers"), "Authorization")
|
||||||
|
|
||||||
|
other := e.do(http.MethodGet, "/api/v1/music/list", "Origin", "https://evil.example.com")
|
||||||
|
assert.Empty(t, other.Header().Get("Access-Control-Allow-Origin"), "other origins get no CORS headers")
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestCORSWithoutAllowedOrigins(t *testing.T) {
|
||||||
|
e := newTestEnv(t)
|
||||||
|
rec := e.do(http.MethodGet, "/api/v1/version", "Origin", "https://music.example.com")
|
||||||
|
assert.Equal(t, http.StatusOK, rec.Code, "the request itself works; the browser blocks reading it")
|
||||||
|
assert.Empty(t, rec.Header().Get("Access-Control-Allow-Origin"))
|
||||||
|
}
|
||||||
Reference in new issue
Block a user