diff --git a/cmd/dis_user.go b/cmd/dis_user.go index c1536c1..d1927d6 100644 --- a/cmd/dis_user.go +++ b/cmd/dis_user.go @@ -135,6 +135,11 @@ func (du disUser) runDelete(context wisski_distillery.Context) error { return user.Delete(context.Context) } +var errPasswordPolicy = exit.Error{ + Message: "password policy failed: %s", + ExitCode: exit.ExitGeneric, +} + func (du disUser) runSetPassword(context wisski_distillery.Context) error { user, err := context.Environment.Auth().User(context.Context, du.Positionals.User) if err != nil { @@ -160,8 +165,8 @@ func (du disUser) runSetPassword(context wisski_distillery.Context) error { if passwd != passwd1 { return errPasswordsNotIdentical } - if len(passwd) == 0 { - return errPasswordsNotIdentical + if err := user.CheckPasswordPolicy(passwd); err != nil { + return errPasswordPolicy.WithMessageF(err) } } diff --git a/internal/dis/component/auth/panel/password.go b/internal/dis/component/auth/panel/password.go index ea4022e..0b65293 100644 --- a/internal/dis/component/auth/panel/password.go +++ b/internal/dis/component/auth/panel/password.go @@ -18,7 +18,6 @@ var passwordTemplate = static.AssetsUser.MustParseShared("password.html", passwo var ( errPasswordsNotIdentical = errors.New("passwords are not identical") - errPasswordIsEmpty = errors.New("password is empty") errCredentialsIncorrect = errors.New("credentials are not correct") errPasswordSetFailure = errors.New("error saving new password") errTOTPSetFailure = errors.New("unable to disable totp") @@ -47,10 +46,6 @@ func (panel *UserPanel) routePassword(ctx context.Context) http.Handler { return struct{}{}, errPasswordsNotIdentical } - if new == "" { - return struct{}{}, errPasswordIsEmpty - } - user, err := panel.Dependencies.Auth.UserOf(r) if err != nil { return struct{}{}, err @@ -62,6 +57,14 @@ func (panel *UserPanel) routePassword(ctx context.Context) http.Handler { return struct{}{}, errCredentialsIncorrect } } + + { + err := user.CheckPasswordPolicy(new) + if err != nil { + return struct{}{}, err + } + } + { err := user.SetPassword(r.Context(), []byte(new)) if err != nil { diff --git a/internal/dis/component/auth/user.go b/internal/dis/component/auth/user.go index 67856da..73f9b95 100644 --- a/internal/dis/component/auth/user.go +++ b/internal/dis/component/auth/user.go @@ -6,9 +6,11 @@ import ( "encoding/base64" "fmt" "image/png" + "strings" "github.com/FAU-CDI/wisski-distillery/internal/dis/component" "github.com/FAU-CDI/wisski-distillery/internal/models" + "github.com/FAU-CDI/wisski-distillery/pkg/password" "github.com/pkg/errors" "github.com/pquerna/otp" "github.com/pquerna/otp/totp" @@ -232,9 +234,49 @@ func (au *AuthUser) UnsetPassword(ctx context.Context) error { return au.Save(ctx) } -var ErrNoUser = errors.New("user is nil") -var ErrUserDisabled = errors.New("user is disabled") -var ErrUserBlank = errors.New("user has no password set") +const MinPasswordLength = 8 + +var ( + ErrPolicyBlank = errors.New("password is blank") + ErrPolicyTooShort = errors.New(fmt.Sprintf("password is too short: minimum length %d", MinPasswordLength)) + ErrPolicyKnown = errors.New("password is on the list of known passwords") + ErrPolicyUsername = errors.New("password may not be identical to username") +) + +// CheckPasswordPolicy checks if the given password would pass the password policy. +// +// The password policy checks that the password has a minimum length of [MinPasswordLength] +// and that it is not a common password. +// It also checks that password and username are not identical. +func (auth *Auth) CheckPasswordPolicy(candidate string, username string) error { + if candidate == "" { + return ErrPolicyBlank + } + + if strings.EqualFold(candidate, username) { + return ErrPolicyUsername + } + + if len(candidate) < MinPasswordLength { + return ErrPolicyTooShort + } + + if err := password.CheckCommonPassword(func(common string) (bool, error) { return common == candidate, nil }); err != nil { + return ErrPolicyKnown + } + + return nil +} + +func (au *AuthUser) CheckPasswordPolicy(candidate string) error { + return au.auth.CheckPasswordPolicy(candidate, au.User.User) +} + +var ( + ErrNoUser = errors.New("user is nil") + ErrUserDisabled = errors.New("user is disabled") + ErrUserBlank = errors.New("user has no password set") +) // CheckPassword checks if this user can login with the provided password. // Returns nil on success, an error otherwise. diff --git a/internal/dis/component/control/admin/html/users.html b/internal/dis/component/control/admin/html/users.html index 7fbf4d4..596927b 100644 --- a/internal/dis/component/control/admin/html/users.html +++ b/internal/dis/component/control/admin/html/users.html @@ -14,9 +14,20 @@ {{ end }} {{ define "content" }} +
+ + {{ $E := .Error }} + {{ if not (eq $E "") }} +
+

+ {{ $E }} +

+
+ {{ end }} + diff --git a/internal/dis/component/control/admin/users.go b/internal/dis/component/control/admin/users.go index c41be19..fae70bc 100644 --- a/internal/dis/component/control/admin/users.go +++ b/internal/dis/component/control/admin/users.go @@ -4,6 +4,7 @@ import ( "context" "errors" "net/http" + "net/url" _ "embed" @@ -24,14 +25,15 @@ var userTemplate = static.AssetsAdmin.MustParseShared( type userContext struct { custom.BaseContext - httpx.FormContext + Error string Users []*auth.AuthUser } func (admin *Admin) users(r *http.Request) (uc userContext, err error) { admin.Dependencies.Custom.Update(&uc, r) + uc.Error = r.URL.Query().Get("error") uc.Users, err = admin.Dependencies.Auth.Users(r.Context()) return } @@ -78,6 +80,12 @@ func (admin *Admin) createUser(ctx context.Context) http.Handler { return cu, errCreateInvalidPassword } + // check the password policy + err = admin.Dependencies.Auth.CheckPasswordPolicy(cu.Passsword, cu.User) + if err != nil { + return cu, err + } + return cu, nil }, @@ -144,7 +152,7 @@ func (admin *Admin) useraction(ctx context.Context, name string, action func(r * if err := action(r, user); err != nil { logger.Err(err).Str("action", name).Msg("failed to act on user") - httpx.HTMLInterceptor.Fallback.ServeHTTP(w, r) + http.Redirect(w, r, "/admin/users/?error="+url.QueryEscape(err.Error()), http.StatusSeeOther) return } @@ -185,6 +193,11 @@ func (admin *Admin) usersPasswordHandler(ctx context.Context) http.Handler { if password == "" { return httpx.ErrBadRequest } + // check the password policy + err := user.CheckPasswordPolicy(password) + if err != nil { + return err + } return user.SetPassword(r.Context(), []byte(password)) }) } diff --git a/internal/wisski/ingredient/php/users/password.go b/internal/wisski/ingredient/php/users/password.go index 5939ecc..c502e3e 100644 --- a/internal/wisski/ingredient/php/users/password.go +++ b/internal/wisski/ingredient/php/users/password.go @@ -1,16 +1,13 @@ package users import ( - "bufio" "context" - "embed" "errors" "fmt" "io" - "io/fs" - "strings" "github.com/FAU-CDI/wisski-distillery/internal/phpx" + "github.com/FAU-CDI/wisski-distillery/pkg/password" ) var errGetValidator = errors.New("GetPasswordValidator: Unknown Error") @@ -57,14 +54,6 @@ func (pv PasswordValidator) Check(ctx context.Context, password string) bool { var errPasswordUsername = errors.New("username === password") -type CommonPasswordError struct { - Password CommonPassword -} - -func (cpe CommonPasswordError) Error() string { - return fmt.Sprintf("%q from %q", cpe.Password.Password, cpe.Password.Source) -} - func (pv PasswordValidator) CheckDictionary(ctx context.Context, writer io.Writer) error { var counter int @@ -75,7 +64,7 @@ func (pv PasswordValidator) CheckDictionary(ctx context.Context, writer io.Write } return errPasswordUsername } - for candidate := range CommonPasswords() { + for candidate := range password.CommonPasswords() { if ctx.Err() != nil { continue } @@ -86,59 +75,9 @@ func (pv PasswordValidator) CheckDictionary(ctx context.Context, writer io.Write } if result { - return &CommonPasswordError{Password: candidate} + return &password.CommonPasswordError{CommonPassword: candidate} } } return ctx.Err() } - -//go:embed passwords -var passwordsEmbed embed.FS - -type CommonPassword struct { - Password string - Source string -} - -// CommonPasswords returns a channel of most common passwords -func CommonPasswords() <-chan CommonPassword { - pChan := make(chan CommonPassword, 10) - go func() { - defer close(pChan) - - fs.WalkDir(passwordsEmbed, ".", func(path string, d fs.DirEntry, err error) error { - if err != nil { - return err - } - - // get the full path - if d.IsDir() || !strings.HasSuffix(path, ".txt") { - return nil - } - - // open it - file, err := passwordsEmbed.Open(path) - if err != nil { - return err - } - defer file.Close() - - // scan it line by line - scanner := bufio.NewScanner(file) - for scanner.Scan() { - line := strings.TrimSpace(scanner.Text()) - if line == "" || strings.HasPrefix(line, "//") { - continue - } - pChan <- CommonPassword{ - Password: line, - Source: path, - } - } - - return scanner.Err() - }) - }() - return pChan -} diff --git a/pkg/password/common.go b/pkg/password/common.go new file mode 100644 index 0000000..df359b2 --- /dev/null +++ b/pkg/password/common.go @@ -0,0 +1,96 @@ +package password + +import ( + "bufio" + "embed" + "fmt" + "io/fs" + "strings" +) + +// CommonPasswordError +type CommonPasswordError struct { + CommonPassword +} + +func (cpe CommonPasswordError) Error() string { + return fmt.Sprintf("%q from %q", cpe.Password, cpe.Source) +} + +type CommonPassword struct { + Password string + Source string +} + +//go:embed common +var commonEmbed embed.FS + +// CommonPasswords returns a channel that contains all passwords. +// The caller must drain the channel. +func CommonPasswords() <-chan CommonPassword { + pChan := make(chan CommonPassword, 10) + go func() { + defer close(pChan) + + fs.WalkDir(commonEmbed, ".", func(path string, d fs.DirEntry, err error) error { + if err != nil { + return err + } + + // get the full path + if d.IsDir() || !strings.HasSuffix(path, ".txt") { + return nil + } + + // open it + file, err := commonEmbed.Open(path) + if err != nil { + return err + } + defer file.Close() + + // scan it line by line + scanner := bufio.NewScanner(file) + for scanner.Scan() { + line := strings.TrimSpace(scanner.Text()) + if line == "" || strings.HasPrefix(line, "//") { + continue + } + pChan <- CommonPassword{ + Password: line, + Source: path, + } + } + + return scanner.Err() + }) + }() + return pChan +} + +// CheckCommonPassword checks if a password is a common password. +// +// check is called with each candidate password to perform the check. +// check should return a boolean indicating if the password in question corresponds to the candidate. +// +// CheckCommonPassword returns one of three error values. +// +// - a CommonPasswordError (when a password matches a common password) +// - an error returned by check (assuming some check went wrong) +// - or nil (when a password is not a common password +func CheckCommonPassword(check func(candidate string) (bool, error)) error { + for commmon := range CommonPasswords() { + ok, err := check(commmon.Password) + if err != nil { + return err + } + + // password validation passed + if ok { + return CommonPasswordError{ + CommonPassword: commmon, + } + } + } + return nil +} diff --git a/internal/wisski/ingredient/php/users/passwords/top10_000.txt b/pkg/password/common/top10_000.txt similarity index 100% rename from internal/wisski/ingredient/php/users/passwords/top10_000.txt rename to pkg/password/common/top10_000.txt diff --git a/internal/wisski/ingredient/php/users/passwords/wisski.txt b/pkg/password/common/wisski.txt similarity index 100% rename from internal/wisski/ingredient/php/users/passwords/wisski.txt rename to pkg/password/common/wisski.txt