diff --git a/internal/httpd/httpd.go b/internal/httpd/httpd.go index fe6c088f..f556f1ac 100644 --- a/internal/httpd/httpd.go +++ b/internal/httpd/httpd.go @@ -194,6 +194,8 @@ var ( cleanupTicker *time.Ticker cleanupDone chan bool invalidatedJWTTokens tokenManager + httpServers []*httpdServer // Track HTTP servers for OIDC reload + httpServersMutex sync.RWMutex // Protect httpServers access webRootPath string webBasePath string webBaseAdminPath string @@ -1090,6 +1092,11 @@ func (c *Conf) Initialize(configDir string, isShared int) error { } logger.Info(logSender, "", "initializing HTTP server with config %+v", c.getRedacted()) configurationDir = configDir + + httpServersMutex.Lock() + httpServers = nil + httpServersMutex.Unlock() + invalidatedJWTTokens = newTokenManager(isShared) resetCodesMgr = newResetCodeManager(isShared) oidcMgr = newOIDCManager(isShared) @@ -1153,6 +1160,10 @@ func (c *Conf) Initialize(configDir string, isShared int) error { server := newHttpdServer(b, staticFilesPath, c.SigningPassphrase, c.Cors, openAPIPath) server.setShared(isShared) + httpServersMutex.Lock() + httpServers = append(httpServers, server) + httpServersMutex.Unlock() + exitChannel <- server.listenAndServe() }(binding) } @@ -1182,6 +1193,24 @@ func ReloadCertificateMgr() error { return nil } +// ReloadOIDC reloads OIDC client secrets for all running HTTP servers with OIDC configured +func ReloadOIDC() error { + httpServersMutex.RLock() + defer httpServersMutex.RUnlock() + + for _, server := range httpServers { + if server.binding.OIDC.ConfigURL != "" { + if err := server.binding.OIDC.ReloadSecret(); err != nil { + logger.Warn(logSender, "", "error reloading OIDC config for binding %s: %v", + server.binding.Address, err) + return err + } + logger.Debug(logSender, "", "OIDC configuration reloaded for binding %s", server.binding.Address) + } + } + return nil +} + func getConfigPath(name, configDir string) string { if !util.IsFileInputValid(name) { return "" diff --git a/internal/httpd/oidc.go b/internal/httpd/oidc.go index 8f612cde..4391d51b 100644 --- a/internal/httpd/oidc.go +++ b/internal/httpd/oidc.go @@ -184,6 +184,42 @@ func (o *OIDC) initialize() error { return nil } +// ReloadSecret reloads the OIDC client secret from the configured ClientSecretFile. +func (o *OIDC) ReloadSecret() error { + if o.ConfigURL == "" { + return nil // OIDC not configured + } + + if o.ClientSecretFile == "" { + return nil + } + + oldSecret := o.ClientSecret + + secret, err := util.ReadConfigFromFile(o.ClientSecretFile, configurationDir) + if err != nil { + return fmt.Errorf("failed to reload OIDC client secret from file %q: %w", o.ClientSecretFile, err) + } + + if strings.TrimSpace(secret) == "" { + return fmt.Errorf("OIDC client secret from file %q is empty", o.ClientSecretFile) + } + + o.ClientSecret = secret + + if o.oauth2Config != nil { + if config, ok := o.oauth2Config.(*oauth2.Config); ok { + config.ClientSecret = o.ClientSecret + logger.Debug(logSender, "", "OIDC client secret reloaded successfully from file %q", o.ClientSecretFile) + } else { + o.ClientSecret = oldSecret + return fmt.Errorf("OIDC oauth2Config has unexpected type, secret reload aborted") + } + } + + return nil +} + func (o *OIDC) getVerifier(ctx context.Context) OIDCTokenVerifier { if o.verifier != nil { return o.verifier diff --git a/internal/httpd/oidc_test.go b/internal/httpd/oidc_test.go index cb0a4fc3..52365ae0 100644 --- a/internal/httpd/oidc_test.go +++ b/internal/httpd/oidc_test.go @@ -1775,3 +1775,191 @@ func getPreLoginScriptContent(user dataprovider.User, nonJSONResponse bool) []by } return content } + +func TestOIDCReloadSecret(t *testing.T) { + // Save original httpServers safely + httpServersMutex.Lock() + originalServers := make([]*httpdServer, len(httpServers)) + copy(originalServers, httpServers) + httpServersMutex.Unlock() + + defer func() { + httpServersMutex.Lock() + httpServers = originalServers + httpServersMutex.Unlock() + }() + + originalSecret := "oldSecret123456789" + newSecret := "newSecret987654321" + + // Create temporary file for the secret + secretFile := filepath.Join(os.TempDir(), util.GenerateUniqueID()) + defer os.Remove(secretFile) + + err := os.WriteFile(secretFile, []byte(originalSecret), 0600) + assert.NoError(t, err) + + // Test OIDC config + config := OIDC{ + ClientID: "test-client", + ClientSecretFile: secretFile, + ConfigURL: fmt.Sprintf("http://%v/auth/realms/sftpgo", oidcMockAddr), + RedirectBaseURL: "http://127.0.0.1:8081/", + UsernameField: "preferred_username", + Scopes: []string{oidc.ScopeOpenID}, + } + + // Initial initialization + err = config.initialize() + assert.NoError(t, err) + assert.Equal(t, originalSecret, config.ClientSecret) + + // Update the secret file + err = os.WriteFile(secretFile, []byte(newSecret), 0600) + assert.NoError(t, err) + + // Test reloadSecret method + err = config.ReloadSecret() + assert.NoError(t, err) + assert.Equal(t, newSecret, config.ClientSecret) + + // Verify oauth2Config was updated + if config.oauth2Config != nil { + oauth2Cfg := config.oauth2Config.(*oauth2.Config) + assert.Equal(t, newSecret, oauth2Cfg.ClientSecret) + } + + // Test with missing file + os.Remove(secretFile) + err = config.ReloadSecret() + assert.Error(t, err) + assert.Contains(t, err.Error(), "failed to reload OIDC client secret from file") + + // Test with empty secret file + emptySecretFile := filepath.Join(os.TempDir(), util.GenerateUniqueID()) + defer os.Remove(emptySecretFile) + err = os.WriteFile(emptySecretFile, []byte(""), 0600) + assert.NoError(t, err) + + config.ClientSecretFile = emptySecretFile + err = config.ReloadSecret() + assert.Error(t, err) + assert.Contains(t, err.Error(), "is empty") + + // Test with whitespace-only secret + err = os.WriteFile(emptySecretFile, []byte(" \n\t "), 0600) + assert.NoError(t, err) + + err = config.ReloadSecret() + assert.Error(t, err) + assert.Contains(t, err.Error(), "is empty") + + // Test with no ConfigURL (OIDC not configured) + config.ConfigURL = "" + err = config.ReloadSecret() + assert.NoError(t, err) // Should return nil without error + + // Test with no ClientSecretFile configured + config.ConfigURL = fmt.Sprintf("http://%v/auth/realms/sftpgo", oidcMockAddr) + config.ClientSecretFile = "" + err = config.ReloadSecret() + assert.NoError(t, err) // Should return nil without error + + // Test ReloadOIDC function with valid config + validSecretFile := filepath.Join(os.TempDir(), util.GenerateUniqueID()) + defer os.Remove(validSecretFile) + err = os.WriteFile(validSecretFile, []byte(originalSecret), 0600) + assert.NoError(t, err) + + binding := Binding{ + Address: "127.0.0.1:8080", + OIDC: OIDC{ + ClientID: "test-client", + ClientSecretFile: validSecretFile, + ConfigURL: fmt.Sprintf("http://%v/auth/realms/sftpgo", oidcMockAddr), + RedirectBaseURL: "http://127.0.0.1:8081/", + UsernameField: "preferred_username", + Scopes: []string{oidc.ScopeOpenID}, + }, + } + + err = binding.OIDC.initialize() + assert.NoError(t, err) + assert.Equal(t, originalSecret, binding.OIDC.ClientSecret) + + // Create and add mock server safely + server := &httpdServer{binding: binding} + httpServersMutex.Lock() + httpServers = []*httpdServer{server} + httpServersMutex.Unlock() + + // Update the secret file + err = os.WriteFile(validSecretFile, []byte(newSecret), 0600) + assert.NoError(t, err) + + // Test ReloadOIDC function + err = ReloadOIDC() + assert.NoError(t, err) + + // Verify the secret was reloaded via ReloadOIDC safely + httpServersMutex.RLock() + assert.Equal(t, newSecret, httpServers[0].binding.OIDC.ClientSecret) + httpServersMutex.RUnlock() +} + +func TestOIDCReloadEdgeCases(t *testing.T) { + // Save original httpServers + originalServers := httpServers + defer func() { + httpServers = originalServers + }() + + // Test 1: Invalid config file + binding := Binding{ + Address: "127.0.0.1:8080", + OIDC: OIDC{ + ClientID: "test-client", + ClientSecretFile: "/non/existent/file", + ConfigURL: fmt.Sprintf("http://%v/auth/realms/sftpgo", oidcMockAddr), + }, + } + server := &httpdServer{binding: binding} + httpServers = []*httpdServer{server} + + err := ReloadOIDC() + assert.Error(t, err) + + // Test 2: No servers + httpServers = nil + err = ReloadOIDC() + assert.NoError(t, err) + + // Test 3: Nil oauth2Config + secretFile := filepath.Join(os.TempDir(), util.GenerateUniqueID()) + defer os.Remove(secretFile) + + secret := "testSecret123456789" + err = os.WriteFile(secretFile, []byte(secret), 0600) + assert.NoError(t, err) + + config := OIDC{ + ClientID: "test-client", + ClientSecretFile: secretFile, + ConfigURL: fmt.Sprintf("http://%v/auth/realms/sftpgo", oidcMockAddr), + oauth2Config: nil, + } + + err = config.ReloadSecret() + assert.NoError(t, err) + assert.Equal(t, secret, config.ClientSecret) + + // Test 4: Wrong oauth2Config type + mockConfig := &mockOAuth2Config{} + config.oauth2Config = mockConfig + config.ClientSecret = "oldSecret" + + err = config.ReloadSecret() + assert.Error(t, err) + assert.Contains(t, err.Error(), "oauth2Config has unexpected type") + assert.Equal(t, "oldSecret", config.ClientSecret) // rollback verification +} diff --git a/internal/service/service_windows.go b/internal/service/service_windows.go index 16be97ae..ed974102 100644 --- a/internal/service/service_windows.go +++ b/internal/service/service_windows.go @@ -149,6 +149,10 @@ loop: if err != nil { logger.Warn(logSender, "", "error reloading cert manager: %v", err) } + err = httpd.ReloadOIDC() + if err != nil { + logger.Warn(logSender, "", "error reloading OIDC: %v", err) + } err = ftpd.ReloadCertificateMgr() if err != nil { logger.Warn(logSender, "", "error reloading FTPD cert manager: %v", err) diff --git a/internal/service/signals_unix.go b/internal/service/signals_unix.go index cecbea9f..d24a7d17 100644 --- a/internal/service/signals_unix.go +++ b/internal/service/signals_unix.go @@ -59,6 +59,10 @@ func handleSIGHUP() { if err != nil { logger.Warn(logSender, "", "error reloading cert manager: %v", err) } + err = httpd.ReloadOIDC() + if err != nil { + logger.Warn(logSender, "", "error reloading OIDC: %v", err) + } err = ftpd.ReloadCertificateMgr() if err != nil { logger.Warn(logSender, "", "error reloading FTPD cert manager: %v", err)