[TT-10273] CORS check should be performed after API Version check #7179
Conversation
|
I'm a bot and I 👍 this PR title. 🤖 |
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
|
|
API Changes --- prev.txt 2025-07-17 12:39:26.140839266 +0000
+++ current.txt 2025-07-17 12:39:16.489822435 +0000
@@ -9096,6 +9096,19 @@
}
BundleSaver is an interface used by bundle saver structures.
+type CORSMiddleware struct {
+ *BaseMiddleware
+ // Has unexported fields.
+}
+
+func (c *CORSMiddleware) EnabledForSpec() bool
+
+func (c *CORSMiddleware) Init()
+
+func (c *CORSMiddleware) Name() string
+
+func (c *CORSMiddleware) ProcessRequest(w http.ResponseWriter, r *http.Request, _ interface{}) (error, int)
+
type CertificateCheckMW struct {
*BaseMiddleware
} |
PR Code Suggestions ✨Explore these optional code suggestions:
|
…d-after-api-version-check
…d-after-api-version-check
…d-after-api-version-check
…d-after-api-version-check
…d-after-api-version-check
|
/probe review it pls. Check for any change will break existing behaviour outside the ticket description |
…d-after-api-version-check
CORS Middleware Refactoring AnalysisThis PR addresses an issue with CORS (Cross-Origin Resource Sharing) handling in the Tyk API Gateway, specifically for versioned OAS APIs. The problem was that CORS middleware was being applied at the router level, which executed before the API Version Check middleware. This meant that for versioned APIs, the base API's CORS configuration was always used, regardless of any version-specific CORS configurations. Key Changes
Benefits
Potential Considerations
Impact on Existing BehaviorThe PR changes how CORS is applied in the middleware chain, which could potentially affect existing APIs that rely on the current behavior. However, the change is designed to fix a bug where version-specific CORS configurations were not being respected, so the impact should be positive for most users. For non-versioned APIs, the behavior should remain the same, as the CORS middleware will still be applied, just at a different point in the chain. For versioned APIs, this change ensures that the correct version-specific CORS configuration is used. The OAuth endpoint handling has also been updated to conditionally apply CORS, which should provide more consistent behavior across all API endpoints. Overall, this PR represents a well-structured refactoring that addresses a specific issue with CORS handling in versioned APIs, with comprehensive tests to ensure the new implementation works correctly. Tip: Mention me again using |
CORS Middleware Execution Order FixThis PR addresses an important issue with CORS handling in Tyk Gateway, particularly for versioned APIs. The problem was that CORS middleware was being applied at the router level, which meant it executed before the API Version Check middleware in the chain. This caused versioned Tyk OAS APIs to always use the base API's CORS configuration, regardless of any version-specific CORS configurations. Key Changes
This implementation ensures that CORS checks are performed after API version checks, allowing version-specific CORS configurations to be correctly applied. The change maintains backward compatibility while fixing the issue for versioned APIs. Potential ImpactThe PR should not break existing behavior for non-versioned APIs, as it simply moves the CORS handling from the router level to the middleware chain. For versioned APIs, it will now correctly apply version-specific CORS configurations, which is the intended behavior. The changes are well-tested with both unit tests and integration tests, covering various scenarios including preflight requests, regular requests, and OAuth endpoints with CORS. Tip: Mention me again using |
Analysis of PR #7179: CORS Check Should Be Performed After API Version CheckThis PR addresses an important issue with CORS handling in the Tyk API Gateway, specifically focusing on the order of middleware execution for versioned APIs. The problem was that CORS middleware was being applied at the router level, which executes before the API Version Check middleware in the chain. This meant that for versioned Tyk OAS APIs, the CORS configuration from the base API was always used, regardless of any version-specific CORS configurations. Key Changes
Impact and Benefits
Technical Implementation DetailsThe implementation moves CORS handling from being a router-level middleware to a standard middleware in the chain:
This PR successfully resolves the issue by ensuring that CORS checks are performed after API version checks, allowing for proper version-specific CORS configurations to be applied. Tip: Mention me again using |
CORS Middleware Refactoring AnalysisThis PR addresses an issue with CORS handling in Tyk Gateway, particularly for versioned OAS APIs. The problem was that CORS checks were being performed at the router level, which executed before the API Version Check middleware. This meant that for versioned APIs, the CORS configuration from the base API was always used, regardless of any version-specific CORS configurations. Key Changes
BenefitsThis refactoring ensures that version-specific CORS configurations are properly respected in versioned Tyk OAS APIs. By moving CORS handling from the router level to the middleware chain, the system now correctly applies the CORS configuration associated with the specific API version being accessed, rather than always using the base API's configuration. The implementation is clean and follows the established middleware pattern in Tyk, making it maintainable and consistent with the rest of the codebase. Tip: Mention me again using |
…d-after-api-version-check
CORS Middleware Execution Order Fix AnalysisThis PR addresses an important issue with CORS handling in the Tyk Gateway, particularly for versioned APIs. The problem was that CORS middleware was being applied at the router level, which executed before the API Version Check middleware. This meant that for versioned Tyk OAS APIs, the CORS configuration from the base API was always used, regardless of any version-specific CORS configurations. Key Changes1. Dedicated CORS MiddlewareThe PR introduces a new type CORSMiddleware struct {
*BaseMiddleware
corsHandler *cors.Cors
}This middleware initializes a CORS handler with the API spec's CORS configuration and handles CORS requests as part of the middleware chain. For preflight OPTIONS requests, it returns early with 2. Changed Middleware OrderingThe PR changes where CORS handling occurs in the middleware chain:
This ensures that version-specific CORS configurations are respected since the version check happens first. 3. OAuth Endpoint HandlingThe PR adds a func createCORSWrapper(spec *APISpec) func(http.HandlerFunc) http.HandlerFunc {
if !spec.CORS.Enable {
return func(h http.HandlerFunc) http.HandlerFunc {
return h
}
}
return func(h http.HandlerFunc) http.HandlerFunc {
return cors.New(corsConfig(spec)).HandlerFunc(h)
}
}OAuth endpoints are now wrapped with CORS only when it's enabled for the API, ensuring consistent behavior across all endpoints. 4. Comprehensive TestingThe PR includes extensive tests for the new CORS middleware, covering both unit tests and integration tests to verify correct behavior in various scenarios. Impact AssessmentThis change ensures that version-specific CORS configurations are properly applied for versioned APIs. The implementation maintains backward compatibility while fixing the issue:
The PR doesn't introduce any breaking changes to existing behavior outside the ticket description. It simply ensures that CORS checks are performed after API version checks, which is the expected behavior. The implementation is clean, well-tested, and follows the established middleware pattern in Tyk, making it maintainable and consistent with the rest of the codebase. Tip: Mention me again using |
CORS Middleware Refactoring AnalysisThis PR addresses an important issue with CORS (Cross-Origin Resource Sharing) handling in the Tyk API Gateway, specifically for versioned OAS APIs. The problem was that CORS middleware was being applied at the router level, which executed before the API Version Check middleware in the chain. This meant that for versioned Tyk OAS APIs, the CORS configuration from the base API was always used, regardless of any version-specific CORS configurations. Key Changes1. Moved CORS Handling from Router Level to Middleware ChainBefore:
After:
2. New CORSMiddleware ImplementationThe new
3. Updated OAuth Endpoint Handling
4. Comprehensive Test Coverage
Benefits
This implementation ensures that CORS checks are performed after API version checks, allowing version-specific CORS configurations to be correctly applied. The change maintains backward compatibility while fixing the issue for versioned APIs. Tip: Mention me again using |
CORS Middleware Refactoring AnalysisThis PR addresses an important issue with CORS (Cross-Origin Resource Sharing) handling in the Tyk API Gateway, particularly for versioned APIs. The problem was that CORS middleware was being applied at the router level, which executed before the API Version Check middleware. This meant that for versioned Tyk OAS APIs, the CORS configuration from the base API was always used, regardless of any version-specific CORS configurations. Key Changes
Benefits
This implementation ensures that CORS checks are performed after API version checks, allowing version-specific CORS configurations to be correctly applied. The change maintains backward compatibility while fixing the issue for versioned APIs. Tip: Mention me again using |
CORS Middleware Execution Order FixIssue OverviewThis PR addresses an important bug in how CORS (Cross-Origin Resource Sharing) is handled in the Tyk API Gateway, particularly for versioned APIs. The problem was that CORS middleware was being applied at the router level, which meant it executed before the API Version Check middleware in the chain. As a result, for versioned Tyk OAS APIs, the CORS configuration from the base API was always used, regardless of any version-specific CORS configurations. Key Changes1. Removed Router-Level CORS MiddlewareThe PR removes the router-level CORS middleware application from the // Old implementation (removed)
if spec.CORS.Enable {
c := cors.New(cors.Options{
AllowedOrigins: spec.CORS.AllowedOrigins,
AllowedMethods: spec.CORS.AllowedMethods,
AllowedHeaders: spec.CORS.AllowedHeaders,
ExposedHeaders: spec.CORS.ExposedHeaders,
AllowCredentials: spec.CORS.AllowCredentials,
MaxAge: spec.CORS.MaxAge,
OptionsPassthrough: spec.CORS.OptionsPassthrough,
Debug: spec.CORS.Debug,
})
router.Use(c.Handler)
}2. Created Dedicated CORS MiddlewareA new type CORSMiddleware struct {
*BaseMiddleware
corsHandler *cors.Cors
}
func (c *CORSMiddleware) Name() string {
return "CORSMiddleware"
}
func (c *CORSMiddleware) EnabledForSpec() bool {
return c.Spec.CORS.Enable
}
func (c *CORSMiddleware) Init() {
c.corsHandler = cors.New(cors.Options{
AllowedOrigins: c.Spec.CORS.AllowedOrigins,
AllowedMethods: c.Spec.CORS.AllowedMethods,
AllowedHeaders: c.Spec.CORS.AllowedHeaders,
ExposedHeaders: c.Spec.CORS.ExposedHeaders,
AllowCredentials: c.Spec.CORS.AllowCredentials,
MaxAge: c.Spec.CORS.MaxAge,
OptionsPassthrough: c.Spec.CORS.OptionsPassthrough,
Debug: c.Spec.CORS.Debug,
})
}
func (c *CORSMiddleware) ProcessRequest(w http.ResponseWriter, r *http.Request, _ interface{}) (error, int) {
c.corsHandler.HandlerFunc(w, r)
if r.Method == http.MethodOptions && !c.Spec.CORS.OptionsPassthrough {
return nil, middleware.StatusRespond
}
return nil, http.StatusOK
}3. Added CORS Middleware to Chain After Version CheckThe new CORS middleware is now added to the middleware chain immediately after the VersionCheck middleware in if enableVersionCheck {
chainArray = append(chainArray, gw.createMiddleware(&VersionCheck{BaseMiddleware: &gw.BaseMiddleware}))
}
if spec.CORS.Enable {
chainArray = append(chainArray, gw.createMiddleware(&CORSMiddleware{BaseMiddleware: &gw.BaseMiddleware}))
}4. Updated OAuth Endpoint HandlersThe PR also updates how CORS is applied to OAuth endpoints by creating a func createCORSWrapper(spec *APISpec) func(handler http.HandlerFunc) http.HandlerFunc {
var corsHandler func(http.Handler) http.Handler
if spec.CORS.Enable {
corsHandler = cors.New(cors.Options{
AllowedOrigins: spec.CORS.AllowedOrigins,
AllowedMethods: spec.CORS.AllowedMethods,
AllowedHeaders: spec.CORS.AllowedHeaders,
ExposedHeaders: spec.CORS.ExposedHeaders,
AllowCredentials: spec.CORS.AllowCredentials,
MaxAge: spec.CORS.MaxAge,
OptionsPassthrough: spec.CORS.OptionsPassthrough,
Debug: spec.CORS.Debug,
}).Handler
}
return func(handler http.HandlerFunc) http.HandlerFunc {
if corsHandler == nil {
return handler
}
return func(w http.ResponseWriter, r *http.Request) {
corsHandler(handler).ServeHTTP(w, r)
}
}
}This function is then used to wrap OAuth endpoint handlers: wrapWithCORS := createCORSWrapper(spec)
muxer.Handle(apiAuthorizePath, gw.checkIsAPIOwner(allowMethods(oauthHandlers.HandleGenerateAuthCodeData, "POST")))
muxer.HandleFunc(clientAuthPath, wrapWithCORS(allowMethods(oauthHandlers.HandleAuthorizePassthrough, "GET", "POST")))
muxer.HandleFunc(clientAccessPath, wrapWithCORS(addSecureAndCacheHeaders(allowMethods(oauthHandlers.HandleAccessRequest, "GET", "POST"))))
muxer.HandleFunc(revokeToken, wrapWithCORS(oauthHandlers.HandleRevokeToken))
muxer.HandleFunc(revokeAllTokens, wrapWithCORS(oauthHandlers.HandleRevokeAllTokens))5. Added Comprehensive TestsThe PR adds comprehensive tests for the new CORS middleware in
Benefits and Impact
Potential Considerations
ConclusionThis PR successfully addresses the issue of CORS checks being performed before API version checks by refactoring the CORS handling into a dedicated middleware and positioning it correctly in the middleware chain. The implementation follows Tyk's middleware pattern, maintains backward compatibility, and includes comprehensive tests to ensure correct behavior. Tip: Mention me again using |
CORS Middleware Execution Order Fix AnalysisThis PR addresses an important issue with CORS (Cross-Origin Resource Sharing) handling in the Tyk API Gateway, particularly for versioned APIs. The problem was that CORS middleware was being applied at the router level, which executed before the API Version Check middleware in the chain. This meant that for versioned Tyk OAS APIs, the CORS configuration from the base API was always used, regardless of any version-specific CORS configurations. Key Changes1. Moved CORS from Router Level to Middleware ChainBefore:
After:
2. New CORSMiddleware ImplementationThe new
3. Updated OAuth Endpoint Handling
4. Comprehensive Test Coverage
Benefits
This implementation ensures that CORS checks are performed after API version checks, allowing version-specific CORS configurations to be correctly applied. The change maintains backward compatibility while fixing the issue for versioned APIs. Tip: Mention me again using |
CORS Middleware Execution Order FixThis PR addresses an important issue with CORS (Cross-Origin Resource Sharing) handling in the Tyk API Gateway, particularly for versioned APIs. The problem was that CORS middleware was being applied at the router level, which executed before the API Version Check middleware in the chain. This meant that for versioned Tyk OAS APIs, the CORS configuration from the base API was always used, regardless of any version-specific CORS configurations. Key Changes1. Removed Router-Level CORS MiddlewareThe PR removes the router-level CORS middleware application from the 2. Created Dedicated CORS MiddlewareA new type CORSMiddleware struct {
*BaseMiddleware
corsHandler *cors.Cors
}This middleware initializes a CORS handler with the API spec's CORS configuration and handles CORS requests as part of the middleware chain. For preflight OPTIONS requests, it returns early with 3. Changed Middleware OrderingThe new CORS middleware is now added to the middleware chain immediately after the VersionCheck middleware: gw.mwAppendEnabled(&chainArray, &VersionCheck{BaseMiddleware: baseMid.Copy()})
gw.mwAppendEnabled(&chainArray, &CORSMiddleware{BaseMiddleware: baseMid.Copy()})This ensures that version-specific CORS configurations are respected since the version check happens first. 4. Updated OAuth Endpoint HandlingThe PR adds a func createCORSWrapper(spec *APISpec) func(http.HandlerFunc) http.HandlerFunc {
// Only create CORS handler if CORS is enabled
var corsHandler func(http.Handler) http.Handler
if spec.CORS.Enable {
corsHandler = cors.New(cors.Options{...}).Handler
}
return func(handler http.HandlerFunc) http.HandlerFunc {
if corsHandler == nil {
return handler
}
return func(w http.ResponseWriter, r *http.Request) {
corsHandler(handler).ServeHTTP(w, r)
}
}
}OAuth endpoints are now wrapped with CORS only when it's enabled for the API, ensuring consistent behavior across all endpoints. Benefits
This implementation ensures that CORS checks are performed after API version checks, allowing version-specific CORS configurations to be correctly applied. The change maintains backward compatibility while fixing the issue for versioned APIs. Tip: Mention me again using |
CORS Middleware Execution Order FixThis PR addresses an important issue with CORS (Cross-Origin Resource Sharing) handling in the Tyk API Gateway, particularly for versioned APIs. The problem was that CORS middleware was being applied at the router level, which executed before the API Version Check middleware in the chain. This meant that for versioned Tyk OAS APIs, the CORS configuration from the base API was always used, regardless of any version-specific CORS configurations. Key Changes1. Removed Router-Level CORS MiddlewareThe PR removes the router-level CORS middleware application from the 2. Created Dedicated CORS MiddlewareA new type CORSMiddleware struct {
*BaseMiddleware
corsHandler *cors.Cors
}
func (c *CORSMiddleware) ProcessRequest(w http.ResponseWriter, r *http.Request, _ interface{}) (error, int) {
c.corsHandler.HandlerFunc(w, r)
if r.Method == http.MethodOptions && !c.Spec.CORS.OptionsPassthrough {
return nil, middleware.StatusRespond
}
return nil, http.StatusOK
}3. Changed Middleware OrderingThe new CORS middleware is now added to the middleware chain immediately after the Version Check middleware: gw.mwAppendEnabled(&chainArray, &VersionCheck{BaseMiddleware: baseMid.Copy()})
gw.mwAppendEnabled(&chainArray, &CORSMiddleware{BaseMiddleware: baseMid.Copy()})This ensures that version-specific CORS configurations are respected since the version check happens first. 4. Updated OAuth Endpoint HandlingThe PR adds a func createCORSWrapper(spec *APISpec) func(handler http.HandlerFunc) http.HandlerFunc {
// Only create CORS handler if CORS is enabled
if spec.CORS.Enable {
// Initialize CORS handler with API's CORS configuration
}
// Return a wrapper function that either applies CORS or passes through
}OAuth endpoints are now wrapped with CORS only when it's enabled for the API, ensuring consistent behavior across all endpoints. 5. Comprehensive TestingThe PR includes extensive tests for the new CORS middleware in
Benefits
This implementation ensures that CORS checks are performed after API version checks, allowing version-specific CORS configurations to be correctly applied. The change maintains backward compatibility while fixing the issue for versioned APIs. Tip: Mention me again using |
CORS Middleware Execution Order FixIssue OverviewThis PR addresses an important issue with CORS (Cross-Origin Resource Sharing) handling in the Tyk API Gateway, particularly for versioned APIs. The problem was that CORS middleware was being applied at the router level, which executed before the API Version Check middleware in the chain. This meant that for versioned Tyk OAS APIs, the CORS configuration from the base API was always used, regardless of any version-specific CORS configurations. Key Changes1. Removed Router-Level CORS MiddlewarePreviously, CORS was applied at the router level in the if spec.CORS.Enable {
c := cors.New(cors.Options{
AllowedOrigins: spec.CORS.AllowedOrigins,
AllowedMethods: spec.CORS.AllowedMethods,
AllowedHeaders: spec.CORS.AllowedHeaders,
ExposedHeaders: spec.CORS.ExposedHeaders,
AllowCredentials: spec.CORS.AllowCredentials,
MaxAge: spec.CORS.MaxAge,
OptionsPassthrough: spec.CORS.OptionsPassthrough,
Debug: spec.CORS.Debug,
})
router.Use(c.Handler)
}This code has been completely removed from 2. Added Dedicated CORS MiddlewareA new type CORSMiddleware struct {
*BaseMiddleware
corsHandler *cors.Cors
}
func (c *CORSMiddleware) ProcessRequest(w http.ResponseWriter, r *http.Request, _ interface{}) (error, int) {
c.corsHandler.HandlerFunc(w, r)
if r.Method == http.MethodOptions && !c.Spec.CORS.OptionsPassthrough {
return nil, middleware.StatusRespond
}
return nil, http.StatusOK
}This middleware handles CORS preflight requests and applies CORS headers to regular requests. For preflight OPTIONS requests, it returns early with 3. Changed Middleware OrderingThe new CORS middleware is now added to the middleware chain after the Version Check middleware: if enableVersionCheck {
chainArray = append(chainArray, gw.createMiddleware(&VersionCheck{BaseMiddleware: baseMid.Copy()}))
}
if spec.CORS.Enable {
chainArray = append(chainArray, gw.createMiddleware(&CORSMiddleware{BaseMiddleware: baseMid.Copy()}))
}This ensures that version-specific CORS configurations are respected, as the version check happens first. 4. Updated OAuth Endpoint HandlingA func createCORSWrapper(spec *APISpec) func(http.HandlerFunc) http.HandlerFunc {
var corsHandler func(http.Handler) http.Handler
if spec.CORS.Enable {
corsHandler = cors.New(cors.Options{
AllowedOrigins: spec.CORS.AllowedOrigins,
AllowedMethods: spec.CORS.AllowedMethods,
AllowedHeaders: spec.CORS.AllowedHeaders,
ExposedHeaders: spec.CORS.ExposedHeaders,
AllowCredentials: spec.CORS.AllowCredentials,
MaxAge: spec.CORS.MaxAge,
OptionsPassthrough: spec.CORS.OptionsPassthrough,
Debug: spec.CORS.Debug,
}).Handler
}
return func(handler http.HandlerFunc) http.HandlerFunc {
if corsHandler == nil {
return handler
}
return func(w http.ResponseWriter, r *http.Request) {
corsHandler(handler).ServeHTTP(w, r)
}
}
}OAuth endpoints are now wrapped with CORS only when it's enabled for the API: wrapWithCORS := createCORSWrapper(spec)
muxer.HandleFunc(clientAuthPath, wrapWithCORS(allowMethods(oauthHandlers.HandleAuthorizePassthrough, "GET", "POST")))
muxer.HandleFunc(clientAccessPath, wrapWithCORS(addSecureAndCacheHeaders(allowMethods(oauthHandlers.HandleAccessRequest, "GET", "POST"))))
muxer.HandleFunc(revokeToken, wrapWithCORS(oauthHandlers.HandleRevokeToken))
muxer.HandleFunc(revokeAllTokens, wrapWithCORS(oauthHandlers.HandleRevokeAllTokens))5. Comprehensive TestingThe PR includes comprehensive tests for the new CORS middleware:
Benefits
ImpactThis change ensures that CORS checks are performed after API version checks, allowing version-specific CORS configurations to be correctly applied. The implementation maintains backward compatibility while fixing the issue for versioned APIs. For non-versioned APIs, the behavior remains the same - CORS is still applied, just at a different point in the chain. For versioned APIs, the correct version-specific CORS configuration will now be used. The PR doesn't introduce any breaking changes to existing behavior outside the ticket description. It simply ensures that CORS checks are performed after API version checks, which is the expected behavior. Tip: Mention me again using |
…d-after-api-version-check
CORS Middleware Refactoring AnalysisThis PR addresses an important issue with CORS handling in Tyk Gateway, particularly for versioned OAS APIs. The problem was that CORS middleware was being applied at the router level, which executed before the API version check middleware in the chain. This caused version-specific CORS configurations to be ignored, as the base API's CORS configuration was always used regardless of the API version. Key Changes1. Dedicated CORS Middleware Implementation
func (c *CORSMiddleware) ProcessRequest(w http.ResponseWriter, r *http.Request, _ interface{}) (error, int) {
c.corsHandler.HandlerFunc(w, r)
if r.Method == http.MethodOptions && !c.Spec.CORS.OptionsPassthrough {
return nil, middleware.StatusRespond
}
return nil, http.StatusOK
}2. Middleware Chain Reordering
gw.mwAppendEnabled(&chainArray, &VersionCheck{BaseMiddleware: baseMid.Copy()})
gw.mwAppendEnabled(&chainArray, &CORSMiddleware{BaseMiddleware: baseMid.Copy()})3. OAuth Endpoint Handling
4. Test Improvements
Benefits
This refactoring ensures that CORS handling works correctly with API versioning, particularly for OAS APIs, allowing API developers to define different CORS configurations for different API versions. Tip: Mention me again using |
CORS Middleware Refactoring AnalysisThis PR addresses an important issue with CORS handling in Tyk OAS APIs by moving CORS processing from the router level to a dedicated middleware in the chain. Current Implementation IssuesThe current implementation applies CORS middleware at the router level by wrapping the handler: // In api_loader.go
if len(spec.CORS.AllowedOrigins) > 0 {
c := cors.New(cors.Options{...})
chainObj.ThisHandler = c.Handler(chainObj.ThisHandler)
}This causes a critical problem: CORS checks execute before the API Version Check middleware, meaning that for versioned Tyk OAS APIs, the CORS configuration from the base API is always used, regardless of any version-specific CORS configurations. Solution OverviewThe PR implements these key changes:
Benefits
This change ensures that APIs with versioning can have different CORS configurations per version, which is essential for APIs that need to support different cross-origin access patterns across versions. Tip: Mention me again using |
PR Review: CORS Check Should Be Performed After API Version CheckThis PR addresses an important issue with CORS handling in Tyk OAS APIs. The current implementation applies CORS middleware at the router level, which executes before the API Version Check middleware in the chain. This means that for versioned Tyk OAS APIs, the CORS configuration from the base API is always used, regardless of any version-specific CORS configurations. Key Changes
Technical AnalysisThe core issue was that CORS handling was being applied at the router level before any version-specific context was established. By moving CORS handling into the middleware chain after version checking, the PR ensures that the correct version-specific CORS configuration is used. The implementation is clean and follows Tyk's middleware patterns. The The tests are comprehensive and cover various scenarios, including:
Impact and BenefitsThis change ensures that:
The PR maintains backward compatibility while fixing the issue, and the comprehensive test suite ensures the changes work as expected. ConclusionThis is a well-implemented fix for an important issue affecting versioned OAS APIs. The changes are focused, well-tested, and follow Tyk's architectural patterns. The PR should be approved. Tip: Mention me again using |
Dependency Impact Review: CORS Middleware RefactoringSummary of ChangesThis PR addresses an important issue with CORS handling in Tyk OAS APIs. The current implementation applies CORS middleware at the router level, which executes before the API Version Check middleware. This means that for versioned Tyk OAS APIs, the CORS configuration from the base API is always used, regardless of any version-specific CORS configurations. The solution refactors CORS handling into a dedicated middleware that's placed in the middleware chain after the version check, ensuring that version-specific CORS configurations are properly applied. Technical ImplementationThe PR makes three key changes:
Test CoverageThe PR includes comprehensive tests in
Dependency Impact AnalysisThis change has minimal impact on dependencies:
Security ConsiderationsThe PR maintains the same security properties as the original implementation:
Performance ImpactThe performance impact should be negligible:
ConclusionThis PR correctly addresses the issue of CORS handling for versioned APIs by moving CORS processing from the router level to a dedicated middleware in the chain after version checking. The implementation is clean, well-tested, and follows established patterns in the codebase. The change is focused on fixing a specific issue without introducing unnecessary complexity or dependencies. I recommend approving this PR as it properly fixes the issue with minimal risk. Tip: Mention me again using |
|
MaciekMis
deleted the
TT-10273-oas-cors-check-should-be-performed-after-api-version-check
branch
|
/release to release-5.8 |
|
Working on it! Note that it can take a few minutes. |
) ### **User description** <details open> <summary><a href="https://tyktech.atlassian.net/browse/TT-10273" title="TT-10273" target="_blank">TT-10273</a></summary> <br /> <table> <tr> <th>Summary</th> <td>[OAS] CORS check should be performed after API Version check</td> </tr> <tr> <th>Type</th> <td> <img alt="Bug" src="https://tyktech.atlassian.net/rest/api/2/universal_avatar/view/type/issuetype/avatar/10303?size=medium" /> Bug </td> </tr> <tr> <th>Status</th> <td>In Dev</td> </tr> <tr> <th>Points</th> <td>N/A</td> </tr> <tr> <th>Labels</th> <td><a href="https://tyktech.atlassian.net/issues?jql=project%20%3D%20TT%20AND%20labels%20%3D%20codilime_refined%20ORDER%20BY%20created%20DESC" title="codilime_refined">codilime_refined</a></td> </tr> </table> </details> <!-- do not remove this marker as it will break jira-lint's functionality. added_by_jira_lint --> --- <!-- Provide a general summary of your changes in the Title above --> ## Description The current implementation applies CORS middleware at the router level, which executes before the API Version Check middleware in the chain. This means that for versioned Tyk OAS APIs, the CORS configuration from the base API is always used, regardless of any version-specific CORS configurations. ## 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) - [x] 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 ___ ### **PR Type** Bug fix, Enhancement, Tests ___ ### **Description** - Refactored CORS handling into a dedicated middleware for correct execution order - Ensured CORS checks occur after API version checks for OAS APIs - Updated OAuth endpoint handlers to apply CORS only when enabled - Added comprehensive unit and integration tests for the new CORS middleware ___ ### **Changes diagram** ```mermaid flowchart LR OldCORS["CORS at router level"] -- "removed" --> X1[""] VersionCheck["API Version Check Middleware"] -- "now before" --> NewCORS["CORSMiddleware"] NewCORS -- "added to middleware chain" --> APIHandler["API Handler"] OAuthEndpoints["OAuth Endpoints"] -- "wrapped with CORS if enabled" --> OAuthCORS["CORS Wrapper"] TestsOld["Old CORS tests"] -- "removed" --> X2[""] TestsNew["New CORSMiddleware tests"] -- "added" --> CORSMiddleware ``` ___ ### **Changes walkthrough** 📝 <table><thead><tr><th></th><th align="left">Relevant files</th></tr></thead><tbody><tr><td><strong>Enhancement</strong></td><td><table> <tr> <td> <details> <summary><strong>api_loader.go</strong><dd><code>Move CORS handling to middleware chain after version check</code></dd></summary> <hr> gateway/api_loader.go <li>Removed router-level CORS middleware application<br> <li> Added CORSMiddleware to the middleware chain after version check </details> </td> <td><a href="https://github.com/TykTechnologies/tyk/pull/7179/files#diff-cdf0b7f176c9d18e1a314b78ddefc2cb3a94b3de66f1f360174692c915734c68">+1/-16</a> </td> </tr> <tr> <td> <details> <summary><strong>mw_cors.go</strong><dd><code>Add CORSMiddleware implementation for CORS handling</code> </dd></summary> <hr> gateway/mw_cors.go <li>Introduced CORSMiddleware struct and logic<br> <li> Handles CORS requests as part of middleware chain<br> <li> Returns early for preflight OPTIONS requests </details> </td> <td><a href="https://github.com/TykTechnologies/tyk/pull/7179/files#diff-b83b51e5f6dc341b26ecc1ef7bd72d680f398e599eea327662e2169a3b55f208">+43/-0</a> </td> </tr> <tr> <td> <details> <summary><strong>server.go</strong><dd><code>Apply CORS wrapper to OAuth endpoints when enabled</code> </dd></summary> <hr> gateway/server.go <li>Updated OAuth endpoint handlers to wrap with CORS only if enabled<br> <li> Added createCORSWrapper utility for conditional CORS wrapping </details> </td> <td><a href="https://github.com/TykTechnologies/tyk/pull/7179/files#diff-4652d1bf175a0be8f5e61ef7177c9666f23e077d8626b73ac9d13358fa8b525b">+34/-4</a> </td> </tr> </table></td></tr><tr><td><strong>Tests</strong></td><td><table> <tr> <td> <details> <summary><strong>api_loader_test.go</strong><dd><code>Remove outdated CORS tests</code> </dd></summary> <hr> gateway/api_loader_test.go - Removed legacy CORS test function </details> </td> <td><a href="https://github.com/TykTechnologies/tyk/pull/7179/files#diff-f696545a659f4d96421b253edef4bcc8da0e7f52120b8f8866d32cbbb7cc1afc">+0/-73</a> </td> </tr> <tr> <td> <details> <summary><strong>mw_cors_test.go</strong><dd><code>Add comprehensive CORSMiddleware tests</code> </dd></summary> <hr> gateway/mw_cors_test.go <li>Added unit tests for CORSMiddleware logic<br> <li> Added integration tests for CORS behavior on APIs and OAuth endpoints<br> <li> Utility functions for CORS config and middleware creation </details> </td> <td><a href="https://github.com/TykTechnologies/tyk/pull/7179/files#diff-60316be235ab2e13eab4b17dfa535b441db3c29d388c433cca7d6fbe4d26c3a0">+157/-0</a> </td> </tr> </table></td></tr></tr></tbody></table> ___ > <details> <summary> Need help?</summary><li>Type <code>/help how to ...</code> in the comments thread for any questions about PR-Agent usage.</li><li>Check out the <a href="https://qodo-merge-docs.qodo.ai/usage-guide/">documentation</a> for more information.</li></details> (cherry picked from commit 714f6d4)
|
@MaciekMis Created merge PRs |
|
/release to release-5.9 |
|
Working on it! Note that it can take a few minutes. |
) ### **User description** <details open> <summary><a href="https://tyktech.atlassian.net/browse/TT-10273" title="TT-10273" target="_blank">TT-10273</a></summary> <br /> <table> <tr> <th>Summary</th> <td>[OAS] CORS check should be performed after API Version check</td> </tr> <tr> <th>Type</th> <td> <img alt="Bug" src="https://tyktech.atlassian.net/rest/api/2/universal_avatar/view/type/issuetype/avatar/10303?size=medium" /> Bug </td> </tr> <tr> <th>Status</th> <td>In Dev</td> </tr> <tr> <th>Points</th> <td>N/A</td> </tr> <tr> <th>Labels</th> <td><a href="https://tyktech.atlassian.net/issues?jql=project%20%3D%20TT%20AND%20labels%20%3D%20codilime_refined%20ORDER%20BY%20created%20DESC" title="codilime_refined">codilime_refined</a></td> </tr> </table> </details> <!-- do not remove this marker as it will break jira-lint's functionality. added_by_jira_lint --> --- <!-- Provide a general summary of your changes in the Title above --> ## Description The current implementation applies CORS middleware at the router level, which executes before the API Version Check middleware in the chain. This means that for versioned Tyk OAS APIs, the CORS configuration from the base API is always used, regardless of any version-specific CORS configurations. ## 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) - [x] 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 ___ ### **PR Type** Bug fix, Enhancement, Tests ___ ### **Description** - Refactored CORS handling into a dedicated middleware for correct execution order - Ensured CORS checks occur after API version checks for OAS APIs - Updated OAuth endpoint handlers to apply CORS only when enabled - Added comprehensive unit and integration tests for the new CORS middleware ___ ### **Changes diagram** ```mermaid flowchart LR OldCORS["CORS at router level"] -- "removed" --> X1[""] VersionCheck["API Version Check Middleware"] -- "now before" --> NewCORS["CORSMiddleware"] NewCORS -- "added to middleware chain" --> APIHandler["API Handler"] OAuthEndpoints["OAuth Endpoints"] -- "wrapped with CORS if enabled" --> OAuthCORS["CORS Wrapper"] TestsOld["Old CORS tests"] -- "removed" --> X2[""] TestsNew["New CORSMiddleware tests"] -- "added" --> CORSMiddleware ``` ___ ### **Changes walkthrough** 📝 <table><thead><tr><th></th><th align="left">Relevant files</th></tr></thead><tbody><tr><td><strong>Enhancement</strong></td><td><table> <tr> <td> <details> <summary><strong>api_loader.go</strong><dd><code>Move CORS handling to middleware chain after version check</code></dd></summary> <hr> gateway/api_loader.go <li>Removed router-level CORS middleware application<br> <li> Added CORSMiddleware to the middleware chain after version check </details> </td> <td><a href="https://github.com/TykTechnologies/tyk/pull/7179/files#diff-cdf0b7f176c9d18e1a314b78ddefc2cb3a94b3de66f1f360174692c915734c68">+1/-16</a> </td> </tr> <tr> <td> <details> <summary><strong>mw_cors.go</strong><dd><code>Add CORSMiddleware implementation for CORS handling</code> </dd></summary> <hr> gateway/mw_cors.go <li>Introduced CORSMiddleware struct and logic<br> <li> Handles CORS requests as part of middleware chain<br> <li> Returns early for preflight OPTIONS requests </details> </td> <td><a href="https://github.com/TykTechnologies/tyk/pull/7179/files#diff-b83b51e5f6dc341b26ecc1ef7bd72d680f398e599eea327662e2169a3b55f208">+43/-0</a> </td> </tr> <tr> <td> <details> <summary><strong>server.go</strong><dd><code>Apply CORS wrapper to OAuth endpoints when enabled</code> </dd></summary> <hr> gateway/server.go <li>Updated OAuth endpoint handlers to wrap with CORS only if enabled<br> <li> Added createCORSWrapper utility for conditional CORS wrapping </details> </td> <td><a href="https://github.com/TykTechnologies/tyk/pull/7179/files#diff-4652d1bf175a0be8f5e61ef7177c9666f23e077d8626b73ac9d13358fa8b525b">+34/-4</a> </td> </tr> </table></td></tr><tr><td><strong>Tests</strong></td><td><table> <tr> <td> <details> <summary><strong>api_loader_test.go</strong><dd><code>Remove outdated CORS tests</code> </dd></summary> <hr> gateway/api_loader_test.go - Removed legacy CORS test function </details> </td> <td><a href="https://github.com/TykTechnologies/tyk/pull/7179/files#diff-f696545a659f4d96421b253edef4bcc8da0e7f52120b8f8866d32cbbb7cc1afc">+0/-73</a> </td> </tr> <tr> <td> <details> <summary><strong>mw_cors_test.go</strong><dd><code>Add comprehensive CORSMiddleware tests</code> </dd></summary> <hr> gateway/mw_cors_test.go <li>Added unit tests for CORSMiddleware logic<br> <li> Added integration tests for CORS behavior on APIs and OAuth endpoints<br> <li> Utility functions for CORS config and middleware creation </details> </td> <td><a href="https://github.com/TykTechnologies/tyk/pull/7179/files#diff-60316be235ab2e13eab4b17dfa535b441db3c29d388c433cca7d6fbe4d26c3a0">+157/-0</a> </td> </tr> </table></td></tr></tr></tbody></table> ___ > <details> <summary> Need help?</summary><li>Type <code>/help how to ...</code> in the comments thread for any questions about PR-Agent usage.</li><li>Check out the <a href="https://qodo-merge-docs.qodo.ai/usage-guide/">documentation</a> for more information.</li></details> (cherry picked from commit 714f6d4)
|
@MaciekMis Seems like there is conflict and it require manual merge. |
…er API Version check (#7179) [TT-10273] CORS check should be performed after API Version check (#7179) ### **User description** <details open> <summary><a href="https://tyktech.atlassian.net/browse/TT-10273" title="TT-10273" target="_blank">TT-10273</a></summary> <br /> <table> <tr> <th>Summary</th> <td>[OAS] CORS check should be performed after API Version check</td> </tr> <tr> <th>Type</th> <td> <img alt="Bug" src="https://tyktech.atlassian.net/rest/api/2/universal_avatar/view/type/issuetype/avatar/10303?size=medium" /> Bug </td> </tr> <tr> <th>Status</th> <td>In Dev</td> </tr> <tr> <th>Points</th> <td>N/A</td> </tr> <tr> <th>Labels</th> <td><a href="https://tyktech.atlassian.net/issues?jql=project%20%3D%20TT%20AND%20labels%20%3D%20codilime_refined%20ORDER%20BY%20created%20DESC" title="codilime_refined">codilime_refined</a></td> </tr> </table> </details> <!-- do not remove this marker as it will break jira-lint's functionality. added_by_jira_lint --> --- <!-- Provide a general summary of your changes in the Title above --> ## Description The current implementation applies CORS middleware at the router level, which executes before the API Version Check middleware in the chain. This means that for versioned Tyk OAS APIs, the CORS configuration from the base API is always used, regardless of any version-specific CORS configurations. ## 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) - [x] 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 ___ ### **PR Type** Bug fix, Enhancement, Tests ___ ### **Description** - Refactored CORS handling into a dedicated middleware for correct execution order - Ensured CORS checks occur after API version checks for OAS APIs - Updated OAuth endpoint handlers to apply CORS only when enabled - Added comprehensive unit and integration tests for the new CORS middleware ___ ### **Changes diagram** ```mermaid flowchart LR OldCORS["CORS at router level"] -- "removed" --> X1[""] VersionCheck["API Version Check Middleware"] -- "now before" --> NewCORS["CORSMiddleware"] NewCORS -- "added to middleware chain" --> APIHandler["API Handler"] OAuthEndpoints["OAuth Endpoints"] -- "wrapped with CORS if enabled" --> OAuthCORS["CORS Wrapper"] TestsOld["Old CORS tests"] -- "removed" --> X2[""] TestsNew["New CORSMiddleware tests"] -- "added" --> CORSMiddleware ``` ___ ### **Changes walkthrough** 📝 <table><thead><tr><th></th><th align="left">Relevant files</th></tr></thead><tbody><tr><td><strong>Enhancement</strong></td><td><table> <tr> <td> <details> <summary><strong>api_loader.go</strong><dd><code>Move CORS handling to middleware chain after version check</code></dd></summary> <hr> gateway/api_loader.go <li>Removed router-level CORS middleware application<br> <li> Added CORSMiddleware to the middleware chain after version check </details> </td> <td><a href="https://github.com/TykTechnologies/tyk/pull/7179/files#diff-cdf0b7f176c9d18e1a314b78ddefc2cb3a94b3de66f1f360174692c915734c68">+1/-16</a> </td> </tr> <tr> <td> <details> <summary><strong>mw_cors.go</strong><dd><code>Add CORSMiddleware implementation for CORS handling</code> </dd></summary> <hr> gateway/mw_cors.go <li>Introduced CORSMiddleware struct and logic<br> <li> Handles CORS requests as part of middleware chain<br> <li> Returns early for preflight OPTIONS requests </details> </td> <td><a href="https://github.com/TykTechnologies/tyk/pull/7179/files#diff-b83b51e5f6dc341b26ecc1ef7bd72d680f398e599eea327662e2169a3b55f208">+43/-0</a> </td> </tr> <tr> <td> <details> <summary><strong>server.go</strong><dd><code>Apply CORS wrapper to OAuth endpoints when enabled</code> </dd></summary> <hr> gateway/server.go <li>Updated OAuth endpoint handlers to wrap with CORS only if enabled<br> <li> Added createCORSWrapper utility for conditional CORS wrapping </details> </td> <td><a href="https://github.com/TykTechnologies/tyk/pull/7179/files#diff-4652d1bf175a0be8f5e61ef7177c9666f23e077d8626b73ac9d13358fa8b525b">+34/-4</a> </td> </tr> </table></td></tr><tr><td><strong>Tests</strong></td><td><table> <tr> <td> <details> <summary><strong>api_loader_test.go</strong><dd><code>Remove outdated CORS tests</code> </dd></summary> <hr> gateway/api_loader_test.go - Removed legacy CORS test function </details> </td> <td><a href="https://github.com/TykTechnologies/tyk/pull/7179/files#diff-f696545a659f4d96421b253edef4bcc8da0e7f52120b8f8866d32cbbb7cc1afc">+0/-73</a> </td> </tr> <tr> <td> <details> <summary><strong>mw_cors_test.go</strong><dd><code>Add comprehensive CORSMiddleware tests</code> </dd></summary> <hr> gateway/mw_cors_test.go <li>Added unit tests for CORSMiddleware logic<br> <li> Added integration tests for CORS behavior on APIs and OAuth endpoints<br> <li> Utility functions for CORS config and middleware creation </details> </td> <td><a href="https://github.com/TykTechnologies/tyk/pull/7179/files#diff-60316be235ab2e13eab4b17dfa535b441db3c29d388c433cca7d6fbe4d26c3a0">+157/-0</a> </td> </tr> </table></td></tr></tr></tbody></table> ___ > <details> <summary> Need help?</summary><li>Type <code>/help how to ...</code> in the comments thread for any questions about PR-Agent usage.</li><li>Check out the <a href="https://qodo-merge-docs.qodo.ai/usage-guide/">documentation</a> for more information.</li></details>
…er API Version check (#7179) (#7242) ### **User description** [TT-10273] CORS check should be performed after API Version check (#7179) ### **User description** <details open> <summary><a href="https://tyktech.atlassian.net/browse/TT-10273" title="TT-10273" target="_blank">TT-10273</a></summary> <br /> <table> <tr> <th>Summary</th> <td>[OAS] CORS check should be performed after API Version check</td> </tr> <tr> <th>Type</th> <td> <img alt="Bug" src="https://tyktech.atlassian.net/rest/api/2/universal_avatar/view/type/issuetype/avatar/10303?size=medium" /> Bug </td> </tr> <tr> <th>Status</th> <td>In Dev</td> </tr> <tr> <th>Points</th> <td>N/A</td> </tr> <tr> <th>Labels</th> <td><a href="https://tyktech.atlassian.net/issues?jql=project%20%3D%20TT%20AND%20labels%20%3D%20codilime_refined%20ORDER%20BY%20created%20DESC" title="codilime_refined">codilime_refined</a></td> </tr> </table> </details> <!-- do not remove this marker as it will break jira-lint's functionality. added_by_jira_lint --> --- <!-- Provide a general summary of your changes in the Title above --> ## Description The current implementation applies CORS middleware at the router level, which executes before the API Version Check middleware in the chain. This means that for versioned Tyk OAS APIs, the CORS configuration from the base API is always used, regardless of any version-specific CORS configurations. ## 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) - [x] 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 ___ ### **PR Type** Bug fix, Enhancement, Tests ___ ### **Description** - Refactored CORS handling into a dedicated middleware for correct execution order - Ensured CORS checks occur after API version checks for OAS APIs - Updated OAuth endpoint handlers to apply CORS only when enabled - Added comprehensive unit and integration tests for the new CORS middleware ___ ### **Changes diagram** ```mermaid flowchart LR OldCORS["CORS at router level"] -- "removed" --> X1[""] VersionCheck["API Version Check Middleware"] -- "now before" --> NewCORS["CORSMiddleware"] NewCORS -- "added to middleware chain" --> APIHandler["API Handler"] OAuthEndpoints["OAuth Endpoints"] -- "wrapped with CORS if enabled" --> OAuthCORS["CORS Wrapper"] TestsOld["Old CORS tests"] -- "removed" --> X2[""] TestsNew["New CORSMiddleware tests"] -- "added" --> CORSMiddleware ``` ___ ### **Changes walkthrough** 📝 <table><thead><tr><th></th><th align="left">Relevant files</th></tr></thead><tbody><tr><td><strong>Enhancement</strong></td><td><table> <tr> <td> <details> <summary><strong>api_loader.go</strong><dd><code>Move CORS handling to middleware chain after version check</code></dd></summary> <hr> gateway/api_loader.go <li>Removed router-level CORS middleware application<br> <li> Added CORSMiddleware to the middleware chain after version check </details> </td> <td><a href="https://github.com/TykTechnologies/tyk/pull/7179/files#diff-cdf0b7f176c9d18e1a314b78ddefc2cb3a94b3de66f1f360174692c915734c68">+1/-16</a> </td> </tr> <tr> <td> <details> <summary><strong>mw_cors.go</strong><dd><code>Add CORSMiddleware implementation for CORS handling</code> </dd></summary> <hr> gateway/mw_cors.go <li>Introduced CORSMiddleware struct and logic<br> <li> Handles CORS requests as part of middleware chain<br> <li> Returns early for preflight OPTIONS requests </details> </td> <td><a href="https://github.com/TykTechnologies/tyk/pull/7179/files#diff-b83b51e5f6dc341b26ecc1ef7bd72d680f398e599eea327662e2169a3b55f208">+43/-0</a> </td> </tr> <tr> <td> <details> <summary><strong>server.go</strong><dd><code>Apply CORS wrapper to OAuth endpoints when enabled</code> </dd></summary> <hr> gateway/server.go <li>Updated OAuth endpoint handlers to wrap with CORS only if enabled<br> <li> Added createCORSWrapper utility for conditional CORS wrapping </details> </td> <td><a href="https://github.com/TykTechnologies/tyk/pull/7179/files#diff-4652d1bf175a0be8f5e61ef7177c9666f23e077d8626b73ac9d13358fa8b525b">+34/-4</a> </td> </tr> </table></td></tr><tr><td><strong>Tests</strong></td><td><table> <tr> <td> <details> <summary><strong>api_loader_test.go</strong><dd><code>Remove outdated CORS tests</code> </dd></summary> <hr> gateway/api_loader_test.go - Removed legacy CORS test function </details> </td> <td><a href="https://github.com/TykTechnologies/tyk/pull/7179/files#diff-f696545a659f4d96421b253edef4bcc8da0e7f52120b8f8866d32cbbb7cc1afc">+0/-73</a> </td> </tr> <tr> <td> <details> <summary><strong>mw_cors_test.go</strong><dd><code>Add comprehensive CORSMiddleware tests</code> </dd></summary> <hr> gateway/mw_cors_test.go <li>Added unit tests for CORSMiddleware logic<br> <li> Added integration tests for CORS behavior on APIs and OAuth endpoints<br> <li> Utility functions for CORS config and middleware creation </details> </td> <td><a href="https://github.com/TykTechnologies/tyk/pull/7179/files#diff-60316be235ab2e13eab4b17dfa535b441db3c29d388c433cca7d6fbe4d26c3a0">+157/-0</a> </td> </tr> </table></td></tr></tr></tbody></table> ___ > <details> <summary> Need help?</summary><li>Type <code>/help how to ...</code> in the comments thread for any questions about PR-Agent usage.</li><li>Check out the <a href="https://qodo-merge-docs.qodo.ai/usage-guide/">documentation</a> for more information.</li></details> [TT-10273]: https://tyktech.atlassian.net/browse/TT-10273?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ ___ ### **PR Type** Bug fix, Enhancement, Tests ___ ### **Description** - Refactored CORS handling into a dedicated middleware after version check - Ensured CORS checks use version-specific configuration for OAS APIs - Updated OAuth endpoint handlers to conditionally apply CORS - Added comprehensive unit and integration tests for new CORS middleware ___ ### **Changes diagram** ```mermaid flowchart LR OldCORS["CORS at router level"] -- "removed" --> X1[""] VersionCheck["API Version Check Middleware"] -- "now before" --> NewCORS["CORSMiddleware"] NewCORS -- "added to middleware chain" --> APIHandler["API Handler"] OAuthEndpoints["OAuth Endpoints"] -- "wrapped with CORS if enabled" --> OAuthCORS["CORS Wrapper"] TestsOld["Old CORS tests"] -- "removed" --> X2[""] TestsNew["New CORSMiddleware tests"] -- "added" --> CORSMiddleware ``` ___ ### **Changes walkthrough** 📝 <table><thead><tr><th></th><th align="left">Relevant files</th></tr></thead><tbody><tr><td><strong>Enhancement</strong></td><td><table> <tr> <td> <details> <summary><strong>api_loader.go</strong><dd><code>Move CORS handling to middleware chain after version check</code></dd></summary> <hr> gateway/api_loader.go <li>Removed router-level CORS middleware application<br> <li> Added CORSMiddleware to middleware chain after version check </details> </td> <td><a href="https://github.com/TykTechnologies/tyk/pull/7242/files#diff-cdf0b7f176c9d18e1a314b78ddefc2cb3a94b3de66f1f360174692c915734c68">+1/-16</a> </td> </tr> <tr> <td> <details> <summary><strong>mw_cors.go</strong><dd><code>Add CORSMiddleware implementation for CORS handling</code> </dd></summary> <hr> gateway/mw_cors.go <li>Introduced CORSMiddleware struct and logic<br> <li> Handles CORS requests as part of middleware chain<br> <li> Returns early for preflight OPTIONS requests </details> </td> <td><a href="https://github.com/TykTechnologies/tyk/pull/7242/files#diff-b83b51e5f6dc341b26ecc1ef7bd72d680f398e599eea327662e2169a3b55f208">+43/-0</a> </td> </tr> <tr> <td> <details> <summary><strong>server.go</strong><dd><code>Apply CORS wrapper to OAuth endpoints when enabled</code> </dd></summary> <hr> gateway/server.go <li>Updated OAuth endpoint handlers to wrap with CORS only if enabled<br> <li> Added createCORSWrapper utility for conditional CORS wrapping </details> </td> <td><a href="https://github.com/TykTechnologies/tyk/pull/7242/files#diff-4652d1bf175a0be8f5e61ef7177c9666f23e077d8626b73ac9d13358fa8b525b">+34/-4</a> </td> </tr> </table></td></tr><tr><td><strong>Tests</strong></td><td><table> <tr> <td> <details> <summary><strong>api_loader_test.go</strong><dd><code>Remove outdated CORS tests</code> </dd></summary> <hr> gateway/api_loader_test.go - Removed legacy CORS test function </details> </td> <td><a href="https://github.com/TykTechnologies/tyk/pull/7242/files#diff-f696545a659f4d96421b253edef4bcc8da0e7f52120b8f8866d32cbbb7cc1afc">+0/-73</a> </td> </tr> <tr> <td> <details> <summary><strong>mw_cors_test.go</strong><dd><code>Add comprehensive CORSMiddleware tests</code> </dd></summary> <hr> gateway/mw_cors_test.go <li>Added unit tests for CORSMiddleware logic<br> <li> Added integration tests for CORS behavior on APIs and OAuth endpoints<br> <li> Utility functions for CORS config and middleware creation </details> </td> <td><a href="https://github.com/TykTechnologies/tyk/pull/7242/files#diff-60316be235ab2e13eab4b17dfa535b441db3c29d388c433cca7d6fbe4d26c3a0">+187/-0</a> </td> </tr> <tr> <td> <details> <summary><strong>reverse_proxy_test.go</strong><dd><code>Remove CORS header checks from GraphQL passthrough test</code> </dd></summary> <hr> gateway/reverse_proxy_test.go <li>Removed CORS header assertions from GraphQL options passthrough test </details> </td> <td><a href="https://github.com/TykTechnologies/tyk/pull/7242/files#diff-ce040f6555143f760fba6059744bc600b6954f0966dfb0fa2832b5eabf7a3c3f">+0/-5</a> </td> </tr> </table></td></tr></tr></tbody></table> ___ > <details> <summary> Need help?</summary><li>Type <code>/help how to ...</code> in the comments thread for any questions about PR-Agent usage.</li><li>Check out the <a href="https://qodo-merge-docs.qodo.ai/usage-guide/">documentation</a> for more information.</li></details> Co-authored-by: Maciej Miś <[email protected]>
User description
TT-10273
Description
The current implementation applies CORS middleware at the router level, which executes before the API Version Check middleware in the chain. This means that for versioned Tyk OAS APIs, the CORS configuration from the base API is always used, regardless of any version-specific CORS configurations.
Related Issue
Motivation and Context
How This Has Been Tested
Screenshots (if appropriate)
Types of changes
Checklist
PR Type
Bug fix, Enhancement, Tests
Description
Refactored CORS handling into a dedicated middleware for correct execution order
Ensured CORS checks occur after API version checks for OAS APIs
Updated OAuth endpoint handlers to apply CORS only when enabled
Added comprehensive unit and integration tests for the new CORS middleware
Changes diagram
Changes walkthrough 📝
api_loader.go
Move CORS handling to middleware chain after version checkgateway/api_loader.go
mw_cors.go
Add CORSMiddleware implementation for CORS handlinggateway/mw_cors.go
server.go
Apply CORS wrapper to OAuth endpoints when enabledgateway/server.go
api_loader_test.go
Remove outdated CORS testsgateway/api_loader_test.go
mw_cors_test.go
Add comprehensive CORSMiddleware testsgateway/mw_cors_test.go