[TT-13375/TT-13422] Add validation rules for Upstream auth

[jeffy-mathew]

User description

TT-13422

Summary	Add validation rules on backend
Type	Sub-task
Status	In Dev
Points	N/A
Labels	-

---

Description

Add validation rules for upstream auth

Related Issue

Motivation and Context

How This Has Been Tested

Screenshots (if appropriate)

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)
• Refactoring or add test (improvements in base code or adds test coverage to functionality)

Checklist

• I ensured that the documentation is up to date
• I explained why this PR updates go.mod in detail with reasoning why it's required
• I would like a code coverage CI quality gate exception and have explained why

---

PR Type

enhancement, tests

---

Description

• Added new validation rule RuleUpstreamAuth to enforce constraints on upstream authentication modes.
• Introduced constants for OAuth authorization types and refactored code to use these constants.
• Implemented error handling for multiple upstream authentication modes and invalid OAuth authorization types.
• Added comprehensive test cases to validate the new upstream authentication rules.

---

PRDescriptionHeader.CHANGES_WALKTHROUGH

	Relevant files
Enhancement	api_definitions.go Add constants for OAuth authorization types                            apidef/api_definitions.go Added constants for OAuth authorization types. +3/-0      validator.go Implement validation rules for upstream authentication      apidef/validator.go Introduced new validation rule RuleUpstreamAuth. Added error handling for multiple upstream authentication modes. Implemented checks for OAuth authorization types. +45/-0    mw_oauth2_auth.go Refactor OAuth authorization type handling                              gateway/mw_oauth2_auth.go Removed redundant constants for OAuth authorization types. Updated logic to use new constants from apidef. +4/-6
Tests	validator_test.go Add test cases for upstream authentication validation        apidef/validator_test.go Added test cases for RuleUpstreamAuth validation. Tested various scenarios for upstream authentication. +92/-0    mw_oauth2_auth_test.go Update test cases with new OAuth constants                              gateway/mw_oauth2_auth_test.go Updated test cases to use new OAuth authorization type constants. +2/-2

---

💡 PR-Agent usage: Comment /help "your question" on any pull request to receive relevant information

[buger]

I'm a bot and I 👍 this PR title. 🤖

[github-actions]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪

🧪 PR contains tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Error Handling Consider adding more specific error messages for different OAuth authorization type errors to aid in debugging and user feedback. Error Handling Ensure consistent and clear error handling for OAuth header provider configurations to prevent runtime issues.

[github-actions]

API Changes

--- prev.txt	2024-10-29 11:45:49.471446051 +0000
+++ current.txt	2024-10-29 11:45:43.147435634 +0000
@@ -74,6 +74,11 @@
 	OAuthType         = "oauth"
 	ExternalOAuthType = "externalOAuth"
 	OIDCType          = "oidc"
+
+	// OAuthAuthorizationTypeClientCredentials is the authorization type for client credentials flow.
+	OAuthAuthorizationTypeClientCredentials = "clientCredentials"
+	// OAuthAuthorizationTypePassword is the authorization type for password flow.
+	OAuthAuthorizationTypePassword = "password"
 )
 const (
 	GraphQLEngineDataSourceKindREST    = "REST"
@@ -1231,11 +1236,22 @@
 	ErrAPINotFound                = errors.New("API not found")
 	ErrMissingAPIID               = errors.New("missing API ID")
 )
+var (
+	// ErrMultipleUpstreamAuthEnabled is the error to be returned when multiple upstream authentication modes are configured.
+	ErrMultipleUpstreamAuthEnabled = errors.New("multiple upstream authentication modes not allowed")
+	// ErrMultipleUpstreamOAuthAuthorizationType is the error to return when multiple OAuth authorization types are configured.
+	ErrMultipleUpstreamOAuthAuthorizationType = errors.New("multiple upstream OAuth authorization type not allowed")
+	// ErrUpstreamOAuthAuthorizationTypeRequired is the error to return when OAuth authorization type is not specified.
+	ErrUpstreamOAuthAuthorizationTypeRequired = errors.New("upstream OAuth authorization type is required")
+	// ErrInvalidUpstreamOAuthAuthorizationType is the error to return when configured OAuth authorization type is invalid.
+	ErrInvalidUpstreamOAuthAuthorizationType = errors.New("invalid OAuth authorization type")
+)
 var DefaultValidationRuleSet = ValidationRuleSet{
 	&RuleUniqueDataSourceNames{},
 	&RuleAtLeastEnableOneAuthSource{},
 	&RuleValidateIPList{},
 	&RuleValidateEnforceTimeout{},
+	&RuleUpstreamAuth{},
 }
 var ErrAllAuthSourcesDisabled = "all auth sources are disabled for %s, at least one of header/cookie/query must be enabled"
 var ErrDuplicateDataSourceName = errors.New("duplicate data source names are not allowed")
@@ -2270,6 +2286,13 @@
 
 func (r *RuleUniqueDataSourceNames) Validate(apiDef *APIDefinition, validationResult *ValidationResult)
 
+type RuleUpstreamAuth struct{}
+    RuleUpstreamAuth implements validations for upstream authentication
+    configurations.
+
+func (r *RuleUpstreamAuth) Validate(apiDef *APIDefinition, validationResult *ValidationResult)
+    Validate validates api definition upstream authentication configurations.
+
 type RuleValidateEnforceTimeout struct{}
 
 func (r *RuleValidateEnforceTimeout) Validate(apiDef *APIDefinition, validationResult *ValidationResult)
@@ -8401,10 +8424,8 @@
 	ECDSASign = "ecdsa"
 )
 const (
-	UpstreamOAuthErrorEventName    = "UpstreamOAuthError"
-	UpstreamOAuthMiddlewareName    = "UpstreamOAuth"
-	ClientCredentialsAuthorizeType = "clientCredentials"
-	PasswordAuthorizeType          = "password"
+	UpstreamOAuthErrorEventName = "UpstreamOAuthError"
+	UpstreamOAuthMiddlewareName = "UpstreamOAuth"
 )
 const (
 	ErrOAuthAuthorizationFieldMissing   = "oauth.auth_field_missing"

[github-actions]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Score
Possible issue	Add validation for configurations where no upstream authentication method is enabled Add validation for the scenario where neither BasicAuth nor OAuth is enabled, which might be an unintended configuration error. apidef/validator.go [220-223] -if upstreamAuth.BasicAuth.Enabled && upstreamAuth.OAuth.Enabled { +if !upstreamAuth.BasicAuth.Enabled && !upstreamAuth.OAuth.Enabled { + validationResult.IsValid = false + validationResult.AppendError(ErrNoUpstreamAuthEnabled) +} else if upstreamAuth.BasicAuth.Enabled && upstreamAuth.OAuth.Enabled { validationResult.IsValid = false validationResult.AppendError(ErrMultipleUpstreamAuthEnabled) } Suggestion importance[1-10]: 8 Why: This suggestion addresses a potential configuration error where neither BasicAuth nor OAuth is enabled, which could lead to unintended behavior. Adding this validation improves the robustness of the code by ensuring that at least one authentication method is enabled.	8
Enhancement	Optimize the validation of OAuth authorization types using a map for better scalability and readability Refactor the validation logic to handle multiple OAuth authorization types more efficiently by using a set or map to check for valid types. apidef/validator.go [242-245] -if authType := upstreamAuth.OAuth.AllowedAuthorizeTypes[0]; authType != OAuthAuthorizationTypeClientCredentials && authType != OAuthAuthorizationTypePassword { - validationResult.IsValid = false - validationResult.AppendError(ErrInvalidUpstreamOAuthAuthorizationType) +validTypes := map[string]bool{ + OAuthAuthorizationTypeClientCredentials: true, + OAuthAuthorizationTypePassword: true, +} +for _, authType := range upstreamAuth.OAuth.AllowedAuthorizeTypes { + if !validTypes[authType] { + validationResult.IsValid = false + validationResult.AppendError(ErrInvalidUpstreamOAuthAuthorizationType) + break + } } Suggestion importance[1-10]: 7 Why: The suggestion refactors the validation logic to use a map, which enhances readability and scalability. This change is beneficial for handling multiple authorization types efficiently, although the current implementation is not incorrect.	7
	Clarify the error message when multiple OAuth authorization types are provided Ensure that the error message for multiple authorization types is clear and specifies which types are conflicting. gateway/mw_oauth2_auth.go [169-170] case len(oauthConfig.AllowedAuthorizeTypes) > 1: - return nil, fmt.Errorf("both client credentials and password authentication are provided") + return nil, fmt.Errorf("multiple OAuth authorization types provided: %s", strings.Join(oauthConfig.AllowedAuthorizeTypes, ", ")) Suggestion importance[1-10]: 5 Why: This suggestion enhances the clarity of the error message by specifying the conflicting types, which aids in debugging and understanding the issue. It is a useful improvement for error reporting.	5
	Improve error messaging for unsupported OAuth authorization types Implement a default case in the switch statement to handle unexpected authorization types more gracefully. gateway/mw_oauth2_auth.go [176-177] default: - return nil, fmt.Errorf("no valid OAuth configuration provided") + return nil, fmt.Errorf("provided OAuth authorization type is not supported") Suggestion importance[1-10]: 3 Why: The suggestion improves the error message for unsupported OAuth authorization types, making it more descriptive. While this enhances user understanding, it is a minor improvement and does not affect functionality.	3

[sonarcloud]

Quality Gate failed

Failed conditions
0.0% Coverage on New Code (required ≥ 80%)

See analysis details on SonarCloud