Unverified Commit 72a27504 authored by Minseong Choi's avatar Minseong Choi 💬
Browse files

fix(config): refuse mojang as an auth-source tag

Mojang is prepended in code as the first, identity source, and the
config templates say not to list it. Nothing enforced that. A listed
tag = "mojang" loaded, and nano's startup list printed it as if Mojang
had been pointed at that url, while the real Mojang was still asked
first. The listed entry was a separate third-party source: asked again
on every login that got past Mojang, adding up to five seconds when its
url was Mojang's own and it answered 204 each time.

Any case of "mojang" is now rejected at load with a message saying
Mojang is built in and must not be listed. The duplicate-tag check could
not catch this because the built-in source never passes through it.

The new test loads "mojang" and "Mojang" through LoadNano; both loaded
before this change.
parent 2c74080b
Loading
Loading
Loading
Loading
+6 −0
Changes for internal/config/config.go: 6 added lines, 0 removed lines.
Original line number Diff line number Diff line
@@ -364,6 +364,12 @@ func (c *Config) validateAuthSources() error {
		if strings.Contains(s.Tag, ":") {
			return fmt.Errorf("config: [[auth_source]] tag %q contains ':'; the tag and a player's native id are joined with ':' to derive their UUID, so a ':' in a tag would let another source mint this source's players", s.Tag)
		}
		// Mojang is the built-in first source. A listed "mojang" is never it: it is asked again,
		// after Mojang, on every login that reaches it, and nano's startup list then reads as if
		// Mojang had been pointed at that url.
		if strings.EqualFold(s.Tag, "mojang") {
			return fmt.Errorf("config: [[auth_source]] tag %q is reserved: Mojang is built in as the first source and must not be listed", s.Tag)
		}
		if _, dup := seenTags[s.Tag]; dup {
			return fmt.Errorf("config: [[auth_source]] tag %q is used twice — tags are per-source UUID namespaces and must be unique", s.Tag)
		}
+11 −0
Changes for internal/config/config_test.go: 11 added lines, 0 removed lines.
Original line number Diff line number Diff line
@@ -308,6 +308,17 @@ url = "postgres://felis@db/felis"
	}
}

// TestLoadRejectsMojangAuthSourceTag: Mojang is prepended in code, so a listed "mojang" is a
// second, different source that only looks like a Mojang override.
func TestLoadRejectsMojangAuthSourceTag(t *testing.T) {
	for _, tag := range []string{"mojang", "Mojang"} {
		_, err := config.LoadNano(writeTOML(t, "[[auth_source]]\ntag = \""+tag+"\"\nprefix = \"MJ\"\nurl = \"https://sessionserver.mojang.com/session/minecraft/hasJoined\"\n"))
		if err == nil || !strings.Contains(err.Error(), "built in") {
			t.Errorf("tag %q: err = %v, want a refusal saying Mojang is built in", tag, err)
		}
	}
}

// TestLoadRejectsSchemelessAuthSourceURL pins the silently-dead-source guard: a URL with no
// http(s):// scheme makes http.NewRequest fail, so the source never validates any login yet
// felis-api boots green. Reject at load with the scheme contract spelled out. An empty tag