Update.

Signed-off-by: xuezhaojun <zxue@redhat.com>

Author: xuezhaojun

pkg/cloudevents/server/grpc/authz/kube/sar.go

@@ -62,47 +62,45 @@ func NewSARAuthorizer(kubeClient kubernetes.Interface) *SARAuthorizer {
 	}
 }
 
-func (s *SARAuthorizer) AuthorizeRequest(ctx context.Context, req any) error {
+func (s *SARAuthorizer) AuthorizeRequest(ctx context.Context, req any) (authz.Decision, string, error) {
 	pReq, ok := req.(*pbv1.PublishRequest)
 	if !ok {
-		return fmt.Errorf("unsupported request type %T", req)
+		return authz.DecisionDeny, "unsupported request type", fmt.Errorf("unsupported request type %T", req)
 	}
 
 	eventsType, err := types.ParseCloudEventsType(pReq.Event.Type)
 	if err != nil {
-		return err
+		return authz.DecisionDeny, "invalid event type", err
 	}
 
 	// the event of grpc publish request is the original cloudevent data, we need a `ce-` prefix
 	// to get the event attribute
 	clusterAttr, ok := pReq.Event.Attributes[fmt.Sprintf("ce-%s", types.ExtensionClusterName)]
 	if !ok {
-		return fmt.Errorf("missing ce-clustername in event attributes, %v", pReq.Event.Attributes)
+		return authz.DecisionDeny, "missing cluster name", fmt.Errorf("missing ce-clustername in event attributes, %v", pReq.Event.Attributes)
 	}
 
-	if err := s.authorize(ctx, clusterAttr.GetCeString(), *eventsType); err != nil {
-		return err
-	}
-	return nil
+	decision, reason, err := s.authorize(ctx, clusterAttr.GetCeString(), *eventsType)
+	return decision, reason, err
 }
 
-func (s *SARAuthorizer) AuthorizeStream(ctx context.Context, ss grpc.ServerStream, info *grpc.StreamServerInfo) (grpc.ServerStream, error) {
+func (s *SARAuthorizer) AuthorizeStream(ctx context.Context, ss grpc.ServerStream, info *grpc.StreamServerInfo) (authz.Decision, string, grpc.ServerStream, error) {
 	if info.IsClientStream {
-		return ss, nil
+		return authz.DecisionAllow, "client stream allowed", ss, nil
 	}
 
 	if info.FullMethod != pbv1.CloudEventService_Subscribe_FullMethodName {
-		return nil, fmt.Errorf("unsupported service full method %s for SARAuthorizer", info.FullMethod)
+		return authz.DecisionDeny, "unsupported method", nil, fmt.Errorf("unsupported service full method %s for SARAuthorizer", info.FullMethod)
 	}
 
 	var req pbv1.SubscriptionRequest
 	if err := ss.RecvMsg(&req); err != nil {
-		return nil, err
+		return authz.DecisionDeny, "failed to receive message", nil, err
 	}
 
 	eventDataType, err := types.ParseCloudEventsDataType(req.DataType)
 	if err != nil {
-		return nil, err
+		return authz.DecisionDeny, "invalid data type", nil, err
 	}
 
 	eventsType := types.CloudEventsType{
@@ -111,34 +109,36 @@ func (s *SARAuthorizer) AuthorizeStream(ctx context.Context, ss grpc.ServerStrea
 		Action:              types.WatchRequestAction,
 	}
 
-	if err := s.authorize(ss.Context(), req.ClusterName, eventsType); err != nil {
-		return nil, err
+	decision, reason, err := s.authorize(ss.Context(), req.ClusterName, eventsType)
+	if err != nil {
+		return decision, reason, nil, err
 	}
 
-	return &wrappedAuthorizedStream{ServerStream: ss, authorizedReq: &req}, nil
+	return decision, reason, &wrappedAuthorizedStream{ServerStream: ss, authorizedReq: &req}, nil
 }
 
-func (s *SARAuthorizer) authorize(ctx context.Context, cluster string, eventsType types.CloudEventsType) error {
+func (s *SARAuthorizer) authorize(ctx context.Context, cluster string, eventsType types.CloudEventsType) (authz.Decision, string, error) {
 	user, groups, err := userInfo(ctx)
 	if err != nil {
-		return err
+		return authz.DecisionDeny, "failed to extract user info", err
 	}
 
 	sar, err := toSubjectAccessReview(cluster, user, groups, eventsType)
 	if err != nil {
-		return err
+		return authz.DecisionDeny, "failed to create SAR", err
 	}
 
 	created, err := s.kubeClient.AuthorizationV1().SubjectAccessReviews().Create(
 		ctx, sar, metav1.CreateOptions{})
 	if err != nil {
-		return err
+		return authz.DecisionDeny, "SAR request failed", err
 	}
 	if !created.Status.Allowed {
-		return fmt.Errorf("the event %s is not allowed, (cluster=%s, sar=%v, reason=%v)",
+		reason := fmt.Sprintf("access denied for event %s (cluster=%s, reason=%s)", eventsType, cluster, created.Status.Reason)
+		return authz.DecisionDeny, reason, fmt.Errorf("the event %s is not allowed, (cluster=%s, sar=%v, reason=%v)",
 			eventsType, cluster, sar.Spec, created.Status)
 	}
-	return nil
+	return authz.DecisionAllow, "access granted by SAR", nil
 }
 
 func userInfo(ctx context.Context) (user string, groups []string, err error) {

pkg/cloudevents/server/grpc/authz/kube/sar_test.go

@@ -18,6 +18,7 @@ import (
 	"open-cluster-management.io/sdk-go/pkg/cloudevents/clients/work/payload"
 	"open-cluster-management.io/sdk-go/pkg/cloudevents/generic/types"
 	"open-cluster-management.io/sdk-go/pkg/server/grpc/authn"
+	"open-cluster-management.io/sdk-go/pkg/server/grpc/authz"
 )
 
 func TestSARAuthorize(t *testing.T) {
@@ -197,13 +198,19 @@ func TestSARAuthorize(t *testing.T) {
 
 			auth := NewSARAuthorizer(client)
 
-			err := auth.authorize(tc.userCtx(), tc.cluster, tc.eventsType)
+			decision, reason, err := auth.authorize(tc.userCtx(), tc.cluster, tc.eventsType)
 			if tc.expectErr && err == nil {
 				t.Errorf("expected error, got nil")
 			}
 			if !tc.expectErr && err != nil {
 				t.Errorf("unexpected error: %v", err)
 			}
+			if !tc.expectErr && decision != authz.DecisionAllow {
+				t.Errorf("expected DecisionAllow, got %v, reason: %s", decision, reason)
+			}
+			if tc.expectErr && decision != authz.DecisionDeny {
+				t.Errorf("expected DecisionDeny, got %v, reason: %s", decision, reason)
+			}
 		})
 	}
 }

pkg/server/grpc/authz/interface.go

@@ -6,23 +6,38 @@ import (
 	"google.golang.org/grpc"
 )
 
+// Decision represents the result of an authorization request.
+type Decision int
+
+const (
+	// DecisionDeny means the request is explicitly denied.
+	DecisionDeny Decision = iota
+	// DecisionAllow means the request is explicitly allowed.
+	DecisionAllow
+	// DecisionNoOpinion means the authorizer has no opinion on the request.
+	// This allows the authorization chain to continue to the next authorizer.
+	DecisionNoOpinion
+)
+
 // UnaryAuthorizer defines the interface for authorizing unary gRPC requests.
 // Implementations should validate whether the authenticated user has permission
 // to perform the requested operation based on the context and request.
 type UnaryAuthorizer interface {
 	// AuthorizeRequest validates whether the user in the context is authorized
-	// to perform the operation represented by the request. Returns an error
-	// if authorization fails, or nil if the request is authorized.
-	AuthorizeRequest(ctx context.Context, req any) error
+	// to perform the operation represented by the request. Returns a Decision
+	// indicating the authorization result, a reason string explaining the decision,
+	// and an error if the authorization process itself fails.
+	AuthorizeRequest(ctx context.Context, req any) (Decision, string, error)
 }
 
 // StreamAuthorizer defines the interface for authorizing streaming gRPC requests.
 // Implementations should validate whether the authenticated user has permission
 // to establish and maintain the streaming connection.
 type StreamAuthorizer interface {
 	// AuthorizeStream validates whether the user in the context is authorized
-	// to establish the streaming connection. Returns a potentially wrapped ServerStream
-	// and an error if authorization fails, or the original/wrapped stream and nil if authorized.
+	// to establish the streaming connection. Returns a Decision indicating the
+	// authorization result, a reason string explaining the decision, a potentially
+	// wrapped ServerStream, and an error if the authorization process itself fails.
 	// The returned ServerStream can be used to intercept and authorize individual messages.
-	AuthorizeStream(ctx context.Context, ss grpc.ServerStream, info *grpc.StreamServerInfo) (grpc.ServerStream, error)
+	AuthorizeStream(ctx context.Context, ss grpc.ServerStream, info *grpc.StreamServerInfo) (Decision, string, grpc.ServerStream, error)
 }

pkg/server/grpc/server.go

@@ -173,11 +173,28 @@ func newAuthzUnaryInterceptor(authorizers ...authz.UnaryAuthorizer) grpc.UnarySe
 		handler grpc.UnaryHandler,
 	) (interface{}, error) {
 		var errs []error
+		var reasons []string
 		for _, authorizer := range authorizers {
-			if err := authorizer.AuthorizeRequest(ctx, req); err == nil {
+			decision, reason, err := authorizer.AuthorizeRequest(ctx, req)
+			switch decision {
+			case authz.DecisionAllow:
 				return handler(ctx, req)
-			} else {
-				errs = append(errs, err)
+			case authz.DecisionDeny:
+				if err != nil {
+					errs = append(errs, err)
+				}
+				if reason != "" {
+					reasons = append(reasons, reason)
+				}
+				return nil, fmt.Errorf("access denied: %s", reason)
+			case authz.DecisionNoOpinion:
+				if err != nil {
+					errs = append(errs, err)
+				}
+				if reason != "" {
+					reasons = append(reasons, reason)
+				}
+				// Continue to next authorizer
 			}
 		}
 
@@ -245,12 +262,29 @@ func newAuthzStreamInterceptor(authorizers []authz.StreamAuthorizer) grpc.Stream
 		handler grpc.StreamHandler,
 	) error {
 		var errs []error
+		var reasons []string
 
 		for _, authorizer := range authorizers {
-			if authorizedStream, err := authorizer.AuthorizeStream(ss.Context(), ss, info); err == nil {
+			decision, reason, authorizedStream, err := authorizer.AuthorizeStream(ss.Context(), ss, info)
+			switch decision {
+			case authz.DecisionAllow:
 				return handler(srv, authorizedStream)
-			} else {
-				errs = append(errs, err)
+			case authz.DecisionDeny:
+				if err != nil {
+					errs = append(errs, err)
+				}
+				if reason != "" {
+					reasons = append(reasons, reason)
+				}
+				return fmt.Errorf("access denied: %s", reason)
+			case authz.DecisionNoOpinion:
+				if err != nil {
+					errs = append(errs, err)
+				}
+				if reason != "" {
+					reasons = append(reasons, reason)
+				}
+				// Continue to next authorizer
 			}
 		}