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);