yesBad/proton-bridge
1
0
Fork 0
forked from Silverfish/proton-bridge
Code Actions Activity

fix(GODT-1945): Handle disabled addresses correctly

Browse Source 
When listing all a user's email addresses (e.g. for apple mail autoconf)
we need to exclude disabled addresses. Similarly, we need to remove
them from gluon if using split mode.
This commit is contained in:
James Houlahan
2023-02-23 16:21:41 +01:00
parent c15917aba4
commit 810705ba01
6 changed files with 264 additions and 48 deletions
Download Patch File Download Diff File Expand all files Collapse all files

77
internal/bridge/user_event_test.go
View File

@ -534,6 +534,83 @@ func TestBridge_User_SendDraftRemoveDraftFlag(t *testing.T) {
})
}
func TestBridge_User_DisableEnableAddress(t *testing.T) {
withEnv(t, func(ctx context.Context, s *server.Server, netCtl *proton.NetCtl, locator bridge.Locator, storeKey []byte) {
// Create a user.
userID, _, err := s.CreateUser("user", password)
require.NoError(t, err)
// Create an additional address for the user.
aliasID, err := s.CreateAddress(userID, "alias@"+s.GetDomain(), password)
require.NoError(t, err)
withBridge(ctx, t, s.GetHostURL(), netCtl, locator, storeKey, func(bridge *bridge.Bridge, mocks *bridge.Mocks) {
require.NoError(t, getErr(bridge.LoginFull(ctx, "user", password, nil, nil)))
// Initially we should list the address.
info, err := bridge.QueryUserInfo("user")
require.NoError(t, err)
require.Contains(t, info.Addresses, "alias@"+s.GetDomain())
})
// Disable the address.
withClient(ctx, t, s, "user", password, func(ctx context.Context, c *proton.Client) {
require.NoError(t, c.DisableAddress(ctx, aliasID))
})
withBridge(ctx, t, s.GetHostURL(), netCtl, locator, storeKey, func(bridge *bridge.Bridge, mocks *bridge.Mocks) {
// Eventually we shouldn't list the address.
require.Eventually(t, func() bool {
info, err := bridge.QueryUserInfo("user")
require.NoError(t, err)
return xslices.Index(info.Addresses, "alias@"+s.GetDomain()) < 0
}, 5*time.Second, 100*time.Millisecond)
})
// Enable the address.
withClient(ctx, t, s, "user", password, func(ctx context.Context, c *proton.Client) {
require.NoError(t, c.EnableAddress(ctx, aliasID))
})
withBridge(ctx, t, s.GetHostURL(), netCtl, locator, storeKey, func(bridge *bridge.Bridge, mocks *bridge.Mocks) {
// Eventually we should list the address.
require.Eventually(t, func() bool {
info, err := bridge.QueryUserInfo("user")
require.NoError(t, err)
return xslices.Index(info.Addresses, "alias@"+s.GetDomain()) >= 0
}, 5*time.Second, 100*time.Millisecond)
})
})
}
func TestBridge_User_CreateDisabledAddress(t *testing.T) {
withEnv(t, func(ctx context.Context, s *server.Server, netCtl *proton.NetCtl, locator bridge.Locator, storeKey []byte) {
// Create a user.
userID, _, err := s.CreateUser("user", password)
require.NoError(t, err)
// Create an additional address for the user.
aliasID, err := s.CreateAddress(userID, "alias@"+s.GetDomain(), password)
require.NoError(t, err)
// Immediately disable the address.
withClient(ctx, t, s, "user", password, func(ctx context.Context, c *proton.Client) {
require.NoError(t, c.DisableAddress(ctx, aliasID))
})
withBridge(ctx, t, s.GetHostURL(), netCtl, locator, storeKey, func(bridge *bridge.Bridge, mocks *bridge.Mocks) {
require.NoError(t, getErr(bridge.LoginFull(ctx, "user", password, nil, nil)))
// Initially we shouldn't list the address.
info, err := bridge.QueryUserInfo("user")
require.NoError(t, err)
require.NotContains(t, info.Addresses, "alias@"+s.GetDomain())
})
})
}
// userLoginAndSync logs in user and waits until user is fully synced.
func userLoginAndSync(
ctx context.Context,

60
internal/bridge/user_events.go
View File

@ -37,9 +37,14 @@ func (bridge *Bridge) handleUserEvent(ctx context.Context, user *user.User, even
return fmt.Errorf("failed to handle user address created event: %w", err)
}
case events.UserAddressUpdated:
if err := bridge.handleUserAddressUpdated(ctx, user, event); err != nil {
return fmt.Errorf("failed to handle user address updated event: %w", err)
case events.UserAddressEnabled:
if err := bridge.handleUserAddressEnabled(ctx, user, event); err != nil {
return fmt.Errorf("failed to handle user address enabled event: %w", err)
}
case events.UserAddressDisabled:
if err := bridge.handleUserAddressDisabled(ctx, user, event); err != nil {
return fmt.Errorf("failed to handle user address disabled event: %w", err)
}
case events.UserAddressDeleted:
@ -66,7 +71,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 user.GetAddressMode() != vault.SplitMode {
return nil
}
if bridge.imapServer == nil {
return fmt.Errorf("no imap server instance running")
}
@ -79,26 +87,53 @@ func (bridge *Bridge) handleUserAddressCreated(ctx context.Context, user *user.U
if err := user.SetGluonID(event.AddressID, gluonID); err != nil {
return fmt.Errorf("failed to set gluon ID: %w", err)
}
return nil
}
func (bridge *Bridge) handleUserAddressEnabled(ctx context.Context, user *user.User, event events.UserAddressEnabled) error {
if user.GetAddressMode() != vault.SplitMode {
return nil
}
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)
}
if err := user.SetGluonID(event.AddressID, gluonID); err != nil {
return fmt.Errorf("failed to set gluon ID: %w", err)
}
return nil
}
// GODT-1948: Handle addresses that have been disabled!
func (bridge *Bridge) handleUserAddressUpdated(_ context.Context, user *user.User, _ events.UserAddressUpdated) error {
switch user.GetAddressMode() {
case vault.CombinedMode:
return fmt.Errorf("not implemented")
func (bridge *Bridge) handleUserAddressDisabled(ctx context.Context, user *user.User, event events.UserAddressDisabled) error {
if user.GetAddressMode() != vault.SplitMode {
return nil
}
case vault.SplitMode:
return fmt.Errorf("not implemented")
gluonID, ok := user.GetGluonID(event.AddressID)
if !ok {
return fmt.Errorf("gluon ID not found for address %s", event.AddressID)
}
if err := bridge.imapServer.RemoveUser(ctx, gluonID, true); err != nil {
return fmt.Errorf("failed to remove user from IMAP server: %w", err)
}
if err := user.RemoveGluonID(event.AddressID, gluonID); err != nil {
return fmt.Errorf("failed to remove gluon ID for address: %w", err)
}
return nil
}
func (bridge *Bridge) handleUserAddressDeleted(ctx context.Context, user *user.User, event events.UserAddressDeleted) error {
if user.GetAddressMode() == vault.SplitMode {
if user.GetAddressMode() != vault.SplitMode {
return nil
}
if bridge.imapServer == nil {
return fmt.Errorf("no imap server instance running")
}
@ -115,7 +150,6 @@ func (bridge *Bridge) handleUserAddressDeleted(ctx context.Context, user *user.U
if err := user.RemoveGluonID(event.AddressID, gluonID); err != nil {
return fmt.Errorf("failed to remove gluon ID for address: %w", err)
}
}
return nil
}

24
internal/events/address.go
View File

@ -35,6 +35,30 @@ func (event UserAddressCreated) String() string {
return fmt.Sprintf("UserAddressCreated: UserID: %s, AddressID: %s, Email: %s", event.UserID, event.AddressID, logging.Sensitive(event.Email))
}
type UserAddressEnabled struct {
eventBase
UserID string
AddressID string
Email string
}
func (event UserAddressEnabled) String() string {
return fmt.Sprintf("UserAddressEnabled: UserID: %s, AddressID: %s, Email: %s", event.UserID, event.AddressID, logging.Sensitive(event.Email))
}
type UserAddressDisabled struct {
eventBase
UserID string
AddressID string
Email string
}
func (event UserAddressDisabled) String() string {
return fmt.Sprintf("UserAddressDisabled: UserID: %s, AddressID: %s, Email: %s", event.UserID, event.AddressID, logging.Sensitive(event.Email))
}
type UserAddressUpdated struct {
eventBase

23
internal/frontend/cli/frontend.go
View File

@ -310,6 +310,14 @@ func (f *frontendCLI) watchEvents(eventCh <-chan events.Event) { // nolint:gocyc
case events.IMAPLoginFailed:
f.Printf("An IMAP login attempt failed for user %v\n", event.Username)
case events.UserAddressEnabled:
user, err := f.bridge.GetUserInfo(event.UserID)
if err != nil {
return
}
f.Printf("An address for %s was enabled. You may need to reconfigure your email client.\n", user.Username)
case events.UserAddressUpdated:
user, err := f.bridge.GetUserInfo(event.UserID)
if err != nil {
@ -318,8 +326,21 @@ func (f *frontendCLI) watchEvents(eventCh <-chan events.Event) { // nolint:gocyc
f.Printf("Address changed for %s. You may need to reconfigure your email client.\n", user.Username)
case events.UserAddressDisabled:
user, err := f.bridge.GetUserInfo(event.UserID)
if err != nil {
return
}
f.Printf("An address for %s was disabled. You may need to reconfigure your email client.\n", user.Username)
case events.UserAddressDeleted:
f.notifyLogout(event.Email)
user, err := f.bridge.GetUserInfo(event.UserID)
if err != nil {
return
}
f.Printf("An address for %s was disabled. You may need to reconfigure your email client.\n", user.Username)
case events.SyncStarted:
user, err := f.bridge.GetUserInfo(event.UserID)

68
internal/user/events.go
View File

@ -205,6 +205,12 @@ func (user *User) handleCreateAddressEvent(ctx context.Context, event proton.Add
user.apiAddrs[event.Address.ID] = event.Address
// If the address is disabled.
if event.Address.Status != proton.AddressStatusEnabled {
return nil
}
// If the address is enabled, we need to hook it up to the update channels.
switch user.vault.AddressMode() {
case vault.CombinedMode:
primAddr, err := getAddrIdx(user.apiAddrs, 0)
@ -231,6 +237,10 @@ func (user *User) handleCreateAddressEvent(ctx context.Context, event proton.Add
// Perform the sync in an RLock.
return safe.RLockRet(func() error {
if event.Address.Status != proton.AddressStatusEnabled {
return nil
}
if user.vault.AddressMode() == vault.SplitMode {
if err := syncLabels(ctx, user.apiLabels, user.updateCh[event.Address.ID]); err != nil {
return fmt.Errorf("failed to sync labels to new address: %w", err)
@ -248,18 +258,58 @@ func (user *User) handleUpdateAddressEvent(_ context.Context, event proton.Addre
"email": logging.Sensitive(event.Address.Email),
}).Info("Handling address updated event")
if _, ok := user.apiAddrs[event.Address.ID]; !ok {
oldAddr, ok := user.apiAddrs[event.Address.ID]
if !ok {
user.log.Debugf("Address %q does not exist", event.Address.ID)
return nil
}
user.apiAddrs[event.Address.ID] = event.Address
switch {
// If the address was newly enabled:
case oldAddr.Status != proton.AddressStatusEnabled && event.Address.Status == proton.AddressStatusEnabled:
switch user.vault.AddressMode() {
case vault.CombinedMode:
primAddr, err := getAddrIdx(user.apiAddrs, 0)
if err != nil {
return fmt.Errorf("failed to get primary address: %w", err)
}
user.updateCh[event.Address.ID] = user.updateCh[primAddr.ID]
case vault.SplitMode:
user.updateCh[event.Address.ID] = queue.NewQueuedChannel[imap.Update](0, 0)
}
user.eventCh.Enqueue(events.UserAddressEnabled{
UserID: user.apiUser.ID,
AddressID: event.Address.ID,
Email: event.Address.Email,
})
// If the address was newly disabled:
case oldAddr.Status == proton.AddressStatusEnabled && event.Address.Status != proton.AddressStatusEnabled:
if user.vault.AddressMode() == vault.SplitMode {
user.updateCh[event.ID].CloseAndDiscardQueued()
}
delete(user.updateCh, event.ID)
user.eventCh.Enqueue(events.UserAddressDisabled{
UserID: user.apiUser.ID,
AddressID: event.Address.ID,
Email: event.Address.Email,
})
// Otherwise it's just an update:
default:
user.eventCh.Enqueue(events.UserAddressUpdated{
UserID: user.apiUser.ID,
AddressID: event.Address.ID,
Email: event.Address.Email,
})
}
return nil
}, user.apiAddrsLock)
@ -275,12 +325,20 @@ func (user *User) handleDeleteAddressEvent(_ context.Context, event proton.Addre
return nil
}
if user.vault.AddressMode() == vault.SplitMode {
user.updateCh[event.ID].CloseAndDiscardQueued()
delete(user.updateCh, event.ID)
delete(user.apiAddrs, event.ID)
// If the address was disabled to begin with, we don't need to do anything.
if addr.Status != proton.AddressStatusEnabled {
return nil
}
delete(user.apiAddrs, event.ID)
// Otherwise, in split mode, drop the update queue.
if user.vault.AddressMode() == vault.SplitMode {
user.updateCh[event.ID].CloseAndDiscardQueued()
}
// And in either mode, remove the address from the update channel map.
delete(user.updateCh, event.ID)
user.eventCh.Enqueue(events.UserAddressDeleted{
UserID: user.apiUser.ID,

6
internal/user/user.go
View File

@ -264,11 +264,13 @@ func (user *User) Match(query string) bool {
}, user.apiUserLock, user.apiAddrsLock)
}
// Emails returns all the user's email addresses.
// Emails returns all the user's active email addresses.
// It returns them in sorted order; the user's primary address is first.
func (user *User) Emails() []string {
return safe.RLockRet(func() []string {
addresses := maps.Values(user.apiAddrs)
addresses := xslices.Filter(maps.Values(user.apiAddrs), func(addr proton.Address) bool {
return addr.Status == proton.AddressStatusEnabled
})
slices.SortFunc(addresses, func(a, b proton.Address) bool {
return a.Order < b.Order