[trafficcontrol] 02/02: Updated according to PR comments

[ra...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

rawlin pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/trafficcontrol.git

commit 8995906cf717611999058a6a9d10abe9c070cf3c
Author: moltzaum <ma...@moltzau.net>
AuthorDate: Fri Sep 21 12:40:52 2018 -0600

    Updated according to PR comments
---
 .../traffic_ops_golang/auth/authenticate_test.go   | 32 ++++++++++++---
 .../traffic_ops_golang/auth/password_check.go      | 47 +++++++++++-----------
 2 files changed, 51 insertions(+), 28 deletions(-)

diff --git a/traffic_ops/traffic_ops_golang/auth/authenticate_test.go b/traffic_ops/traffic_ops_golang/auth/authenticate_test.go
index 99d636f..3333b4d 100644
--- a/traffic_ops/traffic_ops_golang/auth/authenticate_test.go
+++ b/traffic_ops/traffic_ops_golang/auth/authenticate_test.go
@@ -20,7 +20,6 @@ package auth
  */
 
 import (
-	"github.com/apache/trafficcontrol/lib/go-log"
 	"testing"
 )
 
@@ -52,18 +51,41 @@ func TestScryptPasswordIsRequired(t *testing.T) {
 	}
 }
 
-func TestPasswordStrength(t *testing.T) {
+// The purpose of this test is to show that all password requirements are being met
+func TestUsernamePassword(t *testing.T) {
+
+	passwords := []string{"username", "password", "pa$$word", "", "red"}
+	expected := []bool{false, false, true, false, false}
+	LoadPasswordBlacklist("app/conf/invalid_passwords.txt")
+
+	for i, password := range passwords {
+		if ok, err := IsGoodLoginPair("username", password); ok != expected[i] {
+			if expected[i] {
+				t.Errorf("\"%s\" should have been marked as an invalid password", password)
+			} else {
+				t.Errorf("\"%s\" should be an ok password, but got the error: %v", password, err)
+			}
+		}
+	}
+
+	if ok, _ := IsGoodLoginPair("", "GoOdPa$$woRd"); ok {
+		t.Errorf("An empty username should not pass")
+	}
+}
+
+// The purpose of this test is to show that the file is being read, and we can tell if a password is in the file
+func TestCommonPassword(t *testing.T) {
 
 	passwords := []string{"password", "pa$$word"}
 	expected := []bool{true, false}
 	LoadPasswordBlacklist("app/conf/invalid_passwords.txt")
 
 	for i, password := range passwords {
-		if IsInvalidPassword(password) != expected[i] {
+		if IsCommonPassword(password) != expected[i] {
 			if expected[i] {
-				t.Errorf("%s should have been marked as an invalid password", password)
+				t.Errorf("\"%s\" should have been marked as an invalid password", password)
 			} else {
-				t.Errorf("%s should be an ok password", password)
+				t.Errorf("\"%s\" should be an ok password", password)
 			}
 		}
 	}
diff --git a/traffic_ops/traffic_ops_golang/auth/password_check.go b/traffic_ops/traffic_ops_golang/auth/password_check.go
index 323723a..71efb5e 100644
--- a/traffic_ops/traffic_ops_golang/auth/password_check.go
+++ b/traffic_ops/traffic_ops_golang/auth/password_check.go
@@ -21,6 +21,7 @@ package auth
 
 import (
 	"bufio"
+	"errors"
 	"fmt"
 	"os"
 	"strings"
@@ -29,17 +30,17 @@ import (
 )
 
 // A lookup table, bool will always be true
-var invalidPasswords map[string]bool
+var commonPasswords map[string]bool
 
 // Expects a relative path from the traffic_ops directory
 func LoadPasswordBlacklist(filePath string) error {
 
-	if invalidPasswords == nil {
-		invalidPasswords = make(map[string]bool)
-	} else {
-		return fmt.Errorf("Password blacklist is already loaded")
+	if commonPasswords != nil {
+		return errors.New("Password blacklist is already loaded")
 	}
 
+	commonPasswords = make(map[string]bool)
+
 	pwd, err := os.Getwd()
 	if err != nil {
 		return err
@@ -55,40 +56,40 @@ func LoadPasswordBlacklist(filePath string) error {
 
 	scanner := bufio.NewScanner(in)
 	for scanner.Scan() {
-		invalidPasswords[scanner.Text()] = true
+		commonPasswords[scanner.Text()] = true
 	}
 
 	in.Close()
 	return scanner.Err()
 }
 
-func IsInvalidPassword(pw string) bool {
-	_, bad := invalidPasswords[pw]
-	return bad
+func IsCommonPassword(pw string) bool {
+	_, yes := commonPasswords[pw]
+	return yes
 }
 
-func IsGoodPassword(pw string, confirmPw string, username string) error {
+func IsGoodLoginPair(username string, password string) (bool, error) {
 
-	if pw == "" {
-		return nil
+	if username == "" {
+		return false, errors.New("Your username cannot be blank.")
 	}
 
-	if pw != confirmPw {
-		return fmt.Errorf("Passwords do not match.")
+	if username == password {
+		return false, errors.New("Your password cannot be your username.")
 	}
 
-	if pw == username {
-		return fmt.Errorf("Your password cannot be the same as your username.")
-	}
+	return IsGoodPassword(password)
+}
+
+func IsGoodPassword(password string) (bool, error) {
 
-	if len(pw) < 8 {
-		return fmt.Errorf("Password must be greater than 7 chars.")
+	if len(password) < 8 {
+		return false, errors.New("Password must be greater than 7 characters.")
 	}
 
-	if IsInvalidPassword(pw) {
-		return fmt.Errorf("Password is too common.")
+	if IsCommonPassword(password) {
+		return false, errors.New("Password is too common.")
 	}
 
-	// At this point we're happy with the password
-	return nil
+	return true, nil
 }