Skip to content

Commit 5598ac0

Browse files
f0sself0ssel
authored
fix: prevent email from being altered (coder#1863)
1 parent cfa316b commit 5598ac0

File tree

7 files changed

+23
-95
lines changed

7 files changed

+23
-95
lines changed

‎coderd/users.go

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -254,19 +254,12 @@ func (api *API) putUserProfile(rw http.ResponseWriter, r *http.Request) {
254254
return
255255
}
256256
existentUser, err := api.Database.GetUserByEmailOrUsername(r.Context(), database.GetUserByEmailOrUsernameParams{
257-
Email: params.Email,
258257
Username: params.Username,
259258
})
260259
isDifferentUser := existentUser.ID != user.ID
261260

262261
if err == nil && isDifferentUser {
263262
responseErrors := []httpapi.Error{}
264-
if existentUser.Email == params.Email {
265-
responseErrors = append(responseErrors, httpapi.Error{
266-
Field: "email",
267-
Detail: "this value is already in use and should be unique",
268-
})
269-
}
270263
if existentUser.Username == params.Username {
271264
responseErrors = append(responseErrors, httpapi.Error{
272265
Field: "username",
@@ -288,7 +281,7 @@ func (api *API) putUserProfile(rw http.ResponseWriter, r *http.Request) {
288281

289282
updatedUserProfile, err := api.Database.UpdateUserProfile(r.Context(), database.UpdateUserProfileParams{
290283
ID: user.ID,
291-
Email: params.Email,
284+
Email: user.Email,
292285
Username: params.Username,
293286
UpdatedAt: database.Now(),
294287
})

‎coderd/users_test.go

Lines changed: 2 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -259,7 +259,6 @@ func TestUpdateUserProfile(t *testing.T) {
259259
coderdtest.CreateFirstUser(t, client)
260260
_, err := client.UpdateUserProfile(context.Background(), uuid.New().String(), codersdk.UpdateUserProfileRequest{
261261
Username: "newusername",
262-
Email: "newemail@coder.com",
263262
})
264263
var apiErr *codersdk.Error
265264
require.ErrorAs(t, err, &apiErr)
@@ -268,25 +267,6 @@ func TestUpdateUserProfile(t *testing.T) {
268267
require.Equal(t, http.StatusBadRequest, apiErr.StatusCode())
269268
})
270269

271-
t.Run("ConflictingEmail", func(t *testing.T) {
272-
t.Parallel()
273-
client := coderdtest.New(t, nil)
274-
user := coderdtest.CreateFirstUser(t, client)
275-
existentUser, _ := client.CreateUser(context.Background(), codersdk.CreateUserRequest{
276-
Email: "bruno@coder.com",
277-
Username: "bruno",
278-
Password: "password",
279-
OrganizationID: user.OrganizationID,
280-
})
281-
_, err := client.UpdateUserProfile(context.Background(), codersdk.Me, codersdk.UpdateUserProfileRequest{
282-
Username: "newusername",
283-
Email: existentUser.Email,
284-
})
285-
var apiErr *codersdk.Error
286-
require.ErrorAs(t, err, &apiErr)
287-
require.Equal(t, http.StatusConflict, apiErr.StatusCode())
288-
})
289-
290270
t.Run("ConflictingUsername", func(t *testing.T) {
291271
t.Parallel()
292272
client := coderdtest.New(t, nil)
@@ -300,38 +280,22 @@ func TestUpdateUserProfile(t *testing.T) {
300280
require.NoError(t, err)
301281
_, err = client.UpdateUserProfile(context.Background(), codersdk.Me, codersdk.UpdateUserProfileRequest{
302282
Username: existentUser.Username,
303-
Email: "newemail@coder.com",
304283
})
305284
var apiErr *codersdk.Error
306285
require.ErrorAs(t, err, &apiErr)
307286
require.Equal(t, http.StatusConflict, apiErr.StatusCode())
308287
})
309288

310-
t.Run("UpdateUsernameAndEmail", func(t *testing.T) {
289+
t.Run("UpdateUsername", func(t *testing.T) {
311290
t.Parallel()
312291
client := coderdtest.New(t, nil)
313292
coderdtest.CreateFirstUser(t, client)
293+
_, _ = client.User(context.Background(), codersdk.Me)
314294
userProfile, err := client.UpdateUserProfile(context.Background(), codersdk.Me, codersdk.UpdateUserProfileRequest{
315295
Username: "newusername",
316-
Email: "newemail@coder.com",
317296
})
318297
require.NoError(t, err)
319298
require.Equal(t, userProfile.Username, "newusername")
320-
require.Equal(t, userProfile.Email, "newemail@coder.com")
321-
})
322-
323-
t.Run("UpdateUsername", func(t *testing.T) {
324-
t.Parallel()
325-
client := coderdtest.New(t, nil)
326-
coderdtest.CreateFirstUser(t, client)
327-
me, _ := client.User(context.Background(), codersdk.Me)
328-
userProfile, err := client.UpdateUserProfile(context.Background(), codersdk.Me, codersdk.UpdateUserProfileRequest{
329-
Username: me.Username,
330-
Email: "newemail@coder.com",
331-
})
332-
require.NoError(t, err)
333-
require.Equal(t, userProfile.Username, me.Username)
334-
require.Equal(t, userProfile.Email, "newemail@coder.com")
335299
})
336300
}
337301

‎codersdk/users.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,6 @@ type CreateUserRequest struct {
6060
}
6161

6262
type UpdateUserProfileRequest struct {
63-
Email string `json:"email" validate:"required,email"`
6463
Username string `json:"username" validate:"required,username"`
6564
}
6665

‎site/src/api/typesGenerated.ts

Lines changed: 12 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ export interface AgentGitSSHKey {
1212
readonly private_key: string
1313
}
1414

15-
// From codersdk/users.go:152:6
15+
// From codersdk/users.go:151:6
1616
export interface AuthMethods {
1717
readonly password: boolean
1818
readonly github: boolean
@@ -44,7 +44,7 @@ export interface CreateFirstUserResponse {
4444
readonly organization_id: string
4545
}
4646

47-
// From codersdk/users.go:147:6
47+
// From codersdk/users.go:146:6
4848
export interface CreateOrganizationRequest {
4949
readonly name: string
5050
}
@@ -100,7 +100,7 @@ export interface CreateWorkspaceRequest {
100100
readonly parameter_values?: CreateParameterRequest[]
101101
}
102102

103-
// From codersdk/users.go:143:6
103+
// From codersdk/users.go:142:6
104104
export interface GenerateAPIKeyResponse {
105105
readonly key: string
106106
}
@@ -118,13 +118,13 @@ export interface GoogleInstanceIdentityToken {
118118
readonly json_web_token: string
119119
}
120120

121-
// From codersdk/users.go:132:6
121+
// From codersdk/users.go:131:6
122122
export interface LoginWithPasswordRequest {
123123
readonly email: string
124124
readonly password: string
125125
}
126126

127-
// From codersdk/users.go:138:6
127+
// From codersdk/users.go:137:6
128128
export interface LoginWithPasswordResponse {
129129
readonly session_token: string
130130
}
@@ -276,20 +276,19 @@ export interface UpdateActiveTemplateVersion {
276276
readonly id: string
277277
}
278278

279-
// From codersdk/users.go:72:6
279+
// From codersdk/users.go:71:6
280280
export interface UpdateRoles {
281281
readonly roles: string[]
282282
}
283283

284-
// From codersdk/users.go:67:6
284+
// From codersdk/users.go:66:6
285285
export interface UpdateUserPasswordRequest {
286286
readonly old_password: string
287287
readonly password: string
288288
}
289289

290290
// From codersdk/users.go:62:6
291291
export interface UpdateUserProfileRequest {
292-
readonly email: string
293292
readonly username: string
294293
}
295294

@@ -320,29 +319,29 @@ export interface User {
320319
readonly roles: Role[]
321320
}
322321

323-
// From codersdk/users.go:97:6
322+
// From codersdk/users.go:96:6
324323
export interface UserAuthorization {
325324
readonly object: UserAuthorizationObject
326325
readonly action: string
327326
}
328327

329-
// From codersdk/users.go:113:6
328+
// From codersdk/users.go:112:6
330329
export interface UserAuthorizationObject {
331330
readonly resource_type: string
332331
readonly owner_id?: string
333332
readonly organization_id?: string
334333
readonly resource_id?: string
335334
}
336335

337-
// From codersdk/users.go:86:6
336+
// From codersdk/users.go:85:6
338337
export interface UserAuthorizationRequest {
339338
readonly checks: Record<string, UserAuthorization>
340339
}
341340

342-
// From codersdk/users.go:81:6
341+
// From codersdk/users.go:80:6
343342
export type UserAuthorizationResponse = Record<string, boolean>
344343

345-
// From codersdk/users.go:76:6
344+
// From codersdk/users.go:75:6
346345
export interface UserRoles {
347346
readonly roles: string[]
348347
readonly organization_roles: Record<string, string[]>

‎site/src/components/SettingsAccountForm/SettingsAccountForm.tsx

Lines changed: 4 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -8,25 +8,23 @@ import { LoadingButton } from "../LoadingButton/LoadingButton"
88
import { Stack } from "../Stack/Stack"
99

1010
interface AccountFormValues {
11-
email: string
1211
username: string
1312
}
1413

1514
export const Language = {
1615
usernameLabel: "Username",
1716
emailLabel: "Email",
18-
emailInvalid: "Please enter a valid email address.",
19-
emailRequired: "Please enter an email address.",
2017
updateSettings: "Update settings",
2118
}
2219

2320
const validationSchema = Yup.object({
24-
email: Yup.string().trim().email(Language.emailInvalid).required(Language.emailRequired),
2521
username: nameValidator(Language.usernameLabel),
2622
})
2723

2824
export type AccountFormErrors = FormikErrors<AccountFormValues>
25+
2926
export interface AccountFormProps {
27+
email: string
3028
isLoading: boolean
3129
initialValues: AccountFormValues
3230
onSubmit: (values: AccountFormValues) => void
@@ -35,6 +33,7 @@ export interface AccountFormProps {
3533
}
3634

3735
export const AccountForm: React.FC<AccountFormProps> = ({
36+
email,
3837
isLoading,
3938
onSubmit,
4039
initialValues,
@@ -52,14 +51,7 @@ export const AccountForm: React.FC<AccountFormProps> = ({
5251
<>
5352
<form onSubmit={form.handleSubmit}>
5453
<Stack>
55-
<TextField
56-
{...getFieldHelpers("email")}
57-
onChange={onChangeTrimmed(form)}
58-
autoComplete="email"
59-
fullWidth
60-
label={Language.emailLabel}
61-
variant="outlined"
62-
/>
54+
<TextField disabled fullWidth label={Language.emailLabel} value={email} variant="outlined" />
6355
<TextField
6456
{...getFieldHelpers("username")}
6557
onChange={onChangeTrimmed(form)}

‎site/src/pages/SettingsPages/AccountPage/AccountPage.test.tsx

Lines changed: 2 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -17,13 +17,11 @@ const renderPage = () => {
1717
}
1818

1919
const newData = {
20-
email: "user@coder.com",
2120
username: "user",
2221
}
2322

2423
const fillAndSubmitForm = async () => {
25-
await waitFor(() => screen.findByLabelText("Email"))
26-
fireEvent.change(screen.getByLabelText("Email"), { target: { value: newData.email } })
24+
await waitFor(() => screen.findByLabelText("Username"))
2725
fireEvent.change(screen.getByLabelText("Username"), { target: { value: newData.username } })
2826
fireEvent.click(screen.getByText(AccountForm.Language.updateSettings))
2927
}
@@ -34,6 +32,7 @@ describe("AccountPage", () => {
3432
jest.spyOn(API, "updateProfile").mockImplementationOnce((userId, data) =>
3533
Promise.resolve({
3634
id: userId,
35+
email: "user@coder.com",
3736
created_at: new Date().toString(),
3837
status: "active",
3938
organization_ids: ["123"],
@@ -51,25 +50,6 @@ describe("AccountPage", () => {
5150
})
5251
})
5352

54-
describe("when the email is already taken", () => {
55-
it("shows an error", async () => {
56-
jest.spyOn(API, "updateProfile").mockRejectedValueOnce({
57-
isAxiosError: true,
58-
response: {
59-
data: { message: "Invalid profile", errors: [{ detail: "Email is already in use", field: "email" }] },
60-
},
61-
})
62-
63-
const { user } = renderPage()
64-
await fillAndSubmitForm()
65-
66-
const errorMessage = await screen.findByText("Email is already in use")
67-
expect(errorMessage).toBeDefined()
68-
expect(API.updateProfile).toBeCalledTimes(1)
69-
expect(API.updateProfile).toBeCalledWith(user.id, newData)
70-
})
71-
})
72-
7353
describe("when the username is already taken", () => {
7454
it("shows an error", async () => {
7555
jest.spyOn(API, "updateProfile").mockRejectedValueOnce({

‎site/src/pages/SettingsPages/AccountPage/AccountPage.tsx

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,10 +26,11 @@ export const AccountPage: React.FC = () => {
2626
return (
2727
<Section title={Language.title}>
2828
<AccountForm
29+
email={me.email}
2930
error={hasUnknownError ? Language.unknownError : undefined}
3031
formErrors={formErrors}
3132
isLoading={authState.matches("signedIn.profile.updatingProfile")}
32-
initialValues={{ username: me.username, email: me.email }}
33+
initialValues={{ username: me.username }}
3334
onSubmit={(data) => {
3435
authSend({
3536
type: "UPDATE_PROFILE",

0 commit comments

Comments
 (0)