From eee2c73a61c7f2670f905e9e020619e31f3db740 Mon Sep 17 00:00:00 2001 From: Xavier Michelon Date: Thu, 4 May 2023 19:57:17 +0200 Subject: [PATCH] feat(GODT-2610): re-use previous password when removing and adding back account. --- internal/vault/password_archive.go | 46 ++++++++++++++++++++++++ internal/vault/settings_test.go | 27 ++++++++++++++ internal/vault/types_password_archive.go | 25 +++++++++++++ internal/vault/types_settings.go | 4 +++ internal/vault/types_user.go | 4 +-- internal/vault/vault.go | 12 +++++-- tests/bdd_test.go | 2 ++ tests/ctx_bridge_test.go | 1 + tests/ctx_test.go | 2 ++ tests/features/user/relogin.feature | 11 +++++- tests/user_test.go | 33 +++++++++++++++++ 11 files changed, 161 insertions(+), 6 deletions(-) create mode 100644 internal/vault/password_archive.go create mode 100644 internal/vault/types_password_archive.go diff --git a/internal/vault/password_archive.go b/internal/vault/password_archive.go new file mode 100644 index 00000000..cf471adb --- /dev/null +++ b/internal/vault/password_archive.go @@ -0,0 +1,46 @@ +// 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 . + +package vault + +import ( + "crypto/sha256" + "fmt" +) + +// set archives the password for an email address, overwriting any existing archived value. +func (p *PasswordArchive) set(emailAddress string, password []byte) { + if p.Archive == nil { + p.Archive = make(map[string][]byte) + } + + p.Archive[emailHashString(emailAddress)] = password +} + +// get retrieves the archived password for an email address, or nil if not found. +func (p *PasswordArchive) get(emailAddress string) []byte { + if p.Archive == nil { + return nil + } + + return p.Archive[emailHashString(emailAddress)] +} + +// emailHashString returns a hash string for an email address as a hexadecimal string. +func emailHashString(emailAddress string) string { + return fmt.Sprintf("%x", sha256.Sum256([]byte(emailAddress))) +} diff --git a/internal/vault/settings_test.go b/internal/vault/settings_test.go index 3996b9ae..37efda0f 100644 --- a/internal/vault/settings_test.go +++ b/internal/vault/settings_test.go @@ -238,3 +238,30 @@ func TestVault_Settings_LastUserAgent(t *testing.T) { // Check the default first start value. require.Equal(t, vault.DefaultUserAgent, s.GetLastUserAgent()) } + +func Test_Settings_PasswordArchive(t *testing.T) { + // Create a new test vault. + s := newVault(t) + + // The store should have no users. + require.Empty(t, s.GetUserIDs()) + + // Create a new user. + user, err := s.AddUser("userID1", "username1", "username1@pm.me", "authUID1", "authRef1", []byte("keyPass1")) + require.NoError(t, err) + bridgePass := user.BridgePass() + + // Remove the user. + require.NoError(t, user.Close()) + require.NoError(t, s.DeleteUser("userID1")) + + // Add a different user. Another password is generated. + user, err = s.AddUser("userID2", "username2", "username2@pm.me", "authUID2", "authRef2", []byte("keyPass2")) + require.NoError(t, err) + require.NotEqual(t, user.BridgePass(), bridgePass) + + // Add the first user again. The password is restored. + user, err = s.AddUser("userID1", "username1", "username1@pm.me", "authUID1", "authRef1", []byte("keyPass1")) + require.NoError(t, err) + require.Equal(t, user.BridgePass(), bridgePass) +} diff --git a/internal/vault/types_password_archive.go b/internal/vault/types_password_archive.go new file mode 100644 index 00000000..715727f4 --- /dev/null +++ b/internal/vault/types_password_archive.go @@ -0,0 +1,25 @@ +// 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 . + +package vault + +// PasswordArchive maps a list email address hashes to passwords. +// The type is not defined as a map alias to prevent having to handle nil default values when vault was created by an older version of the application. +type PasswordArchive struct { + // we store the SHA-256 sum as string for readability and JSON marshalling of map[[32]byte][]byte will not be allowed, thus breaking vault-editor. + Archive map[string][]byte +} diff --git a/internal/vault/types_settings.go b/internal/vault/types_settings.go index d6961673..158791ea 100644 --- a/internal/vault/types_settings.go +++ b/internal/vault/types_settings.go @@ -53,6 +53,8 @@ type Settings struct { LastHeartbeatSent time.Time + PasswordArchive PasswordArchive + // **WARNING**: These entry can't be removed until they vault has proper migration support. SyncWorkers int SyncAttPool int @@ -105,5 +107,7 @@ func newDefaultSettings(gluonDir string) Settings { LastUserAgent: DefaultUserAgent, LastHeartbeatSent: time.Time{}, + + PasswordArchive: PasswordArchive{}, } } diff --git a/internal/vault/types_user.go b/internal/vault/types_user.go index a41085fb..9da47835 100644 --- a/internal/vault/types_user.go +++ b/internal/vault/types_user.go @@ -73,7 +73,7 @@ func (status SyncStatus) IsComplete() bool { return status.HasLabels && status.HasMessages } -func newDefaultUser(userID, username, primaryEmail, authUID, authRef string, keyPass []byte) UserData { +func newDefaultUser(userID, username, primaryEmail, authUID, authRef string, keyPass, bridgePass []byte) UserData { return UserData{ UserID: userID, Username: username, @@ -82,7 +82,7 @@ func newDefaultUser(userID, username, primaryEmail, authUID, authRef string, key GluonKey: newRandomToken(32), GluonIDs: make(map[string]string), UIDValidity: make(map[string]imap.UID), - BridgePass: newRandomToken(16), + BridgePass: bridgePass, AddressMode: CombinedMode, AuthUID: authUID, diff --git a/internal/vault/vault.go b/internal/vault/vault.go index 0b1c7002..e57c623f 100644 --- a/internal/vault/vault.go +++ b/internal/vault/vault.go @@ -133,7 +133,8 @@ func (vault *Vault) ForUser(parallelism int, fn func(*User) error) error { } // AddUser creates a new user in the vault with the given ID, username and password. -// A bridge password and gluon key are generated using the package's token generator. +// A gluon key is generated using the package's token generator. If a password is found in the password archive for this user, +// it is restored, otherwise a new bridge password is generated using the package's token generator. func (vault *Vault) AddUser(userID, username, primaryEmail, authUID, authRef string, keyPass []byte) (*User, error) { logrus.WithField("userID", userID).Info("Adding vault user") @@ -145,7 +146,12 @@ func (vault *Vault) AddUser(userID, username, primaryEmail, authUID, authRef str }); idx >= 0 { exists = true } else { - data.Users = append(data.Users, newDefaultUser(userID, username, primaryEmail, authUID, authRef, keyPass)) + bridgePass := data.Settings.PasswordArchive.get(primaryEmail) + if len(bridgePass) == 0 { + bridgePass = newRandomToken(16) + } + + data.Users = append(data.Users, newDefaultUser(userID, username, primaryEmail, authUID, authRef, keyPass, bridgePass)) } }); err != nil { return nil, err @@ -177,7 +183,7 @@ func (vault *Vault) DeleteUser(userID string) error { if idx < 0 { return } - + data.Settings.PasswordArchive.set(data.Users[idx].PrimaryEmail, data.Users[idx].BridgePass) data.Users = append(data.Users[:idx], data.Users[idx+1:]...) }) } diff --git a/tests/bdd_test.go b/tests/bdd_test.go index 1dadb870..def2ac15 100644 --- a/tests/bdd_test.go +++ b/tests/bdd_test.go @@ -180,6 +180,8 @@ func TestFeatures(testingT *testing.T) { ctx.Step(`^user "([^"]*)" is not listed$`, s.userIsNotListed) ctx.Step(`^user "([^"]*)" finishes syncing$`, s.userFinishesSyncing) 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) // ==== IMAP ==== ctx.Step(`^user "([^"]*)" connects IMAP client "([^"]*)"$`, s.userConnectsIMAPClient) diff --git a/tests/ctx_bridge_test.go b/tests/ctx_bridge_test.go index 5c62b0bf..b153ac39 100644 --- a/tests/ctx_bridge_test.go +++ b/tests/ctx_bridge_test.go @@ -114,6 +114,7 @@ func (t *testCtx) initBridge() (<-chan events.Event, error) { } else if corrupt { return nil, fmt.Errorf("vault is corrupt") } + t.vault = vault // Create the underlying cookie jar. jar, err := cookiejar.New(nil) diff --git a/tests/ctx_test.go b/tests/ctx_test.go index 369e865f..5fad8fee 100644 --- a/tests/ctx_test.go +++ b/tests/ctx_test.go @@ -36,6 +36,7 @@ import ( "github.com/ProtonMail/proton-bridge/v3/internal/bridge" frontend "github.com/ProtonMail/proton-bridge/v3/internal/frontend/grpc" "github.com/ProtonMail/proton-bridge/v3/internal/locations" + "github.com/ProtonMail/proton-bridge/v3/internal/vault" "github.com/bradenaw/juniper/xslices" "github.com/cucumber/godog" "github.com/emersion/go-imap/client" @@ -135,6 +136,7 @@ type testCtx struct { // bridge holds the bridge app under test. bridge *bridge.Bridge + vault *vault.Vault // service holds the gRPC frontend service under test. service *frontend.Service diff --git a/tests/features/user/relogin.feature b/tests/features/user/relogin.feature index 021a6696..faf5f5cd 100644 --- a/tests/features/user/relogin.feature +++ b/tests/features/user/relogin.feature @@ -12,4 +12,13 @@ Feature: A logged out user can login again Scenario: Cannot login to removed account When user "[user:user]" is deleted - Then user "[user:user]" is not listed \ No newline at end of file + Then user "[user:user]" is not listed + + Scenario: Bridge password persists after logout/login + Given there exists an account with username "testUser" and password "password" + And the user logs in with username "testUser" and password "password" + And the bridge password of user "testUser" is changed to "YnJpZGdlcGFzc3dvcmQK" + And user "testUser" is deleted + And the user logs in with username "testUser" and password "password" + Then user "testUser" is eventually listed and connected + And the bridge password of user "testUser" is equal to "YnJpZGdlcGFzc3dvcmQK" diff --git a/tests/user_test.go b/tests/user_test.go index 17542987..c06f0bd7 100644 --- a/tests/user_test.go +++ b/tests/user_test.go @@ -28,6 +28,8 @@ import ( "github.com/ProtonMail/go-proton-api" "github.com/ProtonMail/gopenpgp/v2/crypto" "github.com/ProtonMail/proton-bridge/v3/internal/bridge" + "github.com/ProtonMail/proton-bridge/v3/internal/vault" + "github.com/ProtonMail/proton-bridge/v3/pkg/algo" "github.com/bradenaw/juniper/iterator" "github.com/bradenaw/juniper/xslices" "github.com/cucumber/godog" @@ -426,6 +428,37 @@ func (s *scenario) userHasTelemetrySetTo(username string, telemetry int) error { }) } +func (s *scenario) bridgePasswordOfUserIsChangedTo(username, bridgePassword string) error { + b, err := algo.B64RawDecode([]byte(bridgePassword)) + if err != nil { + return errors.New("the password is not base64 encoded") + } + + var setErr error + if err := s.t.vault.GetUser( + s.t.getUserByName(username).getUserID(), + func(user *vault.User) { setErr = user.SetBridgePass(b) }, + ); err != nil { + return err + } + + return setErr +} + +func (s *scenario) bridgePasswordOfUserIsEqualTo(username, bridgePassword string) error { + userInfo, err := s.t.bridge.QueryUserInfo(username) + if err != nil { + return err + } + + readPassword := string(userInfo.BridgePass) + if readPassword != bridgePassword { + return fmt.Errorf("bridge password mismatch, expected '%v', got '%v'", bridgePassword, readPassword) + } + + return nil +} + func (s *scenario) addAdditionalAddressToAccount(username, address string, disabled bool) error { userID := s.t.getUserByName(username).getUserID()