From efd93027ce53b41dbf645b5fa86103e4effa29bd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?McKayla=20=E3=81=AF=E3=81=AA?= Date: Fri, 26 Jun 2026 16:35:03 -0600 Subject: [PATCH] feat: allow editing user avatars (#26652) Adds an avatar URL field to the admin **Edit user** page, available only for users whose login type is `password` or `none`. For identity-provider login types (`github`, `oidc`) the avatar is synced from the IdP on every login, so the field is hidden and the API ignores any submitted avatar to avoid confusing overwrites. The field reuses the same emoji picker + URL input (`IconField`) already used for template, group, and organization icons. A follow-up PR will add the same control to the self-service Account settings page.
Implementation plan & decisions **Goal:** Let an admin set/clear a user's avatar from the Edit user page, gated to `password`/`none` login types. **Backend** - Add `avatar_url` to `codersdk.UpdateUserProfileRequest`. - `putUserProfile` applies the submitted avatar only for `password`/`none`; otherwise it preserves the existing (IdP-synced) value. - Regenerated TS types and API docs via `make gen`. **Frontend** - `EditUserForm` renders an `IconField` ("Avatar URL") when the login type allows it. - `EditUserPage` passes the avatar value and a `canEditAvatar` flag. - `AccountPage` round-trips `avatar_url` so the shared request type doesn't wipe avatars on the self-service path. **Gating** is enforced in both the UI (field hidden) and the backend (submitted value ignored for IdP login types). **Tests/stories:** backend `TestUpdateUserProfile` covers apply (password) and ignore (SSO); `EditUserForm` stories cover the shown/hidden states with interaction tests.
--- > Generated by Coder Agents on behalf of @aslilac. --- coderd/apidoc/docs.go | 5 ++ coderd/apidoc/swagger.json | 5 ++ coderd/users.go | 10 ++- coderd/users_test.go | 62 +++++++++++++++++++ codersdk/users.go | 4 ++ docs/reference/api/schemas.md | 10 +-- docs/reference/api/users.md | 1 + site/src/api/typesGenerated.ts | 6 ++ .../EditUserPage/EditUserForm.stories.tsx | 38 ++++++++++++ site/src/pages/EditUserPage/EditUserForm.tsx | 15 +++++ site/src/pages/EditUserPage/EditUserPage.tsx | 4 ++ .../AccountPage/AccountForm.stories.tsx | 1 + .../AccountPage/AccountForm.test.tsx | 3 + .../AccountPage/AccountPage.test.tsx | 4 +- .../AccountPage/AccountPage.tsx | 6 +- site/src/theme/externalImages.test.ts | 10 +++ site/src/theme/externalImages.ts | 10 ++- 17 files changed, 185 insertions(+), 9 deletions(-) diff --git a/coderd/apidoc/docs.go b/coderd/apidoc/docs.go index d31e7468a7..fdbdb9f46d 100644 --- a/coderd/apidoc/docs.go +++ b/coderd/apidoc/docs.go @@ -25192,6 +25192,11 @@ const docTemplate = `{ "username" ], "properties": { + "avatar_url": { + "description": "AvatarURL is only applied for users whose login type is password or\nnone. For other login types the avatar is synced from the identity\nprovider on login, so a submitted value is ignored.", + "type": "string", + "format": "uri" + }, "name": { "type": "string" }, diff --git a/coderd/apidoc/swagger.json b/coderd/apidoc/swagger.json index 6c7ba2819e..93bfcc07bd 100644 --- a/coderd/apidoc/swagger.json +++ b/coderd/apidoc/swagger.json @@ -23153,6 +23153,11 @@ "type": "object", "required": ["username"], "properties": { + "avatar_url": { + "description": "AvatarURL is only applied for users whose login type is password or\nnone. For other login types the avatar is synced from the identity\nprovider on login, so a submitted value is ignored.", + "type": "string", + "format": "uri" + }, "name": { "type": "string" }, diff --git a/coderd/users.go b/coderd/users.go index 8815b6edb0..d067a63734 100644 --- a/coderd/users.go +++ b/coderd/users.go @@ -917,11 +917,19 @@ func (api *API) putUserProfile(rw http.ResponseWriter, r *http.Request) { return } + // Avatars for password and none login types are managed manually. For + // other login types the avatar is synced from the identity provider on + // login, so we preserve the existing value and ignore any submitted one. + avatarURL := user.AvatarURL + if user.LoginType == database.LoginTypePassword || user.LoginType == database.LoginTypeNone { + avatarURL = params.AvatarURL + } + updatedUserProfile, err := api.Database.UpdateUserProfile(ctx, database.UpdateUserProfileParams{ ID: user.ID, Email: user.Email, Name: params.Name, - AvatarURL: user.AvatarURL, + AvatarURL: avatarURL, Username: params.Username, UpdatedAt: dbtime.Now(), }) diff --git a/coderd/users_test.go b/coderd/users_test.go index fd4d5e6ec3..2893c7126b 100644 --- a/coderd/users_test.go +++ b/coderd/users_test.go @@ -1333,6 +1333,68 @@ func TestUpdateUserProfile(t *testing.T) { require.ErrorAs(t, err, &apiErr) require.Equal(t, http.StatusBadRequest, apiErr.StatusCode()) }) + + t.Run("UpdateAvatar", func(t *testing.T) { + t.Parallel() + client := coderdtest.New(t, nil) + coderdtest.CreateFirstUser(t, client) + + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) + defer cancel() + + me, err := client.User(ctx, codersdk.Me) + require.NoError(t, err) + + // The first user is a password user, so the avatar is editable. + const newAvatar = "/emojis/1f600.png" + userProfile, err := client.UpdateUserProfile(ctx, codersdk.Me, codersdk.UpdateUserProfileRequest{ + Username: me.Username, + Name: me.Name, + AvatarURL: newAvatar, + }) + require.NoError(t, err) + require.Equal(t, newAvatar, userProfile.AvatarURL) + }) + + t.Run("IgnoresAvatarForSSOUser", func(t *testing.T) { + t.Parallel() + client, db := coderdtest.NewWithDatabase(t, nil) + // The first user is an owner and can update other users' profiles. + coderdtest.CreateFirstUser(t, client) + + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) + defer cancel() + + // Avatars for SSO users are synced from the identity provider on login, + // so a submitted avatar must be ignored and the existing one preserved. + ssoUser := dbgen.User(t, db, database.User{ + Email: "sso-avatar@coder.com", + Username: "sso-avatar", + LoginType: database.LoginTypeOIDC, + }) + + // dbgen.User does not persist the avatar at creation, so set it directly + // to emulate an avatar synced from the identity provider. + const idpAvatar = "https://idp.example.com/avatar.png" + //nolint:gocritic // Test setup requires a system context to set the avatar. + ssoUser, err := db.UpdateUserProfile(dbauthz.AsSystemRestricted(ctx), database.UpdateUserProfileParams{ + ID: ssoUser.ID, + Email: ssoUser.Email, + Name: ssoUser.Name, + AvatarURL: idpAvatar, + Username: ssoUser.Username, + UpdatedAt: dbtime.Now(), + }) + require.NoError(t, err) + + userProfile, err := client.UpdateUserProfile(ctx, ssoUser.ID.String(), codersdk.UpdateUserProfileRequest{ + Username: ssoUser.Username, + Name: ssoUser.Name, + AvatarURL: "/emojis/1f600.png", + }) + require.NoError(t, err) + require.Equal(t, idpAvatar, userProfile.AvatarURL) + }) } func TestUpdateUserPassword(t *testing.T) { diff --git a/codersdk/users.go b/codersdk/users.go index 341b56cb5b..351f20d2a5 100644 --- a/codersdk/users.go +++ b/codersdk/users.go @@ -224,6 +224,10 @@ func (r *CreateUserRequestWithOrgs) UnmarshalJSON(data []byte) error { type UpdateUserProfileRequest struct { Username string `json:"username" validate:"required,username"` Name string `json:"name" validate:"user_real_name"` + // AvatarURL is only applied for users whose login type is password or + // none. For other login types the avatar is synced from the identity + // provider on login, so a submitted value is ignored. + AvatarURL string `json:"avatar_url" format:"uri"` } type ValidateUserPasswordRequest struct { diff --git a/docs/reference/api/schemas.md b/docs/reference/api/schemas.md index d640ba77df..4e4a49745c 100644 --- a/docs/reference/api/schemas.md +++ b/docs/reference/api/schemas.md @@ -13436,6 +13436,7 @@ Restarts will only happen on weekdays in this list on weeks which line up with W ```json { + "avatar_url": "http://example.com", "name": "string", "username": "string" } @@ -13443,10 +13444,11 @@ Restarts will only happen on weekdays in this list on weeks which line up with W ### Properties -| Name | Type | Required | Restrictions | Description | -|------------|--------|----------|--------------|-------------| -| `name` | string | false | | | -| `username` | string | true | | | +| Name | Type | Required | Restrictions | Description | +|--------------|--------|----------|--------------|---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------| +| `avatar_url` | string | false | | Avatar URL is only applied for users whose login type is password or none. For other login types the avatar is synced from the identity provider on login, so a submitted value is ignored. | +| `name` | string | false | | | +| `username` | string | true | | | ## codersdk.UpdateUserQuietHoursScheduleRequest diff --git a/docs/reference/api/users.md b/docs/reference/api/users.md index 1ba07d48b4..2c044e3b13 100644 --- a/docs/reference/api/users.md +++ b/docs/reference/api/users.md @@ -1406,6 +1406,7 @@ curl -X PUT http://coder-server:8080/api/v2/users/{user}/profile \ ```json { + "avatar_url": "http://example.com", "name": "string", "username": "string" } diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index f7a2c67b74..cd7ca43186 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -9299,6 +9299,12 @@ export interface UpdateUserPreferenceSettingsRequest { export interface UpdateUserProfileRequest { readonly username: string; readonly name: string; + /** + * AvatarURL is only applied for users whose login type is password or + * none. For other login types the avatar is synced from the identity + * provider on login, so a submitted value is ignored. + */ + readonly avatar_url: string; } // From codersdk/users.go diff --git a/site/src/pages/EditUserPage/EditUserForm.stories.tsx b/site/src/pages/EditUserPage/EditUserForm.stories.tsx index f26290c913..54ecf57107 100644 --- a/site/src/pages/EditUserPage/EditUserForm.stories.tsx +++ b/site/src/pages/EditUserPage/EditUserForm.stories.tsx @@ -1,5 +1,6 @@ import type { Meta, StoryObj } from "@storybook/react-vite"; import { action } from "storybook/actions"; +import { expect, userEvent, within } from "storybook/test"; import { mockApiError } from "#/testHelpers/entities"; import { EditUserForm } from "./EditUserForm"; @@ -10,9 +11,11 @@ const meta: Meta = { onCancel: action("cancel"), onSubmit: action("submit"), isLoading: false, + canEditAvatar: true, initialValues: { username: "john-doe", name: "John Doe", + avatar_url: "", }, }, }; @@ -27,10 +30,45 @@ export const NoDisplayName: Story = { initialValues: { username: "jane-doe", name: "", + avatar_url: "", }, }, }; +export const WithAvatar: Story = { + args: { + initialValues: { + username: "john-doe", + name: "John Doe", + avatar_url: "/emojis/1f600.png", + }, + }, +}; + +export const EditAvatar: Story = { + play: async ({ canvasElement }) => { + const canvas = within(canvasElement); + const field = canvas.getByLabelText("Avatar URL"); + await userEvent.clear(field); + // Typing happens one character at a time, so the value passes through + // incomplete states like "https:" that must not crash the preview. + await userEvent.type(field, "https://example.com/avatar.png"); + await expect(field).toHaveValue("https://example.com/avatar.png"); + }, +}; + +// The avatar field is hidden for login types whose avatar is synced from an +// identity provider (e.g. github, oidc). +export const CannotEditAvatar: Story = { + args: { + canEditAvatar: false, + }, + play: async ({ canvasElement }) => { + const canvas = within(canvasElement); + await expect(canvas.queryByLabelText("Avatar URL")).not.toBeInTheDocument(); + }, +}; + export const FormError: Story = { args: { error: mockApiError({ diff --git a/site/src/pages/EditUserPage/EditUserForm.tsx b/site/src/pages/EditUserPage/EditUserForm.tsx index c79db6205d..6718ca7ce7 100644 --- a/site/src/pages/EditUserPage/EditUserForm.tsx +++ b/site/src/pages/EditUserPage/EditUserForm.tsx @@ -8,6 +8,7 @@ import { Button } from "#/components/Button/Button"; import { FormFooter } from "#/components/Form/Form"; import { FormField } from "#/components/FormField/FormField"; import { FullPageForm } from "#/components/FullPageForm/FullPageForm"; +import { IconField } from "#/components/IconField/IconField"; import { Spinner } from "#/components/Spinner/Spinner"; import { displayNameValidator, @@ -19,12 +20,15 @@ import { const validationSchema = Yup.object({ username: nameValidator("Username"), name: displayNameValidator("Full name"), + avatar_url: Yup.string(), }); interface EditUserFormProps { error?: unknown; isLoading: boolean; initialValues: UpdateUserProfileRequest; + /** Allows hiding the avatar setting when it would be overwritten later by the user's identity provider. */ + canEditAvatar: boolean; onSubmit: (values: UpdateUserProfileRequest) => void; onCancel: () => void; } @@ -33,6 +37,7 @@ export const EditUserForm: FC = ({ error, isLoading, initialValues, + canEditAvatar, onSubmit, onCancel, }) => { @@ -81,6 +86,16 @@ export const EditUserForm: FC = ({ onBlur={form.handleBlur} autoComplete="name" /> + + {canEditAvatar && ( + form.setFieldValue("avatar_url", value)} + fullWidth + /> + )} diff --git a/site/src/pages/EditUserPage/EditUserPage.tsx b/site/src/pages/EditUserPage/EditUserPage.tsx index 5642e87176..26ae980991 100644 --- a/site/src/pages/EditUserPage/EditUserPage.tsx +++ b/site/src/pages/EditUserPage/EditUserPage.tsx @@ -70,7 +70,11 @@ const EditUserPage: FC = () => { initialValues={{ username: userData.username, name: userData.name ?? "", + avatar_url: userData.avatar_url ?? "", }} + canEditAvatar={ + userData.login_type === "password" || userData.login_type === "none" + } onSubmit={handleSubmit} onCancel={() => { navigate("..", { relative: "path" }); diff --git a/site/src/pages/UserSettingsPage/AccountPage/AccountForm.stories.tsx b/site/src/pages/UserSettingsPage/AccountPage/AccountForm.stories.tsx index c58b630568..bffdb0f53b 100644 --- a/site/src/pages/UserSettingsPage/AccountPage/AccountForm.stories.tsx +++ b/site/src/pages/UserSettingsPage/AccountPage/AccountForm.stories.tsx @@ -11,6 +11,7 @@ const meta: Meta = { initialValues: { username: "test-user", name: "Test User", + avatar_url: "", }, updateProfileError: undefined, }, diff --git a/site/src/pages/UserSettingsPage/AccountPage/AccountForm.test.tsx b/site/src/pages/UserSettingsPage/AccountPage/AccountForm.test.tsx index 88727aa143..45e4c38131 100644 --- a/site/src/pages/UserSettingsPage/AccountPage/AccountForm.test.tsx +++ b/site/src/pages/UserSettingsPage/AccountPage/AccountForm.test.tsx @@ -14,6 +14,7 @@ describe("AccountForm", () => { const mockInitialValues: UpdateUserProfileRequest = { username: MockUserMember.username, name: MockUserMember.name ?? MockUserMember.username, + avatar_url: MockUserMember.avatar_url ?? "", }; // When @@ -42,6 +43,7 @@ describe("AccountForm", () => { const mockInitialValues: UpdateUserProfileRequest = { username: MockUserMember.username, name: MockUserMember.name ?? MockUserMember.username, + avatar_url: MockUserMember.avatar_url ?? "", }; // When @@ -65,6 +67,7 @@ describe("AccountForm", () => { const mockInitialValues: UpdateUserProfileRequest = { username: MockUserMember.username, name: MockUserMember.name ?? MockUserMember.username, + avatar_url: MockUserMember.avatar_url ?? "", }; // When diff --git a/site/src/pages/UserSettingsPage/AccountPage/AccountPage.test.tsx b/site/src/pages/UserSettingsPage/AccountPage/AccountPage.test.tsx index e152c3f747..e799712bc2 100644 --- a/site/src/pages/UserSettingsPage/AccountPage/AccountPage.test.tsx +++ b/site/src/pages/UserSettingsPage/AccountPage/AccountPage.test.tsx @@ -1,12 +1,13 @@ import { fireEvent, screen, waitFor } from "@testing-library/react"; import { API } from "#/api/api"; -import { mockApiError } from "#/testHelpers/entities"; +import { MockUserOwner, mockApiError } from "#/testHelpers/entities"; import { renderWithAuth } from "#/testHelpers/renderHelpers"; import AccountPage from "./AccountPage"; const newData = { username: "user", name: "Mr User", + avatar_url: MockUserOwner.avatar_url, }; const fillAndSubmitForm = async () => { @@ -33,7 +34,6 @@ describe("AccountPage", () => { status: "active", organization_ids: ["123"], roles: [], - avatar_url: "", last_seen_at: new Date().toISOString(), login_type: "password", has_ai_seat: false, diff --git a/site/src/pages/UserSettingsPage/AccountPage/AccountPage.tsx b/site/src/pages/UserSettingsPage/AccountPage/AccountPage.tsx index 41fd7aa22c..68e46230d0 100644 --- a/site/src/pages/UserSettingsPage/AccountPage/AccountPage.tsx +++ b/site/src/pages/UserSettingsPage/AccountPage/AccountPage.tsx @@ -38,7 +38,11 @@ const AccountPage: FC = () => { email={me.email} updateProfileError={updateProfileError} isLoading={isUpdatingProfile} - initialValues={{ username: me.username, name: me.name ?? "" }} + initialValues={{ + username: me.username, + name: me.name ?? "", + avatar_url: me.avatar_url ?? "", + }} onSubmit={updateProfile} /> diff --git a/site/src/theme/externalImages.test.ts b/site/src/theme/externalImages.test.ts index 0b8986410f..3e5e24a9bc 100644 --- a/site/src/theme/externalImages.test.ts +++ b/site/src/theme/externalImages.test.ts @@ -29,6 +29,16 @@ describe("externalImage parameters", () => { expect(someoneElsesWidgetsStyles).toBeUndefined(); }); + test("incomplete or invalid URLs return no styles", () => { + // A user typing a URL produces invalid intermediate values that + // new URL() would throw on. These must not crash. + for (const value of ["https:", "http:/", "://", "not a url"]) { + expect( + getExternalImageStylesFromUrl(forDarkThemes, value), + ).toBeUndefined(); + } + }); + test("blackWithColor brightness", () => { const tryCase = (params: string) => parseImageParameters(forDarkThemes, params); diff --git a/site/src/theme/externalImages.ts b/site/src/theme/externalImages.ts index ca2efcd2f6..ca407041bf 100644 --- a/site/src/theme/externalImages.ts +++ b/site/src/theme/externalImages.ts @@ -117,7 +117,15 @@ export function getExternalImageStylesFromUrl( return undefined; } - const url = new URL(urlString, location.origin); + // While a user types a URL the value can be incomplete or invalid (e.g. + // "https:"). new URL() throws on those, so treat them as having no special + // styles instead of crashing the render. + let url: URL; + try { + url = new URL(urlString, location.origin); + } catch { + return undefined; + } if (url.search) { return parseImageParameters(modes, url.search);