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

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

Closed jeffy-mathew closed 2 weeks ago

jeffy-mathew commented 2 weeks ago

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

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) - Not compliant** Fully compliant requirements: - None Not compliant requirements: - Add validation rules on backend to ensure that configuration strings are not empty.
⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Validation Removal
The PR removes validation that ensures configuration strings are not empty. This is contrary to the ticket requirements. Validation Removal
The PR removes validation that ensures configuration strings are not empty. This is contrary to the ticket requirements. Validation Removal
The PR removes validation that ensures configuration strings are not empty. This is contrary to the ticket requirements. Validation Removal
The PR removes validation that ensures configuration strings are not empty. This is contrary to the ticket requirements. Validation Removal
The PR removes validation that ensures configuration strings are not empty. This is contrary to the ticket requirements.
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
Security
Reinstate stricter validation for critical fields to prevent empty values ___ **Reinstate the use of the X-Tyk-NonEmptyString definition for critical fields like
username, password, clientId, clientSecret, and tokenUrl to ensure these fields are
not empty, enhancing validation and security.** [apidef/oas/schema/x-tyk-api-gateway.json [2029-2030]](https://github.com/TykTechnologies/tyk/pull/6702/files#diff-78828969c0c04cc1a776dfc93a8bad3c499a8c83e6169f83e96d090bed3e7dd0R2029-R2030) ```diff "username": { - "type": "string" + "$ref": "#/definitions/X-Tyk-NonEmptyString" } ```
Suggestion importance[1-10]: 9 Why: Reverting to the 'X-Tyk-NonEmptyString' definition for fields like 'username' ensures that these critical fields are not left empty, thereby enhancing security and data integrity. This is a significant improvement in validation practices.
9
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

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

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

jeffy-mathew commented 2 weeks ago

/release to release-5.7

tykbot[bot] commented 2 weeks ago

Working on it! Note that it can take a few minutes.

jeffy-mathew commented 2 weeks ago

/release to release-5.7.0

tykbot[bot] commented 2 weeks ago

Working on it! Note that it can take a few minutes.

tykbot[bot] commented 2 weeks ago

@jeffy-mathew Seems like there is conflict and it require manual merge.

tykbot[bot] commented 2 weeks ago

@jeffy-mathew Seems like there is conflict and it require manual merge.