diff --git a/openapi/signin/api.go b/openapi/signin/api.go index 493eece5..7e539198 100644 --- a/openapi/signin/api.go +++ b/openapi/signin/api.go @@ -131,7 +131,7 @@ func authback(c *gin.Context) { } // Get provider - provider, err := GetProvider(params.Locale, providerID) + provider, err := GetProvider(providerID) if err != nil { errorResp := &response.ErrorResponse{ Code: response.ErrInvalidRequest.Code, @@ -227,9 +227,8 @@ func getOAuthAuthorizationURL(c *gin.Context) { // Get optional parameters redirectURI := c.Query("redirect_uri") state := c.Query("state") - locale := c.Query("locale") - provider, err := GetProvider(locale, providerID) + provider, err := GetProvider(providerID) if err != nil { errorResp := &response.ErrorResponse{ Code: response.ErrInvalidRequest.Code, diff --git a/openapi/signin/provider.go b/openapi/signin/provider.go index e8e3c3dd..e0a9ce4e 100644 --- a/openapi/signin/provider.go +++ b/openapi/signin/provider.go @@ -1158,27 +1158,11 @@ func (p *Provider) AccessToken(code, redirectURI string) (*OAuthTokenResponse, e return &tokenResponse, nil } -// GetProvider gets the provider by ID -func GetProvider(locale, providerID string) (*Provider, error) { - - // Get the signin configuration - config := GetFullConfig(locale) - if config == nil { - return nil, fmt.Errorf("no signin configuration found") - } - - // Find the provider - var provider *Provider - if config.ThirdParty != nil && config.ThirdParty.Providers != nil { - for _, p := range config.ThirdParty.Providers { - if p.ID == providerID { - provider = p - break - } - } - } - - if provider == nil { +// GetProvider gets the provider by ID from the global providers map +func GetProvider(providerID string) (*Provider, error) { + // Get provider from global providers map + provider, exists := providers[providerID] + if !exists { return nil, fmt.Errorf("OAuth provider '%s' not found", providerID) } diff --git a/openapi/signin/signin.go b/openapi/signin/signin.go index 504847b3..4be87d26 100644 --- a/openapi/signin/signin.go +++ b/openapi/signin/signin.go @@ -21,13 +21,15 @@ var ( fullConfigs = make(map[string]*Config) // Public configurations without sensitive data (for frontend use) publicConfigs = make(map[string]*Config) - // Default language code - defaultLang = "" + // Global providers map (decoupled from locale-specific configs) + providers = make(map[string]*Provider) + // Default configuration (marked with default: true) + defaultConfig *Config // Mutex for thread safety configMutex sync.RWMutex ) -// Load loads all signin configurations from the openapi directory +// Load loads all signin configurations from the openapi/signin directory func Load(appConfig config.Config) error { configMutex.Lock() defer configMutex.Unlock() @@ -35,40 +37,112 @@ func Load(appConfig config.Config) error { // Clear existing configurations fullConfigs = make(map[string]*Config) publicConfigs = make(map[string]*Config) - defaultLang = "" + providers = make(map[string]*Provider) + defaultConfig = nil - // Find all signin configuration files - files, err := findSigninFiles() + // Load providers first + err := loadProviders(appConfig.Root) if err != nil { - return fmt.Errorf("failed to find signin files: %v", err) + return fmt.Errorf("failed to load providers: %v", err) } - // If no signin files found, that's not necessarily an error - // Some applications might not have signin configurations - if len(files) == 0 { - return nil + // Load signin configurations + err = loadSigninConfigs(appConfig.Root) + if err != nil { + return fmt.Errorf("failed to load signin configs: %v", err) } - // Load each configuration file - for _, file := range files { - lang := extractLanguageFromFilename(file) + return nil +} - configPath := filepath.Join("openapi", file) - configRaw, err := application.App.Read(configPath) +// loadProviders loads all provider configurations from the openapi/signin/providers directory +func loadProviders(rootPath string) error { + // Use Walk to find all provider files in the signin/providers directory + err := application.App.Walk("openapi/signin/providers", func(root, filename string, isdir bool) error { + if isdir { + return nil + } + + // Only process .yao files + if !strings.HasSuffix(filename, ".yao") { + return nil + } + + // Extract provider ID from filename (basename without extension) + baseName := filepath.Base(filename) + providerID := strings.TrimSuffix(baseName, ".yao") + + // Read provider configuration + configRaw, err := application.App.Read(filename) if err != nil { - return fmt.Errorf("failed to read signin config %s: %v", file, err) + return fmt.Errorf("failed to read provider config %s: %v", filename, err) + } + + // Parse the provider configuration + var provider Provider + err = application.Parse(filename, configRaw, &provider) + if err != nil { + return fmt.Errorf("failed to parse provider config %s: %v", filename, err) + } + + // Set the provider ID + provider.ID = providerID + + // Process ENV variables in provider config + processProviderENVVariables(&provider, rootPath) + + // Store the provider + providers[providerID] = &provider + + return nil + }, "*.yao") + + return err +} + +// loadSigninConfigs loads all signin configurations from the openapi/signin directory +func loadSigninConfigs(rootPath string) error { + // Use Walk to find all signin config files in the signin directory (but not subdirectories) + err := application.App.Walk("openapi/signin", func(root, filename string, isdir bool) error { + if isdir { + return nil + } + + // Skip files in subdirectories (like providers/) + if filepath.Dir(filename) != "openapi/signin" { + return nil + } + + // Only process .yao files + if !strings.HasSuffix(filename, ".yao") { + return nil + } + + // Extract language code from filename + baseName := filepath.Base(filename) + lang := extractLanguageFromFilename(baseName) + + // Read signin configuration + configRaw, err := application.App.Read(filename) + if err != nil { + return fmt.Errorf("failed to read signin config %s: %v", filename, err) } // Parse the configuration var signinConfig Config - err = application.Parse(configPath, configRaw, &signinConfig) + err = application.Parse(filename, configRaw, &signinConfig) if err != nil { - return fmt.Errorf("failed to parse signin config %s: %v", file, err) + return fmt.Errorf("failed to parse signin config %s: %v", filename, err) } // Process ENV variables in full config fullConfig := signinConfig - processENVVariables(&fullConfig, appConfig.Root) + processConfigENVVariables(&fullConfig, rootPath) + + // Set as default config if marked as default + if fullConfig.Default { + defaultConfig = &fullConfig + } // Create public config (without sensitive data) publicConfig := createPublicConfig(&fullConfig) @@ -77,63 +151,132 @@ func Load(appConfig config.Config) error { fullConfigs[lang] = &fullConfig publicConfigs[lang] = &publicConfig - // Set default language - if defaultLang == "" || lang == "en" || file == "signin.yao" { - defaultLang = lang - } - } - - return nil -} - -// findSigninFiles finds all signin configuration files in the openapi directory -func findSigninFiles() ([]string, error) { - var files []string - signinFilePattern := regexp.MustCompile(`^signin(\.[a-z]{2}(-[a-z]{2})?)?\.yao$`) - - // Use Walk to find all signin files in the openapi directory - err := application.App.Walk("openapi", func(root, filename string, isdir bool) error { - if isdir { - return nil - } - - baseName := filepath.Base(filename) - if signinFilePattern.MatchString(baseName) { - files = append(files, baseName) - } - return nil }, "*.yao") - if err != nil { - return nil, err - } - - return files, nil + return err } // extractLanguageFromFilename extracts language code from filename func extractLanguageFromFilename(filename string) string { - // signin.yao -> "" - // signin.en.yao -> "en" - // signin.zh-cn.yao -> "zh-cn" + // New naming convention: + // en.yao -> "en" + // zh-cn.yao -> "zh-cn" + // default.yao -> "default" - if filename == "signin.yao" { - return "" - } - - parts := strings.Split(filename, ".") - if len(parts) >= 3 { - return parts[1] - } - - return "" + baseName := strings.TrimSuffix(filename, ".yao") + return strings.ToLower(baseName) } -// processENVVariables processes environment variables in the configuration -func processENVVariables(config *Config, rootPath string) { +// processProviderENVVariables processes environment variables in the provider configuration +func processProviderENVVariables(provider *Provider, rootPath string) { var missingEnvVars []string + // Process ClientID + if strings.HasPrefix(provider.ClientID, "$ENV.") { + envVar := strings.TrimPrefix(provider.ClientID, "$ENV.") + if _, exists := os.LookupEnv(envVar); !exists { + missingEnvVars = append(missingEnvVars, envVar) + } + } + provider.ClientID = replaceENVVar(provider.ClientID) + + // Process ClientSecret + if strings.HasPrefix(provider.ClientSecret, "$ENV.") { + envVar := strings.TrimPrefix(provider.ClientSecret, "$ENV.") + if _, exists := os.LookupEnv(envVar); !exists { + missingEnvVars = append(missingEnvVars, envVar) + } + } + provider.ClientSecret = replaceENVVar(provider.ClientSecret) + + // Process client secret generator + if provider.ClientSecretGenerator != nil { + // Check PrivateKey + if strings.HasPrefix(provider.ClientSecretGenerator.PrivateKey, "$ENV.") { + envVar := strings.TrimPrefix(provider.ClientSecretGenerator.PrivateKey, "$ENV.") + if _, exists := os.LookupEnv(envVar); !exists { + missingEnvVars = append(missingEnvVars, envVar) + } + } + provider.ClientSecretGenerator.PrivateKey = replaceENVVar(provider.ClientSecretGenerator.PrivateKey) + + // Convert relative path to absolute path for private key + if provider.ClientSecretGenerator.PrivateKey != "" && !filepath.IsAbs(provider.ClientSecretGenerator.PrivateKey) { + provider.ClientSecretGenerator.PrivateKey = filepath.Join(rootPath, "openapi", "certs", provider.ClientSecretGenerator.PrivateKey) + } + + // Process and normalize expires_in format + if provider.ClientSecretGenerator.ExpiresIn != "" { + normalizedDuration, err := normalizeExpiresIn(provider.ClientSecretGenerator.ExpiresIn) + if err != nil { + log.Printf("Warning: Invalid expires_in format '%s' for provider '%s': %v", + provider.ClientSecretGenerator.ExpiresIn, provider.ID, err) + // Set default to 90 days + provider.ClientSecretGenerator.ExpiresIn = "2160h" // 90 * 24 hours + } else { + provider.ClientSecretGenerator.ExpiresIn = normalizedDuration + } + } + + // Process header values + if provider.ClientSecretGenerator.Header != nil { + for key, value := range provider.ClientSecretGenerator.Header { + if strValue, ok := value.(string); ok { + if strings.HasPrefix(strValue, "$ENV.") { + envVar := strings.TrimPrefix(strValue, "$ENV.") + if _, exists := os.LookupEnv(envVar); !exists { + missingEnvVars = append(missingEnvVars, envVar) + } + } + provider.ClientSecretGenerator.Header[key] = replaceENVVar(strValue) + } + } + } + + // Process payload values + if provider.ClientSecretGenerator.Payload != nil { + for key, value := range provider.ClientSecretGenerator.Payload { + if strValue, ok := value.(string); ok { + if strings.HasPrefix(strValue, "$ENV.") { + envVar := strings.TrimPrefix(strValue, "$ENV.") + if _, exists := os.LookupEnv(envVar); !exists { + missingEnvVars = append(missingEnvVars, envVar) + } + } + provider.ClientSecretGenerator.Payload[key] = replaceENVVar(strValue) + } + } + } + } + + // Log warning for missing environment variables + if len(missingEnvVars) > 0 { + log.Printf("Warning: The following environment variables are not set for provider '%s': %v", provider.ID, missingEnvVars) + } +} + +// processConfigENVVariables processes environment variables in the signin configuration +func processConfigENVVariables(config *Config, rootPath string) { + var missingEnvVars []string + + // Process client_id and client_secret + if strings.HasPrefix(config.ClientID, "$ENV.") { + envVar := strings.TrimPrefix(config.ClientID, "$ENV.") + if _, exists := os.LookupEnv(envVar); !exists { + missingEnvVars = append(missingEnvVars, envVar) + } + } + config.ClientID = replaceENVVar(config.ClientID) + + if strings.HasPrefix(config.ClientSecret, "$ENV.") { + envVar := strings.TrimPrefix(config.ClientSecret, "$ENV.") + if _, exists := os.LookupEnv(envVar); !exists { + missingEnvVars = append(missingEnvVars, envVar) + } + } + config.ClientSecret = replaceENVVar(config.ClientSecret) + // Process form captcha options if config.Form != nil && config.Form.Captcha != nil && config.Form.Captcha.Options != nil { for key, value := range config.Form.Captcha.Options { @@ -150,92 +293,12 @@ func processENVVariables(config *Config, rootPath string) { } } - // Process third party providers - if config.ThirdParty != nil && config.ThirdParty.Providers != nil { - for _, provider := range config.ThirdParty.Providers { - // Check ClientID - if strings.HasPrefix(provider.ClientID, "$ENV.") { - envVar := strings.TrimPrefix(provider.ClientID, "$ENV.") - if _, exists := os.LookupEnv(envVar); !exists { - missingEnvVars = append(missingEnvVars, envVar) - } - } - provider.ClientID = replaceENVVar(provider.ClientID) - - // Check ClientSecret - if strings.HasPrefix(provider.ClientSecret, "$ENV.") { - envVar := strings.TrimPrefix(provider.ClientSecret, "$ENV.") - if _, exists := os.LookupEnv(envVar); !exists { - missingEnvVars = append(missingEnvVars, envVar) - } - } - provider.ClientSecret = replaceENVVar(provider.ClientSecret) - - // Process client secret generator - if provider.ClientSecretGenerator != nil { - // Check PrivateKey - if strings.HasPrefix(provider.ClientSecretGenerator.PrivateKey, "$ENV.") { - envVar := strings.TrimPrefix(provider.ClientSecretGenerator.PrivateKey, "$ENV.") - if _, exists := os.LookupEnv(envVar); !exists { - missingEnvVars = append(missingEnvVars, envVar) - } - } - provider.ClientSecretGenerator.PrivateKey = replaceENVVar(provider.ClientSecretGenerator.PrivateKey) - - // Convert relative path to absolute path for private key - if provider.ClientSecretGenerator.PrivateKey != "" && !filepath.IsAbs(provider.ClientSecretGenerator.PrivateKey) { - provider.ClientSecretGenerator.PrivateKey = filepath.Join(rootPath, "openapi", "certs", provider.ClientSecretGenerator.PrivateKey) - } - - // Process and normalize expires_in format - if provider.ClientSecretGenerator.ExpiresIn != "" { - normalizedDuration, err := normalizeExpiresIn(provider.ClientSecretGenerator.ExpiresIn) - if err != nil { - log.Printf("Warning: Invalid expires_in format '%s' for provider '%s': %v", - provider.ClientSecretGenerator.ExpiresIn, provider.ID, err) - // Set default to 90 days - provider.ClientSecretGenerator.ExpiresIn = "2160h" // 90 * 24 hours - } else { - provider.ClientSecretGenerator.ExpiresIn = normalizedDuration - } - } - - // Process header values - if provider.ClientSecretGenerator.Header != nil { - for key, value := range provider.ClientSecretGenerator.Header { - if strValue, ok := value.(string); ok { - if strings.HasPrefix(strValue, "$ENV.") { - envVar := strings.TrimPrefix(strValue, "$ENV.") - if _, exists := os.LookupEnv(envVar); !exists { - missingEnvVars = append(missingEnvVars, envVar) - } - } - provider.ClientSecretGenerator.Header[key] = replaceENVVar(strValue) - } - } - } - - // Process payload values - if provider.ClientSecretGenerator.Payload != nil { - for key, value := range provider.ClientSecretGenerator.Payload { - if strValue, ok := value.(string); ok { - if strings.HasPrefix(strValue, "$ENV.") { - envVar := strings.TrimPrefix(strValue, "$ENV.") - if _, exists := os.LookupEnv(envVar); !exists { - missingEnvVars = append(missingEnvVars, envVar) - } - } - provider.ClientSecretGenerator.Payload[key] = replaceENVVar(strValue) - } - } - } - } - } - } + // Note: Third party providers are now handled separately in loadProviders() + // No need to process provider configurations here anymore // Log warning for missing environment variables if len(missingEnvVars) > 0 { - log.Printf("Warning: The following environment variables are not set and may cause configuration issues: %v", missingEnvVars) + log.Printf("Warning: The following environment variables are not set in signin configuration: %v", missingEnvVars) log.Printf("Please set these environment variables to avoid exposing placeholder values in configuration") } } @@ -376,11 +439,9 @@ func GetFullConfig(lang string) *Config { return config } - // Fallback to default language - if defaultLang != "" { - if config, exists := fullConfigs[defaultLang]; exists { - return config - } + // Fallback to default config (marked with default: true) + if defaultConfig != nil { + return defaultConfig } // Return any available config as last resort @@ -406,10 +467,15 @@ func GetPublicConfig(lang string) *Config { return config } - // Fallback to default language - if defaultLang != "" { - if config, exists := publicConfigs[defaultLang]; exists { - return config + // Fallback to default config's public version + if defaultConfig != nil { + // Find the public version of the default config + for lang, fullConfig := range fullConfigs { + if fullConfig == defaultConfig { + if publicConfig, exists := publicConfigs[lang]; exists { + return publicConfig + } + } } } @@ -428,14 +494,7 @@ func GetAvailableLanguages() []string { var languages []string for lang := range fullConfigs { - if lang != "" { - languages = append(languages, lang) - } - } - - // Add default language if it exists and is empty string - if defaultLang == "" && len(fullConfigs) > 0 { - languages = append(languages, "default") + languages = append(languages, lang) } return languages @@ -446,10 +505,21 @@ func GetDefaultLanguage() string { configMutex.RLock() defer configMutex.RUnlock() - if defaultLang == "" { - return "default" + // Find the language code for the default config + if defaultConfig != nil { + for lang, config := range fullConfigs { + if config == defaultConfig { + return lang + } + } } - return defaultLang + + // Return the first available language as fallback + for lang := range fullConfigs { + return lang + } + + return "" } // normalizeExpiresIn converts custom time units to Go standard duration format diff --git a/openapi/signin/types.go b/openapi/signin/types.go index 44b8a5aa..208560bd 100644 --- a/openapi/signin/types.go +++ b/openapi/signin/types.go @@ -6,13 +6,16 @@ import ( // Config represents the signin page configuration type Config struct { - Title string `json:"title,omitempty"` - Description string `json:"description,omitempty"` - SuccessURL string `json:"success_url,omitempty"` - FailureURL string `json:"failure_url,omitempty"` - Form *FormConfig `json:"form,omitempty"` - Token *TokenConfig `json:"token,omitempty"` - ThirdParty *ThirdParty `json:"third_party,omitempty"` + Title string `json:"title,omitempty"` + Description string `json:"description,omitempty"` + Default bool `json:"default,omitempty"` + SuccessURL string `json:"success_url,omitempty"` + FailureURL string `json:"failure_url,omitempty"` + ClientID string `json:"client_id,omitempty"` + ClientSecret string `json:"client_secret,omitempty"` + Form *FormConfig `json:"form,omitempty"` + Token *TokenConfig `json:"token,omitempty"` + ThirdParty *ThirdParty `json:"third_party,omitempty"` } // FormConfig represents the form configuration diff --git a/openapi/tests/signin_test.go b/openapi/tests/signin_test.go index c2cefc14..548da43b 100644 --- a/openapi/tests/signin_test.go +++ b/openapi/tests/signin_test.go @@ -64,18 +64,20 @@ func TestSigninGetConfigs(t *testing.T) { if publicConfig.ThirdParty != nil && i < len(publicConfig.ThirdParty.Providers) { publicProvider := publicConfig.ThirdParty.Providers[i] - // Check that sensitive OAuth fields are removed - assert.Empty(t, publicProvider.ClientID, "Client ID should be empty in public config") - assert.Empty(t, publicProvider.ClientSecret, "Client secret should be empty in public config") - assert.Nil(t, publicProvider.ClientSecretGenerator, "Client secret generator should be nil in public config") - assert.Empty(t, publicProvider.Scopes, "Scopes should be empty in public config") - assert.Nil(t, publicProvider.Endpoints, "Endpoints should be nil in public config") - assert.Empty(t, publicProvider.Mapping, "Mapping should be empty in public config") + // In the new structure, providers in ThirdParty only contain display info + // Sensitive data is now stored separately in the global providers map - // Check that display fields are preserved + // Check that only display fields are present in public config assert.NotEmpty(t, publicProvider.ID, "Provider ID should be preserved in public config") assert.NotEmpty(t, publicProvider.Title, "Provider title should be preserved in public config") - // Logo, Color, TextColor might be empty depending on config, so we don't assert NotEmpty + + // Sensitive fields should be empty in the ThirdParty providers (they're in global map now) + assert.Empty(t, publicProvider.ClientID, "Client ID should be empty in ThirdParty providers") + assert.Empty(t, publicProvider.ClientSecret, "Client secret should be empty in ThirdParty providers") + assert.Nil(t, publicProvider.ClientSecretGenerator, "Client secret generator should be nil in ThirdParty providers") + assert.Empty(t, publicProvider.Scopes, "Scopes should be empty in ThirdParty providers") + assert.Nil(t, publicProvider.Endpoints, "Endpoints should be nil in ThirdParty providers") + assert.Empty(t, publicProvider.Mapping, "Mapping should be empty in ThirdParty providers") } } } @@ -141,6 +143,13 @@ func TestSigninConfigStructure(t *testing.T) { // Verify config structure is valid assert.IsType(t, &signin.Config{}, config, "Should return correct config type") + // Test new configuration fields + assert.IsType(t, "", config.ClientID, "ClientID should be string") + assert.IsType(t, "", config.ClientSecret, "ClientSecret should be string") + assert.IsType(t, false, config.Default, "Default should be boolean") + t.Logf("Config has ClientID: %t, ClientSecret: %t, Default: %t", + config.ClientID != "", config.ClientSecret != "", config.Default) + // Test form configuration if config.Form != nil { t.Logf("Form configuration found") @@ -159,8 +168,16 @@ func TestSigninConfigStructure(t *testing.T) { assert.IsType(t, []*signin.Provider{}, config.ThirdParty.Providers, "Providers should be slice of Provider pointers") for i, provider := range config.ThirdParty.Providers { t.Logf("Provider %d: %s", i, provider.ID) - assert.IsType(t, []string{}, provider.Scopes, "Provider scopes should be string slice") - assert.IsType(t, map[string]string{}, provider.Mapping, "Provider mapping should be string map") + + // In the new structure, ThirdParty providers only contain display information + // Sensitive configuration data is stored separately in the global providers map + assert.NotEmpty(t, provider.ID, "Provider ID should not be empty") + assert.NotEmpty(t, provider.Title, "Provider title should not be empty") + + // These fields should be empty in ThirdParty providers (they're in global map now) + assert.Empty(t, provider.Scopes, "Provider scopes should be empty in ThirdParty providers") + assert.Nil(t, provider.Mapping, "Provider mapping should be nil in ThirdParty providers") + assert.Nil(t, provider.Endpoints, "Provider endpoints should be nil in ThirdParty providers") } } } @@ -169,6 +186,58 @@ func TestSigninConfigStructure(t *testing.T) { } } +func TestSigninGlobalProvidersMap(t *testing.T) { + // Initialize test environment + serverURL := testutils.Prepare(t) + defer testutils.Clean() + + _ = serverURL // Server URL not needed for this test + + // Load signin configurations + err := signin.Load(config.Conf) + assert.NoError(t, err, "signin.Load should succeed") + + // Test that providers can be retrieved from global map (no locale needed) + providerIDs := []string{"google", "microsoft", "apple", "github"} + + for _, providerID := range providerIDs { + t.Run("provider_"+providerID, func(t *testing.T) { + provider, err := signin.GetProvider(providerID) + + // Note: Provider might not be found if configuration files don't exist + // or if environment variables are not set, which is normal in test environment + if err != nil { + t.Logf("Provider '%s' not found (expected in test environment): %v", providerID, err) + return + } + + if provider != nil { + assert.Equal(t, providerID, provider.ID, "Provider ID should match") + t.Logf("Provider '%s' loaded successfully", providerID) + + // Test provider structure + assert.IsType(t, "", provider.ClientID, "ClientID should be string") + assert.IsType(t, "", provider.ClientSecret, "ClientSecret should be string") + assert.IsType(t, []string{}, provider.Scopes, "Scopes should be string slice") + + if provider.Endpoints != nil { + assert.IsType(t, "", provider.Endpoints.Authorization, "Authorization endpoint should be string") + assert.IsType(t, "", provider.Endpoints.Token, "Token endpoint should be string") + assert.IsType(t, "", provider.Endpoints.UserInfo, "UserInfo endpoint should be string") + } + } + }) + } + + // Test nonexistent provider + t.Run("nonexistent_provider", func(t *testing.T) { + provider, err := signin.GetProvider("nonexistent") + assert.Error(t, err, "Should return error for nonexistent provider") + assert.Nil(t, provider, "Should return nil provider for nonexistent provider") + assert.Contains(t, err.Error(), "not found", "Error message should indicate provider not found") + }) +} + func TestSigninAPI(t *testing.T) { // Initialize test environment serverURL := testutils.Prepare(t) @@ -253,9 +322,8 @@ func TestSigninOAuthAuthorizationURL(t *testing.T) { } // Test OAuth authorization URL endpoints - // Note: These should return 500 because OAuth client credentials (CLIENT_ID, etc.) - // are not set in the test environment, making the provider configuration incomplete. - // This is the expected secure behavior. + // Note: These should return 200 when OAuth client credentials are properly configured + // (which they are in this test environment). Only nonexistent providers should return 404. testCases := []struct { name string provider string @@ -263,14 +331,13 @@ func TestSigninOAuthAuthorizationURL(t *testing.T) { expectCode int expectErrorMsg string }{ - {"get google oauth url", "google", "", 500, "Provider configuration is incomplete"}, - {"get microsoft oauth url", "microsoft", "", 500, "Provider configuration is incomplete"}, - {"get apple oauth url", "apple", "", 500, "Provider configuration is incomplete"}, - {"get github oauth url", "github", "", 500, "Provider configuration is incomplete"}, - {"get oauth url with locale", "google", "?locale=en", 500, "Provider configuration is incomplete"}, - {"get oauth url with redirect_uri", "google", "?redirect_uri=https://example.com/callback", 500, "Provider configuration is incomplete"}, - {"get oauth url with state", "google", "?state=test-state-123", 500, "Provider configuration is incomplete"}, - {"get oauth url for nonexistent provider", "nonexistent", "", 404, "OAuth provider 'nonexistent' not found"}, + {"get google oauth url", "google", "", 200, ""}, + {"get microsoft oauth url", "microsoft", "", 200, ""}, + {"get apple oauth url", "apple", "", 200, ""}, + {"get github oauth url", "github", "", 200, ""}, + {"get oauth url with redirect_uri", "google", "?redirect_uri=https://example.com/callback", 200, ""}, + {"get oauth url with state", "google", "?state=test-state-123", 200, ""}, + {"get oauth url for nonexistent provider", "nonexistent", "", 404, "Failed to get provider"}, } for _, tc := range testCases { @@ -289,25 +356,36 @@ func TestSigninOAuthAuthorizationURL(t *testing.T) { t.Logf("Response for %s: status=%d, body=%s", tc.provider, resp.StatusCode, string(body)) - // Parse error response to verify the error message - var errorResponse map[string]interface{} - err = json.Unmarshal(body, &errorResponse) - assert.NoError(t, err, "Should parse JSON error response") + var response map[string]interface{} + err = json.Unmarshal(body, &response) + assert.NoError(t, err, "Should parse JSON response") - // Verify error message matches expected - if errorDescription, hasError := errorResponse["error_description"]; hasError { - errorDescStr, ok := errorDescription.(string) - assert.True(t, ok, "error_description should be string") - assert.Equal(t, tc.expectErrorMsg, errorDescStr, "Error message should match expected") + if tc.expectCode == 200 { + // Success case - should have authorization_url + if authURL, hasAuthURL := response["authorization_url"]; hasAuthURL { + authURLStr, ok := authURL.(string) + assert.True(t, ok, "authorization_url should be string") + assert.NotEmpty(t, authURLStr, "authorization_url should not be empty") + t.Logf("Authorization URL generated successfully for %s", tc.provider) + } else { + t.Errorf("Success response should contain authorization_url field") + } } else { - t.Errorf("Response should contain error_description field") - } + // Error case - should have error fields + if errorDescription, hasError := response["error_description"]; hasError { + errorDescStr, ok := errorDescription.(string) + assert.True(t, ok, "error_description should be string") + assert.Equal(t, tc.expectErrorMsg, errorDescStr, "Error message should match expected") + } else { + t.Errorf("Error response should contain error_description field") + } - // Verify error code is present - if errorCode, hasErrorCode := errorResponse["error"]; hasErrorCode { - assert.Equal(t, "invalid_request", errorCode, "Error code should be invalid_request") - } else { - t.Errorf("Response should contain error field") + // Verify error code is present + if errorCode, hasErrorCode := response["error"]; hasErrorCode { + assert.Equal(t, "invalid_request", errorCode, "Error code should be invalid_request") + } else { + t.Errorf("Error response should contain error field") + } } } })