feat: migrate to gopenpgp v2

This commit is contained in:
James Houlahan
2020-06-05 09:33:37 +02:00
parent de16f6f2d1
commit c19bb0fa97
54 changed files with 928 additions and 684 deletions

View File

@ -53,9 +53,6 @@ type User struct {
lock sync.RWMutex
isAuthorized bool
unlockingKeyringLock sync.Mutex
wasKeyringUnlocked bool
}
// newUser creates a new user.
@ -99,10 +96,6 @@ func (u *User) client() pmapi.Client {
// if necessary), and setting the imap idle updates channel (used to send imap idle updates to the imap backend if
// something in the store changed).
func (u *User) init(idleUpdates chan imapBackend.Update) (err error) {
u.unlockingKeyringLock.Lock()
u.wasKeyringUnlocked = false
u.unlockingKeyringLock.Unlock()
u.log.Info("Initialising user")
// Reload the user's credentials (if they log out and back in we need the new
@ -197,22 +190,14 @@ func (u *User) authorizeIfNecessary(emitEvent bool) (err error) {
// unlockIfNecessary will not trigger keyring unlocking if it was already successfully unlocked.
func (u *User) unlockIfNecessary() error {
u.unlockingKeyringLock.Lock()
defer u.unlockingKeyringLock.Unlock()
if u.wasKeyringUnlocked {
if u.client().IsUnlocked() {
return nil
}
if _, err := u.client().Unlock(u.creds.MailboxPassword); err != nil {
if err := u.client().Unlock([]byte(u.creds.MailboxPassword)); err != nil {
return errors.Wrap(err, "failed to unlock user")
}
if err := u.client().UnlockAddresses([]byte(u.creds.MailboxPassword)); err != nil {
return errors.Wrap(err, "failed to unlock user addresses")
}
u.wasKeyringUnlocked = true
return nil
}
@ -228,14 +213,10 @@ func (u *User) authorizeAndUnlock() (err error) {
return errors.Wrap(err, "failed to refresh API auth")
}
if _, err = u.client().Unlock(u.creds.MailboxPassword); err != nil {
if err := u.client().Unlock([]byte(u.creds.MailboxPassword)); err != nil {
return errors.Wrap(err, "failed to unlock user")
}
if err = u.client().UnlockAddresses([]byte(u.creds.MailboxPassword)); err != nil {
return errors.Wrap(err, "failed to unlock user addresses")
}
return nil
}
@ -432,12 +413,8 @@ func (u *User) UpdateUser() error {
return err
}
if _, err = u.client().Unlock(u.creds.MailboxPassword); err != nil {
return err
}
if err := u.client().UnlockAddresses([]byte(u.creds.MailboxPassword)); err != nil {
return err
if err = u.client().Unlock([]byte(u.creds.MailboxPassword)); err != nil {
return errors.Wrap(err, "failed to unlock user")
}
emails := u.client().Addresses().ActiveEmails()
@ -514,10 +491,6 @@ func (u *User) Logout() (err error) {
return
}
u.unlockingKeyringLock.Lock()
u.wasKeyringUnlocked = false
u.unlockingKeyringLock.Unlock()
u.client().Logout()
if err = u.credStorer.Logout(u.userID); err != nil {

View File

@ -35,12 +35,11 @@ func TestUpdateUser(t *testing.T) {
defer cleanUpUserData(user)
gomock.InOrder(
m.pmapiClient.EXPECT().Unlock("pass").Return(nil, nil),
m.pmapiClient.EXPECT().UnlockAddresses([]byte("pass")).Return(nil),
m.pmapiClient.EXPECT().IsUnlocked().Return(false),
m.pmapiClient.EXPECT().Unlock([]byte("pass")).Return(nil),
m.pmapiClient.EXPECT().UpdateUser().Return(nil, nil),
m.pmapiClient.EXPECT().Unlock("pass").Return(nil, nil),
m.pmapiClient.EXPECT().UnlockAddresses([]byte(testCredentials.MailboxPassword)).Return(nil),
m.pmapiClient.EXPECT().Unlock([]byte(testCredentials.MailboxPassword)).Return(nil),
m.pmapiClient.EXPECT().Addresses().Return([]*pmapi.Address{testPMAPIAddress}),
m.credentialsStore.EXPECT().UpdateEmails("user", []string{testPMAPIAddress.Email}),
@ -155,8 +154,8 @@ func TestCheckBridgeLoginOK(t *testing.T) {
defer cleanUpUserData(user)
gomock.InOrder(
m.pmapiClient.EXPECT().Unlock("pass").Return(nil, nil),
m.pmapiClient.EXPECT().UnlockAddresses([]byte("pass")).Return(nil),
m.pmapiClient.EXPECT().IsUnlocked().Return(false),
m.pmapiClient.EXPECT().Unlock([]byte("pass")).Return(nil),
)
err := user.CheckBridgeLogin(testCredentials.BridgePassword)
@ -166,6 +165,28 @@ func TestCheckBridgeLoginOK(t *testing.T) {
assert.NoError(t, err)
}
func TestCheckBridgeLoginTwiceOK(t *testing.T) {
m := initMocks(t)
defer m.ctrl.Finish()
user := testNewUser(m)
defer cleanUpUserData(user)
gomock.InOrder(
m.pmapiClient.EXPECT().IsUnlocked().Return(false),
m.pmapiClient.EXPECT().Unlock([]byte("pass")).Return(nil),
m.pmapiClient.EXPECT().IsUnlocked().Return(true),
)
err := user.CheckBridgeLogin(testCredentials.BridgePassword)
waitForEvents()
assert.NoError(t, err)
err = user.CheckBridgeLogin(testCredentials.BridgePassword)
waitForEvents()
assert.NoError(t, err)
}
func TestCheckBridgeLoginUpgradeApplication(t *testing.T) {
m := initMocks(t)
defer m.ctrl.Finish()
@ -220,8 +241,8 @@ func TestCheckBridgeLoginBadPassword(t *testing.T) {
defer cleanUpUserData(user)
gomock.InOrder(
m.pmapiClient.EXPECT().Unlock("pass").Return(nil, nil),
m.pmapiClient.EXPECT().UnlockAddresses([]byte("pass")).Return(nil),
m.pmapiClient.EXPECT().IsUnlocked().Return(false),
m.pmapiClient.EXPECT().Unlock([]byte("pass")).Return(nil),
)
err := user.CheckBridgeLogin("wrong!")

View File

@ -114,33 +114,7 @@ func TestNewUserUnlockFails(t *testing.T) {
m.credentialsStore.EXPECT().Get("user").Return(testCredentials, nil),
m.pmapiClient.EXPECT().AuthRefresh("token").Return(testAuthRefresh, nil),
m.pmapiClient.EXPECT().Unlock("pass").Return(nil, errors.New("bad password")),
m.credentialsStore.EXPECT().Logout("user").Return(nil),
m.pmapiClient.EXPECT().Logout(),
m.credentialsStore.EXPECT().Logout("user").Return(nil),
m.credentialsStore.EXPECT().Get("user").Return(testCredentialsDisconnected, nil),
)
checkNewUserHasCredentials(testCredentialsDisconnected, m)
}
func TestNewUserUnlockAddressesFails(t *testing.T) {
m := initMocks(t)
defer m.ctrl.Finish()
m.clientManager.EXPECT().GetClient("user").Return(m.pmapiClient).MinTimes(1)
m.eventListener.EXPECT().Emit(events.LogoutEvent, "user")
m.eventListener.EXPECT().Emit(events.UserRefreshEvent, "user")
m.eventListener.EXPECT().Emit(events.CloseConnectionEvent, "user@pm.me")
gomock.InOrder(
m.credentialsStore.EXPECT().Get("user").Return(testCredentials, nil),
m.credentialsStore.EXPECT().Get("user").Return(testCredentials, nil),
m.pmapiClient.EXPECT().AuthRefresh("token").Return(testAuthRefresh, nil),
m.pmapiClient.EXPECT().Unlock("pass").Return(nil, nil),
m.pmapiClient.EXPECT().UnlockAddresses([]byte("pass")).Return(errors.New("bad password")),
m.pmapiClient.EXPECT().Unlock([]byte("pass")).Return(errors.New("bad password")),
m.credentialsStore.EXPECT().Logout("user").Return(nil),
m.pmapiClient.EXPECT().Logout(),
m.credentialsStore.EXPECT().Logout("user").Return(nil),

View File

@ -182,17 +182,8 @@ func (u *Users) closeAllConnections() {
}
}
// Login authenticates a user.
// The login flow:
// * Authenticate user:
// client, auth, err := users.Authenticate(username, password)
//
// * In case user `auth.HasTwoFactor()`, ask for it and fully authenticate the user.
// auth2FA, err := client.Auth2FA(twoFactorCode)
//
// * In case user `auth.HasMailboxPassword()`, ask for it, otherwise use `password`
// and then finish the login procedure.
// user, err := users.FinishLogin(client, auth, mailboxPassword)
// Login authenticates a user by username/password, returning an authorised client and an auth object.
// The authorisation scope may not yet be full if the user has 2FA enabled.
func (u *Users) Login(username, password string) (authClient pmapi.Client, auth *pmapi.Auth, err error) {
u.crashBandicoot(username)
@ -214,7 +205,6 @@ func (u *Users) Login(username, password string) (authClient pmapi.Client, auth
}
// FinishLogin finishes the login procedure and adds the user into the credentials store.
// See `Login` for more details of the login flow.
func (u *Users) FinishLogin(authClient pmapi.Client, auth *pmapi.Auth, mbPassword string) (user *User, err error) { //nolint[funlen]
defer func() {
if err == pmapi.ErrUpgradeApplication {
@ -230,20 +220,22 @@ func (u *Users) FinishLogin(authClient pmapi.Client, auth *pmapi.Auth, mbPasswor
authClient.Logout()
}()
apiUser, hashedPassword, err := getAPIUser(authClient, auth, mbPassword)
apiUser, passphrase, err := getAPIUser(authClient, mbPassword)
if err != nil {
log.WithError(err).Error("Failed to get API user")
return
}
log.Info("Got API user")
var ok bool
if user, ok = u.hasUser(apiUser.ID); ok {
if err = u.connectExistingUser(user, auth, hashedPassword); err != nil {
if err = u.connectExistingUser(user, auth, passphrase); err != nil {
log.WithError(err).Error("Failed to connect existing user")
return
}
} else {
if err = u.addNewUser(apiUser, auth, hashedPassword); err != nil {
if err = u.addNewUser(apiUser, auth, passphrase); err != nil {
log.WithError(err).Error("Failed to add new user")
return
}
@ -255,13 +247,15 @@ func (u *Users) FinishLogin(authClient pmapi.Client, auth *pmapi.Auth, mbPasswor
}
// connectExistingUser connects an existing user.
func (u *Users) connectExistingUser(user *User, auth *pmapi.Auth, hashedPassword string) (err error) {
func (u *Users) connectExistingUser(user *User, auth *pmapi.Auth, passphrase string) (err error) {
if user.IsConnected() {
return errors.New("user is already connected")
}
log.Info("Connecting existing user")
// Update the user's password in the cred store in case they changed it.
if err = u.credStorer.UpdatePassword(user.ID(), hashedPassword); err != nil {
if err = u.credStorer.UpdatePassword(user.ID(), passphrase); err != nil {
return errors.Wrap(err, "failed to update password of user in credentials store")
}
@ -283,7 +277,7 @@ func (u *Users) connectExistingUser(user *User, auth *pmapi.Auth, hashedPassword
}
// addNewUser adds a new user.
func (u *Users) addNewUser(apiUser *pmapi.User, auth *pmapi.Auth, hashedPassword string) (err error) {
func (u *Users) addNewUser(apiUser *pmapi.User, auth *pmapi.Auth, passphrase string) (err error) {
u.lock.Lock()
defer u.lock.Unlock()
@ -299,7 +293,7 @@ func (u *Users) addNewUser(apiUser *pmapi.User, auth *pmapi.Auth, hashedPassword
activeEmails := client.Addresses().ActiveEmails()
if _, err = u.credStorer.Add(apiUser.ID, apiUser.Name, auth.GenToken(), hashedPassword, activeEmails); err != nil {
if _, err = u.credStorer.Add(apiUser.ID, apiUser.Name, auth.GenToken(), passphrase, activeEmails); err != nil {
return errors.Wrap(err, "failed to add user to credentials store")
}
@ -321,21 +315,27 @@ func (u *Users) addNewUser(apiUser *pmapi.User, auth *pmapi.Auth, hashedPassword
return err
}
func getAPIUser(client pmapi.Client, auth *pmapi.Auth, mbPassword string) (user *pmapi.User, hashedPassword string, err error) {
hashedPassword, err = pmapi.HashMailboxPassword(mbPassword, auth.KeySalt)
func getAPIUser(client pmapi.Client, mbPassword string) (user *pmapi.User, passphrase string, err error) {
salt, err := client.AuthSalt()
if err != nil {
log.WithError(err).Error("Could not get salt")
return
}
passphrase, err = pmapi.HashMailboxPassword(mbPassword, salt)
if err != nil {
log.WithError(err).Error("Could not hash mailbox password")
return
}
// We unlock the user's PGP key here to detect if the user's mailbox password is wrong.
if _, err = client.Unlock(hashedPassword); err != nil {
if err = client.Unlock([]byte(passphrase)); err != nil {
log.WithError(err).Error("Wrong mailbox password")
return
}
if user, err = client.CurrentUser(); err != nil {
log.WithError(err).Error("Could not load API user")
log.WithError(err).Error("Could not load user data")
return
}

View File

@ -39,7 +39,8 @@ func TestUsersFinishLoginBadMailboxPassword(t *testing.T) {
m.credentialsStore.EXPECT().List().Return([]string{}, nil),
// Set up mocks for FinishLogin.
m.pmapiClient.EXPECT().Unlock(testCredentials.MailboxPassword).Return(nil, err),
m.pmapiClient.EXPECT().AuthSalt().Return("", nil),
m.pmapiClient.EXPECT().Unlock([]byte(testCredentials.MailboxPassword)).Return(err),
m.pmapiClient.EXPECT().DeleteAuth(),
m.pmapiClient.EXPECT().Logout(),
)
@ -57,7 +58,8 @@ func TestUsersFinishLoginUpgradeApplication(t *testing.T) {
m.credentialsStore.EXPECT().List().Return([]string{}, nil),
// Set up mocks for FinishLogin.
m.pmapiClient.EXPECT().Unlock(testCredentials.MailboxPassword).Return(nil, pmapi.ErrUpgradeApplication),
m.pmapiClient.EXPECT().AuthSalt().Return("", nil),
m.pmapiClient.EXPECT().Unlock([]byte(testCredentials.MailboxPassword)).Return(pmapi.ErrUpgradeApplication),
m.eventListener.EXPECT().Emit(events.UpgradeApplicationEvent, ""),
m.pmapiClient.EXPECT().DeleteAuth().Return(err),
@ -70,7 +72,6 @@ func TestUsersFinishLoginUpgradeApplication(t *testing.T) {
func refreshWithToken(token string) *pmapi.Auth {
return &pmapi.Auth{
RefreshToken: token,
KeySalt: "", // No salting in tests.
}
}
@ -93,7 +94,8 @@ func TestUsersFinishLoginNewUser(t *testing.T) {
m.credentialsStore.EXPECT().List().Return([]string{}, nil),
// getAPIUser() loads user info from API (e.g. userID).
m.pmapiClient.EXPECT().Unlock(testCredentials.MailboxPassword).Return(nil, nil),
m.pmapiClient.EXPECT().AuthSalt().Return("", nil),
m.pmapiClient.EXPECT().Unlock([]byte(testCredentials.MailboxPassword)).Return(nil),
m.pmapiClient.EXPECT().CurrentUser().Return(testPMAPIUser, nil),
// addNewUser()
@ -106,8 +108,7 @@ func TestUsersFinishLoginNewUser(t *testing.T) {
// user.init() in addNewUser
m.credentialsStore.EXPECT().Get("user").Return(credentialsWithToken(":afterLogin"), nil),
m.pmapiClient.EXPECT().AuthRefresh(":afterLogin").Return(refreshWithToken("afterCredentials"), nil),
m.pmapiClient.EXPECT().Unlock(testCredentials.MailboxPassword).Return(nil, nil),
m.pmapiClient.EXPECT().UnlockAddresses([]byte(testCredentials.MailboxPassword)).Return(nil),
m.pmapiClient.EXPECT().Unlock([]byte(testCredentials.MailboxPassword)).Return(nil),
// store.New() in user.init
m.pmapiClient.EXPECT().ListLabels().Return([]*pmapi.Label{}, nil),
@ -158,7 +159,8 @@ func TestUsersFinishLoginExistingDisconnectedUser(t *testing.T) {
m.pmapiClient.EXPECT().Addresses().Return(nil),
// getAPIUser() loads user info from API (e.g. userID).
m.pmapiClient.EXPECT().Unlock(testCredentials.MailboxPassword).Return(nil, nil),
m.pmapiClient.EXPECT().AuthSalt().Return("", nil),
m.pmapiClient.EXPECT().Unlock([]byte(testCredentials.MailboxPassword)).Return(nil),
m.pmapiClient.EXPECT().CurrentUser().Return(testPMAPIUser, nil),
// connectExistingUser()
@ -169,8 +171,7 @@ func TestUsersFinishLoginExistingDisconnectedUser(t *testing.T) {
// user.init() in connectExistingUser
m.credentialsStore.EXPECT().Get("user").Return(credentialsWithToken(":afterLogin"), nil),
m.pmapiClient.EXPECT().AuthRefresh(":afterLogin").Return(refreshWithToken("afterCredentials"), nil),
m.pmapiClient.EXPECT().Unlock(testCredentials.MailboxPassword).Return(nil, nil),
m.pmapiClient.EXPECT().UnlockAddresses([]byte(testCredentials.MailboxPassword)).Return(nil),
m.pmapiClient.EXPECT().Unlock([]byte(testCredentials.MailboxPassword)).Return(nil),
// store.New() in user.init
m.pmapiClient.EXPECT().ListLabels().Return([]*pmapi.Label{}, nil),
@ -206,7 +207,8 @@ func TestUsersFinishLoginConnectedUser(t *testing.T) {
// Then, try to log in again...
gomock.InOrder(
m.pmapiClient.EXPECT().Unlock(testCredentials.MailboxPassword).Return(nil, nil),
m.pmapiClient.EXPECT().AuthSalt().Return("", nil),
m.pmapiClient.EXPECT().Unlock([]byte(testCredentials.MailboxPassword)).Return(nil),
m.pmapiClient.EXPECT().CurrentUser().Return(testPMAPIUser, nil),
m.pmapiClient.EXPECT().DeleteAuth(),
m.pmapiClient.EXPECT().Logout(),

View File

@ -93,8 +93,7 @@ func mockConnectedUser(m mocks) {
m.credentialsStore.EXPECT().Get("user").Return(testCredentials, nil),
m.pmapiClient.EXPECT().AuthRefresh("token").Return(testAuthRefresh, nil),
m.pmapiClient.EXPECT().Unlock(testCredentials.MailboxPassword).Return(nil, nil),
m.pmapiClient.EXPECT().UnlockAddresses([]byte(testCredentials.MailboxPassword)).Return(nil),
m.pmapiClient.EXPECT().Unlock([]byte(testCredentials.MailboxPassword)).Return(nil),
// Set up mocks for store initialisation for the authorized user.
m.pmapiClient.EXPECT().ListLabels().Return([]*pmapi.Label{}, nil),

View File

@ -49,11 +49,9 @@ func TestMain(m *testing.M) {
var (
testAuth = &pmapi.Auth{ //nolint[gochecknoglobals]
RefreshToken: "tok",
KeySalt: "", // No salting in tests.
}
testAuthRefresh = &pmapi.Auth{ //nolint[gochecknoglobals]
RefreshToken: "reftok",
KeySalt: "", // No salting in tests.
}
testCredentials = &credentials.Credentials{ //nolint[gochecknoglobals]
@ -209,8 +207,7 @@ func testNewUsersWithUsers(t *testing.T, m mocks) *Users {
m.credentialsStore.EXPECT().Get("user").Return(testCredentials, nil),
m.credentialsStore.EXPECT().Get("user").Return(testCredentials, nil),
m.pmapiClient.EXPECT().AuthRefresh("token").Return(testAuthRefresh, nil),
m.pmapiClient.EXPECT().Unlock("pass").Return(nil, nil),
m.pmapiClient.EXPECT().UnlockAddresses([]byte("pass")).Return(nil),
m.pmapiClient.EXPECT().Unlock([]byte("pass")).Return(nil),
m.pmapiClient.EXPECT().ListLabels().Return([]*pmapi.Label{}, nil),
m.pmapiClient.EXPECT().CountMessages("").Return([]*pmapi.MessagesCount{}, nil),
m.pmapiClient.EXPECT().Addresses().Return([]*pmapi.Address{testPMAPIAddress}),
@ -219,8 +216,7 @@ func testNewUsersWithUsers(t *testing.T, m mocks) *Users {
m.credentialsStore.EXPECT().Get("users").Return(testCredentialsSplit, nil),
m.credentialsStore.EXPECT().Get("users").Return(testCredentialsSplit, nil),
m.pmapiClient.EXPECT().AuthRefresh("token").Return(testAuthRefresh, nil),
m.pmapiClient.EXPECT().Unlock("pass").Return(nil, nil),
m.pmapiClient.EXPECT().UnlockAddresses([]byte("pass")).Return(nil),
m.pmapiClient.EXPECT().Unlock([]byte("pass")).Return(nil),
m.pmapiClient.EXPECT().ListLabels().Return([]*pmapi.Label{}, nil),
m.pmapiClient.EXPECT().CountMessages("").Return([]*pmapi.MessagesCount{}, nil),
m.pmapiClient.EXPECT().Addresses().Return(testPMAPIAddresses),