Properly use the 401 and 403 HTTP status codes

According to the HTTP standard, a lack of authentication credentials or
incorrect authentication credentials should send back a 401
(Unauthorized) with a `WWW-Authenticate` header with a challenge that
can be used to authenticate. This is because a 401 status should be sent
when an authentication attempt can be retried by the browser.

The 403 (Forbidden) status code should be sent when authentication
succeeded, but the user does not have the necessary authorization.
Previously, the server would always send a 401 status code.

CHANGELOG.md

@@ -34,6 +34,7 @@ With this release the systemd configuration files for InfluxDB will use the syst
 - [#6507](https://github.com/influxdata/influxdb/issues/6507): Refactor monitor service to avoid expvar and write monitor statistics on a truncated time interval.
 - [#6805](https://github.com/influxdata/influxdb/issues/6805): Allow any variant of the help option to trigger the help.
 - [#5499](https://github.com/influxdata/influxdb/issues/5499): Add stats and diagnostics to the TSM engine.
+- [#6959](https://github.com/influxdata/influxdb/issues/6959): Return 403 Forbidden when authentication succeeds but authorization fails.
 
 ### Bugfixes
 

etc/config.sample.toml

@@ -168,6 +168,7 @@ reporting-disabled = false
   ### Use a separate private key location.
   # https-private-key = ""
   max-row-limit = 10000
+  realm = "InfluxDB"
 
 ###
 ### [subsciber]

services/httpd/config.go

@@ -1,7 +1,12 @@
 package httpd
 
-// DefaultBindAddress is the default address to bind to.
-const DefaultBindAddress = ":8086"
+const (
+	// DefaultBindAddress is the default address to bind to.
+	DefaultBindAddress = ":8086"
+
+	// DefaultRealm is the default realm sent back when issuing a basic auth challenge.
+	DefaultRealm = "InfluxDB"
+)
 
 // Config represents a configuration for a HTTP service.
 type Config struct {
@@ -16,16 +21,18 @@ type Config struct {
 	MaxRowLimit        int    `toml:"max-row-limit"`
 	MaxConnectionLimit int    `toml:"max-connection-limit"`
 	SharedSecret       string `toml:"shared-secret"`
+	Realm              string `toml:"realm"`
 }
 
 // NewConfig returns a new Config with default settings.
 func NewConfig() Config {
 	return Config{
 		Enabled:          true,
-		BindAddress:      ":8086",
+		BindAddress:      DefaultBindAddress,
 		LogEnabled:       true,
 		HTTPSEnabled:     false,
 		HTTPSCertificate: "/etc/ssl/influxdb.pem",
 		MaxRowLimit:      DefaultChunkSize,
+		Realm:            DefaultRealm,
 	}
 }

services/httpd/handler.go

@@ -385,7 +385,7 @@ func (h *Handler) serveQuery(w http.ResponseWriter, r *http.Request, user *meta.
 			if err, ok := err.(meta.ErrAuthorize); ok {
 				h.Logger.Printf("Unauthorized request | user: %q | query: %q | database %q\n", err.User, err.Query.String(), err.Database)
 			}
-			h.httpError(w, "error authorizing query: "+err.Error(), pretty, http.StatusUnauthorized)
+			h.httpError(w, "error authorizing query: "+err.Error(), pretty, http.StatusForbidden)
 			return
 		}
 	}
@@ -539,13 +539,13 @@ func (h *Handler) serveWrite(w http.ResponseWriter, r *http.Request, user *meta.
 	}
 
 	if h.Config.AuthEnabled && user == nil {
-		h.resultError(w, influxql.Result{Err: fmt.Errorf("user is required to write to database %q", database)}, http.StatusUnauthorized)
+		h.resultError(w, influxql.Result{Err: fmt.Errorf("user is required to write to database %q", database)}, http.StatusForbidden)
 		return
 	}
 
 	if h.Config.AuthEnabled {
 		if err := h.WriteAuthorizer.AuthorizeWrite(user.Name, database); err != nil {
-			h.resultError(w, influxql.Result{Err: fmt.Errorf("%q user is not authorized to write to database %q", user.Name, database)}, http.StatusUnauthorized)
+			h.resultError(w, influxql.Result{Err: fmt.Errorf("%q user is not authorized to write to database %q", user.Name, database)}, http.StatusForbidden)
 			return
 		}
 	}
@@ -761,6 +761,11 @@ func (h *Handler) serveExpvar(w http.ResponseWriter, r *http.Request) {
 // h.httpError writes an error to the client in a standard format.
 func (h *Handler) httpError(w http.ResponseWriter, error string, pretty bool, code int) {
 	w.Header().Add("Content-Type", "application/json")
+	if code == http.StatusUnauthorized {
+		// If an unauthorized header will be sent back, add a WWW-Authenticate header
+		// as an authorization challenge.
+		w.Header().Set("WWW-Authenticate", fmt.Sprintf("Basic realm=\"%s\"", h.Config.Realm))
+	}
 	h.writeHeader(w, code)
 	response := Response{Err: errors.New(error)}
 	var b []byte

services/httpd/handler_test.go

@@ -274,19 +274,80 @@ func TestHandler_Query_ErrInvalidQuery(t *testing.T) {
 	}
 }
 
-// Ensure the handler returns a status 401 if the user is not authorized.
-// func TestHandler_Query_ErrUnauthorized(t *testing.T) {
-// 	h := NewHandler(false)
-// 	h.QueryExecutor.AuthorizeFn = func(u *meta.UserInfo, q *influxql.Query, db string) error {
-// 		return errors.New("marker")
-// 	}
+// Ensure the handler returns an appropriate 401 or 403 status when authentication or authorization fails.
+func TestHandler_Query_ErrAuthorize(t *testing.T) {
+	h := NewHandler(true)
+	h.QueryAuthorizer.AuthorizeQueryFn = func(u *meta.UserInfo, q *influxql.Query, db string) error {
+		return errors.New("marker")
+	}
+	h.MetaClient.UsersFn = func() []meta.UserInfo {
+		return []meta.UserInfo{
+			{
+				Name:  "admin",
+				Hash:  "admin",
+				Admin: true,
+			},
+			{
+				Name: "user1",
+				Hash: "abcd",
+				Privileges: map[string]influxql.Privilege{
+					"db0": influxql.ReadPrivilege,
+				},
+			},
+		}
+	}
+	h.MetaClient.AuthenticateFn = func(u, p string) (*meta.UserInfo, error) {
+		for _, user := range h.MetaClient.Users() {
+			if u == user.Name {
+				if p == user.Hash {
+					return &user, nil
+				}
+				return nil, meta.ErrAuthenticate
+			}
+		}
+		return nil, meta.ErrUserNotFound
+	}
 
-// 	w := httptest.NewRecorder()
-// 	h.ServeHTTP(w, MustNewJSONRequest("GET", "/query?u=bar&db=foo&q=SHOW+SERIES+FROM+bar", nil))
-// 	if w.Code != http.StatusUnauthorized {
-// 		t.Fatalf("unexpected status: %d", w.Code)
-// 	}
-// }
+	for i, tt := range []struct {
+		user     string
+		password string
+		query    string
+		code     int
+	}{
+		{
+			query: "/query?q=SHOW+DATABASES",
+			code:  http.StatusUnauthorized,
+		},
+		{
+			user:     "user1",
+			password: "abcd",
+			query:    "/query?q=SHOW+DATABASES",
+			code:     http.StatusForbidden,
+		},
+		{
+			user:     "user2",
+			password: "abcd",
+			query:    "/query?q=SHOW+DATABASES",
+			code:     http.StatusUnauthorized,
+		},
+	} {
+		w := httptest.NewRecorder()
+		r := MustNewJSONRequest("GET", tt.query, nil)
+		params := r.URL.Query()
+		if tt.user != "" {
+			params.Set("u", tt.user)
+		}
+		if tt.password != "" {
+			params.Set("p", tt.password)
+		}
+		r.URL.RawQuery = params.Encode()
+
+		h.ServeHTTP(w, r)
+		if w.Code != tt.code {
+			t.Errorf("%d. unexpected status: got=%d exp=%d\noutput: %s", i, w.Code, tt.code, w.Body.String())
+		}
+	}
+}
 
 // Ensure the handler returns a status 200 if an error is returned in the result.
 func TestHandler_Query_ErrResult(t *testing.T) {