Add a password policy for distillery users

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 {
+		if err := user.CheckPasswordPolicy(passwd); err != nil {
-			return errPasswordsNotIdentical
+			return errPasswordPolicy.WithMessageF(err)
 		}
 	}
 

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 {

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")
+const MinPasswordLength = 8
-var ErrUserDisabled = errors.New("user is disabled")
+
-var ErrUserBlank = errors.New("user has no password set")
+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.

internal/dis/component/control/admin/html/users.html

@@ -14,9 +14,20 @@
 {{ end }}
 
 {{ define "content" }}
+
 <div class="pure-u-1">
     <div class="padding">
         <div class="overflow">
+
+            {{ $E := .Error }}
+            {{ if not (eq $E "") }}
+                <div class="pure-form-group">
+                    <p class="error-message">
+                        {{ $E }}
+                    </p>
+                </div>
+            {{ end }}
+
             <table class="pure-table pure-table-bordered">
                 <thead>
                     <tr>

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))
 	})
 }

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
-}

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
+}