Fix HTTP code for permission denied errors

Fixes #3697

The existing code and test case only covered the leader behavior. When
querying against non-leaders the error has an "rpc error: " prefix.

To provide consistency in HTTP error response I also strip the "rpc
error: " prefix for 403 responses as they offer no beneficial additional
information (and in theory disclose a tiny bit of data to unauthorized
users, but it would be a pretty weird bit of data to use in a malicious
way).

command/agent/http.go

@@ -11,6 +11,7 @@ import (
 	"net/http/pprof"
 	"os"
 	"strconv"
+	"strings"
 	"time"
 
 	"github.com/NYTimes/gziphandler"
@@ -281,17 +282,22 @@ func (s *HTTPServer) wrap(handler func(resp http.ResponseWriter, req *http.Reque
 		if err != nil {
 			s.logger.Printf("[ERR] http: Request %v, error: %v", reqURL, err)
 			code := 500
+			errMsg := err.Error()
 			if http, ok := err.(HTTPCodedError); ok {
 				code = http.Code()
 			} else {
-				switch err.Error() {
-				case structs.ErrPermissionDenied.Error(), structs.ErrTokenNotFound.Error():
+				// RPC errors get wrapped, so manually unwrap by only looking at their suffix
+				if strings.HasSuffix(errMsg, structs.ErrPermissionDenied.Error()) {
+					errMsg = structs.ErrPermissionDenied.Error()
+					code = 403
+				} else if strings.HasSuffix(errMsg, structs.ErrTokenNotFound.Error()) {
+					errMsg = structs.ErrTokenNotFound.Error()
 					code = 403
 				}
 			}
 
 			resp.WriteHeader(code)
-			resp.Write([]byte(err.Error()))
+			resp.Write([]byte(errMsg))
 			return
 		}
 

command/agent/http_test.go

@@ -225,15 +225,28 @@ func TestPermissionDenied(t *testing.T) {
 	})
 	defer s.Shutdown()
 
-	resp := httptest.NewRecorder()
-	handler := func(resp http.ResponseWriter, req *http.Request) (interface{}, error) {
-		return nil, structs.ErrPermissionDenied
+	{
+		resp := httptest.NewRecorder()
+		handler := func(resp http.ResponseWriter, req *http.Request) (interface{}, error) {
+			return nil, structs.ErrPermissionDenied
+		}
+
+		req, _ := http.NewRequest("GET", "/v1/job/foo", nil)
+		s.Server.wrap(handler)(resp, req)
+		assert.Equal(t, resp.Code, 403)
 	}
 
-	urlStr := "/v1/job/foo"
-	req, _ := http.NewRequest("GET", urlStr, nil)
-	s.Server.wrap(handler)(resp, req)
-	assert.Equal(t, resp.Code, 403)
+	// When remote RPC is used the errors have "rpc error: " prependend
+	{
+		resp := httptest.NewRecorder()
+		handler := func(resp http.ResponseWriter, req *http.Request) (interface{}, error) {
+			return nil, fmt.Errorf("rpc error: %v", structs.ErrPermissionDenied)
+		}
+
+		req, _ := http.NewRequest("GET", "/v1/job/foo", nil)
+		s.Server.wrap(handler)(resp, req)
+		assert.Equal(t, resp.Code, 403)
+	}
 }
 
 func TestTokenNotFound(t *testing.T) {