From 1c374b59d301a4b7e10b06d28101ecdf5e17fd45 Mon Sep 17 00:00:00 2001
From: Leander Beernaert <leander.beernaert@proton.ch>
Date: Tue, 29 Nov 2022 08:38:21 +0100
Subject: [PATCH] GODT-2160: Prevent double closing of bridge if restart fails

Set imapServer instance to nil once the server is no longer running to
prevent multiple calls to close on shutdown.
---
 internal/bridge/bridge_test.go | 24 ++++++++++++++++++++++++
 internal/bridge/imap.go        | 18 ++++++++++++++++--
 internal/bridge/user_events.go |  8 ++++++++
 3 files changed, 48 insertions(+), 2 deletions(-)

diff --git a/internal/bridge/bridge_test.go b/internal/bridge/bridge_test.go
index 79e0b205..905dff2a 100644
--- a/internal/bridge/bridge_test.go
+++ b/internal/bridge/bridge_test.go
@@ -442,6 +442,30 @@ func TestBridge_FactoryReset(t *testing.T) {
 	})
 }
 
+/* // This test will be enabled in a followup patch
+func TestBridge_ChangeCacheDirectory(t *testing.T) {
+	withEnv(t, func(ctx context.Context, s *server.Server, netCtl *proton.NetCtl, locator bridge.Locator, vaultKey []byte) {
+		withBridge(ctx, t, s.GetHostURL(), netCtl, locator, vaultKey, func(bridge *bridge.Bridge, mocks *bridge.Mocks) {
+			newCacheDir := t.TempDir()
+
+			// Login the user.
+			userID, err := bridge.LoginFull(ctx, username, password, nil, nil)
+			require.NoError(t, err)
+
+			// The user is now connected.
+			require.Equal(t, []string{userID}, bridge.GetUserIDs())
+			require.Equal(t, []string{userID}, getConnectedUserIDs(t, bridge))
+
+			// Change directory
+			_ = bridge.SetGluonDir(ctx, newCacheDir)
+
+			// The user is still there
+			//require.Equal(t, []string{userID}, bridge.GetUserIDs())
+			//require.Equal(t, []string{userID}, getConnectedUserIDs(t, bridge))
+		})
+	})
+} */
+
 // withEnv creates the full test environment and runs the tests.
 func withEnv(t *testing.T, tests func(context.Context, *server.Server, *proton.NetCtl, bridge.Locator, []byte), opts ...server.Option) {
 	server := server.New(opts...)
diff --git a/internal/bridge/imap.go b/internal/bridge/imap.go
index 6cf36bf6..6f3a75d6 100644
--- a/internal/bridge/imap.go
+++ b/internal/bridge/imap.go
@@ -47,6 +47,10 @@ const (
 )
 
 func (bridge *Bridge) serveIMAP() error {
+	if bridge.imapServer == nil {
+		return fmt.Errorf("no imap server instance running")
+	}
+
 	logrus.Info("Starting IMAP server")
 
 	imapListener, err := newListener(bridge.vault.GetIMAPPort(), bridge.vault.GetIMAPSSL(), bridge.tlsConfig)
@@ -82,8 +86,11 @@ func (bridge *Bridge) restartIMAP() error {
 func (bridge *Bridge) closeIMAP(ctx context.Context) error {
 	logrus.Info("Closing IMAP server")
 
-	if err := bridge.imapServer.Close(ctx); err != nil {
-		return fmt.Errorf("failed to close IMAP server: %w", err)
+	if bridge.imapServer != nil {
+		if err := bridge.imapServer.Close(ctx); err != nil {
+			return fmt.Errorf("failed to close IMAP server: %w", err)
+		}
+		bridge.imapServer = nil
 	}
 
 	if bridge.imapListener != nil {
@@ -97,6 +104,10 @@ func (bridge *Bridge) closeIMAP(ctx context.Context) error {
 
 // addIMAPUser connects the given user to gluon.
 func (bridge *Bridge) addIMAPUser(ctx context.Context, user *user.User) error {
+	if bridge.imapServer == nil {
+		return fmt.Errorf("no imap server instance running")
+	}
+
 	imapConn, err := user.NewIMAPConnectors()
 	if err != nil {
 		return fmt.Errorf("failed to create IMAP connectors: %w", err)
@@ -135,6 +146,9 @@ func (bridge *Bridge) addIMAPUser(ctx context.Context, user *user.User) error {
 
 // removeIMAPUser disconnects the given user from gluon, optionally also removing its files.
 func (bridge *Bridge) removeIMAPUser(ctx context.Context, user *user.User, withData bool) error {
+	if bridge.imapServer == nil {
+		return fmt.Errorf("no imap server instance running")
+	}
 	logrus.WithFields(logrus.Fields{
 		"userID":   user.ID(),
 		"withData": withData,
diff --git a/internal/bridge/user_events.go b/internal/bridge/user_events.go
index af5a6cfe..663cfaef 100644
--- a/internal/bridge/user_events.go
+++ b/internal/bridge/user_events.go
@@ -58,6 +58,10 @@ func (bridge *Bridge) handleUserEvent(ctx context.Context, user *user.User, even
 
 func (bridge *Bridge) handleUserAddressCreated(ctx context.Context, user *user.User, event events.UserAddressCreated) error {
 	if user.GetAddressMode() == vault.SplitMode {
+		if bridge.imapServer == nil {
+			return fmt.Errorf("no imap server instance running")
+		}
+
 		gluonID, err := bridge.imapServer.AddUser(ctx, user.NewIMAPConnector(event.AddressID), user.GluonKey())
 		if err != nil {
 			return fmt.Errorf("failed to add user to IMAP server: %w", err)
@@ -86,6 +90,10 @@ func (bridge *Bridge) handleUserAddressUpdated(_ context.Context, user *user.Use
 
 func (bridge *Bridge) handleUserAddressDeleted(ctx context.Context, user *user.User, event events.UserAddressDeleted) error {
 	if user.GetAddressMode() == vault.SplitMode {
+		if bridge.imapServer == nil {
+			return fmt.Errorf("no imap server instance running")
+		}
+
 		gluonID, ok := user.GetGluonID(event.AddressID)
 		if !ok {
 			return fmt.Errorf("gluon ID not found for address %s", event.AddressID)