Merge pull request #2628 from s-urbaniak/issue-2627

🐛 pkg/server: ensure that home workspace handler gets authz with audit …

pkg/server/config.go

@@ -331,7 +331,16 @@ func NewConfig(opts *kcpserveroptions.CompletedOptions) (*Config, error) {
 		apiHandler = WithRequestIdentity(apiHandler)
 		apiHandler = authorization.WithDeepSubjectAccessReview(apiHandler)
 
+		// The following ensures that only the default main api handler chain executes authorizers which log audit messages.
+		// All other invocations of the same authorizer chain still work but do not produce audit log entries.
+		// This compromises audit log size and information overflow vs. having audit reasons for the main api handler only.
+		// First, remember authorizer chain with audit logging disabled.
+		authorizerWithoutAudit := genericConfig.Authorization.Authorizer
+		// configure audit logging enabled authorizer chain and build the apiHandler using this configuration.
+		genericConfig.Authorization.Authorizer = authorization.EnableAuditLogging(genericConfig.Authorization.Authorizer)
 		apiHandler = genericapiserver.DefaultBuildHandlerChainFromAuthz(apiHandler, genericConfig)
+		// reset authorizer chain with audit logging disabled.
+		genericConfig.Authorization.Authorizer = authorizerWithoutAudit
 
 		if opts.HomeWorkspaces.Enabled {
 			apiHandler, err = WithHomeWorkspaces(
@@ -352,10 +361,7 @@ func NewConfig(opts *kcpserveroptions.CompletedOptions) (*Config, error) {
 			}
 		}
 
-		authorizerWithoutAudit := genericConfig.Authorization.Authorizer
-		genericConfig.Authorization.Authorizer = authorization.EnableAuditLogging(genericConfig.Authorization.Authorizer)
 		apiHandler = genericapiserver.DefaultBuildHandlerChainBeforeAuthz(apiHandler, genericConfig)
-		genericConfig.Authorization.Authorizer = authorizerWithoutAudit
 
 		// this will be replaced in DefaultBuildHandlerChain. So at worst we get twice as many warning.
 		// But this is not harmful as the kcp warnings are not many.

test/e2e/audit/audit_log_test.go

@@ -62,12 +62,29 @@ func TestAuditLogs(t *testing.T) {
 	require.NoError(t, err, "Error reading auditfile")
 
 	lines := strings.Split(string(data), "\n")
-	var auditEvent audit.Event
-	err = json.Unmarshal([]byte(lines[0]), &auditEvent)
+	var requestAuditEvent, responseAuditEvent audit.Event
+	err = json.Unmarshal([]byte(lines[0]), &requestAuditEvent)
+	require.NoError(t, err, "Error parsing JSON data")
+	err = json.Unmarshal([]byte(lines[1]), &responseAuditEvent)
 	require.NoError(t, err, "Error parsing JSON data")
 
 	workspaceNameSent := ws.Spec.Cluster
-	workspaceNameRecvd := auditEvent.Annotations["tenancy.kcp.io/workspace"]
-
-	require.Equal(t, workspaceNameSent, workspaceNameRecvd)
+	require.Equal(t, workspaceNameSent, requestAuditEvent.Annotations["tenancy.kcp.io/workspace"])
+	require.Equal(t, workspaceNameSent, responseAuditEvent.Annotations["tenancy.kcp.io/workspace"])
+
+	for _, annotation := range []string{
+		"authorization.k8s.io",
+		"bootstrap.authorization.kcp.io",
+		"content.authorization.kcp.io",
+		"maxpermissionpolicy.authorization.kcp.io",
+		"requiredgroups.authorization.kcp.io",
+		"systemcrd.authorization.kcp.io",
+	} {
+		if _, ok := responseAuditEvent.Annotations[annotation+"/decision"]; !ok {
+			t.Errorf("expected annotation %v/decision but got none", annotation)
+		}
+		if _, ok := responseAuditEvent.Annotations[annotation+"/reason"]; !ok {
+			t.Errorf("expected annotation %v/reason but got none", annotation)
+		}
+	}
 }