From 184da4c97a38b1895371d32b35027a61a80108f4 Mon Sep 17 00:00:00 2001 From: Kofo Okesola Date: Wed, 31 May 2023 08:07:30 +0100 Subject: [PATCH 1/6] added input validator stage --- pkg/operationreport/externalerror.go | 15 +++ pkg/variablevalidator/variablevalidator.go | 102 ++++++++++++++++++ .../variablevalidator_test.go | 88 +++++++++++++++ 3 files changed, 205 insertions(+) create mode 100644 pkg/variablevalidator/variablevalidator.go create mode 100644 pkg/variablevalidator/variablevalidator_test.go diff --git a/pkg/operationreport/externalerror.go b/pkg/operationreport/externalerror.go index b141b6555e..d7fb2497a3 100644 --- a/pkg/operationreport/externalerror.go +++ b/pkg/operationreport/externalerror.go @@ -27,6 +27,8 @@ const ( UnknownFieldOfInputObjectErrMsg = `Field "%s" is not defined by type "%s".` DuplicatedFieldInputObjectErrMsg = `There can be only one input field named "%s".` ValueIsNotAnInputObjectTypeErrMsg = `Expected value of type "%s", found %s.` + VariableNotProvidedErrMsg = `Required variable "$%s" was not provided` + VariableValidationFailedErrMsg = `Validation for variable "%s" failed: %s` ) type ExternalError struct { @@ -242,6 +244,19 @@ func ErrValueIsNotAnInputObjectType(value, inputType ast.ByteSlice, position pos return err } +func ErrVariableNotProvided(name ast.ByteSlice, position position.Position) (err ExternalError) { + err.Message = fmt.Sprintf(VariableNotProvidedErrMsg, name) + err.Locations = LocationsFromPosition(position) + + return err +} + +func ErrVariableValidationFailed(name ast.ByteSlice, message string, position position.Position) (err ExternalError) { + err.Message = fmt.Sprintf(VariableValidationFailedErrMsg, name, message) + err.Locations = LocationsFromPosition(position) + return err +} + func ErrValueDoesntSatisfyString(value, inputType ast.ByteSlice, position position.Position) (err ExternalError) { err.Message = fmt.Sprintf(NotStringErrMsg, inputType, value) err.Locations = LocationsFromPosition(position) diff --git a/pkg/variablevalidator/variablevalidator.go b/pkg/variablevalidator/variablevalidator.go new file mode 100644 index 0000000000..07cbcfa154 --- /dev/null +++ b/pkg/variablevalidator/variablevalidator.go @@ -0,0 +1,102 @@ +package variablevalidator + +import ( + "bytes" + "context" + "errors" + "github.com/TykTechnologies/graphql-go-tools/pkg/ast" + "github.com/TykTechnologies/graphql-go-tools/pkg/astvisitor" + "github.com/TykTechnologies/graphql-go-tools/pkg/graphqljsonschema" + "github.com/TykTechnologies/graphql-go-tools/pkg/operationreport" + "github.com/buger/jsonparser" +) + +type VariableValidator struct { + walker *astvisitor.Walker + visitor *validatorVisitor +} + +func NewVariableValidator() *VariableValidator { + walker := astvisitor.Walker{} + validator := VariableValidator{ + walker: &walker, + visitor: &validatorVisitor{ + Walker: &walker, + currentOperation: ast.InvalidRef, + }, + } + + validator.walker.RegisterEnterDocumentVisitor(validator.visitor) + validator.walker.RegisterEnterOperationVisitor(validator.visitor) + validator.walker.RegisterLeaveOperationVisitor(validator.visitor) + validator.walker.RegisterEnterVariableDefinitionVisitor(validator.visitor) + + return &validator +} + +type validatorVisitor struct { + *astvisitor.Walker + + operationName, variables []byte + currentOperation int + operation, definition *ast.Document +} + +func (v *validatorVisitor) EnterDocument(operation, definition *ast.Document) { + v.operation, v.definition = operation, definition +} + +func (v *validatorVisitor) EnterVariableDefinition(ref int) { + if v.currentOperation == ast.InvalidRef { + return + } + typeRef := v.operation.VariableDefinitions[ref].Type + + variableName := v.operation.VariableDefinitionNameBytes(ref) + variable, t, _, err := jsonparser.Get(v.variables, string(variableName)) + if t == jsonparser.NotExist && v.operation.TypeIsNonNull(typeRef) { + v.StopWithExternalErr(operationreport.ErrVariableNotProvided(variableName, v.operation.VariableDefinitions[ref].VariableValue.Position)) + return + } + if err != nil { + v.StopWithInternalErr(errors.New("error parsing variables")) + return + } + + jsonSchema := graphqljsonschema.FromTypeRef(v.operation, v.definition, typeRef) + schemaValidator, err := graphqljsonschema.NewValidatorFromSchema(jsonSchema) + if err != nil { + v.StopWithInternalErr(err) + return + } + if err := schemaValidator.Validate(context.Background(), variable); err != nil { + v.StopWithExternalErr(operationreport.ErrVariableValidationFailed(variableName, err.Error(), v.operation.VariableDefinitions[ref].VariableValue.Position)) + return + } +} + +func (v *validatorVisitor) EnterOperationDefinition(ref int) { + if v.operationName == nil { + v.currentOperation = ref + return + } + + if bytes.Equal(v.operationName, v.operation.OperationDefinitionNameBytes(ref)) { + v.currentOperation = ref + } +} + +func (v *validatorVisitor) LeaveOperationDefinition(ref int) { + if v.currentOperation == ref { + v.Stop() + } +} + +func (v *VariableValidator) Validate(operation, definition *ast.Document, operationName, variables []byte, report *operationreport.Report) { + if v.visitor != nil { + v.visitor.operationName = operationName + v.visitor.variables = variables + } + + v.walker.Walk(operation, definition, report) +} diff --git a/pkg/variablevalidator/variablevalidator_test.go b/pkg/variablevalidator/variablevalidator_test.go new file mode 100644 index 0000000000..0bee5875d2 --- /dev/null +++ b/pkg/variablevalidator/variablevalidator_test.go @@ -0,0 +1,88 @@ +package variablevalidator + +import ( + "github.com/TykTechnologies/graphql-go-tools/internal/pkg/unsafeparser" + "github.com/TykTechnologies/graphql-go-tools/pkg/asttransform" + "github.com/TykTechnologies/graphql-go-tools/pkg/operationreport" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "testing" +) + +const testDefinition = ` +input CustomInput { + requiredField: String! + optionalField: String +} + +type Query{ + simpleQuery(code: ID): String +} + +type Mutation { + customInputNonNull(in: CustomInput!): String +}` + +const testQuery = ` +query testQuery($code: ID!){ + simpleQuery(code: $code) +} +` + +const customInputMutation = ` +mutation testMutation($in: CustomInput!){ + customInputNonNull(in: $in) +}` + +func TestVariableValidator(t *testing.T) { + testCases := []struct { + name string + operation string + variables string + expectedError string + }{ + { + name: "basic variable query", + operation: testQuery, + variables: `{"code":"NG"}`, + }, + { + name: "missing variable", + operation: testQuery, + variables: `{"codes":"NG"}`, + expectedError: `Required variable "$code" was not provided`, + }, + { + name: "no variable passed", + operation: testQuery, + variables: "", + expectedError: `Required variable "$code" was not provided`, + }, + { + name: "nested input variable", + operation: customInputMutation, + variables: `{"in":{"optionalField":"test"}}`, + expectedError: `Validation for variable "in" failed: validation failed: /: {"optionalField":"te... "requiredField" value is required`, + }, + } + for _, c := range testCases { + t.Run(c.name, func(t *testing.T) { + definitionDocument := unsafeparser.ParseGraphqlDocumentString(testDefinition) + require.NoError(t, asttransform.MergeDefinitionWithBaseSchema(&definitionDocument)) + + operationDocument := unsafeparser.ParseGraphqlDocumentString(c.operation) + + report := operationreport.Report{} + validator := NewVariableValidator() + validator.Validate(&operationDocument, &definitionDocument, nil, []byte(c.variables), &report) + + if c.expectedError == "" && report.HasErrors() { + t.Fatalf("expected no error, instead got %s", report.Error()) + } + if c.expectedError != "" { + require.Equal(t, 1, len(report.ExternalErrors)) + assert.Equal(t, c.expectedError, report.ExternalErrors[0].Message) + } + }) + } +} From 5ed515cd49c608460b32b7f2d27954b71f1ad663 Mon Sep 17 00:00:00 2001 From: Kofo Okesola Date: Wed, 31 May 2023 08:48:48 +0100 Subject: [PATCH 2/6] added multiple operation test --- pkg/variablevalidator/variablevalidator.go | 7 ++++++- .../variablevalidator_test.go | 21 +++++++++++++++++-- 2 files changed, 25 insertions(+), 3 deletions(-) diff --git a/pkg/variablevalidator/variablevalidator.go b/pkg/variablevalidator/variablevalidator.go index 07cbcfa154..c760a91703 100644 --- a/pkg/variablevalidator/variablevalidator.go +++ b/pkg/variablevalidator/variablevalidator.go @@ -4,6 +4,7 @@ import ( "bytes" "context" "errors" + "fmt" "github.com/TykTechnologies/graphql-go-tools/pkg/ast" "github.com/TykTechnologies/graphql-go-tools/pkg/astvisitor" "github.com/TykTechnologies/graphql-go-tools/pkg/graphqljsonschema" @@ -63,6 +64,10 @@ func (v *validatorVisitor) EnterVariableDefinition(ref int) { return } + if t == jsonparser.String { + variable = []byte(fmt.Sprintf(`"%s"`, string(variable))) + } + jsonSchema := graphqljsonschema.FromTypeRef(v.operation, v.definition, typeRef) schemaValidator, err := graphqljsonschema.NewValidatorFromSchema(jsonSchema) if err != nil { @@ -76,7 +81,7 @@ func (v *validatorVisitor) EnterVariableDefinition(ref int) { } func (v *validatorVisitor) EnterOperationDefinition(ref int) { - if v.operationName == nil { + if len(v.operationName) == 0 { v.currentOperation = ref return } diff --git a/pkg/variablevalidator/variablevalidator_test.go b/pkg/variablevalidator/variablevalidator_test.go index 0bee5875d2..90ff020b3b 100644 --- a/pkg/variablevalidator/variablevalidator_test.go +++ b/pkg/variablevalidator/variablevalidator_test.go @@ -23,17 +23,29 @@ type Mutation { customInputNonNull(in: CustomInput!): String }` -const testQuery = ` +const ( + testQuery = ` query testQuery($code: ID!){ simpleQuery(code: $code) } ` -const customInputMutation = ` + customInputMutation = ` mutation testMutation($in: CustomInput!){ customInputNonNull(in: $in) }` + customMultipleOperation = ` +query testQuery($code: ID!){ + simpleQuery(code: $code) +} + +mutation testMutation($in: CustomInput!){ + customInputNonNull(in: $in) +} +` +) + func TestVariableValidator(t *testing.T) { testCases := []struct { name string @@ -64,6 +76,11 @@ func TestVariableValidator(t *testing.T) { variables: `{"in":{"optionalField":"test"}}`, expectedError: `Validation for variable "in" failed: validation failed: /: {"optionalField":"te... "requiredField" value is required`, }, + { + name: "multiple operation should validate single operation", + operation: customMultipleOperation, + variables: `{"code":"NG"}`, + }, } for _, c := range testCases { t.Run(c.name, func(t *testing.T) { From 8c3ab3b3aa610457ec1321594745b8ecd5ae2a05 Mon Sep 17 00:00:00 2001 From: Kofo Okesola Date: Wed, 31 May 2023 08:53:47 +0100 Subject: [PATCH 3/6] added int example --- .../variablevalidator_test.go | 22 +++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/pkg/variablevalidator/variablevalidator_test.go b/pkg/variablevalidator/variablevalidator_test.go index 90ff020b3b..dc17fd0dcb 100644 --- a/pkg/variablevalidator/variablevalidator_test.go +++ b/pkg/variablevalidator/variablevalidator_test.go @@ -17,6 +17,7 @@ input CustomInput { type Query{ simpleQuery(code: ID): String + inputOfInt(code: Int!): String } type Mutation { @@ -28,6 +29,11 @@ const ( query testQuery($code: ID!){ simpleQuery(code: $code) } +` + testQueryInt = ` +query testQuery($code: Int!){ + inputOfInt(code: $code) +} ` customInputMutation = ` @@ -50,6 +56,7 @@ func TestVariableValidator(t *testing.T) { testCases := []struct { name string operation string + operationName string variables string expectedError string }{ @@ -58,6 +65,11 @@ func TestVariableValidator(t *testing.T) { operation: testQuery, variables: `{"code":"NG"}`, }, + { + name: "basic variable query of int", + operation: testQueryInt, + variables: `{"code":1}`, + }, { name: "missing variable", operation: testQuery, @@ -77,10 +89,16 @@ func TestVariableValidator(t *testing.T) { expectedError: `Validation for variable "in" failed: validation failed: /: {"optionalField":"te... "requiredField" value is required`, }, { - name: "multiple operation should validate single operation", + name: "multiple operation should validate first operation", operation: customMultipleOperation, variables: `{"code":"NG"}`, }, + { + name: "multiple operation should validate operation name", + operation: customMultipleOperation, + operationName: "testMutation", + variables: `{"in":{"requiredField":"test"}}`, + }, } for _, c := range testCases { t.Run(c.name, func(t *testing.T) { @@ -91,7 +109,7 @@ func TestVariableValidator(t *testing.T) { report := operationreport.Report{} validator := NewVariableValidator() - validator.Validate(&operationDocument, &definitionDocument, nil, []byte(c.variables), &report) + validator.Validate(&operationDocument, &definitionDocument, []byte(c.operationName), []byte(c.variables), &report) if c.expectedError == "" && report.HasErrors() { t.Fatalf("expected no error, instead got %s", report.Error()) From 8b91a4be38df60b730a0b90c4941c0dbd039d290 Mon Sep 17 00:00:00 2001 From: Kofo Okesola Date: Wed, 31 May 2023 09:10:51 +0100 Subject: [PATCH 4/6] added input_validation fil --- pkg/graphql/input_validation.go | 44 ++++++++++++++++++++++++++++ pkg/graphql/input_validation_test.go | 30 +++++++++++++++++++ 2 files changed, 74 insertions(+) create mode 100644 pkg/graphql/input_validation.go create mode 100644 pkg/graphql/input_validation_test.go diff --git a/pkg/graphql/input_validation.go b/pkg/graphql/input_validation.go new file mode 100644 index 0000000000..f9c8d8f1b0 --- /dev/null +++ b/pkg/graphql/input_validation.go @@ -0,0 +1,44 @@ +package graphql + +import ( + "github.com/TykTechnologies/graphql-go-tools/pkg/operationreport" + "github.com/TykTechnologies/graphql-go-tools/pkg/variablevalidator" +) + +type InputValidationResult struct { + Valid bool + Errors Errors +} + +func inputValidationResultFromReport(report operationreport.Report) (InputValidationResult, error) { + result := InputValidationResult{ + Valid: false, + Errors: nil, + } + + if !report.HasErrors() { + result.Valid = true + return result, nil + } + + result.Errors = RequestErrorsFromOperationReport(report) + + var err error + if len(report.InternalErrors) > 0 { + err = report.InternalErrors[0] + } + + return result, err +} + +func (r *Request) ValidateInput(schema *Schema) (InputValidationResult, error) { + validator := variablevalidator.NewVariableValidator() + + report := r.parseQueryOnce() + if report.HasErrors() { + return inputValidationResultFromReport(report) + } + validator.Validate(&r.document, &schema.document, []byte(r.OperationName), r.Variables, &report) + + return inputValidationResultFromReport(report) +} diff --git a/pkg/graphql/input_validation_test.go b/pkg/graphql/input_validation_test.go new file mode 100644 index 0000000000..2bfcb62787 --- /dev/null +++ b/pkg/graphql/input_validation_test.go @@ -0,0 +1,30 @@ +package graphql + +import ( + "github.com/TykTechnologies/graphql-go-tools/pkg/starwars" + "github.com/stretchr/testify/assert" + "testing" +) + +func TestRequest_ValidateInput(t *testing.T) { + t.Run("Should pass input validation", func(t *testing.T) { + schema := starwarsSchema(t) + request := requestForQuery(t, starwars.FileDroidWithArgAndVarQuery) + request.Variables = []byte(`{"droidID":"testID"}`) + + result, err := request.ValidateInput(schema) + assert.NoError(t, err) + assert.True(t, result.Valid) + assert.Nil(t, result.Errors) + }) + + t.Run("Should fail input validation", func(t *testing.T) { + schema := starwarsSchema(t) + request := requestForQuery(t, starwars.FileDroidWithArgAndVarQuery) + + result, err := request.ValidateInput(schema) + assert.NoError(t, err) + assert.False(t, result.Valid) + assert.Equal(t, `Required variable "$droidID" was not provided, locations: [{Line:1 Column:13}], path: [query]`, result.Errors.Error()) + }) +} From 0c6a69daacf66dbc56261dcc3589b799220f9c55 Mon Sep 17 00:00:00 2001 From: Kofo Okesola Date: Fri, 2 Jun 2023 08:44:01 +0100 Subject: [PATCH 5/6] added to optional execution engine stages --- pkg/graphql/execution_engine_v2.go | 11 +++++++++++ pkg/graphql/execution_engine_v2_custom.go | 21 +++++++++++++++++---- 2 files changed, 28 insertions(+), 4 deletions(-) diff --git a/pkg/graphql/execution_engine_v2.go b/pkg/graphql/execution_engine_v2.go index 59fcccb49b..c41a87c4e2 100644 --- a/pkg/graphql/execution_engine_v2.go +++ b/pkg/graphql/execution_engine_v2.go @@ -257,6 +257,17 @@ func (e *ExecutionEngineV2) ValidateForSchema(operation *Request) error { return nil } +func (e *ExecutionEngineV2) InputValidation(operation *Request) error { + result, err := operation.ValidateInput(e.config.schema) + if err != nil { + return err + } + if !result.Valid { + return result.Errors + } + return nil +} + func (e *ExecutionEngineV2) Setup(ctx context.Context, postProcessor *postprocess.Processor, resolveContext *resolve.Context, operation *Request, options ...ExecutionOptionsV2) { for i := range options { options[i](postProcessor, resolveContext) diff --git a/pkg/graphql/execution_engine_v2_custom.go b/pkg/graphql/execution_engine_v2_custom.go index 21d6080749..f76f30adbf 100644 --- a/pkg/graphql/execution_engine_v2_custom.go +++ b/pkg/graphql/execution_engine_v2_custom.go @@ -23,6 +23,10 @@ type CustomExecutionEngineV2ValidatorStage interface { ValidateForSchema(operation *Request) error } +type CustomExecutionEngineV2InputValidationStage interface { + InputValidation(operation *Request) error +} + type CustomExecutionEngineV2ResolverStage interface { Setup(ctx context.Context, postProcessor *postprocess.Processor, resolveContext *resolve.Context, operation *Request, options ...ExecutionOptionsV2) Plan(postProcessor *postprocess.Processor, operation *Request, report *operationreport.Report) (plan.Plan, error) @@ -34,6 +38,7 @@ type CustomExecutionEngineV2 interface { CustomExecutionEngineV2NormalizerStage CustomExecutionEngineV2ValidatorStage CustomExecutionEngineV2ResolverStage + CustomExecutionEngineV2InputValidationStage } type ExecutionEngineV2Executor interface { @@ -54,8 +59,9 @@ type CustomExecutionEngineV2RequiredStages struct { } type CustomExecutionEngineV2OptionalStages struct { - NormalizerStage CustomExecutionEngineV2NormalizerStage - ValidatorStage CustomExecutionEngineV2ValidatorStage + NormalizerStage CustomExecutionEngineV2NormalizerStage + ValidatorStage CustomExecutionEngineV2ValidatorStage + InputValidationStage CustomExecutionEngineV2InputValidationStage } type CustomExecutionEngineV2Executor struct { @@ -69,8 +75,9 @@ func NewCustomExecutionEngineV2Executor(executionEngineV2 CustomExecutionEngineV ResolverStage: executionEngineV2, }, OptionalStages: &CustomExecutionEngineV2OptionalStages{ - NormalizerStage: executionEngineV2, - ValidatorStage: executionEngineV2, + NormalizerStage: executionEngineV2, + ValidatorStage: executionEngineV2, + InputValidationStage: executionEngineV2, }, } @@ -117,6 +124,12 @@ func (c *CustomExecutionEngineV2Executor) Execute(ctx context.Context, operation } } + if c.ExecutionStages.OptionalStages != nil && c.ExecutionStages.OptionalStages.InputValidationStage != nil { + if err := c.ExecutionStages.OptionalStages.InputValidationStage.InputValidation(operation); err != nil { + return err + } + } + execContext := c.getExecutionCtx() defer c.putExecutionCtx(execContext) execContext.prepare(ctx, operation.Variables, operation.request) From d0f77a27bf0d2b9b9949863ee470f2cd4f6d0405 Mon Sep 17 00:00:00 2001 From: Kofo Okesola Date: Mon, 5 Jun 2023 08:32:19 +0100 Subject: [PATCH 6/6] removed input validation from default optional stage --- pkg/graphql/execution_engine_v2_custom.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/pkg/graphql/execution_engine_v2_custom.go b/pkg/graphql/execution_engine_v2_custom.go index f76f30adbf..ce16378337 100644 --- a/pkg/graphql/execution_engine_v2_custom.go +++ b/pkg/graphql/execution_engine_v2_custom.go @@ -75,9 +75,8 @@ func NewCustomExecutionEngineV2Executor(executionEngineV2 CustomExecutionEngineV ResolverStage: executionEngineV2, }, OptionalStages: &CustomExecutionEngineV2OptionalStages{ - NormalizerStage: executionEngineV2, - ValidatorStage: executionEngineV2, - InputValidationStage: executionEngineV2, + NormalizerStage: executionEngineV2, + ValidatorStage: executionEngineV2, }, }