fix(GODT-3123): Trigger bad event on empty EventID on existing accounts

See `checkIrrecoverableEventID` for more details.
This commit is contained in:
Leander Beernaert 2023-11-15 09:45:27 +01:00
parent 96517b7fb1
commit 2d44ccaee0
6 changed files with 191 additions and 1 deletions

View File

@ -494,7 +494,7 @@ func (bridge *Bridge) addUser(
return fmt.Errorf("failed to add vault user: %w", err)
}
if err := bridge.addUserWithVault(ctx, client, apiUser, vaultUser); err != nil {
if err := bridge.addUserWithVault(ctx, client, apiUser, vaultUser, isNew); err != nil {
if _, ok := err.(*resty.ResponseError); ok || isLogin {
logrus.WithError(err).Error("Failed to add user, clearing its secrets from vault")
@ -529,6 +529,7 @@ func (bridge *Bridge) addUserWithVault(
client *proton.Client,
apiUser proton.User,
vault *vault.User,
isNew bool,
) error {
statsPath, err := bridge.locator.ProvideStatsPath()
if err != nil {
@ -556,6 +557,7 @@ func (bridge *Bridge) addUserWithVault(
&bridgeEventSubscription{b: bridge},
bridge.syncService,
syncSettingsPath,
isNew,
)
if err != nil {
return fmt.Errorf("failed to create user: %w", err)

View File

@ -58,6 +58,10 @@ func (s Status) IsComplete() bool {
return s.HasLabels && s.HasMessages
}
func (s Status) InProgress() bool {
return s.HasLabels || s.HasMessageCount
}
// Regulator is an abstraction for the sync service, since it regulates the number of concurrent sync activities.
type Regulator interface {
Sync(ctx context.Context, stage *Job)

View File

@ -105,6 +105,7 @@ func New(
eventSubscription events.Subscription,
syncService syncservice.Regulator,
syncConfigDir string,
isNew bool,
) (*User, error) {
user, err := newImpl(
ctx,
@ -122,6 +123,7 @@ func New(
eventSubscription,
syncService,
syncConfigDir,
isNew,
)
if err != nil {
// Cleanup any pending resources on error
@ -152,6 +154,7 @@ func newImpl(
eventSubscription events.Subscription,
syncService syncservice.Regulator,
syncConfigDir string,
isNew bool,
) (*User, error) {
logrus.WithField("userID", apiUser.ID).Info("Creating new user")
@ -295,6 +298,14 @@ func newImpl(
return nil
})
// If it's not a fresh user check the eventID and evaluate whether it is valid. If it's a new user, we don't
// need to perform this check.
if !isNew {
if err := checkIrrecoverableEventID(ctx, encVault.EventID(), apiUser.ID, syncConfigDir, user); err != nil {
return nil, err
}
}
// Start Event Service
lastEventID, err := user.eventService.Start(ctx, user.serviceGroup)
if err != nil {

View File

@ -0,0 +1,68 @@
// Copyright (c) 2023 Proton AG
//
// This file is part of Proton Mail Bridge.
//
// Proton Mail Bridge is free software: you can redistribute it and/or modify
// it under the terms of the GNU General Public License as published by
// the Free Software Foundation, either version 3 of the License, or
// (at your option) any later version.
//
// Proton Mail Bridge is distributed in the hope that it will be useful,
// but WITHOUT ANY WARRANTY; without even the implied warranty of
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
// GNU General Public License for more details.
//
// You should have received a copy of the GNU General Public License
// along with Proton Mail Bridge. If not, see <https://www.gnu.org/licenses/>.
package user
import (
"context"
"fmt"
"github.com/ProtonMail/proton-bridge/v3/internal/events"
"github.com/ProtonMail/proton-bridge/v3/internal/services/imapservice"
)
func checkIrrecoverableEventID(
ctx context.Context,
lastEventID,
userID,
syncConfigDir string,
publisher events.EventPublisher,
) error {
// If we detect that the event ID stored in the vault got reset, the user is not a new account and
// we have started or finished syncing: this is an irrecoverable state and we should produce a bad event.
if lastEventID != "" {
return nil
}
syncConfigPath := imapservice.GetSyncConfigPath(syncConfigDir, userID)
syncState, err := imapservice.NewSyncState(syncConfigPath)
if err != nil {
return fmt.Errorf("failed to read imap sync state: %w", err)
}
syncStatus, err := syncState.GetSyncStatus(ctx)
if err != nil {
return fmt.Errorf("failed to imap sync status: %w", err)
}
if syncStatus.IsComplete() || syncStatus.InProgress() {
publisher.PublishEvent(ctx, newEmptyEventIDBadEvent(userID))
}
return nil
}
func newEmptyEventIDBadEvent(userID string) events.UserBadEvent {
return events.UserBadEvent{
UserID: userID,
OldEventID: "",
NewEventID: "",
EventInfo: "EventID missing from vault",
Error: fmt.Errorf("eventID in vault is empty, when it shouldn't be"),
}
}

View File

@ -0,0 +1,104 @@
// Copyright (c) 2023 Proton AG
//
// This file is part of Proton Mail Bridge.
//
// Proton Mail Bridge is free software: you can redistribute it and/or modify
// it under the terms of the GNU General Public License as published by
// the Free Software Foundation, either version 3 of the License, or
// (at your option) any later version.
//
// Proton Mail Bridge is distributed in the hope that it will be useful,
// but WITHOUT ANY WARRANTY; without even the implied warranty of
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
// GNU General Public License for more details.
//
// You should have received a copy of the GNU General Public License
// along with Proton Mail Bridge. If not, see <https://www.gnu.org/licenses/>.
package user
import (
"context"
"testing"
"github.com/ProtonMail/proton-bridge/v3/internal/events/mocks"
"github.com/ProtonMail/proton-bridge/v3/internal/services/imapservice"
"github.com/golang/mock/gomock"
"github.com/stretchr/testify/require"
)
func TestCheckIrrecoverableEventID_EventIDIsEmptyButNoSyncStarted(t *testing.T) {
tmpDir := t.TempDir()
userID := "foo"
mockCtrl := gomock.NewController(t)
publisher := mocks.NewMockEventPublisher(mockCtrl)
require.NoError(t, checkIrrecoverableEventID(context.Background(), "", userID, tmpDir, publisher))
}
func TestCheckIrrecoverableEventID_EventIDIsNotEmptyButNoSyncStarted(t *testing.T) {
tmpDir := t.TempDir()
userID := "foo"
mockCtrl := gomock.NewController(t)
publisher := mocks.NewMockEventPublisher(mockCtrl)
require.NoError(t, checkIrrecoverableEventID(context.Background(), "ffoofo", userID, tmpDir, publisher))
}
func TestCheckIrrecoverableEventID_EventIDIsEmptyButSyncStarted(t *testing.T) {
tmpDir := t.TempDir()
userID := "foo"
mockCtrl := gomock.NewController(t)
publisher := mocks.NewMockEventPublisher(mockCtrl)
publisher.EXPECT().PublishEvent(gomock.Any(), gomock.Eq(newEmptyEventIDBadEvent(userID)))
require.NoError(t, genSyncState(context.Background(), userID, tmpDir, false))
require.NoError(t, checkIrrecoverableEventID(context.Background(), "", userID, tmpDir, publisher))
}
func TestCheckIrrecoverableEventID_EventIDIsEmptyButSyncFinished(t *testing.T) {
tmpDir := t.TempDir()
userID := "foo"
mockCtrl := gomock.NewController(t)
publisher := mocks.NewMockEventPublisher(mockCtrl)
publisher.EXPECT().PublishEvent(gomock.Any(), gomock.Eq(newEmptyEventIDBadEvent(userID)))
require.NoError(t, genSyncState(context.Background(), userID, tmpDir, true))
require.NoError(t, checkIrrecoverableEventID(context.Background(), "", userID, tmpDir, publisher))
}
func TestCheckIrrecoverableEventID_EventIDIsNotEmptyButSyncFinished(t *testing.T) {
tmpDir := t.TempDir()
userID := "foo"
mockCtrl := gomock.NewController(t)
publisher := mocks.NewMockEventPublisher(mockCtrl)
require.NoError(t, genSyncState(context.Background(), userID, tmpDir, true))
require.NoError(t, checkIrrecoverableEventID(context.Background(), "some event", userID, tmpDir, publisher))
}
func genSyncState(ctx context.Context, userID, dir string, finished bool) error {
s, err := imapservice.NewSyncState(imapservice.GetSyncConfigPath(dir, userID))
if err != nil {
return err
}
if finished {
if err := s.SetHasLabels(ctx, true); err != nil {
return err
}
if err := s.SetHasMessages(ctx, true); err != nil {
return err
}
if err := s.SetMessageCount(ctx, 10); err != nil {
return err
}
} else {
if err := s.SetHasLabels(ctx, true); err != nil {
return err
}
}
return nil
}

View File

@ -158,6 +158,7 @@ func withUser(tb testing.TB, ctx context.Context, _ *server.Server, m *proton.Ma
nullEventSubscription,
nil,
"",
true,
)
require.NoError(tb, err)
defer user.Close()