Fix CRM filtering when graphQL variables are used (#1952) (#1953)

* Fix CRM filtering when graphQL variables are used

* fix imports

* Bump PR version

* Fix linter issues

Co-authored-by: Dimitar Petrov <buldozer9993@gmail.com>

chart/compass/values.yaml

@@ -75,7 +75,7 @@ global:
       version: "PR-1910"
     director:
       dir:
-      version: "PR-1951"
+      version: "PR-1952"
     gateway:
       dir:
       version: "PR-1910"

components/director/internal/ownertenant/middleware.go

@@ -3,6 +3,8 @@ package ownertenant
 import (
 	"context"
 
+	"github.com/google/uuid"
+
 	gqlgen "github.com/99designs/gqlgen/graphql"
 	"github.com/kyma-incubator/compass/components/director/internal/domain/tenant"
 	"github.com/kyma-incubator/compass/components/director/internal/model"
@@ -83,6 +85,22 @@ func (m *middleware) InterceptField(ctx context.Context, next gqlgen.Resolver) (
 	}
 
 	if len(id) > 0 { // If the mutation has an ID argument - potentially parent entity
+		if _, err := uuid.Parse(id); err != nil { // GraphQL variables are used and the raw value is the name of the variable. We should find the real value behind the variable.
+			idValue, ok := fieldCtx.Args[id]
+			if !ok {
+				log.C(ctx).Infof("Value for ID argument %s is not GUID or a graphQL variable", id)
+				return next(ctx)
+			}
+
+			idString, ok := idValue.(string)
+			if !ok {
+				log.C(ctx).Infof("Value for %s graphQL variable provided for ID argument is not string", id)
+				return next(ctx)
+			}
+
+			id = idString // resolve the variable
+		}
+
 		tnt, err := tenant.LoadFromContext(ctx)
 		if err != nil {
 			log.C(ctx).Infof("Mutation call does not have a valid tenant in the context: %s. Proceeding without modifications...", err)

components/director/internal/ownertenant/middleware_test.go

@@ -23,9 +23,9 @@ func TestInterceptField(t *testing.T) {
 	testErr := errors.New("Test error")
 	txGen := txtest.NewTransactionContextGenerator(testErr)
 
-	parentTenantID := "parentTenant"
-	ownerTenantID := "ownerTenant"
-	ownerTenantExternalID := "ownerTenantExternalID"
+	parentTenantID := "56384145-8b6c-4394-855a-c6e55619448b"
+	ownerTenantID := "68182a53-ba3a-4cef-8932-055d5c7e0e91"
+	ownerTenantExternalID := "916aa40c-fe7b-4edb-bc34-158c728326b6"
 
 	ownerTenant := &model.BusinessTenantMapping{
 		ID:             ownerTenantID,
@@ -37,7 +37,7 @@ func TestInterceptField(t *testing.T) {
 		Status:         "Active",
 	}
 
-	entityID := "id"
+	entityID := "dc9964c8-4e81-4a58-bc7a-44e788ee1fdd"
 
 	fixFieldCtx := &gqlgen.FieldContext{
 		Object: ownertenant.MutationObject,
@@ -113,6 +113,128 @@ func TestInterceptField(t *testing.T) {
 				}
 			},
 		},
+		{
+			Name:            "Mutation with parent ID in graphQL variable executed by parent tenant should impersonate owner tenant",
+			TransactionerFn: txGen.ThatSucceeds,
+			TenantIndexRepoFn: func() *automock.TenantIndexRepository {
+				repo := &automock.TenantIndexRepository{}
+				repo.On("GetOwnerTenantByResourceID", txtest.CtxWithDBMatcher(), parentTenantID, entityID).Return(ownerTenantID, nil).Once()
+				return repo
+			},
+			TenantRepoFn: func() *automock.TenantRepository {
+				repo := &automock.TenantRepository{}
+				repo.On("Get", txtest.CtxWithDBMatcher(), ownerTenantID).Return(ownerTenant, nil).Once()
+				return repo
+			},
+			ContextProviderFn: func() context.Context {
+				ctx := context.TODO()
+				ctx = gqlgen.WithFieldContext(ctx, &gqlgen.FieldContext{
+					Object: ownertenant.MutationObject,
+					Args: map[string]interface{}{
+						"id": entityID,
+					},
+					Field: gqlgen.CollectedField{
+						Field: &ast.Field{
+							Arguments: ast.ArgumentList{
+								&ast.Argument{
+									Value: &ast.Value{
+										Raw: "id",
+										Definition: &ast.Definition{
+											Name:    "ID",
+											BuiltIn: true,
+										},
+									},
+								},
+							},
+						},
+					},
+				})
+				ctx = tenant.SaveToContext(ctx, parentTenantID, parentTenantID)
+				return ctx
+			},
+			NextResolverFn: func() gqlgen.Resolver {
+				return func(ctx context.Context) (res interface{}, err error) {
+					tnt, err := tenant.LoadFromContext(ctx)
+					require.NoError(t, err)
+					require.Equal(t, ownerTenantID, tnt)
+					return nil, nil
+				}
+			},
+		},
+		{
+			Name:            "Mutation with no GUID or GraphQL variable provided for ID should proceed without any modification",
+			TransactionerFn: txGen.ThatDoesntStartTransaction,
+			TenantIndexRepoFn: func() *automock.TenantIndexRepository {
+				return &automock.TenantIndexRepository{}
+			},
+			TenantRepoFn: func() *automock.TenantRepository {
+				return &automock.TenantRepository{}
+			},
+			ContextProviderFn: func() context.Context {
+				ctx := context.TODO()
+				ctx = gqlgen.WithFieldContext(ctx, &gqlgen.FieldContext{
+					Object: ownertenant.MutationObject,
+					Args: map[string]interface{}{
+						"non-existing": "non-existing",
+					},
+					Field: gqlgen.CollectedField{
+						Field: &ast.Field{
+							Arguments: ast.ArgumentList{
+								&ast.Argument{
+									Value: &ast.Value{
+										Raw: "non-guid-or-variable",
+										Definition: &ast.Definition{
+											Name:    "ID",
+											BuiltIn: true,
+										},
+									},
+								},
+							},
+						},
+					},
+				})
+				ctx = tenant.SaveToContext(ctx, parentTenantID, parentTenantID)
+				return ctx
+			},
+			NextResolverFn: assertTenantNotModifiedResolverFn,
+		},
+		{
+			Name:            "Mutation with non-string variable provided for ID should proceed without any modification",
+			TransactionerFn: txGen.ThatDoesntStartTransaction,
+			TenantIndexRepoFn: func() *automock.TenantIndexRepository {
+				return &automock.TenantIndexRepository{}
+			},
+			TenantRepoFn: func() *automock.TenantRepository {
+				return &automock.TenantRepository{}
+			},
+			ContextProviderFn: func() context.Context {
+				ctx := context.TODO()
+				ctx = gqlgen.WithFieldContext(ctx, &gqlgen.FieldContext{
+					Object: ownertenant.MutationObject,
+					Args: map[string]interface{}{
+						"id": struct{}{},
+					},
+					Field: gqlgen.CollectedField{
+						Field: &ast.Field{
+							Arguments: ast.ArgumentList{
+								&ast.Argument{
+									Value: &ast.Value{
+										Raw: "id",
+										Definition: &ast.Definition{
+											Name:    "ID",
+											BuiltIn: true,
+										},
+									},
+								},
+							},
+						},
+					},
+				})
+				ctx = tenant.SaveToContext(ctx, parentTenantID, parentTenantID)
+				return ctx
+			},
+			NextResolverFn: assertTenantNotModifiedResolverFn,
+		},
 		{
 			Name:            "Nil GraphQL Field context should proceed without any modification",
 			TransactionerFn: txGen.ThatDoesntStartTransaction,