git.iko.soy/typeable/oauth2-proxy
on master help sign in
Reload authenticated-emails-file upon update

This change extracts the UserMap class from NewValidator() so that its LoadAuthenticatedEmailsFile() method can be called concurrently. This method is called by a goroutine containing a fsnotify.Watcher watching the authenticated emails file. Watching isn't forever aborted when the authenticated emails file disappears. The goroutine will call os.Stat() up to twenty times a second if the file is persistently missing, but that's the pathological case, not the common one. The common case is that some editors (including Vim) will perform a rename-and-replace when updating a file, triggering fsnotify.Rename events, and the file will temporarily disappear. This watcher goroutine handles that case. Also, on some platforms (notably Arch Linux), a remove will be preceded by a fsnotify.Chmod, causing a race between the upcoming fsnotify.Remove and the call to UserMap.LoadAuthenticatedEmailsFile(). Hence, we treat fsnotify.Chmod the same as fsnotify.Remove and fsnotify.Rename. There's no significant penalty to re-adding a file to the watcher. Also contains the following small changes from the summary of commits below: - Minor optimization of email domain search - Fixed api_test.go on Windows - Add deferred File.Close() calls where needed - Log error and return if emails file doesn't parse These are the original commits from #89 squashed into this one: 0c6f2b6 Refactor validator_test to prepare for more tests e0c792b Add more test cases to validator_test a9a9d93 Minor optimization of email domain search b763ea5 Extract LoadAuthenticatedEmailsFile() 8cdaf7f Introduce synchronized UserMap type 1b84eef Add UserMap methods, locking af15dcf Reload authenticated-emails-file upon update 6d95548 Make UserMap operations lock-free Per: - http://stackoverflow.com/questions/21447463/is-assigning-a-pointer-atomic-in-golang - https://groups.google.com/forum/#!msg/golang-nuts/ueSvaEKgyLY/ZW_74IC4PekJ 75755d5 Fix tests on Windows d0eab2e Ignore email file watcher Chmod events 0b9798b Fix watcher on Ubuntu 12.04 3a8251a WaitForReplacement() to retry emails file watch a57fd29 Add deferred File.Close() calls where needed Because correctness: Don't leak file handles anywhere, and prepare for future panics and early returns. 52ed3fd Log error and return if emails file doesn't parse 40100d4 Add gopkg.in/fsnotify.v1 dependency to Godeps file 17dfbbc Avoid a race when Remove is preceded by Chmod

commitca91b5eddd7c00c76b3f9cff425ec4e15c8718f7 authorMike Bland <michael.bland@gsa.gov> dateSat 9 May 2015, 19:31 (11 years ago) parent26170c5
9 files changed+398−29
Godeps+1 api/api_test.go+2−2 htpasswd.go+1 validator.go+58−17 validator_test.go+104−10 validator_watcher_copy_test.go+50 validator_watcher_test.go+105 watcher.go+64 watcher_unsupported.go+13
Godeps+1
@@ -2,3 +2,4 @@ github.com/BurntSushi/toml 3883ac1ce943878302255f538fce319d23226223
22github.com/bitly/go-simplejson 3378bdcb5cebedcbf8b5750edee28010f128fe24
33github.com/mreiferson/go-options ee94b57f2fbf116075426f853e5abbcdfeca8b3d
44github.com/bmizerany/assert e17e99893cb6509f428e1728281c2ad60a6b31e3
5+gopkg.in/fsnotify.v1 v1.2.0
api/api_test.go+2−2
@@ -40,8 +40,8 @@ func TestRequestFailure(t *testing.T) {
4040 resp, err := Request(req)
4141 assert.Equal(t, (*simplejson.Json)(nil), resp)
4242 assert.NotEqual(t, nil, err)
43− if !strings.HasSuffix(err.Error(), "connection refused") {
44− t.Error("expected error when a connection fails")
43+ if !strings.Contains(err.Error(), "refused") {
44+ t.Error("expected error when a connection fails: ", err)
4545 }
4646}
4747
htpasswd.go+1
@@ -21,6 +21,7 @@ func NewHtpasswdFromFile(path string) (*HtpasswdFile, error) {
2121 if err != nil {
2222 return nil, err
2323 }
24+ defer r.Close()
2425 return NewHtpasswd(r)
2526}
2627
validator.go+58−17
@@ -6,43 +6,84 @@ import (
66 "log"
77 "os"
88 "strings"
9+ "sync/atomic"
10+ "unsafe"
911)
1012
11−func NewValidator(domains []string, usersFile string) func(string) bool {
12− validUsers := make(map[string]bool)
13+type UserMap struct {
14+ usersFile string
15+ m unsafe.Pointer
16+}
1317
18+func NewUserMap(usersFile string, onUpdate func()) *UserMap {
19+ um := &UserMap{usersFile: usersFile}
20+ m := make(map[string]bool)
21+ atomic.StorePointer(&um.m, unsafe.Pointer(&m))
1422 if usersFile != "" {
1523 log.Printf("using authenticated emails file %s", usersFile)
16− r, err := os.Open(usersFile)
17− if err != nil {
18− log.Fatalf("failed opening authenticated-emails-file=%q, %s", usersFile, err)
19− }
20− csv_reader := csv.NewReader(r)
21− csv_reader.Comma = ','
22− csv_reader.Comment = '#'
23− csv_reader.TrimLeadingSpace = true
24− records, err := csv_reader.ReadAll()
25− for _, r := range records {
26− validUsers[strings.ToLower(r[0])] = true
24+ started := WatchForUpdates(usersFile, func() {
25+ um.LoadAuthenticatedEmailsFile()
26+ onUpdate()
27+ })
28+ if started {
29+ log.Printf("watching %s for updates", usersFile)
2730 }
31+ um.LoadAuthenticatedEmailsFile()
2832 }
33+ return um
34+}
35+
36+func (um *UserMap) IsValid(email string) (result bool) {
37+ m := *(*map[string]bool)(atomic.LoadPointer(&um.m))
38+ _, result = m[email]
39+ return
40+}
41+
42+func (um *UserMap) LoadAuthenticatedEmailsFile() {
43+ r, err := os.Open(um.usersFile)
44+ if err != nil {
45+ log.Fatalf("failed opening authenticated-emails-file=%q, %s", um.usersFile, err)
46+ }
47+ defer r.Close()
48+ csv_reader := csv.NewReader(r)
49+ csv_reader.Comma = ','
50+ csv_reader.Comment = '#'
51+ csv_reader.TrimLeadingSpace = true
52+ records, err := csv_reader.ReadAll()
53+ if err != nil {
54+ log.Printf("error reading authenticated-emails-file=%q, %s", um.usersFile, err)
55+ return
56+ }
57+ updated := make(map[string]bool)
58+ for _, r := range records {
59+ updated[strings.ToLower(r[0])] = true
60+ }
61+ atomic.StorePointer(&um.m, unsafe.Pointer(&updated))
62+}
63+
64+func newValidatorImpl(domains []string, usersFile string,
65+ onUpdate func()) func(string) bool {
66+ validUsers := NewUserMap(usersFile, onUpdate)
2967
3068 for i, domain := range domains {
31− domains[i] = strings.ToLower(domain)
69+ domains[i] = fmt.Sprintf("@%s", strings.ToLower(domain))
3270 }
3371
3472 validator := func(email string) bool {
3573 email = strings.ToLower(email)
3674 valid := false
3775 for _, domain := range domains {
38− emailSuffix := fmt.Sprintf("@%s", domain)
39− valid = valid || strings.HasSuffix(email, emailSuffix)
76+ valid = valid || strings.HasSuffix(email, domain)
4077 }
4178 if !valid {
42− _, valid = validUsers[email]
79+ valid = validUsers.IsValid(email)
4380 }
4481 log.Printf("validating: is %s valid? %v", email, valid)
4582 return valid
4683 }
4784 return validator
4885}
86+
87+func NewValidator(domains []string, usersFile string) func(string) bool {
88+ return newValidatorImpl(domains, usersFile, func() {})
89+}
validator_test.go+104−10
@@ -7,23 +7,117 @@ import (
77 "testing"
88)
99
10−func TestValidatorComparisonsAreCaseInsensitive(t *testing.T) {
11− auth_email_file, err := ioutil.TempFile("", "test_auth_emails_")
10+type ValidatorTest struct {
11+ auth_email_file *os.File
12+}
13+
14+func NewValidatorTest(t *testing.T) *ValidatorTest {
15+ vt := &ValidatorTest{}
16+ var err error
17+ vt.auth_email_file, err = ioutil.TempFile("", "test_auth_emails_")
1218 if err != nil {
1319 t.Fatal("failed to create temp file: " + err.Error())
1420 }
15− defer os.Remove(auth_email_file.Name())
21+ return vt
22+}
1623
17− auth_email_file.WriteString(
18− strings.Join([]string{"Foo.Bar@Example.Com"}, "\n"))
19− err = auth_email_file.Close()
20− if err != nil {
21− t.Fatal("failed to close temp file " + auth_email_file.Name() +
22− ": " + err.Error())
24+func (vt *ValidatorTest) TearDown() {
25+ os.Remove(vt.auth_email_file.Name())
26+}
27+
28+// This will close vt.auth_email_file.
29+func (vt *ValidatorTest) WriteEmails(t *testing.T, emails []string) {
30+ defer vt.auth_email_file.Close()
31+ vt.auth_email_file.WriteString(strings.Join(emails, "\n"))
32+ if err := vt.auth_email_file.Close(); err != nil {
33+ t.Fatal("failed to close temp file " +
34+ vt.auth_email_file.Name() + ": " + err.Error())
35+ }
36+}
37+
38+func TestValidatorEmpty(t *testing.T) {
39+ vt := NewValidatorTest(t)
40+ defer vt.TearDown()
41+
42+ vt.WriteEmails(t, []string(nil))
43+ domains := []string(nil)
44+ validator := NewValidator(domains, vt.auth_email_file.Name())
45+
46+ if validator("foo.bar@example.com") {
47+ t.Error("nothing should validate when the email and " +
48+ "domain lists are empty")
49+ }
50+}
51+
52+func TestValidatorSingleEmail(t *testing.T) {
53+ vt := NewValidatorTest(t)
54+ defer vt.TearDown()
55+
56+ vt.WriteEmails(t, []string{"foo.bar@example.com"})
57+ domains := []string(nil)
58+ validator := NewValidator(domains, vt.auth_email_file.Name())
59+
60+ if !validator("foo.bar@example.com") {
61+ t.Error("email should validate")
62+ }
63+ if validator("baz.quux@example.com") {
64+ t.Error("email from same domain but not in list " +
65+ "should not validate when domain list is empty")
2366 }
67+}
68+
69+func TestValidatorSingleDomain(t *testing.T) {
70+ vt := NewValidatorTest(t)
71+ defer vt.TearDown()
72+
73+ vt.WriteEmails(t, []string(nil))
74+ domains := []string{"example.com"}
75+ validator := NewValidator(domains, vt.auth_email_file.Name())
76+
77+ if !validator("foo.bar@example.com") {
78+ t.Error("email should validate")
79+ }
80+ if !validator("baz.quux@example.com") {
81+ t.Error("email from same domain should validate")
82+ }
83+}
84+
85+func TestValidatorMultipleEmailsMultipleDomains(t *testing.T) {
86+ vt := NewValidatorTest(t)
87+ defer vt.TearDown()
88+
89+ vt.WriteEmails(t, []string{
90+ "xyzzy@example.com",
91+ "plugh@example.com",
92+ })
93+ domains := []string{"example0.com", "example1.com"}
94+ validator := NewValidator(domains, vt.auth_email_file.Name())
95+
96+ if !validator("foo.bar@example0.com") {
97+ t.Error("email from first domain should validate")
98+ }
99+ if !validator("baz.quux@example1.com") {
100+ t.Error("email from second domain should validate")
101+ }
102+ if !validator("xyzzy@example.com") {
103+ t.Error("first email in list should validate")
104+ }
105+ if !validator("plugh@example.com") {
106+ t.Error("second email in list should validate")
107+ }
108+ if validator("xyzzy.plugh@example.com") {
109+ t.Error("email not in list that matches no domains " +
110+ "should not validate")
111+ }
112+}
113+
114+func TestValidatorComparisonsAreCaseInsensitive(t *testing.T) {
115+ vt := NewValidatorTest(t)
116+ defer vt.TearDown()
24117
118+ vt.WriteEmails(t, []string{"Foo.Bar@Example.Com"})
25119 domains := []string{"Frobozz.Com"}
26− validator := NewValidator(domains, auth_email_file.Name())
120+ validator := NewValidator(domains, vt.auth_email_file.Name())
27121
28122 if !validator("foo.bar@example.com") {
29123 t.Error("loaded email addresses are not lower-cased")
validator_watcher_copy_test.gonew file+50
@@ -0,0 +1,50 @@
1+// +build go1.3
2+// +build !plan9,!solaris,!windows
3+
4+// Turns out you can't copy over an existing file on Windows.
5+
6+package main
7+
8+import (
9+ "io/ioutil"
10+ "os"
11+ "testing"
12+)
13+
14+func (vt *ValidatorTest) UpdateEmailFileViaCopyingOver(
15+ t *testing.T, emails []string) {
16+ orig_file := vt.auth_email_file
17+ var err error
18+ vt.auth_email_file, err = ioutil.TempFile("", "test_auth_emails_")
19+ if err != nil {
20+ t.Fatal("failed to create temp file for copy: " + err.Error())
21+ }
22+ vt.WriteEmails(t, emails)
23+ err = os.Rename(vt.auth_email_file.Name(), orig_file.Name())
24+ if err != nil {
25+ t.Fatal("failed to copy over temp file: " + err.Error())
26+ }
27+ vt.auth_email_file = orig_file
28+}
29+
30+func TestValidatorOverwriteEmailListViaCopyingOver(t *testing.T) {
31+ vt := NewValidatorTest(t)
32+ defer vt.TearDown()
33+
34+ vt.WriteEmails(t, []string{"xyzzy@example.com"})
35+ domains := []string(nil)
36+ updated := make(chan bool)
37+ validator := newValidatorImpl(domains, vt.auth_email_file.Name(),
38+ func() { updated <- true })
39+
40+ if !validator("xyzzy@example.com") {
41+ t.Error("email in list should validate")
42+ }
43+
44+ vt.UpdateEmailFileViaCopyingOver(t, []string{"plugh@example.com"})
45+ <-updated
46+
47+ if validator("xyzzy@example.com") {
48+ t.Error("email removed from list should not validate")
49+ }
50+}
validator_watcher_test.gonew file+105
@@ -0,0 +1,105 @@
1+// +build go1.3
2+// +build !plan9,!solaris
3+
4+package main
5+
6+import (
7+ "io/ioutil"
8+ "os"
9+ "testing"
10+)
11+
12+func (vt *ValidatorTest) UpdateEmailFile(t *testing.T, emails []string) {
13+ var err error
14+ vt.auth_email_file, err = os.OpenFile(
15+ vt.auth_email_file.Name(), os.O_WRONLY|os.O_CREATE, 0600)
16+ if err != nil {
17+ t.Fatal("failed to re-open temp file for updates")
18+ }
19+ vt.WriteEmails(t, emails)
20+}
21+
22+func (vt *ValidatorTest) UpdateEmailFileViaRenameAndReplace(
23+ t *testing.T, emails []string) {
24+ orig_file := vt.auth_email_file
25+ var err error
26+ vt.auth_email_file, err = ioutil.TempFile("", "test_auth_emails_")
27+ if err != nil {
28+ t.Fatal("failed to create temp file for rename and replace: " +
29+ err.Error())
30+ }
31+ vt.WriteEmails(t, emails)
32+
33+ moved_name := orig_file.Name() + "-moved"
34+ err = os.Rename(orig_file.Name(), moved_name)
35+ err = os.Rename(vt.auth_email_file.Name(), orig_file.Name())
36+ if err != nil {
37+ t.Fatal("failed to rename and replace temp file: " +
38+ err.Error())
39+ }
40+ vt.auth_email_file = orig_file
41+ os.Remove(moved_name)
42+}
43+
44+func TestValidatorOverwriteEmailListDirectly(t *testing.T) {
45+ vt := NewValidatorTest(t)
46+ defer vt.TearDown()
47+
48+ vt.WriteEmails(t, []string{
49+ "xyzzy@example.com",
50+ "plugh@example.com",
51+ })
52+ domains := []string(nil)
53+ updated := make(chan bool)
54+ validator := newValidatorImpl(domains, vt.auth_email_file.Name(),
55+ func() { updated <- true })
56+
57+ if !validator("xyzzy@example.com") {
58+ t.Error("first email in list should validate")
59+ }
60+ if !validator("plugh@example.com") {
61+ t.Error("second email in list should validate")
62+ }
63+ if validator("xyzzy.plugh@example.com") {
64+ t.Error("email not in list that matches no domains " +
65+ "should not validate")
66+ }
67+
68+ vt.UpdateEmailFile(t, []string{
69+ "xyzzy.plugh@example.com",
70+ "plugh@example.com",
71+ })
72+ <-updated
73+
74+ if validator("xyzzy@example.com") {
75+ t.Error("email removed from list should not validate")
76+ }
77+ if !validator("plugh@example.com") {
78+ t.Error("email retained in list should validate")
79+ }
80+ if !validator("xyzzy.plugh@example.com") {
81+ t.Error("email added to list should validate")
82+ }
83+}
84+
85+func TestValidatorOverwriteEmailListViaRenameAndReplace(t *testing.T) {
86+ vt := NewValidatorTest(t)
87+ defer vt.TearDown()
88+
89+ vt.WriteEmails(t, []string{"xyzzy@example.com"})
90+ domains := []string(nil)
91+ updated := make(chan bool)
92+ validator := newValidatorImpl(domains, vt.auth_email_file.Name(),
93+ func() { updated <- true })
94+
95+ if !validator("xyzzy@example.com") {
96+ t.Error("email in list should validate")
97+ }
98+
99+ vt.UpdateEmailFileViaRenameAndReplace(t, []string{"plugh@example.com"})
100+ <-updated
101+
102+ if validator("xyzzy@example.com") {
103+ t.Error("email removed from list should not validate")
104+ }
105+}
watcher.gonew file+64
@@ -0,0 +1,64 @@
1+// +build go1.3
2+// +build !plan9,!solaris
3+
4+package main
5+
6+import (
7+ "log"
8+ "os"
9+ "path/filepath"
10+ "time"
11+
12+ "gopkg.in/fsnotify.v1"
13+)
14+
15+func WaitForReplacement(event fsnotify.Event, watcher *fsnotify.Watcher) {
16+ const sleep_interval = 50 * time.Millisecond
17+
18+ // Avoid a race when fsnofity.Remove is preceded by fsnotify.Chmod.
19+ if event.Op&fsnotify.Chmod != 0 {
20+ time.Sleep(sleep_interval)
21+ }
22+ for {
23+ if _, err := os.Stat(event.Name); err == nil {
24+ if err := watcher.Add(event.Name); err == nil {
25+ log.Printf("watching resumed for %s", event.Name)
26+ return
27+ }
28+ }
29+ time.Sleep(sleep_interval)
30+ }
31+}
32+
33+func WatchForUpdates(filename string, action func()) bool {
34+ filename = filepath.Clean(filename)
35+ watcher, err := fsnotify.NewWatcher()
36+ if err != nil {
37+ log.Fatal("failed to create watcher for ", filename, ": ", err)
38+ }
39+ go func() {
40+ defer watcher.Close()
41+ for {
42+ select {
43+ case event := <-watcher.Events:
44+ // On Arch Linux, it appears Chmod events precede Remove events,
45+ // which causes a race between action() and the coming Remove event.
46+ // If the Remove wins, the action() (which calls
47+ // UserMap.LoadAuthenticatedEmailsFile()) crashes when the file
48+ // can't be opened.
49+ if event.Op&(fsnotify.Remove|fsnotify.Rename|fsnotify.Chmod) != 0 {
50+ log.Printf("watching interrupted on event: %s", event)
51+ WaitForReplacement(event, watcher)
52+ }
53+ log.Printf("reloading after event: %s", event)
54+ action()
55+ case err := <-watcher.Errors:
56+ log.Printf("error watching %s: %s", filename, err)
57+ }
58+ }
59+ }()
60+ if err = watcher.Add(filename); err != nil {
61+ log.Fatal("failed to add ", filename, " to watcher: ", err)
62+ }
63+ return true
64+}
watcher_unsupported.gonew file+13
@@ -0,0 +1,13 @@
1+// +build go1.1
2+// +build plan9,solaris
3+
4+package main
5+
6+import (
7+ "log"
8+)
9+
10+func WatchForUpdates(filename string, action func()) bool {
11+ log.Printf("file watching not implemented on this platform")
12+ return false
13+}