added in checking of password strength when creating a new user

MFrank2016 · Mar 13, 2020 · 946fb66 · 946fb66
1 parent 33ed4b5
commit 946fb66
Show file tree

Hide file tree

Showing 2 changed files with 59 additions and 13 deletions.
diff --git a/api/keystore/service.go b/api/keystore/service.go
@@ -21,16 +21,30 @@ import (
 	"github.com/ava-labs/gecko/vms/components/codec"
 
 	jsoncodec "github.com/ava-labs/gecko/utils/json"
+	zxcvbn "github.com/nbutton23/zxcvbn-go"
 )
 
 const (
 	// maxUserPassLen is the maximum length of the username or password allowed
 	maxUserPassLen = 1024
+
+	// requiredPassScore defines the score a password must achieve to be accepted
+	// as a password with strong characteristics by the zxcvbn package
+	//
+	// The scoring mechanism defined is as follows;
+	//
+	// 0 # too guessable: risky password. (guesses < 10^3)
+	// 1 # very guessable: protection from throttled online attacks. (guesses < 10^6)
+	// 2 # somewhat guessable: protection from unthrottled online attacks. (guesses < 10^8)
+	// 3 # safely unguessable: moderate protection from offline slow-hash scenario. (guesses < 10^10)
+	// 4 # very unguessable: strong protection from offline slow-hash scenario. (guesses >= 10^10)
+	requiredPassScore = 2
 )
 
 var (
 	errEmptyUsername     = errors.New("username can't be the empty string")
 	errUserPassMaxLength = fmt.Errorf("CreateUser call rejected due to username or password exceeding maximum length of %d chars", maxUserPassLen)
+	errWeakPassword      = errors.New("Failed to create user as the given password is to weak. Passwords must be 8 or more characters and contain a combination of UPPER and lowercase letters, numbers, and special characters")
 )
 
 // KeyValuePair ...
@@ -133,6 +147,10 @@ func (ks *Keystore) CreateUser(_ *http.Request, args *CreateUserArgs, reply *Cre
 		return fmt.Errorf("user already exists: %s", args.Username)
 	}
 
+	if zxcvbn.PasswordStrength(args.Password, nil).Score < requiredPassScore {
+		return errWeakPassword
+	}
+
 	usr := &User{}
 	if err := usr.Initialize(args.Password); err != nil {
 		return err

diff --git a/api/keystore/service_test.go b/api/keystore/service_test.go
@@ -14,6 +14,12 @@ import (
 	"github.com/ava-labs/gecko/utils/logging"
 )
 
+var (
+	// strongPassword defines a password used for the following tests that
+	// scores high enough to pass the password strength scoring system
+	strongPassword = "N_+=_jJ;^(<;{4,:*m6CET}'&N;83FYK.wtNpwp-Jt"
+)
+
 func TestServiceListNoUsers(t *testing.T) {
 	ks := Keystore{}
 	ks.Initialize(logging.NoLog{}, memdb.New())
@@ -35,7 +41,7 @@ func TestServiceCreateUser(t *testing.T) {
 		reply := CreateUserReply{}
 		if err := ks.CreateUser(nil, &CreateUserArgs{
 			Username: "bob",
-			Password: "launch",
+			Password: strongPassword,
 		}, &reply); err != nil {
 			t.Fatal(err)
 		}
@@ -75,7 +81,7 @@ func TestServiceCreateUserArgsCheck(t *testing.T) {
 		reply := CreateUserReply{}
 		err := ks.CreateUser(nil, &CreateUserArgs{
 			Username: genStr(maxUserPassLen + 1),
-			Password: "shortpass",
+			Password: strongPassword,
 		}, &reply)
 
 		if reply.Success || err != errUserPassMaxLength {
@@ -105,7 +111,29 @@ func TestServiceCreateUserArgsCheck(t *testing.T) {
 			t.Fatalf("A user exists when there should be none")
 		}
 	}
+}
+
+// TestServiceCreateUserWeakPassword tests creating a new user with a weak
+// password to ensure the password strength check is working
+func TestServiceCreateUserWeakPassword(t *testing.T) {
+	ks := Keystore{}
+	ks.Initialize(logging.NoLog{}, memdb.New())
+
+	{
+		reply := CreateUserReply{}
+		err := ks.CreateUser(nil, &CreateUserArgs{
+			Username: "bob",
+			Password: "weak",
+		}, &reply)
 
+		if err != errWeakPassword {
+			t.Error("Unexpected error occurred when testing weak password:", err)
+		}
+
+		if reply.Success {
+			t.Fatal("User was created when it should have been rejected due to weak password")
+		}
+	}
 }
 
 func TestServiceCreateDuplicate(t *testing.T) {
@@ -116,7 +144,7 @@ func TestServiceCreateDuplicate(t *testing.T) {
 		reply := CreateUserReply{}
 		if err := ks.CreateUser(nil, &CreateUserArgs{
 			Username: "bob",
-			Password: "launch",
+			Password: strongPassword,
 		}, &reply); err != nil {
 			t.Fatal(err)
 		}
@@ -129,7 +157,7 @@ func TestServiceCreateDuplicate(t *testing.T) {
 		reply := CreateUserReply{}
 		if err := ks.CreateUser(nil, &CreateUserArgs{
 			Username: "bob",
-			Password: "launch!",
+			Password: strongPassword,
 		}, &reply); err == nil {
 			t.Fatalf("Should have errored due to the username already existing")
 		}
@@ -142,7 +170,7 @@ func TestServiceCreateUserNoName(t *testing.T) {
 
 	reply := CreateUserReply{}
 	if err := ks.CreateUser(nil, &CreateUserArgs{
-		Password: "launch",
+		Password: strongPassword,
 	}, &reply); err == nil {
 		t.Fatalf("Shouldn't have allowed empty username")
 	}
@@ -156,7 +184,7 @@ func TestServiceUseBlockchainDB(t *testing.T) {
 		reply := CreateUserReply{}
 		if err := ks.CreateUser(nil, &CreateUserArgs{
 			Username: "bob",
-			Password: "launch",
+			Password: strongPassword,
 		}, &reply); err != nil {
 			t.Fatal(err)
 		}
@@ -166,7 +194,7 @@ func TestServiceUseBlockchainDB(t *testing.T) {
 	}
 
 	{
-		db, err := ks.GetDatabase(ids.Empty, "bob", "launch")
+		db, err := ks.GetDatabase(ids.Empty, "bob", strongPassword)
 		if err != nil {
 			t.Fatal(err)
 		}
@@ -176,7 +204,7 @@ func TestServiceUseBlockchainDB(t *testing.T) {
 	}
 
 	{
-		db, err := ks.GetDatabase(ids.Empty, "bob", "launch")
+		db, err := ks.GetDatabase(ids.Empty, "bob", strongPassword)
 		if err != nil {
 			t.Fatal(err)
 		}
@@ -196,7 +224,7 @@ func TestServiceExportImport(t *testing.T) {
 		reply := CreateUserReply{}
 		if err := ks.CreateUser(nil, &CreateUserArgs{
 			Username: "bob",
-			Password: "launch",
+			Password: strongPassword,
 		}, &reply); err != nil {
 			t.Fatal(err)
 		}
@@ -206,7 +234,7 @@ func TestServiceExportImport(t *testing.T) {
 	}
 
 	{
-		db, err := ks.GetDatabase(ids.Empty, "bob", "launch")
+		db, err := ks.GetDatabase(ids.Empty, "bob", strongPassword)
 		if err != nil {
 			t.Fatal(err)
 		}
@@ -218,7 +246,7 @@ func TestServiceExportImport(t *testing.T) {
 	exportReply := ExportUserReply{}
 	if err := ks.ExportUser(nil, &ExportUserArgs{
 		Username: "bob",
-		Password: "launch",
+		Password: strongPassword,
 	}, &exportReply); err != nil {
 		t.Fatal(err)
 	}
@@ -230,7 +258,7 @@ func TestServiceExportImport(t *testing.T) {
 		reply := ImportUserReply{}
 		if err := newKS.ImportUser(nil, &ImportUserArgs{
 			Username: "bob",
-			Password: "launch",
+			Password: strongPassword,
 			User:     exportReply.User,
 		}, &reply); err != nil {
 			t.Fatal(err)
@@ -241,7 +269,7 @@ func TestServiceExportImport(t *testing.T) {
 	}
 
 	{
-		db, err := newKS.GetDatabase(ids.Empty, "bob", "launch")
+		db, err := newKS.GetDatabase(ids.Empty, "bob", strongPassword)
 		if err != nil {
 			t.Fatal(err)
 		}