Skip to content

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

Merged
rhianeKobar merged 13 commits into
masterfrom
TT-11655/Graphql-APIs-are-unable-to-handle-OPTIONS-requests
Apr 22, 2024
Merged

[TT-11655] Graphql APIs are unable to handle OPTIONS requests#6221
rhianeKobar merged 13 commits into
masterfrom
TT-11655/Graphql-APIs-are-unable-to-handle-OPTIONS-requests

Conversation

@rhianeKobar

@rhianeKobar rhianeKobar commented Apr 10, 2024 •
edited by github-actions Bot
Loading

Copy link
Copy Markdown
Contributor

User description

Description

Links to this ticket

Related Issue

Motivation and Context

How This Has Been Tested

Screenshots (if appropriate)

Types of changes

  • 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

  • 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

Relevant files
Bug fix
graphql_go_tools_v1.go
Allow OPTIONS Requests for CORS Preflight in GraphQL         

internal/graphengine/graphql_go_tools_v1.go

  • Removed an error that prevented OPTIONS requests from being handled,
    allowing for CORS preflight requests to be processed.
    		•  +1/-2     

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

    @github-actions

    github-actions Bot commented Apr 10, 2024

    Copy link
    Copy Markdown
    Contributor

    PR Description updated to latest commit (4b7a69b)

    @github-actions

    github-actions Bot commented Apr 10, 2024

    Copy link
    Copy Markdown
    Contributor

    API Changes

    no api changes detected

    @github-actions

    github-actions Bot commented Apr 10, 2024

    Copy link
    Copy Markdown
    Contributor

    PR Review

    ⏱️ Estimated effort to review [1-5]

    2, because the change is small and localized but requires understanding of the context and potential side effects on CORS handling and WebSocket upgrades.

    🧪 Relevant tests

    No

    🔍 Possible issues

    Missing Error Handling: Removing the error for CORS preflight requests without proper handling might introduce issues with CORS policy enforcement.

    🔒 Security concerns

    No
    Code feedback:
    relevant fileinternal/graphengine/graphql_go_tools_v1.go
    suggestion      

    Consider implementing a more explicit handling or logging for CORS preflight requests to ensure that CORS policies are correctly enforced and to aid in debugging. [important]
    relevant linereturn ReverseProxyTypeNone, nil
    ✨ Review tool usage guide:

    Overview:
    The review tool scans the PR code changes, and generates a PR review which includes several types of feedbacks, such as possible PR issues, security threats and relevant test in the PR. More feedbacks can be added by configuring the tool.

    The tool can be triggered automatically every time a new PR is opened, or can be invoked manually by commenting on any PR.

    • When commenting, to edit configurations related to the review tool (pr_reviewer section), use the following template:
    /review --pr_reviewer.some_config1=... --pr_reviewer.some_config2=...

    [pr_reviewer]
some_config1=...
some_config2=...

    See the review usage page for a comprehensive guide on using this tool.

    @github-actions

    github-actions Bot commented Apr 10, 2024

    Copy link
    Copy Markdown
    Contributor

    PR Code Suggestions

    CategorySuggestions                                                                                                                                                       
    Enhancement
    Add logging for better debugging of CORS preflight request handling.

    Consider adding logging before returning ReverseProxyTypeNone for CORS preflight requests
    when params.NeedsEngine is true. This will help in debugging and understanding the flow of
    requests, especially when CORS preflight requests are being handled differently.

    internal/graphengine/graphql_go_tools_v1.go [498]

    +log.Debug("Handling CORS preflight request with ReverseProxyTypeNone due to params.NeedsEngine being true")
 return ReverseProxyTypeNone, nil
    ✨ Improve tool usage guide:

    Overview:
    The improve tool scans the PR code changes, and automatically generates suggestions for improving the PR code. The tool can be triggered automatically every time a new PR is opened, or can be invoked manually by commenting on a PR.

    • When commenting, to edit configurations related to the improve tool (pr_code_suggestions section), use the following template:
    /improve --pr_code_suggestions.some_config1=... --pr_code_suggestions.some_config2=...

    [pr_code_suggestions]
some_config1=...
some_config2=...

    See the improve usage page for a comprehensive guide on using this tool.

    @github-actions

    github-actions Bot commented Apr 10, 2024

    Copy link
    Copy Markdown
    Contributor

    💥 CI tests failed 🙈

    git-state

    all ok

    Please look at the run or in the Checks tab.

    @github-actions

    github-actions Bot commented Apr 10, 2024

    Copy link
    Copy Markdown
    Contributor

    💥 CI tests failed 🙈

    git-state

    all ok

    Please look at the run or in the Checks tab.

    @github-actions

    github-actions Bot commented Apr 17, 2024

    Copy link
    Copy Markdown
    Contributor

    💥 CI tests failed 🙈

    git-state

    all ok

    Please look at the run or in the Checks tab.

    1 similar comment
    @github-actions

    github-actions Bot commented Apr 17, 2024

    Copy link
    Copy Markdown
    Contributor

    💥 CI tests failed 🙈

    git-state

    all ok

    Please look at the run or in the Checks tab.

    @github-actions

    github-actions Bot commented Apr 17, 2024

    Copy link
    Copy Markdown
    Contributor

    💥 CI tests failed 🙈

    git-state

    all ok

    Please look at the run or in the Checks tab.

    1 similar comment
    @github-actions

    github-actions Bot commented Apr 17, 2024

    Copy link
    Copy Markdown
    Contributor

    💥 CI tests failed 🙈

    git-state

    all ok

    Please look at the run or in the Checks tab.

    @rhianeKobar
    modify gql go tools tests
    559b5d6
    @rhianeKobar
    Merge remote-tracking branch 'origin/TT-11655/Graphql-APIs-are-unable…
    4aa8c38 
    …-to-handle-OPTIONS-requests' into TT-11655/Graphql-APIs-are-unable-to-handle-OPTIONS-requests
    @github-actions

    github-actions Bot commented Apr 19, 2024

    Copy link
    Copy Markdown
    Contributor

    💥 CI tests failed 🙈

    git-state

    diff --git a/internal/graphengine/engine.go b/internal/graphengine/engine.go
index 71ca0b8..b3a0152 100644
--- a/internal/graphengine/engine.go
+++ b/internal/graphengine/engine.go
@@ -6,8 +6,9 @@ import (
 	"io"
 	"net/http"
 
-	"github.com/TykTechnologies/tyk/apidef"
 	"github.com/gorilla/websocket"
+
+	"github.com/TykTechnologies/tyk/apidef"
 )
 
 type EngineVersion int

    Please look at the run or in the Checks tab.

    @github-actions

    github-actions Bot commented Apr 22, 2024

    Copy link
    Copy Markdown
    Contributor

    💥 CI tests failed 🙈

    git-state

    diff --git a/internal/graphengine/engine.go b/internal/graphengine/engine.go
index 71ca0b8..b3a0152 100644
--- a/internal/graphengine/engine.go
+++ b/internal/graphengine/engine.go
@@ -6,8 +6,9 @@ import (
 	"io"
 	"net/http"
 
-	"github.com/TykTechnologies/tyk/apidef"
 	"github.com/gorilla/websocket"
+
+	"github.com/TykTechnologies/tyk/apidef"
 )
 
 type EngineVersion int

    Please look at the run or in the Checks tab.

    @rhianeKobar
    make linter happy
    693d5ac
    @rhianeKobar
    Merge remote-tracking branch 'origin/TT-11655/Graphql-APIs-are-unable…
    c81d28e 
    …-to-handle-OPTIONS-requests' into TT-11655/Graphql-APIs-are-unable-to-handle-OPTIONS-requests
    @rhianeKobar
    make sonarcloud happy
    dc0d6bc
    @sonarqubecloud

    sonarqubecloud Bot commented Apr 22, 2024

    Copy link
    Copy Markdown

    Quality Gate Failed Quality Gate failed

    Failed conditions
    5.7% Duplication on New Code (required ≤ 3%)

    See analysis details on SonarCloud

    @rhianeKobar
    rhianeKobar merged commit 3e76542 into master Apr 22, 2024
    @rhianeKobar
    rhianeKobar deleted the TT-11655/Graphql-APIs-are-unable-to-handle-OPTIONS-requests branch April 22, 2024 08:57
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Reviewers

    @pvormste pvormste pvormste approved these changes

    Assignees

    No one assigned

    Labels

    Bug fix Review effort [1-5]: 2

    Projects

    None yet

    Milestone

    No milestone

    Development

    Successfully merging this pull request may close these issues.

    2 participants

    @rhianeKobar @pvormste