You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficcontrol.apache.org by ra...@apache.org on 2018/09/21 22:57:14 UTC

[trafficcontrol] branch master updated (70d8893 -> 8995906)

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

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


    from 70d8893  Changing back the default cdn name value
     new 1df4f53  Invalid password validation with tests
     new 8995906  Updated according to PR comments

The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 .../traffic_ops_golang/auth/authenticate_test.go   | 41 ++++++++++
 .../traffic_ops_golang/auth/password_check.go      | 95 ++++++++++++++++++++++
 .../traffic_ops_golang/traffic_ops_golang.go       |  7 ++
 3 files changed, 143 insertions(+)
 create mode 100644 traffic_ops/traffic_ops_golang/auth/password_check.go


[trafficcontrol] 01/02: Invalid password validation with tests

Posted by 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 1df4f536686389e71e25338946964a87825a9216
Author: moltzaum <ma...@moltzau.net>
AuthorDate: Thu Sep 20 15:51:25 2018 -0600

    Invalid password validation with tests
---
 .../traffic_ops_golang/auth/authenticate_test.go   | 19 +++++
 .../traffic_ops_golang/auth/password_check.go      | 94 ++++++++++++++++++++++
 .../traffic_ops_golang/traffic_ops_golang.go       |  7 ++
 3 files changed, 120 insertions(+)

diff --git a/traffic_ops/traffic_ops_golang/auth/authenticate_test.go b/traffic_ops/traffic_ops_golang/auth/authenticate_test.go
index 036986e..99d636f 100644
--- a/traffic_ops/traffic_ops_golang/auth/authenticate_test.go
+++ b/traffic_ops/traffic_ops_golang/auth/authenticate_test.go
@@ -20,6 +20,7 @@ package auth
  */
 
 import (
+	"github.com/apache/trafficcontrol/lib/go-log"
 	"testing"
 )
 
@@ -50,3 +51,21 @@ func TestScryptPasswordIsRequired(t *testing.T) {
 		t.Errorf("scrypt password should be required")
 	}
 }
+
+func TestPasswordStrength(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 expected[i] {
+				t.Errorf("%s should have been marked as an invalid password", password)
+			} else {
+				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
new file mode 100644
index 0000000..323723a
--- /dev/null
+++ b/traffic_ops/traffic_ops_golang/auth/password_check.go
@@ -0,0 +1,94 @@
+package auth
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import (
+	"bufio"
+	"fmt"
+	"os"
+	"strings"
+
+	"github.com/apache/trafficcontrol/lib/go-log"
+)
+
+// A lookup table, bool will always be true
+var invalidPasswords 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")
+	}
+
+	pwd, err := os.Getwd()
+	if err != nil {
+		return err
+	}
+
+	filePath = fmt.Sprintf("%straffic_ops/%s", pwd[:strings.Index(pwd, "traffic_ops")], filePath)
+
+	log.Infof("full path to password blacklist: %s\n", filePath)
+	in, err := os.Open(filePath)
+	if err != nil {
+		return err
+	}
+
+	scanner := bufio.NewScanner(in)
+	for scanner.Scan() {
+		invalidPasswords[scanner.Text()] = true
+	}
+
+	in.Close()
+	return scanner.Err()
+}
+
+func IsInvalidPassword(pw string) bool {
+	_, bad := invalidPasswords[pw]
+	return bad
+}
+
+func IsGoodPassword(pw string, confirmPw string, username string) error {
+
+	if pw == "" {
+		return nil
+	}
+
+	if pw != confirmPw {
+		return fmt.Errorf("Passwords do not match.")
+	}
+
+	if pw == username {
+		return fmt.Errorf("Your password cannot be the same as your username.")
+	}
+
+	if len(pw) < 8 {
+		return fmt.Errorf("Password must be greater than 7 chars.")
+	}
+
+	if IsInvalidPassword(pw) {
+		return fmt.Errorf("Password is too common.")
+	}
+
+	// At this point we're happy with the password
+	return nil
+}
diff --git a/traffic_ops/traffic_ops_golang/traffic_ops_golang.go b/traffic_ops/traffic_ops_golang/traffic_ops_golang.go
index c8348fb..c042d51 100644
--- a/traffic_ops/traffic_ops_golang/traffic_ops_golang.go
+++ b/traffic_ops/traffic_ops_golang/traffic_ops_golang.go
@@ -33,6 +33,7 @@ import (
 
 	"github.com/apache/trafficcontrol/lib/go-log"
 	"github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/about"
+	"github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/auth"
 	"github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/config"
 
 	"github.com/jmoiron/sqlx"
@@ -108,6 +109,12 @@ func main() {
 		Event Log:            %s
 		LDAP Enabled:         %v`, cfg.Port, cfg.DB.Hostname, cfg.DB.User, cfg.DB.DBName, cfg.DB.SSL, cfg.MaxDBConnections, cfg.Listen[0], cfg.Insecure, cfg.CertPath, cfg.KeyPath, time.Duration(cfg.ProxyTimeout)*time.Second, time.Duration(cfg.ProxyKeepAlive)*time.Second, time.Duration(cfg.ProxyTLSTimeout)*time.Second, time.Duration(cfg.ProxyReadHeaderTimeout)*time.Second, time.Duration(cfg.ReadTimeout)*time.Second, time.Duration(cfg.ReadHeaderTimeout)*time.Second, time.Duration(cfg.WriteTimeou [...]
 
+	err := auth.LoadPasswordBlacklist("app/conf/invalid_passwords.txt")
+	if err != nil {
+		log.Errorf("loading password blacklist: %v\n", err)
+		return
+	}
+
 	sslStr := "require"
 	if !cfg.DB.SSL {
 		sslStr = "disable"


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

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