Skip to content

Commit e2128e6

Browse files
defelmnqdefelmnq
committed
site: change logic to generate error instead of helper text on password validation
1 parent 51b1e51 commit e2128e6

File tree

8 files changed

+99
-13
lines changed

8 files changed

+99
-13
lines changed

‎coderd/userauth_test.go

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1880,6 +1880,39 @@ func TestUserForgotPassword(t *testing.T) {
18801880
requireCanLogin(t, ctx, anotherClient, anotherUser.Email, oldPassword)
18811881
})
18821882

1883+
t.Run("CannotChangePasswordWithWeakPassword", func(t *testing.T) {
1884+
t.Parallel()
1885+
1886+
notifyEnq := &testutil.FakeNotificationsEnqueuer{}
1887+
1888+
client := coderdtest.New(t, &coderdtest.Options{
1889+
NotificationsEnqueuer: notifyEnq,
1890+
})
1891+
user := coderdtest.CreateFirstUser(t, client)
1892+
1893+
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
1894+
defer cancel()
1895+
1896+
anotherClient, anotherUser := coderdtest.CreateAnotherUser(t, client, user.OrganizationID)
1897+
1898+
oneTimePasscode := requireRequestOneTimePasscode(t, ctx, anotherClient, notifyEnq, anotherUser.Email, anotherUser.ID)
1899+
1900+
err := anotherClient.ChangePasswordWithOneTimePasscode(ctx, codersdk.ChangePasswordWithOneTimePasscodeRequest{
1901+
Email: anotherUser.Email,
1902+
OneTimePasscode: oneTimePasscode,
1903+
Password: "notstrong",
1904+
})
1905+
var apiErr *codersdk.Error
1906+
require.ErrorAs(t, err, &apiErr)
1907+
require.Equal(t, http.StatusBadRequest, apiErr.StatusCode())
1908+
require.Contains(t, apiErr.Message, "Invalid password.")
1909+
require.Equal(t, 1, len(apiErr.Validations))
1910+
require.Equal(t, "password", apiErr.Validations[0].Field)
1911+
1912+
requireCannotLogin(t, ctx, anotherClient, anotherUser.Email, "notstrong")
1913+
requireCanLogin(t, ctx, anotherClient, anotherUser.Email, oldPassword)
1914+
})
1915+
18831916
t.Run("CannotChangePasswordOfAnotherUser", func(t *testing.T) {
18841917
t.Parallel()
18851918

‎coderd/userpassword/userpassword.go

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010
"strconv"
1111
"strings"
1212

13+
passwordvalidator "github.com/wagslane/go-password-validator"
1314
"golang.org/x/crypto/pbkdf2"
1415
"golang.org/x/exp/slices"
1516
"golang.org/x/xerrors"
@@ -137,8 +138,10 @@ func hashWithSaltAndIter(password string, salt []byte, iter int) string {
137138
// It returns properly formatted errors for detailed form validation on the client.
138139
func Validate(password string) error {
139140
// Ensure passwords are secure enough!
140-
if len(password) < 6 {
141-
return xerrors.Errorf("password must be at least %d characters", 6)
141+
// See: https://github.com/wagslane/go-password-validator#what-entropy-value-should-i-use
142+
err := passwordvalidator.Validate(password, 52)
143+
if err != nil {
144+
return err
142145
}
143146
if len(password) > 64 {
144147
return xerrors.Errorf("password must be no more than %d characters", 64)

‎coderd/userpassword/userpassword_test.go

Lines changed: 44 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -20,9 +20,10 @@ func TestUserPasswordValidate(t *testing.T) {
2020
password string
2121
wantErr bool
2222
}{
23-
{"Invalid - Too short password", "pass", true},
24-
{"Invalid - Too long password", strings.Repeat("a", 65), true},
25-
{"Ok", "CorrectPassword", false},
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},
2627
}
2728

2829
for _, tt := range tests {
@@ -49,11 +50,46 @@ func TestUserPasswordCompare(t *testing.T) {
4950
wantErr bool
5051
wantEqual bool
5152
}{
52-
{"Legacy", "$pbkdf2-sha256$65535$z8c1p1C2ru9EImBP1I+ZNA$pNjE3Yk0oG0PmJ0Je+y7ENOVlSkn/b0BEqqdKsq6Y97wQBq0xT+lD5bWJpyIKJqQICuPZcEaGDKrXJn8+SIHRg", "tomato", false, false, true},
53-
{"Same", "password", "password", true, false, true},
54-
{"Different", "password", "notpassword", true, false, false},
55-
{"Invalid", "invalidhash", "password", false, true, false},
56-
{"InvalidParts", "abcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyz", "test", false, true, false},
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+
},
5793
}
5894

5995
for _, tt := range tests {

‎go.mod

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -163,6 +163,7 @@ require (
163163
github.com/u-root/u-root v0.14.0
164164
github.com/unrolled/secure v1.17.0
165165
github.com/valyala/fasthttp v1.56.0
166+
github.com/wagslane/go-password-validator v0.3.0
166167
go.mozilla.org/pkcs7 v0.9.0
167168
go.nhat.io/otelsql v0.14.0
168169
go.opentelemetry.io/otel v1.30.0

‎go.sum

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -967,6 +967,8 @@ github.com/vmihailenco/msgpack/v5 v5.4.1 h1:cQriyiUvjTwOHg8QZaPihLWeRAAVoCpE00IU
967967
github.com/vmihailenco/msgpack/v5 v5.4.1/go.mod h1:GaZTsDaehaPpQVyxrf5mtQlH+pc21PIudVV/E3rRQok=
968968
github.com/vmihailenco/tagparser/v2 v2.0.0 h1:y09buUbR+b5aycVFQs/g70pqKVZNBmxwAhO7/IwNM9g=
969969
github.com/vmihailenco/tagparser/v2 v2.0.0/go.mod h1:Wri+At7QHww0WTrCBeu4J6bNtoV6mEfg5OIWRZA9qds=
970+
github.com/wagslane/go-password-validator v0.3.0 h1:vfxOPzGHkz5S146HDpavl0cw1DSVP061Ry2PX0/ON6I=
971+
github.com/wagslane/go-password-validator v0.3.0/go.mod h1:TI1XJ6T5fRdRnHqHt14pvy1tNVnrwe7m3/f1f2fDphQ=
970972
github.com/wlynxg/anet v0.0.3/go.mod h1:eay5PRQr7fIVAMbTbchTnO9gG65Hg/uYGdc7mguHxoA=
971973
github.com/x448/float16 v0.8.4 h1:qLwI1I70+NjRFUR3zs1JPUCgaCXSh3SW62uAKT1mSBM=
972974
github.com/x448/float16 v0.8.4/go.mod h1:14CWIYCyZA/cWjXOioeEpHeN/83MdbZDRQHoFcYsOfg=

‎site/src/pages/CreateUserPage/CreateUserForm.tsx

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -205,13 +205,16 @@ export const CreateUserForm: FC<
205205
helperText:
206206
(form.values.login_type !== "password" &&
207207
"No password required for this login type") ||
208-
(form.values.password !== "" && !passwordIsValid && "password is not strong."),
208+
(form.values.password !== "" &&
209+
!passwordIsValid &&
210+
"password is not strong enough."),
209211
})}
210212
autoComplete="current-password"
211213
fullWidth
212214
id="password"
213215
data-testid="password-input"
214216
disabled={form.values.login_type !== "password"}
217+
error={!!(form.values.password !== "" && !passwordIsValid)}
215218
label={Language.passwordLabel}
216219
type="password"
217220
/>

‎site/src/pages/SetupPage/SetupPageView.tsx

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -183,7 +183,10 @@ export const SetupPageView: FC<SetupPageViewProps> = ({
183183
id="password"
184184
label={Language.passwordLabel}
185185
type="password"
186-
helperText={form.values.password !== "" && !passwordIsValid ? "Password is not strong." : ""}
186+
error={!!(form.values.password !== "" && !passwordIsValid)}
187+
helperText={
188+
!passwordIsValid ? "Password is not strong enough." : ""
189+
}
187190
/>
188191
<label
189192
htmlFor="trial"

‎site/src/pages/UserSettingsPage/SecurityPage/SecurityForm.tsx

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,12 @@ export const SecurityForm: FC<SecurityFormProps> = ({
9696
autoComplete="password"
9797
fullWidth
9898
label={Language.newPasswordLabel}
99-
helperText={form.values.password !== "" && !passwordIsValid ? "Password is not strong." : ""}
99+
error={!!(form.values.password !== "" && !passwordIsValid)}
100+
helperText={
101+
form.values.password !== "" && !passwordIsValid
102+
? "Password is not strong enough."
103+
: ""
104+
}
100105
type="password"
101106
/>
102107
<TextField

0 commit comments

Comments
 (0)