Skip to content

Commit a042df6

Browse files
defelmnqBrunoQuaresma
authored and
jaaydenh
committed
fix: improve password validation flow (#15132)
Refers to #14984 Currently, password validation is done backend side and is not explicit enough so it can be painful to create first users. We'd like to make this validation easier - but also duplicate it frontend side to make it smoother. Flows involved : - First user set password - New user set password - Change password --------- Co-authored-by: BrunoQuaresma <bruno_nonato_quaresma@hotmail.com>
1 parent 302ca4f commit a042df6

File tree

21 files changed

+531
-75
lines changed

21 files changed

+531
-75
lines changed

‎coderd/apidoc/docs.go

+61
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/apidoc/swagger.json

+53
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/coderd.go

+1
Original file line numberDiff line numberDiff line change
@@ -1047,6 +1047,7 @@ func New(options *Options) *API {
10471047
r.Use(httpmw.RateLimit(options.LoginRateLimit, time.Minute))
10481048
r.Post("/login", api.postLogin)
10491049
r.Post("/otp/request", api.postRequestOneTimePasscode)
1050+
r.Post("/validate-password", api.validateUserPassword)
10501051
r.Post("/otp/change-password", api.postChangePasswordWithOneTimePasscode)
10511052
r.Route("/oauth2", func(r chi.Router) {
10521053
r.Route("/github", func(r chi.Router) {

‎coderd/userauth.go

+35
Original file line numberDiff line numberDiff line change
@@ -447,6 +447,41 @@ func (api *API) postChangePasswordWithOneTimePasscode(rw http.ResponseWriter, r
447447
}
448448
}
449449

450+
// ValidateUserPassword validates the complexity of a user password and that it is secured enough.
451+
//
452+
// @Summary Validate user password
453+
// @ID validate-user-password
454+
// @Security CoderSessionToken
455+
// @Produce json
456+
// @Accept json
457+
// @Tags Authorization
458+
// @Param request body codersdk.ValidateUserPasswordRequest true "Validate user password request"
459+
// @Success 200 {object} codersdk.ValidateUserPasswordResponse
460+
// @Router /users/validate-password [post]
461+
func (*API) validateUserPassword(rw http.ResponseWriter, r *http.Request) {
462+
var (
463+
ctx = r.Context()
464+
valid = true
465+
details = ""
466+
)
467+
468+
var req codersdk.ValidateUserPasswordRequest
469+
if !httpapi.Read(ctx, rw, r, &req) {
470+
return
471+
}
472+
473+
err := userpassword.Validate(req.Password)
474+
if err != nil {
475+
valid = false
476+
details = err.Error()
477+
}
478+
479+
httpapi.Write(ctx, rw, http.StatusOK, codersdk.ValidateUserPasswordResponse{
480+
Valid: valid,
481+
Details: details,
482+
})
483+
}
484+
450485
// Authenticates the user with an email and password.
451486
//
452487
// @Summary Log in user

‎coderd/userpassword/userpassword_test.go

+97-41
Original file line numberDiff line numberDiff line change
@@ -5,53 +5,109 @@
55
package userpassword_test
66

77
import (
8+
"strings"
89
"testing"
910

1011
"github.com/stretchr/testify/require"
1112

1213
"github.com/coder/coder/v2/coderd/userpassword"
1314
)
1415

15-
func TestUserPassword(t *testing.T) {
16+
func TestUserPasswordValidate(t *testing.T) {
1617
t.Parallel()
17-
t.Run("Legacy", func(t *testing.T) {
18-
t.Parallel()
19-
// Ensures legacy v1 passwords function for v2.
20-
// This has is manually generated using a print statement from v1 code.
21-
equal, err := userpassword.Compare("$pbkdf2-sha256$65535$z8c1p1C2ru9EImBP1I+ZNA$pNjE3Yk0oG0PmJ0Je+y7ENOVlSkn/b0BEqqdKsq6Y97wQBq0xT+lD5bWJpyIKJqQICuPZcEaGDKrXJn8+SIHRg", "tomato")
22-
require.NoError(t, err)
23-
require.True(t, equal)
24-
})
25-
26-
t.Run("Same", func(t *testing.T) {
27-
t.Parallel()
28-
hash, err := userpassword.Hash("password")
29-
require.NoError(t, err)
30-
equal, err := userpassword.Compare(hash, "password")
31-
require.NoError(t, err)
32-
require.True(t, equal)
33-
})
34-
35-
t.Run("Different", func(t *testing.T) {
36-
t.Parallel()
37-
hash, err := userpassword.Hash("password")
38-
require.NoError(t, err)
39-
equal, err := userpassword.Compare(hash, "notpassword")
40-
require.NoError(t, err)
41-
require.False(t, equal)
42-
})
43-
44-
t.Run("Invalid", func(t *testing.T) {
45-
t.Parallel()
46-
equal, err := userpassword.Compare("invalidhash", "password")
47-
require.False(t, equal)
48-
require.Error(t, err)
49-
})
50-
51-
t.Run("InvalidParts", func(t *testing.T) {
52-
t.Parallel()
53-
equal, err := userpassword.Compare("abcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyz", "test")
54-
require.False(t, equal)
55-
require.Error(t, err)
56-
})
18+
tests := []struct {
19+
name string
20+
password string
21+
wantErr bool
22+
}{
23+
{name: "Invalid - Too short password", password: "pass", wantErr: true},
24+
{name: "Invalid - Too long password", password: strings.Repeat("a", 65), wantErr: true},
25+
{name: "Invalid - easy password", password: "password", wantErr: true},
26+
{name: "Ok", password: "PasswordSecured123!", wantErr: false},
27+
}
28+
29+
for _, tt := range tests {
30+
tt := tt
31+
t.Run(tt.name, func(t *testing.T) {
32+
t.Parallel()
33+
err := userpassword.Validate(tt.password)
34+
if tt.wantErr {
35+
require.Error(t, err)
36+
} else {
37+
require.NoError(t, err)
38+
}
39+
})
40+
}
41+
}
42+
43+
func TestUserPasswordCompare(t *testing.T) {
44+
t.Parallel()
45+
tests := []struct {
46+
name string
47+
passwordToValidate string
48+
password string
49+
shouldHash bool
50+
wantErr bool
51+
wantEqual bool
52+
}{
53+
{
54+
name: "Legacy",
55+
passwordToValidate: "$pbkdf2-sha256$65535$z8c1p1C2ru9EImBP1I+ZNA$pNjE3Yk0oG0PmJ0Je+y7ENOVlSkn/b0BEqqdKsq6Y97wQBq0xT+lD5bWJpyIKJqQICuPZcEaGDKrXJn8+SIHRg",
56+
password: "tomato",
57+
shouldHash: false,
58+
wantErr: false,
59+
wantEqual: true,
60+
},
61+
{
62+
name: "Same",
63+
passwordToValidate: "password",
64+
password: "password",
65+
shouldHash: true,
66+
wantErr: false,
67+
wantEqual: true,
68+
},
69+
{
70+
name: "Different",
71+
passwordToValidate: "password",
72+
password: "notpassword",
73+
shouldHash: true,
74+
wantErr: false,
75+
wantEqual: false,
76+
},
77+
{
78+
name: "Invalid",
79+
passwordToValidate: "invalidhash",
80+
password: "password",
81+
shouldHash: false,
82+
wantErr: true,
83+
wantEqual: false,
84+
},
85+
{
86+
name: "InvalidParts",
87+
passwordToValidate: "abcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyz",
88+
password: "test",
89+
shouldHash: false,
90+
wantErr: true,
91+
wantEqual: false,
92+
},
93+
}
94+
95+
for _, tt := range tests {
96+
tt := tt
97+
t.Run(tt.name, func(t *testing.T) {
98+
t.Parallel()
99+
if tt.shouldHash {
100+
hash, err := userpassword.Hash(tt.passwordToValidate)
101+
require.NoError(t, err)
102+
tt.passwordToValidate = hash
103+
}
104+
equal, err := userpassword.Compare(tt.passwordToValidate, tt.password)
105+
if tt.wantErr {
106+
require.Error(t, err)
107+
} else {
108+
require.NoError(t, err)
109+
}
110+
require.Equal(t, tt.wantEqual, equal)
111+
})
112+
}
57113
}

‎coderd/users_test.go

+18
Original file line numberDiff line numberDiff line change
@@ -1219,6 +1219,24 @@ func TestUpdateUserPassword(t *testing.T) {
12191219
require.Equal(t, database.AuditActionWrite, auditor.AuditLogs()[numLogs-1].Action)
12201220
})
12211221

1222+
t.Run("ValidateUserPassword", func(t *testing.T) {
1223+
t.Parallel()
1224+
auditor := audit.NewMock()
1225+
client := coderdtest.New(t, &coderdtest.Options{Auditor: auditor})
1226+
1227+
_ = coderdtest.CreateFirstUser(t, client)
1228+
1229+
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
1230+
defer cancel()
1231+
1232+
resp, err := client.ValidateUserPassword(ctx, codersdk.ValidateUserPasswordRequest{
1233+
Password: "MySecurePassword!",
1234+
})
1235+
1236+
require.NoError(t, err, "users shoud be able to validate complexity of a potential new password")
1237+
require.True(t, resp.Valid)
1238+
})
1239+
12221240
t.Run("ChangingPasswordDeletesKeys", func(t *testing.T) {
12231241
t.Parallel()
12241242

0 commit comments

Comments
 (0)