From e32694de0fbd1e8c85aee0234c2da01d4f6671bb Mon Sep 17 00:00:00 2001 From: Roman Perekhod Date: Tue, 19 Sep 2023 17:31:58 +0200 Subject: [PATCH] add ability to validation against the Banned-Passwords List --- .../add-passwod-banned-password-list.md | 5 ++++ .../owncloud/ocs/data/capabilities.go | 13 +++++----- .../handlers/apps/sharing/shares/public.go | 4 ++-- .../handlers/apps/sharing/shares/shares.go | 3 ++- pkg/password/password_policies.go | 24 +++++++++++++++++-- pkg/password/password_policies_test.go | 16 +++++++++++++ 6 files changed, 54 insertions(+), 11 deletions(-) create mode 100644 changelog/unreleased/add-passwod-banned-password-list.md diff --git a/changelog/unreleased/add-passwod-banned-password-list.md b/changelog/unreleased/add-passwod-banned-password-list.md new file mode 100644 index 00000000000..8819b1cfa5b --- /dev/null +++ b/changelog/unreleased/add-passwod-banned-password-list.md @@ -0,0 +1,5 @@ +Enhancement: Add the Banned-Passwords List + +Add ability to validation against the Banned-Passwords List OCIS-3809 + +https://github.com/cs3org/reva/pull/4197 diff --git a/internal/http/services/owncloud/ocs/data/capabilities.go b/internal/http/services/owncloud/ocs/data/capabilities.go index 27f24561666..377d2c13018 100644 --- a/internal/http/services/owncloud/ocs/data/capabilities.go +++ b/internal/http/services/owncloud/ocs/data/capabilities.go @@ -88,12 +88,13 @@ type CapabilitiesGraph struct { // CapabilitiesPasswordPolicy hold the password policy capabilities type CapabilitiesPasswordPolicy struct { - MinCharacters int `json:"min_characters" xml:"min_characters" mapstructure:"min_characters"` - MaxCharacters int `json:"max_characters" xml:"max_characters" mapstructure:"max_characters"` - MinLowerCaseCharacters int `json:"min_lowercase_characters" xml:"min_lowercase_characters" mapstructure:"min_lowercase_characters"` - MinUpperCaseCharacters int `json:"min_uppercase_characters" xml:"min_uppercase_characters" mapstructure:"min_uppercase_characters"` - MinDigits int `json:"min_digits" xml:"min_digits" mapstructure:"min_digits"` - MinSpecialCharacters int `json:"min_special_characters" xml:"min_special_characters" mapstructure:"min_special_characters"` + MinCharacters int `json:"min_characters" xml:"min_characters" mapstructure:"min_characters"` + MaxCharacters int `json:"max_characters" xml:"max_characters" mapstructure:"max_characters"` + MinLowerCaseCharacters int `json:"min_lowercase_characters" xml:"min_lowercase_characters" mapstructure:"min_lowercase_characters"` + MinUpperCaseCharacters int `json:"min_uppercase_characters" xml:"min_uppercase_characters" mapstructure:"min_uppercase_characters"` + MinDigits int `json:"min_digits" xml:"min_digits" mapstructure:"min_digits"` + MinSpecialCharacters int `json:"min_special_characters" xml:"min_special_characters" mapstructure:"min_special_characters"` + BannedPasswordsList map[string]struct{} `mapstructure:"banned_passwords_list"` } // CapabilitiesGraphUsers holds the graph user capabilities diff --git a/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/public.go b/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/public.go index bdb96e3d903..c62b730027f 100644 --- a/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/public.go +++ b/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/public.go @@ -153,7 +153,7 @@ func (h *Handler) createPublicLinkShare(w http.ResponseWriter, r *http.Request, if err := h.passwordValidator.Validate(password); err != nil { return nil, &ocsError{ Code: response.MetaBadRequest.StatusCode, - Message: "password validation failed", + Message: err.Error(), Error: fmt.Errorf("password validation failed: %w", err), } } @@ -479,7 +479,7 @@ func (h *Handler) updatePublicShare(w http.ResponseWriter, r *http.Request, shar // skip validation if the clear password scenario if len(newPassword[0]) > 0 { if err := h.passwordValidator.Validate(newPassword[0]); err != nil { - response.WriteOCSError(w, r, response.MetaBadRequest.StatusCode, fmt.Errorf("missing required password %w", err).Error(), err) + response.WriteOCSError(w, r, response.MetaBadRequest.StatusCode, err.Error(), err) return } } diff --git a/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/shares.go b/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/shares.go index 28ed61adc77..6b64f8a1f51 100644 --- a/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/shares.go +++ b/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/shares.go @@ -1592,7 +1592,7 @@ func publicPwdEnforced(c *config.Config) passwordEnforced { func passwordPolicies(c *config.Config) password.Validator { if c.Capabilities.Capabilities == nil || c.Capabilities.Capabilities.PasswordPolicy == nil { - return password.NewPasswordPolicy(0, 0, 0, 0, 0) + return password.NewPasswordPolicy(0, 0, 0, 0, 0, nil) } return password.NewPasswordPolicy( c.Capabilities.Capabilities.PasswordPolicy.MinCharacters, @@ -1600,6 +1600,7 @@ func passwordPolicies(c *config.Config) password.Validator { c.Capabilities.Capabilities.PasswordPolicy.MinUpperCaseCharacters, c.Capabilities.Capabilities.PasswordPolicy.MinDigits, c.Capabilities.Capabilities.PasswordPolicy.MinSpecialCharacters, + c.Capabilities.Capabilities.PasswordPolicy.BannedPasswordsList, ) } diff --git a/pkg/password/password_policies.go b/pkg/password/password_policies.go index 4cada0b9d34..75ffb25b6db 100644 --- a/pkg/password/password_policies.go +++ b/pkg/password/password_policies.go @@ -23,18 +23,24 @@ type Policies struct { minUpperCaseCharacters int minDigits int minSpecialCharacters int + bannedPasswordsList map[string]struct{} digitsRegexp *regexp.Regexp specialCharactersRegexp *regexp.Regexp } // NewPasswordPolicy returns a new NewPasswordPolicy instance -func NewPasswordPolicy(minCharacters, minLowerCaseCharacters, minUpperCaseCharacters, minDigits, minSpecialCharacters int) Validator { +func NewPasswordPolicy(minCharacters, minLowerCaseCharacters, minUpperCaseCharacters, minDigits, minSpecialCharacters int, + bannedPasswordsList map[string]struct{}) Validator { + if len(bannedPasswordsList) == 0 { + bannedPasswordsList = nil + } p := &Policies{ minCharacters: minCharacters, minLowerCaseCharacters: minLowerCaseCharacters, minUpperCaseCharacters: minUpperCaseCharacters, minDigits: minDigits, minSpecialCharacters: minSpecialCharacters, + bannedPasswordsList: bannedPasswordsList, } p.digitsRegexp = regexp.MustCompile("[0-9]") @@ -48,7 +54,11 @@ func (s Policies) Validate(str string) error { if !utf8.ValidString(str) { return fmt.Errorf("the password contains invalid characters") } - err := s.validateCharacters(str) + err := s.validateBannedList(str) + if err != nil { + return err + } + err = s.validateCharacters(str) if err != nil { allErr = errors.Join(allErr, err) } @@ -74,6 +84,16 @@ func (s Policies) Validate(str string) error { return nil } +func (s Policies) validateBannedList(str string) error { + if s.bannedPasswordsList == nil { + return nil + } + if _, ok := s.bannedPasswordsList[str]; ok { + return fmt.Errorf("weak password is not allowed") + } + return nil +} + func (s Policies) validateCharacters(str string) error { if s.count(str) < s.minCharacters { return fmt.Errorf("at least %d characters are required", s.minCharacters) diff --git a/pkg/password/password_policies_test.go b/pkg/password/password_policies_test.go index f1874045d54..72d5c7c473e 100644 --- a/pkg/password/password_policies_test.go +++ b/pkg/password/password_policies_test.go @@ -11,6 +11,7 @@ func TestPolicies_Validate(t *testing.T) { minUpperCaseCharacters int minDigits int minSpecialCharacters int + bannedPasswordsList map[string]struct{} } tests := []struct { name string @@ -52,6 +53,19 @@ func TestPolicies_Validate(t *testing.T) { args: "0äÖ-", wantErr: true, }, + { + name: "banned list", + fields: fields{ + minCharacters: 10, + minLowerCaseCharacters: 3, + minUpperCaseCharacters: 3, + minDigits: 3, + minSpecialCharacters: 1, + bannedPasswordsList: map[string]struct{}{"123abcABC!": struct{}{}}, + }, + args: "123abcABC!", + wantErr: true, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -61,6 +75,7 @@ func TestPolicies_Validate(t *testing.T) { tt.fields.minUpperCaseCharacters, tt.fields.minDigits, tt.fields.minSpecialCharacters, + tt.fields.bannedPasswordsList, ) if err := s.Validate(tt.args); (err != nil) != tt.wantErr { t.Errorf("Validate() error = %v, wantErr %v", err.Error(), tt.wantErr) @@ -125,6 +140,7 @@ func TestPasswordPolicies_Count(t *testing.T) { tt.fields.wantUpperCaseCharacters, tt.fields.wantDigits, tt.fields.wantSpecialCharacters, + nil, ) s := i.(*Policies) if got := s.count(tt.args); got != tt.fields.wantCharacters {