[7.x] Separate authentication from authorization (backport #5545) (#5612

)

* Separate authentication from authorization (#5545)

* Separate authentication from authorization

Overhaul beater/authorization (now beater/auth) to provide
two primary interfaces: Authenticator and Authorizer.

Authenticator is responsible for authenticating a client
(typically but not always an agent), returning
authentication details (e.g. username for API Key) and an
Authorizer for authorizing specific actions.

The auth middleware and interceptor authenticate and store
the returned Authorizer in request context, so it can be
used by specific method handlers. Authorize accepts an
auth.Action and auth.Resource to authorize.

HTTP and gRPC status codes are changed to follow the
semantics more correctly. Failure to authenticate leads to
401 (HTTP) and Unauthenticated (gRPC). Failure to authorize
leads to 403 (HTTP) and PermissionDenied (gRPC).

* tests/system: adjust test for API Key handling

We no longer cache API Key credentials which are
malformatted, i.e. where they're not base64-encoded
id:key. Update the test to match the new behaviour.

* tests/system: adjust to new RUM auth behaviour

We now behave the same for RUM as for backend agents
when auth is not configured. That is, no rate limiting
and no agent config restriction. These only apply when
auth is configured, so update the tests to configure
auth.

* Fix comments

(cherry picked from commit b742bbf)

# Conflicts:
#	changelogs/head.asciidoc

* Delete head.asciidoc

Co-authored-by: Andrew Wilkins <[email protected]>

beater/api/asset/sourcemap/handler.go

@@ -28,6 +28,7 @@ import (
 
 	"github.com/elastic/beats/v7/libbeat/monitoring"
 
+	"github.com/elastic/apm-server/beater/auth"
 	"github.com/elastic/apm-server/beater/request"
 	"github.com/elastic/apm-server/model"
 	"github.com/elastic/apm-server/publish"
@@ -53,6 +54,20 @@ func Handler(report publish.Reporter) request.Handler {
 			c.Write()
 			return
 		}
+
+		if err := auth.Authorize(c.Request.Context(), auth.ActionSourcemapUpload, auth.Resource{}); err != nil {
+			if errors.Is(err, auth.ErrUnauthorized) {
+				id := request.IDResponseErrorsForbidden
+				status := request.MapResultIDToStatus[id]
+				c.Result.Set(id, status.Code, err.Error(), nil, nil)
+			} else {
+				c.Result.SetDefault(request.IDResponseErrorsServiceUnavailable)
+				c.Result.Err = err
+			}
+			c.Write()
+			return
+		}
+
 		var smap model.Sourcemap
 		decodingCount.Inc()
 		if err := decode(c.Request, &smap); err != nil {

beater/api/asset/sourcemap/handler_test.go

@@ -35,6 +35,7 @@ import (
 	"github.com/stretchr/testify/require"
 
 	"github.com/elastic/apm-server/approvaltest"
+	"github.com/elastic/apm-server/beater/auth"
 	"github.com/elastic/apm-server/beater/beatertest"
 	"github.com/elastic/apm-server/beater/request"
 	"github.com/elastic/apm-server/publish"
@@ -89,6 +90,20 @@ func TestAssetHandler(t *testing.T) {
 			},
 			code: http.StatusAccepted,
 		},
+		"unauthorized": {
+			authorizer: func(context.Context, auth.Action, auth.Resource) error {
+				return auth.ErrUnauthorized
+			},
+			code: http.StatusForbidden,
+			body: beatertest.ResultErrWrap("unauthorized"),
+		},
+		"auth_unavailable": {
+			authorizer: func(context.Context, auth.Action, auth.Resource) error {
+				return errors.New("boom")
+			},
+			code: http.StatusServiceUnavailable,
+			body: beatertest.ResultErrWrap("service unavailable"),
+		},
 	}
 	for name, tc := range testcases {
 		t.Run(name, func(t *testing.T) {
@@ -106,6 +121,7 @@ type testcaseT struct {
 	sourcemapInput string
 	contentType    string
 	reporter       func(ctx context.Context, p publish.PendingReq) error
+	authorizer     authorizerFunc
 
 	missingSourcemap, missingServiceName, missingServiceVersion, missingBundleFilepath bool
 
@@ -117,6 +133,11 @@ func (tc *testcaseT) setup() error {
 	if tc.w == nil {
 		tc.w = httptest.NewRecorder()
 	}
+	if tc.authorizer == nil {
+		tc.authorizer = func(ctx context.Context, action auth.Action, resource auth.Resource) error {
+			return nil
+		}
+	}
 	if tc.r == nil {
 		buf := bytes.Buffer{}
 		w := multipart.NewWriter(&buf)
@@ -149,6 +170,7 @@ func (tc *testcaseT) setup() error {
 			tc.contentType = w.FormDataContentType()
 		}
 		tc.r.Header.Set("Content-Type", tc.contentType)
+		tc.r = tc.r.WithContext(auth.ContextWithAuthorizer(tc.r.Context(), tc.authorizer))
 	}
 
 	if tc.reporter == nil {
@@ -160,3 +182,9 @@ func (tc *testcaseT) setup() error {
 	h(c)
 	return nil
 }
+
+type authorizerFunc func(context.Context, auth.Action, auth.Resource) error
+
+func (f authorizerFunc) Authorize(ctx context.Context, action auth.Action, resource auth.Resource) error {
+	return f(ctx, action, resource)
+}

beater/api/asset/sourcemap/test_approved/integration/TestSourcemapHandler_AuthorizationMiddleware/Unauthorized.approved.json

@@ -1,3 +1,3 @@
 {
-    "error": "missing or improperly formatted Authorization header: expected 'Authorization: Bearer secret_token' or 'Authorization: ApiKey base64(API key ID:API key)'"
+    "error": "authentication failed: missing or improperly formatted Authorization header: expected 'Authorization: Bearer secret_token' or 'Authorization: ApiKey base64(API key ID:API key)'"
 }

beater/api/config/agent/handler.go

@@ -30,7 +30,7 @@ import (
 	"github.com/elastic/beats/v7/libbeat/monitoring"
 
 	"github.com/elastic/apm-server/agentcfg"
-	"github.com/elastic/apm-server/beater/authorization"
+	"github.com/elastic/apm-server/beater/auth"
 	"github.com/elastic/apm-server/beater/config"
 	"github.com/elastic/apm-server/beater/headers"
 	"github.com/elastic/apm-server/beater/request"
@@ -100,25 +100,21 @@ func (h *handler) Handle(c *request.Context) {
 	// Only service, and not agent, is known for config queries.
 	// For anonymous/untrusted agents, we filter the results using
 	// query.InsecureAgents below.
-	authResource := authorization.Resource{ServiceName: query.Service.Name}
-	authResult, err := authorization.AuthorizedFor(c.Request.Context(), authResource)
-	if err != nil {
-		c.Result.SetDefault(request.IDResponseErrorsServiceUnavailable)
-		c.Result.Err = err
-		c.Write()
-		return
-	}
-	if !authResult.Authorized {
-		id := request.IDResponseErrorsUnauthorized
-		status := request.MapResultIDToStatus[id]
-		if authResult.Reason != "" {
-			status.Keyword = authResult.Reason
+	authResource := auth.Resource{ServiceName: query.Service.Name}
+	if err := auth.Authorize(c.Request.Context(), auth.ActionAgentConfig, authResource); err != nil {
+		if errors.Is(err, auth.ErrUnauthorized) {
+			id := request.IDResponseErrorsForbidden
+			status := request.MapResultIDToStatus[id]
+			c.Result.Set(id, status.Code, err.Error(), nil, nil)
+		} else {
+			c.Result.SetDefault(request.IDResponseErrorsServiceUnavailable)
+			c.Result.Err = err
 		}
-		c.Result.Set(id, status.Code, status.Keyword, nil, nil)
 		c.Write()
 		return
 	}
-	if authResult.Anonymous {
+	if c.Authentication.Method == "" {
+		// Unauthenticated client, restrict results.
 		query.InsecureAgents = h.allowAnonymousAgents
 	}
 
@@ -238,7 +234,7 @@ func extractQueryError(c *request.Context, err error) {
 }
 
 func authErrMsg(c *request.Context, fullMsg, shortMsg string) string {
-	if !c.AuthResult.Anonymous {
+	if c.Authentication.Method != "" {
 		return fullMsg
 	}
 	return shortMsg

beater/api/config/agent/handler_test.go

@@ -37,7 +37,7 @@ import (
 	libkibana "github.com/elastic/beats/v7/libbeat/kibana"
 
 	"github.com/elastic/apm-server/agentcfg"
-	"github.com/elastic/apm-server/beater/authorization"
+	"github.com/elastic/apm-server/beater/auth"
 	"github.com/elastic/apm-server/beater/config"
 	"github.com/elastic/apm-server/beater/headers"
 	"github.com/elastic/apm-server/beater/request"
@@ -200,29 +200,32 @@ func TestAgentConfigHandlerAnonymousAccess(t *testing.T) {
 	for _, tc := range []struct {
 		anonymous    bool
 		response     string
-		authResource *authorization.Resource
+		authResource *auth.Resource
 	}{{
 		anonymous:    false,
 		response:     `{"error":"APM Server is not authorized to query Kibana. Please configure apm-server.kibana.username and apm-server.kibana.password, and ensure the user has the necessary privileges."}`,
-		authResource: &authorization.Resource{ServiceName: "opbeans"},
+		authResource: &auth.Resource{ServiceName: "opbeans"},
 	}, {
 		anonymous:    true,
 		response:     `{"error":"Unauthorized"}`,
-		authResource: &authorization.Resource{ServiceName: "opbeans"},
+		authResource: &auth.Resource{ServiceName: "opbeans"},
 	}} {
 		r := httptest.NewRequest(http.MethodGet, target(map[string]string{"service.name": "opbeans"}), nil)
 		c, w := newRequestContext(r)
-		c.AuthResult.Authorized = true
-		c.AuthResult.Anonymous = tc.anonymous
 
-		var requestedResource *authorization.Resource
-		c.Request = withAuthorization(c.Request,
-			authorizedForFunc(func(ctx context.Context, resource authorization.Resource) (authorization.Result, error) {
+		c.Authentication.Method = "none"
+		if tc.anonymous {
+			c.Authentication.Method = ""
+		}
+
+		var requestedResource *auth.Resource
+		c.Request = withAuthorizer(c.Request,
+			authorizerFunc(func(ctx context.Context, action auth.Action, resource auth.Resource) error {
 				if requestedResource != nil {
-					panic("expected only one AuthorizedFor request")
+					panic("expected only one Authorize request")
 				}
 				requestedResource = &resource
-				return c.AuthResult, nil
+				return nil
 			}),
 		)
 		h(c)
@@ -238,19 +241,18 @@ func TestAgentConfigHandlerAuthorizedForService(t *testing.T) {
 
 	r := httptest.NewRequest(http.MethodGet, target(map[string]string{"service.name": "opbeans"}), nil)
 	ctx, w := newRequestContext(r)
-	ctx.AuthResult.Authorized = true
 
-	var queriedResource authorization.Resource
-	ctx.Request = withAuthorization(ctx.Request,
-		authorizedForFunc(func(ctx context.Context, resource authorization.Resource) (authorization.Result, error) {
+	var queriedResource auth.Resource
+	ctx.Request = withAuthorizer(ctx.Request,
+		authorizerFunc(func(ctx context.Context, action auth.Action, resource auth.Resource) error {
 			queriedResource = resource
-			return authorization.Result{Authorized: false}, nil
+			return auth.ErrUnauthorized
 		}),
 	)
 	h(ctx)
 
-	assert.Equal(t, http.StatusUnauthorized, w.Code, w.Body.String())
-	assert.Equal(t, authorization.Resource{ServiceName: "opbeans"}, queriedResource)
+	assert.Equal(t, http.StatusForbidden, w.Code, w.Body.String())
+	assert.Equal(t, auth.Resource{ServiceName: "opbeans"}, queriedResource)
 }
 
 func TestAgentConfigHandler_NoKibanaClient(t *testing.T) {
@@ -313,6 +315,7 @@ func TestAgentConfigRum(t *testing.T) {
 	r := httptest.NewRequest(http.MethodPost, "/rum", convert.ToReader(m{
 		"service": m{"name": "opbeans"}}))
 	ctx, w := newRequestContext(r)
+	ctx.Authentication.Method = "" // unauthenticated
 	h(ctx)
 	var actual map[string]string
 	json.Unmarshal(w.Body.Bytes(), &actual)
@@ -334,9 +337,9 @@ func TestAgentConfigNotRum(t *testing.T) {
 	r := httptest.NewRequest(http.MethodPost, "/backend", convert.ToReader(m{
 		"service": m{"name": "opbeans"}}))
 	ctx, w := newRequestContext(r)
-	ctx.Request = withAuthorization(ctx.Request,
-		authorizedForFunc(func(context.Context, authorization.Resource) (authorization.Result, error) {
-			return authorization.Result{Authorized: true, Anonymous: false}, nil
+	ctx.Request = withAuthorizer(ctx.Request,
+		authorizerFunc(func(context.Context, auth.Action, auth.Resource) error {
+			return nil
 		}),
 	)
 	h(ctx)
@@ -351,6 +354,7 @@ func TestAgentConfigNoLeak(t *testing.T) {
 	r := httptest.NewRequest(http.MethodPost, "/rum", convert.ToReader(m{
 		"service": m{"name": "opbeans"}}))
 	ctx, w := newRequestContext(r)
+	ctx.Authentication.Method = "" // unauthenticated
 	h(ctx)
 	var actual map[string]string
 	json.Unmarshal(w.Body.Bytes(), &actual)
@@ -412,9 +416,9 @@ func TestAgentConfigTraceContext(t *testing.T) {
 
 func sendRequest(h request.Handler, r *http.Request) *httptest.ResponseRecorder {
 	ctx, recorder := newRequestContext(r)
-	ctx.Request = withAuthorization(ctx.Request,
-		authorizedForFunc(func(context.Context, authorization.Resource) (authorization.Result, error) {
-			return authorization.Result{Authorized: true}, nil
+	ctx.Request = withAuthorizer(ctx.Request,
+		authorizerFunc(func(context.Context, auth.Action, auth.Resource) error {
+			return nil
 		}),
 	)
 	h(ctx)
@@ -425,7 +429,8 @@ func newRequestContext(r *http.Request) (*request.Context, *httptest.ResponseRec
 	w := httptest.NewRecorder()
 	ctx := request.NewContext()
 	ctx.Reset(w, r)
-	ctx.Request = withAnonymousAuthorization(ctx.Request)
+	ctx.Request = withAnonymousAuthorizer(ctx.Request)
+	ctx.Authentication.Method = auth.MethodNone
 	return ctx, w
 }
 
@@ -458,18 +463,18 @@ func (c *recordingKibanaClient) Send(ctx context.Context, method string, path st
 	return c.Client.Send(ctx, method, path, params, header, body)
 }
 
-func withAnonymousAuthorization(req *http.Request) *http.Request {
-	return withAuthorization(req, authorizedForFunc(func(context.Context, authorization.Resource) (authorization.Result, error) {
-		return authorization.Result{Authorized: true, Anonymous: true}, nil
+func withAnonymousAuthorizer(req *http.Request) *http.Request {
+	return withAuthorizer(req, authorizerFunc(func(context.Context, auth.Action, auth.Resource) error {
+		return nil
 	}))
 }
 
-func withAuthorization(req *http.Request, auth authorization.Authorization) *http.Request {
-	return req.WithContext(authorization.ContextWithAuthorization(req.Context(), auth))
+func withAuthorizer(req *http.Request, authz auth.Authorizer) *http.Request {
+	return req.WithContext(auth.ContextWithAuthorizer(req.Context(), authz))
 }
 
-type authorizedForFunc func(context.Context, authorization.Resource) (authorization.Result, error)
+type authorizerFunc func(context.Context, auth.Action, auth.Resource) error
 
-func (f authorizedForFunc) AuthorizedFor(ctx context.Context, resource authorization.Resource) (authorization.Result, error) {
-	return f(ctx, resource)
+func (f authorizerFunc) Authorize(ctx context.Context, action auth.Action, resource auth.Resource) error {
+	return f(ctx, action, resource)
 }

beater/api/config/agent/test_approved/integration/TestConfigAgentHandler_AuthorizationMiddleware/Unauthorized.approved.json

@@ -1,3 +1,3 @@
 {
-    "error": "missing or improperly formatted Authorization header: expected 'Authorization: Bearer secret_token' or 'Authorization: ApiKey base64(API key ID:API key)'"
+    "error": "authentication failed: missing or improperly formatted Authorization header: expected 'Authorization: Bearer secret_token' or 'Authorization: ApiKey base64(API key ID:API key)'"
 }

beater/api/intake/handler.go

@@ -27,6 +27,7 @@ import (
 
 	"github.com/elastic/beats/v7/libbeat/monitoring"
 
+	"github.com/elastic/apm-server/beater/auth"
 	"github.com/elastic/apm-server/beater/headers"
 	"github.com/elastic/apm-server/beater/ratelimit"
 	"github.com/elastic/apm-server/beater/request"
@@ -154,6 +155,8 @@ func writeStreamResult(c *request.Context, sr *stream.Result) {
 					errID = request.IDResponseErrorsValidate
 				case errors.Is(err, ratelimit.ErrRateLimitExceeded):
 					errID = request.IDResponseErrorsRateLimit
+				case errors.Is(err, auth.ErrUnauthorized):
+					errID = request.IDResponseErrorsForbidden
 				}
 			}
 			jsonResult.Errors[i] = jsonError{Message: err.Error()}

beater/api/intake/test_approved/integration/backend/TestIntakeBackendHandler_AuthorizationMiddleware/Unauthorized.approved.json

@@ -1,3 +1,3 @@
 {
-    "error": "missing or improperly formatted Authorization header: expected 'Authorization: Bearer secret_token' or 'Authorization: ApiKey base64(API key ID:API key)'"
+    "error": "authentication failed: missing or improperly formatted Authorization header: expected 'Authorization: Bearer secret_token' or 'Authorization: ApiKey base64(API key ID:API key)'"
 }