Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 48 additions & 13 deletions clients/dashboard/src/api/identity.ts
Original file line number Diff line number Diff line change
Expand Up @@ -146,10 +146,6 @@ export async function getMyPermissions(): Promise<string[]> {
return (await apiFetch<string[] | null>(`/api/v1/identity/permissions`)) ?? [];
}

export async function getMyProfile(): Promise<UserDto> {
return apiFetch<UserDto>("/api/v1/identity/profile");
}

export async function registerUser(input: RegisterUserInput): Promise<RegisterUserResponse> {
return apiFetch<RegisterUserResponse>(`/api/v1/identity/register`, {
method: "POST",
Expand Down Expand Up @@ -391,22 +387,61 @@ export async function endImpersonation(): Promise<EndImpersonationResponse> {
// -----------------------------

export type UpdateProfileInput = {
firstName?: string | null;
lastName?: string | null;
phoneNumber?: string | null;
/**
* The profile the form was seeded from, and the ETag that read carried. Both come from the
* caller rather than from a read inside the save: the lost update this guards against happens
* between the moment the user saw the values and the moment they press save, so a tag fetched
* inside the save has no chance of being stale and no chance of catching anything.
*/
profile: UserDto;
expectedETag: string | null;
firstName: string | null;
lastName: string | null;
phoneNumber: string | null;
};

/**
* Updates the authenticated user's profile. Maps to UpdateUserCommand
* server-side. Image and email changes go through their own dedicated
* endpoints — this is for the editable profile fields surfaced in
* settings/profile. Reads the current profile first so unset optional
* fields keep their existing values instead of being nulled.
* Reads the profile along with the ETag the server publishes for it. The tag is the
* profile's version marker: echoing it back in `If-Match` on the PUT below is what lets
* the server reject a save built from a snapshot someone else has since changed.
*
* Use this as the read that populates an edit form, and hand the tag it returns back to
* {@link updateMyProfile}. A tag read at save time cannot detect anything.
*/
export async function getMyProfileWithETag(): Promise<{ profile: UserDto; etag: string | null }> {
let etag: string | null = null;
const profile = await apiFetch<UserDto>("/api/v1/identity/profile", {
onResponse: (response) => {
// Strip a `W/` prefix rather than pass it through. The endpoint only ever emits a strong
// validator, so a weak one is an artefact of the transport: a compressing edge (Cloudflare
// does this by default once it re-encodes a response) downgrades the tag it forwards. Sending
// it back as-is means the server drops it under the strong comparison `If-Match` mandates and
// answers 412 forever, which is a profile the user can never save.
const header = response.headers.get("ETag");
etag = header ? header.replace(/^W\//, "") : null;
},
});
return { profile, etag };
}

/**
* Updates the authenticated user's profile. Maps to UpdateUserCommand server-side. Image and
* email changes go through their own dedicated endpoints — this is for the editable profile
* fields surfaced in settings/profile. The unedited fields come off `input.profile`, the copy
* the form was seeded from, so they keep their values instead of being nulled.
*
* The PUT carries `If-Match` with that same copy's ETag, so the server answers 412 when the
* profile moved after the user last saw it, instead of accepting a full representation built
* from a stale snapshot and blanking the concurrent change. A 412 is NOT retried here: the only
* body this function has is the one the user typed against the old values, and resending it
* against a fresh tag performs exactly the overwrite the 412 exists to prevent. The caller
* decides — normally by telling the user the profile changed and asking for a deliberate re-save.
*/
export async function updateMyProfile(input: UpdateProfileInput): Promise<void> {
const profile = await getMyProfile();
const { profile, expectedETag } = input;
await apiFetch<unknown>(`/api/v1/identity/profile`, {
method: "PUT",
headers: expectedETag ? { "If-Match": expectedETag } : undefined,
body: JSON.stringify({
id: profile.id,
firstName: input.firstName ?? profile.firstName ?? null,
Expand Down
10 changes: 6 additions & 4 deletions clients/dashboard/src/components/layout/topbar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ import {
DropdownMenuTrigger,
} from "@/components/ui/dropdown-menu";
import { Avatar } from "@/components/ui/avatar";
import { getMyProfile } from "@/api/identity";
import { getMyProfileWithETag } from "@/api/identity";
import { useAuth } from "@/auth/use-auth";
import { useSseStatus } from "@/sse/sse-context";
import { useTheme } from "@/components/theme/theme-provider";
Expand Down Expand Up @@ -149,13 +149,15 @@ function SimpleMenuItem({
export function Topbar() {
const { user, logout } = useAuth();
// Shared with the Profile settings page (same query key), so changing the
// photo there invalidates this and the topbar avatar updates live.
// photo there invalidates this and the topbar avatar updates live. That sharing is also why
// this reads through the ETag-carrying variant: one query key must hold one shape, and the
// settings page needs the tag to save against.
const { data: profile } = useQuery({
queryKey: ["identity", "me"],
queryFn: getMyProfile,
queryFn: getMyProfileWithETag,
staleTime: 5 * 60 * 1000,
});
const avatarUrl = profile?.imageUrl ?? null;
const avatarUrl = profile?.profile.imageUrl ?? null;
const { status: sseStatus, eventCount } = useSseStatus();
const { mode, setMode } = useTheme();
const { setOpen: setPaletteOpen } = useCommandPalette();
Expand Down
11 changes: 10 additions & 1 deletion clients/dashboard/src/lib/api-client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,13 @@ type RequestInitEx = RequestInit & {
* uploads) should override this explicitly.
*/
timeoutMs?: number;
/**
* Called with the final response before its body is read, so a caller can pick up a
* response header `apiFetch` does not model — the `ETag` on `GET /identity/profile`,
* which a later `PUT` echoes back in `If-Match`. Runs for error responses too, and
* must not throw.
*/
onResponse?: (response: Response) => void;
};

const DEFAULT_TIMEOUT_MS = 30_000;
Expand Down Expand Up @@ -156,7 +163,7 @@ export async function apiFetch<T = unknown>(
path: string,
init: RequestInitEx = {},
): Promise<T> {
const { skipAuth, headers, timeoutMs = DEFAULT_TIMEOUT_MS, signal, ...rest } = init;
const { skipAuth, headers, timeoutMs = DEFAULT_TIMEOUT_MS, signal, onResponse, ...rest } = init;

const mergedHeaders = new Headers(headers);
if (!mergedHeaders.has("Content-Type") && rest.body && typeof rest.body === "string") {
Expand Down Expand Up @@ -218,6 +225,8 @@ export async function apiFetch<T = unknown>(
}
}

onResponse?.(response);

if (!response.ok) {
const problem = await parseError(response);
throw new ApiRequestError(
Expand Down
103 changes: 79 additions & 24 deletions clients/dashboard/src/pages/settings/profile.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@ import { useMutation, useQuery, useQueryClient } from "@tanstack/react-query";
import { Camera, Fingerprint, UserCircle2 } from "lucide-react";
import { toast } from "sonner";
import { useAuth } from "@/auth/use-auth";
import { getMyProfile, setProfileImage, updateMyProfile } from "@/api/identity";
import { getMyProfileWithETag, setProfileImage, updateMyProfile } from "@/api/identity";
import type { UserDto } from "@/api/identity";
import { ApiRequestError } from "@/lib/api-client";
import { Button } from "@/components/ui/button";
import { Input } from "@/components/ui/input";
Expand All @@ -19,11 +20,18 @@ export function ProfileSettings() {

const profileQuery = useQuery({
queryKey: PROFILE_KEY,
queryFn: getMyProfile,
queryFn: getMyProfileWithETag,
});

const profile = profileQuery.data;
const profile = profileQuery.data?.profile;
const loading = profileQuery.isLoading;

// The version this form is editing against, captured when the form is seeded. Deliberately a
// ref and not `profileQuery.data`: a background refetch would otherwise move it to a version
// the user never saw, and the save would carry an If-Match that matches whatever someone else
// just wrote — silently overwriting it, which is exactly what the ETag exists to prevent. It
// moves only on a deliberate step: a successful save, or the user being told about a conflict.
const editingVersionRef = useRef<{ profile: UserDto; etag: string | null } | null>(null);
const [firstName, setFirstName] = useState("");
const [lastName, setLastName] = useState("");
const [phone, setPhone] = useState("");
Expand All @@ -35,29 +43,59 @@ export function ProfileSettings() {
const seededRef = useRef(false);
useEffect(() => {
if (seededRef.current) return;
if (profile) {
setFirstName(profile.firstName ?? "");
setLastName(profile.lastName ?? "");
setPhone(profile.phoneNumber ?? "");
if (profileQuery.data) {
const { profile: seeded } = profileQuery.data;
setFirstName(seeded.firstName ?? "");
setLastName(seeded.lastName ?? "");
setPhone(seeded.phoneNumber ?? "");
editingVersionRef.current = profileQuery.data;
seededRef.current = true;
} else if (user && loading) {
setFirstName(user.name?.split(" ")[0] ?? "");
setLastName(user.name?.split(" ").slice(1).join(" ") ?? "");
}
}, [profile, user, loading]);
}, [profileQuery.data, user, loading]);

// Re-reads the profile and adopts it as the version the form edits against, so the next save
// carries a tag the server will accept.
const adoptCurrentVersion = async () => {
const fresh = await queryClient.fetchQuery({
queryKey: PROFILE_KEY,
queryFn: getMyProfileWithETag,
// Must reach the network. The client's default staleTime would hand back the cached copy,
// and the cached copy carries the very tag the server just rejected.
staleTime: 0,
});
editingVersionRef.current = fresh;
return fresh;
};

const saveMutation = useMutation({
mutationFn: () =>
updateMyProfile({
firstName: firstName.trim() || null,
lastName: lastName.trim() || null,
phoneNumber: phone.trim() || null,
}),
onSuccess: () => {
mutationFn: updateMyProfile,
onSuccess: async () => {
toast.success("Profile saved");
queryClient.invalidateQueries({ queryKey: PROFILE_KEY });
// The save moved the profile on, so the tag the form holds is spent: adopt the new one or
// a second save in the same sitting would 412 against the user's own write. Awaited rather
// than fire-and-forget: isPending has to stay true until the new tag is in hand, or the
// button re-enables over a spent one and a quick second click 412s against the user's own
// save. It also keeps a failed refetch from becoming an unhandled rejection that would
// strand the form on a tag the server has already rejected.
await adoptCurrentVersion();
},
onError: (err: unknown) => {
onError: async (err: unknown) => {
// 412 means someone else wrote the profile after this form was seeded. Do NOT resend: the
// only body available is the one typed against the old values, and pushing it through
// against a fresh tag performs the overwrite the 412 just prevented. Keep the user's
// edits on screen, adopt the current version, and let them decide whether to save again.
if (err instanceof ApiRequestError && err.status === 412) {
await adoptCurrentVersion();
toast.warning("Profile changed elsewhere", {
description:
"Someone updated this profile while you were editing. Review your changes and save again to apply them.",
});
return;
}

const message =
err instanceof ApiRequestError
? err.problem?.detail ?? err.problem?.title ?? err.message
Expand All @@ -68,7 +106,17 @@ export function ProfileSettings() {

const onSubmit = (e: FormEvent<HTMLFormElement>) => {
e.preventDefault();
saveMutation.mutate();
const editing = editingVersionRef.current;
if (!editing) return;
// Everything the save needs travels through mutate(), never through state the callbacks
// close over: the values that go out must be the ones on screen when the button was pressed.
saveMutation.mutate({
profile: editing.profile,
expectedETag: editing.etag,
firstName: firstName.trim() || null,
lastName: lastName.trim() || null,
phoneNumber: phone.trim() || null,
});
};

const onReset = () => {
Expand All @@ -84,12 +132,19 @@ export function ProfileSettings() {
(profile?.firstName ?? "") !== firstName ||
(profile?.lastName ?? "") !== lastName ||
(profile?.phoneNumber ?? "") !== phone;
// A save carries the unedited fields and the version tag off the profile read, so until that
// read lands there is nothing to save against. Disabled rather than silently doing nothing.
const canSave = profileQuery.isSuccess;

const imageMutation = useMutation({
mutationFn: (url: string | null) => setProfileImage(url),
onSuccess: () => {
onSuccess: async () => {
toast.success("Profile image updated");
queryClient.invalidateQueries({ queryKey: PROFILE_KEY });
// Setting the image is a second write to the same row, so ASP.NET Identity rotates the
// concurrency stamp and the tag this form is holding is spent. Adopting the new version (which
// also refreshes the cache, so the topbar avatar still updates) keeps the next save from
// answering 412 and telling the user someone else edited their profile.
await adoptCurrentVersion();
},
onError: (e: unknown) => {
const message =
Expand All @@ -108,8 +163,8 @@ export function ProfileSettings() {
className="flex items-start gap-2 rounded-lg border border-[oklch(from_var(--color-destructive)_l_c_h_/_0.30)] bg-[oklch(from_var(--color-destructive)_l_c_h_/_0.06)] px-3 py-2 text-[13px] text-[var(--color-destructive)]"
>
<span>
Couldn't load your profile. Showing details from your session;
saved changes may not reflect the latest server state.
Couldn't load your profile. Showing details from your session; saving is disabled
until the profile loads, because a save has to carry the version it was read at.
</span>
</div>
)}
Expand Down Expand Up @@ -137,12 +192,12 @@ export function ProfileSettings() {
type="button"
variant="ghost"
onClick={onReset}
disabled={saving || !dirty}
disabled={saving || !dirty || !canSave}
size="sm"
>
Reset
</Button>
<Button type="submit" disabled={saving || !dirty} size="sm">
<Button type="submit" disabled={saving || !dirty || !canSave} size="sm">
{saving ? "Saving…" : "Save changes"}
</Button>
</div>
Expand Down
7 changes: 4 additions & 3 deletions clients/dashboard/src/pages/settings/security.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ import {
changePassword,
disableTwoFactor,
enrollTwoFactor,
getMyProfile,
getMyProfileWithETag,
verifyEnrollTwoFactor,
type TwoFactorEnrollmentResponse,
} from "@/api/identity";
Expand Down Expand Up @@ -95,8 +95,9 @@ function apiErrorMessage(err: unknown, fallback: string): string {
export function SecuritySettings() {
const queryClient = useQueryClient();

const profileQuery = useQuery({ queryKey: PROFILE_KEY, queryFn: getMyProfile });
const twoFactorEnabled = profileQuery.data?.twoFactorEnabled ?? false;
// Same key as the topbar and the profile page, so same shape: the read carries the ETag.
const profileQuery = useQuery({ queryKey: PROFILE_KEY, queryFn: getMyProfileWithETag });
const twoFactorEnabled = profileQuery.data?.profile.twoFactorEnabled ?? false;

const sessionsQuery = useQuery({
queryKey: ["identity", "sessions", "me"],
Expand Down
Loading
Loading