TykTechnologies / tyk

Tyk Open Source API Gateway written in Go, supporting REST, GraphQL, TCP and gRPC protocols
Other
9.75k stars 1.09k forks source link

Merging to release-5.7: Revert "[TT-13422] Do not allow empty string in upstream auth configuration strings" (#6702) #6703

Closed buger closed 2 weeks ago

buger commented 2 weeks ago

User description

Revert "TT-13422 Do not allow empty string in upstream auth configuration strings" (#6702)

Reverts TykTechnologies/tyk#6699 temporary revert with common change for AuthSource


PR Type

enhancement, bug fix


Description


Changes walkthrough πŸ“

Relevant files
Enhancement
x-tyk-api-gateway.json
Revert non-empty string enforcement and update auth source references

apidef/oas/schema/x-tyk-api-gateway.json
  • Reverted the use of X-Tyk-NonEmptyString for certain fields.
  • Introduced a new definition X-Tyk-UpstreamAuthSource.
  • Updated references from X-Tyk-AuthSource to X-Tyk-UpstreamAuthSource.
  • +15/-4   

    πŸ’‘ PR-Agent usage: Comment /help "your question" on any pull request to receive relevant information

    buger commented 2 weeks ago

    :broken_heart: The detected issue is not in one of the allowed statuses :broken_heart:

    Detected Status DoD Check :x:
    Allowed Statuses In Dev,In Code Review,Ready for Testing,In Test,In Progress,In Review :heavy_check_mark:

    Please ensure your jira story is in one of the allowed statuses

    github-actions[bot] commented 2 weeks ago

    PR Reviewer Guide πŸ”

    Here are some key observations to aid the review process:

    **🎫 Ticket compliance analysis βœ…** **[6699](https://github.com/TykTechnologies/tyk/issues/6699) - Fully compliant** Fully compliant requirements: - Add validation rules on backend to prevent empty strings in upstream authentication configuration.
    ⏱️ Estimated effort to review: 2 πŸ”΅πŸ”΅βšͺβšͺβšͺ
    πŸ§ͺ No relevant tests
    πŸ”’ No security concerns identified
    ⚑ Recommended focus areas for review

    Schema Inconsistency
    The PR introduces a new definition 'X-Tyk-UpstreamAuthSource' but does not remove or deprecate the old 'X-Tyk-AuthSource'. This might lead to inconsistencies or confusion in the schema usage.
    github-actions[bot] commented 2 weeks ago

    API Changes

    no api changes detected
    github-actions[bot] commented 2 weeks ago

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Score
    Enhancement
    Add a validation pattern to the "name" property to prevent empty strings ___ **Consider adding a validation pattern to the "name" property under
    "X-Tyk-UpstreamAuthSource" to ensure it does not accept empty strings or unintended
    characters.** [apidef/oas/schema/x-tyk-api-gateway.json [2153-2154]](https://github.com/TykTechnologies/tyk/pull/6703/files#diff-78828969c0c04cc1a776dfc93a8bad3c499a8c83e6169f83e96d090bed3e7dd0R2153-R2154) ```diff "name": { - "type": "string" + "type": "string", + "pattern": "\\S+" } ```
    Suggestion importance[1-10]: 7 Why: Adding a validation pattern to ensure non-empty strings in the "name" property under "X-Tyk-UpstreamAuthSource" enhances data integrity and prevents potential errors from empty or invalid inputs.
    7
    Possible issue
    Review and enhance the "X-Tyk-UpstreamAuthSource" definition to ensure it meets all necessary constraints ___ **Ensure that the "X-Tyk-UpstreamAuthSource" definition includes all necessary
    properties and constraints to match the intended use cases, as it replaces
    "X-Tyk-AuthSource" which might have had additional constraints.** [apidef/oas/schema/x-tyk-api-gateway.json [2147-2154]](https://github.com/TykTechnologies/tyk/pull/6703/files#diff-78828969c0c04cc1a776dfc93a8bad3c499a8c83e6169f83e96d090bed3e7dd0R2147-R2154) ```diff "X-Tyk-UpstreamAuthSource": { "type": "object", "properties": { "enabled": { "type": "boolean" }, "name": { - "type": "string" + "type": "string", + "pattern": "\\S+" } } } ```
    Suggestion importance[1-10]: 6 Why: The suggestion to review and potentially add more constraints to the "X-Tyk-UpstreamAuthSource" is valid as it replaces "X-Tyk-AuthSource" which might have had different constraints. This ensures the new definition aligns with intended use cases and maintains consistency.
    6
    sonarcloud[bot] commented 2 weeks ago

    Quality Gate Failed Quality Gate failed

    Failed conditions
    0.0% Coverage on New Code (required β‰₯ 80%)

    See analysis details on SonarQube Cloud