fix(config): reject auth-source urls the resolver cannot query
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.
This commit is contained in:
2 files changed
+51
-5
No files matched your search
@@ -6,6 +6,7 @@ package config
|
|||||||
|
|
||||||
import (
|
import (
|
||||||
"fmt"
|
"fmt"
|
||||||
|
"net/url"
|
||||||
"regexp"
|
"regexp"
|
||||||
"strings"
|
"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
|
// (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
|
// hold); a duplicate prefix collapses two same-named players from different sources onto
|
||||||
// one in-game name
|
// one in-game name
|
||||||
// (they stay distinct identities, but neither can be online while the other is); a
|
// (they stay distinct identities, but neither can be online while the other is); a URL
|
||||||
// scheme-less URL makes http.NewRequest fail so the source is silently dead (never validates
|
// the resolver cannot query leaves the source silently dead (never validates any login).
|
||||||
// any login). All fail fast at load, not per-login. Split out from Validate so the nano-only
|
// 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
|
// LoadNano (no control-plane fields) enforces the identical rules — the impersonation guard
|
||||||
// has one owner, shared by full-api and nano.
|
// has one owner, shared by full-api and nano.
|
||||||
func (c *Config) validateAuthSources() error {
|
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)
|
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{}{}
|
seenPrefixes[lower] = struct{}{}
|
||||||
if !strings.HasPrefix(s.URL, "http://") && !strings.HasPrefix(s.URL, "https://") {
|
if problem := hasJoinedURLProblem(s.URL); problem != "" {
|
||||||
return fmt.Errorf("config: [[auth_source]] %q url %q must be a scheme-qualified http(s):// hasJoined endpoint", s.Tag, s.URL)
|
return fmt.Errorf("config: [[auth_source]] %q url %q %s", s.Tag, s.URL, problem)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
return nil
|
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 ""
|
||||||
|
}
|
||||||
@@ -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
|
// 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]] —
|
// 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
|
// the control-plane requirements (database.url, root_domain) that full Load enforces are
|
||||||
|
|||||||
Reference in new issue
Block a user