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

fix(nano): report failing sources instead of treating them as a no 

A source that timed out, answered 5xx or 429, redirected, or sent a 200
without a usable profile was skipped exactly like one that answered 204.
With nobody else validating, the login got a 204 and Velocity told the
player their account is offline-mode. Nothing was logged, so a dead or
mistyped source URL, or an http:// root that now redirects to https since
redirects stopped being followed, failed every one of its players with
no trace.

Each such failure now logs the source tag and the cause; for a 3xx it
names the Location to configure instead. When no source validates and at
least one failed, the answer is 503, which Velocity reports as the auth
servers being down and logs with the status. A source answering 204 is
still a plain no, and a validating source still wins regardless of
failures before it.

The new subtest puts a 503 source, a redirecting source and an
unreachable one each behind a Mojang that answers 204, and expects 503.
Against the previous handler every case returns 204.
parent a0f54df2

internal/api/handlers_hasjoined.go

+34 −9
Changes for internal/api/handlers_hasjoined.go: 34 added lines, 9 removed lines.
Original line number Diff line number Diff line
@@ -6,6 +6,7 @@ import ( 
	"encoding/json"

	"fmt"

	"io"

	"log"

	"net/http"

	"net/url"

	"regexp"
@@ -112,8 +113,15 @@ func (a *API) handleHasJoined(w http.ResponseWriter, r *http.Request) { 
		return

	}



	prof, src := a.resolveHasJoined(r.Context(), username, serverID, q.Get("ip"))

	prof, src, failed := a.resolveHasJoined(r.Context(), username, serverID, q.Get("ip"))

	if prof == nil {

		// With a source down, "nobody knows this player" is not established: its player may

		// be the one logging in. 503 makes Velocity report the auth servers as down and log

		// the status, where a 204 would tell that player their account is offline-mode.

		if failed {

			w.WriteHeader(http.StatusServiceUnavailable)

			return

		}

		w.WriteHeader(http.StatusNoContent)

		return

	}
@@ -294,9 +302,11 @@ func lookupPremiumName(ctx context.Context, username string) (bool, error) { 
}



// resolveHasJoined queries each configured source in priority order and returns the

// first that validates the session (200 with a profile). A source that is down, answers

// non-200 (204 = "not my player"), or returns garbage is skipped.

func (a *API) resolveHasJoined(ctx context.Context, username, serverID, ip string) (*sessionProfile, AuthSource) {

// first that validates the session (200 with a profile). 204 is "not my player". Any other

// outcome (unreachable, another status, a body that is not a profile) skips the source too,

// but is logged with its tag and reported as failed: otherwise a dead or mistyped source

// looks exactly like a player it does not know, and nobody finds out.

func (a *API) resolveHasJoined(ctx context.Context, username, serverID, ip string) (prof *sessionProfile, src AuthSource, failed bool) {

	for _, src := range a.AuthSources {

		u := src.URL + "?username=" + url.QueryEscape(username) + "&serverId=" + url.QueryEscape(serverID)

		if ip != "" {
@@ -304,23 +314,38 @@ func (a *API) resolveHasJoined(ctx context.Context, username, serverID, ip strin 
		}

		req, err := http.NewRequestWithContext(ctx, http.MethodGet, u, nil)

		if err != nil {

			log.Printf("hasJoined: source %q: %v", src.Tag, err)

			failed = true

			continue

		}

		resp, err := authHTTPClient.Do(req)

		if err != nil {

			log.Printf("hasJoined: source %q: %v", src.Tag, err)

			failed = true

			continue

		}

		if resp.StatusCode != http.StatusOK {

			resp.Body.Close()

			switch {

			case resp.StatusCode == http.StatusNoContent:

			case resp.StatusCode >= 300 && resp.StatusCode < 400:

				log.Printf("hasJoined: source %q answered %s with Location %q; redirects are not followed, so set its url to the final endpoint", src.Tag, resp.Status, resp.Header.Get("Location"))

				failed = true

			default:

				log.Printf("hasJoined: source %q answered %s", src.Tag, resp.Status)

				failed = true

			}

			continue

		}

		var prof sessionProfile

		err = json.NewDecoder(io.LimitReader(resp.Body, 1<<16)).Decode(&prof)

		var p sessionProfile

		err = json.NewDecoder(io.LimitReader(resp.Body, 1<<16)).Decode(&p)

		resp.Body.Close()

		if err != nil || prof.ID == "" {

		if err != nil || p.ID == "" {

			log.Printf("hasJoined: source %q answered 200 without a usable profile (err=%v)", src.Tag, err)

			failed = true

			continue

		}

		return &prof, src

		return &p, src, failed

	}

	return nil, AuthSource{}

	return nil, AuthSource{}, failed

}

internal/api/handlers_hasjoined_test.go

+24 −0
Changes for internal/api/handlers_hasjoined_test.go: 24 added lines, 0 removed lines.
Original line number Diff line number Diff line
@@ -189,6 +189,30 @@ func TestHasJoined(t *testing.T) { 
		}

	})



	// A source that could not answer has not said no. With nobody validating, the login is

	// an outage whether that source errored, redirected or was unreachable.

	t.Run("failing source and no validator -> 503", func(t *testing.T) {

		down := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {

			w.WriteHeader(http.StatusServiceUnavailable)

		}))

		t.Cleanup(down.Close)

		redirector := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {

			http.Redirect(w, r, "https://elsewhere.example/hasJoined", http.StatusMovedPermanently)

		}))

		t.Cleanup(redirector.Close)

		nobody := fakeYgg(t, "", "")

		for _, failing := range []string{down.URL, redirector.URL, "http://127.0.0.1:1"} {

			api := newTestAPI(newFakeRepo(), newFakeCluster())

			api.AuthSources = []AuthSource{

				{Tag: "mojang", URL: nobody.URL, Identity: true},

				{Tag: "littleskin", Prefix: "LS", URL: failing},

			}

			if w := getHasJoined(api.InternalHandler(), "Ghost", "abc"); w.Code != http.StatusServiceUnavailable {

				t.Fatalf("source %s: code = %d, want 503", failing, w.Code)

			}

		}

	})



	// A skinless player's profile may come back with properties [], null or absent. The

	// relay must still send an array: Velocity's GameProfile parser throws on a missing or

	// null key and the login hangs, where the same answer sent straight to Velocity works.