agent: protect the ui metrics proxy endpoint behind ACLs (#9099)

This ensures the metrics proxy endpoint is ACL protected behind a
wildcard `service:read` and `node:read` set of rules. For Consul
Enterprise these will need to span all namespaces:

```
service_prefix "" { policy = "read" }
node_prefix ""    { policy = "read" }

namespace_prefix "" {
  service_prefix "" { policy = "read" }
  node_prefix ""    { policy = "read" }
}
```

This PR contains just the backend changes. The frontend changes to
actually pass the consul token header to the proxy through the JS plugin
will come in another PR.

acl/acl_test.go

@@ -100,6 +100,10 @@ func checkAllowNodeRead(t *testing.T, authz Authorizer, prefix string, entCtx *A
 	require.Equal(t, Allow, authz.NodeRead(prefix, entCtx))
 }
 
+func checkAllowNodeReadAll(t *testing.T, authz Authorizer, _ string, entCtx *AuthorizerContext) {
+	require.Equal(t, Allow, authz.NodeReadAll(entCtx))
+}
+
 func checkAllowNodeWrite(t *testing.T, authz Authorizer, prefix string, entCtx *AuthorizerContext) {
 	require.Equal(t, Allow, authz.NodeWrite(prefix, entCtx))
 }
@@ -124,6 +128,10 @@ func checkAllowServiceRead(t *testing.T, authz Authorizer, prefix string, entCtx
 	require.Equal(t, Allow, authz.ServiceRead(prefix, entCtx))
 }
 
+func checkAllowServiceReadAll(t *testing.T, authz Authorizer, _ string, entCtx *AuthorizerContext) {
+	require.Equal(t, Allow, authz.ServiceReadAll(entCtx))
+}
+
 func checkAllowServiceWrite(t *testing.T, authz Authorizer, prefix string, entCtx *AuthorizerContext) {
 	require.Equal(t, Allow, authz.ServiceWrite(prefix, entCtx))
 }
@@ -204,6 +212,10 @@ func checkDenyNodeRead(t *testing.T, authz Authorizer, prefix string, entCtx *Au
 	require.Equal(t, Deny, authz.NodeRead(prefix, entCtx))
 }
 
+func checkDenyNodeReadAll(t *testing.T, authz Authorizer, _ string, entCtx *AuthorizerContext) {
+	require.Equal(t, Deny, authz.NodeReadAll(entCtx))
+}
+
 func checkDenyNodeWrite(t *testing.T, authz Authorizer, prefix string, entCtx *AuthorizerContext) {
 	require.Equal(t, Deny, authz.NodeWrite(prefix, entCtx))
 }
@@ -228,6 +240,10 @@ func checkDenyServiceRead(t *testing.T, authz Authorizer, prefix string, entCtx
 	require.Equal(t, Deny, authz.ServiceRead(prefix, entCtx))
 }
 
+func checkDenyServiceReadAll(t *testing.T, authz Authorizer, _ string, entCtx *AuthorizerContext) {
+	require.Equal(t, Deny, authz.ServiceReadAll(entCtx))
+}
+
 func checkDenyServiceWrite(t *testing.T, authz Authorizer, prefix string, entCtx *AuthorizerContext) {
 	require.Equal(t, Deny, authz.ServiceWrite(prefix, entCtx))
 }
@@ -308,6 +324,10 @@ func checkDefaultNodeRead(t *testing.T, authz Authorizer, prefix string, entCtx
 	require.Equal(t, Default, authz.NodeRead(prefix, entCtx))
 }
 
+func checkDefaultNodeReadAll(t *testing.T, authz Authorizer, _ string, entCtx *AuthorizerContext) {
+	require.Equal(t, Default, authz.NodeReadAll(entCtx))
+}
+
 func checkDefaultNodeWrite(t *testing.T, authz Authorizer, prefix string, entCtx *AuthorizerContext) {
 	require.Equal(t, Default, authz.NodeWrite(prefix, entCtx))
 }
@@ -332,6 +352,10 @@ func checkDefaultServiceRead(t *testing.T, authz Authorizer, prefix string, entC
 	require.Equal(t, Default, authz.ServiceRead(prefix, entCtx))
 }
 
+func checkDefaultServiceReadAll(t *testing.T, authz Authorizer, _ string, entCtx *AuthorizerContext) {
+	require.Equal(t, Default, authz.ServiceReadAll(entCtx))
+}
+
 func checkDefaultServiceWrite(t *testing.T, authz Authorizer, prefix string, entCtx *AuthorizerContext) {
 	require.Equal(t, Default, authz.ServiceWrite(prefix, entCtx))
 }
@@ -381,12 +405,14 @@ func TestACL(t *testing.T) {
 				{name: "DenyKeyringWrite", check: checkDenyKeyringWrite},
 				{name: "DenyKeyWrite", check: checkDenyKeyWrite},
 				{name: "DenyNodeRead", check: checkDenyNodeRead},
+				{name: "DenyNodeReadAll", check: checkDenyNodeReadAll},
 				{name: "DenyNodeWrite", check: checkDenyNodeWrite},
 				{name: "DenyOperatorRead", check: checkDenyOperatorRead},
 				{name: "DenyOperatorWrite", check: checkDenyOperatorWrite},
 				{name: "DenyPreparedQueryRead", check: checkDenyPreparedQueryRead},
 				{name: "DenyPreparedQueryWrite", check: checkDenyPreparedQueryWrite},
 				{name: "DenyServiceRead", check: checkDenyServiceRead},
+				{name: "DenyServiceReadAll", check: checkDenyServiceReadAll},
 				{name: "DenyServiceWrite", check: checkDenyServiceWrite},
 				{name: "DenySessionRead", check: checkDenySessionRead},
 				{name: "DenySessionWrite", check: checkDenySessionWrite},
@@ -411,12 +437,14 @@ func TestACL(t *testing.T) {
 				{name: "AllowKeyringWrite", check: checkAllowKeyringWrite},
 				{name: "AllowKeyWrite", check: checkAllowKeyWrite},
 				{name: "AllowNodeRead", check: checkAllowNodeRead},
+				{name: "AllowNodeReadAll", check: checkAllowNodeReadAll},
 				{name: "AllowNodeWrite", check: checkAllowNodeWrite},
 				{name: "AllowOperatorRead", check: checkAllowOperatorRead},
 				{name: "AllowOperatorWrite", check: checkAllowOperatorWrite},
 				{name: "AllowPreparedQueryRead", check: checkAllowPreparedQueryRead},
 				{name: "AllowPreparedQueryWrite", check: checkAllowPreparedQueryWrite},
 				{name: "AllowServiceRead", check: checkAllowServiceRead},
+				{name: "AllowServiceReadAll", check: checkAllowServiceReadAll},
 				{name: "AllowServiceWrite", check: checkAllowServiceWrite},
 				{name: "AllowSessionRead", check: checkAllowSessionRead},
 				{name: "AllowSessionWrite", check: checkAllowSessionWrite},
@@ -441,12 +469,14 @@ func TestACL(t *testing.T) {
 				{name: "AllowKeyringWrite", check: checkAllowKeyringWrite},
 				{name: "AllowKeyWrite", check: checkAllowKeyWrite},
 				{name: "AllowNodeRead", check: checkAllowNodeRead},
+				{name: "AllowNodeReadAll", check: checkAllowNodeReadAll},
 				{name: "AllowNodeWrite", check: checkAllowNodeWrite},
 				{name: "AllowOperatorRead", check: checkAllowOperatorRead},
 				{name: "AllowOperatorWrite", check: checkAllowOperatorWrite},
 				{name: "AllowPreparedQueryRead", check: checkAllowPreparedQueryRead},
 				{name: "AllowPreparedQueryWrite", check: checkAllowPreparedQueryWrite},
 				{name: "AllowServiceRead", check: checkAllowServiceRead},
+				{name: "AllowServiceReadAll", check: checkAllowServiceReadAll},
 				{name: "AllowServiceWrite", check: checkAllowServiceWrite},
 				{name: "AllowSessionRead", check: checkAllowSessionRead},
 				{name: "AllowSessionWrite", check: checkAllowSessionWrite},
@@ -995,6 +1025,7 @@ func TestACL(t *testing.T) {
 				}),
 			},
 			checks: []aclCheck{
+				{name: "ReadAllDenied", prefix: "", check: checkDenyNodeReadAll},
 				{name: "DefaultReadDenied", prefix: "nope", check: checkDenyNodeRead},
 				{name: "DefaultWriteDenied", prefix: "nope", check: checkDenyNodeWrite},
 				{name: "DenyReadDenied", prefix: "root-nope", check: checkDenyNodeRead},
@@ -1075,6 +1106,7 @@ func TestACL(t *testing.T) {
 				}),
 			},
 			checks: []aclCheck{
+				{name: "ReadAllDenied", prefix: "", check: checkDenyNodeReadAll},
 				{name: "DefaultReadAllowed", prefix: "nope", check: checkAllowNodeRead},
 				{name: "DefaultWriteAllowed", prefix: "nope", check: checkAllowNodeWrite},
 				{name: "DenyReadDenied", prefix: "root-nope", check: checkDenyNodeRead},
@@ -1335,6 +1367,7 @@ func TestACL(t *testing.T) {
 				}),
 			},
 			checks: []aclCheck{
+				{name: "ServiceReadAllDenied", prefix: "", check: checkDenyServiceReadAll},
 				{name: "KeyReadDenied", prefix: "other", check: checkDenyKeyRead},
 				{name: "KeyWriteDenied", prefix: "other", check: checkDenyKeyWrite},
 				{name: "KeyWritePrefixDenied", prefix: "other", check: checkDenyKeyWritePrefix},
@@ -1464,6 +1497,7 @@ func TestACL(t *testing.T) {
 				}),
 			},
 			checks: []aclCheck{
+				{name: "ServiceReadAllDenied", prefix: "", check: checkDenyServiceReadAll},
 				{name: "KeyReadAllowed", prefix: "other", check: checkAllowKeyRead},
 				{name: "KeyWriteAllowed", prefix: "other", check: checkAllowKeyWrite},
 				{name: "KeyWritePrefixAllowed", prefix: "other", check: checkAllowKeyWritePrefix},
@@ -1708,6 +1742,9 @@ func TestACL(t *testing.T) {
 				},
 			},
 			checks: []aclCheck{
+				{name: "NodeReadAllDenied", prefix: "", check: checkDenyNodeReadAll},
+				{name: "ServiceReadAllDenied", prefix: "", check: checkDenyServiceReadAll},
+
 				{name: "AgentReadPrefixAllowed", prefix: "fo", check: checkAllowAgentRead},
 				{name: "AgentWritePrefixDenied", prefix: "fo", check: checkDenyAgentWrite},
 				{name: "AgentReadPrefixAllowed", prefix: "for", check: checkAllowAgentRead},
@@ -2101,3 +2138,78 @@ func TestACLEnforce(t *testing.T) {
 		})
 	}
 }
+
+func TestACL_ReadAll(t *testing.T) {
+	type testcase struct {
+		name  string
+		rules string
+		check func(t *testing.T, authz Authorizer, prefix string, entCtx *AuthorizerContext)
+	}
+
+	tests := []testcase{
+		{
+			name:  "node:bar:read",
+			rules: `node "bar" { policy = "read" }`,
+			check: checkDenyNodeReadAll,
+		},
+		{
+			name:  "node:bar:write",
+			rules: `node "bar" { policy = "write" }`,
+			check: checkDenyNodeReadAll,
+		},
+		{
+			name:  "node:*:read",
+			rules: `node_prefix "" { policy = "read" }`,
+			check: checkAllowNodeReadAll,
+		},
+		{
+			name:  "node:*:write",
+			rules: `node_prefix "" { policy = "write" }`,
+			check: checkAllowNodeReadAll,
+		},
+		{
+			name:  "service:bar:read",
+			rules: `service "bar" { policy = "read" }`,
+			check: checkDenyServiceReadAll,
+		},
+		{
+			name:  "service:bar:write",
+			rules: `service "bar" { policy = "write" }`,
+			check: checkDenyServiceReadAll,
+		},
+		{
+			name:  "service:*:read",
+			rules: `service_prefix "" { policy = "read" }`,
+			check: checkAllowServiceReadAll,
+		},
+		{
+			name:  "service:*:write",
+			rules: `service_prefix "" { policy = "write" }`,
+			check: checkAllowServiceReadAll,
+		},
+	}
+
+	body := func(t *testing.T, rules string, defaultPolicy Authorizer, check func(t *testing.T, authz Authorizer, prefix string, entCtx *AuthorizerContext)) {
+		t.Helper()
+
+		policy, err := NewPolicyFromSource("", 0, rules, SyntaxCurrent, nil, nil)
+		require.NoError(t, err)
+
+		acl, err := NewPolicyAuthorizerWithDefaults(defaultPolicy, []*Policy{policy}, nil)
+		require.NoError(t, err)
+
+		check(t, acl, "", nil)
+	}
+
+	for _, tc := range tests {
+		tc := tc
+		t.Run(tc.name, func(t *testing.T) {
+			t.Run("default deny", func(t *testing.T) {
+				body(t, tc.rules, DenyAll(), tc.check)
+			})
+			t.Run("default allow", func(t *testing.T) {
+				body(t, tc.rules, AllowAll(), checkAllowNodeReadAll)
+			})
+		})
+	}
+}

acl/authorizer.go

@@ -107,6 +107,9 @@ type Authorizer interface {
 	// NodeRead checks for permission to read (discover) a given node.
 	NodeRead(string, *AuthorizerContext) EnforcementDecision
 
+	// NodeReadAll checks for permission to read (discover) all nodes.
+	NodeReadAll(*AuthorizerContext) EnforcementDecision
+
 	// NodeWrite checks for permission to create or update (register) a
 	// given node.
 	NodeWrite(string, *AuthorizerContext) EnforcementDecision
@@ -130,6 +133,9 @@ type Authorizer interface {
 	// ServiceRead checks for permission to read a given service
 	ServiceRead(string, *AuthorizerContext) EnforcementDecision
 
+	// ServiceReadAll checks for permission to read all services
+	ServiceReadAll(*AuthorizerContext) EnforcementDecision
+
 	// ServiceWrite checks for permission to create or update a given
 	// service
 	ServiceWrite(string, *AuthorizerContext) EnforcementDecision

acl/authorizer_test.go

@@ -12,6 +12,8 @@ type mockAuthorizer struct {
 	mock.Mock
 }
 
+var _ Authorizer = (*mockAuthorizer)(nil)
+
 // ACLRead checks for permission to list all the ACLs
 func (m *mockAuthorizer) ACLRead(ctx *AuthorizerContext) EnforcementDecision {
 	ret := m.Called(ctx)
@@ -115,6 +117,11 @@ func (m *mockAuthorizer) NodeRead(segment string, ctx *AuthorizerContext) Enforc
 	return ret.Get(0).(EnforcementDecision)
 }
 
+func (m *mockAuthorizer) NodeReadAll(ctx *AuthorizerContext) EnforcementDecision {
+	ret := m.Called(ctx)
+	return ret.Get(0).(EnforcementDecision)
+}
+
 // NodeWrite checks for permission to create or update (register) a
 // given node.
 func (m *mockAuthorizer) NodeWrite(segment string, ctx *AuthorizerContext) EnforcementDecision {
@@ -156,6 +163,11 @@ func (m *mockAuthorizer) ServiceRead(segment string, ctx *AuthorizerContext) Enf
 	return ret.Get(0).(EnforcementDecision)
 }
 
+func (m *mockAuthorizer) ServiceReadAll(ctx *AuthorizerContext) EnforcementDecision {
+	ret := m.Called(ctx)
+	return ret.Get(0).(EnforcementDecision)
+}
+
 // ServiceWrite checks for permission to create or update a given
 // service
 func (m *mockAuthorizer) ServiceWrite(segment string, ctx *AuthorizerContext) EnforcementDecision {

acl/chained_authorizer.go

@@ -152,6 +152,12 @@ func (c *ChainedAuthorizer) NodeRead(node string, entCtx *AuthorizerContext) Enf
 	})
 }
 
+func (c *ChainedAuthorizer) NodeReadAll(entCtx *AuthorizerContext) EnforcementDecision {
+	return c.executeChain(func(authz Authorizer) EnforcementDecision {
+		return authz.NodeReadAll(entCtx)
+	})
+}
+
 // NodeWrite checks for permission to create or update (register) a
 // given node.
 func (c *ChainedAuthorizer) NodeWrite(node string, entCtx *AuthorizerContext) EnforcementDecision {
@@ -199,6 +205,12 @@ func (c *ChainedAuthorizer) ServiceRead(name string, entCtx *AuthorizerContext)
 	})
 }
 
+func (c *ChainedAuthorizer) ServiceReadAll(entCtx *AuthorizerContext) EnforcementDecision {
+	return c.executeChain(func(authz Authorizer) EnforcementDecision {
+		return authz.ServiceReadAll(entCtx)
+	})
+}
+
 // ServiceWrite checks for permission to create or update a given
 // service
 func (c *ChainedAuthorizer) ServiceWrite(name string, entCtx *AuthorizerContext) EnforcementDecision {

acl/chained_authorizer_test.go

@@ -6,6 +6,8 @@ import (
 
 type testAuthorizer EnforcementDecision
 
+var _ Authorizer = testAuthorizer(Allow)
+
 func (authz testAuthorizer) ACLRead(*AuthorizerContext) EnforcementDecision {
 	return EnforcementDecision(authz)
 }
@@ -54,6 +56,9 @@ func (authz testAuthorizer) KeyringWrite(*AuthorizerContext) EnforcementDecision
 func (authz testAuthorizer) NodeRead(string, *AuthorizerContext) EnforcementDecision {
 	return EnforcementDecision(authz)
 }
+func (authz testAuthorizer) NodeReadAll(*AuthorizerContext) EnforcementDecision {
+	return EnforcementDecision(authz)
+}
 func (authz testAuthorizer) NodeWrite(string, *AuthorizerContext) EnforcementDecision {
 	return EnforcementDecision(authz)
 }
@@ -72,6 +77,9 @@ func (authz testAuthorizer) PreparedQueryWrite(string, *AuthorizerContext) Enfor
 func (authz testAuthorizer) ServiceRead(string, *AuthorizerContext) EnforcementDecision {
 	return EnforcementDecision(authz)
 }
+func (authz testAuthorizer) ServiceReadAll(*AuthorizerContext) EnforcementDecision {
+	return EnforcementDecision(authz)
+}
 func (authz testAuthorizer) ServiceWrite(string, *AuthorizerContext) EnforcementDecision {
 	return EnforcementDecision(authz)
 }

acl/policy_authorizer.go

@@ -350,7 +350,7 @@ type enforceCallback func(raw interface{}, prefixOnly bool) EnforcementDecision
 func anyAllowed(tree *radix.Tree, enforceFn enforceCallback) EnforcementDecision {
 	decision := Default
 
-	// special case for handling a catch-all prefix rule. If the rule woul Deny access then our default decision
+	// special case for handling a catch-all prefix rule. If the rule would Deny access then our default decision
 	// should be to Deny, but this decision should still be overridable with other more specific rules.
 	if raw, found := tree.Get(""); found {
 		decision = enforceFn(raw, true)
@@ -686,6 +686,10 @@ func (p *policyAuthorizer) NodeRead(name string, _ *AuthorizerContext) Enforceme
 	return Default
 }
 
+func (p *policyAuthorizer) NodeReadAll(_ *AuthorizerContext) EnforcementDecision {
+	return p.allAllowed(p.nodeRules, AccessRead)
+}
+
 // NodeWrite checks if writing (registering) a node is allowed
 func (p *policyAuthorizer) NodeWrite(name string, _ *AuthorizerContext) EnforcementDecision {
 	if rule, ok := getPolicy(name, p.nodeRules); ok {
@@ -720,6 +724,10 @@ func (p *policyAuthorizer) ServiceRead(name string, _ *AuthorizerContext) Enforc
 	return Default
 }
 
+func (p *policyAuthorizer) ServiceReadAll(_ *AuthorizerContext) EnforcementDecision {
+	return p.allAllowed(p.serviceRules, AccessRead)
+}
+
 // ServiceWrite checks if writing (registering) a service is allowed
 func (p *policyAuthorizer) ServiceWrite(name string, _ *AuthorizerContext) EnforcementDecision {
 	if rule, ok := getPolicy(name, p.serviceRules); ok {

acl/static_authorizer.go

@@ -142,6 +142,13 @@ func (s *staticAuthorizer) NodeRead(string, *AuthorizerContext) EnforcementDecis
 	return Deny
 }
 
+func (s *staticAuthorizer) NodeReadAll(*AuthorizerContext) EnforcementDecision {
+	if s.defaultAllow {
+		return Allow
+	}
+	return Deny
+}
+
 func (s *staticAuthorizer) NodeWrite(string, *AuthorizerContext) EnforcementDecision {
 	if s.defaultAllow {
 		return Allow
@@ -184,6 +191,13 @@ func (s *staticAuthorizer) ServiceRead(string, *AuthorizerContext) EnforcementDe
 	return Deny
 }
 
+func (s *staticAuthorizer) ServiceReadAll(*AuthorizerContext) EnforcementDecision {
+	if s.defaultAllow {
+		return Allow
+	}
+	return Deny
+}
+
 func (s *staticAuthorizer) ServiceWrite(string, *AuthorizerContext) EnforcementDecision {
 	if s.defaultAllow {
 		return Allow

agent/http.go

@@ -895,12 +895,26 @@ func (s *HTTPHandlers) parseDC(req *http.Request, dc *string) {
 // parseTokenInternal is used to parse the ?token query param or the X-Consul-Token header or
 // Authorization Bearer token (RFC6750).
 func (s *HTTPHandlers) parseTokenInternal(req *http.Request, token *string) {
-	tok := ""
 	if other := req.URL.Query().Get("token"); other != "" {
-		tok = other
+		*token = other
-	} else if other := req.Header.Get("X-Consul-Token"); other != "" {
+		return
-		tok = other
+	}
-	} else if other := req.Header.Get("Authorization"); other != "" {
+
+	if ok := s.parseTokenFromHeaders(req, token); ok {
+		return
+	}
+
+	*token = ""
+	return
+}
+
+func (s *HTTPHandlers) parseTokenFromHeaders(req *http.Request, token *string) bool {
+	if other := req.Header.Get("X-Consul-Token"); other != "" {
+		*token = other
+		return true
+	}
+
+	if other := req.Header.Get("Authorization"); other != "" {
 		// HTTP Authorization headers are in the format: <Scheme>[SPACE]<Value>
 		// Ref. https://tools.ietf.org/html/rfc7236#section-3
 		parts := strings.Split(other, " ")
@@ -916,13 +930,18 @@ func (s *HTTPHandlers) parseTokenInternal(req *http.Request, token *string) {
 			if strings.ToLower(scheme) == "bearer" {
 				// Since Bearer tokens shouldn't contain spaces (rfc6750#section-2.1)
 				// "value" is tokenized, only the first item is used
-				tok = strings.TrimSpace(strings.Split(value, " ")[0])
+				*token = strings.TrimSpace(strings.Split(value, " ")[0])
+				return true
 			}
 		}
 	}
 
-	*token = tok
+	return false
-	return
+}
+
+func (s *HTTPHandlers) clearTokenFromHeaders(req *http.Request) {
+	req.Header.Del("X-Consul-Token")
+	req.Header.Del("Authorization")
 }
 
 // parseTokenWithDefault passes through to parseTokenInternal and optionally resolves proxy tokens to real ACL tokens.

agent/ui_endpoint.go

@@ -9,6 +9,7 @@ import (
 	"sort"
 	"strings"
 
+	"github.com/hashicorp/consul/acl"
 	"github.com/hashicorp/consul/agent/config"
 	"github.com/hashicorp/consul/agent/structs"
 	"github.com/hashicorp/consul/api"
@@ -566,6 +567,33 @@ func (s *HTTPHandlers) UIMetricsProxy(resp http.ResponseWriter, req *http.Reques
 		return nil, NotFoundError{Reason: "Metrics proxy is not enabled"}
 	}
 
+	// Fetch the ACL token, if provided, but ONLY from headers since other
+	// metrics proxies might use a ?token query string parameter for something.
+	var token string
+	s.parseTokenFromHeaders(req, &token)
+
+	// Clear the token from the headers so we don't end up proxying it.
+	s.clearTokenFromHeaders(req)
+
+	var entMeta structs.EnterpriseMeta
+	authz, err := s.agent.resolveTokenAndDefaultMeta(token, &entMeta, nil)
+	if err != nil {
+		return nil, err
+	}
+
+	if authz != nil {
+		// This endpoint requires wildcard read on all services and all nodes.
+		//
+		// In enterprise it requires this _in all namespaces_ too.
+		wildMeta := structs.WildcardEnterpriseMeta()
+		var authzContext acl.AuthorizerContext
+		wildMeta.FillAuthzContext(&authzContext)
+
+		if authz.NodeReadAll(&authzContext) != acl.Allow || authz.ServiceReadAll(&authzContext) != acl.Allow {
+			return nil, acl.ErrPermissionDenied
+		}
+	}
+
 	log := s.agent.logger.Named(logging.UIMetricsProxy)
 
 	// Construct the new URL from the path and the base path. Note we do this here

agent/ui_endpoint_oss_test.go

@@ -0,0 +1,159 @@
+// +build !consulent
+
+package agent
+
+import (
+	"fmt"
+	"net/http"
+	"net/http/httptest"
+	"sync/atomic"
+	"testing"
+
+	"github.com/hashicorp/consul/agent/structs"
+	"github.com/hashicorp/consul/testrpc"
+	"github.com/stretchr/testify/require"
+)
+
+func TestUIEndpoint_MetricsProxy_ACLDeny(t *testing.T) {
+	t.Parallel()
+
+	var (
+		lastHeadersSent atomic.Value
+		backendCalled   atomic.Value
+	)
+	backend := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		backendCalled.Store(true)
+		lastHeadersSent.Store(r.Header)
+		if r.URL.Path == "/some/prefix/ok" {
+			w.Write([]byte("OK"))
+			return
+		}
+		http.Error(w, "not found on backend", http.StatusNotFound)
+	}))
+	defer backend.Close()
+
+	backendURL := backend.URL + "/some/prefix"
+
+	a := NewTestAgent(t, TestACLConfig()+fmt.Sprintf(`
+		ui_config {
+			enabled = true
+			metrics_proxy {
+				base_url = %q
+			}
+		}
+		http_config {
+			response_headers {
+				"Access-Control-Allow-Origin" = "*"
+			}
+		}
+	`, backendURL))
+	defer a.Shutdown()
+
+	h := a.srv.handler(true)
+
+	testrpc.WaitForLeader(t, a.RPC, "dc1")
+
+	const endpointPath = "/v1/internal/ui/metrics-proxy"
+
+	// create some ACL things
+	for name, rules := range map[string]string{
+		"one-service":  `service "foo" { policy = "read" }`,
+		"all-services": `service_prefix "" { policy = "read" }`,
+		"one-node":     `node "bar" { policy = "read" }`,
+		"all-nodes":    `node_prefix "" { policy = "read" }`,
+	} {
+		req := structs.ACLPolicySetRequest{
+			Policy: structs.ACLPolicy{
+				Name:  name,
+				Rules: rules,
+			},
+			Datacenter:   "dc1",
+			WriteRequest: structs.WriteRequest{Token: "root"},
+		}
+		var policy structs.ACLPolicy
+		require.NoError(t, a.RPC("ACL.PolicySet", &req, &policy))
+	}
+
+	makeToken := func(t *testing.T, policyNames []string) string {
+		req := structs.ACLTokenSetRequest{
+			ACLToken:     structs.ACLToken{},
+			Datacenter:   "dc1",
+			WriteRequest: structs.WriteRequest{Token: "root"},
+		}
+		for _, name := range policyNames {
+			req.ACLToken.Policies = append(req.ACLToken.Policies, structs.ACLTokenPolicyLink{Name: name})
+		}
+		require.Len(t, req.ACLToken.Policies, len(policyNames))
+
+		var token structs.ACLToken
+		require.NoError(t, a.RPC("ACL.TokenSet", &req, &token))
+		return token.SecretID
+	}
+
+	type testcase struct {
+		name     string
+		token    string
+		policies []string
+		expect   int
+	}
+
+	for _, tc := range []testcase{
+		{name: "no token", token: "", expect: http.StatusForbidden},
+		{name: "root token", token: "root", expect: http.StatusOK},
+		//
+		{name: "one node", policies: []string{"one-node"}, expect: http.StatusForbidden},
+		{name: "all nodes", policies: []string{"all-nodes"}, expect: http.StatusForbidden},
+		//
+		{name: "one service", policies: []string{"one-service"}, expect: http.StatusForbidden},
+		{name: "all services", policies: []string{"all-services"}, expect: http.StatusForbidden},
+		//
+		{name: "one service one node", policies: []string{"one-service", "one-node"}, expect: http.StatusForbidden},
+		{name: "all services one node", policies: []string{"all-services", "one-node"}, expect: http.StatusForbidden},
+		//
+		{name: "one service all nodes", policies: []string{"one-service", "one-node"}, expect: http.StatusForbidden},
+		{name: "all services all nodes", policies: []string{"all-services", "all-nodes"}, expect: http.StatusOK},
+	} {
+		tc := tc
+		t.Run(tc.name, func(t *testing.T) {
+			if tc.token == "" {
+				tc.token = makeToken(t, tc.policies)
+			}
+
+			t.Run("via query param should not work", func(t *testing.T) {
+				req := httptest.NewRequest("GET", endpointPath+"/ok?token="+tc.token, nil)
+				rec := httptest.NewRecorder()
+				backendCalled.Store(false)
+				h.ServeHTTP(rec, req)
+				require.Equal(t, http.StatusForbidden, rec.Code)
+
+				require.False(t, backendCalled.Load().(bool))
+			})
+
+			for _, headerName := range []string{"x-consul-token", "authorization"} {
+				headerVal := tc.token
+				if headerName == "authorization" {
+					headerVal = "bearer " + tc.token
+				}
+
+				t.Run("via header "+headerName, func(t *testing.T) {
+					req := httptest.NewRequest("GET", endpointPath+"/ok", nil)
+					req.Header.Set(headerName, headerVal)
+					rec := httptest.NewRecorder()
+					backendCalled.Store(false)
+					h.ServeHTTP(rec, req)
+					require.Equal(t, tc.expect, rec.Code)
+
+					headersSent, _ := lastHeadersSent.Load().(http.Header)
+					if tc.expect == http.StatusOK {
+						require.True(t, backendCalled.Load().(bool))
+						// Ensure we didn't accidentally ship our consul token to the proxy.
+						require.Empty(t, headersSent.Get("X-Consul-Token"))
+						require.Empty(t, headersSent.Get("Authorization"))
+					} else {
+						require.False(t, backendCalled.Load().(bool))
+					}
+				})
+			}
+		})
+	}
+}