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

fix: review comments

Browse Source
This commit is contained in:
James Houlahan
2020-06-11 12:16:26 +02:00
parent c19bb0fa97
commit 5cb78b0a03
3 changed files with 16 additions and 17 deletions
Download Patch File Download Diff File Expand all files Collapse all files

9
internal/smtp/repro_test.go
View File

@ -21,11 +21,10 @@ import (
"testing" "testing"
"github.com/ProtonMail/gopenpgp/v2/crypto" "github.com/ProtonMail/gopenpgp/v2/crypto"
"github.com/google/go-cmp/cmp"
"github.com/stretchr/testify/assert" "github.com/stretchr/testify/assert"
) )
func TestThing(t *testing.T) { func TestKeyRingsAreEqualAfterFiltering(t *testing.T) {
// Load the key. // Load the key.
key, err := crypto.NewKeyFromArmored(testPublicKey) key, err := crypto.NewKeyFromArmored(testPublicKey)
if err != nil { if err != nil {
@ -45,10 +44,10 @@ func TestThing(t *testing.T) {
} }
// Filtering shouldn't make them unequal. // Filtering shouldn't make them unequal.
assert.True(t, isEqual(keyRing, validKeyRings[0])) assert.True(t, isEqual(t, keyRing, validKeyRings[0]))
} }
func isEqual(a, b *crypto.KeyRing) bool { func isEqual(t *testing.T, a, b *crypto.KeyRing) bool {
if a == nil && b == nil { if a == nil && b == nil {
return true return true
} }
@ -67,7 +66,7 @@ func isEqual(a, b *crypto.KeyRing) bool {
aFPs := aKeys[i].GetSHA256Fingerprints() aFPs := aKeys[i].GetSHA256Fingerprints()
bFPs := bKeys[i].GetSHA256Fingerprints() bFPs := bKeys[i].GetSHA256Fingerprints()
if !cmp.Equal(aFPs, bFPs) { if !assert.Equal(t, aFPs, bFPs) {
return false return false
} }
} }

22
internal/users/users.go
View File

@ -205,7 +205,7 @@ func (u *Users) Login(username, password string) (authClient pmapi.Client, auth
} }
// FinishLogin finishes the login procedure and adds the user into the credentials store. // FinishLogin finishes the login procedure and adds the user into the credentials store.
func (u *Users) FinishLogin(authClient pmapi.Client, auth *pmapi.Auth, mbPassword string) (user *User, err error) { //nolint[funlen] func (u *Users) FinishLogin(authClient pmapi.Client, auth *pmapi.Auth, mbPassphrase string) (user *User, err error) { //nolint[funlen]
defer func() { defer func() {
if err == pmapi.ErrUpgradeApplication { if err == pmapi.ErrUpgradeApplication {
u.events.Emit(events.UpgradeApplicationEvent, "") u.events.Emit(events.UpgradeApplicationEvent, "")
@ -220,7 +220,7 @@ func (u *Users) FinishLogin(authClient pmapi.Client, auth *pmapi.Auth, mbPasswor
authClient.Logout() authClient.Logout()
}() }()
apiUser, passphrase, err := getAPIUser(authClient, mbPassword) apiUser, hashedPassphrase, err := getAPIUser(authClient, mbPassphrase)
if err != nil { if err != nil {
log.WithError(err).Error("Failed to get API user") log.WithError(err).Error("Failed to get API user")
return return
@ -230,12 +230,12 @@ func (u *Users) FinishLogin(authClient pmapi.Client, auth *pmapi.Auth, mbPasswor
var ok bool var ok bool
if user, ok = u.hasUser(apiUser.ID); ok { if user, ok = u.hasUser(apiUser.ID); ok {
if err = u.connectExistingUser(user, auth, passphrase); err != nil { if err = u.connectExistingUser(user, auth, hashedPassphrase); err != nil {
log.WithError(err).Error("Failed to connect existing user") log.WithError(err).Error("Failed to connect existing user")
return return
} }
} else { } else {
if err = u.addNewUser(apiUser, auth, passphrase); err != nil { if err = u.addNewUser(apiUser, auth, hashedPassphrase); err != nil {
log.WithError(err).Error("Failed to add new user") log.WithError(err).Error("Failed to add new user")
return return
} }
@ -247,7 +247,7 @@ func (u *Users) FinishLogin(authClient pmapi.Client, auth *pmapi.Auth, mbPasswor
} }
// connectExistingUser connects an existing user. // connectExistingUser connects an existing user.
func (u *Users) connectExistingUser(user *User, auth *pmapi.Auth, passphrase string) (err error) { func (u *Users) connectExistingUser(user *User, auth *pmapi.Auth, hashedPassphrase string) (err error) {
if user.IsConnected() { if user.IsConnected() {
return errors.New("user is already connected") return errors.New("user is already connected")
} }
@ -255,7 +255,7 @@ func (u *Users) connectExistingUser(user *User, auth *pmapi.Auth, passphrase str
log.Info("Connecting existing user") log.Info("Connecting existing user")
// Update the user's password in the cred store in case they changed it. // Update the user's password in the cred store in case they changed it.
if err = u.credStorer.UpdatePassword(user.ID(), passphrase); err != nil { if err = u.credStorer.UpdatePassword(user.ID(), hashedPassphrase); err != nil {
return errors.Wrap(err, "failed to update password of user in credentials store") return errors.Wrap(err, "failed to update password of user in credentials store")
} }
@ -277,7 +277,7 @@ func (u *Users) connectExistingUser(user *User, auth *pmapi.Auth, passphrase str
} }
// addNewUser adds a new user. // addNewUser adds a new user.
func (u *Users) addNewUser(apiUser *pmapi.User, auth *pmapi.Auth, passphrase string) (err error) { func (u *Users) addNewUser(apiUser *pmapi.User, auth *pmapi.Auth, hashedPassphrase string) (err error) {
u.lock.Lock() u.lock.Lock()
defer u.lock.Unlock() defer u.lock.Unlock()
@ -293,7 +293,7 @@ func (u *Users) addNewUser(apiUser *pmapi.User, auth *pmapi.Auth, passphrase str
activeEmails := client.Addresses().ActiveEmails() activeEmails := client.Addresses().ActiveEmails()
if _, err = u.credStorer.Add(apiUser.ID, apiUser.Name, auth.GenToken(), passphrase, activeEmails); err != nil { if _, err = u.credStorer.Add(apiUser.ID, apiUser.Name, auth.GenToken(), hashedPassphrase, activeEmails); err != nil {
return errors.Wrap(err, "failed to add user to credentials store") return errors.Wrap(err, "failed to add user to credentials store")
} }
@ -315,21 +315,21 @@ func (u *Users) addNewUser(apiUser *pmapi.User, auth *pmapi.Auth, passphrase str
return err return err
} }
func getAPIUser(client pmapi.Client, mbPassword string) (user *pmapi.User, passphrase string, err error) { func getAPIUser(client pmapi.Client, mbPassphrase string) (user *pmapi.User, hashedPassphrase string, err error) {
salt, err := client.AuthSalt() salt, err := client.AuthSalt()
if err != nil { if err != nil {
log.WithError(err).Error("Could not get salt") log.WithError(err).Error("Could not get salt")
return return
} }
passphrase, err = pmapi.HashMailboxPassword(mbPassword, salt) hashedPassphrase, err = pmapi.HashMailboxPassword(mbPassphrase, salt)
if err != nil { if err != nil {
log.WithError(err).Error("Could not hash mailbox password") log.WithError(err).Error("Could not hash mailbox password")
return return
} }
// We unlock the user's PGP key here to detect if the user's mailbox password is wrong. // We unlock the user's PGP key here to detect if the user's mailbox password is wrong.
if err = client.Unlock([]byte(passphrase)); err != nil { if err = client.Unlock([]byte(hashedPassphrase)); err != nil {
log.WithError(err).Error("Wrong mailbox password") log.WithError(err).Error("Wrong mailbox password")
return return
} }

2
test/features/imap/auth.feature
View File

@ -71,7 +71,7 @@ Feature: IMAP auth
@ignore-live @ignore-live
Scenario: Authenticates with disabled primary address Scenario: Authenticates with disabled primary address
Given there is connected user "userDisabledPrimaryAddress" Given there is connected user "userDisabledPrimaryAddress"
When IMAP client authenticates "userDisabledPrimaryAddress" with address "disabled" When IMAP client authenticates "userDisabledPrimaryAddress" with address "secondary"
Then IMAP response is "OK" Then IMAP response is "OK"
Scenario: Authenticates two users Scenario: Authenticates two users