Skip to content

Commit 5e44f8a

Browse files
committed
fix: tighten username whitespace handling
- Trim the draft in ChangeUsernameDialog before validating and saving, closing without a request when nothing changed - Normalize PATCH /me input via usernameSchema (trim + non-empty) - Trim the POST /me/claim-legacy username so copy/pasted credentials still match imported usernames - Reject legacy imports containing usernames with leading or trailing whitespace rather than storing them verbatim
1 parent ea4e11c commit 5e44f8a

5 files changed

Lines changed: 38 additions & 12 deletions

File tree

‎src/lib/components/ChangeUsernameDialog.svelte‎

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,10 @@
1414
let draftUsername = $state("");
1515
let saveError = $state<string | null>(null);
1616
let saving = $state(false);
17+
let trimmedDraftUsername = $derived(draftUsername.trim());
1718
let usernameError = $derived.by(() => {
18-
if (draftUsername.length > 64) return "Username must be 64 characters or fewer.";
19-
if (!draftUsername.trim()) return "Enter a username.";
19+
if (trimmedDraftUsername.length > 64) return "Username must be 64 characters or fewer.";
20+
if (!trimmedDraftUsername) return "Enter a username.";
2021
return undefined;
2122
});
2223
let wasOpen = false;
@@ -29,22 +30,25 @@
2930
wasOpen = open;
3031
});
3132
33+
let unchanged = $derived(trimmedDraftUsername === (auth.username ?? ""));
34+
3235
function close(): void {
3336
if (!saving) open = false;
3437
}
3538
3639
async function save(): Promise<void> {
37-
if (usernameError) {
38-
saveError = null;
40+
saveError = null;
41+
if (unchanged) {
42+
open = false;
3943
return;
4044
}
45+
if (usernameError) return;
4146
4247
saving = true;
43-
saveError = null;
4448
try {
4549
const user = await fetchApi<UserProfile>("/me", {
4650
method: "PATCH",
47-
body: { username: draftUsername.trim() },
51+
body: { username: trimmedDraftUsername },
4852
});
4953
auth.setUser(user);
5054
await Promise.all([

‎src/lib/domain.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import { epochMilliseconds, minecraftVersionSchema, nonEmptyString } from "./sch
33

44
export const PATCH_STATUSES = ["AVAILABLE", "WIP", "DONE"] as const;
55

6-
export const usernameSchema = v.pipe(nonEmptyString, v.maxLength(64));
6+
export const usernameSchema = v.pipe(v.string(), v.trim(), v.nonEmpty(), v.maxLength(64));
77
const nonNegativeInteger = v.pipe(v.number(), v.safeInteger(), v.minValue(0));
88

99
export const patchStatusSchema = v.picklist(PATCH_STATUSES);

‎src/lib/schemas.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import * as v from "valibot";
22

3-
export const nonEmptyString = v.pipe(v.string(), v.minLength(1));
3+
export const nonEmptyString = v.pipe(v.string(), v.nonEmpty());
44
export const minecraftVersionSchema = nonEmptyString;
55
export const epochMilliseconds = v.pipe(
66
v.number(),

‎src/lib/server/api.ts‎

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,13 @@ const createApi = ({ localIdentity, enforceSameOrigin = true }: ApiOptions = {})
5757
});
5858
api.post(
5959
"/me/claim-legacy",
60-
vValidator("json", v.strictObject({ username: nonEmptyString, password: nonEmptyString })),
60+
vValidator(
61+
"json",
62+
v.strictObject({
63+
username: v.pipe(v.string(), v.trim(), v.nonEmpty()),
64+
password: nonEmptyString,
65+
}),
66+
),
6167
async (c) => {
6268
const input = c.req.valid("json");
6369
const result = await database(c.env).claimLegacyUser(currentUser(c).id, input.username, input.password);
@@ -129,7 +135,15 @@ const createApi = ({ localIdentity, enforceSameOrigin = true }: ApiOptions = {})
129135
"json",
130136
v.strictObject({
131137
exportedAt: epochMilliseconds,
132-
legacyUsers: v.array(v.strictObject({ username: nonEmptyString, passwordHash: nonEmptyString })),
138+
legacyUsers: v.array(
139+
v.strictObject({
140+
username: v.pipe(
141+
nonEmptyString,
142+
v.check((username) => username.trim() === username, "Username has leading or trailing whitespace."),
143+
),
144+
passwordHash: nonEmptyString,
145+
}),
146+
),
133147
patches: v.pipe(v.array(patchSchema), v.minLength(1)),
134148
}),
135149
(result, c) => {

‎test/api.test.ts‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,9 +26,9 @@ const exportPatch = (overrides: Record<string, unknown> = {}) => ({
2626
duration: 5_000,
2727
...overrides,
2828
});
29-
const exportPayload = (patches: Record<string, unknown>[]) => ({
29+
const exportPayload = (patches: Record<string, unknown>[], legacyUsers: Record<string, unknown>[] = []) => ({
3030
exportedAt: 1_700_000_000_000,
31-
legacyUsers: [],
31+
legacyUsers,
3232
patches,
3333
});
3434

@@ -227,6 +227,14 @@ describe("Patch Roulette API", () => {
227227
expect((await api("/import-legacy-data", json(payload, "bob"))).status).toBe(409);
228228
});
229229

230+
it("rejects legacy users whose usernames have leading or trailing whitespace", async () => {
231+
const response = await api(
232+
"/import-legacy-data",
233+
json(exportPayload([exportPatch()], [{ username: " old-alice ", passwordHash: hashSync("old-password", 4) }])),
234+
);
235+
expect(response.status).toBe(400);
236+
});
237+
230238
it("rejects patch records with impossible lifecycle fields", async () => {
231239
const invalidPatches = [
232240
exportPatch({ status: "AVAILABLE", responsibleUser: "alice", duration: null }),

0 commit comments

Comments
 (0)