mirror of
https://github.com/coder/coder.git
synced 2026-09-24 15:04:27 +08:00
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>
This commit is contained in:
Vincent Vielle
co-authored by
BrunoQuaresma
BrunoQuaresma
parent
8b5a18cade
commit
4fe2c5f09a
Generated
+61
@@ -5373,6 +5373,45 @@ const docTemplate = `{
|
||||
}
|
||||
}
|
||||
},
|
||||
"/users/validate-password": {
|
||||
"post": {
|
||||
"security": [
|
||||
{
|
||||
"CoderSessionToken": []
|
||||
}
|
||||
],
|
||||
"consumes": [
|
||||
"application/json"
|
||||
],
|
||||
"produces": [
|
||||
"application/json"
|
||||
],
|
||||
"tags": [
|
||||
"Authorization"
|
||||
],
|
||||
"summary": "Validate user password",
|
||||
"operationId": "validate-user-password",
|
||||
"parameters": [
|
||||
{
|
||||
"description": "Validate user password request",
|
||||
"name": "request",
|
||||
"in": "body",
|
||||
"required": true,
|
||||
"schema": {
|
||||
"$ref": "#/definitions/codersdk.ValidateUserPasswordRequest"
|
||||
}
|
||||
}
|
||||
],
|
||||
"responses": {
|
||||
"200": {
|
||||
"description": "OK",
|
||||
"schema": {
|
||||
"$ref": "#/definitions/codersdk.ValidateUserPasswordResponse"
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
},
|
||||
"/users/{user}": {
|
||||
"get": {
|
||||
"security": [
|
||||
@@ -14096,6 +14135,28 @@ const docTemplate = `{
|
||||
"UserStatusSuspended"
|
||||
]
|
||||
},
|
||||
"codersdk.ValidateUserPasswordRequest": {
|
||||
"type": "object",
|
||||
"required": [
|
||||
"password"
|
||||
],
|
||||
"properties": {
|
||||
"password": {
|
||||
"type": "string"
|
||||
}
|
||||
}
|
||||
},
|
||||
"codersdk.ValidateUserPasswordResponse": {
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"details": {
|
||||
"type": "string"
|
||||
},
|
||||
"valid": {
|
||||
"type": "boolean"
|
||||
}
|
||||
}
|
||||
},
|
||||
"codersdk.ValidationError": {
|
||||
"type": "object",
|
||||
"required": [
|
||||
|
||||
Generated
+53
@@ -4737,6 +4737,39 @@
|
||||
}
|
||||
}
|
||||
},
|
||||
"/users/validate-password": {
|
||||
"post": {
|
||||
"security": [
|
||||
{
|
||||
"CoderSessionToken": []
|
||||
}
|
||||
],
|
||||
"consumes": ["application/json"],
|
||||
"produces": ["application/json"],
|
||||
"tags": ["Authorization"],
|
||||
"summary": "Validate user password",
|
||||
"operationId": "validate-user-password",
|
||||
"parameters": [
|
||||
{
|
||||
"description": "Validate user password request",
|
||||
"name": "request",
|
||||
"in": "body",
|
||||
"required": true,
|
||||
"schema": {
|
||||
"$ref": "#/definitions/codersdk.ValidateUserPasswordRequest"
|
||||
}
|
||||
}
|
||||
],
|
||||
"responses": {
|
||||
"200": {
|
||||
"description": "OK",
|
||||
"schema": {
|
||||
"$ref": "#/definitions/codersdk.ValidateUserPasswordResponse"
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
},
|
||||
"/users/{user}": {
|
||||
"get": {
|
||||
"security": [
|
||||
@@ -12817,6 +12850,26 @@
|
||||
"UserStatusSuspended"
|
||||
]
|
||||
},
|
||||
"codersdk.ValidateUserPasswordRequest": {
|
||||
"type": "object",
|
||||
"required": ["password"],
|
||||
"properties": {
|
||||
"password": {
|
||||
"type": "string"
|
||||
}
|
||||
}
|
||||
},
|
||||
"codersdk.ValidateUserPasswordResponse": {
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"details": {
|
||||
"type": "string"
|
||||
},
|
||||
"valid": {
|
||||
"type": "boolean"
|
||||
}
|
||||
}
|
||||
},
|
||||
"codersdk.ValidationError": {
|
||||
"type": "object",
|
||||
"required": ["detail", "field"],
|
||||
|
||||
@@ -1047,6 +1047,7 @@ func New(options *Options) *API {
|
||||
r.Use(httpmw.RateLimit(options.LoginRateLimit, time.Minute))
|
||||
r.Post("/login", api.postLogin)
|
||||
r.Post("/otp/request", api.postRequestOneTimePasscode)
|
||||
r.Post("/validate-password", api.validateUserPassword)
|
||||
r.Post("/otp/change-password", api.postChangePasswordWithOneTimePasscode)
|
||||
r.Route("/oauth2", func(r chi.Router) {
|
||||
r.Route("/github", func(r chi.Router) {
|
||||
|
||||
@@ -447,6 +447,41 @@ func (api *API) postChangePasswordWithOneTimePasscode(rw http.ResponseWriter, r
|
||||
}
|
||||
}
|
||||
|
||||
// ValidateUserPassword validates the complexity of a user password and that it is secured enough.
|
||||
//
|
||||
// @Summary Validate user password
|
||||
// @ID validate-user-password
|
||||
// @Security CoderSessionToken
|
||||
// @Produce json
|
||||
// @Accept json
|
||||
// @Tags Authorization
|
||||
// @Param request body codersdk.ValidateUserPasswordRequest true "Validate user password request"
|
||||
// @Success 200 {object} codersdk.ValidateUserPasswordResponse
|
||||
// @Router /users/validate-password [post]
|
||||
func (*API) validateUserPassword(rw http.ResponseWriter, r *http.Request) {
|
||||
var (
|
||||
ctx = r.Context()
|
||||
valid = true
|
||||
details = ""
|
||||
)
|
||||
|
||||
var req codersdk.ValidateUserPasswordRequest
|
||||
if !httpapi.Read(ctx, rw, r, &req) {
|
||||
return
|
||||
}
|
||||
|
||||
err := userpassword.Validate(req.Password)
|
||||
if err != nil {
|
||||
valid = false
|
||||
details = err.Error()
|
||||
}
|
||||
|
||||
httpapi.Write(ctx, rw, http.StatusOK, codersdk.ValidateUserPasswordResponse{
|
||||
Valid: valid,
|
||||
Details: details,
|
||||
})
|
||||
}
|
||||
|
||||
// Authenticates the user with an email and password.
|
||||
//
|
||||
// @Summary Log in user
|
||||
|
||||
@@ -5,6 +5,7 @@
|
||||
package userpassword_test
|
||||
|
||||
import (
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
"github.com/stretchr/testify/require"
|
||||
@@ -12,46 +13,101 @@ import (
|
||||
"github.com/coder/coder/v2/coderd/userpassword"
|
||||
)
|
||||
|
||||
func TestUserPassword(t *testing.T) {
|
||||
func TestUserPasswordValidate(t *testing.T) {
|
||||
t.Parallel()
|
||||
t.Run("Legacy", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
// Ensures legacy v1 passwords function for v2.
|
||||
// This has is manually generated using a print statement from v1 code.
|
||||
equal, err := userpassword.Compare("$pbkdf2-sha256$65535$z8c1p1C2ru9EImBP1I+ZNA$pNjE3Yk0oG0PmJ0Je+y7ENOVlSkn/b0BEqqdKsq6Y97wQBq0xT+lD5bWJpyIKJqQICuPZcEaGDKrXJn8+SIHRg", "tomato")
|
||||
require.NoError(t, err)
|
||||
require.True(t, equal)
|
||||
})
|
||||
tests := []struct {
|
||||
name string
|
||||
password string
|
||||
wantErr bool
|
||||
}{
|
||||
{name: "Invalid - Too short password", password: "pass", wantErr: true},
|
||||
{name: "Invalid - Too long password", password: strings.Repeat("a", 65), wantErr: true},
|
||||
{name: "Invalid - easy password", password: "password", wantErr: true},
|
||||
{name: "Ok", password: "PasswordSecured123!", wantErr: false},
|
||||
}
|
||||
|
||||
t.Run("Same", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
hash, err := userpassword.Hash("password")
|
||||
require.NoError(t, err)
|
||||
equal, err := userpassword.Compare(hash, "password")
|
||||
require.NoError(t, err)
|
||||
require.True(t, equal)
|
||||
})
|
||||
for _, tt := range tests {
|
||||
tt := tt
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
t.Parallel()
|
||||
err := userpassword.Validate(tt.password)
|
||||
if tt.wantErr {
|
||||
require.Error(t, err)
|
||||
} else {
|
||||
require.NoError(t, err)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
t.Run("Different", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
hash, err := userpassword.Hash("password")
|
||||
require.NoError(t, err)
|
||||
equal, err := userpassword.Compare(hash, "notpassword")
|
||||
require.NoError(t, err)
|
||||
require.False(t, equal)
|
||||
})
|
||||
func TestUserPasswordCompare(t *testing.T) {
|
||||
t.Parallel()
|
||||
tests := []struct {
|
||||
name string
|
||||
passwordToValidate string
|
||||
password string
|
||||
shouldHash bool
|
||||
wantErr bool
|
||||
wantEqual bool
|
||||
}{
|
||||
{
|
||||
name: "Legacy",
|
||||
passwordToValidate: "$pbkdf2-sha256$65535$z8c1p1C2ru9EImBP1I+ZNA$pNjE3Yk0oG0PmJ0Je+y7ENOVlSkn/b0BEqqdKsq6Y97wQBq0xT+lD5bWJpyIKJqQICuPZcEaGDKrXJn8+SIHRg",
|
||||
password: "tomato",
|
||||
shouldHash: false,
|
||||
wantErr: false,
|
||||
wantEqual: true,
|
||||
},
|
||||
{
|
||||
name: "Same",
|
||||
passwordToValidate: "password",
|
||||
password: "password",
|
||||
shouldHash: true,
|
||||
wantErr: false,
|
||||
wantEqual: true,
|
||||
},
|
||||
{
|
||||
name: "Different",
|
||||
passwordToValidate: "password",
|
||||
password: "notpassword",
|
||||
shouldHash: true,
|
||||
wantErr: false,
|
||||
wantEqual: false,
|
||||
},
|
||||
{
|
||||
name: "Invalid",
|
||||
passwordToValidate: "invalidhash",
|
||||
password: "password",
|
||||
shouldHash: false,
|
||||
wantErr: true,
|
||||
wantEqual: false,
|
||||
},
|
||||
{
|
||||
name: "InvalidParts",
|
||||
passwordToValidate: "abcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyz",
|
||||
password: "test",
|
||||
shouldHash: false,
|
||||
wantErr: true,
|
||||
wantEqual: false,
|
||||
},
|
||||
}
|
||||
|
||||
t.Run("Invalid", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
equal, err := userpassword.Compare("invalidhash", "password")
|
||||
require.False(t, equal)
|
||||
require.Error(t, err)
|
||||
})
|
||||
|
||||
t.Run("InvalidParts", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
equal, err := userpassword.Compare("abcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyz", "test")
|
||||
require.False(t, equal)
|
||||
require.Error(t, err)
|
||||
})
|
||||
for _, tt := range tests {
|
||||
tt := tt
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
t.Parallel()
|
||||
if tt.shouldHash {
|
||||
hash, err := userpassword.Hash(tt.passwordToValidate)
|
||||
require.NoError(t, err)
|
||||
tt.passwordToValidate = hash
|
||||
}
|
||||
equal, err := userpassword.Compare(tt.passwordToValidate, tt.password)
|
||||
if tt.wantErr {
|
||||
require.Error(t, err)
|
||||
} else {
|
||||
require.NoError(t, err)
|
||||
}
|
||||
require.Equal(t, tt.wantEqual, equal)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1219,6 +1219,24 @@ func TestUpdateUserPassword(t *testing.T) {
|
||||
require.Equal(t, database.AuditActionWrite, auditor.AuditLogs()[numLogs-1].Action)
|
||||
})
|
||||
|
||||
t.Run("ValidateUserPassword", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
auditor := audit.NewMock()
|
||||
client := coderdtest.New(t, &coderdtest.Options{Auditor: auditor})
|
||||
|
||||
_ = coderdtest.CreateFirstUser(t, client)
|
||||
|
||||
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
|
||||
defer cancel()
|
||||
|
||||
resp, err := client.ValidateUserPassword(ctx, codersdk.ValidateUserPasswordRequest{
|
||||
Password: "MySecurePassword!",
|
||||
})
|
||||
|
||||
require.NoError(t, err, "users shoud be able to validate complexity of a potential new password")
|
||||
require.True(t, resp.Valid)
|
||||
})
|
||||
|
||||
t.Run("ChangingPasswordDeletesKeys", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
|
||||
Reference in New Issue
Block a user