From 64d203491c5314205871f11a661a0b548a800db0 Mon Sep 17 00:00:00 2001 From: jordi collell Date: Fri, 8 May 2015 07:41:48 +0200 Subject: [PATCH 1/4] Some failing tests --- middleware/basicauth/basicauth_test.go | 58 ++++++++++++++++++++++++++ 1 file changed, 58 insertions(+) create mode 100644 middleware/basicauth/basicauth_test.go diff --git a/middleware/basicauth/basicauth_test.go b/middleware/basicauth/basicauth_test.go new file mode 100644 index 00000000..7b551ba4 --- /dev/null +++ b/middleware/basicauth/basicauth_test.go @@ -0,0 +1,58 @@ +package basicauth + +import ( + "encoding/base64" + "fmt" + "net/http" + "net/http/httptest" + "testing" + + "github.com/mholt/caddy/middleware" +) + +func TestBasicAuth(t *testing.T) { + + rw := BasicAuth{ + Next: middleware.HandlerFunc(contentHandler), + Rules: []Rule{ + {Username: "test", Password: "ttest", Resources: []string{"/testing"}}, + }, + } + + tests := []struct { + from string + result int + cred string + }{ + {"/testing", http.StatusOK, "test:ttest"}, + {"/testing", http.StatusUnauthorized, ""}, + + } + + //auth := "Basic " + base64.StdEncoding.EncodeToString([]byte("foo:bar")) + for i, test := range tests { + + + req, err := http.NewRequest("GET", test.from, nil) + if err != nil { + t.Fatalf("Test %d: Could not create HTTP request %v", i, err) + } + auth := "Basic " + base64.StdEncoding.EncodeToString([]byte(test.cred)) + req.Header.Set("Authorization", auth) + + rec := httptest.NewRecorder() + rw.ServeHTTP(rec, req) + + if rec.Code != test.result { + t.Errorf("Test %d: Expected Header '%d' but was '%d'", + i, test.result, rec.Code) + } + + } + +} + +func contentHandler(w http.ResponseWriter, r *http.Request) (int, error) { + fmt.Fprintf(w, r.URL.String()) + return 0, nil +} \ No newline at end of file From 253c069b26a36694c553b5cf239ccc6e38af87a7 Mon Sep 17 00:00:00 2001 From: jordi collell Date: Fri, 8 May 2015 09:41:17 +0200 Subject: [PATCH 2/4] if basic auth fails should write unauthorized to response --- middleware/basicauth/basicauth.go | 1 + middleware/basicauth/basicauth_test.go | 15 ++++++++++++--- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/middleware/basicauth/basicauth.go b/middleware/basicauth/basicauth.go index de29b8d9..0ab92437 100644 --- a/middleware/basicauth/basicauth.go +++ b/middleware/basicauth/basicauth.go @@ -31,6 +31,7 @@ func (a BasicAuth) ServeHTTP(w http.ResponseWriter, r *http.Request) (int, error // Check credentials if !ok || username != rule.Username || password != rule.Password { w.Header().Set("WWW-Authenticate", "Basic") + w.WriteHeader(http.StatusUnauthorized) return http.StatusUnauthorized, nil } diff --git a/middleware/basicauth/basicauth_test.go b/middleware/basicauth/basicauth_test.go index 7b551ba4..6ba34b65 100644 --- a/middleware/basicauth/basicauth_test.go +++ b/middleware/basicauth/basicauth_test.go @@ -24,12 +24,14 @@ func TestBasicAuth(t *testing.T) { result int cred string }{ + {"/testing", http.StatusUnauthorized, "ttest:test"}, {"/testing", http.StatusOK, "test:ttest"}, + {"/testing", http.StatusUnauthorized, ""}, } - //auth := "Basic " + base64.StdEncoding.EncodeToString([]byte("foo:bar")) + for i, test := range tests { @@ -41,7 +43,14 @@ func TestBasicAuth(t *testing.T) { req.Header.Set("Authorization", auth) rec := httptest.NewRecorder() - rw.ServeHTTP(rec, req) + result, err := rw.ServeHTTP(rec, req) + if err != nil { + t.Fatalf("Test %d: Could not ServeHTTP %v", i, err) + } + if result != test.result { + t.Errorf("Test %d: Expected Header '%d' but was '%d'", + i, test.result, result) + } if rec.Code != test.result { t.Errorf("Test %d: Expected Header '%d' but was '%d'", @@ -54,5 +63,5 @@ func TestBasicAuth(t *testing.T) { func contentHandler(w http.ResponseWriter, r *http.Request) (int, error) { fmt.Fprintf(w, r.URL.String()) - return 0, nil + return http.StatusOK, nil } \ No newline at end of file From 4c1185492766f9b112809575271a609f7b246619 Mon Sep 17 00:00:00 2001 From: jordi collell Date: Sat, 9 May 2015 08:11:02 +0200 Subject: [PATCH 3/4] added header match and a new failing test --- middleware/basicauth/basicauth.go | 1 - middleware/basicauth/basicauth_test.go | 64 ++++++++++++++++++++++++-- 2 files changed, 59 insertions(+), 6 deletions(-) diff --git a/middleware/basicauth/basicauth.go b/middleware/basicauth/basicauth.go index 0ab92437..de29b8d9 100644 --- a/middleware/basicauth/basicauth.go +++ b/middleware/basicauth/basicauth.go @@ -31,7 +31,6 @@ func (a BasicAuth) ServeHTTP(w http.ResponseWriter, r *http.Request) (int, error // Check credentials if !ok || username != rule.Username || password != rule.Password { w.Header().Set("WWW-Authenticate", "Basic") - w.WriteHeader(http.StatusUnauthorized) return http.StatusUnauthorized, nil } diff --git a/middleware/basicauth/basicauth_test.go b/middleware/basicauth/basicauth_test.go index 6ba34b65..04d9fc83 100644 --- a/middleware/basicauth/basicauth_test.go +++ b/middleware/basicauth/basicauth_test.go @@ -26,7 +26,6 @@ func TestBasicAuth(t *testing.T) { }{ {"/testing", http.StatusUnauthorized, "ttest:test"}, {"/testing", http.StatusOK, "test:ttest"}, - {"/testing", http.StatusUnauthorized, ""}, } @@ -51,16 +50,71 @@ func TestBasicAuth(t *testing.T) { t.Errorf("Test %d: Expected Header '%d' but was '%d'", i, test.result, result) } - - if rec.Code != test.result { - t.Errorf("Test %d: Expected Header '%d' but was '%d'", - i, test.result, rec.Code) + if result == http.StatusUnauthorized { + headers := rec.Header() + if val, ok := headers["Www-Authenticate"]; ok { + if val[0] != "Basic" { + t.Errorf("Test %d, Www-Authenticate should be %s provided %s", i, "Basic", val[0]) + } + } else { + t.Errorf("Test %d, should provide a header Www-Authenticate", i) + } } + } } + +func TestMultipleOverlappingRules(t *testing.T) { + rw := BasicAuth{ + Next: middleware.HandlerFunc(contentHandler), + Rules: []Rule{ + {Username: "t", Password: "p1", Resources: []string{"/t"}}, + {Username: "t1", Password: "p2", Resources: []string{"/t/t"}}, + }, + } + + tests := []struct { + from string + result int + cred string + }{ + {"/t", http.StatusOK, "t:p1"}, + {"/t/t", http.StatusOK, "t:p1"}, + {"/t/t", http.StatusOK, "t1:p2"}, + + } + + + for i, test := range tests { + + + req, err := http.NewRequest("GET", test.from, nil) + if err != nil { + t.Fatalf("Test %d: Could not create HTTP request %v", i, err) + } + auth := "Basic " + base64.StdEncoding.EncodeToString([]byte(test.cred)) + req.Header.Set("Authorization", auth) + + rec := httptest.NewRecorder() + result, err := rw.ServeHTTP(rec, req) + if err != nil { + t.Fatalf("Test %d: Could not ServeHTTP %v", i, err) + } + if result != test.result { + t.Errorf("Test %d: Expected Header '%d' but was '%d'", + i, test.result, result) + } + + + } + +} + + + func contentHandler(w http.ResponseWriter, r *http.Request) (int, error) { fmt.Fprintf(w, r.URL.String()) return http.StatusOK, nil From 99fa4581aa0e8c309f0745687d8aa7e832b31ae4 Mon Sep 17 00:00:00 2001 From: jordi collell Date: Sun, 10 May 2015 08:20:58 +0200 Subject: [PATCH 4/4] basicauth: patch for overlapping rules --- middleware/basicauth/basicauth.go | 20 +++++++++++++++++--- middleware/basicauth/basicauth_test.go | 6 +++--- 2 files changed, 20 insertions(+), 6 deletions(-) diff --git a/middleware/basicauth/basicauth.go b/middleware/basicauth/basicauth.go index de29b8d9..25d1d504 100644 --- a/middleware/basicauth/basicauth.go +++ b/middleware/basicauth/basicauth.go @@ -19,6 +19,10 @@ type BasicAuth struct { // ServeHTTP implements the middleware.Handler interface. func (a BasicAuth) ServeHTTP(w http.ResponseWriter, r *http.Request) (int, error) { + + var hasAuth bool + var isAuthenticated bool + for _, rule := range a.Rules { for _, res := range rule.Resources { if !middleware.Path(r.URL.Path).Matches(res) { @@ -27,16 +31,26 @@ func (a BasicAuth) ServeHTTP(w http.ResponseWriter, r *http.Request) (int, error // Path matches; parse auth header username, password, ok := r.BasicAuth() + hasAuth = true // Check credentials if !ok || username != rule.Username || password != rule.Password { - w.Header().Set("WWW-Authenticate", "Basic") - return http.StatusUnauthorized, nil + continue } - + // flag set only on success authentication + isAuthenticated = true + } + } + + if hasAuth { + if !isAuthenticated { + w.Header().Set("WWW-Authenticate", "Basic") + return http.StatusUnauthorized, nil + } else { // "It's an older code, sir, but it checks out. I was about to clear them." return a.Next.ServeHTTP(w, r) } + } // Pass-thru when no paths match diff --git a/middleware/basicauth/basicauth_test.go b/middleware/basicauth/basicauth_test.go index 04d9fc83..b590bc35 100644 --- a/middleware/basicauth/basicauth_test.go +++ b/middleware/basicauth/basicauth_test.go @@ -84,12 +84,13 @@ func TestMultipleOverlappingRules(t *testing.T) { {"/t", http.StatusOK, "t:p1"}, {"/t/t", http.StatusOK, "t:p1"}, {"/t/t", http.StatusOK, "t1:p2"}, - + {"/a", http.StatusOK, "t1:p2"}, + {"/t/t", http.StatusUnauthorized, "t1:p3"}, + {"/t", http.StatusUnauthorized, "t1:p2"}, } for i, test := range tests { - req, err := http.NewRequest("GET", test.from, nil) if err != nil { @@ -108,7 +109,6 @@ func TestMultipleOverlappingRules(t *testing.T) { i, test.result, result) } - } }