Merging to release-5.10: [TT-15956] Proprietary auth methods are auto populated to OAS Security when changing to compliant mode (#7425)

[buger]

User description

TT-15956 Proprietary auth methods are auto populated to OAS Security when changing to compliant mode (#7425)

User description

Description

When switching between Legacy and Compliant security processing modes,
authentication methods are being incorrectly placed in security fields,
causing proprietary auth (like customAuth) to appear in the OAS security
section and preventing users from adding new auth methods.

Related Issue

https://tyktech.atlassian.net/browse/TT-15956?atlOrigin=eyJpIjoiNjE0ZGRmYWExNmExNGRiMmIyZDI3ZDJhOTRmNGFjZDMiLCJwIjoiaiJ9

Motivation and Context

https://tyktech.atlassian.net/browse/TT-15956?atlOrigin=eyJpIjoiNjE0ZGRmYWExNmExNGRiMmIyZDI3ZDJhOTRmNGFjZDMiLCJwIjoiaiJ9

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

---

PR Type

Bug fix, Tests, Enhancement

---

Description

• Separate proprietary vs standard auth storage

• Add proprietary scheme detection helpers

• Filter proprietary auth in legacy mode

• Extensive tests for mixed auth scenarios

---

Diagram Walkthrough

flowchart LR
  OAS["OAS.fillSecurity/extractSecurityTo"]
  Detect["Proprietary scheme detection helpers"]
  Split["Split into OAS security vs vendor security"]
  Legacy["Legacy mode filtering (storage only)"]
  Tests["New tests for detection and modes"]

  OAS -- "calls" --> Detect
  Detect -- "classify scheme" --> Split
  OAS -- "applies" --> Split
  OAS -- "applies" --> Legacy
  Split -- "verified by" --> Tests
  Legacy -- "verified by" --> Tests

Loading

File Walkthrough

	Relevant files
Enhancement	security.go Proprietary auth detection and security splitting                apidef/oas/security.go Add helper methods to detect proprietary schemes. Use detection to split security into OAS vs vendor. Filter proprietary schemes in legacy mode storage. Adjust extraction to honor vendor security in compliant mode. +135/-15
Tests	security_test.go Tests for auth separation and mode handling                            apidef/oas/security_test.go Add tests for proprietary scheme detection helpers. Update legacy mode to exclude proprietary from OAS. Add mode switching and mixed auth scenarios. Validate OAS vs vendor security separation. +748/-10

---

---

PR Type

Bug fix, Enhancement, Tests

---

Description

• Validate compliant-mode auth configuration

• Split OAS vs vendor auth consistently

• Filter proprietary auth in legacy storage

• Add extensive auth mode tests

---

Diagram Walkthrough

flowchart LR
  OAS["OAS.Validate"]
  ValSec["validateSecurity()"]
  ValComp["validateCompliantModeAuthentication()"]
  Fill["fillSecurity()"]
  Split["Split OAS vs Vendor security"]
  Extract["extractSecurityTo()"]
  Detect["Proprietary scheme detection"]
  Tests["Unit tests for modes and detection"]

  OAS -- calls --> ValSec
  OAS -- calls --> ValComp
  Fill -- uses --> Detect
  Fill -- "separates into" --> Split
  Extract -- "reads from" --> Split
  Split -- "verified by" --> Tests
  ValComp -- "verified by" --> Tests

Loading

File Walkthrough

	Relevant files
Enhancement	oas.go Compliant-mode authentication validation in OAS                    apidef/oas/oas.go Add compliant-mode auth validation to Validate. Enforce error on enabled-but-unreferenced auth. Improve error messages casing. Collect OAS and vendor security when validating. +126/-3  security.go Security splitting and proprietary detection                          apidef/oas/security.go Add proprietary scheme detection helpers. Split security into OAS vs vendor in compliant mode. Filter proprietary auth from OAS in legacy mode. Extend extraction to consider vendor security. +170/-44
Tests	oas_test.go Tests for compliant-mode auth validation                                  apidef/oas/oas_test.go Update expected error messages casing. Add tests for compliant-mode auth validation. Cover multiple auth enablement scenarios. +379/-2  security_test.go Comprehensive tests for auth separation and modes                apidef/oas/security_test.go Adjust legacy test to expect proprietary filtering. Add extensive tests for detection helpers. Add mode switching and mixed auth tests. Add JWT OR-auth retrieval tests. +1362/-10 mw_auth_or_wrapper_test.go Fix expected status in compliant-mode test                              gateway/mw_auth_or_wrapper_test.go Update expected HTTP status for invalid oauth in compliant mode. +1/-1

---

[probelabs]

🔍 Code Analysis Results

Change Impact Analysis

What this PR accomplishes

This pull request resolves a critical bug where Tyk-proprietary authentication methods (e.g., HMAC, custom plugins) were incorrectly bleeding into the standard OpenAPI Specification (OAS) security field when an API was switched to "compliant" mode. This polluted the OAS document, breaking compliance and preventing users from managing security schemes correctly.

The solution establishes a strict and permanent separation between standard OAS-compliant security schemes and Tyk's proprietary ones. Proprietary schemes are now exclusively managed within the x-tyk-api-gateway vendor extension, regardless of whether the API is in "legacy" or "compliant" mode. This ensures the OAS document remains clean and valid while preserving all of Tyk's extended authentication capabilities.

Additionally, a new validation layer is introduced for "compliant" mode, which prevents misconfigurations by ensuring any enabled authentication method is explicitly listed in a security requirement.

Key Technical Changes

1. Proprietary Scheme Detection (apidef/oas/security.go):

   • A new set of helper functions, spearheaded by isProprietaryAuthScheme, has been introduced. This logic reliably identifies whether a security scheme is a standard OAS type (like JWT, OAuth2, Basic) or a Tyk-proprietary one (like HMAC, custom plugins) by inspecting its type and where it's defined.

2. Segregated Security Storage (apidef/oas/security.go):

   • The fillSecurity function, which populates the OAS document from a Tyk API definition, has been fundamentally reworked.
   • In Compliant Mode: It now intelligently separates security requirements. Requirements containing only standard schemes are placed in the top-level OAS security array. If a requirement contains any proprietary scheme (a mixed "AND" condition), the entire requirement is moved to x-tyk-api-gateway.server.authentication.security to preserve the logic.
   • In Legacy Mode: The logic is updated to filter proprietary schemes out of the main OAS security field, moving them to the vendor extension. This is a crucial change that prevents OAS pollution regardless of the mode.

3. Unified Security Extraction (apidef/oas/security.go):

   • The extractSecurityTo function, which reads from an OAS document to configure a Tyk API, is now aware of the new separation. In compliant mode, it correctly reads and combines security requirements from both the standard OAS security field and the x-tyk-api-gateway vendor extension.

4. Strict Compliant Mode Validation (apidef/oas/oas.go):

   • A new validation function, validateCompliantModeAuthentication, has been added. When an API is in "compliant" mode, this function cross-references all enabled authentication methods against the defined security requirements (both standard and vendor). If an enabled method is not used in any requirement, validation fails, preventing runtime errors from misconfigured APIs.

Affected System Components

• API Definition Processing: The core logic for loading, validating, and saving API definitions that use the OAS format is significantly impacted. The gateway will now handle mixed authentication schemes more robustly.
• Tyk Dashboard & API Designer: The user-facing issue is directly resolved. The Dashboard will now generate correct and compliant OAS definitions when switching between security modes or configuring mixed authentication.
• API Gateway: The gateway's configuration loader will benefit from the clearer separation, leading to more predictable authentication behavior.

---

Architecture Visualization

The following diagrams illustrate the key architectural changes introduced by this pull request.

1. Authentication Scheme Segregation Flow

This diagram shows how the updated fillSecurity function processes a list of security requirements and segregates them into the standard OAS document or the Tyk vendor extension.

flowchart TD
    subgraph "Tyk API Definition"
        A["Security Requirements: [jwt], [hmac, basic]"]
    end

    subgraph "OAS Processing Logic (fillSecurity)"
        B{For each requirement}
        C{Contains proprietary scheme?}
        B --> C
        C -- Yes --> D[Move ENTIRE requirement to Vendor Extension]
        C -- No --> E[Add requirement to OAS Security]
    end

    subgraph "Final OAS Document"
        F["OAS `security` field"]
        G["`x-tyk-api-gateway.security` field"]
    end

    A --> B
    D --> G
    E --> F

    style F fill:#d4edda,stroke:#c3e6cb
    style G fill:#f8d7da,stroke:#f5c6cb

Loading

Explanation: The logic iterates through each security requirement. The [jwt] requirement contains only a standard scheme, so it's placed in the main OAS security field. The [hmac, basic] requirement is a mixed "AND" condition containing a proprietary scheme (hmac), so the entire requirement is moved to the x-tyk-api-gateway vendor extension to preserve its integrity.

2. Compliant Mode Validation Sequence

This diagram illustrates the new validation process that runs when an API definition is saved in "compliant" mode.

sequenceDiagram
    participant Client
    participant TykGateway as Tyk Gateway
    participant OAS_Validator as OAS Validator
    participant TykExtension as "x-tyk-api-gateway"

    Client->>TykGateway: Save API Definition (Compliant Mode)
    TykGateway->>OAS_Validator: Validate()
    OAS_Validator->>TykExtension: Get all enabled auth schemes (e.g., JWT, HMAC)
    OAS_Validator->>OAS_Validator: Collect schemes from OAS `security`
    OAS_Validator->>TykExtension: Collect schemes from vendor `security`
    OAS_Validator->>OAS_Validator: For each enabled scheme...
    alt Is scheme configured in a security requirement?
        OAS_Validator-->>OAS_Validator: Yes, continue
    else No, it's misconfigured
        OAS_Validator-->>TykGateway: Return Validation Error
        TykGateway-->>Client: Reject with Error ("hmac auth enabled but not configured")
    end
    OAS_Validator-->>TykGateway: Validation Success
    TykGateway-->>Client: Acknowledge Success

Loading

Explanation: This flow ensures that if a developer enables an authentication method like HMAC in the Tyk extension, they cannot forget to add it to a security requirement. This prevents "dead" or misconfigured authentication rules and improves the reliability of multi-auth APIs.

---

Powered by Visor from Probelabs

Last updated: 2025-10-11T14:48:16.099Z | Triggered by: synchronize | Commit: 3ced380

[probelabs]

🔍 Code Analysis Results

Security Issues (2)

Severity	Location	Issue
🔴 Critical	apidef/oas/security.go:1023-1045	In legacy mode, a security requirement containing a mix of standard and proprietary schemes (an "AND" condition) is incorrectly split into separate requirements. The standard schemes are placed in the OAS `security` field and the proprietary ones in the vendor extension. This fundamentally changes the security logic from a single "AND" requirement to two separate "OR" requirements, which weakens the intended security policy. 💡 Suggestion To preserve the intended "AND" logic, the handling of mixed requirements should be consistent with compliant mode. If a security requirement contains any proprietary scheme, the entire requirement should be moved to the `tykAuth.Security` vendor extension.
🔴 Critical	apidef/oas/security.go:1070-1141	In legacy mode, the `extractSecurityTo` function only reads security requirements from the standard OAS `security` field and ignores the `tykAuth.Security` vendor extension. Since the corresponding `fillSecurity` function now moves all proprietary schemes to this vendor extension, any proprietary authentication configuration (e.g., HMAC, custom auth) will be lost upon saving and reloading the API definition. This constitutes a data loss bug that can silently disable authentication methods, potentially leaving an API unprotected. 💡 Suggestion Modify the `extractSecurityTo` function to ensure that in legacy mode, it reads security requirements from both the standard `s.Security` field and the vendor extension `tykAuth.Security`. While legacy mode should still only process the first valid requirement found, it must check both locations to correctly reconstruct the API's full security configuration and prevent data loss.

Performance Issues (4)

Severity	Location	Issue
🔴 Critical	apidef/oas/security.go:1125-1141	In legacy mode, the `extractSecurityTo` function only reads from the standard OAS `security` field and completely ignores the vendor extension (`tykAuth.Security`). Since the corresponding `fillSecurity` function now moves all proprietary authentication schemes to this vendor extension even in legacy mode, any proprietary security configuration will be lost upon saving and reloading the API definition. This constitutes a critical data loss bug that can silently disable authentication for proprietary schemes. 💡 Suggestion Modify the `extractSecurityTo` function to ensure that in legacy mode, it reads security requirements from both the standard `s.Security` field and the vendor extension `tykAuth.Security`. While legacy mode should still only process the first valid requirement found, it must check both locations to correctly reconstruct the API's security configuration and prevent data loss.
🟠 Error	apidef/oas/security.go:1032-1045	In legacy mode, the `fillSecurity` function incorrectly handles security requirements that contain a mix of standard and proprietary schemes (an "AND" condition). Instead of moving the entire mixed requirement to the vendor extension as it does in compliant mode, it splits the requirement. The standard schemes are placed in the OAS `security` field, and the proprietary ones are placed in the vendor extension. This fundamentally changes the security logic from a single "AND" requirement to two separate "OR" requirements, which is an unintended and breaking change in behavior. 💡 Suggestion Refactor the legacy mode logic in `fillSecurity` to align with the compliant mode's handling of mixed requirements. If a security requirement contains any proprietary scheme, the entire requirement should be moved to `tykAuth.Security`. This ensures that the "AND" relationship is preserved and that the storage of security requirements is consistent regardless of the processing mode, as implied by the comment on line 1029.
🟠 Error	apidef/oas/security_test.go:1787	The file contains unresolved merge conflict markers (`<<<<<<< HEAD`, `=======`, `>>>>>>>`). These markers make the file syntactically incorrect, and the code will not compile. They must be removed, and the intended code changes must be correctly integrated before this pull request can be merged. 💡 Suggestion Please resolve the merge conflict in `apidef/oas/security_test.go` by removing the conflict markers and ensuring the final code reflects the intended combination of changes from both branches.
🟡 Warning	apidef/oas/oas.go:501	This pull request changes the capitalization style for several error messages from sentence case to lowercase (e.g., "No components..." becomes "no components..."). While this change is applied consistently within the new and modified code in this PR, it is inconsistent with the error message style in other parts of the codebase. For better overall consistency, it's recommended to adhere to a single capitalization style for error messages throughout the project. 💡 Suggestion Please verify if this change in error message capitalization aligns with the project's established coding style. If the convention is to use sentence case, consider reverting these changes to maintain consistency across the application.

Quality Issues (4)

Severity	Location	Issue
🔴 Critical	apidef/oas/security.go:1102-1105	In legacy mode, the `extractSecurityTo` function fails to read proprietary security schemes from the vendor extension, leading to their silent deletion when an API definition is saved and reloaded. The check for vendor security is incorrectly restricted to 'compliant' mode, causing any proprietary auth configuration to be lost during the extraction process in 'legacy' mode. 💡 Suggestion Modify the condition that checks for vendor security to operate regardless of the processing mode. The logic should read from both the standard OAS `security` field and the `tykAuth.Security` vendor extension in both legacy and compliant modes to correctly reconstruct the API's full security configuration and prevent data loss.
🟠 Error	apidef/oas/security.go:1032-1045	In legacy mode, a security requirement containing a mix of standard and proprietary schemes (an 'AND' condition) is incorrectly split. Standard schemes are placed in the OAS `security` field, while proprietary ones go to the vendor extension. This incorrectly changes the security logic from a single 'AND' requirement to two separate 'OR' requirements, which is an unintended breaking change. 💡 Suggestion Refactor the legacy mode logic in `fillSecurity` to align with the compliant mode's handling of mixed requirements. If a security requirement contains any proprietary scheme, the entire requirement should be moved to `tykAuth.Security`. This ensures the 'AND' relationship is preserved and that storage is consistent regardless of the processing mode.
🟡 Warning	apidef/oas/security_test.go:1710-1750	The test case named `"legacy mode filters proprietary auth"` validates the incorrect behavior where a mixed security requirement is split into two separate 'OR' conditions. The test passes, but it confirms a flawed implementation and masks the critical data loss and logic alteration bugs. 💡 Suggestion Rewrite the test to assert the correct behavior. It should verify that a mixed 'AND' requirement is kept intact within the vendor extension in legacy mode and that a full round-trip (fill -> extract -> fill) does not result in the loss of any security configuration.
🟡 Warning	apidef/oas/oas.go:500-508	This pull request changes the capitalization style for several error messages from sentence case to lowercase (e.g., "No components..." becomes "no components..."). This may be inconsistent with the prevailing style in the rest of the codebase. 💡 Suggestion Verify if this change in error message capitalization aligns with the project's established coding style. If the convention is to use sentence case, consider reverting these changes to maintain consistency across the project.

Style Issues (2)

Severity	Location	Issue
🟠 Error	apidef/oas/security.go:1118-1141	In legacy mode, the `extractSecurityTo` function only reads security requirements from the standard OAS `security` field, completely ignoring the vendor extension (`tykAuth.Security`). Since the corresponding `fillSecurity` function now moves all proprietary authentication schemes to this vendor extension, any proprietary security configuration will be lost when the API definition is saved and reloaded. This constitutes a data loss bug that can silently disable authentication methods. 💡 Suggestion Modify the `extractSecurityTo` function to ensure that in legacy mode, it reads security requirements from both the standard `s.Security` field and the vendor extension `tykAuth.Security`. While legacy mode should still only process the first valid requirement found, it must check both locations to correctly reconstruct the API's security configuration and prevent data loss.
🟠 Error	apidef/oas/security.go:1027-1042	In legacy mode, the `fillSecurity` function incorrectly handles security requirements that contain a mix of standard and proprietary schemes (an "AND" condition). Instead of moving the entire mixed requirement to the vendor extension as it does in compliant mode, it splits the requirement. The standard schemes are placed in the OAS `security` field, and the proprietary ones are placed in the vendor extension. This fundamentally changes the security logic from a single "AND" requirement to two separate "OR" requirements, which is an unintended and breaking change in behavior. 💡 Suggestion Refactor the legacy mode logic in `fillSecurity` to align with the compliant mode's handling of mixed requirements. If a security requirement contains any proprietary scheme, the entire requirement should be moved to `tykAuth.Security`. This ensures that the "AND" relationship is preserved and that the storage of security requirements is consistent regardless of the processing mode.

✅ Dependency Check Passed

No dependency issues found – changes LGTM.

✅ Connectivity Check Passed

No connectivity issues found – changes LGTM.

---

Powered by Visor from Probelabs

Last updated: 2025-10-11T14:48:17.339Z | Triggered by: synchronize | Commit: 3ced380

[github-actions]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
General	Align test with runtime behavior Verify that the production code returns 400 for invalid OAuth in compliant mode; if runtime behavior remains 403, this test will consistently fail. Align the test with the actual handler behavior or adjust the handler to return 400 to match this expectation. gateway/mw_auth_or_wrapper_test.go [382-389] { Method: "GET", Path: "/test-complex-compliant/", Headers: map[string]string{ "Authorization": "Bearer invalid-oauth", }, - Code: http.StatusBadRequest, + Code: http.StatusForbidden, // change to BadRequest only after handler behavior is updated }, Suggestion importance[1-10]: 6 __ Why: The PR changes the expected status from 403 to 400; suggesting to keep the test aligned with actual handler behavior helps avoid false failures. Since it asks to verify runtime behavior, impact is moderate.	Low
	Clarify error message listing Use a comma-separated list for multiple methods to improve clarity and avoid awkward "and" joins for more than two items. This also makes error parsing by tools more reliable. apidef/oas/oas.go [626-635] // Return error if any misconfigured methods found if len(misconfiguredMethods) > 0 { - methodList := strings.Join(misconfiguredMethods, " and ") - var authWord string - if len(misconfiguredMethods) == 1 { - authWord = "auth" - } else { + methodList := strings.Join(misconfiguredMethods, ", ") + authWord := "auth" + if len(misconfiguredMethods) > 1 { authWord = "auth methods" } return fmt.Errorf("invalid multi-auth configuration: %s %s enabled but not configured in a security requirement", methodList, authWord) } Suggestion importance[1-10]: 4 __ Why: Changing the joiner to commas is stylistic and can aid readability and parsing, but it does not affect correctness or tests; improvement is minor.	Low
Possible issue	Prevent nil dereference Guard against nil s.T.Security to avoid a potential nil dereference when the OpenAPI doc has no security requirements. Add a quick nil check before iterating. apidef/oas/oas.go [536-540] -for _, requirement := range s.T.Security { - for schemeName := range requirement { - configuredAuthMethods[schemeName] = true +if s.T.Security != nil { + for _, requirement := range s.T.Security { + for schemeName := range requirement { + configuredAuthMethods[schemeName] = true + } } } Suggestion importance[1-10]: 5 __ Why: Iterating over s.T.Security without a nil check could panic if s.T.Security is nil; adding a guard is a safe minor improvement. Impact is moderate since the function already handles empty cases elsewhere but this tightens robustness.	Low