From ae7ae2886f389db69da8b7ee44f3cb66ae511104 Mon Sep 17 00:00:00 2001 From: James Houlahan Date: Mon, 28 Nov 2022 19:58:10 +0100 Subject: [PATCH] GODT-2041: Crash after factory reset I forgot to remove the user from the users map during factory reset. This meant the (deleted) would attempt to be closed during teardown. --- internal/bridge/bridge_test.go | 21 +++++++++++++++++++++ internal/bridge/user.go | 5 ++--- internal/bridge/user_events.go | 1 - 3 files changed, 23 insertions(+), 4 deletions(-) diff --git a/internal/bridge/bridge_test.go b/internal/bridge/bridge_test.go index fc5e8629..79e0b205 100644 --- a/internal/bridge/bridge_test.go +++ b/internal/bridge/bridge_test.go @@ -421,6 +421,27 @@ func TestBridge_AddressWithoutKeys(t *testing.T) { }) } +func TestBridge_FactoryReset(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) { + // 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)) + + // Perform a factory reset. + bridge.FactoryReset(ctx) + + // The user is gone. + require.Equal(t, []string{}, bridge.GetUserIDs()) + require.Equal(t, []string{}, 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/user.go b/internal/bridge/user.go index 951c3c9f..e8eff23c 100644 --- a/internal/bridge/user.go +++ b/internal/bridge/user.go @@ -223,8 +223,6 @@ func (bridge *Bridge) LogoutUser(ctx context.Context, userID string) error { return ErrNoSuchUser } - defer delete(bridge.users, user.ID()) - bridge.logoutUser(ctx, user, true, false) bridge.publish(events.UserLoggedOut{ @@ -245,7 +243,6 @@ func (bridge *Bridge) DeleteUser(ctx context.Context, userID string) error { } if user, ok := bridge.users[userID]; ok { - defer delete(bridge.users, user.ID()) bridge.logoutUser(ctx, user, true, true) } @@ -528,6 +525,8 @@ func (bridge *Bridge) newVaultUser( // logout logs out the given user, optionally logging them out from the API too. func (bridge *Bridge) logoutUser(ctx context.Context, user *user.User, withAPI, withData bool) { + defer delete(bridge.users, user.ID()) + logrus.WithFields(logrus.Fields{ "userID": user.ID(), "withAPI": withAPI, diff --git a/internal/bridge/user_events.go b/internal/bridge/user_events.go index 6d844b4a..af5a6cfe 100644 --- a/internal/bridge/user_events.go +++ b/internal/bridge/user_events.go @@ -119,7 +119,6 @@ func (bridge *Bridge) handleUserRefreshed(ctx context.Context, user *user.User) func (bridge *Bridge) handleUserDeauth(ctx context.Context, user *user.User) { safe.Lock(func() { - defer delete(bridge.users, user.ID()) bridge.logoutUser(ctx, user, false, false) }, bridge.usersLock) }