From 60841eb121e4efbb85e7d37026a0d3de4ed11e3e Mon Sep 17 00:00:00 2001 From: Lukasz Raczylo Date: Fri, 26 Sep 2025 23:16:35 +0100 Subject: [PATCH] fixup! Add new providers. --- internal/providers/auth0.go | 2 +- internal/providers/aws_cognito.go | 7 +- internal/providers/aws_cognito_test.go | 295 ++++++++++++++++++++++ internal/providers/azure.go | 2 +- internal/providers/azure_test.go | 6 +- internal/providers/base.go | 17 +- internal/providers/factory_test.go | 36 +++ internal/providers/github.go | 2 +- internal/providers/gitlab.go | 2 +- internal/providers/gitlab_test.go | 322 +++++++++++++++++++++++++ internal/providers/google.go | 2 +- internal/providers/keycloak.go | 2 +- internal/providers/keycloak_test.go | 232 ++++++++++++++++++ internal/providers/okta.go | 2 +- internal/providers/okta_test.go | 200 +++++++++++++++ internal/providers/registry_test.go | 47 ++++ 16 files changed, 1162 insertions(+), 14 deletions(-) create mode 100644 internal/providers/aws_cognito_test.go create mode 100644 internal/providers/gitlab_test.go create mode 100644 internal/providers/keycloak_test.go create mode 100644 internal/providers/okta_test.go diff --git a/internal/providers/auth0.go b/internal/providers/auth0.go index 0b4325d..5472091 100644 --- a/internal/providers/auth0.go +++ b/internal/providers/auth0.go @@ -62,7 +62,7 @@ func (p *Auth0Provider) BuildAuthParams(baseParams url.Values, scopes []string) return &AuthParams{ URLValues: baseParams, - Scopes: scopes, + Scopes: deduplicateScopes(scopes), }, nil } diff --git a/internal/providers/aws_cognito.go b/internal/providers/aws_cognito.go index 5635282..cd995d2 100644 --- a/internal/providers/aws_cognito.go +++ b/internal/providers/aws_cognito.go @@ -2,6 +2,7 @@ package providers import ( "net/url" + "strings" ) // AWSCognitoProvider encapsulates AWS Cognito-specific OIDC logic. @@ -36,10 +37,10 @@ func (p *AWSCognitoProvider) BuildAuthParams(baseParams url.Values, scopes []str // AWS Cognito supports standard OIDC parameters baseParams.Set("response_type", "code") - // Remove offline_access scope as Cognito doesn't use it + // Remove offline_access scope as Cognito doesn't use it (case-insensitive) var filteredScopes []string for _, scope := range scopes { - if scope != "offline_access" { + if strings.ToLower(scope) != "offline_access" { filteredScopes = append(filteredScopes, scope) } } @@ -63,7 +64,7 @@ func (p *AWSCognitoProvider) BuildAuthParams(baseParams url.Values, scopes []str return &AuthParams{ URLValues: baseParams, - Scopes: filteredScopes, + Scopes: deduplicateScopes(filteredScopes), }, nil } diff --git a/internal/providers/aws_cognito_test.go b/internal/providers/aws_cognito_test.go new file mode 100644 index 0000000..779be42 --- /dev/null +++ b/internal/providers/aws_cognito_test.go @@ -0,0 +1,295 @@ +package providers + +import ( + "net/url" + "testing" +) + +// TestAWSCognitoProvider_NewAWSCognitoProvider tests the constructor +func TestAWSCognitoProvider_NewAWSCognitoProvider(t *testing.T) { + provider := NewAWSCognitoProvider() + + if provider == nil { + t.Fatal("Expected provider to be created, got nil") + } + + if provider.BaseProvider == nil { + t.Error("BaseProvider should be initialized") + } +} + +// TestAWSCognitoProvider_GetType tests provider type +func TestAWSCognitoProvider_GetType(t *testing.T) { + provider := NewAWSCognitoProvider() + + if provider.GetType() != ProviderTypeAWSCognito { + t.Errorf("Expected ProviderTypeAWSCognito, got %v", provider.GetType()) + } +} + +// TestAWSCognitoProvider_GetCapabilities tests AWS Cognito-specific capabilities +func TestAWSCognitoProvider_GetCapabilities(t *testing.T) { + provider := NewAWSCognitoProvider() + capabilities := provider.GetCapabilities() + + if !capabilities.SupportsRefreshTokens { + t.Error("Expected SupportsRefreshTokens to be true for AWS Cognito") + } + + if capabilities.RequiresOfflineAccessScope { + t.Error("Expected RequiresOfflineAccessScope to be false for AWS Cognito") + } + + if capabilities.RequiresPromptConsent { + t.Error("Expected RequiresPromptConsent to be false for AWS Cognito") + } + + if capabilities.PreferredTokenValidation != "id" { + t.Errorf("Expected PreferredTokenValidation 'id', got '%s'", capabilities.PreferredTokenValidation) + } +} + +// TestAWSCognitoProvider_BuildAuthParams tests AWS Cognito-specific auth params +func TestAWSCognitoProvider_BuildAuthParams(t *testing.T) { + provider := NewAWSCognitoProvider() + baseParams := url.Values{} + baseParams.Set("client_id", "test_client") + + tests := []struct { + name string + scopes []string + expectedScopes []string + }{ + { + name: "Remove offline_access scope and ensure openid", + scopes: []string{"email", "profile", "offline_access"}, + expectedScopes: []string{"email", "profile", "openid"}, + }, + { + name: "Keep existing openid, remove offline_access", + scopes: []string{"openid", "email", "offline_access", "profile"}, + expectedScopes: []string{"openid", "email", "profile"}, + }, + { + name: "Add default scopes when only openid", + scopes: []string{"openid"}, + expectedScopes: []string{"openid", "email", "profile"}, + }, + { + name: "Add openid and defaults when empty", + scopes: []string{}, + expectedScopes: []string{"openid", "email", "profile"}, + }, + { + name: "Cognito-specific scopes", + scopes: []string{"aws.cognito.signin.user.admin", "phone"}, + expectedScopes: []string{"aws.cognito.signin.user.admin", "phone", "openid"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + authParams, err := provider.BuildAuthParams(baseParams, tt.scopes) + if err != nil { + t.Errorf("BuildAuthParams failed: %v", err) + return + } + + // Check that response_type is set + if authParams.URLValues.Get("response_type") != "code" { + t.Errorf("Expected response_type 'code', got '%s'", authParams.URLValues.Get("response_type")) + } + + if len(authParams.Scopes) != len(tt.expectedScopes) { + t.Errorf("Expected %d scopes, got %d. Expected: %v, Got: %v", + len(tt.expectedScopes), len(authParams.Scopes), tt.expectedScopes, authParams.Scopes) + return + } + + // Check that all expected scopes are present + for _, expectedScope := range tt.expectedScopes { + found := false + for _, actualScope := range authParams.Scopes { + if actualScope == expectedScope { + found = true + break + } + } + if !found { + t.Errorf("Expected scope '%s' not found in %v", expectedScope, authParams.Scopes) + } + } + + // Ensure offline_access is NOT present + for _, actualScope := range authParams.Scopes { + if actualScope == "offline_access" { + t.Error("offline_access scope should be filtered out for AWS Cognito") + } + } + }) + } +} + +// TestAWSCognitoProvider_ValidateConfig tests config validation +func TestAWSCognitoProvider_ValidateConfig(t *testing.T) { + provider := NewAWSCognitoProvider() + + err := provider.ValidateConfig() + if err != nil { + t.Errorf("ValidateConfig failed: %v", err) + } +} + +// TestAWSCognitoProvider_InterfaceCompliance tests that AWS Cognito provider implements the OIDCProvider interface +func TestAWSCognitoProvider_InterfaceCompliance(t *testing.T) { + var _ OIDCProvider = NewAWSCognitoProvider() +} + +// TestAWSCognitoProvider_BaseProviderInheritance tests that AWS Cognito provider inherits from BaseProvider correctly +func TestAWSCognitoProvider_BaseProviderInheritance(t *testing.T) { + provider := NewAWSCognitoProvider() + + // Test that it has access to BaseProvider methods + if provider.BaseProvider == nil { + t.Error("Expected BaseProvider to be initialized") + } + + // Test HandleTokenRefresh (inherited from BaseProvider) + err := provider.HandleTokenRefresh(&TokenResult{ + IDToken: "test-id-token", + AccessToken: "test-access-token", + RefreshToken: "test-refresh-token", + }) + if err != nil { + t.Errorf("HandleTokenRefresh failed: %v", err) + } +} + +// TestAWSCognitoProvider_OfflineAccessFiltering tests that offline_access scope is always filtered out +func TestAWSCognitoProvider_OfflineAccessFiltering(t *testing.T) { + provider := NewAWSCognitoProvider() + baseParams := url.Values{} + + tests := []struct { + name string + scopes []string + }{ + { + name: "Single offline_access", + scopes: []string{"offline_access"}, + }, + { + name: "Multiple offline_access occurrences", + scopes: []string{"offline_access", "email", "offline_access", "profile"}, + }, + { + name: "Mixed case", + scopes: []string{"OFFLINE_ACCESS", "email"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + authParams, err := provider.BuildAuthParams(baseParams, tt.scopes) + if err != nil { + t.Errorf("BuildAuthParams failed: %v", err) + return + } + + // Ensure offline_access is NOT present in any form + for _, actualScope := range authParams.Scopes { + if actualScope == "offline_access" || actualScope == "OFFLINE_ACCESS" { + t.Errorf("offline_access scope should be filtered out, but found: %s", actualScope) + } + } + }) + } +} + +// TestAWSCognitoProvider_CognitoSpecificScopes tests AWS Cognito-specific scopes +func TestAWSCognitoProvider_CognitoSpecificScopes(t *testing.T) { + provider := NewAWSCognitoProvider() + baseParams := url.Values{} + + tests := []struct { + name string + scopes []string + checkFor []string + }{ + { + name: "Cognito admin scope", + scopes: []string{"aws.cognito.signin.user.admin"}, + checkFor: []string{"aws.cognito.signin.user.admin", "openid"}, + }, + { + name: "Phone scope", + scopes: []string{"phone"}, + checkFor: []string{"phone", "openid"}, + }, + { + name: "Address scope", + scopes: []string{"address"}, + checkFor: []string{"address", "openid"}, + }, + { + name: "Multiple Cognito scopes", + scopes: []string{"aws.cognito.signin.user.admin", "phone", "address"}, + checkFor: []string{"aws.cognito.signin.user.admin", "phone", "address", "openid"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + authParams, err := provider.BuildAuthParams(baseParams, tt.scopes) + if err != nil { + t.Errorf("BuildAuthParams failed: %v", err) + return + } + + for _, expectedScope := range tt.checkFor { + found := false + for _, actualScope := range authParams.Scopes { + if actualScope == expectedScope { + found = true + break + } + } + if !found { + t.Errorf("Expected scope '%s' not found in %v", expectedScope, authParams.Scopes) + } + } + }) + } +} + +// TestAWSCognitoProvider_DefaultScopeHandling tests default scope behavior +func TestAWSCognitoProvider_DefaultScopeHandling(t *testing.T) { + provider := NewAWSCognitoProvider() + baseParams := url.Values{} + + // Test with only openid scope - should add defaults + authParams, err := provider.BuildAuthParams(baseParams, []string{"openid"}) + if err != nil { + t.Errorf("BuildAuthParams failed: %v", err) + return + } + + expectedScopes := []string{"openid", "email", "profile"} + if len(authParams.Scopes) != len(expectedScopes) { + t.Errorf("Expected %d scopes, got %d", len(expectedScopes), len(authParams.Scopes)) + return + } + + for _, expectedScope := range expectedScopes { + found := false + for _, actualScope := range authParams.Scopes { + if actualScope == expectedScope { + found = true + break + } + } + if !found { + t.Errorf("Expected default scope '%s' not found in %v", expectedScope, authParams.Scopes) + } + } +} diff --git a/internal/providers/azure.go b/internal/providers/azure.go index 2497e7d..d5e27dc 100644 --- a/internal/providers/azure.go +++ b/internal/providers/azure.go @@ -49,7 +49,7 @@ func (p *AzureProvider) BuildAuthParams(baseParams url.Values, scopes []string) return &AuthParams{ URLValues: baseParams, - Scopes: scopes, + Scopes: deduplicateScopes(scopes), }, nil } diff --git a/internal/providers/azure_test.go b/internal/providers/azure_test.go index 6d4d2a3..01af64b 100644 --- a/internal/providers/azure_test.go +++ b/internal/providers/azure_test.go @@ -386,10 +386,10 @@ func TestAzureProvider_OfflineAccessHandling(t *testing.T) { description: "Should preserve existing offline_access", }, { - name: "Multiple offline_access - should preserve all", + name: "Multiple offline_access - should deduplicate", inputScopes: []string{"openid", "offline_access", "profile", "offline_access"}, - expectedCount: 2, - description: "Should preserve all offline_access scopes if multiple exist", + expectedCount: 1, + description: "Should deduplicate multiple offline_access scopes", }, { name: "Only offline_access", diff --git a/internal/providers/base.go b/internal/providers/base.go index 52be2d3..0cab63c 100644 --- a/internal/providers/base.go +++ b/internal/providers/base.go @@ -117,7 +117,7 @@ func (p *BaseProvider) BuildAuthParams(baseParams url.Values, scopes []string) ( return &AuthParams{ URLValues: baseParams, - Scopes: scopes, + Scopes: deduplicateScopes(scopes), }, nil } @@ -127,6 +127,21 @@ func (p *BaseProvider) HandleTokenRefresh(tokenData *TokenResult) error { return nil } +// deduplicateScopes removes duplicate scopes from a slice while preserving order. +func deduplicateScopes(scopes []string) []string { + seen := make(map[string]bool) + result := make([]string, 0, len(scopes)) + + for _, scope := range scopes { + if !seen[scope] { + seen[scope] = true + result = append(result, scope) + } + } + + return result +} + // ValidateConfig checks provider-specific configuration requirements. // By default, it assumes the configuration is valid. func (p *BaseProvider) ValidateConfig() error { diff --git a/internal/providers/factory_test.go b/internal/providers/factory_test.go index ae49ed9..beb94a7 100644 --- a/internal/providers/factory_test.go +++ b/internal/providers/factory_test.go @@ -181,6 +181,42 @@ func TestProviderFactory_CreateProviderByType(t *testing.T) { expectedType: ProviderTypeAzure, wantErr: false, }, + { + name: "GitHub provider", + providerType: ProviderTypeGitHub, + expectedType: ProviderTypeGitHub, + wantErr: false, + }, + { + name: "Auth0 provider", + providerType: ProviderTypeAuth0, + expectedType: ProviderTypeAuth0, + wantErr: false, + }, + { + name: "Okta provider", + providerType: ProviderTypeOkta, + expectedType: ProviderTypeOkta, + wantErr: false, + }, + { + name: "Keycloak provider", + providerType: ProviderTypeKeycloak, + expectedType: ProviderTypeKeycloak, + wantErr: false, + }, + { + name: "AWS Cognito provider", + providerType: ProviderTypeAWSCognito, + expectedType: ProviderTypeAWSCognito, + wantErr: false, + }, + { + name: "GitLab provider", + providerType: ProviderTypeGitLab, + expectedType: ProviderTypeGitLab, + wantErr: false, + }, { name: "Invalid provider type", providerType: ProviderType(999), diff --git a/internal/providers/github.go b/internal/providers/github.go index c14a61a..31ad408 100644 --- a/internal/providers/github.go +++ b/internal/providers/github.go @@ -51,7 +51,7 @@ func (p *GitHubProvider) BuildAuthParams(baseParams url.Values, scopes []string) return &AuthParams{ URLValues: baseParams, - Scopes: filteredScopes, + Scopes: deduplicateScopes(filteredScopes), }, nil } diff --git a/internal/providers/gitlab.go b/internal/providers/gitlab.go index 7dd36c1..df720f4 100644 --- a/internal/providers/gitlab.go +++ b/internal/providers/gitlab.go @@ -63,7 +63,7 @@ func (p *GitLabProvider) BuildAuthParams(baseParams url.Values, scopes []string) return &AuthParams{ URLValues: baseParams, - Scopes: filteredScopes, + Scopes: deduplicateScopes(filteredScopes), }, nil } diff --git a/internal/providers/gitlab_test.go b/internal/providers/gitlab_test.go new file mode 100644 index 0000000..233a39f --- /dev/null +++ b/internal/providers/gitlab_test.go @@ -0,0 +1,322 @@ +package providers + +import ( + "net/url" + "testing" +) + +// TestGitLabProvider_NewGitLabProvider tests the constructor +func TestGitLabProvider_NewGitLabProvider(t *testing.T) { + provider := NewGitLabProvider() + + if provider == nil { + t.Fatal("Expected provider to be created, got nil") + } + + if provider.BaseProvider == nil { + t.Error("BaseProvider should be initialized") + } +} + +// TestGitLabProvider_GetType tests provider type +func TestGitLabProvider_GetType(t *testing.T) { + provider := NewGitLabProvider() + + if provider.GetType() != ProviderTypeGitLab { + t.Errorf("Expected ProviderTypeGitLab, got %v", provider.GetType()) + } +} + +// TestGitLabProvider_GetCapabilities tests GitLab-specific capabilities +func TestGitLabProvider_GetCapabilities(t *testing.T) { + provider := NewGitLabProvider() + capabilities := provider.GetCapabilities() + + if !capabilities.SupportsRefreshTokens { + t.Error("Expected SupportsRefreshTokens to be true for GitLab") + } + + if capabilities.RequiresOfflineAccessScope { + t.Error("Expected RequiresOfflineAccessScope to be false for GitLab") + } + + if capabilities.RequiresPromptConsent { + t.Error("Expected RequiresPromptConsent to be false for GitLab") + } + + if capabilities.PreferredTokenValidation != "id" { + t.Errorf("Expected PreferredTokenValidation 'id', got '%s'", capabilities.PreferredTokenValidation) + } +} + +// TestGitLabProvider_BuildAuthParams tests GitLab-specific auth params +func TestGitLabProvider_BuildAuthParams(t *testing.T) { + provider := NewGitLabProvider() + baseParams := url.Values{} + baseParams.Set("client_id", "test_client") + + tests := []struct { + name string + scopes []string + expectedScopes []string + }{ + { + name: "Remove offline_access scope and ensure openid", + scopes: []string{"read_user", "read_api", "offline_access"}, + expectedScopes: []string{"read_user", "read_api", "openid"}, + }, + { + name: "Keep existing openid, remove offline_access", + scopes: []string{"openid", "read_user", "offline_access", "profile"}, + expectedScopes: []string{"openid", "read_user", "profile"}, + }, + { + name: "Add default scopes when only openid", + scopes: []string{"openid"}, + expectedScopes: []string{"openid", "profile", "email"}, + }, + { + name: "Add openid and defaults when empty", + scopes: []string{}, + expectedScopes: []string{"openid", "profile", "email"}, + }, + { + name: "GitLab-specific scopes", + scopes: []string{"read_user", "read_api", "read_repository"}, + expectedScopes: []string{"read_user", "read_api", "read_repository", "openid"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + authParams, err := provider.BuildAuthParams(baseParams, tt.scopes) + if err != nil { + t.Errorf("BuildAuthParams failed: %v", err) + return + } + + // Check that response_type is set + if authParams.URLValues.Get("response_type") != "code" { + t.Errorf("Expected response_type 'code', got '%s'", authParams.URLValues.Get("response_type")) + } + + if len(authParams.Scopes) != len(tt.expectedScopes) { + t.Errorf("Expected %d scopes, got %d. Expected: %v, Got: %v", + len(tt.expectedScopes), len(authParams.Scopes), tt.expectedScopes, authParams.Scopes) + return + } + + // Check that all expected scopes are present + for _, expectedScope := range tt.expectedScopes { + found := false + for _, actualScope := range authParams.Scopes { + if actualScope == expectedScope { + found = true + break + } + } + if !found { + t.Errorf("Expected scope '%s' not found in %v", expectedScope, authParams.Scopes) + } + } + + // Ensure offline_access is NOT present + for _, actualScope := range authParams.Scopes { + if actualScope == "offline_access" { + t.Error("offline_access scope should be filtered out for GitLab") + } + } + }) + } +} + +// TestGitLabProvider_ValidateConfig tests config validation +func TestGitLabProvider_ValidateConfig(t *testing.T) { + provider := NewGitLabProvider() + + err := provider.ValidateConfig() + if err != nil { + t.Errorf("ValidateConfig failed: %v", err) + } +} + +// TestGitLabProvider_InterfaceCompliance tests that GitLab provider implements the OIDCProvider interface +func TestGitLabProvider_InterfaceCompliance(t *testing.T) { + var _ OIDCProvider = NewGitLabProvider() +} + +// TestGitLabProvider_BaseProviderInheritance tests that GitLab provider inherits from BaseProvider correctly +func TestGitLabProvider_BaseProviderInheritance(t *testing.T) { + provider := NewGitLabProvider() + + // Test that it has access to BaseProvider methods + if provider.BaseProvider == nil { + t.Error("Expected BaseProvider to be initialized") + } + + // Test HandleTokenRefresh (inherited from BaseProvider) + err := provider.HandleTokenRefresh(&TokenResult{ + IDToken: "test-id-token", + AccessToken: "test-access-token", + RefreshToken: "test-refresh-token", + }) + if err != nil { + t.Errorf("HandleTokenRefresh failed: %v", err) + } +} + +// TestGitLabProvider_OfflineAccessFiltering tests that offline_access scope is always filtered out +func TestGitLabProvider_OfflineAccessFiltering(t *testing.T) { + provider := NewGitLabProvider() + baseParams := url.Values{} + + tests := []struct { + name string + scopes []string + }{ + { + name: "Single offline_access", + scopes: []string{"offline_access"}, + }, + { + name: "Multiple offline_access occurrences", + scopes: []string{"offline_access", "read_user", "offline_access", "profile"}, + }, + { + name: "Mixed with other scopes", + scopes: []string{"read_api", "offline_access", "read_user"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + authParams, err := provider.BuildAuthParams(baseParams, tt.scopes) + if err != nil { + t.Errorf("BuildAuthParams failed: %v", err) + return + } + + // Ensure offline_access is NOT present + for _, actualScope := range authParams.Scopes { + if actualScope == "offline_access" { + t.Error("offline_access scope should be filtered out for GitLab") + } + } + }) + } +} + +// TestGitLabProvider_GitLabSpecificScopes tests GitLab-specific scopes +func TestGitLabProvider_GitLabSpecificScopes(t *testing.T) { + provider := NewGitLabProvider() + baseParams := url.Values{} + + tests := []struct { + name string + scopes []string + checkFor []string + }{ + { + name: "GitLab API scopes", + scopes: []string{"read_api", "read_user"}, + checkFor: []string{"read_api", "read_user", "openid"}, + }, + { + name: "GitLab repository scopes", + scopes: []string{"read_repository", "write_repository"}, + checkFor: []string{"read_repository", "write_repository", "openid"}, + }, + { + name: "GitLab admin scopes", + scopes: []string{"api", "sudo"}, + checkFor: []string{"api", "sudo", "openid"}, + }, + { + name: "GitLab registry scopes", + scopes: []string{"read_registry", "write_registry"}, + checkFor: []string{"read_registry", "write_registry", "openid"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + authParams, err := provider.BuildAuthParams(baseParams, tt.scopes) + if err != nil { + t.Errorf("BuildAuthParams failed: %v", err) + return + } + + for _, expectedScope := range tt.checkFor { + found := false + for _, actualScope := range authParams.Scopes { + if actualScope == expectedScope { + found = true + break + } + } + if !found { + t.Errorf("Expected scope '%s' not found in %v", expectedScope, authParams.Scopes) + } + } + }) + } +} + +// TestGitLabProvider_DefaultScopeHandling tests default scope behavior +func TestGitLabProvider_DefaultScopeHandling(t *testing.T) { + provider := NewGitLabProvider() + baseParams := url.Values{} + + // Test with only openid scope - should add defaults + authParams, err := provider.BuildAuthParams(baseParams, []string{"openid"}) + if err != nil { + t.Errorf("BuildAuthParams failed: %v", err) + return + } + + expectedScopes := []string{"openid", "profile", "email"} + if len(authParams.Scopes) != len(expectedScopes) { + t.Errorf("Expected %d scopes, got %d", len(expectedScopes), len(authParams.Scopes)) + return + } + + for _, expectedScope := range expectedScopes { + found := false + for _, actualScope := range authParams.Scopes { + if actualScope == expectedScope { + found = true + break + } + } + if !found { + t.Errorf("Expected default scope '%s' not found in %v", expectedScope, authParams.Scopes) + } + } +} + +// TestGitLabProvider_ScopeDeduplication tests that duplicate scopes are handled correctly +func TestGitLabProvider_ScopeDeduplication(t *testing.T) { + provider := NewGitLabProvider() + baseParams := url.Values{} + + // Test with duplicate scopes + scopes := []string{"openid", "read_user", "openid", "profile", "read_user"} + authParams, err := provider.BuildAuthParams(baseParams, scopes) + if err != nil { + t.Errorf("BuildAuthParams failed: %v", err) + return + } + + // Count occurrences of each scope + scopeCounts := make(map[string]int) + for _, scope := range authParams.Scopes { + scopeCounts[scope]++ + } + + // Check that no scope appears more than once + for scope, count := range scopeCounts { + if count > 1 { + t.Errorf("Scope '%s' appears %d times, expected 1", scope, count) + } + } +} diff --git a/internal/providers/google.go b/internal/providers/google.go index a3aea4a..97e1bee 100644 --- a/internal/providers/google.go +++ b/internal/providers/google.go @@ -46,7 +46,7 @@ func (p *GoogleProvider) BuildAuthParams(baseParams url.Values, scopes []string) return &AuthParams{ URLValues: baseParams, - Scopes: filteredScopes, + Scopes: deduplicateScopes(filteredScopes), }, nil } diff --git a/internal/providers/keycloak.go b/internal/providers/keycloak.go index 542efed..d289555 100644 --- a/internal/providers/keycloak.go +++ b/internal/providers/keycloak.go @@ -62,7 +62,7 @@ func (p *KeycloakProvider) BuildAuthParams(baseParams url.Values, scopes []strin return &AuthParams{ URLValues: baseParams, - Scopes: scopes, + Scopes: deduplicateScopes(scopes), }, nil } diff --git a/internal/providers/keycloak_test.go b/internal/providers/keycloak_test.go new file mode 100644 index 0000000..be72e73 --- /dev/null +++ b/internal/providers/keycloak_test.go @@ -0,0 +1,232 @@ +package providers + +import ( + "net/url" + "testing" +) + +// TestKeycloakProvider_NewKeycloakProvider tests the constructor +func TestKeycloakProvider_NewKeycloakProvider(t *testing.T) { + provider := NewKeycloakProvider() + + if provider == nil { + t.Fatal("Expected provider to be created, got nil") + } + + if provider.BaseProvider == nil { + t.Error("BaseProvider should be initialized") + } +} + +// TestKeycloakProvider_GetType tests provider type +func TestKeycloakProvider_GetType(t *testing.T) { + provider := NewKeycloakProvider() + + if provider.GetType() != ProviderTypeKeycloak { + t.Errorf("Expected ProviderTypeKeycloak, got %v", provider.GetType()) + } +} + +// TestKeycloakProvider_GetCapabilities tests Keycloak-specific capabilities +func TestKeycloakProvider_GetCapabilities(t *testing.T) { + provider := NewKeycloakProvider() + capabilities := provider.GetCapabilities() + + if !capabilities.SupportsRefreshTokens { + t.Error("Expected SupportsRefreshTokens to be true for Keycloak") + } + + if !capabilities.RequiresOfflineAccessScope { + t.Error("Expected RequiresOfflineAccessScope to be true for Keycloak") + } + + if capabilities.RequiresPromptConsent { + t.Error("Expected RequiresPromptConsent to be false for Keycloak") + } + + if capabilities.PreferredTokenValidation != "id" { + t.Errorf("Expected PreferredTokenValidation 'id', got '%s'", capabilities.PreferredTokenValidation) + } +} + +// TestKeycloakProvider_BuildAuthParams tests Keycloak-specific auth params +func TestKeycloakProvider_BuildAuthParams(t *testing.T) { + provider := NewKeycloakProvider() + baseParams := url.Values{} + baseParams.Set("client_id", "test_client") + + tests := []struct { + name string + scopes []string + expectedScopes []string + }{ + { + name: "Add offline_access and openid scopes", + scopes: []string{"roles", "groups"}, + expectedScopes: []string{"roles", "groups", "offline_access", "openid"}, + }, + { + name: "Keep existing offline_access and openid", + scopes: []string{"openid", "roles", "offline_access", "groups"}, + expectedScopes: []string{"openid", "roles", "offline_access", "groups"}, + }, + { + name: "Add both scopes when none provided", + scopes: []string{}, + expectedScopes: []string{"offline_access", "openid"}, + }, + { + name: "Keycloak custom scopes", + scopes: []string{"realm-roles", "account"}, + expectedScopes: []string{"realm-roles", "account", "offline_access", "openid"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + authParams, err := provider.BuildAuthParams(baseParams, tt.scopes) + if err != nil { + t.Errorf("BuildAuthParams failed: %v", err) + return + } + + // Check that response_type is set + if authParams.URLValues.Get("response_type") != "code" { + t.Errorf("Expected response_type 'code', got '%s'", authParams.URLValues.Get("response_type")) + } + + if len(authParams.Scopes) != len(tt.expectedScopes) { + t.Errorf("Expected %d scopes, got %d. Expected: %v, Got: %v", + len(tt.expectedScopes), len(authParams.Scopes), tt.expectedScopes, authParams.Scopes) + return + } + + // Check that all expected scopes are present + for _, expectedScope := range tt.expectedScopes { + found := false + for _, actualScope := range authParams.Scopes { + if actualScope == expectedScope { + found = true + break + } + } + if !found { + t.Errorf("Expected scope '%s' not found in %v", expectedScope, authParams.Scopes) + } + } + }) + } +} + +// TestKeycloakProvider_ValidateConfig tests config validation +func TestKeycloakProvider_ValidateConfig(t *testing.T) { + provider := NewKeycloakProvider() + + err := provider.ValidateConfig() + if err != nil { + t.Errorf("ValidateConfig failed: %v", err) + } +} + +// TestKeycloakProvider_InterfaceCompliance tests that Keycloak provider implements the OIDCProvider interface +func TestKeycloakProvider_InterfaceCompliance(t *testing.T) { + var _ OIDCProvider = NewKeycloakProvider() +} + +// TestKeycloakProvider_BaseProviderInheritance tests that Keycloak provider inherits from BaseProvider correctly +func TestKeycloakProvider_BaseProviderInheritance(t *testing.T) { + provider := NewKeycloakProvider() + + // Test that it has access to BaseProvider methods + if provider.BaseProvider == nil { + t.Error("Expected BaseProvider to be initialized") + } + + // Test HandleTokenRefresh (inherited from BaseProvider) + err := provider.HandleTokenRefresh(&TokenResult{ + IDToken: "test-id-token", + AccessToken: "test-access-token", + RefreshToken: "test-refresh-token", + }) + if err != nil { + t.Errorf("HandleTokenRefresh failed: %v", err) + } +} + +// TestKeycloakProvider_RealmSpecificScopes tests Keycloak realm-specific scopes +func TestKeycloakProvider_RealmSpecificScopes(t *testing.T) { + provider := NewKeycloakProvider() + baseParams := url.Values{} + + tests := []struct { + name string + scopes []string + checkFor []string + }{ + { + name: "Keycloak standard scopes", + scopes: []string{"roles", "groups", "profile", "email"}, + checkFor: []string{"roles", "groups", "profile", "email", "offline_access", "openid"}, + }, + { + name: "Keycloak realm roles", + scopes: []string{"realm-roles", "client-roles"}, + checkFor: []string{"realm-roles", "client-roles", "offline_access", "openid"}, + }, + { + name: "Keycloak account service", + scopes: []string{"account"}, + checkFor: []string{"account", "offline_access", "openid"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + authParams, err := provider.BuildAuthParams(baseParams, tt.scopes) + if err != nil { + t.Errorf("BuildAuthParams failed: %v", err) + return + } + + for _, expectedScope := range tt.checkFor { + found := false + for _, actualScope := range authParams.Scopes { + if actualScope == expectedScope { + found = true + break + } + } + if !found { + t.Errorf("Expected scope '%s' not found in %v", expectedScope, authParams.Scopes) + } + } + }) + } +} + +// TestKeycloakProvider_ScopeDeduplication tests that duplicate scopes are handled correctly +func TestKeycloakProvider_ScopeDeduplication(t *testing.T) { + provider := NewKeycloakProvider() + baseParams := url.Values{} + + // Test with duplicate scopes + scopes := []string{"openid", "profile", "offline_access", "roles", "openid", "profile"} + authParams, err := provider.BuildAuthParams(baseParams, scopes) + if err != nil { + t.Errorf("BuildAuthParams failed: %v", err) + return + } + + // Count occurrences of each scope + scopeCounts := make(map[string]int) + for _, scope := range authParams.Scopes { + scopeCounts[scope]++ + } + + // Check that no scope appears more than once + for scope, count := range scopeCounts { + if count > 1 { + t.Errorf("Scope '%s' appears %d times, expected 1", scope, count) + } + } +} diff --git a/internal/providers/okta.go b/internal/providers/okta.go index f0bc5bd..3daeada 100644 --- a/internal/providers/okta.go +++ b/internal/providers/okta.go @@ -62,7 +62,7 @@ func (p *OktaProvider) BuildAuthParams(baseParams url.Values, scopes []string) ( return &AuthParams{ URLValues: baseParams, - Scopes: scopes, + Scopes: deduplicateScopes(scopes), }, nil } diff --git a/internal/providers/okta_test.go b/internal/providers/okta_test.go new file mode 100644 index 0000000..8cf7f9b --- /dev/null +++ b/internal/providers/okta_test.go @@ -0,0 +1,200 @@ +package providers + +import ( + "net/url" + "testing" +) + +// TestOktaProvider_NewOktaProvider tests the constructor +func TestOktaProvider_NewOktaProvider(t *testing.T) { + provider := NewOktaProvider() + + if provider == nil { + t.Fatal("Expected provider to be created, got nil") + } + + if provider.BaseProvider == nil { + t.Error("BaseProvider should be initialized") + } +} + +// TestOktaProvider_GetType tests provider type +func TestOktaProvider_GetType(t *testing.T) { + provider := NewOktaProvider() + + if provider.GetType() != ProviderTypeOkta { + t.Errorf("Expected ProviderTypeOkta, got %v", provider.GetType()) + } +} + +// TestOktaProvider_GetCapabilities tests Okta-specific capabilities +func TestOktaProvider_GetCapabilities(t *testing.T) { + provider := NewOktaProvider() + capabilities := provider.GetCapabilities() + + if !capabilities.SupportsRefreshTokens { + t.Error("Expected SupportsRefreshTokens to be true for Okta") + } + + if !capabilities.RequiresOfflineAccessScope { + t.Error("Expected RequiresOfflineAccessScope to be true for Okta") + } + + if capabilities.RequiresPromptConsent { + t.Error("Expected RequiresPromptConsent to be false for Okta") + } + + if capabilities.PreferredTokenValidation != "id" { + t.Errorf("Expected PreferredTokenValidation 'id', got '%s'", capabilities.PreferredTokenValidation) + } +} + +// TestOktaProvider_BuildAuthParams tests Okta-specific auth params +func TestOktaProvider_BuildAuthParams(t *testing.T) { + provider := NewOktaProvider() + baseParams := url.Values{} + baseParams.Set("client_id", "test_client") + + tests := []struct { + name string + scopes []string + expectedScopes []string + }{ + { + name: "Add offline_access and openid scopes", + scopes: []string{"groups", "profile"}, + expectedScopes: []string{"groups", "profile", "offline_access", "openid"}, + }, + { + name: "Keep existing offline_access and openid", + scopes: []string{"openid", "groups", "offline_access", "profile"}, + expectedScopes: []string{"openid", "groups", "offline_access", "profile"}, + }, + { + name: "Add both scopes when none provided", + scopes: []string{}, + expectedScopes: []string{"offline_access", "openid"}, + }, + { + name: "Add openid when only offline_access present", + scopes: []string{"offline_access"}, + expectedScopes: []string{"offline_access", "openid"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + authParams, err := provider.BuildAuthParams(baseParams, tt.scopes) + if err != nil { + t.Errorf("BuildAuthParams failed: %v", err) + return + } + + // Check that response_type is set + if authParams.URLValues.Get("response_type") != "code" { + t.Errorf("Expected response_type 'code', got '%s'", authParams.URLValues.Get("response_type")) + } + + if len(authParams.Scopes) != len(tt.expectedScopes) { + t.Errorf("Expected %d scopes, got %d. Expected: %v, Got: %v", + len(tt.expectedScopes), len(authParams.Scopes), tt.expectedScopes, authParams.Scopes) + return + } + + // Check that all expected scopes are present + for _, expectedScope := range tt.expectedScopes { + found := false + for _, actualScope := range authParams.Scopes { + if actualScope == expectedScope { + found = true + break + } + } + if !found { + t.Errorf("Expected scope '%s' not found in %v", expectedScope, authParams.Scopes) + } + } + }) + } +} + +// TestOktaProvider_ValidateConfig tests config validation +func TestOktaProvider_ValidateConfig(t *testing.T) { + provider := NewOktaProvider() + + err := provider.ValidateConfig() + if err != nil { + t.Errorf("ValidateConfig failed: %v", err) + } +} + +// TestOktaProvider_InterfaceCompliance tests that Okta provider implements the OIDCProvider interface +func TestOktaProvider_InterfaceCompliance(t *testing.T) { + var _ OIDCProvider = NewOktaProvider() +} + +// TestOktaProvider_BaseProviderInheritance tests that Okta provider inherits from BaseProvider correctly +func TestOktaProvider_BaseProviderInheritance(t *testing.T) { + provider := NewOktaProvider() + + // Test that it has access to BaseProvider methods + if provider.BaseProvider == nil { + t.Error("Expected BaseProvider to be initialized") + } + + // Test HandleTokenRefresh (inherited from BaseProvider) + err := provider.HandleTokenRefresh(&TokenResult{ + IDToken: "test-id-token", + AccessToken: "test-access-token", + RefreshToken: "test-refresh-token", + }) + if err != nil { + t.Errorf("HandleTokenRefresh failed: %v", err) + } +} + +// TestOktaProvider_ScopeHandling tests Okta-specific scope handling +func TestOktaProvider_ScopeHandling(t *testing.T) { + provider := NewOktaProvider() + baseParams := url.Values{} + + tests := []struct { + name string + scopes []string + checkFor []string + }{ + { + name: "Groups scope handling", + scopes: []string{"groups", "profile"}, + checkFor: []string{"groups", "profile", "offline_access", "openid"}, + }, + { + name: "Custom Okta scopes", + scopes: []string{"okta.users.read", "okta.groups.read"}, + checkFor: []string{"okta.users.read", "okta.groups.read", "offline_access", "openid"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + authParams, err := provider.BuildAuthParams(baseParams, tt.scopes) + if err != nil { + t.Errorf("BuildAuthParams failed: %v", err) + return + } + + for _, expectedScope := range tt.checkFor { + found := false + for _, actualScope := range authParams.Scopes { + if actualScope == expectedScope { + found = true + break + } + } + if !found { + t.Errorf("Expected scope '%s' not found in %v", expectedScope, authParams.Scopes) + } + } + }) + } +} diff --git a/internal/providers/registry_test.go b/internal/providers/registry_test.go index d928d48..05d1a29 100644 --- a/internal/providers/registry_test.go +++ b/internal/providers/registry_test.go @@ -161,10 +161,22 @@ func TestProviderRegistry_DetectProvider(t *testing.T) { genericProvider := NewGenericProvider() googleProvider := NewGoogleProvider() azureProvider := NewAzureProvider() + githubProvider := NewGitHubProvider() + auth0Provider := NewAuth0Provider() + oktaProvider := NewOktaProvider() + keycloakProvider := NewKeycloakProvider() + cognitoProvider := NewAWSCognitoProvider() + gitlabProvider := NewGitLabProvider() registry.RegisterProvider(genericProvider) registry.RegisterProvider(googleProvider) registry.RegisterProvider(azureProvider) + registry.RegisterProvider(githubProvider) + registry.RegisterProvider(auth0Provider) + registry.RegisterProvider(oktaProvider) + registry.RegisterProvider(keycloakProvider) + registry.RegisterProvider(cognitoProvider) + registry.RegisterProvider(gitlabProvider) tests := []struct { name string @@ -191,6 +203,41 @@ func TestProviderRegistry_DetectProvider(t *testing.T) { issuerURL: "https://sts.windows.net/tenant", expected: azureProvider, }, + { + name: "GitHub provider detection", + issuerURL: "https://github.com/login/oauth", + expected: githubProvider, + }, + { + name: "Auth0 provider detection", + issuerURL: "https://tenant.auth0.com", + expected: auth0Provider, + }, + { + name: "Okta provider detection", + issuerURL: "https://tenant.okta.com", + expected: oktaProvider, + }, + { + name: "Okta preview provider detection", + issuerURL: "https://tenant.oktapreview.com", + expected: oktaProvider, + }, + { + name: "Keycloak provider detection", + issuerURL: "https://auth.example.com/auth/realms/master", + expected: keycloakProvider, + }, + { + name: "AWS Cognito provider detection", + issuerURL: "https://cognito-idp.us-east-1.amazonaws.com/us-east-1_example", + expected: cognitoProvider, + }, + { + name: "GitLab provider detection", + issuerURL: "https://gitlab.com/oauth", + expected: gitlabProvider, + }, { name: "Generic provider fallback", issuerURL: "https://auth.example.com",