fix(BRIDGE-261): delete gluon data during user deletion; integration tests; FF kill switch; Sentry report if error;

internal/bridge/bridge.go

@@ -733,3 +733,8 @@ func (bridge *Bridge) ReportMessageWithContext(message string, messageCtx report
 		}).Info("Error occurred when sending Report to Sentry")
 	}
 }
+
+// GetUsers is only used for testing purposes.
+func (bridge *Bridge) GetUsers() map[string]*user.User {
+	return bridge.users
+}

internal/bridge/user.go

@@ -33,6 +33,7 @@ import (
 	"github.com/ProtonMail/proton-bridge/v3/internal/safe"
 	"github.com/ProtonMail/proton-bridge/v3/internal/services/imapservice"
 	"github.com/ProtonMail/proton-bridge/v3/internal/try"
+	"github.com/ProtonMail/proton-bridge/v3/internal/unleash"
 	"github.com/ProtonMail/proton-bridge/v3/internal/user"
 	"github.com/ProtonMail/proton-bridge/v3/internal/vault"
 	"github.com/go-resty/resty/v2"
@@ -607,7 +608,7 @@ func (bridge *Bridge) newVaultUser(
 	return bridge.vault.GetOrAddUser(apiUser.ID, apiUser.Name, apiUser.Email, authUID, authRef, saltedKeyPass)
 }
 
-// logout logs out the given user, optionally logging them out from the API too.
+// logoutUser logs out the given user, optionally logging them out from the API and deleting user related gluon data.
 func (bridge *Bridge) logoutUser(ctx context.Context, user *user.User, withAPI, withData bool) {
 	defer delete(bridge.users, user.ID())
 
@@ -617,7 +618,7 @@ func (bridge *Bridge) logoutUser(ctx context.Context, user *user.User, withAPI,
 		"withData": withData,
 	}).Debug("Logging out user")
 
-	if err := user.Logout(ctx, withAPI); err != nil {
+	if err := user.Logout(ctx, withAPI, withData, bridge.unleashService.GetFlagValue(unleash.UserRemovalGluonDataCleanupDisabled)); err != nil {
 		logUser.WithError(err).Error("Failed to logout user")
 	}
 

internal/services/imapservice/service.go

@@ -233,6 +233,12 @@ func (s *Service) OnLogout(ctx context.Context) error {
 	return err
 }
 
+func (s *Service) OnDelete(ctx context.Context) error {
+	_, err := s.cpc.Send(ctx, &onDeleteReq{})
+
+	return err
+}
+
 func (s *Service) ShowAllMail(ctx context.Context, v bool) error {
 	_, err := s.cpc.Send(ctx, &showAllMailReq{v: v})
 
@@ -362,6 +368,11 @@ func (s *Service) run(ctx context.Context) { //nolint gocyclo
 				err := s.removeConnectorsFromServer(ctx, s.connectors, false)
 				req.Reply(ctx, nil, err)
 
+			case *onDeleteReq:
+				s.log.Debug("Delete Request")
+				err := s.removeConnectorsFromServer(ctx, s.connectors, true)
+				req.Reply(ctx, nil, err)
+
 			case *showAllMailReq:
 				s.log.Debug("Show all mail request")
 				req.Reply(ctx, nil, nil)
@@ -644,6 +655,8 @@ type onLogoutReq struct{}
 
 type showAllMailReq struct{ v bool }
 
+type onDeleteReq struct{}
+
 type setAddressModeReq struct {
 	mode usertypes.AddressMode
 }

internal/unleash/service.go

@@ -37,8 +37,9 @@ var pollJitter = 2 * time.Minute  //nolint:gochecknoglobals
 const filename = "unleash_flags"
 
 const (
-	EventLoopNotificationDisabled   = "InboxBridgeEventLoopNotificationDisabled"
+	EventLoopNotificationDisabled       = "InboxBridgeEventLoopNotificationDisabled"
-	IMAPAuthenticateCommandDisabled = "InboxBridgeImapAuthenticateCommandDisabled"
+	IMAPAuthenticateCommandDisabled     = "InboxBridgeImapAuthenticateCommandDisabled"
+	UserRemovalGluonDataCleanupDisabled = "InboxBridgeUserRemovalGluonDataCleanupDisabled"
 )
 
 type requestFeaturesFn func(ctx context.Context) (proton.FeatureFlagResult, error)

internal/user/user.go

@@ -592,8 +592,13 @@ func (user *User) CheckAuth(email string, password []byte) (string, error) {
 }
 
 // Logout logs the user out from the API.
-func (user *User) Logout(ctx context.Context, withAPI bool) error {
+func (user *User) Logout(ctx context.Context, withAPI, withData, withDataDisabledKillSwitch bool) error {
-	user.log.WithField("withAPI", withAPI).Info("Logging out user")
+	user.log.WithFields(
+		logrus.Fields{
+			"withAPI":                    withAPI,
+			"withData":                   withData,
+			"withDataDisabledKillSwitch": withDataDisabledKillSwitch,
+		}).Info("Logging out user")
 
 	user.log.Debug("Canceling ongoing tasks")
 
@@ -601,8 +606,20 @@ func (user *User) Logout(ctx context.Context, withAPI bool) error {
 		return fmt.Errorf("failed to remove user from smtp server: %w", err)
 	}
 
-	if err := user.imapService.OnLogout(ctx); err != nil {
+	if withData && !withDataDisabledKillSwitch {
-		return fmt.Errorf("failed to remove user from imap server: %w", err)
+		if err := user.imapService.OnDelete(ctx); err != nil {
+			if rerr := user.reporter.ReportMessageWithContext("Failed to delete user IMAP data", map[string]any{
+				"error": err.Error(),
+			}); rerr != nil {
+				logrus.WithError(rerr).Info("Failed to report user IMAP deletion issue to Sentry")
+			}
+
+			return fmt.Errorf("failed to delete user from imap server: %w", err)
+		}
+	} else {
+		if err := user.imapService.OnLogout(ctx); err != nil {
+			return fmt.Errorf("failed to remove user from imap server: %w", err)
+		}
 	}
 
 	user.tasks.CancelAndWait()

tests/features/user/delete_imap.feature

@@ -0,0 +1,25 @@
+Feature: User deletion with IMAP data removal
+  Background:
+    Given there exists an account with username "[user:user]" and password "password"
+    And the account "[user:user]" has the following custom mailboxes:
+      | name | type   |
+      | one  | folder |
+    And the address "[user:user]@[domain]" of account "[user:user]" has the following messages in "Folders/one":
+      | from          | to            | subject | unread |
+      | a@example.com | a@example.com | one     | true   |
+      | b@example.com | b@example.com | two     | false  |
+      | c@example.com | c@example.com | three   | true   |
+      | c@example.com | c@example.com | four    | false   |
+    Then it succeeds
+    When bridge starts
+    And the user logs in with username "[user:user]" and password "password"
+    And user "[user:user]" finishes syncing
+    Then it succeeds
+
+  Scenario: User is deleted from Bridge and IMAP data is removed
+    When user "[user:user]" connects and authenticates IMAP client "1"
+    Then IMAP client "1" sees the following mailbox info for "Folders/one":
+      | name        | total | unread |
+      | Folders/one | 4     | 2      |
+    And user "[user:user]" is deleted alongside IMAP data for client "1"
+    Then it succeeds

tests/steps_test.go

@@ -116,6 +116,7 @@ func (s *scenario) steps(ctx *godog.ScenarioContext) {
 	ctx.Step(`^user "([^"]*)" has telemetry set to (\d+)$`, s.userHasTelemetrySetTo)
 	ctx.Step(`^the bridge password of user "([^"]*)" is changed to "([^"]*)"`, s.bridgePasswordOfUserIsChangedTo)
 	ctx.Step(`^the bridge password of user "([^"]*)" is equal to "([^"]*)"`, s.bridgePasswordOfUserIsEqualTo)
+	ctx.Step(`^user "([^"]*)" is deleted alongside IMAP data for client "([^"]*)"$`, s.userIsDeletedAndImapDataRemoved)
 
 	// ==== ACCOUNT SETTINGS ====
 	ctx.Step(`^the account "([^"]*)" has public key attachment "([^"]*)"`, s.accountHasPublicKeyAttachment)

tests/user_test.go

@@ -22,6 +22,8 @@ import (
 	"errors"
 	"fmt"
 	"net/mail"
+	"os"
+	"path/filepath"
 	"strings"
 	"time"
 
@@ -388,6 +390,70 @@ func (s *scenario) userIsDeleted(username string) error {
 	return s.t.bridge.DeleteUser(context.Background(), s.t.getUserByName(username).getUserID())
 }
 
+func (s *scenario) userIsDeletedAndImapDataRemoved(username string) error {
+	gluonCacheDir := s.t.bridge.GetGluonCacheDir()
+	userID := s.t.getUserByName(username).userID
+	userMap := s.t.bridge.GetUsers()
+	userObj, ok := userMap[userID]
+	if !ok {
+		return fmt.Errorf("could not find user object")
+	}
+
+	gluonIDMap := userObj.GetGluonIDs()
+	gluonIDs := make([]string, 0, len(gluonIDMap))
+	for _, id := range gluonIDMap {
+		gluonIDs = append(gluonIDs, id)
+	}
+
+	var relevantPaths []string
+	if err := filepath.Walk(gluonCacheDir, func(path string, _ os.FileInfo, err error) error {
+		if err != nil {
+			return err
+		}
+		for _, gluonID := range gluonIDs {
+			if strings.Contains(path, gluonID) {
+				relevantPaths = append(relevantPaths, path)
+			}
+		}
+		return nil
+	}); err != nil {
+		return err
+	}
+
+	if len(relevantPaths) == 0 {
+		return fmt.Errorf("found no user related gluon paths")
+	}
+
+	if err := s.t.bridge.DeleteUser(context.Background(), userID); err != nil {
+		return fmt.Errorf("could not delete user: %w", err)
+	}
+
+	foundDeferredDelete := false
+	var remainingPaths []string
+	if err := filepath.Walk(gluonCacheDir, func(path string, _ os.FileInfo, err error) error {
+		if err != nil {
+			return err
+		}
+		for _, gluonID := range gluonIDs {
+			if strings.Contains(path, gluonID) {
+				remainingPaths = append(remainingPaths, path)
+			}
+		}
+		if strings.Contains(path, "deferred_delete") {
+			foundDeferredDelete = true
+		}
+		return nil
+	}); err != nil {
+		return err
+	}
+
+	if len(remainingPaths) == 0 && foundDeferredDelete {
+		return nil
+	}
+
+	return fmt.Errorf("user gluon data is still present or could not find deferred deletion directory")
+}
+
 func (s *scenario) theAuthOfUserIsRevoked(username string) error {
 	return s.t.withClient(context.Background(), username, func(ctx context.Context, client *proton.Client) error {
 		return client.AuthRevokeAll(ctx)