Merge pull request #4170 from 2403905/OCIS-3767-add

Password policies update

changelog/unreleased/update-passwod-policies.md

@@ -0,0 +1,7 @@
+Enhancement: Update password policies
+
+The Password policies have been updated.
+The special characters list became constant.
+OCIS-3767
+
+https://github.com/cs3org/reva/pull/4170

internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/shares.go

@@ -126,7 +126,6 @@ type GatewayClientGetter func() (gateway.GatewayAPIClient, error)
 
 // Init initializes this and any contained handlers
 func (h *Handler) Init(c *config.Config) error {
-	var err error
 	h.gatewayAddr = c.GatewaySvc
 	h.machineAuthAPIKey = c.MachineAuthAPIKey
 	h.storageRegistryAddr = c.StorageregistrySvc
@@ -142,10 +141,7 @@ func (h *Handler) Init(c *config.Config) error {
 	h.deniable = c.EnableDenials
 	h.resharing = resharing(c)
 	h.publicPasswordEnforced = publicPwdEnforced(c)
-	h.passwordValidator, err = passwordPolicies(c)
-	if err != nil {
-		return err
-	}
+	h.passwordValidator = passwordPolicies(c)
 
 	h.statCache = cache.GetStatCache(c.StatCacheStore, c.StatCacheNodes, c.StatCacheDatabase, "stat", time.Duration(c.StatCacheTTL)*time.Second, c.StatCacheSize)
 	if c.CacheWarmupDriver != "" {
@@ -1594,28 +1590,17 @@ func publicPwdEnforced(c *config.Config) passwordEnforced {
 	return enf
 }
 
-func passwordPolicies(c *config.Config) (password.Validator, error) {
-	var pv password.Validator
-	var err error
+func passwordPolicies(c *config.Config) password.Validator {
 	if c.Capabilities.Capabilities == nil || c.Capabilities.Capabilities.PasswordPolicies == nil {
-		pv, err = password.NewPasswordPolicies(0, 0, 0, 0, 0, "")
-		if err != nil {
-			return nil, fmt.Errorf("can't init the Password Policies %w", err)
-		}
-		return pv, nil
+		return password.NewPasswordPolicies(0, 0, 0, 0, 0)
 	}
-	pv, err = password.NewPasswordPolicies(
+	return password.NewPasswordPolicies(
 		c.Capabilities.Capabilities.PasswordPolicies.MinCharacters,
 		c.Capabilities.Capabilities.PasswordPolicies.MinLowerCaseCharacters,
 		c.Capabilities.Capabilities.PasswordPolicies.MinUpperCaseCharacters,
 		c.Capabilities.Capabilities.PasswordPolicies.MinDigits,
 		c.Capabilities.Capabilities.PasswordPolicies.MinSpecialCharacters,
-		c.Capabilities.Capabilities.PasswordPolicies.SpecialCharacters,
 	)
-	if err != nil {
-		return nil, fmt.Errorf("can't init the Password Policies %w", err)
-	}
-	return pv, nil
 }
 
 // sufficientPermissions returns true if the `existing` permissions contain the `requested` permissions

pkg/password/password_policies.go

@@ -8,6 +8,9 @@ import (
 	"unicode/utf8"
 )
 
+// https://owasp.org/www-community/password-special-characters
+var _defaultSpecialCharacters = " !\"#$%&'()*+,-./:;<=>?@[\\]^_`{|}~"
+
 // Validator describes the interface providing a password Validate method
 type Validator interface {
 	Validate(str string) error
@@ -20,32 +23,23 @@ type Policies struct {
 	minUpperCaseCharacters  int
 	minDigits               int
 	minSpecialCharacters    int
-	specialCharacters       string
 	digitsRegexp            *regexp.Regexp
 	specialCharactersRegexp *regexp.Regexp
 }
 
 // NewPasswordPolicies returns a new NewPasswordPolicies instance
-func NewPasswordPolicies(minCharacters, minLowerCaseCharacters, minUpperCaseCharacters, minDigits, minSpecialCharacters int,
-	specialCharacters string) (Validator, error) {
+func NewPasswordPolicies(minCharacters, minLowerCaseCharacters, minUpperCaseCharacters, minDigits, minSpecialCharacters int) Validator {
 	p := &Policies{
 		minCharacters:          minCharacters,
 		minLowerCaseCharacters: minLowerCaseCharacters,
 		minUpperCaseCharacters: minUpperCaseCharacters,
 		minDigits:              minDigits,
 		minSpecialCharacters:   minSpecialCharacters,
-		specialCharacters:      specialCharacters,
 	}
 
 	p.digitsRegexp = regexp.MustCompile("[0-9]")
-	if len(specialCharacters) > 0 {
-		var err error
-		p.specialCharactersRegexp, err = regexp.Compile(specialCharactersExp(specialCharacters))
-		if err != nil {
-			return nil, err
-		}
-	}
-	return p, nil
+	p.specialCharactersRegexp = regexp.MustCompile(specialCharactersExp(_defaultSpecialCharacters))
+	return p
 }
 
 // Validate implements a password validation regarding the policy
@@ -110,7 +104,7 @@ func (s Policies) validateDigits(str string) error {
 
 func (s Policies) validateSpecialCharacters(str string) error {
 	if s.countSpecialCharacters(str) < s.minSpecialCharacters {
-		return fmt.Errorf("at least %d special characters are required. %s", s.minSpecialCharacters, s.specialCharacters)
+		return fmt.Errorf("at least %d special characters are required. %s", s.minSpecialCharacters, _defaultSpecialCharacters)
 	}
 	return nil
 }

pkg/password/password_policies_test.go

@@ -4,16 +4,13 @@ import (
 	"testing"
 )
 
-var _defaultSpecialCharacters = "!\"#$%&'()*+,-./:;<=>?@[\\]^_`{|}~"
-
 func TestPolicies_Validate(t *testing.T) {
 	type fields struct {
 		minCharacters          int
 		minLowerCaseCharacters int
 		minUpperCaseCharacters int
 		minDigits              int
 		minSpecialCharacters   int
-		specialCharacters      string
 	}
 	tests := []struct {
 		name    string
@@ -29,44 +26,19 @@ func TestPolicies_Validate(t *testing.T) {
 				minUpperCaseCharacters: 29,
 				minDigits:              10,
 				minSpecialCharacters:   32,
-				specialCharacters:      _defaultSpecialCharacters,
 			},
-			args: "1234567890abcdefghijklmnopqrstuvwxyzäöüABCDEFGHIJKLMNOPQRSTUVWXYZÄÖÜ!\"#$%&'()*+,-./:;<=>?@[\\]^_`{|}~",
+			args: "1234567890abcdefghijklmnopqrstuvwxyzäöüABCDEFGHIJKLMNOPQRSTUVWXYZÄÖÜ !\"#$%&'()*+,-./:;<=>?@[\\]^_`{|}~",
 		},
 		{
-			name: "exactly one",
+			name: "exactly",
 			fields: fields{
-				minCharacters:          1,
-				minLowerCaseCharacters: 1,
-				minUpperCaseCharacters: 1,
+				minCharacters:          19,
+				minLowerCaseCharacters: 7,
+				minUpperCaseCharacters: 7,
 				minDigits:              1,
 				minSpecialCharacters:   1,
-				specialCharacters:      "-",
-			},
-			args: "0äÖ-",
-		},
-		{
-			name: "exactly + special",
-			fields: fields{
-				minCharacters:          12,
-				minLowerCaseCharacters: 3,
-				minUpperCaseCharacters: 3,
-				minDigits:              1,
-				minSpecialCharacters:   9,
-				specialCharacters:      "-世界 ßAaBb",
-			},
-			args: "0äÖ-世界 ßAaBb",
-		},
-		{
-			name: "exactly cyrillic",
-			fields: fields{
-				minCharacters:          6,
-				minLowerCaseCharacters: 3,
-				minUpperCaseCharacters: 3,
-				minDigits:              0,
-				minSpecialCharacters:   0,
 			},
-			args: "іІїЇЯяЙй",
+			args: "0äÖ-世界іІїЇЯяЙйßAaBb",
 		},
 		{
 			name: "error",
@@ -76,25 +48,20 @@ func TestPolicies_Validate(t *testing.T) {
 				minUpperCaseCharacters: 2,
 				minDigits:              2,
 				minSpecialCharacters:   2,
-				specialCharacters:      "",
 			},
 			args:    "0äÖ-",
 			wantErr: true,
 		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			s, err := NewPasswordPolicies(
+			s := NewPasswordPolicies(
 				tt.fields.minCharacters,
 				tt.fields.minLowerCaseCharacters,
 				tt.fields.minUpperCaseCharacters,
 				tt.fields.minDigits,
 				tt.fields.minSpecialCharacters,
-				tt.fields.specialCharacters,
 			)
-			if err != nil {
-				t.Error(err)
-			}
 			if err := s.Validate(tt.args); (err != nil) != tt.wantErr {
 				t.Errorf("Validate() error = %v, wantErr %v", err.Error(), tt.wantErr)
 			}
@@ -109,7 +76,6 @@ func TestPasswordPolicies_Count(t *testing.T) {
 		wantUpperCaseCharacters int
 		wantDigits              int
 		wantSpecialCharacters   int
-		specialCharacters       string
 	}
 	tests := []struct {
 		name    string
@@ -120,76 +86,46 @@ func TestPasswordPolicies_Count(t *testing.T) {
 		{
 			name: "all in one",
 			fields: want{
-				wantCharacters:          100,
+				wantCharacters:          101,
 				wantLowerCaseCharacters: 29,
 				wantUpperCaseCharacters: 29,
 				wantDigits:              10,
-				wantSpecialCharacters:   32,
-				specialCharacters:       _defaultSpecialCharacters,
+				wantSpecialCharacters:   33,
 			},
-			args: "1234567890abcdefghijklmnopqrstuvwxyzäöüABCDEFGHIJKLMNOPQRSTUVWXYZÄÖÜ!\"#$%&'()*+,-./:;<=>?@[\\]^_`{|}~",
+			args: "1234567890abcdefghijklmnopqrstuvwxyzäöüABCDEFGHIJKLMNOPQRSTUVWXYZÄÖÜ !\"#$%&'()*+,-./:;<=>?@[\\]^_`{|}~",
 		},
 		{
 			name: "length only",
 			fields: want{
-				wantCharacters:          4,
+				wantCharacters:          3,
 				wantLowerCaseCharacters: 0,
 				wantUpperCaseCharacters: 0,
 				wantDigits:              0,
 				wantSpecialCharacters:   0,
 			},
-			args: "世界 ß",
+			args: "世界ß",
 		},
 		{
 			name: "length only",
 			fields: want{
-				wantCharacters:          8,
-				wantLowerCaseCharacters: 2,
-				wantUpperCaseCharacters: 2,
-				wantDigits:              0,
-				wantSpecialCharacters:   8,
-				specialCharacters:       "世界 ßAaBb",
-			},
-			args: "世界 ßAaBb",
-		},
-		{
-			name: "empty",
-			fields: want{
-				wantCharacters:          0,
-				wantLowerCaseCharacters: 0,
-				wantUpperCaseCharacters: 0,
-				wantDigits:              0,
-				wantSpecialCharacters:   0,
-			},
-			args: "",
-		},
-		{
-			name: "check '-' sing",
-			fields: want{
-				wantCharacters:          33,
-				wantLowerCaseCharacters: 0,
-				wantUpperCaseCharacters: 0,
-				wantDigits:              0,
-				wantSpecialCharacters:   5,
-				specialCharacters:       `_!-+`,
+				wantCharacters:          21,
+				wantLowerCaseCharacters: 7,
+				wantUpperCaseCharacters: 7,
+				wantDigits:              1,
+				wantSpecialCharacters:   3,
 			},
-			args: "!!\"#$%&'()*+,-./:;<=>?@[\\]^_`{|}~",
+			args: "0äÖ-世界 іІїЇЯяЙй ßAaBb",
 		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			i, err := NewPasswordPolicies(
+			i := NewPasswordPolicies(
 				tt.fields.wantCharacters,
 				tt.fields.wantLowerCaseCharacters,
 				tt.fields.wantUpperCaseCharacters,
 				tt.fields.wantDigits,
 				tt.fields.wantSpecialCharacters,
-				tt.fields.specialCharacters,
 			)
-			if err != nil {
-				t.Error(err)
-				return
-			}
 			s := i.(*Policies)
 			if got := s.count(tt.args); got != tt.fields.wantCharacters {
 				t.Errorf("count() = %v, want %v", got, tt.fields.wantCharacters)