From 2c74080b78d766503b3ce8ce64b6f96f1d478ffa Mon Sep 17 00:00:00 2001 From: Minseong Choi Date: Tue, 22 Sep 2026 13:23:02 +0900 Subject: [PATCH] fix(config): reject auth-source urls the resolver cannot query MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The url check only looked for an http:// or https:// prefix. Several shapes passed it and then left the source dead at login time: no host ("https://"), a bad port, surrounding whitespace (sent as %20 and answered 404), and any query or fragment. The resolver appends "?username=…&serverId=…" to the url as a string, so an existing query swallows those parameters and a fragment hides them from the request entirely. Each loaded green, and every login from that source failed. The url is now parsed and must be http or https with a host, no query, no fragment and no surrounding whitespace. Load and LoadNano share the check. The shipped LittleSkin default and plain http:// endpoints, such as a same-host root on loopback, still load. The new test feeds each rejected shape to LoadNano. Against the previous prefix check, six of the seven load; only ftp:// was refused. --- internal/config/config.go | 33 ++++++++++++++++++++++++++++----- internal/config/config_test.go | 23 +++++++++++++++++++++++ 2 files changed, 51 insertions(+), 5 deletions(-) diff --git a/internal/config/config.go b/internal/config/config.go index ee9b383..c03834b 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -6,6 +6,7 @@ package config import ( "fmt" + "net/url" "regexp" "strings" @@ -343,9 +344,9 @@ var authSourcePrefixRe = regexp.MustCompile(`^[A-Za-z0-9]{1,4}$`) // (cross-source impersonation — the exact invariant the per-source rewrite exists to // hold); a duplicate prefix collapses two same-named players from different sources onto // one in-game name -// (they stay distinct identities, but neither can be online while the other is); a -// scheme-less URL makes http.NewRequest fail so the source is silently dead (never validates -// any login). All fail fast at load, not per-login. Split out from Validate so the nano-only +// (they stay distinct identities, but neither can be online while the other is); a URL +// the resolver cannot query leaves the source silently dead (never validates any login). +// All fail fast at load, not per-login. Split out from Validate so the nano-only // LoadNano (no control-plane fields) enforces the identical rules — the impersonation guard // has one owner, shared by full-api and nano. func (c *Config) validateAuthSources() error { @@ -377,9 +378,31 @@ func (c *Config) validateAuthSources() error { return fmt.Errorf("config: [[auth_source]] prefix %q is used twice — two sources sharing a prefix rewrite their same-named players onto the same in-game name", s.Prefix) } seenPrefixes[lower] = struct{}{} - if !strings.HasPrefix(s.URL, "http://") && !strings.HasPrefix(s.URL, "https://") { - return fmt.Errorf("config: [[auth_source]] %q url %q must be a scheme-qualified http(s):// hasJoined endpoint", s.Tag, s.URL) + if problem := hasJoinedURLProblem(s.URL); problem != "" { + return fmt.Errorf("config: [[auth_source]] %q url %q %s", s.Tag, s.URL, problem) } } return nil } + +// hasJoinedURLProblem says why u cannot be queried as a hasJoined endpoint, or "" if it +// can. The resolver appends "?username=…&serverId=…" to it as a string, so a query or +// fragment already in it swallows those parameters, and a URL the client cannot send only +// fails one login at a time, with the source looking like it knows nobody. +func hasJoinedURLProblem(u string) string { + if strings.TrimSpace(u) != u { + return "has leading or trailing whitespace" + } + p, err := url.Parse(u) + switch { + case err != nil: + return "does not parse: " + err.Error() + case p.Scheme != "http" && p.Scheme != "https": + return "must be a scheme-qualified http(s):// hasJoined endpoint" + case p.Host == "": + return "has no host" + case strings.ContainsAny(u, "?#"): + return "must not carry a query or fragment; the username and serverId parameters are appended to it" + } + return "" +} diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 50d6bb4..18801b8 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -331,6 +331,29 @@ url = "bare.example.net/hasJoined" } } +// TestLoadRejectsUnqueryableAuthSourceURL covers the URL shapes that carry a scheme yet can +// never be queried: the resolver appends the query string to the URL verbatim, so each of +// these would load green and leave a source that silently validates nobody. +func TestLoadRejectsUnqueryableAuthSourceURL(t *testing.T) { + for _, u := range []string{ + "ftp://a.example.net/hasJoined", + "https://", + "https://a.example.net/hasJoined?token=x", + "https://a.example.net/hasJoined?", + "https://a.example.net/hasJoined#x", + "https://a.example.net/hasJoined ", + "https://a.example.net:bad/hasJoined", + } { + _, err := config.LoadNano(writeTOML(t, "[[auth_source]]\ntag = \"a\"\nprefix = \"AA\"\nurl = \""+u+"\"\n")) + if err == nil { + t.Errorf("url %q loaded; it can never be queried", u) + } + } + if _, err := config.LoadNano(writeTOML(t, "[[auth_source]]\ntag = \"a\"\nprefix = \"AA\"\nurl = \"http://127.0.0.1:8080/hasJoined\"\n")); err != nil { + t.Errorf("a plain loopback endpoint must load: %v", err) + } +} + // TestLoadNanoAcceptsMinimalConfig is the linchpin of the Felis-nano fold: a nano host has no // Postgres and no FQDN, so LoadNano must accept a felis.toml carrying ONLY [[auth_source]] — // the control-plane requirements (database.url, root_domain) that full Load enforces are