Commit c3523305 authored by Matt Holt's avatar Matt Holt Committed by GitHub

Merge pull request #1325 from mholt/authheader

basicauth: Remove Authorization header on successful authz (issue #1324)
parents 3f770603 54acb9b2
...@@ -34,8 +34,7 @@ type BasicAuth struct { ...@@ -34,8 +34,7 @@ type BasicAuth struct {
// ServeHTTP implements the httpserver.Handler interface. // ServeHTTP implements the httpserver.Handler interface.
func (a BasicAuth) ServeHTTP(w http.ResponseWriter, r *http.Request) (int, error) { func (a BasicAuth) ServeHTTP(w http.ResponseWriter, r *http.Request) (int, error) {
var hasAuth bool var protected, isAuthenticated bool
var isAuthenticated bool
for _, rule := range a.Rules { for _, rule := range a.Rules {
for _, res := range rule.Resources { for _, res := range rule.Resources {
...@@ -43,29 +42,33 @@ func (a BasicAuth) ServeHTTP(w http.ResponseWriter, r *http.Request) (int, error ...@@ -43,29 +42,33 @@ func (a BasicAuth) ServeHTTP(w http.ResponseWriter, r *http.Request) (int, error
continue continue
} }
// Path matches; parse auth header // path matches; this endpoint is protected
protected = true
// parse auth header
username, password, ok := r.BasicAuth() username, password, ok := r.BasicAuth()
hasAuth = true
// Check credentials // check credentials
if !ok || if !ok ||
username != rule.Username || username != rule.Username ||
!rule.Password(password) { !rule.Password(password) {
continue continue
} }
// Flag set only on successful authentication // by this point, authentication was successful
isAuthenticated = true isAuthenticated = true
// remove credentials from request to avoid leaking upstream
r.Header.Del("Authorization")
} }
} }
if hasAuth { if protected && !isAuthenticated {
if !isAuthenticated { // browsers show a message that says something like:
w.Header().Set("WWW-Authenticate", "Basic realm=\"Restricted\"") // "The website says: <realm>"
return http.StatusUnauthorized, nil // which is kinda dumb, but whatever.
} w.Header().Set("WWW-Authenticate", "Basic realm=\"Restricted\"")
// "It's an older code, sir, but it checks out. I was about to clear them." return http.StatusUnauthorized, nil
return a.Next.ServeHTTP(w, r)
} }
// Pass-through when no paths match // Pass-through when no paths match
......
...@@ -17,51 +17,57 @@ func TestBasicAuth(t *testing.T) { ...@@ -17,51 +17,57 @@ func TestBasicAuth(t *testing.T) {
rw := BasicAuth{ rw := BasicAuth{
Next: httpserver.HandlerFunc(contentHandler), Next: httpserver.HandlerFunc(contentHandler),
Rules: []Rule{ Rules: []Rule{
{Username: "test", Password: PlainMatcher("ttest"), Resources: []string{"/testing"}}, {Username: "okuser", Password: PlainMatcher("okpass"), Resources: []string{"/testing"}},
}, },
} }
tests := []struct { tests := []struct {
from string from string
result int result int
cred string user string
password string
}{ }{
{"/testing", http.StatusUnauthorized, "ttest:test"}, {"/testing", http.StatusOK, "okuser", "okpass"},
{"/testing", http.StatusOK, "test:ttest"}, {"/testing", http.StatusUnauthorized, "baduser", "okpass"},
{"/testing", http.StatusUnauthorized, ""}, {"/testing", http.StatusUnauthorized, "okuser", "badpass"},
{"/testing", http.StatusUnauthorized, "OKuser", "okpass"},
{"/testing", http.StatusUnauthorized, "OKuser", "badPASS"},
{"/testing", http.StatusUnauthorized, "", "okpass"},
{"/testing", http.StatusUnauthorized, "okuser", ""},
{"/testing", http.StatusUnauthorized, "", ""},
} }
for i, test := range tests { for i, test := range tests {
req, err := http.NewRequest("GET", test.from, nil) req, err := http.NewRequest("GET", test.from, nil)
if err != nil { if err != nil {
t.Fatalf("Test %d: Could not create HTTP request %v", i, err) t.Fatalf("Test %d: Could not create HTTP request: %v", i, err)
} }
auth := "Basic " + base64.StdEncoding.EncodeToString([]byte(test.cred)) req.SetBasicAuth(test.user, test.password)
req.Header.Set("Authorization", auth)
rec := httptest.NewRecorder() rec := httptest.NewRecorder()
result, err := rw.ServeHTTP(rec, req) result, err := rw.ServeHTTP(rec, req)
if err != nil { if err != nil {
t.Fatalf("Test %d: Could not ServeHTTP %v", i, err) t.Fatalf("Test %d: Could not ServeHTTP: %v", i, err)
} }
if result != test.result { if result != test.result {
t.Errorf("Test %d: Expected Header '%d' but was '%d'", t.Errorf("Test %d: Expected status code %d but was %d",
i, test.result, result) i, test.result, result)
} }
if result == http.StatusUnauthorized { if test.result == http.StatusUnauthorized {
headers := rec.Header() headers := rec.Header()
if val, ok := headers["Www-Authenticate"]; ok { if val, ok := headers["Www-Authenticate"]; ok {
if val[0] != "Basic realm=\"Restricted\"" { if got, want := val[0], "Basic realm=\"Restricted\""; got != want {
t.Errorf("Test %d, Www-Authenticate should be %s provided %s", i, "Basic", val[0]) t.Errorf("Test %d: Www-Authenticate header should be '%s', got: '%s'", i, want, got)
} }
} else { } else {
t.Errorf("Test %d, should provide a header Www-Authenticate", i) t.Errorf("Test %d: response should have a 'Www-Authenticate' header", i)
}
} else {
if got, want := req.Header.Get("Authorization"), ""; got != want {
t.Errorf("Test %d: Expected Authorization header to be stripped from request after successful authentication, but is: %s", i, got)
} }
} }
} }
} }
func TestMultipleOverlappingRules(t *testing.T) { func TestMultipleOverlappingRules(t *testing.T) {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment