Skip to content

Commit b3f62c9

Browse files
f0sselkylecarbs
f0ssel
authored and
kylecarbs
committed
fix: Add route for user to change own password (#1812)
1 parent 99b77fb commit b3f62c9

File tree

6 files changed

+99
-13
lines changed

6 files changed

+99
-13
lines changed

‎coderd/roles.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ func (api *API) assignableOrgRoles(rw http.ResponseWriter, r *http.Request) {
4040
func (api *API) checkPermissions(rw http.ResponseWriter, r *http.Request) {
4141
user := httpmw.UserParam(r)
4242

43-
if !api.Authorize(rw, r, rbac.ActionRead, rbac.ResourceUser.WithOwner(user.ID.String())) {
43+
if !api.Authorize(rw, r, rbac.ActionRead, rbac.ResourceUser.WithID(user.ID.String())) {
4444
return
4545
}
4646

‎coderd/userpassword/userpassword.go

+16
Original file line numberDiff line numberDiff line change
@@ -121,3 +121,19 @@ func hashWithSaltAndIter(password string, salt []byte, iter int) string {
121121

122122
return fmt.Sprintf("$%s$%d$%s$%s", hashScheme, iter, encSalt, encHash)
123123
}
124+
125+
// Validate checks that the plain text password meets the minimum password requirements.
126+
// It returns properly formatted errors for detailed form validation on the client.
127+
func Validate(password string) error {
128+
const (
129+
minLength = 8
130+
maxLength = 64
131+
)
132+
if len(password) < minLength {
133+
return xerrors.Errorf("Password must be at least %d characters.", minLength)
134+
}
135+
if len(password) > maxLength {
136+
return xerrors.Errorf("Password must be no more than %d characters.", maxLength)
137+
}
138+
return nil
139+
}

‎coderd/users.go

+38
Original file line numberDiff line numberDiff line change
@@ -355,6 +355,7 @@ func (api *API) putUserStatus(status database.UserStatus) func(rw http.ResponseW
355355
func (api *API) putUserPassword(rw http.ResponseWriter, r *http.Request) {
356356
var (
357357
user = httpmw.UserParam(r)
358+
apiKey = httpmw.APIKey(r)
358359
params codersdk.UpdateUserPasswordRequest
359360
)
360361

@@ -366,6 +367,43 @@ func (api *API) putUserPassword(rw http.ResponseWriter, r *http.Request) {
366367
return
367368
}
368369

370+
err := userpassword.Validate(params.Password)
371+
if err != nil {
372+
httpapi.Write(rw, http.StatusBadRequest, httpapi.Response{
373+
Errors: []httpapi.Error{
374+
{
375+
Field: "password",
376+
Detail: err.Error(),
377+
},
378+
},
379+
})
380+
return
381+
}
382+
383+
// we want to require old_password field if the user is changing their
384+
// own password. This is to prevent a compromised session from being able
385+
// to change password and lock out the user.
386+
if user.ID == apiKey.UserID {
387+
ok, err := userpassword.Compare(string(user.HashedPassword), params.OldPassword)
388+
if err != nil {
389+
httpapi.Write(rw, http.StatusInternalServerError, httpapi.Response{
390+
Message: fmt.Sprintf("compare user password: %s", err.Error()),
391+
})
392+
return
393+
}
394+
if !ok {
395+
httpapi.Write(rw, http.StatusBadRequest, httpapi.Response{
396+
Errors: []httpapi.Error{
397+
{
398+
Field: "old_password",
399+
Detail: "Old password is incorrect.",
400+
},
401+
},
402+
})
403+
return
404+
}
405+
}
406+
369407
hashedPassword, err := userpassword.Hash(params.Password)
370408
if err != nil {
371409
httpapi.Write(rw, http.StatusInternalServerError, httpapi.Response{

‎coderd/users_test.go

+30
Original file line numberDiff line numberDiff line change
@@ -324,6 +324,36 @@ func TestUpdateUserPassword(t *testing.T) {
324324
})
325325
require.NoError(t, err, "member should login successfully with the new password")
326326
})
327+
t.Run("MemberCanUpdateOwnPassword", func(t *testing.T) {
328+
t.Parallel()
329+
client := coderdtest.New(t, nil)
330+
admin := coderdtest.CreateFirstUser(t, client)
331+
member := coderdtest.CreateAnotherUser(t, client, admin.OrganizationID)
332+
err := member.UpdateUserPassword(context.Background(), "me", codersdk.UpdateUserPasswordRequest{
333+
OldPassword: "testpass",
334+
Password: "newpassword",
335+
})
336+
require.NoError(t, err, "member should be able to update own password")
337+
})
338+
t.Run("MemberCantUpdateOwnPasswordWithoutOldPassword", func(t *testing.T) {
339+
t.Parallel()
340+
client := coderdtest.New(t, nil)
341+
admin := coderdtest.CreateFirstUser(t, client)
342+
member := coderdtest.CreateAnotherUser(t, client, admin.OrganizationID)
343+
err := member.UpdateUserPassword(context.Background(), "me", codersdk.UpdateUserPasswordRequest{
344+
Password: "newpassword",
345+
})
346+
require.Error(t, err, "member should not be able to update own password without providing old password")
347+
})
348+
t.Run("AdminCantUpdateOwnPasswordWithoutOldPassword", func(t *testing.T) {
349+
t.Parallel()
350+
client := coderdtest.New(t, nil)
351+
_ = coderdtest.CreateFirstUser(t, client)
352+
err := client.UpdateUserPassword(context.Background(), "me", codersdk.UpdateUserPasswordRequest{
353+
Password: "newpassword",
354+
})
355+
require.Error(t, err, "admin should not be able to update own password without providing old password")
356+
})
327357
}
328358

329359
func TestGrantRoles(t *testing.T) {

‎codersdk/users.go

+2-1
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,8 @@ type UpdateUserProfileRequest struct {
6565
}
6666

6767
type UpdateUserPasswordRequest struct {
68-
Password string `json:"password" validate:"required"`
68+
OldPassword string `json:"old_password" validate:""`
69+
Password string `json:"password" validate:"required"`
6970
}
7071

7172
type UpdateRoles struct {

‎site/src/api/typesGenerated.ts

+12-11
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:151:6
15+
// From codersdk/users.go:152: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:146:6
47+
// From codersdk/users.go:147: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:142:6
103+
// From codersdk/users.go:143: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:131:6
121+
// From codersdk/users.go:132:6
122122
export interface LoginWithPasswordRequest {
123123
readonly email: string
124124
readonly password: string
125125
}
126126

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

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

284284
// From codersdk/users.go:67:6
285285
export interface UpdateUserPasswordRequest {
286+
readonly old_password: string
286287
readonly password: string
287288
}
288289

@@ -319,29 +320,29 @@ export interface User {
319320
readonly roles: Role[]
320321
}
321322

322-
// From codersdk/users.go:96:6
323+
// From codersdk/users.go:97:6
323324
export interface UserAuthorization {
324325
readonly object: UserAuthorizationObject
325326
readonly action: string
326327
}
327328

328-
// From codersdk/users.go:112:6
329+
// From codersdk/users.go:113:6
329330
export interface UserAuthorizationObject {
330331
readonly resource_type: string
331332
readonly owner_id?: string
332333
readonly organization_id?: string
333334
readonly resource_id?: string
334335
}
335336

336-
// From codersdk/users.go:85:6
337+
// From codersdk/users.go:86:6
337338
export interface UserAuthorizationRequest {
338339
readonly checks: Record<string, UserAuthorization>
339340
}
340341

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

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

0 commit comments

Comments
 (0)