diff --git a/docs/openapi.yaml b/docs/openapi.yaml index d7a9dca..c1f5977 100644 --- a/docs/openapi.yaml +++ b/docs/openapi.yaml @@ -1422,6 +1422,8 @@ paths: additionalProperties: type: string enum: [retiring, start_failed, owner, wake, owner_only, allowlist] + '400': + $ref: '#/components/responses/BadRequest' '401': $ref: '#/components/responses/Unauthorized' @@ -1505,6 +1507,8 @@ paths: required: [linked] properties: linked: { type: boolean } + '400': + $ref: '#/components/responses/BadRequest' '401': $ref: '#/components/responses/Unauthorized' @@ -1637,6 +1641,8 @@ paths: required: [blacklisted] properties: blacklisted: { type: boolean } + '400': + $ref: '#/components/responses/BadRequest' '401': $ref: '#/components/responses/Unauthorized' @@ -4684,6 +4690,8 @@ paths: properties: ok: { type: boolean, const: true } mc_uuid: { type: string, format: uuid } + '400': + $ref: '#/components/responses/BadRequest' '401': $ref: '#/components/responses/Unauthorized' '403': diff --git a/internal/api/handlers_account.go b/internal/api/handlers_account.go index 61fdb73..e437c03 100644 --- a/internal/api/handlers_account.go +++ b/internal/api/handlers_account.go @@ -6,6 +6,8 @@ import ( "net/http" "strings" "time" + + "github.com/google/uuid" ) // Account-linking endpoints (spec §10). The flow is forced by the @@ -51,6 +53,28 @@ func validAuthSource(s string) bool { return s == authSourceMojang || s == authSourceThirdParty } +// errBadMCUUID answers an mc_uuid that is not a UUID. Every mc_uuid column is +// Postgres's uuid type, which refuses such text with 22P02, and that reached the +// caller as a 500. +var errBadMCUUID = newError(http.StatusBadRequest, "bad_mc_uuid", + "mc_uuid must be a UUID, such as 069a79f4-44e9-4726-a5be-fca90e38aaf5") + +// parseMCUUID reads an mc_uuid from a request: surrounding spaces trimmed, empty +// → 400 bad_request "mc_uuid is required", not a UUID → errBadMCUUID. It returns +// the canonical lowercase hyphenated form, the text Postgres gives back for the +// column, so what a handler stores, echoes and compares is one spelling. +func parseMCUUID(s string) (string, error) { + s = strings.TrimSpace(s) + if s == "" { + return "", newError(http.StatusBadRequest, "bad_request", "mc_uuid is required") + } + id, err := uuid.Parse(s) + if err != nil { + return "", errBadMCUUID + } + return id.String(), nil +} + // deriveAuthSource infers the auth source from the UUID's version nibble when // the minting backend omitted auth_source. Felis-nano rewrites every // third-party profile to a name-based UUIDv3 under its namespace before it ever @@ -104,8 +128,9 @@ func (a *API) handleCreateLinkCode(w http.ResponseWriter, r *http.Request) { writeError(w, r, err) return } - if req.MCUUID == "" { - writeError(w, r, newError(http.StatusBadRequest, "bad_request", "mc_uuid is required")) + mcUUID, err := parseMCUUID(req.MCUUID) + if err != nil { + writeError(w, r, err) return } // Default an omitted source from the UUID's version nibble (v3 = felis-nano @@ -114,7 +139,7 @@ func (a *API) handleCreateLinkCode(w http.ResponseWriter, r *http.Request) { // stored value the panel will later mislabel. authSource := req.AuthSource if authSource == "" { - authSource = deriveAuthSource(req.MCUUID) + authSource = deriveAuthSource(mcUUID) } if !validAuthSource(authSource) { writeError(w, r, newError(http.StatusBadRequest, "bad_request", @@ -127,7 +152,7 @@ func (a *API) handleCreateLinkCode(w http.ResponseWriter, r *http.Request) { return } expiresAt := a.now().Add(linkCodeTTL) - if err := a.Repo.CreateLinkCode(r.Context(), code, req.MCUUID, authSource, expiresAt); err != nil { + if err := a.Repo.CreateLinkCode(r.Context(), code, mcUUID, authSource, expiresAt); err != nil { writeError(w, r, err) return } @@ -171,12 +196,12 @@ func (a *API) handleCreateLinkCode(w http.ResponseWriter, r *http.Request) { // handlers_player_reclaim.go keeps CODE-ONLY (reclaimed_by_user_id stays NULL on // the verifiable path); this endpoint reports link completion only, not that choice. func (a *API) handleLinkStatus(w http.ResponseWriter, r *http.Request) { - mcUUID := r.PathValue("mc_uuid") - if mcUUID == "" { - writeError(w, r, newError(http.StatusBadRequest, "bad_request", "mc_uuid is required")) + mcUUID, err := parseMCUUID(r.PathValue("mc_uuid")) + if err != nil { + writeError(w, r, err) return } - _, err := a.Repo.UserByMCUUID(r.Context(), mcUUID) + _, err = a.Repo.UserByMCUUID(r.Context(), mcUUID) switch { case errors.Is(err, ErrNotFound): // Not linked yet. For the poller this is simply "keep waiting": velocity diff --git a/internal/api/handlers_account_migrate.go b/internal/api/handlers_account_migrate.go index 2d055a0..d62fc9c 100644 --- a/internal/api/handlers_account_migrate.go +++ b/internal/api/handlers_account_migrate.go @@ -111,9 +111,9 @@ func (a *API) handleMigrateStart(w http.ResponseWriter, r *http.Request) { writeError(w, r, err) return } - mcUUID := strings.TrimSpace(req.MCUUID) - if mcUUID == "" { - writeError(w, r, newError(http.StatusBadRequest, "bad_request", "mc_uuid is required")) + mcUUID, err := parseMCUUID(req.MCUUID) + if err != nil { + writeError(w, r, err) return } sourceUserID, err := a.Repo.UserByMCUUID(r.Context(), mcUUID) diff --git a/internal/api/handlers_allowlist.go b/internal/api/handlers_allowlist.go index 2921e4d..52d0ed9 100644 --- a/internal/api/handlers_allowlist.go +++ b/internal/api/handlers_allowlist.go @@ -66,7 +66,7 @@ func (a *API) handleAllowlistSetWake(w http.ResponseWriter, r *http.Request) { } id, err := uuid.Parse(r.PathValue("uuid")) if err != nil { - writeError(w, r, newError(http.StatusBadRequest, "bad_request", "invalid Minecraft UUID")) + writeError(w, r, errBadMCUUID) return } var req allowlistWakeRequest diff --git a/internal/api/handlers_internal.go b/internal/api/handlers_internal.go index 07f540d..1423763 100644 --- a/internal/api/handlers_internal.go +++ b/internal/api/handlers_internal.go @@ -83,11 +83,12 @@ func (a *API) handleJoinEvent(w http.ResponseWriter, r *http.Request) { writeError(w, r, err) return } - if req.MCUUID == "" { - writeError(w, r, newError(http.StatusBadRequest, "bad_request", "mc_uuid is required")) + mcUUID, err := parseMCUUID(req.MCUUID) + if err != nil { + writeError(w, r, err) return } - if err := a.Repo.RecordJoin(r.Context(), name, req.MCUUID); err != nil { + if err := a.Repo.RecordJoin(r.Context(), name, mcUUID); err != nil { a.writeLookupError(w, r, err) return } @@ -120,8 +121,9 @@ func (a *API) handleInternalWake(w http.ResponseWriter, r *http.Request) { writeError(w, r, err) return } - if req.MCUUID == "" { - writeError(w, r, newError(http.StatusBadRequest, "bad_request", "mc_uuid is required")) + mcUUID, err := parseMCUUID(req.MCUUID) + if err != nil { + writeError(w, r, err) return } @@ -148,7 +150,7 @@ func (a *API) handleInternalWake(w http.ResponseWriter, r *http.Request) { return } - if err := a.authorizeWakeByUUID(r.Context(), req.MCUUID, info, rec); err != nil { + if err := a.authorizeWakeByUUID(r.Context(), mcUUID, info, rec); err != nil { writeError(w, r, err) return } @@ -235,8 +237,9 @@ func (a *API) handleInternalClaim(w http.ResponseWriter, r *http.Request) { writeError(w, r, err) return } - if req.MCUUID == "" { - writeError(w, r, newError(http.StatusBadRequest, "bad_request", "mc_uuid is required")) + mcUUID, err := parseMCUUID(req.MCUUID) + if err != nil { + writeError(w, r, err) return } @@ -245,7 +248,7 @@ func (a *API) handleInternalClaim(w http.ResponseWriter, r *http.Request) { // no separate IsLinked check (mirrors the external claim's order, link → quota // → write). ErrNotFound here is "claimer not linked" (412), never "server // missing" — that distinction is the claim call's, below. - userID, err := a.Repo.UserByMCUUID(r.Context(), req.MCUUID) + userID, err := a.Repo.UserByMCUUID(r.Context(), mcUUID) if err != nil { if errors.Is(err, ErrNotFound) { writeError(w, r, newError(http.StatusPreconditionFailed, "not_linked", @@ -364,9 +367,9 @@ const ( // since a retry gets past those. Retiring comes first: nobody may start such a // server, so it is the reason a stranger is shown too. func (a *API) handleInternalMenuAccess(w http.ResponseWriter, r *http.Request) { - mcUUID := r.PathValue("mc_uuid") - if mcUUID == "" { - writeError(w, r, newError(http.StatusBadRequest, "bad_request", "mc_uuid is required")) + mcUUID, err := parseMCUUID(r.PathValue("mc_uuid")) + if err != nil { + writeError(w, r, err) return } infos, err := a.Cluster.ListServers(r.Context()) diff --git a/internal/api/handlers_mcuuid_test.go b/internal/api/handlers_mcuuid_test.go new file mode 100644 index 0000000..8a2acca --- /dev/null +++ b/internal/api/handlers_mcuuid_test.go @@ -0,0 +1,70 @@ +package api + +import ( + "encoding/json" + "net/http" + "testing" +) + +// Every door that takes an mc_uuid refuses text that is not a UUID with 400 +// bad_mc_uuid. The columns are Postgres uuids, and such text used to reach the +// database, fail there with 22P02 and come back as a 500. A UUID in another +// spelling is stored and echoed in the one Postgres gives back. +func TestMCUUIDMustBeAUUID(t *testing.T) { + repo := newFakeRepo() + repo.byName["survival"] = &ServerRecord{Name: "survival", OwnerID: "u1"} + repo.claimOK["survival"] = true + repo.seedUser(UserView{ID: "u2", Username: "alice", Role: "user"}) + cl := newFakeCluster() + cl.byName["survival"] = &ServerInfo{Name: "survival", AutostartPolicy: "public"} + api := newTestAPI(repo, cl) + api.External = staticExternal{p: &Principal{UserID: "owner1", Role: "owner", ViaAdminAccess: true}} + ih, eh := api.InternalHandler(), api.ExternalHandler() + + const bad = "not-a-uuid" + body := `{"mc_uuid":"` + bad + `"}` + for _, c := range []struct { + h http.Handler + method, path, body string + }{ + {ih, "POST", "/api/v1/internal/servers/survival/join-event", body}, + {ih, "POST", "/api/v1/internal/servers/survival/wake", body}, + {ih, "POST", "/api/v1/internal/servers/survival/claim", body}, + {ih, "GET", "/api/v1/internal/player/menu-access/" + bad, ""}, + {ih, "POST", "/api/v1/internal/account/link/code", body}, + {ih, "GET", "/api/v1/internal/account/link/status/" + bad, ""}, + {ih, "POST", "/api/v1/internal/account/migrate/start", body}, + {ih, "GET", "/api/v1/internal/player/blacklist/" + bad, ""}, + {eh, "PUT", "/api/v1/servers/survival/allowlist/" + bad, `{"can_wake":true}`}, + {eh, "DELETE", "/api/v1/users/u2/links/" + bad, ""}, + {eh, "POST", "/api/v1/users/u2/links", body}, + } { + w := do(c.h, c.method, c.path, c.body, jsonHeader) + if w.Code != http.StatusBadRequest || decodeErr(t, w) != "bad_mc_uuid" { + t.Errorf("%s %s with %q: code = %d body %s, want 400 bad_mc_uuid", c.method, c.path, bad, w.Code, w.Body.String()) + } + } + if len(repo.joins) != 0 || len(repo.linkCodes) != 0 || len(repo.links) != 0 || len(repo.audits) != 0 { + t.Fatalf("a refused mc_uuid wrote joins=%v codes=%v links=%v audits=%v", repo.joins, repo.linkCodes, repo.links, repo.audits) + } + + const spelled, canonical = " 069A79F444E94726A5BEFCA90E38AAF5 ", "069a79f4-44e9-4726-a5be-fca90e38aaf5" + w := do(eh, "POST", "/api/v1/users/u2/links", `{"mc_uuid":"`+spelled+`"}`, jsonHeader) + var linked struct { + MCUUID string `json:"mc_uuid"` + } + if err := json.Unmarshal(w.Body.Bytes(), &linked); err != nil || w.Code != http.StatusOK || linked.MCUUID != canonical { + t.Fatalf("link of %q: code = %d body %s, want 200 echoing %s", spelled, w.Code, w.Body.String(), canonical) + } + if repo.links[canonical] != "u2" || len(repo.links) != 1 { + t.Fatalf("links = %v, want only %s → u2", repo.links, canonical) + } + if w := do(ih, "POST", "/api/v1/internal/account/link/code", `{"mc_uuid":"`+spelled+`"}`, jsonHeader); w.Code != http.StatusCreated { + t.Fatalf("link code for %q: code = %d body %s, want 201", spelled, w.Code, w.Body.String()) + } + for _, lc := range repo.linkCodes { + if lc.mcUUID != canonical { + t.Fatalf("link code minted for %q, want %s", lc.mcUUID, canonical) + } + } +} diff --git a/internal/api/handlers_player_reclaim.go b/internal/api/handlers_player_reclaim.go index 8405c48..bea1658 100644 --- a/internal/api/handlers_player_reclaim.go +++ b/internal/api/handlers_player_reclaim.go @@ -140,9 +140,9 @@ func (a *API) handleReclaimUsername(w http.ResponseWriter, r *http.Request) { // squatter before letting them in; the genuine Mojang UUID (same name, different // UUID) is never on the list, so it always passes. func (a *API) handleCheckBlacklist(w http.ResponseWriter, r *http.Request) { - mcUUID := r.PathValue("mc_uuid") - if mcUUID == "" { - writeError(w, r, newError(http.StatusBadRequest, "bad_request", "mc_uuid is required")) + mcUUID, err := parseMCUUID(r.PathValue("mc_uuid")) + if err != nil { + writeError(w, r, err) return } blacklisted, err := a.Repo.IsUsernameBlacklisted(r.Context(), mcUUID) diff --git a/internal/api/handlers_users.go b/internal/api/handlers_users.go index 2be6d04..6077356 100644 --- a/internal/api/handlers_users.go +++ b/internal/api/handlers_users.go @@ -447,11 +447,15 @@ func (a *API) handleUnbindUserPasskeys(w http.ResponseWriter, r *http.Request) { // (DELETE /users/{id}/links/{mc_uuid}). func (a *API) handleUnlinkAccount(w http.ResponseWriter, r *http.Request) { userID := r.PathValue("id") - mcUUID := r.PathValue("mc_uuid") - if userID == "" || mcUUID == "" { + if userID == "" { writeError(w, r, errBadRequest) return } + mcUUID, err := parseMCUUID(r.PathValue("mc_uuid")) + if err != nil { + writeError(w, r, err) + return + } if err := a.Repo.UnlinkAccount(r.Context(), userID, mcUUID); err != nil { if errors.Is(err, ErrNotFound) { @@ -484,16 +488,16 @@ func (a *API) handleLinkAccount(w http.ResponseWriter, r *http.Request) { writeError(w, r, err) return } - if body.MCUUID == "" { - writeError(w, r, newError(http.StatusBadRequest, "bad_request", - "mc_uuid is required")) + mcUUID, err := parseMCUUID(body.MCUUID) + if err != nil { + writeError(w, r, err) return } if body.AuthSource == "" { // Same version-nibble inference as the mint path (handlers_account.go): // defaulting to mojang here would leave a force-linked thirdparty UUID // outside the reclaim guard. - body.AuthSource = deriveAuthSource(body.MCUUID) + body.AuthSource = deriveAuthSource(mcUUID) } if !validAuthSource(body.AuthSource) { writeError(w, r, newError(http.StatusBadRequest, "bad_request", @@ -501,7 +505,7 @@ func (a *API) handleLinkAccount(w http.ResponseWriter, r *http.Request) { return } - if err := a.Repo.LinkAccount(r.Context(), userID, body.MCUUID, body.AuthSource); err != nil { + if err := a.Repo.LinkAccount(r.Context(), userID, mcUUID, body.AuthSource); err != nil { if errors.Is(err, ErrConflict) { writeError(w, r, newError(http.StatusConflict, "already_linked", "this UUID is already linked to a different user")) @@ -518,7 +522,7 @@ func (a *API) handleLinkAccount(w http.ResponseWriter, r *http.Request) { a.audit(r, "user.link_account", userID) writeJSON(w, http.StatusOK, map[string]any{ "ok": true, - "mc_uuid": body.MCUUID, + "mc_uuid": mcUUID, "auth_source": body.AuthSource, }) } diff --git a/panel/src/i18n/resources/en-US/errors.json b/panel/src/i18n/resources/en-US/errors.json index 7e6548c..a8eeacd 100644 --- a/panel/src/i18n/resources/en-US/errors.json +++ b/panel/src/i18n/resources/en-US/errors.json @@ -4,6 +4,7 @@ "not_linked": "Link your Minecraft account before claiming (Account → Link).", "invalid_code": "That code is invalid or expired — request a fresh one and try again.", "already_linked": "That Minecraft account is already linked to another user.", + "bad_mc_uuid": "That isn't a Minecraft UUID. It looks like 069a79f4-44e9-4726-a5be-fca90e38aaf5, with or without the dashes.", "quota_exceeded": "You have reached your server quota.", "already_claimed": "Someone else just claimed this server.", "image_not_whitelisted": "That image is not on the whitelist.", diff --git a/panel/src/i18n/resources/zh-CN/errors.json b/panel/src/i18n/resources/zh-CN/errors.json index b894e93..baa140c 100644 --- a/panel/src/i18n/resources/zh-CN/errors.json +++ b/panel/src/i18n/resources/zh-CN/errors.json @@ -4,6 +4,7 @@ "not_linked": "请先关联 Minecraft 账户(账户页 → 关联)。", "invalid_code": "代码无效或已过期——请重新获取后再试。", "already_linked": "该 Minecraft 账户已关联至其他用户。", + "bad_mc_uuid": "这不是 Minecraft UUID。它的样子是 069a79f4-44e9-4726-a5be-fca90e38aaf5,短横线可带可不带。", "quota_exceeded": "服务器数量已达配额上限。", "already_claimed": "该服务器已被他人抢先认领。", "image_not_whitelisted": "该镜像未在白名单中。", diff --git a/panel/src/lib/api.test.ts b/panel/src/lib/api.test.ts index 25f2717..894f4fa 100644 --- a/panel/src/lib/api.test.ts +++ b/panel/src/lib/api.test.ts @@ -438,6 +438,12 @@ describe("api access-control wire shapes", () => { ); }); + it("says what a Minecraft UUID looks like when the typed one is not", () => { + expect(humanizeError({ status: 400, code: "bad_mc_uuid" })).toBe( + "That isn't a Minecraft UUID. It looks like 069a79f4-44e9-4726-a5be-fca90e38aaf5, with or without the dashes.", + ); + }); + it("says email codes are off when the install has no mail relay", async () => { const { humanizeError } = await import("./api"); expect(humanizeError({ status: 503, code: "mail_unavailable" })).toBe( diff --git a/panel/src/lib/api.ts b/panel/src/lib/api.ts index b2ac34e..1d8028c 100644 --- a/panel/src/lib/api.ts +++ b/panel/src/lib/api.ts @@ -1002,6 +1002,8 @@ export function humanizeError(e: unknown): string { return t("invalid_code"); case "already_linked": return t("already_linked"); + case "bad_mc_uuid": + return t("bad_mc_uuid"); case "otp_resend_cooldown": return t("otp_resend_cooldown"); case "otp_locked": diff --git a/panel/src/lib/openapi.gen.ts b/panel/src/lib/openapi.gen.ts index 1be2b46..fcf8bf5 100644 --- a/panel/src/lib/openapi.gen.ts +++ b/panel/src/lib/openapi.gen.ts @@ -3427,6 +3427,7 @@ export interface operations { }; }; }; + 400: components["responses"]["BadRequest"]; 401: components["responses"]["Unauthorized"]; }; }; @@ -3491,6 +3492,7 @@ export interface operations { }; }; }; + 400: components["responses"]["BadRequest"]; 401: components["responses"]["Unauthorized"]; }; }; @@ -3615,6 +3617,7 @@ export interface operations { }; }; }; + 400: components["responses"]["BadRequest"]; 401: components["responses"]["Unauthorized"]; }; }; @@ -6651,6 +6654,7 @@ export interface operations { }; }; }; + 400: components["responses"]["BadRequest"]; 401: components["responses"]["Unauthorized"]; 403: components["responses"]["Forbidden"]; /** @description No linked account for this UUID. */