Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion backend/pop3/pop3.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
47 changes: 47 additions & 0 deletions config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
47 changes: 47 additions & 0 deletions config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
})
}
66 changes: 62 additions & 4 deletions daemon/daemon.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()

Expand Down Expand Up @@ -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
}

Expand Down
38 changes: 38 additions & 0 deletions daemon/daemon_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand Down Expand Up @@ -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")
}
}
63 changes: 61 additions & 2 deletions daemonclient/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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 {
Expand All @@ -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 {
Expand All @@ -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
}

Expand Down Expand Up @@ -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
}
Expand All @@ -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
}
Expand All @@ -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)
}
Expand Down
7 changes: 6 additions & 1 deletion fetcher/fetcher.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
Expand Down
Loading
Loading