Merge pull request #116 from LerianStudio/fix/grpc-product-arg-clarity

refactor(grpc): name product arg consistently in policy interceptors

Author: rodrigodh

auth/middleware/middleware.go

@@ -317,6 +317,16 @@ func (auth *AuthClient) checkAuthorization(ctx context.Context, product, resourc
 		}
 
 		userID, _ := claims["sub"].(string)
+		if userID == "" {
+			logErrorf(ctx, auth.Logger, "Missing sub claim in token")
+
+			err := errors.New("missing sub claim in token")
+
+			tracing.HandleSpanError(span, "Missing sub claim in token", err)
+
+			return false, http.StatusUnauthorized, err
+		}
+
 		sub = fmt.Sprintf("%s/%s", owner, userID)
 	}
 

auth/middleware/middlewareGRPC.go

@@ -26,10 +26,13 @@ type Policy struct {
 	Action   string
 }
 
-// PolicyConfig binds gRPC methods to Policies and optional subject resolution.
+// PolicyConfig binds gRPC methods to Policies and optional product resolution.
 // - MethodPolicies keyed by info.FullMethod ("/pkg.Service/Method").
 // - DefaultPolicy used when a method mapping is absent.
-// - SubResolver derives the subject base (e.g., editor scope). Return "" when not applicable.
+// - SubResolver derives the product identifier (e.g., "midaz") that is forwarded
+//   to checkAuthorization as its product argument. For M2M tokens it becomes the
+//   subject "admin/<product>-editor-role"; for normal-user tokens it is forwarded
+//   for product isolation. Return "" when not applicable.
 type PolicyConfig struct {
 	MethodPolicies map[string]Policy
 	DefaultPolicy  *Policy
@@ -39,11 +42,11 @@ type PolicyConfig struct {
 // NewGRPCAuthUnaryPolicy authorizes unary RPCs via per-method Policy.
 // Behavior:
 // - Resolves the Policy by info.FullMethod; falls back to DefaultPolicy when provided.
-// - Optionally derives the subject using cfg.SubResolver (e.g., editor roles). Empty subject is valid.
+// - Optionally derives the product using cfg.SubResolver (e.g., "midaz"). Empty product is valid.
 // - Rejects missing tokens with codes.Unauthenticated; misconfiguration returns codes.Internal.
 // Telemetry:
 // - Sets app.request.request_id.
-// - Sets app.request.payload with {sub, resource, action} per standard.
+// - Sets app.request.payload with {product, resource, action} per standard.
 func NewGRPCAuthUnaryPolicy(auth *AuthClient, cfg PolicyConfig) grpc.UnaryServerInterceptor {
 	return func(ctx context.Context, req any, info *grpc.UnaryServerInfo, handler grpc.UnaryHandler) (any, error) {
 		if auth == nil || !auth.Enabled || auth.Address == "" {
@@ -69,29 +72,31 @@ func NewGRPCAuthUnaryPolicy(auth *AuthClient, cfg PolicyConfig) grpc.UnaryServer
 			return nil, status.Error(codes.Internal, "internal configuration error")
 		}
 
-		var sub string
+		// product is the resolved product identifier passed as checkAuthorization's
+		// product argument (M2M subject base and normal-user isolation key).
+		var product string
 
 		if cfg.SubResolver != nil {
 			var err error
 
-			sub, err = cfg.SubResolver(ctx, info.FullMethod, req)
+			product, err = cfg.SubResolver(ctx, info.FullMethod, req)
 			if err != nil {
-				tracing.HandleSpanError(span, "failed to resolve subject", err)
+				tracing.HandleSpanError(span, "failed to resolve product", err)
 
 				return nil, status.Error(codes.Internal, "internal configuration error")
 			}
 		}
 
 		payload := map[string]string{
-			"sub":      sub,
+			"product":  product,
 			"resource": pol.Resource,
 			"action":   pol.Action,
 		}
 		if err := tracing.SetSpanAttributesFromValue(span, "app.request.payload", payload, nil); err != nil {
 			tracing.HandleSpanError(span, "failed to set span payload", err)
 		}
 
-		authorized, httpStatus, err := auth.checkAuthorization(ctx, sub, pol.Resource, pol.Action, token)
+		authorized, httpStatus, err := auth.checkAuthorization(ctx, product, pol.Resource, pol.Action, token)
 		if err != nil {
 			return nil, grpcErrorFromHTTP(httpStatus)
 		}
@@ -248,18 +253,20 @@ func NewGRPCAuthStreamPolicy(auth *AuthClient, cfg PolicyConfig) grpc.StreamServ
 			return status.Error(codes.Internal, "internal configuration error")
 		}
 
-		var sub string
+		// product is the resolved product identifier passed as checkAuthorization's
+		// product argument (M2M subject base and normal-user isolation key).
+		var product string
 
 		if cfg.SubResolver != nil {
 			var err error
 
-			sub, err = cfg.SubResolver(ctx, info.FullMethod, nil)
+			product, err = cfg.SubResolver(ctx, info.FullMethod, nil)
 			if err != nil {
 				return status.Error(codes.Internal, "internal configuration error")
 			}
 		}
 
-		authorized, httpStatus, err := auth.checkAuthorization(ctx, sub, pol.Resource, pol.Action, token)
+		authorized, httpStatus, err := auth.checkAuthorization(ctx, product, pol.Resource, pol.Action, token)
 		if err != nil {
 			return grpcErrorFromHTTP(httpStatus)
 		}

auth/middleware/middleware_test.go

@@ -195,6 +195,87 @@ func TestCheckAuthorization_MissingOwnerClaim(t *testing.T) {
 	assert.Contains(t, err.Error(), "missing owner claim")
 }
 
+func TestCheckAuthorization_MissingSubClaim(t *testing.T) {
+	t.Parallel()
+
+	server := mockAuthServer(t, true, http.StatusOK)
+	defer server.Close()
+
+	auth := &AuthClient{
+		Address: server.URL,
+		Enabled: true,
+		Logger:  &testLogger{},
+	}
+
+	// normal-user without "sub" claim must fail closed instead of emitting "<owner>/".
+	token := createTestJWT(jwt.MapClaims{
+		"type":  "normal-user",
+		"owner": "acme-org",
+		// "sub" is intentionally missing
+	})
+
+	authorized, statusCode, err := auth.checkAuthorization(
+		context.Background(), "midaz", "resource", "action", token,
+	)
+
+	require.Error(t, err)
+	assert.False(t, authorized)
+	assert.Equal(t, http.StatusUnauthorized, statusCode)
+	assert.Contains(t, err.Error(), "missing sub claim")
+}
+
+func TestCheckAuthorization_NormalUser_EmptyProduct_NotForwarded(t *testing.T) {
+	t.Parallel()
+
+	// With an empty product the previous behavior must be preserved: the subject
+	// is still the JWT identity and no "product" field is forwarded (gate-by-presence).
+	var capturedBody map[string]string
+
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		err := json.NewDecoder(r.Body).Decode(&capturedBody)
+		if err != nil {
+			t.Errorf("mock server: failed to decode request body: %v", err)
+		}
+
+		w.Header().Set("Content-Type", "application/json")
+		w.WriteHeader(http.StatusOK)
+
+		resp := AuthResponse{Authorized: true}
+
+		encErr := json.NewEncoder(w).Encode(resp)
+		if encErr != nil {
+			t.Errorf("mock server: failed to encode response: %v", encErr)
+		}
+	}))
+	defer server.Close()
+
+	auth := &AuthClient{
+		Address: server.URL,
+		Enabled: true,
+		Logger:  &testLogger{},
+	}
+
+	token := createTestJWT(jwt.MapClaims{
+		"type":  "normal-user",
+		"owner": "acme-org",
+		"sub":   "user123",
+	})
+
+	authorized, statusCode, err := auth.checkAuthorization(
+		context.Background(), "", "resource", "action", token,
+	)
+
+	require.NoError(t, err)
+	assert.True(t, authorized)
+	assert.Equal(t, http.StatusOK, statusCode)
+
+	// Subject is still the JWT identity, unchanged by the empty product.
+	assert.Equal(t, "acme-org/user123", capturedBody["sub"])
+	// No product forwarded when product is empty.
+	_, hasProduct := capturedBody["product"]
+	assert.False(t, hasProduct)
+}
+
 func TestCheckAuthorization_MockServerReturnsAuthorizedTrue(t *testing.T) {
 	t.Parallel()