fix(gmail): empty provider, login (#1703)

## What?

Fixes several issues with Gmail

## Why?

Fixes #1674

Signed-off-by: drew <me@andrinoff.com>

Author: andrinoff

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)
 	}

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()

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)
+		}
+	})
+}

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 <id>" 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
 }
 

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")
+	}
+}

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 <id>".
 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)
 	}

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)
 		}
 	}