From 9670e29d9fd71b46bdd93d70021c7716a569f678 Mon Sep 17 00:00:00 2001 From: James Houlahan Date: Tue, 27 Sep 2022 13:22:07 +0200 Subject: [PATCH] GODT-1815: Start with missing gluon files --- internal/bridge/bridge.go | 13 +++- internal/bridge/bridge_test.go | 28 ++++++- internal/bridge/imap.go | 76 ++++++++++--------- internal/bridge/mocks.go | 17 ++++- internal/bridge/settings.go | 64 ++++++++++++++++ internal/bridge/smtp.go | 36 +-------- internal/bridge/users.go | 6 +- internal/user/sync.go | 2 +- internal/user/user.go | 6 +- internal/vault/user.go | 18 ++--- internal/vault/user_test.go | 12 +-- internal/vault/{store.go => vault.go} | 16 ++++ .../vault/{store_test.go => vault_test.go} | 0 tests/bdd_test.go | 2 + tests/bridge_test.go | 9 +++ tests/ctx_test.go | 6 +- tests/features/user/sync.feature | 23 ++++++ tests/imap_test.go | 8 ++ 18 files changed, 241 insertions(+), 101 deletions(-) rename internal/vault/{store.go => vault.go} (94%) rename internal/vault/{store_test.go => vault_test.go} (100%) diff --git a/internal/bridge/bridge.go b/internal/bridge/bridge.go index 8a0f9214..ca2f297f 100644 --- a/internal/bridge/bridge.go +++ b/internal/bridge/bridge.go @@ -50,9 +50,8 @@ type Bridge struct { imapListener net.Listener // smtpServer is the bridge's SMTP server. - smtpServer *smtp.Server - smtpBackend *smtpBackend - smtpListener net.Listener + smtpServer *smtp.Server + smtpBackend *smtpBackend // updater is the bridge's updater. updater Updater @@ -107,7 +106,13 @@ func New( return nil, fmt.Errorf("failed to load TLS config: %w", err) } - imapServer, err := newIMAPServer(vault.GetGluonDir(), curVersion, tlsConfig) + // TODO: Handle case that the gluon directory is missing! + gluonDir, err := getGluonDir(vault) + if err != nil { + return nil, fmt.Errorf("failed to get Gluon directory: %w", err) + } + + imapServer, err := newIMAPServer(gluonDir, curVersion, tlsConfig) if err != nil { return nil, fmt.Errorf("failed to create IMAP server: %w", err) } diff --git a/internal/bridge/bridge_test.go b/internal/bridge/bridge_test.go index 9c83088c..9f9c7410 100644 --- a/internal/bridge/bridge_test.go +++ b/internal/bridge/bridge_test.go @@ -2,6 +2,7 @@ package bridge_test import ( "context" + "os" "testing" "github.com/Masterminds/semver/v3" @@ -282,6 +283,31 @@ func TestBridge_BadVaultKey(t *testing.T) { }) } +func TestBridge_MissingGluonDir(t *testing.T) { + withEnv(t, func(ctx context.Context, s *server.Server, dialer *bridge.TestDialer, locator bridge.Locator, vaultKey []byte) { + var gluonDir string + + withBridge(t, ctx, s.GetHostURL(), dialer, locator, vaultKey, func(bridge *bridge.Bridge, mocks *bridge.Mocks) { + _, err := bridge.LoginUser(context.Background(), username, password, nil, nil) + require.NoError(t, err) + + // Move the gluon dir. + bridge.SetGluonDir(ctx, t.TempDir()) + + // Get the gluon dir. + gluonDir = bridge.GetGluonDir() + }) + + // The user removes the gluon dir while bridge is not running. + require.NoError(t, os.RemoveAll(gluonDir)) + + // Bridge starts but can't find the gluon dir; there should be no error. + withBridge(t, ctx, s.GetHostURL(), dialer, locator, vaultKey, func(bridge *bridge.Bridge, mocks *bridge.Mocks) { + // ... + }) + }) +} + // withEnv creates the full test environment and runs the tests. func withEnv(t *testing.T, tests func(ctx context.Context, server *server.Server, dialer *bridge.TestDialer, locator bridge.Locator, vaultKey []byte)) { // Create test API. @@ -305,7 +331,7 @@ func withEnv(t *testing.T, tests func(ctx context.Context, server *server.Server ctx, server, bridge.NewTestDialer(), - locations.New(bridge.NewTestLocationsProvider(t), "config-name"), + locations.New(bridge.NewTestLocationsProvider(t.TempDir()), "config-name"), vaultKey, ) } diff --git a/internal/bridge/imap.go b/internal/bridge/imap.go index b247260a..873232d2 100644 --- a/internal/bridge/imap.go +++ b/internal/bridge/imap.go @@ -3,12 +3,16 @@ package bridge import ( "context" "crypto/tls" + "errors" "fmt" + "io/fs" + "os" "github.com/Masterminds/semver/v3" "github.com/ProtonMail/gluon" imapEvents "github.com/ProtonMail/gluon/events" "github.com/ProtonMail/proton-bridge/v2/internal/constants" + "github.com/ProtonMail/proton-bridge/v2/internal/vault" "github.com/sirupsen/logrus" ) @@ -17,38 +21,6 @@ const ( defaultClientVersion = "0.0.1" ) -func (bridge *Bridge) GetIMAPPort() int { - return bridge.vault.GetIMAPPort() -} - -func (bridge *Bridge) SetIMAPPort(newPort int) error { - if newPort == bridge.vault.GetIMAPPort() { - return nil - } - - if err := bridge.vault.SetIMAPPort(newPort); err != nil { - return err - } - - return bridge.restartIMAP(context.Background()) -} - -func (bridge *Bridge) GetIMAPSSL() bool { - return bridge.vault.GetIMAPSSL() -} - -func (bridge *Bridge) SetIMAPSSL(newSSL bool) error { - if newSSL == bridge.vault.GetIMAPSSL() { - return nil - } - - if err := bridge.vault.SetIMAPSSL(newSSL); err != nil { - return err - } - - return bridge.restartIMAP(context.Background()) -} - func (bridge *Bridge) serveIMAP() error { imapListener, err := newListener(bridge.vault.GetIMAPPort(), bridge.vault.GetIMAPSSL(), bridge.tlsConfig) if err != nil { @@ -83,15 +55,34 @@ func (bridge *Bridge) closeIMAP(ctx context.Context) error { func (bridge *Bridge) handleIMAPEvent(event imapEvents.Event) { switch event := event.(type) { case imapEvents.SessionAdded: - if !bridge.identifier.HasClient() { - bridge.identifier.SetClient(defaultClientName, defaultClientVersion) + if bridge.identifier.HasClient() { + return } + bridge.identifier.SetClient(defaultClientName, defaultClientVersion) + case imapEvents.IMAPID: bridge.identifier.SetClient(event.IMAPID.Name, event.IMAPID.Version) } } +func getGluonDir(encVault *vault.Vault) (string, error) { + empty, err := isEmpty(encVault.GetGluonDir()) + if err != nil { + return "", fmt.Errorf("failed to check if gluon dir is empty: %w", err) + } + + if empty { + if err := encVault.ForUser(func(user *vault.User) error { + return user.SetSync(false) + }); err != nil { + return "", fmt.Errorf("failed to reset user sync status: %w", err) + } + } + + return encVault.GetGluonDir(), nil +} + func newIMAPServer(gluonDir string, version *semver.Version, tlsConfig *tls.Config) (*gluon.Server, error) { imapServer, err := gluon.New( gluon.WithTLS(tlsConfig), @@ -115,3 +106,20 @@ func newIMAPServer(gluonDir string, version *semver.Version, tlsConfig *tls.Conf return imapServer, nil } + +func isEmpty(dir string) (bool, error) { + if _, err := os.Stat(dir); err != nil { + if !errors.Is(err, fs.ErrNotExist) { + return false, fmt.Errorf("failed to stat %s: %w", dir, err) + } + + return true, nil + } + + entries, err := os.ReadDir(dir) + if err != nil { + return false, fmt.Errorf("failed to read dir %s: %w", dir, err) + } + + return len(entries) == 0, nil +} diff --git a/internal/bridge/mocks.go b/internal/bridge/mocks.go index e0d08ed8..17673c90 100644 --- a/internal/bridge/mocks.go +++ b/internal/bridge/mocks.go @@ -5,6 +5,7 @@ import ( "crypto/tls" "errors" "net" + "os" "testing" "github.com/Masterminds/semver/v3" @@ -75,10 +76,20 @@ type TestLocationsProvider struct { config, cache string } -func NewTestLocationsProvider(tb testing.TB) *TestLocationsProvider { +func NewTestLocationsProvider(dir string) *TestLocationsProvider { + config, err := os.MkdirTemp(dir, "config") + if err != nil { + panic(err) + } + + cache, err := os.MkdirTemp(dir, "cache") + if err != nil { + panic(err) + } + return &TestLocationsProvider{ - config: tb.TempDir(), - cache: tb.TempDir(), + config: config, + cache: cache, } } diff --git a/internal/bridge/settings.go b/internal/bridge/settings.go index 50d607df..74c1a997 100644 --- a/internal/bridge/settings.go +++ b/internal/bridge/settings.go @@ -26,6 +26,70 @@ func (bridge *Bridge) SetKeychainApp(helper string) error { return vault.SetHelper(vaultDir, helper) } +func (bridge *Bridge) GetIMAPPort() int { + return bridge.vault.GetIMAPPort() +} + +func (bridge *Bridge) SetIMAPPort(newPort int) error { + if newPort == bridge.vault.GetIMAPPort() { + return nil + } + + if err := bridge.vault.SetIMAPPort(newPort); err != nil { + return err + } + + return bridge.restartIMAP(context.Background()) +} + +func (bridge *Bridge) GetIMAPSSL() bool { + return bridge.vault.GetIMAPSSL() +} + +func (bridge *Bridge) SetIMAPSSL(newSSL bool) error { + if newSSL == bridge.vault.GetIMAPSSL() { + return nil + } + + if err := bridge.vault.SetIMAPSSL(newSSL); err != nil { + return err + } + + return bridge.restartIMAP(context.Background()) +} + +func (bridge *Bridge) GetSMTPPort() int { + return bridge.vault.GetSMTPPort() +} + +func (bridge *Bridge) SetSMTPPort(newPort int) error { + if newPort == bridge.vault.GetSMTPPort() { + return nil + } + + if err := bridge.vault.SetSMTPPort(newPort); err != nil { + return err + } + + return bridge.restartSMTP() +} + +func (bridge *Bridge) GetSMTPSSL() bool { + return bridge.vault.GetSMTPSSL() +} + +func (bridge *Bridge) SetSMTPSSL(newSSL bool) error { + if newSSL == bridge.vault.GetSMTPSSL() { + return nil + } + + if err := bridge.vault.SetSMTPSSL(newSSL); err != nil { + return err + } + + return bridge.restartSMTP() +} + func (bridge *Bridge) GetGluonDir() string { return bridge.vault.GetGluonDir() } diff --git a/internal/bridge/smtp.go b/internal/bridge/smtp.go index 96feb974..07978646 100644 --- a/internal/bridge/smtp.go +++ b/internal/bridge/smtp.go @@ -10,48 +10,14 @@ import ( "github.com/sirupsen/logrus" ) -func (bridge *Bridge) GetSMTPPort() int { - return bridge.vault.GetSMTPPort() -} - -func (bridge *Bridge) SetSMTPPort(newPort int) error { - if newPort == bridge.vault.GetSMTPPort() { - return nil - } - - if err := bridge.vault.SetSMTPPort(newPort); err != nil { - return err - } - - return bridge.restartSMTP() -} - -func (bridge *Bridge) GetSMTPSSL() bool { - return bridge.vault.GetSMTPSSL() -} - -func (bridge *Bridge) SetSMTPSSL(newSSL bool) error { - if newSSL == bridge.vault.GetSMTPSSL() { - return nil - } - - if err := bridge.vault.SetSMTPSSL(newSSL); err != nil { - return err - } - - return bridge.restartSMTP() -} - func (bridge *Bridge) serveSMTP() error { smtpListener, err := newListener(bridge.vault.GetSMTPPort(), bridge.vault.GetSMTPSSL(), bridge.tlsConfig) if err != nil { return fmt.Errorf("failed to create SMTP listener: %w", err) } - bridge.smtpListener = smtpListener - go func() { - if err := bridge.smtpServer.Serve(bridge.smtpListener); err != nil { + if err := bridge.smtpServer.Serve(smtpListener); err != nil { logrus.WithError(err).Error("SMTP server stopped") } }() diff --git a/internal/bridge/users.go b/internal/bridge/users.go index bf211972..4714af3f 100644 --- a/internal/bridge/users.go +++ b/internal/bridge/users.go @@ -308,7 +308,7 @@ func (bridge *Bridge) addNewUser( return nil, err } - if err := vaultUser.UpdateGluonData(gluonID, gluonKey); err != nil { + if err := vaultUser.SetGluonAuth(gluonID, gluonKey); err != nil { return nil, err } @@ -336,11 +336,11 @@ func (bridge *Bridge) addExistingUser( return nil, err } - if err := vaultUser.UpdateAuth(authUID, authRef); err != nil { + if err := vaultUser.SetAuth(authUID, authRef); err != nil { return nil, err } - if err := vaultUser.UpdateKeyPass(saltedKeyPass); err != nil { + if err := vaultUser.SetKeyPass(saltedKeyPass); err != nil { return nil, err } diff --git a/internal/user/sync.go b/internal/user/sync.go index c618622a..285b9137 100644 --- a/internal/user/sync.go +++ b/internal/user/sync.go @@ -34,7 +34,7 @@ func (user *User) sync(ctx context.Context) error { UserID: user.ID(), } - if err := user.vault.UpdateSync(true); err != nil { + if err := user.vault.SetSync(true); err != nil { return fmt.Errorf("failed to update sync status: %w", err) } diff --git a/internal/user/user.go b/internal/user/user.go index 6d6a0807..75ea0136 100644 --- a/internal/user/user.go +++ b/internal/user/user.go @@ -56,7 +56,7 @@ func New( return nil, err } - if err := vault.UpdateEventID(eventID); err != nil { + if err := vault.SetEventID(eventID); err != nil { return nil, err } } @@ -85,7 +85,7 @@ func New( // When we receive an auth object, we update it in the store. // This will be used to authorize the user on the next run. client.AddAuthHandler(func(auth liteapi.Auth) { - if err := user.vault.UpdateAuth(auth.UID, auth.RefreshToken); err != nil { + if err := user.vault.SetAuth(auth.UID, auth.RefreshToken); err != nil { logrus.WithError(err).Error("Failed to update auth") } }) @@ -104,7 +104,7 @@ func New( if err := user.handleAPIEvent(event); err != nil { logrus.WithError(err).Error("Failed to handle event") } else { - if err := user.vault.UpdateEventID(event.EventID); err != nil { + if err := user.vault.SetEventID(event.EventID); err != nil { logrus.WithError(err).Error("Failed to update event ID") } } diff --git a/internal/vault/user.go b/internal/vault/user.go index 116644f5..5c599f51 100644 --- a/internal/vault/user.go +++ b/internal/vault/user.go @@ -45,37 +45,37 @@ func (user *User) HasSync() bool { return user.vault.getUser(user.userID).HasSync } -func (user *User) UpdateKeyPass(keyPass []byte) error { +func (user *User) SetKeyPass(keyPass []byte) error { return user.vault.modUser(user.userID, func(data *UserData) { data.KeyPass = keyPass }) } -// UpdateAuth updates the auth secrets for the given user. -func (user *User) UpdateAuth(authUID, authRef string) error { +// SetAuth updates the auth secrets for the given user. +func (user *User) SetAuth(authUID, authRef string) error { return user.vault.modUser(user.userID, func(data *UserData) { data.AuthUID = authUID data.AuthRef = authRef }) } -// UpdateGluonData updates the gluon ID and key for the given user. -func (user *User) UpdateGluonData(gluonID string, gluonKey []byte) error { +// SetGluonAuth updates the gluon ID and key for the given user. +func (user *User) SetGluonAuth(gluonID string, gluonKey []byte) error { return user.vault.modUser(user.userID, func(data *UserData) { data.GluonID = gluonID data.GluonKey = gluonKey }) } -// UpdateEventID updates the event ID for the given user. -func (user *User) UpdateEventID(eventID string) error { +// SetEventID updates the event ID for the given user. +func (user *User) SetEventID(eventID string) error { return user.vault.modUser(user.userID, func(data *UserData) { data.EventID = eventID }) } -// UpdateSync updates the sync state for the given user. -func (user *User) UpdateSync(hasSync bool) error { +// SetSync updates the sync state for the given user. +func (user *User) SetSync(hasSync bool) error { return user.vault.modUser(user.userID, func(data *UserData) { data.HasSync = hasSync }) diff --git a/internal/vault/user_test.go b/internal/vault/user_test.go index 28863acf..682d2c05 100644 --- a/internal/vault/user_test.go +++ b/internal/vault/user_test.go @@ -24,16 +24,16 @@ func TestUser(t *testing.T) { require.NoError(t, err) // Set event IDs for user 1 and 2. - require.NoError(t, user1.UpdateEventID("eventID1")) - require.NoError(t, user2.UpdateEventID("eventID2")) + require.NoError(t, user1.SetEventID("eventID1")) + require.NoError(t, user2.SetEventID("eventID2")) // Set sync state for user 1 and 2. - require.NoError(t, user1.UpdateSync(true)) - require.NoError(t, user2.UpdateSync(false)) + require.NoError(t, user1.SetSync(true)) + require.NoError(t, user2.SetSync(false)) // Set gluon data for user 1 and 2. - require.NoError(t, user1.UpdateGluonData("gluonID1", []byte("gluonKey1"))) - require.NoError(t, user2.UpdateGluonData("gluonID2", []byte("gluonKey2"))) + require.NoError(t, user1.SetGluonAuth("gluonID1", []byte("gluonKey1"))) + require.NoError(t, user2.SetGluonAuth("gluonID2", []byte("gluonKey2"))) // List available users. require.ElementsMatch(t, []string{"userID1", "userID2"}, s.GetUserIDs()) diff --git a/internal/vault/store.go b/internal/vault/vault.go similarity index 94% rename from internal/vault/store.go rename to internal/vault/vault.go index e62897ee..b10dd6f0 100644 --- a/internal/vault/store.go +++ b/internal/vault/vault.go @@ -74,6 +74,22 @@ func (vault *Vault) GetUser(userID string) (*User, error) { }, nil } +// ForUser executes a callback for each user in the vault. +func (vault *Vault) ForUser(fn func(*User) error) error { + for _, userID := range vault.GetUserIDs() { + user, err := vault.GetUser(userID) + if err != nil { + return err + } + + if err := fn(user); err != nil { + return err + } + } + + return nil +} + // AddUser creates a new user in the vault with the given ID and username. // A bridge password is generated using the package's token generator. func (vault *Vault) AddUser(userID, username, authUID, authRef string, keyPass []byte) (*User, error) { diff --git a/internal/vault/store_test.go b/internal/vault/vault_test.go similarity index 100% rename from internal/vault/store_test.go rename to internal/vault/vault_test.go diff --git a/tests/bdd_test.go b/tests/bdd_test.go index 673c11ec..d06770b5 100644 --- a/tests/bdd_test.go +++ b/tests/bdd_test.go @@ -86,6 +86,7 @@ func TestFeatures(testingT *testing.T) { ctx.Step(`^the user changes the IMAP port to (\d+)$`, s.theUserChangesTheIMAPPortTo) ctx.Step(`^the user changes the SMTP port to (\d+)$`, s.theUserChangesTheSMTPPortTo) ctx.Step(`^the user changes the gluon path$`, s.theUserChangesTheGluonPath) + ctx.Step(`^the user deletes the gluon files$`, s.theUserDeletesTheGluonFiles) ctx.Step(`^the user reports a bug$`, s.theUserReportsABug) ctx.Step(`^bridge sends a connection up event$`, s.bridgeSendsAConnectionUpEvent) ctx.Step(`^bridge sends a connection down event$`, s.bridgeSendsAConnectionDownEvent) @@ -127,6 +128,7 @@ func TestFeatures(testingT *testing.T) { ctx.Step(`^IMAP client "([^"]*)" deletes "([^"]*)"$`, s.imapClientDeletesMailbox) ctx.Step(`^IMAP client "([^"]*)" renames "([^"]*)" to "([^"]*)"$`, s.imapClientRenamesMailboxTo) ctx.Step(`^IMAP client "([^"]*)" sees the following mailbox info:$`, s.imapClientSeesTheFollowingMailboxInfo) + ctx.Step(`^IMAP client "([^"]*)" eventually sees the following mailbox info:$`, s.imapClientEventuallySeesTheFollowingMailboxInfo) ctx.Step(`^IMAP client "([^"]*)" sees the following mailbox info for "([^"]*)":$`, s.imapClientSeesTheFollowingMailboxInfoForMailbox) ctx.Step(`^IMAP client "([^"]*)" sees the following mailboxes:$`, s.imapClientSeesTheFollowingMailboxes) ctx.Step(`^IMAP client "([^"]*)" sees "([^"]*)"$`, s.imapClientSeesMailbox) diff --git a/tests/bridge_test.go b/tests/bridge_test.go index 5065b15b..063b65fa 100644 --- a/tests/bridge_test.go +++ b/tests/bridge_test.go @@ -55,6 +55,15 @@ func (s *scenario) theUserChangesTheGluonPath() error { return s.t.bridge.SetGluonDir(context.Background(), gluonDir) } +func (s *scenario) theUserDeletesTheGluonFiles() error { + path, err := s.t.locator.ProvideGluonPath() + if err != nil { + return err + } + + return os.RemoveAll(path) +} + func (s *scenario) theUserHasDisabledAutomaticUpdates() error { var started bool diff --git a/tests/ctx_test.go b/tests/ctx_test.go index fec89d9c..24d30747 100644 --- a/tests/ctx_test.go +++ b/tests/ctx_test.go @@ -70,13 +70,15 @@ type smtpClient struct { } func newTestCtx(tb testing.TB) *testCtx { + dir := tb.TempDir() + dialer := bridge.NewTestDialer() ctx := &testCtx{ - dir: tb.TempDir(), + dir: dir, api: newFakeAPI(), dialer: dialer, - locator: locations.New(bridge.NewTestLocationsProvider(tb), "config-name"), + locator: locations.New(bridge.NewTestLocationsProvider(dir), "config-name"), storeKey: []byte("super-secret-store-key"), mocks: bridge.NewMocks(tb, dialer, defaultVersion, defaultVersion), version: defaultVersion, diff --git a/tests/features/user/sync.feature b/tests/features/user/sync.feature index 967e033d..f9567022 100644 --- a/tests/features/user/sync.feature +++ b/tests/features/user/sync.feature @@ -22,6 +22,29 @@ Feature: Bridge can fully sync an account When bridge restarts And user "user@pm.me" connects and authenticates IMAP client "1" Then IMAP client "1" sees the following mailbox info: + | name | total | unread | + | INBOX | 0 | 0 | + | Drafts | 0 | 0 | + | Sent | 0 | 0 | + | Starred | 0 | 0 | + | Archive | 0 | 0 | + | Spam | 0 | 0 | + | Trash | 0 | 0 | + | All Mail | 4 | 2 | + | Folders | 0 | 0 | + | Folders/one | 2 | 1 | + | Folders/two | 2 | 1 | + | Labels | 0 | 0 | + | Labels/three | 0 | 0 | + + Scenario: If the gluon files are deleted, the account is synced again + Given the user logs in with username "user@pm.me" and password "password" + And user "user@pm.me" finishes syncing + And bridge stops + And the user deletes the gluon files + And bridge starts + When user "user@pm.me" connects and authenticates IMAP client "1" + Then IMAP client "1" eventually sees the following mailbox info: | name | total | unread | | INBOX | 0 | 0 | | Drafts | 0 | 0 | diff --git a/tests/imap_test.go b/tests/imap_test.go index 43e3f889..7d4a92ad 100644 --- a/tests/imap_test.go +++ b/tests/imap_test.go @@ -124,6 +124,14 @@ func (s *scenario) imapClientSeesTheFollowingMailboxInfo(clientID string, table return matchMailboxes(haveMailboxes, table) } +func (s *scenario) imapClientEventuallySeesTheFollowingMailboxInfo(clientID string, table *godog.Table) error { + return eventually( + func() error { return s.imapClientSeesTheFollowingMailboxInfo(clientID, table) }, + 5*time.Second, + 100*time.Millisecond, + ) +} + func (s *scenario) imapClientSeesTheFollowingMailboxInfoForMailbox(clientID, mailbox string, table *godog.Table) error { _, client := s.t.getIMAPClient(clientID)