From d755aa8d43e4be20240dd67d56126990dfedbff7 Mon Sep 17 00:00:00 2001 From: magikstm Date: Sun, 21 May 2017 23:16:00 -0400 Subject: [PATCH 1/4] Caddy usage - htpasswd file isn't reloaded (on insert, on update or on delete) --- htpasswd/auth.go | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/htpasswd/auth.go b/htpasswd/auth.go index b4b0b1b..74a3f75 100644 --- a/htpasswd/auth.go +++ b/htpasswd/auth.go @@ -12,12 +12,14 @@ import ( "io" "os" "strings" + "time" ) // Auth is the htpassword authenticater type Auth struct { filename string userHash map[string]string + modTime time.Time //Used in func reloadIfChanged to reload htpasswd file if it changed } // NewAuth creates an htpassword authenticater @@ -33,6 +35,13 @@ func (a *Auth) parse(filename string) error { if err != nil { return err } + + fileInfo, err := os.Stat(filename) + if err != nil { + return err + } + a.modTime = fileInfo.ModTime() + cr := csv.NewReader(r) cr.Comma = ':' cr.Comment = '#' @@ -57,6 +66,7 @@ func (a *Auth) parse(filename string) error { // Authenticate the user func (a *Auth) Authenticate(username, password string) (bool, error) { + reloadIfChanged(a) if hash, exist := a.userHash[username]; exist { h := []byte(hash) p := []byte(password) @@ -75,6 +85,22 @@ func (a *Auth) Authenticate(username, password string) (bool, error) { return false, nil } +// Reload htpasswd file if it changed during current run +func reloadIfChanged(a *Auth) { + fileInfo, err := os.Stat(a.filename) + if err != nil { + //On error, retain current file + return + } + + currentmodTime := fileInfo.ModTime() + + if currentmodTime != a.modTime { + a.modTime = currentmodTime + a.parse(a.filename) + } +} + func compareSha(hashedPassword, password []byte) bool { d := sha1.New() d.Write(password) From 7d3c4202a326f9a1b0f259e7dc29b896025c7ccf Mon Sep 17 00:00:00 2001 From: magikstm Date: Mon, 22 May 2017 14:07:21 -0400 Subject: [PATCH 2/4] Add RWMutex to protect race conditions on issue #20 --- htpasswd/auth.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/htpasswd/auth.go b/htpasswd/auth.go index 74a3f75..b0ef96c 100644 --- a/htpasswd/auth.go +++ b/htpasswd/auth.go @@ -13,8 +13,11 @@ import ( "os" "strings" "time" + "sync" ) +var mu sync.RWMutex + // Auth is the htpassword authenticater type Auth struct { filename string @@ -47,6 +50,7 @@ func (a *Auth) parse(filename string) error { cr.Comment = '#' cr.TrimLeadingSpace = true + mu.Lock() a.userHash = map[string]string{} for { record, err := cr.Read() @@ -61,12 +65,14 @@ func (a *Auth) parse(filename string) error { } a.userHash[record[0]] = record[1] } + mu.Unlock() return nil } // Authenticate the user func (a *Auth) Authenticate(username, password string) (bool, error) { reloadIfChanged(a) + mu.RLock() if hash, exist := a.userHash[username]; exist { h := []byte(hash) p := []byte(password) @@ -82,6 +88,7 @@ func (a *Auth) Authenticate(username, password string) (bool, error) { } return false, fmt.Errorf("unknown algorithm for user %q", username) } + mu.RUnlock() return false, nil } From 5f2b10d5866999f77461300173f1ea6627db7982 Mon Sep 17 00:00:00 2001 From: magikstm Date: Mon, 22 May 2017 14:58:09 -0400 Subject: [PATCH 3/4] Small corrections (use of defer) for last commit on #20 --- htpasswd/auth.go | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/htpasswd/auth.go b/htpasswd/auth.go index b0ef96c..1f508be 100644 --- a/htpasswd/auth.go +++ b/htpasswd/auth.go @@ -16,13 +16,12 @@ import ( "sync" ) -var mu sync.RWMutex - // Auth is the htpassword authenticater type Auth struct { filename string userHash map[string]string modTime time.Time //Used in func reloadIfChanged to reload htpasswd file if it changed + mu sync.RWMutex } // NewAuth creates an htpassword authenticater @@ -50,7 +49,8 @@ func (a *Auth) parse(filename string) error { cr.Comment = '#' cr.TrimLeadingSpace = true - mu.Lock() + a.mu.Lock() + defer a.mu.Unlock() a.userHash = map[string]string{} for { record, err := cr.Read() @@ -65,14 +65,14 @@ func (a *Auth) parse(filename string) error { } a.userHash[record[0]] = record[1] } - mu.Unlock() return nil } // Authenticate the user func (a *Auth) Authenticate(username, password string) (bool, error) { reloadIfChanged(a) - mu.RLock() + a.mu.RLock() + defer a.mu.RUnlock() if hash, exist := a.userHash[username]; exist { h := []byte(hash) p := []byte(password) @@ -88,7 +88,6 @@ func (a *Auth) Authenticate(username, password string) (bool, error) { } return false, fmt.Errorf("unknown algorithm for user %q", username) } - mu.RUnlock() return false, nil } From 060174b3176defebee6b5b07eb0e3e30434b1c10 Mon Sep 17 00:00:00 2001 From: Sebastian Mancke Date: Tue, 30 May 2017 22:57:27 +0200 Subject: [PATCH 4/4] added test for htpasswd reload --- htpasswd/auth.go | 15 ++++++++------- htpasswd/auth_test.go | 41 +++++++++++++++++++++++++++++++++++------ 2 files changed, 43 insertions(+), 13 deletions(-) diff --git a/htpasswd/auth.go b/htpasswd/auth.go index 1f508be..09332e8 100644 --- a/htpasswd/auth.go +++ b/htpasswd/auth.go @@ -12,16 +12,17 @@ import ( "io" "os" "strings" - "time" "sync" + "time" ) // Auth is the htpassword authenticater type Auth struct { filename string userHash map[string]string - modTime time.Time //Used in func reloadIfChanged to reload htpasswd file if it changed - mu sync.RWMutex + // Used in func reloadIfChanged to reload htpasswd file if it changed + modTime time.Time + mu sync.RWMutex } // NewAuth creates an htpassword authenticater @@ -37,13 +38,13 @@ func (a *Auth) parse(filename string) error { if err != nil { return err } - + fileInfo, err := os.Stat(filename) if err != nil { return err } a.modTime = fileInfo.ModTime() - + cr := csv.NewReader(r) cr.Comma = ':' cr.Comment = '#' @@ -98,9 +99,9 @@ func reloadIfChanged(a *Auth) { //On error, retain current file return } - + currentmodTime := fileInfo.ModTime() - + if currentmodTime != a.modTime { a.modTime = currentmodTime a.parse(a.filename) diff --git a/htpasswd/auth_test.go b/htpasswd/auth_test.go index 40f53b3..f4b0513 100644 --- a/htpasswd/auth_test.go +++ b/htpasswd/auth_test.go @@ -4,6 +4,7 @@ import ( . "github.com/stretchr/testify/assert" "io/ioutil" "testing" + "time" ) // password for all of them is 'secret' @@ -18,7 +19,7 @@ bob-foo:{fooo}sdcsdcsdc/BfQ= ` -func TestClient_Hashes(t *testing.T) { +func TestAuth_Hashes(t *testing.T) { auth, err := NewAuth(writeTmpfile(testfile)) NoError(t, err) @@ -36,7 +37,35 @@ func TestClient_Hashes(t *testing.T) { } } -func TestClient_UnknownUser(t *testing.T) { +func TestAuth_ReloadFile(t *testing.T) { + filename := writeTmpfile(`bob:$apr1$IDZSCL/o$N68zaFDDRivjour94OVeB.`) + auth, err := NewAuth(filename) + NoError(t, err) + + authenticated, err := auth.Authenticate("bob", "secret") + NoError(t, err) + True(t, authenticated) + + authenticated, err = auth.Authenticate("alice", "secret") + NoError(t, err) + False(t, authenticated) + + // The refresh is time based, so we have to wait a second, here + time.Sleep(time.Second) + + err = ioutil.WriteFile(filename, []byte(`alice:$apr1$IDZSCL/o$N68zaFDDRivjour94OVeB.`), 06644) + NoError(t, err) + + authenticated, err = auth.Authenticate("bob", "secret") + NoError(t, err) + False(t, authenticated) + + authenticated, err = auth.Authenticate("alice", "secret") + NoError(t, err) + True(t, authenticated) +} + +func TestAuth_UnknownUser(t *testing.T) { auth, err := NewAuth(writeTmpfile(testfile)) NoError(t, err) @@ -45,12 +74,12 @@ func TestClient_UnknownUser(t *testing.T) { False(t, authenticated) } -func TestClient_ErrorOnMissingFile(t *testing.T) { +func TestAuth_ErrorOnMissingFile(t *testing.T) { _, err := NewAuth("/tmp/foo/bar/nothing") Error(t, err) } -func TestClient_ErrorOnInvalidFileContents(t *testing.T) { +func TestAuth_ErrorOnInvalidFileContents(t *testing.T) { _, err := NewAuth(writeTmpfile("foo bar bazz")) Error(t, err) @@ -58,7 +87,7 @@ func TestClient_ErrorOnInvalidFileContents(t *testing.T) { Error(t, err) } -func TestClient_BadMD5Format(t *testing.T) { +func TestAuth_BadMD5Format(t *testing.T) { // missing $ separator in md5 hash a, err := NewAuth(writeTmpfile("foo:$apr1$IDZSCL/oN68zaFDDRivjour94OVeB.")) NoError(t, err) @@ -68,7 +97,7 @@ func TestClient_BadMD5Format(t *testing.T) { False(t, authenticated) } -func TestClient_Hashes_UnknownAlgoError(t *testing.T) { +func TestAuth_Hashes_UnknownAlgoError(t *testing.T) { auth, err := NewAuth(writeTmpfile(testfile)) NoError(t, err)