v4: users can't rename themselves, /settings lays out again, and its selectors fit a phone — PATCH /api/me refuses a username with a 400 and changes nothing else in that request; a name changes only through PATCH /api/users/:key/username, which needs user:manage, from /@name/manage. The old name still stays an alias for good. renameUser lost its 30-day limit and its exempt flag, and RENAME_LIMITS holds the slug limit alone. /settings shows the username as plain text, and the rename hint left the catalogs. The settings grid sized its control column to its widest content, the rename hint, which stretched every control under it; that column now takes the remaining width and each control keeps its own size. A segmented control wraps instead of overflowing; in a settings row on a phone it takes the full width, and one of more than 4 options (the styles) lays out in two even rows. .btn-border turned red on hover, so Settings, Manage, Retry and Back looked like Log out; it is now neutral (Smooth's accent is red too), and only Log out takes the new btn-border--danger, muted until hovered red. A <button> and a <Link> .btn-border are now the same height. API.md and the module docs say who renames; the backend tests rename through the moderator route and check a self-rename is refused, and the malformed-name check moved to the manage tab's test. 254 backend and 667 frontend tests pass, and tsc, deno check and doc:lint are clean; Smooth, Brutalist, GeoCities and Kanagawa were checked in screenshots at desktop and phone widths.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
@@ -6,9 +6,9 @@
|
||||
* `routes/avatars.ts`.
|
||||
*
|
||||
* - `GET /api/me` — signed in. Answers the caller's own user.
|
||||
* - `PATCH /api/me` — signed in. Changes the caller's description, email and
|
||||
* username, and answers their user. A new username keeps the old one as an
|
||||
* alias, at most once in 30 days but for those with `user:manage`.
|
||||
* - `PATCH /api/me` — signed in. Changes the caller's description and email,
|
||||
* and answers their user. A username changes only by `user:manage`, through
|
||||
* `PATCH /api/users/:key/username`.
|
||||
* - `GET /api/me/media-token` — signed in. Answers a media token, for a
|
||||
* session that predates them.
|
||||
* - `GET /api/me/settings` — signed in. Answers the caller's settings.
|
||||
@@ -38,19 +38,11 @@ import { isUpdateSettingsRequest } from "../validators/settings.ts";
|
||||
import { toPublicUser, toSelfUser } from "../model/user.ts";
|
||||
import { createMediaToken, issueTokens } from "../lib/jwt.ts";
|
||||
import { verifyPassword } from "../lib/password.ts";
|
||||
import { can } from "../lib/permissions.ts";
|
||||
import { type AuthContext, authMiddleware } from "../middleware/auth.ts";
|
||||
import {
|
||||
getUserById,
|
||||
renameUser,
|
||||
updateUser,
|
||||
} from "../services/user-service.ts";
|
||||
import { getUserById, updateUser } from "../services/user-service.ts";
|
||||
import { getSettings, updateSettings } from "../services/settings-service.ts";
|
||||
import { getPlaylistMembershipsForDump } from "../services/playlist-service.ts";
|
||||
import {
|
||||
broadcastUserUpdated,
|
||||
updateClientUsername,
|
||||
} from "../services/ws-service.ts";
|
||||
import { broadcastUserUpdated } from "../services/ws-service.ts";
|
||||
import { VALIDATION } from "../../shared/validation.ts";
|
||||
|
||||
/**
|
||||
@@ -74,16 +66,7 @@ router.patch("/", authMiddleware, async (ctx: AuthContext) => {
|
||||
}
|
||||
const userId = ctx.state.user.userId;
|
||||
// Only these fields: a stolen token alone must not change the role or the
|
||||
// password, which needs the current one. The rename first: it is the change
|
||||
// most likely refused, and a refusal should leave the rest as it was.
|
||||
if (body.username !== undefined) {
|
||||
const renamed = renameUser(
|
||||
userId,
|
||||
body.username,
|
||||
can(ctx.state.user, "user:manage"),
|
||||
);
|
||||
updateClientUsername(userId, renamed.username);
|
||||
}
|
||||
// password, which needs the current one.
|
||||
const updated = await updateUser(userId, {
|
||||
description: body.description,
|
||||
email: body.email,
|
||||
|
||||
@@ -31,7 +31,7 @@
|
||||
* - `PATCH /api/users/:key/role` — permission `user:manage`. Sets another
|
||||
* user's role.
|
||||
* - `PATCH /api/users/:key/username` — permission `user:manage`. Renames a
|
||||
* user, with no rate limit.
|
||||
* user, the caller included. It is the only way a name changes.
|
||||
*
|
||||
* @module
|
||||
*/
|
||||
@@ -293,7 +293,7 @@ router.patch(
|
||||
throw new APIException(400, "Invalid request");
|
||||
}
|
||||
const user = resolveUserKey(ctx.params.key);
|
||||
const publicUser = toPublicUser(renameUser(user.id, username, true));
|
||||
const publicUser = toPublicUser(renameUser(user.id, username));
|
||||
updateClientUsername(publicUser.id, publicUser.username);
|
||||
broadcastUserUpdated(publicUser);
|
||||
const responseBody: APIResponse<PublicUser> = {
|
||||
|
||||
@@ -32,11 +32,7 @@ import { LIKE, likePattern } from "../lib/like.ts";
|
||||
import { parseRows } from "../model/rows.ts";
|
||||
import { isEmailEnabled, sendEmail } from "./email-service.ts";
|
||||
import { renderWelcomeEmail } from "../mail/welcome.ts";
|
||||
import {
|
||||
RENAME_LIMITS,
|
||||
USERNAME_RE,
|
||||
VALIDATION,
|
||||
} from "../../shared/validation.ts";
|
||||
import { USERNAME_RE, VALIDATION } from "../../shared/validation.ts";
|
||||
|
||||
/** Selects full `UserRow`s from `users u`, with the inviter's name joined. */
|
||||
const USER_SELECT =
|
||||
@@ -206,24 +202,18 @@ export function resolveUserKey(key: string): User {
|
||||
}
|
||||
|
||||
/**
|
||||
* Renames user `userId` to `username`, for `PATCH /api/me` and, by a
|
||||
* moderator, `PATCH /api/users/:key/username`, and returns them renamed. The
|
||||
* old name becomes an alias of theirs for good, so `/@old/…` still reaches
|
||||
* them and nobody else may take it; a change of case alone leaves none, since
|
||||
* names are looked up in any case. Taking back a former name of one's own is
|
||||
* allowed. Their dumps' and playlists' slugs belong to their id, so their
|
||||
* paths follow the new name. Unless `exempt` (a moderator), a user may change
|
||||
* names once in `RENAME_LIMITS.USERNAME`'s window.
|
||||
* Renames user `userId` to `username`, for `PATCH /api/users/:key/username`,
|
||||
* which only `user:manage` may call: users can't rename themselves. Returns
|
||||
* them renamed. The old name becomes an alias of theirs for good, so
|
||||
* `/@old/…` still reaches them and nobody else may take it; a change of case
|
||||
* alone leaves none, since names are looked up in any case. Taking back a
|
||||
* former name of their own is allowed. Their dumps' and playlists' slugs
|
||||
* belong to their id, so their paths follow the new name.
|
||||
*
|
||||
* @throws {APIException} 400 when `username` isn't a valid name, 404 when
|
||||
* there is no such user, 409 when the name is or was someone else's, 429 when
|
||||
* the user changed names too recently.
|
||||
* there is no such user, 409 when the name is or was someone else's.
|
||||
*/
|
||||
export function renameUser(
|
||||
userId: string,
|
||||
username: string,
|
||||
exempt: boolean,
|
||||
): User {
|
||||
export function renameUser(userId: string, username: string): User {
|
||||
if (!USERNAME_RE.test(username)) {
|
||||
throw new APIException(
|
||||
400,
|
||||
@@ -233,33 +223,19 @@ export function renameUser(
|
||||
const user = getUserById(userId);
|
||||
if (username === user.username) return user;
|
||||
|
||||
const now = new Date();
|
||||
const now = new Date().toISOString();
|
||||
transaction(() => {
|
||||
assertUsernameAvailable(username, userId);
|
||||
if (username.toLowerCase() !== user.username.toLowerCase()) {
|
||||
const { count, windowMs } = RENAME_LIMITS.USERNAME;
|
||||
const recent = (db.prepare(
|
||||
`SELECT COUNT(*) AS n FROM user_aliases WHERE user_id = ? AND created_at > ?;`,
|
||||
).get(userId, new Date(now.getTime() - windowMs).toISOString()) as {
|
||||
n: number;
|
||||
}).n;
|
||||
if (!exempt && recent >= count) {
|
||||
throw new APIException(
|
||||
429,
|
||||
`A username can only be changed once every ${
|
||||
windowMs / 86_400_000
|
||||
} days`,
|
||||
);
|
||||
}
|
||||
db.prepare(
|
||||
`DELETE FROM user_aliases WHERE user_id = ? AND lower(username) = lower(?);`,
|
||||
).run(userId, username);
|
||||
db.prepare(
|
||||
`INSERT INTO user_aliases (username, user_id, created_at) VALUES (?, ?, ?);`,
|
||||
).run(user.username, userId, now.toISOString());
|
||||
).run(user.username, userId, now);
|
||||
}
|
||||
db.prepare(`UPDATE users SET username = ?, updated_at = ? WHERE id = ?;`)
|
||||
.run(username, now.toISOString(), userId);
|
||||
.run(username, now, userId);
|
||||
});
|
||||
return getUserById(userId);
|
||||
}
|
||||
@@ -342,10 +318,9 @@ export function searchUsersGlobal(query: string, limit: number): PublicUser[] {
|
||||
* Applies `request` to user `userId` and returns the updated user. It trusts
|
||||
* `request`: the routes pick the fields a caller may change. The username
|
||||
* changes through {@link renameUser} alone, which keeps the old one as an
|
||||
* alias, so a `username` here is ignored. A new password
|
||||
* also ends every session opened before it and closes the user's sockets, so
|
||||
* the caller must issue fresh tokens if the user is to stay signed in. A
|
||||
* `description` of null keeps the current one.
|
||||
* alias. A new password also ends every session opened before it and closes
|
||||
* the user's sockets, so the caller must issue fresh tokens if the user is to
|
||||
* stay signed in. A `description` of null keeps the current one.
|
||||
*
|
||||
* @throws {APIException} 404 when there is no such user, 409 when the new
|
||||
* email belongs to another account.
|
||||
@@ -356,8 +331,7 @@ export async function updateUser(
|
||||
): Promise<User> {
|
||||
const user = getUserById(userId);
|
||||
|
||||
const { password, description, email, username: _, ...requestFields } =
|
||||
request;
|
||||
const { password, description, email, ...requestFields } = request;
|
||||
if (email !== undefined && email !== user.email) {
|
||||
assertEmailAvailable(email, userId);
|
||||
}
|
||||
|
||||
@@ -23,7 +23,12 @@ import type {
|
||||
SelfUser,
|
||||
UserSuggestion,
|
||||
} from "../../../shared/user.ts";
|
||||
import { assert, assertEquals, assertFalse } from "@std/assert";
|
||||
import {
|
||||
assert,
|
||||
assertEquals,
|
||||
assertFalse,
|
||||
assertNotEquals,
|
||||
} from "@std/assert";
|
||||
import {
|
||||
createPasswordResetToken,
|
||||
verifyJWT,
|
||||
@@ -69,7 +74,7 @@ test("public profile: by @name in any case or id, without private fields", async
|
||||
assertFalse("passwordHash" in me.data);
|
||||
});
|
||||
|
||||
test("PATCH /api/me: description, email and username only", async () => {
|
||||
test("PATCH /api/me: description and email only", async () => {
|
||||
const user = await createUser();
|
||||
const taken = await createUser();
|
||||
const res = await patch<SelfUser>("/api/me", {
|
||||
@@ -618,17 +623,19 @@ test("settings: signed in only, and a bad value or unknown key is refused", asyn
|
||||
// ── Usernames and paths ──────────────────────────────────────────────────────
|
||||
|
||||
test("renaming: the old name redirects for good, and nobody else can take it", async () => {
|
||||
const admin = await createUser({ role: "admin" });
|
||||
const bob = await createUser();
|
||||
const old = bob.username;
|
||||
const renamed = uniqueName("robert");
|
||||
const dump = insertDump({ userId: bob.id, title: "Never gonna" });
|
||||
const playlist = insertPlaylist({ userId: bob.id, title: "Sunday mix" });
|
||||
const rename = (key: string, username: string) =>
|
||||
patch<PublicUser>(`/api/users/${key}/username`, {
|
||||
auth: admin.auth,
|
||||
json: { username },
|
||||
});
|
||||
|
||||
const res = await patch<SelfUser>("/api/me", {
|
||||
auth: bob.auth,
|
||||
json: { username: renamed },
|
||||
});
|
||||
assertEquals(res.data.username, renamed);
|
||||
assertEquals((await rename(bob.id, renamed)).data.username, renamed);
|
||||
|
||||
// The old name, in any case, still answers the user, under the new one.
|
||||
for (const name of [old, old.toUpperCase(), renamed]) {
|
||||
@@ -648,7 +655,7 @@ test("renaming: the old name redirects for good, and nobody else can take it", a
|
||||
assertEquals([p.data.id, p.data.ownerUsername], [playlist.id, renamed]);
|
||||
}
|
||||
|
||||
// Nobody can register or rename to the old name, in any case.
|
||||
// Nobody can register or be renamed to the old name, in any case.
|
||||
const other = await createUser();
|
||||
const register = await post("/api/users", {
|
||||
json: {
|
||||
@@ -659,19 +666,9 @@ test("renaming: the old name redirects for good, and nobody else can take it", a
|
||||
},
|
||||
});
|
||||
assertEquals(register.status, 409);
|
||||
assertEquals(
|
||||
(await patch("/api/me", { auth: other.auth, json: { username: old } }))
|
||||
.status,
|
||||
409,
|
||||
);
|
||||
assertEquals((await rename(other.id, old)).status, 409);
|
||||
// Nor to a current one.
|
||||
assertEquals(
|
||||
(await patch("/api/me", {
|
||||
auth: other.auth,
|
||||
json: { username: renamed },
|
||||
})).status,
|
||||
409,
|
||||
);
|
||||
assertEquals((await rename(other.id, renamed)).status, 409);
|
||||
// Signing in takes the current name only.
|
||||
assertEquals(
|
||||
(await post("/api/sessions", {
|
||||
@@ -681,60 +678,43 @@ test("renaming: the old name redirects for good, and nobody else can take it", a
|
||||
);
|
||||
});
|
||||
|
||||
test("renaming: once in 30 days, a case change aside, and a moderator is exempt", async () => {
|
||||
test("renaming: only by user:manage, at any time, and never by oneself", async () => {
|
||||
const user = await createUser();
|
||||
const first = uniqueName("first");
|
||||
assertEquals(
|
||||
(await patch("/api/me", { auth: user.auth, json: { username: first } }))
|
||||
.status,
|
||||
200,
|
||||
);
|
||||
// A change of case alone leaves no alias, so it isn't counted.
|
||||
assertEquals(
|
||||
(await patch<SelfUser>("/api/me", {
|
||||
auth: user.auth,
|
||||
json: { username: first.toUpperCase() },
|
||||
})).data.username,
|
||||
first.toUpperCase(),
|
||||
);
|
||||
const again = await patch("/api/me", {
|
||||
// PATCH /api/me refuses a name, and changes nothing else with it.
|
||||
const self = await patch("/api/me", {
|
||||
auth: user.auth,
|
||||
json: { username: uniqueName("second") },
|
||||
json: { username: uniqueName("self"), description: "hi" },
|
||||
});
|
||||
assertEquals(again.status, 429);
|
||||
assertEquals(
|
||||
(await patch("/api/me", { auth: user.auth, json: { username: "a b" } }))
|
||||
.status,
|
||||
400,
|
||||
);
|
||||
assertEquals(self.status, 400);
|
||||
const me = await get<SelfUser>("/api/me", { auth: user.auth });
|
||||
assertEquals(me.data.username, user.username);
|
||||
assertNotEquals(me.data.description, "hi");
|
||||
|
||||
// An admin renames anyone at any time, and themselves too.
|
||||
// An admin renames anyone at any time, themselves too.
|
||||
const admin = await createUser({ role: "admin" });
|
||||
const third = uniqueName("third");
|
||||
assertEquals(
|
||||
(await patch<PublicUser>(`/api/users/${user.id}/username`, {
|
||||
const rename = (key: string, username: string) =>
|
||||
patch<PublicUser>(`/api/users/${key}/username`, {
|
||||
auth: admin.auth,
|
||||
json: { username: third },
|
||||
})).data.username,
|
||||
third,
|
||||
json: { username },
|
||||
});
|
||||
const first = uniqueName("first");
|
||||
const second = uniqueName("second");
|
||||
assertEquals((await rename(user.id, first)).data.username, first);
|
||||
assertEquals((await rename(user.id, second)).data.username, second);
|
||||
// A change of case alone leaves no alias.
|
||||
assertEquals(
|
||||
(await rename(`@${second}`, second.toUpperCase())).data.username,
|
||||
second.toUpperCase(),
|
||||
);
|
||||
assertEquals(
|
||||
(await patch<PublicUser>(`/api/users/@${third}/username`, {
|
||||
auth: admin.auth,
|
||||
json: { username: first },
|
||||
})).data.username,
|
||||
(await rename(`@${first}`, first)).data.username,
|
||||
first,
|
||||
"a former name of one's own can be taken back",
|
||||
);
|
||||
for (let i = 0; i < 2; i++) {
|
||||
assertEquals(
|
||||
(await patch("/api/me", {
|
||||
auth: admin.auth,
|
||||
json: { username: uniqueName("admin") },
|
||||
})).status,
|
||||
200,
|
||||
);
|
||||
}
|
||||
assertEquals((await rename(user.id, "a b")).status, 400);
|
||||
const adminName = uniqueName("admin");
|
||||
assertEquals((await rename(admin.id, adminName)).data.username, adminName);
|
||||
|
||||
// Others may not.
|
||||
const moderator = await createUser({ role: "moderator" });
|
||||
assertEquals(
|
||||
|
||||
@@ -44,7 +44,8 @@ Deno.test("isUpdateUserRequest", () => {
|
||||
assert(isUpdateUserRequest({ description: "hi", email: "a@b.co" }));
|
||||
assertFalse(isUpdateUserRequest({ description: "x".repeat(2001) }));
|
||||
assertFalse(isUpdateUserRequest({ email: "nope" }));
|
||||
assertFalse(isUpdateUserRequest({ username: "no spaces" }));
|
||||
// A name changes only through a moderator's PATCH /api/users/:key/username.
|
||||
assertFalse(isUpdateUserRequest({ username: "well_formed" }));
|
||||
assertFalse(isUpdateUserRequest({ password: "short" }));
|
||||
assertFalse(isUpdateUserRequest({ role: "root" }));
|
||||
assertFalse(isUpdateUserRequest("string"));
|
||||
|
||||
@@ -81,10 +81,9 @@ export function validateRegisterUserRequest(obj: unknown): string | null {
|
||||
export function isUpdateUserRequest(obj: unknown): obj is UpdateUserRequest {
|
||||
if (!obj || typeof obj !== "object") return false;
|
||||
const o = obj as Record<string, unknown>;
|
||||
if ("username" in o) {
|
||||
if (typeof o.username !== "string") return false;
|
||||
if (!USERNAME_RE.test(o.username as string)) return false;
|
||||
}
|
||||
// A user can't rename themselves: a moderator does, through
|
||||
// `PATCH /api/users/:key/username`.
|
||||
if ("username" in o) return false;
|
||||
if ("password" in o) {
|
||||
if (typeof o.password !== "string") return false;
|
||||
const len = (o.password as string).length;
|
||||
|
||||
Reference in New Issue
Block a user