Merge pull request #687 from oauth2-proxy/htpasswd-validator

Refactor HTPasswd Validator

CHANGELOG.md

@@ -11,6 +11,7 @@
 
 ## Changes since v6.0.0
 
+- [#687](https://github.com/oauth2-proxy/oauth2-proxy/pull/687) Refactor HTPasswd Validator (@JoelSpeed)
 - [#624](https://github.com/oauth2-proxy/oauth2-proxy/pull/624) Allow stripping authentication headers from whitelisted requests with `--skip-auth-strip-headers` (@NickMeves)
 - [#673](https://github.com/oauth2-proxy/oauth2-proxy/pull/673) Add --session-cookie-minimal option to create session cookies with no tokens (@NickMeves)
 - [#632](https://github.com/oauth2-proxy/oauth2-proxy/pull/632) Reduce session size by encoding with MessagePack and using LZ4 compression (@NickMeves)

htpasswd.go

@@ -1,72 +0,0 @@
-package main
-
-import (
-	"crypto/sha1"
-	"encoding/base64"
-	"encoding/csv"
-	"io"
-	"os"
-
-	"github.com/oauth2-proxy/oauth2-proxy/pkg/logger"
-	"golang.org/x/crypto/bcrypt"
-)
-
-// Lookup passwords in a htpasswd file
-// Passwords must be generated with -B for bcrypt or -s for SHA1.
-
-// HtpasswdFile represents the structure of an htpasswd file
-type HtpasswdFile struct {
-	Users map[string]string
-}
-
-// NewHtpasswdFromFile constructs an HtpasswdFile from the file at the path given
-func NewHtpasswdFromFile(path string) (*HtpasswdFile, error) {
-	r, err := os.Open(path)
-	if err != nil {
-		return nil, err
-	}
-	defer r.Close()
-	return NewHtpasswd(r)
-}
-
-// NewHtpasswd  consctructs an HtpasswdFile from an io.Reader (opened file)
-func NewHtpasswd(file io.Reader) (*HtpasswdFile, error) {
-	csvReader := csv.NewReader(file)
-	csvReader.Comma = ':'
-	csvReader.Comment = '#'
-	csvReader.TrimLeadingSpace = true
-
-	records, err := csvReader.ReadAll()
-	if err != nil {
-		return nil, err
-	}
-	h := &HtpasswdFile{Users: make(map[string]string)}
-	for _, record := range records {
-		h.Users[record[0]] = record[1]
-	}
-	return h, nil
-}
-
-// Validate checks a users password against the HtpasswdFile entries
-func (h *HtpasswdFile) Validate(user string, password string) bool {
-	realPassword, exists := h.Users[user]
-	if !exists {
-		return false
-	}
-
-	shaPrefix := realPassword[:5]
-	if shaPrefix == "{SHA}" {
-		shaValue := realPassword[5:]
-		d := sha1.New()
-		d.Write([]byte(password))
-		return shaValue == base64.StdEncoding.EncodeToString(d.Sum(nil))
-	}
-
-	bcryptPrefix := realPassword[:4]
-	if bcryptPrefix == "$2a$" || bcryptPrefix == "$2b$" || bcryptPrefix == "$2x$" || bcryptPrefix == "$2y$" {
-		return bcrypt.CompareHashAndPassword([]byte(realPassword), []byte(password)) == nil
-	}
-
-	logger.Printf("Invalid htpasswd entry for %s. Must be a SHA or bcrypt entry.", user)
-	return false
-}

htpasswd_test.go

@@ -1,38 +0,0 @@
-package main
-
-import (
-	"bytes"
-	"fmt"
-	"testing"
-
-	"github.com/stretchr/testify/assert"
-	"golang.org/x/crypto/bcrypt"
-)
-
-func TestSHA(t *testing.T) {
-	file := bytes.NewBuffer([]byte("testuser:{SHA}PaVBVZkYqAjCQCu6UBL2xgsnZhw=\n"))
-	h, err := NewHtpasswd(file)
-	assert.Equal(t, err, nil)
-
-	valid := h.Validate("testuser", "asdf")
-	assert.Equal(t, valid, true)
-}
-
-func TestBcrypt(t *testing.T) {
-	hash1, err := bcrypt.GenerateFromPassword([]byte("password"), 1)
-	assert.Equal(t, err, nil)
-	hash2, err := bcrypt.GenerateFromPassword([]byte("top-secret"), 2)
-	assert.Equal(t, err, nil)
-
-	contents := fmt.Sprintf("testuser1:%s\ntestuser2:%s\n", hash1, hash2)
-	file := bytes.NewBuffer([]byte(contents))
-
-	h, err := NewHtpasswd(file)
-	assert.Equal(t, err, nil)
-
-	valid := h.Validate("testuser1", "password")
-	assert.Equal(t, valid, true)
-
-	valid = h.Validate("testuser2", "top-secret")
-	assert.Equal(t, valid, true)
-}

main.go

@@ -66,15 +66,6 @@ func main() {
 		}
 	}
 
-	if opts.HtpasswdFile != "" {
-		logger.Printf("using htpasswd file %s", opts.HtpasswdFile)
-		oauthproxy.HtpasswdFile, err = NewHtpasswdFromFile(opts.HtpasswdFile)
-		oauthproxy.DisplayHtpasswdForm = opts.DisplayHtpasswdForm
-		if err != nil {
-			logger.Fatalf("FATAL: unable to open %s %s", opts.HtpasswdFile, err)
-		}
-	}
-
 	rand.Seed(time.Now().UnixNano())
 
 	chain := alice.New()

oauthproxy.go

@@ -22,6 +22,7 @@ import (
 	ipapi "github.com/oauth2-proxy/oauth2-proxy/pkg/apis/ip"
 	"github.com/oauth2-proxy/oauth2-proxy/pkg/apis/options"
 	sessionsapi "github.com/oauth2-proxy/oauth2-proxy/pkg/apis/sessions"
+	"github.com/oauth2-proxy/oauth2-proxy/pkg/authentication/basic"
 	"github.com/oauth2-proxy/oauth2-proxy/pkg/cookies"
 	"github.com/oauth2-proxy/oauth2-proxy/pkg/encryption"
 	"github.com/oauth2-proxy/oauth2-proxy/pkg/ip"
@@ -96,8 +97,8 @@ type OAuthProxy struct {
 	sessionStore            sessionsapi.SessionStore
 	ProxyPrefix             string
 	SignInMessage           string
-	HtpasswdFile            *HtpasswdFile
-	DisplayHtpasswdForm     bool
+	basicAuthValidator      basic.Validator
+	displayHtpasswdForm     bool
 	serveMux                http.Handler
 	SetXAuthRequest         bool
 	PassBasicAuth           bool
@@ -314,6 +315,16 @@ func NewOAuthProxy(opts *options.Options, validator func(string) bool) (*OAuthPr
 		}
 	}
 
+	var basicAuthValidator basic.Validator
+	if opts.HtpasswdFile != "" {
+		logger.Printf("using htpasswd file: %s", opts.HtpasswdFile)
+		var err error
+		basicAuthValidator, err = basic.NewHTPasswdValidator(opts.HtpasswdFile)
+		if err != nil {
+			return nil, fmt.Errorf("could not load htpasswdfile: %v", err)
+		}
+	}
+
 	return &OAuthProxy{
 		CookieName:     opts.Cookie.Name,
 		CSRFCookieName: fmt.Sprintf("%v_%v", opts.Cookie.Name, "csrf"),
@@ -364,6 +375,9 @@ func NewOAuthProxy(opts *options.Options, validator func(string) bool) (*OAuthPr
 		trustedIPs:              trustedIPs,
 		Banner:                  opts.Banner,
 		Footer:                  opts.Footer,
+
+		basicAuthValidator:  basicAuthValidator,
+		displayHtpasswdForm: basicAuthValidator != nil,
 	}, nil
 }
 
@@ -386,10 +400,6 @@ func (p *OAuthProxy) GetRedirectURI(host string) string {
 	return u.String()
 }
 
-func (p *OAuthProxy) displayCustomLoginForm() bool {
-	return p.HtpasswdFile != nil && p.DisplayHtpasswdForm
-}
-
 func (p *OAuthProxy) redeemCode(ctx context.Context, host, code string) (s *sessionsapi.SessionState, err error) {
 	if code == "" {
 		return nil, errors.New("missing code")
@@ -526,7 +536,7 @@ func (p *OAuthProxy) SignInPage(rw http.ResponseWriter, req *http.Request, code
 	}{
 		ProviderName:  p.provider.Data().ProviderName,
 		SignInMessage: template.HTML(p.SignInMessage),
-		CustomLogin:   p.displayCustomLoginForm(),
+		CustomLogin:   p.displayHtpasswdForm,
 		Redirect:      redirectURL,
 		Version:       VERSION,
 		ProxyPrefix:   p.ProxyPrefix,
@@ -540,7 +550,7 @@ func (p *OAuthProxy) SignInPage(rw http.ResponseWriter, req *http.Request, code
 
 // ManualSignIn handles basic auth logins to the proxy
 func (p *OAuthProxy) ManualSignIn(rw http.ResponseWriter, req *http.Request) (string, bool) {
-	if req.Method != "POST" || p.HtpasswdFile == nil {
+	if req.Method != "POST" || p.basicAuthValidator == nil {
 		return "", false
 	}
 	user := req.FormValue("username")
@@ -549,7 +559,7 @@ func (p *OAuthProxy) ManualSignIn(rw http.ResponseWriter, req *http.Request) (st
 		return "", false
 	}
 	// check auth
-	if p.HtpasswdFile.Validate(user, passwd) {
+	if p.basicAuthValidator.Validate(user, passwd) {
 		logger.PrintAuthf(user, req, logger.AuthSuccess, "Authenticated via HtpasswdFile")
 		return user, true
 	}
@@ -1159,7 +1169,7 @@ func (p *OAuthProxy) stripAuthHeaders(req *http.Request) {
 // CheckBasicAuth checks the requests Authorization header for basic auth
 // credentials and authenticates these against the proxies HtpasswdFile
 func (p *OAuthProxy) CheckBasicAuth(req *http.Request) (*sessionsapi.SessionState, error) {
-	if p.HtpasswdFile == nil {
+	if p.basicAuthValidator == nil {
 		return nil, nil
 	}
 	auth := req.Header.Get("Authorization")
@@ -1178,7 +1188,7 @@ func (p *OAuthProxy) CheckBasicAuth(req *http.Request) (*sessionsapi.SessionStat
 	if len(pair) != 2 {
 		return nil, fmt.Errorf("invalid format %s", b)
 	}
-	if p.HtpasswdFile.Validate(pair[0], pair[1]) {
+	if p.basicAuthValidator.Validate(pair[0], pair[1]) {
 		logger.PrintAuthf(pair[0], req, logger.AuthSuccess, "Authenticated via basic auth and HTpasswd File")
 		return &sessionsapi.SessionState{User: pair[0]}, nil
 	}

pkg/authentication/basic/basic_suite_test.go

@@ -0,0 +1,16 @@
+package basic
+
+import (
+	"testing"
+
+	"github.com/oauth2-proxy/oauth2-proxy/pkg/logger"
+	. "github.com/onsi/ginkgo"
+	. "github.com/onsi/gomega"
+)
+
+func TestBasicSuite(t *testing.T) {
+	logger.SetOutput(GinkgoWriter)
+
+	RegisterFailHandler(Fail)
+	RunSpecs(t, "Basic")
+}

pkg/authentication/basic/htpasswd.go

@@ -0,0 +1,96 @@
+package basic
+
+import (
+	"crypto/sha1"
+	"encoding/base64"
+	"encoding/csv"
+	"fmt"
+	"io"
+	"os"
+
+	"github.com/oauth2-proxy/oauth2-proxy/pkg/logger"
+	"golang.org/x/crypto/bcrypt"
+)
+
+// htpasswdMap represents the structure of an htpasswd file.
+// Passwords must be generated with -B for bcrypt or -s for SHA1.
+type htpasswdMap struct {
+	users map[string]interface{}
+}
+
+// bcryptPass is used to identify bcrypt passwords in the
+// htpasswdMap users.
+type bcryptPass string
+
+// sha1Pass os used to identify sha1 passwords in the
+// htpasswdMap users.
+type sha1Pass string
+
+// NewHTPasswdValidator constructs an httpasswd based validator from the file
+// at the path given.
+func NewHTPasswdValidator(path string) (Validator, error) {
+	r, err := os.Open(path)
+	if err != nil {
+		return nil, fmt.Errorf("could not open htpasswd file: %v", err)
+	}
+	defer r.Close()
+	return newHtpasswd(r)
+}
+
+// newHtpasswd consctructs an htpasswd from an io.Reader (an opened file).
+func newHtpasswd(file io.Reader) (*htpasswdMap, error) {
+	csvReader := csv.NewReader(file)
+	csvReader.Comma = ':'
+	csvReader.Comment = '#'
+	csvReader.TrimLeadingSpace = true
+
+	records, err := csvReader.ReadAll()
+	if err != nil {
+		return nil, fmt.Errorf("could not read htpasswd file: %v", err)
+	}
+
+	return createHtpasswdMap(records)
+}
+
+// createHtasswdMap constructs an htpasswdMap from the given records
+func createHtpasswdMap(records [][]string) (*htpasswdMap, error) {
+	h := &htpasswdMap{users: make(map[string]interface{})}
+	for _, record := range records {
+		user, realPassword := record[0], record[1]
+		shaPrefix := realPassword[:5]
+		if shaPrefix == "{SHA}" {
+			h.users[user] = sha1Pass(realPassword[5:])
+			continue
+		}
+
+		bcryptPrefix := realPassword[:4]
+		if bcryptPrefix == "$2a$" || bcryptPrefix == "$2b$" || bcryptPrefix == "$2x$" || bcryptPrefix == "$2y$" {
+			h.users[user] = bcryptPass(realPassword)
+			continue
+		}
+
+		// Password is neither sha1 or bcrypt
+		// TODO(JoelSpeed): In the next breaking release, make this return an error.
+		logger.Printf("Invalid htpasswd entry for %s. Must be a SHA or bcrypt entry.", user)
+	}
+	return h, nil
+}
+
+// Validate checks a users password against the htpasswd entries
+func (h *htpasswdMap) Validate(user string, password string) bool {
+	realPassword, exists := h.users[user]
+	if !exists {
+		return false
+	}
+
+	switch real := realPassword.(type) {
+	case sha1Pass:
+		d := sha1.New()
+		d.Write([]byte(password))
+		return string(real) == base64.StdEncoding.EncodeToString(d.Sum(nil))
+	case bcryptPass:
+		return bcrypt.CompareHashAndPassword([]byte(real), []byte(password)) == nil
+	default:
+		return false
+	}
+}

pkg/authentication/basic/htpasswd_test.go

@@ -0,0 +1,96 @@
+package basic
+
+import (
+	. "github.com/onsi/ginkgo"
+	. "github.com/onsi/gomega"
+)
+
+const (
+	adminUser     = "admin"
+	adminPassword = "Adm1n1str$t0r"
+	user1         = "user1"
+	user1Password = "UsErOn3P455"
+	user2         = "user2"
+	user2Password = "us3r2P455W0Rd!"
+)
+
+var _ = Describe("HTPasswd Suite", func() {
+	Context("with an HTPassword Validator", func() {
+		assertHtpasswdMapFromFile := func(filePath string) {
+			var htpasswd *htpasswdMap
+			var err error
+
+			BeforeEach(func() {
+				var validator Validator
+				validator, err = NewHTPasswdValidator(filePath)
+
+				var ok bool
+				htpasswd, ok = validator.(*htpasswdMap)
+				Expect(ok).To(BeTrue())
+			})
+
+			It("does not return an error", func() {
+				Expect(err).ToNot(HaveOccurred())
+			})
+
+			It("has the correct number of users", func() {
+				Expect(htpasswd.users).To(HaveLen(3))
+			})
+
+			It("accepts the correct passwords", func() {
+				Expect(htpasswd.Validate(adminUser, adminPassword)).To(BeTrue())
+				Expect(htpasswd.Validate(user1, user1Password)).To(BeTrue())
+				Expect(htpasswd.Validate(user2, user2Password)).To(BeTrue())
+			})
+
+			It("rejects incorrect passwords", func() {
+				Expect(htpasswd.Validate(adminUser, "asvdfda")).To(BeFalse())
+				Expect(htpasswd.Validate(user1, "BHEdgbtr")).To(BeFalse())
+				Expect(htpasswd.Validate(user2, "12345")).To(BeFalse())
+			})
+
+			It("rejects a non existent user", func() {
+				// Users are case sensitive
+				Expect(htpasswd.Validate("ADMIN", adminPassword)).To(BeFalse())
+			})
+		}
+
+		Context("load from file", func() {
+			Context("with sha1 entries", func() {
+				const filePath = "./test/htpasswd-sha1.txt"
+
+				assertHtpasswdMapFromFile(filePath)
+			})
+
+			Context("with bcrypt entries", func() {
+				const filePath = "./test/htpasswd-bcrypt.txt"
+
+				assertHtpasswdMapFromFile(filePath)
+			})
+
+			Context("with mixed entries", func() {
+				const filePath = "./test/htpasswd-mixed.txt"
+
+				assertHtpasswdMapFromFile(filePath)
+			})
+
+			Context("with a non existent file", func() {
+				const filePath = "./test/htpasswd-doesnt-exist.txt"
+				var validator Validator
+				var err error
+
+				BeforeEach(func() {
+					validator, err = NewHTPasswdValidator(filePath)
+				})
+
+				It("returns an error", func() {
+					Expect(err).To(MatchError("could not open htpasswd file: open ./test/htpasswd-doesnt-exist.txt: no such file or directory"))
+				})
+
+				It("returns a nil validator", func() {
+					Expect(validator).To(BeNil())
+				})
+			})
+		})
+	})
+})

pkg/authentication/basic/test/htpasswd-bcrypt.txt

@@ -0,0 +1,8 @@
+# admin:Adm1n1str$t0r
+admin:$2y$05$SXWrNM7ldtbRzBvUC3VXyOvUeiUcP45XPwM93P5eeGOEPIiAZmJjC
+
+# user1:UsErOn3P455
+user1:$2y$05$/sZYJOk8.3Etg4V6fV7puuXfCJLmV5Q7u3xvKpjBSJUka.t2YtmmG
+
+# user2: us3r2P455W0Rd!
+user2:$2y$05$l22MubgKTZFTjTs8TNg5k.YKvcnM2.bA/.iwl0idef5CbekdvBxva

pkg/authentication/basic/test/htpasswd-mixed.txt

@@ -0,0 +1,8 @@
+# admin:Adm1n1str$t0r
+admin:$2y$05$SXWrNM7ldtbRzBvUC3VXyOvUeiUcP45XPwM93P5eeGOEPIiAZmJjC
+
+# user1:UsErOn3P455
+user1:{SHA}Dvs/L78raajL4jEAHPkwflQXJzI=
+
+# user2: us3r2P455W0Rd!
+user2:{SHA}MoN9/JCJEcYUb6GCQ+2buDvn9pI=

pkg/authentication/basic/test/htpasswd-sha1.txt

@@ -0,0 +1,8 @@
+# admin:Adm1n1str$t0r
+admin:{SHA}gXQeRH0bcaCfhAk2gOLm1uaePMA=
+
+# user1:UsErOn3P455
+user1:{SHA}Dvs/L78raajL4jEAHPkwflQXJzI=
+
+# user2: us3r2P455W0Rd!
+user2:{SHA}MoN9/JCJEcYUb6GCQ+2buDvn9pI=

pkg/authentication/basic/validator.go

@@ -0,0 +1,7 @@
+package basic
+
+// Validator is a minimal interface for something that can validate a
+// username and password combination.
+type Validator interface {
+	Validate(user, password string) bool
+}