From 5350dac16d9f6dc1beca4f7490920c7c34d19d5d Mon Sep 17 00:00:00 2001 From: Garrett Date: Fri, 27 May 2022 18:42:23 +0000 Subject: [PATCH 1/6] fix: prevent email from being altered --- coderd/users.go | 9 +-------- coderd/users_test.go | 40 ++-------------------------------------- codersdk/users.go | 1 - 3 files changed, 3 insertions(+), 47 deletions(-) diff --git a/coderd/users.go b/coderd/users.go index 21745c5d786da..c911a915ce96d 100644 --- a/coderd/users.go +++ b/coderd/users.go @@ -254,19 +254,12 @@ func (api *API) putUserProfile(rw http.ResponseWriter, r *http.Request) { return } existentUser, err := api.Database.GetUserByEmailOrUsername(r.Context(), database.GetUserByEmailOrUsernameParams{ - Email: params.Email, Username: params.Username, }) isDifferentUser := existentUser.ID != user.ID if err == nil && isDifferentUser { responseErrors := []httpapi.Error{} - if existentUser.Email == params.Email { - responseErrors = append(responseErrors, httpapi.Error{ - Field: "email", - Detail: "this value is already in use and should be unique", - }) - } if existentUser.Username == params.Username { responseErrors = append(responseErrors, httpapi.Error{ Field: "username", @@ -288,7 +281,7 @@ func (api *API) putUserProfile(rw http.ResponseWriter, r *http.Request) { updatedUserProfile, err := api.Database.UpdateUserProfile(r.Context(), database.UpdateUserProfileParams{ ID: user.ID, - Email: params.Email, + Email: user.Email, Username: params.Username, UpdatedAt: database.Now(), }) diff --git a/coderd/users_test.go b/coderd/users_test.go index 8e7ea9e827850..a1eebaa08ea4d 100644 --- a/coderd/users_test.go +++ b/coderd/users_test.go @@ -212,7 +212,6 @@ func TestUpdateUserProfile(t *testing.T) { coderdtest.CreateFirstUser(t, client) _, err := client.UpdateUserProfile(context.Background(), uuid.New().String(), codersdk.UpdateUserProfileRequest{ Username: "newusername", - Email: "newemail@coder.com", }) var apiErr *codersdk.Error require.ErrorAs(t, err, &apiErr) @@ -221,25 +220,6 @@ func TestUpdateUserProfile(t *testing.T) { require.Equal(t, http.StatusBadRequest, apiErr.StatusCode()) }) - t.Run("ConflictingEmail", func(t *testing.T) { - t.Parallel() - client := coderdtest.New(t, nil) - user := coderdtest.CreateFirstUser(t, client) - existentUser, _ := client.CreateUser(context.Background(), codersdk.CreateUserRequest{ - Email: "bruno@coder.com", - Username: "bruno", - Password: "password", - OrganizationID: user.OrganizationID, - }) - _, err := client.UpdateUserProfile(context.Background(), codersdk.Me, codersdk.UpdateUserProfileRequest{ - Username: "newusername", - Email: existentUser.Email, - }) - var apiErr *codersdk.Error - require.ErrorAs(t, err, &apiErr) - require.Equal(t, http.StatusConflict, apiErr.StatusCode()) - }) - t.Run("ConflictingUsername", func(t *testing.T) { t.Parallel() client := coderdtest.New(t, nil) @@ -253,38 +233,22 @@ func TestUpdateUserProfile(t *testing.T) { require.NoError(t, err) _, err = client.UpdateUserProfile(context.Background(), codersdk.Me, codersdk.UpdateUserProfileRequest{ Username: existentUser.Username, - Email: "newemail@coder.com", }) var apiErr *codersdk.Error require.ErrorAs(t, err, &apiErr) require.Equal(t, http.StatusConflict, apiErr.StatusCode()) }) - t.Run("UpdateUsernameAndEmail", func(t *testing.T) { + t.Run("UpdateUsername", func(t *testing.T) { t.Parallel() client := coderdtest.New(t, nil) coderdtest.CreateFirstUser(t, client) + _, _ = client.User(context.Background(), codersdk.Me) userProfile, err := client.UpdateUserProfile(context.Background(), codersdk.Me, codersdk.UpdateUserProfileRequest{ Username: "newusername", - Email: "newemail@coder.com", }) require.NoError(t, err) require.Equal(t, userProfile.Username, "newusername") - require.Equal(t, userProfile.Email, "newemail@coder.com") - }) - - t.Run("UpdateUsername", func(t *testing.T) { - t.Parallel() - client := coderdtest.New(t, nil) - coderdtest.CreateFirstUser(t, client) - me, _ := client.User(context.Background(), codersdk.Me) - userProfile, err := client.UpdateUserProfile(context.Background(), codersdk.Me, codersdk.UpdateUserProfileRequest{ - Username: me.Username, - Email: "newemail@coder.com", - }) - require.NoError(t, err) - require.Equal(t, userProfile.Username, me.Username) - require.Equal(t, userProfile.Email, "newemail@coder.com") }) } diff --git a/codersdk/users.go b/codersdk/users.go index c41b660b4b3ec..d2c7d4896c3ed 100644 --- a/codersdk/users.go +++ b/codersdk/users.go @@ -60,7 +60,6 @@ type CreateUserRequest struct { } type UpdateUserProfileRequest struct { - Email string `json:"email" validate:"required,email"` Username string `json:"username" validate:"required,username"` } From 02941b5e992bf04bccf9e53ba9526d4cad58ff6d Mon Sep 17 00:00:00 2001 From: Garrett Date: Fri, 27 May 2022 18:48:11 +0000 Subject: [PATCH 2/6] make gen --- site/src/api/typesGenerated.ts | 25 ++++++++++++------------- 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index 74233aa7d9775..82890515e3044 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -12,7 +12,7 @@ export interface AgentGitSSHKey { readonly private_key: string } -// From codersdk/users.go:152:6 +// From codersdk/users.go:151:6 export interface AuthMethods { readonly password: boolean readonly github: boolean @@ -44,7 +44,7 @@ export interface CreateFirstUserResponse { readonly organization_id: string } -// From codersdk/users.go:147:6 +// From codersdk/users.go:146:6 export interface CreateOrganizationRequest { readonly name: string } @@ -100,7 +100,7 @@ export interface CreateWorkspaceRequest { readonly parameter_values?: CreateParameterRequest[] } -// From codersdk/users.go:143:6 +// From codersdk/users.go:142:6 export interface GenerateAPIKeyResponse { readonly key: string } @@ -118,13 +118,13 @@ export interface GoogleInstanceIdentityToken { readonly json_web_token: string } -// From codersdk/users.go:132:6 +// From codersdk/users.go:131:6 export interface LoginWithPasswordRequest { readonly email: string readonly password: string } -// From codersdk/users.go:138:6 +// From codersdk/users.go:137:6 export interface LoginWithPasswordResponse { readonly session_token: string } @@ -276,12 +276,12 @@ export interface UpdateActiveTemplateVersion { readonly id: string } -// From codersdk/users.go:72:6 +// From codersdk/users.go:71:6 export interface UpdateRoles { readonly roles: string[] } -// From codersdk/users.go:67:6 +// From codersdk/users.go:66:6 export interface UpdateUserPasswordRequest { readonly old_password: string readonly password: string @@ -289,7 +289,6 @@ export interface UpdateUserPasswordRequest { // From codersdk/users.go:62:6 export interface UpdateUserProfileRequest { - readonly email: string readonly username: string } @@ -320,13 +319,13 @@ export interface User { readonly roles: Role[] } -// From codersdk/users.go:97:6 +// From codersdk/users.go:96:6 export interface UserAuthorization { readonly object: UserAuthorizationObject readonly action: string } -// From codersdk/users.go:113:6 +// From codersdk/users.go:112:6 export interface UserAuthorizationObject { readonly resource_type: string readonly owner_id?: string @@ -334,15 +333,15 @@ export interface UserAuthorizationObject { readonly resource_id?: string } -// From codersdk/users.go:86:6 +// From codersdk/users.go:85:6 export interface UserAuthorizationRequest { readonly checks: Record } -// From codersdk/users.go:81:6 +// From codersdk/users.go:80:6 export type UserAuthorizationResponse = Record -// From codersdk/users.go:76:6 +// From codersdk/users.go:75:6 export interface UserRoles { readonly roles: string[] readonly organization_roles: Record From a101fa1e8ca2a80ef7d963d01b5043a7480c0791 Mon Sep 17 00:00:00 2001 From: Garrett Date: Fri, 27 May 2022 18:54:31 +0000 Subject: [PATCH 3/6] disable email field on account page --- .../components/SettingsAccountForm/SettingsAccountForm.tsx | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/site/src/components/SettingsAccountForm/SettingsAccountForm.tsx b/site/src/components/SettingsAccountForm/SettingsAccountForm.tsx index 392671cee5e46..3773b307b4e32 100644 --- a/site/src/components/SettingsAccountForm/SettingsAccountForm.tsx +++ b/site/src/components/SettingsAccountForm/SettingsAccountForm.tsx @@ -15,13 +15,10 @@ interface AccountFormValues { export const Language = { usernameLabel: "Username", emailLabel: "Email", - emailInvalid: "Please enter a valid email address.", - emailRequired: "Please enter an email address.", updateSettings: "Update settings", } const validationSchema = Yup.object({ - email: Yup.string().trim().email(Language.emailInvalid).required(Language.emailRequired), username: nameValidator(Language.usernameLabel), }) @@ -59,6 +56,7 @@ export const AccountForm: React.FC = ({ fullWidth label={Language.emailLabel} variant="outlined" + disabled={true} /> Date: Fri, 27 May 2022 19:05:06 +0000 Subject: [PATCH 4/6] fix test --- .../AccountPage/AccountPage.test.tsx | 22 +------------------ 1 file changed, 1 insertion(+), 21 deletions(-) diff --git a/site/src/pages/SettingsPages/AccountPage/AccountPage.test.tsx b/site/src/pages/SettingsPages/AccountPage/AccountPage.test.tsx index 1e36c150bb30e..dd1f1f14edeca 100644 --- a/site/src/pages/SettingsPages/AccountPage/AccountPage.test.tsx +++ b/site/src/pages/SettingsPages/AccountPage/AccountPage.test.tsx @@ -17,13 +17,11 @@ const renderPage = () => { } const newData = { - email: "user@coder.com", username: "user", } const fillAndSubmitForm = async () => { await waitFor(() => screen.findByLabelText("Email")) - fireEvent.change(screen.getByLabelText("Email"), { target: { value: newData.email } }) fireEvent.change(screen.getByLabelText("Username"), { target: { value: newData.username } }) fireEvent.click(screen.getByText(AccountForm.Language.updateSettings)) } @@ -34,6 +32,7 @@ describe("AccountPage", () => { jest.spyOn(API, "updateProfile").mockImplementationOnce((userId, data) => Promise.resolve({ id: userId, + email: "user@coder.com", created_at: new Date().toString(), status: "active", organization_ids: ["123"], @@ -51,25 +50,6 @@ describe("AccountPage", () => { }) }) - describe("when the email is already taken", () => { - it("shows an error", async () => { - jest.spyOn(API, "updateProfile").mockRejectedValueOnce({ - isAxiosError: true, - response: { - data: { message: "Invalid profile", errors: [{ detail: "Email is already in use", field: "email" }] }, - }, - }) - - const { user } = renderPage() - await fillAndSubmitForm() - - const errorMessage = await screen.findByText("Email is already in use") - expect(errorMessage).toBeDefined() - expect(API.updateProfile).toBeCalledTimes(1) - expect(API.updateProfile).toBeCalledWith(user.id, newData) - }) - }) - describe("when the username is already taken", () => { it("shows an error", async () => { jest.spyOn(API, "updateProfile").mockRejectedValueOnce({ From 740fe782fea9928c67785b57a9c4c21a66fd80bd Mon Sep 17 00:00:00 2001 From: Garrett Date: Fri, 27 May 2022 20:28:24 +0000 Subject: [PATCH 5/6] pr comments --- .../SettingsAccountForm/SettingsAccountForm.tsx | 14 ++++---------- .../SettingsPages/AccountPage/AccountPage.tsx | 3 ++- 2 files changed, 6 insertions(+), 11 deletions(-) diff --git a/site/src/components/SettingsAccountForm/SettingsAccountForm.tsx b/site/src/components/SettingsAccountForm/SettingsAccountForm.tsx index 3773b307b4e32..4c90deb61daa9 100644 --- a/site/src/components/SettingsAccountForm/SettingsAccountForm.tsx +++ b/site/src/components/SettingsAccountForm/SettingsAccountForm.tsx @@ -8,7 +8,6 @@ import { LoadingButton } from "../LoadingButton/LoadingButton" import { Stack } from "../Stack/Stack" interface AccountFormValues { - email: string username: string } @@ -23,7 +22,9 @@ const validationSchema = Yup.object({ }) export type AccountFormErrors = FormikErrors + export interface AccountFormProps { + email: string isLoading: boolean initialValues: AccountFormValues onSubmit: (values: AccountFormValues) => void @@ -32,6 +33,7 @@ export interface AccountFormProps { } export const AccountForm: React.FC = ({ + email, isLoading, onSubmit, initialValues, @@ -49,15 +51,7 @@ export const AccountForm: React.FC = ({ <>
- + { return (
{ authSend({ type: "UPDATE_PROFILE", From 455e90f3497354ca4f798e13aca0e9c24c48c177 Mon Sep 17 00:00:00 2001 From: Garrett Date: Fri, 27 May 2022 22:15:03 +0000 Subject: [PATCH 6/6] fix test --- site/src/pages/SettingsPages/AccountPage/AccountPage.test.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/site/src/pages/SettingsPages/AccountPage/AccountPage.test.tsx b/site/src/pages/SettingsPages/AccountPage/AccountPage.test.tsx index dd1f1f14edeca..c1e23a6df3df2 100644 --- a/site/src/pages/SettingsPages/AccountPage/AccountPage.test.tsx +++ b/site/src/pages/SettingsPages/AccountPage/AccountPage.test.tsx @@ -21,7 +21,7 @@ const newData = { } const fillAndSubmitForm = async () => { - await waitFor(() => screen.findByLabelText("Email")) + await waitFor(() => screen.findByLabelText("Username")) fireEvent.change(screen.getByLabelText("Username"), { target: { value: newData.username } }) fireEvent.click(screen.getByText(AccountForm.Language.updateSettings)) }