[TT-11655] Graphql APIs are unable to handle OPTIONS requests (#6221)

## **User description**
<!-- Provide a general summary of your changes in the Title above -->

## Description

<!-- Describe your changes in detail -->
Links to this
[ticket](https://tyktech.atlassian.net/jira/software/c/projects/TT/boards/26?selectedIssue=TT-11655)
## Related Issue

<!-- This project only accepts pull requests related to open issues. -->
<!-- If suggesting a new feature or change, please discuss it in an
issue first. -->
<!-- If fixing a bug, there should be an issue describing it with steps
to reproduce. -->
<!-- OSS: Please link to the issue here. Tyk: please create/link the
JIRA ticket. -->

## Motivation and Context

<!-- Why is this change required? What problem does it solve? -->

## How This Has Been Tested

<!-- Please describe in detail how you tested your changes -->
<!-- Include details of your testing environment, and the tests -->
<!-- you ran to see how your change affects other areas of the code,
etc. -->
<!-- This information is helpful for reviewers and QA. -->

## Screenshots (if appropriate)

## Types of changes

<!-- What types of changes does your code introduce? Put an `x` in all
the boxes that apply: -->

- [x] Bug fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing
functionality to change)
- [ ] Refactoring or add test (improvements in base code or adds test
coverage to functionality)

## Checklist

<!-- Go over all the following points, and put an `x` in all the boxes
that apply -->
<!-- If there are no documentation updates required, mark the item as
checked. -->
<!-- Raise up any additional concerns not covered by the checklist. -->

- [ ] I ensured that the documentation is up to date
- [ ] I explained why this PR updates go.mod in detail with reasoning
why it's required
- [ ] I would like a code coverage CI quality gate exception and have
explained why


___

## **Type**
Bug fix


___

## **Description**
- Fixes an issue where GraphQL APIs were unable to handle OPTIONS
requests due to an explicit error block. This change allows CORS
preflight requests to be processed, improving compatibility with web
applications that need to perform CORS requests.


___



## **Changes walkthrough**
<table><thead><tr><th></th><th align="left">Relevant
files</th></tr></thead><tbody><tr><td><strong>Bug
fix</strong></td><td><table>
<tr>
  <td>
    <details>
<summary><strong>graphql_go_tools_v1.go</strong><dd><code>Allow OPTIONS
Requests for CORS Preflight in GraphQL</code>&nbsp; &nbsp; &nbsp; &nbsp;
&nbsp; </dd></summary>
<hr>

internal/graphengine/graphql_go_tools_v1.go
<li>Removed an error that prevented OPTIONS requests from being handled,
<br>allowing for CORS preflight requests to be processed.<br>


</details>
    

  </td>
<td><a
href="https://github.com/TykTechnologies/tyk/pull/6221/files#diff-e592cc8ca6ac39e7574765d7f2bbf19193f173791a1b0930d4dde7f9412dc882">+1/-2</a>&nbsp;
&nbsp; &nbsp; </td>
</tr>                    
</table></td></tr></tr></tbody></table>

___

> ✨ **PR-Agent usage**:
>Comment `/help` on the PR to get a list of all available PR-Agent tools
and their descriptions

internal/graphengine/consts.go

@@ -24,4 +24,5 @@ const (
 	ReverseProxyTypeIntrospection
 	ReverseProxyTypeWebsocketUpgrade
 	ReverseProxyTypeGraphEngine
+	ReverseProxyTypePreFlight
 )

internal/graphengine/engine_v1.go

@@ -166,6 +166,11 @@ func (e *EngineV1) HandleReverseProxy(params ReverseProxyParams) (res *http.Resp
 		return e.handoverWebSocketConnectionToGraphQLExecutionEngine(&params)
 	case ReverseProxyTypeGraphEngine:
 		return e.handoverRequestToGraphQLExecutionEngine(gqlRequest, params.OutRequest)
+	case ReverseProxyTypePreFlight:
+		if e.ApiDefinition.GraphQL.ExecutionMode == apidef.GraphQLExecutionModeProxyOnly {
+			return nil, false, nil
+		}
+		return nil, false, errors.New("options passthrough not allowed")
 	case ReverseProxyTypeNone:
 		return nil, false, nil
 	}

internal/graphengine/engine_v1_test.go

@@ -270,10 +270,64 @@ func TestEngineV1_HandleReverseProxy(t *testing.T) {
 		assert.Equal(t, 200, result.StatusCode)
 		assert.Equal(t, `{"data":{"hello":"world"}}`, body.String())
 	})
+
+	t.Run("should return error if reverse proxy pre handler returns proxy type preflight and execution mode is NOT proxy only", func(t *testing.T) {
+		engine, mocks := newTestEngineV1(t, withExecutionModeTestEngineV1(apidef.GraphQLExecutionModeSubgraph))
+		defer mocks.controller.Finish()
+		params := ReverseProxyParams{
+			OutRequest:      &http.Request{},
+			IsCORSPreflight: true,
+		}
+		mocks.reverseProxyPreHandler.EXPECT().PreHandle(gomock.Eq(params)).
+			Return(ReverseProxyTypePreFlight, nil)
+
+		engine.ctxRetrieveRequestFunc = func(r *http.Request) *graphql.Request {
+			return &graphql.Request{
+				Query: "query { hello }",
+			}
+		}
+
+		result, hijacked, err := engine.HandleReverseProxy(params)
+		var body bytes.Buffer
+		if result != nil {
+			_, _ = body.ReadFrom(result.Body)
+		}
+
+		assert.Error(t, err)
+		assert.False(t, hijacked)
+		assert.Nil(t, result)
+	})
+
+	t.Run("should return successful if reverse proxy pre handler returns proxy type preflight and execution mode is proxy only", func(t *testing.T) {
+		engine, mocks := newTestEngineV1(t, withExecutionModeTestEngineV1(apidef.GraphQLExecutionModeProxyOnly))
+		defer mocks.controller.Finish()
+		params := ReverseProxyParams{
+			OutRequest:      &http.Request{},
+			IsCORSPreflight: true,
+		}
+		mocks.reverseProxyPreHandler.EXPECT().PreHandle(gomock.Eq(params)).
+			Return(ReverseProxyTypePreFlight, nil)
+
+		engine.ctxRetrieveRequestFunc = func(r *http.Request) *graphql.Request {
+			return &graphql.Request{}
+		}
+
+		result, hijacked, err := engine.HandleReverseProxy(params)
+		var body bytes.Buffer
+		if result != nil {
+			_, _ = body.ReadFrom(result.Body)
+		}
+
+		assert.NoError(t, err)
+		assert.False(t, hijacked)
+		assert.Nil(t, result)
+	})
+
 }
 
 type testEngineV1Options struct {
-	targetURL string
+	targetURL     string
+	executionMode apidef.GraphQLExecutionMode
 }
 
 type testEngineV1Option func(*testEngineV1Options)
@@ -284,6 +338,12 @@ func withTargetURLTestEngineV1(targetURL string) testEngineV1Option {
 	}
 }
 
+func withExecutionModeTestEngineV1(executionMode apidef.GraphQLExecutionMode) testEngineV1Option {
+	return func(options *testEngineV1Options) {
+		options.executionMode = executionMode
+	}
+}
+
 func newTestEngineV1(t *testing.T, options ...testEngineV1Option) (*EngineV1, engineV1Mocks) {
 	definedOptions := testEngineV1Options{}
 	for _, option := range options {
@@ -299,19 +359,21 @@ func newTestEngineV1(t *testing.T, options ...testEngineV1Option) (*EngineV1, en
 		reverseProxyPreHandler: NewMockReverseProxyPreHandler(ctrl),
 	}
 
+	apiDefinition := generateApiDefinitionEngineV1(definedOptions.targetURL, definedOptions.executionMode)
 	gqlTools := graphqlGoToolsV1{}
 	schema, err := gqlTools.parseSchema(testSchemaEngineV1)
 	require.NoError(t, err)
 
 	executionEngine, err := gqlTools.createExecutionEngine(createExecutionEngineV1Params{
 		logger: abstractlogger.NoopLogger,
-		apiDef: generateApiDefinitionEngineV1(definedOptions.targetURL),
+		apiDef: apiDefinition,
 		schema: schema,
 	})
 
 	engine := &EngineV1{
 		ExecutionEngine:         executionEngine,
 		Schema:                  schema,
+		ApiDefinition:           apiDefinition,
 		logger:                  abstractlogger.NoopLogger,
 		graphqlRequestProcessor: mocks.requestProcessor,
 		complexityChecker:       mocks.complexityChecker,
@@ -322,11 +384,11 @@ func newTestEngineV1(t *testing.T, options ...testEngineV1Option) (*EngineV1, en
 	return engine, mocks
 }
 
-func generateApiDefinitionEngineV1(targetURL string) *apidef.APIDefinition {
+func generateApiDefinitionEngineV1(targetURL string, executionMode apidef.GraphQLExecutionMode) *apidef.APIDefinition {
 	return &apidef.APIDefinition{
 		GraphQL: apidef.GraphQLConfig{
 			Enabled:          true,
-			ExecutionMode:    apidef.GraphQLExecutionModeExecutionEngine,
+			ExecutionMode:    executionMode,
 			Version:          apidef.GraphQLConfigVersion1,
 			Schema:           testSchemaEngineV1,
 			LastSchemaUpdate: nil,

internal/graphengine/engine_v2.go

@@ -247,6 +247,11 @@ func (e *EngineV2) HandleReverseProxy(params ReverseProxyParams) (res *http.Resp
 		return e.handoverWebSocketConnectionToGraphQLExecutionEngine(&params)
 	case ReverseProxyTypeGraphEngine:
 		return e.handoverRequestToGraphQLExecutionEngine(gqlRequest, params.OutRequest)
+	case ReverseProxyTypePreFlight:
+		if e.ApiDefinition.GraphQL.ExecutionMode == apidef.GraphQLExecutionModeProxyOnly {
+			return nil, false, nil
+		}
+		return nil, false, errors.New("options passthrough not allowed")
 	case ReverseProxyTypeNone:
 		return nil, false, nil
 	}

internal/graphengine/engine_v2_test.go

@@ -319,6 +319,63 @@ func TestEngineV2_HandleReverseProxy(t *testing.T) {
 		assert.Equal(t, 200, result.StatusCode)
 		assert.Equal(t, `{"data":{"hello":"world"}}`, body.String())
 	})
+
+	t.Run("should return error if reverse proxy pre handler returns proxy type preflight and execution mode is NOT proxy only", func(t *testing.T) {
+		server := httptest.NewServer(http.HandlerFunc(func(_ http.ResponseWriter, _ *http.Request) {}))
+		t.Cleanup(server.Close)
+
+		engine, mocks := newTestEngineV2(t, withApiDefinitionTestEngineV2(newTestApiDefinitionV2(apidef.GraphQLExecutionModeSubgraph, server.URL)))
+
+		defer mocks.controller.Finish()
+		params := ReverseProxyParams{
+			OutRequest:      &http.Request{},
+			IsCORSPreflight: true,
+		}
+		mocks.reverseProxyPreHandler.EXPECT().PreHandle(gomock.Eq(params)).
+			Return(ReverseProxyTypePreFlight, nil)
+
+		engine.ctxRetrieveRequestFunc = func(r *http.Request) *graphql.Request {
+			return &graphql.Request{}
+		}
+
+		result, hijacked, err := engine.HandleReverseProxy(params)
+		var body bytes.Buffer
+		if result != nil {
+			_, _ = body.ReadFrom(result.Body)
+		}
+
+		assert.Error(t, err)
+		assert.False(t, hijacked)
+		assert.Nil(t, result)
+	})
+
+	t.Run("should return successful if reverse proxy pre handler returns proxy type preflight and execution mode is proxy only", func(t *testing.T) {
+		server := httptest.NewServer(http.HandlerFunc(func(_ http.ResponseWriter, _ *http.Request) {}))
+		t.Cleanup(server.Close)
+
+		engine, mocks := newTestEngineV2(t, withApiDefinitionTestEngineV2(newTestApiDefinitionV2(apidef.GraphQLExecutionModeProxyOnly, server.URL)))
+		defer mocks.controller.Finish()
+		params := ReverseProxyParams{
+			OutRequest:      &http.Request{},
+			IsCORSPreflight: true,
+		}
+		mocks.reverseProxyPreHandler.EXPECT().PreHandle(gomock.Eq(params)).
+			Return(ReverseProxyTypePreFlight, nil)
+
+		engine.ctxRetrieveRequestFunc = func(r *http.Request) *graphql.Request {
+			return &graphql.Request{}
+		}
+
+		result, hijacked, err := engine.HandleReverseProxy(params)
+		var body bytes.Buffer
+		if result != nil {
+			_, _ = body.ReadFrom(result.Body)
+		}
+
+		assert.NoError(t, err)
+		assert.False(t, hijacked)
+		assert.Nil(t, result)
+	})
 }
 
 type testEngineV2Options struct {

internal/graphengine/graphql_go_tools_v1.go

@@ -494,10 +494,7 @@ func (r *reverseProxyPreHandlerV1) PreHandle(params ReverseProxyParams) (reverse
 
 	switch {
 	case params.IsCORSPreflight:
-		if params.NeedsEngine {
-			err = errors.New("options passthrough not allowed")
-			return ReverseProxyTypeNone, err
-		}
+		return ReverseProxyTypePreFlight, nil
 	case params.IsWebSocketUpgrade:
 		if params.NeedsEngine {
 			return ReverseProxyTypeWebsocketUpgrade, nil

internal/graphengine/graphql_go_tools_v1_test.go

@@ -753,7 +753,7 @@ func TestReverseProxyPreHandlerV1_PreHandle(t *testing.T) {
 			)))
 		require.NoError(t, err)
 
-		reverseProxyPreHandler := newTestReverseProxyPreHandlerV1(t)
+		reverseProxyPreHandler := newTestReverseProxyPreHandlerV1(t, apidef.GraphQLExecutionModeSubgraph)
 		reverseProxyPreHandler.ctxRetrieveGraphQLRequest = func(r *http.Request) *graphql.Request {
 			if r == request {
 				return &graphql.Request{
@@ -766,11 +766,10 @@ func TestReverseProxyPreHandlerV1_PreHandle(t *testing.T) {
 
 		result, err := reverseProxyPreHandler.PreHandle(ReverseProxyParams{
 			OutRequest:      request,
-			NeedsEngine:     true,
 			IsCORSPreflight: true,
 		})
-		assert.Error(t, err)
-		assert.Equal(t, ReverseProxyTypeNone, result)
+		assert.NoError(t, err)
+		assert.Equal(t, ReverseProxyTypePreFlight, result)
 	})
 
 	t.Run("should return ReverseProxyTypeWebsocketUpgrade on websocket upgrade", func(t *testing.T) {
@@ -784,7 +783,7 @@ func TestReverseProxyPreHandlerV1_PreHandle(t *testing.T) {
 			)))
 		require.NoError(t, err)
 
-		reverseProxyPreHandler := newTestReverseProxyPreHandlerV1(t)
+		reverseProxyPreHandler := newTestReverseProxyPreHandlerV1(t, apidef.GraphQLExecutionModeProxyOnly)
 		reverseProxyPreHandler.ctxRetrieveGraphQLRequest = func(r *http.Request) *graphql.Request {
 			return nil // an upgrade request won't contain a graphql operation
 		}
@@ -809,7 +808,7 @@ func TestReverseProxyPreHandlerV1_PreHandle(t *testing.T) {
 			)))
 		require.NoError(t, err)
 
-		reverseProxyPreHandler := newTestReverseProxyPreHandlerV1(t)
+		reverseProxyPreHandler := newTestReverseProxyPreHandlerV1(t, apidef.GraphQLExecutionModeProxyOnly)
 		reverseProxyPreHandler.ctxRetrieveGraphQLRequest = func(r *http.Request) *graphql.Request {
 			if r == request {
 				return &graphql.Request{
@@ -838,7 +837,7 @@ func TestReverseProxyPreHandlerV1_PreHandle(t *testing.T) {
 			)))
 		require.NoError(t, err)
 
-		reverseProxyPreHandler := newTestReverseProxyPreHandlerV1(t)
+		reverseProxyPreHandler := newTestReverseProxyPreHandlerV1(t, apidef.GraphQLExecutionModeProxyOnly)
 		reverseProxyPreHandler.ctxRetrieveGraphQLRequest = func(r *http.Request) *graphql.Request {
 			if r == request {
 				return &graphql.Request{
@@ -868,7 +867,7 @@ func TestReverseProxyPreHandlerV1_PreHandle(t *testing.T) {
 			)))
 		require.NoError(t, err)
 
-		reverseProxyPreHandler := newTestReverseProxyPreHandlerV1(t)
+		reverseProxyPreHandler := newTestReverseProxyPreHandlerV1(t, apidef.GraphQLExecutionModeProxyOnly)
 		reverseProxyPreHandler.ctxRetrieveGraphQLRequest = func(r *http.Request) *graphql.Request {
 			if r == request {
 				return &graphql.Request{
@@ -886,6 +885,30 @@ func TestReverseProxyPreHandlerV1_PreHandle(t *testing.T) {
 		assert.NoError(t, err)
 		assert.Equal(t, ReverseProxyTypeNone, result)
 	})
+
+	t.Run("should return ReverseProxyTypePreFlight if CORS pre flight is true", func(t *testing.T) {
+		operation := `{ hello }`
+
+		request, err := http.NewRequest(
+			http.MethodOptions,
+			"http://example.com",
+			bytes.NewBuffer([]byte(
+				fmt.Sprintf(`{"query": "%s"}`, operation),
+			)))
+		require.NoError(t, err)
+
+		reverseProxyPreHandler := newTestReverseProxyPreHandlerV1(t, apidef.GraphQLExecutionModeProxyOnly)
+		reverseProxyPreHandler.ctxRetrieveGraphQLRequest = func(r *http.Request) *graphql.Request {
+			return nil
+		}
+
+		result, err := reverseProxyPreHandler.PreHandle(ReverseProxyParams{
+			OutRequest:      request,
+			IsCORSPreflight: true,
+		})
+		assert.NoError(t, err)
+		assert.Equal(t, ReverseProxyTypePreFlight, result)
+	})
 }
 
 func newTestGraphqlRequestProcessorV1(t *testing.T) *graphqlRequestProcessorV1 {
@@ -966,12 +989,12 @@ func newTestGranularAccessCheckerV1(t *testing.T) *granularAccessCheckerV1 {
 	}
 }
 
-func newTestReverseProxyPreHandlerV1(t *testing.T) *reverseProxyPreHandlerV1 {
+func newTestReverseProxyPreHandlerV1(_ *testing.T, executionMode apidef.GraphQLExecutionMode) *reverseProxyPreHandlerV1 {
 	return &reverseProxyPreHandlerV1{
 		apiDefinition: &apidef.APIDefinition{
 			GraphQL: apidef.GraphQLConfig{
 				Enabled:       true,
-				ExecutionMode: apidef.GraphQLExecutionModeProxyOnly,
+				ExecutionMode: executionMode,
 			},
 		},
 		httpClient: &http.Client{},