diff --git a/backend/pop3/pop3.go b/backend/pop3/pop3.go index 6b861d02..11a83b01 100644 --- a/backend/pop3/pop3.go +++ b/backend/pop3/pop3.go @@ -78,7 +78,7 @@ func (p *Provider) connect() (*pop3client.Conn, error) { return nil, fmt.Errorf("pop3 connect: %w", err) } - if err := conn.Auth(p.account.Email, p.account.Password); err != nil { + if err := conn.Auth(p.account.Email, p.account.ResolvePassword()); err != nil { _ = conn.Quit() return nil, fmt.Errorf("pop3 auth: %w", err) } diff --git a/config/config.go b/config/config.go index 24288670..ec722ae5 100644 --- a/config/config.go +++ b/config/config.go @@ -340,6 +340,53 @@ func (a *Account) GetPOP3Port() int { return 995 // Default POP3 SSL port } +// ResolvePassword returns the account password, re-resolving it on demand when +// the value cached at config-load time is empty. +// +// Resolution at load time can fail for reasons that are temporary or +// process-specific: a pass_cmd whose gpg-agent is not reachable yet (common for +// the auto-started daemon, which has no controlling terminal), or a Secret +// Service keyring that was not up when Matcha launched. Without a retry, a +// single failed lookup leaves Password empty for the whole process lifetime and +// every login fails with a server-side error such as Gmail's +// "NO Empty username or password". +// +// Returns "" for OAuth2 accounts (they authenticate via XOAUTH2) and when no +// source yields a password. +func (a *Account) ResolvePassword() string { + if a == nil { + return "" + } + if a.Password != "" { + return a.Password + } + if a.IsOAuth2() { + return "" + } + + if a.PassCmd != "" { + pwd, err := resolvePassCmd(a.PassCmd) + if err != nil { + log.Printf("matcha: pass_cmd for %s failed: %v", a.Email, err) + return "" + } + return pwd + } + + // In secure mode the password lives in the encrypted config, never the + // keyring, so there is nothing else to try. + if GetSessionKey() != nil { + return "" + } + + pwd, err := keyring.Get(keyringServiceName, a.Email) + if err != nil { + log.Printf("matcha: keyring lookup for %s failed: %v", a.Email, err) + return "" + } + return pwd +} + // GetConfigDir returns the path to the configuration directory (exported). func GetConfigDir() (string, error) { return configDir() diff --git a/config/config_test.go b/config/config_test.go index aaeab88f..5ce909fc 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -678,3 +678,50 @@ func TestPassCmd(t *testing.T) { t.Errorf("Password not resolved from pass_cmd: got %q", acc.Password) } } + +// TestResolvePassword covers the lazy password retry from #1674: a pass_cmd or +// keyring lookup that failed at load time (no gpg-agent, keyring daemon not up) +// left Password empty and every subsequent login failed with the server's +// "Empty username or password". ResolvePassword retries at connect time. +func TestResolvePassword(t *testing.T) { + keyring.MockInit() + t.Setenv("HOME", t.TempDir()) + + t.Run("cached password wins", func(t *testing.T) { + acc := &Account{Email: "a@example.com", Password: "cached", PassCmd: "echo fromcmd"} + if got := acc.ResolvePassword(); got != "cached" { + t.Errorf("ResolvePassword() = %q, want %q", got, "cached") + } + }) + + t.Run("re-runs pass_cmd when empty", func(t *testing.T) { + acc := &Account{Email: "b@example.com", PassCmd: "echo fromcmd"} + if got := acc.ResolvePassword(); got != "fromcmd" { + t.Errorf("ResolvePassword() = %q, want %q", got, "fromcmd") + } + }) + + t.Run("failing pass_cmd yields empty", func(t *testing.T) { + acc := &Account{Email: "c@example.com", PassCmd: "exit 1"} + if got := acc.ResolvePassword(); got != "" { + t.Errorf("ResolvePassword() = %q, want empty", got) + } + }) + + t.Run("falls back to keyring", func(t *testing.T) { + if err := keyring.Set(keyringServiceName, "d@example.com", "fromkeyring"); err != nil { + t.Fatalf("keyring.Set() failed: %v", err) + } + acc := &Account{Email: "d@example.com"} + if got := acc.ResolvePassword(); got != "fromkeyring" { + t.Errorf("ResolvePassword() = %q, want %q", got, "fromkeyring") + } + }) + + t.Run("oauth2 accounts resolve to empty", func(t *testing.T) { + acc := &Account{Email: "e@example.com", AuthMethod: "oauth2", PassCmd: "echo fromcmd"} + if got := acc.ResolvePassword(); got != "" { + t.Errorf("ResolvePassword() = %q, want empty", got) + } + }) +} diff --git a/daemon/daemon.go b/daemon/daemon.go index ea4e5f59..bd795b9d 100644 --- a/daemon/daemon.go +++ b/daemon/daemon.go @@ -218,8 +218,19 @@ func (d *Daemon) ReloadConfig() error { } d.mu.Lock() d.config = cfg + // Providers hold a pointer into the old Accounts slice, so keeping them + // would pin stale credentials (e.g. a re-resolved pass_cmd password). + // Drop them all and rebuild against the freshly loaded accounts. + old := d.providers + d.providers = make(map[string]backend.Provider, len(cfg.Accounts)) d.mu.Unlock() + for id, p := range old { + if err := p.Close(); err != nil { + log.Printf("daemon: error closing provider %s: %v", id, err) + } + } + // Reinitialize providers for new/changed accounts. d.initProviders() @@ -275,14 +286,61 @@ func (d *Daemon) broadcastToSubscribers(accountID, folder, eventType string, dat }) } -// getProvider returns the provider for the given account ID. +// getProvider returns the provider for the given account ID, creating it on +// demand when it is missing. +// +// The daemon outlives any client, so its in-memory config can predate accounts +// that were added later; and a provider that failed to build at startup used to +// stay missing forever. Both showed up as "no provider for account " on +// every delete/archive. Reload from disk and retry before giving up. func (d *Daemon) getProvider(accountID string) (backend.Provider, error) { d.mu.RLock() - defer d.mu.RUnlock() p, ok := d.providers[accountID] - if !ok { - return nil, fmt.Errorf("no provider for account %s", accountID) + known := d.config.GetAccountByID(accountID) != nil + d.mu.RUnlock() + if ok { + return p, nil + } + + if !known { + if err := d.ReloadConfig(); err != nil { + return nil, fmt.Errorf("no provider for account %s (config reload failed: %w)", accountID, err) + } + // ReloadConfig rebuilds every provider, so the account is now covered + // if it exists on disk at all. + d.mu.RLock() + p, ok = d.providers[accountID] + d.mu.RUnlock() + if ok { + return p, nil + } + return nil, fmt.Errorf("no provider for account %s: account not found in config", accountID) + } + + return d.createProvider(accountID) +} + +// createProvider builds and stores a provider for a known account. +func (d *Daemon) createProvider(accountID string) (backend.Provider, error) { + d.mu.Lock() + defer d.mu.Unlock() + + // Another caller may have won the race while the lock was released. + if p, ok := d.providers[accountID]; ok { + return p, nil + } + + acct := d.config.GetAccountByID(accountID) + if acct == nil { + return nil, fmt.Errorf("no provider for account %s: account not found in config", accountID) + } + + p, err := backend.New(acct) + if err != nil { + return nil, fmt.Errorf("create provider for %s: %w", acct.Email, err) } + d.providers[accountID] = p + log.Printf("daemon: provider created on demand for %s (%s)", acct.Email, acct.Protocol) return p, nil } diff --git a/daemon/daemon_test.go b/daemon/daemon_test.go index 3d21ee78..d93418fe 100644 --- a/daemon/daemon_test.go +++ b/daemon/daemon_test.go @@ -9,6 +9,7 @@ import ( "testing" "time" + _ "github.com/floatpane/matcha/backend/imap" // register imap backend for provider tests "github.com/floatpane/matcha/config" "github.com/floatpane/matcha/daemonrpc" ) @@ -206,3 +207,40 @@ func TestDaemon_BroadcastEvent(t *testing.T) { t.Errorf("type = %q, want NewMail", msg.Event.Type) } } + +// TestDaemon_GetProviderCreatesOnDemand covers the "no provider for account" +// regression (#1674): an account present in the config but missing from the +// provider map must get a provider built on demand instead of failing every +// delete/archive for the rest of the daemon's lifetime. +func TestDaemon_GetProviderCreatesOnDemand(t *testing.T) { + d := New(&config.Config{ + Accounts: []config.Account{{ + ID: "acc1", + Email: "user@example.com", + ServiceProvider: "gmail", + Protocol: "imap", + }}, + }) + + // Simulates a provider that failed to build when Run() called initProviders. + if len(d.providers) != 0 { + t.Fatalf("expected empty provider map, got %d", len(d.providers)) + } + + p, err := d.getProvider("acc1") + if err != nil { + t.Fatalf("getProvider: %v", err) + } + if p == nil { + t.Fatal("expected a provider") + } + + // Second call must reuse the cached provider. + p2, err := d.getProvider("acc1") + if err != nil { + t.Fatalf("getProvider (cached): %v", err) + } + if p2 != p { + t.Error("expected the cached provider to be reused") + } +} diff --git a/daemonclient/service.go b/daemonclient/service.go index a98a7d65..518bd749 100644 --- a/daemonclient/service.go +++ b/daemonclient/service.go @@ -6,11 +6,14 @@ import ( "log" "os" "os/exec" + "sync" "time" "github.com/floatpane/matcha/backend" + _ "github.com/floatpane/matcha/backend/imap" // register imap backend for directService _ "github.com/floatpane/matcha/backend/jmap" // register jmap backend for directService _ "github.com/floatpane/matcha/backend/maildir" // register maildir backend for directService + _ "github.com/floatpane/matcha/backend/pop3" // register pop3 backend for directService "github.com/floatpane/matcha/config" "github.com/floatpane/matcha/daemonrpc" "github.com/floatpane/matcha/fetcher" @@ -264,6 +267,7 @@ type directService struct { cfg *config.Config providers map[string]backend.Provider events chan *daemonrpc.Event + mu sync.RWMutex } func newDirectService(cfg *config.Config) *directService { @@ -277,6 +281,9 @@ func newDirectService(cfg *config.Config) *directService { } func (s *directService) initProviders() { + s.mu.Lock() + defer s.mu.Unlock() + for i := range s.cfg.Accounts { acct := &s.cfg.Accounts[i] if _, ok := s.providers[acct.ID]; ok { @@ -291,11 +298,47 @@ func (s *directService) initProviders() { } } +// getProvider returns the provider for an account, creating it on demand. +// A provider missing from the map (account added after the service was built, +// or a constructor that failed once at startup) used to fail every later +// operation with "no provider for account ". func (s *directService) getProvider(accountID string) (backend.Provider, error) { + s.mu.RLock() p, ok := s.providers[accountID] - if !ok { - return nil, &daemonrpc.Error{Code: daemonrpc.ErrCodeInternal, Message: "no provider for account " + accountID} + acct := s.cfg.GetAccountByID(accountID) + s.mu.RUnlock() + if ok { + return p, nil + } + + if acct == nil { + // Config in memory may be stale — reload from disk before giving up. + if err := s.ReloadConfig(); err != nil { + return nil, &daemonrpc.Error{Code: daemonrpc.ErrCodeInternal, Message: "no provider for account " + accountID + ": " + err.Error()} + } + s.mu.RLock() + p, ok = s.providers[accountID] + s.mu.RUnlock() + if ok { + return p, nil + } + return nil, &daemonrpc.Error{Code: daemonrpc.ErrCodeInternal, Message: "no provider for account " + accountID + ": account not found in config"} + } + + s.mu.Lock() + defer s.mu.Unlock() + if p, ok := s.providers[accountID]; ok { + return p, nil } + acct = s.cfg.GetAccountByID(accountID) + if acct == nil { + return nil, &daemonrpc.Error{Code: daemonrpc.ErrCodeInternal, Message: "no provider for account " + accountID + ": account not found in config"} + } + p, err := backend.New(acct) + if err != nil { + return nil, &daemonrpc.Error{Code: daemonrpc.ErrCodeInternal, Message: "create provider for " + acct.Email + ": " + err.Error()} + } + s.providers[accountID] = p return p, nil } @@ -392,7 +435,19 @@ func (s *directService) ReloadConfig() error { if err != nil { return err } + + // Providers point into the old Accounts slice; drop them so the reloaded + // credentials (keyring / pass_cmd) actually take effect. + s.mu.Lock() s.cfg = cfg + old := s.providers + s.providers = make(map[string]backend.Provider, len(cfg.Accounts)) + s.mu.Unlock() + + for _, p := range old { + p.Close() //nolint:errcheck,gosec + } + s.initProviders() return nil } @@ -404,6 +459,8 @@ func (s *directService) Events() <-chan *daemonrpc.Event { func (s *directService) IsDaemon() bool { return false } func (s *directService) Close() error { + s.mu.Lock() + defer s.mu.Unlock() for _, p := range s.providers { p.Close() //nolint:errcheck,gosec } @@ -412,7 +469,9 @@ func (s *directService) Close() error { } func (s *directService) QueueEmail(accountID string, to, cc, bcc []string, subject, body, htmlBody string, images map[string][]byte, attachments map[string][]byte, inReplyTo string, references []string, signSMIME, encryptSMIME, signPGP, encryptPGP bool, _ int) (string, error) { + s.mu.RLock() acct := s.cfg.GetAccountByID(accountID) + s.mu.RUnlock() if acct == nil { return "", fmt.Errorf("no account for %s", accountID) } diff --git a/fetcher/fetcher.go b/fetcher/fetcher.go index 0b956663..8b010bfa 100644 --- a/fetcher/fetcher.go +++ b/fetcher/fetcher.go @@ -445,7 +445,12 @@ func connectWithOptions(account *config.Account, extraOpts *imapclient.Options) return nil, fmt.Errorf("XOAUTH2 authentication failed: %w", err) } } else { - if err := c.Login(account.Email, account.Password).Wait(); err != nil { + password := account.ResolvePassword() + if password == "" { + c.Close() //nolint:errcheck,gosec + return nil, fmt.Errorf("no password available for %s: keyring or pass_cmd returned nothing (see https://docs.matcha.email/Features/PassCmd)", account.Email) + } + if err := c.Login(account.Email, password).Wait(); err != nil { return nil, fmt.Errorf("authentication error: %w", err) } } diff --git a/sender/sender.go b/sender/sender.go index 0a1ac1d6..639359fd 100644 --- a/sender/sender.go +++ b/sender/sender.go @@ -221,8 +221,9 @@ func SendEmail(account *config.Account, to, cc, bcc []string, subject, plainBody return nil, fmt.Errorf("unsupported or missing service_provider: %s", account.ServiceProvider) } - plainAuth := smtp.PlainAuth("", account.Email, account.Password, smtpServer) - loginAuthFallback := &loginAuth{username: account.Email, password: account.Password} + password := account.ResolvePassword() + plainAuth := smtp.PlainAuth("", account.Email, password, smtpServer) + loginAuthFallback := &loginAuth{username: account.Email, password: password} fromHeader := account.FormatFromHeader() @@ -803,8 +804,9 @@ func SendCalendarReply(account *config.Account, to []string, subject, plainBody return nil, fmt.Errorf("unsupported or missing service_provider: %s", account.ServiceProvider) } - plainAuth := smtp.PlainAuth("", account.Email, account.Password, smtpServer) - loginAuthFallback := &loginAuth{username: account.Email, password: account.Password} + password := account.ResolvePassword() + plainAuth := smtp.PlainAuth("", account.Email, password, smtpServer) + loginAuthFallback := &loginAuth{username: account.Email, password: password} fromHeader := account.FormatFromHeader()