diff --git a/changelog/unreleased/issue-376 b/changelog/unreleased/issue-376 new file mode 100644 index 0000000..eca3bdd --- /dev/null +++ b/changelog/unreleased/issue-376 @@ -0,0 +1,7 @@ +Bugfix: Include a Basic authentication challenge when credentials are rejected + +Requests rejected by Basic authentication now include a `WWW-Authenticate` header, +allowing HTTP clients to prompt for credentials. + +https://github.com/restic/rest-server/issues/376 +https://github.com/restic/rest-server/pull/396 diff --git a/handlers.go b/handlers.go index 5938edd..691afa8 100644 --- a/handlers.go +++ b/handlers.go @@ -59,7 +59,7 @@ func (s *Server) ServeHTTP(w http.ResponseWriter, r *http.Request) { // First of all, check auth (will always pass if NoAuth is set) username, ok := s.checkAuth(r) if !ok { - httpDefaultError(w, http.StatusUnauthorized) + s.writeUnauthorized(w) return } diff --git a/mux.go b/mux.go index 9c604b3..a6962f4 100644 --- a/mux.go +++ b/mux.go @@ -56,11 +56,18 @@ func (s *Server) checkAuth(r *http.Request) (username string, ok bool) { return username, true } +func (s *Server) writeUnauthorized(w http.ResponseWriter) { + if !s.NoAuth && s.ProxyAuthUsername == "" { + w.Header().Set("WWW-Authenticate", `Basic realm="rest-server"`) + } + httpDefaultError(w, http.StatusUnauthorized) +} + func (s *Server) wrapMetricsAuth(f http.HandlerFunc) http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { username, ok := s.checkAuth(r) if !ok { - httpDefaultError(w, http.StatusUnauthorized) + s.writeUnauthorized(w) return } if s.PrivateRepos && username != "metrics" { diff --git a/mux_test.go b/mux_test.go index 66c09b0..3558825 100644 --- a/mux_test.go +++ b/mux_test.go @@ -1,6 +1,7 @@ package restserver import ( + "net/http" "net/http/httptest" "testing" ) @@ -78,3 +79,57 @@ func TestCheckAuth(t *testing.T) { }) } } + +func TestUnauthorizedResponsesIncludeBasicChallenge(t *testing.T) { + const challenge = `Basic realm="rest-server"` + + tests := []struct { + name string + server *Server + metrics bool + wantChallenge string + }{ + { + name: "server Basic authentication", + server: &Server{}, + wantChallenge: challenge, + }, + { + name: "server proxy authentication", + server: &Server{ProxyAuthUsername: "X-Remote-User"}, + }, + { + name: "metrics Basic authentication", + server: &Server{}, + metrics: true, + wantChallenge: challenge, + }, + { + name: "metrics proxy authentication", + server: &Server{ProxyAuthUsername: "X-Remote-User"}, + metrics: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + var handler http.HandlerFunc + if tt.metrics { + handler = tt.server.wrapMetricsAuth(func(http.ResponseWriter, *http.Request) { + t.Fatal("handler should not be called for an unauthorized request") + }) + } else { + handler = tt.server.ServeHTTP + } + + rec := httptest.NewRecorder() + handler(rec, httptest.NewRequest(http.MethodGet, "/", nil)) + if rec.Code != http.StatusUnauthorized { + t.Fatalf("status = %d, want %d", rec.Code, http.StatusUnauthorized) + } + if got := rec.Header().Get("WWW-Authenticate"); got != tt.wantChallenge { + t.Errorf("WWW-Authenticate = %q, want %q", got, tt.wantChallenge) + } + }) + } +}