GODT-1986: Handle case where an address has no decryption entities

It's possible (but very rare, I don't think proton still allows it) for an address to have no keys. If we try to load the address keyring for such an address, this change logs a warning that no decryption entities were found in the unlocked keyring. It bumps liteapi to a version that does not return an error when no keys could be unlocked.
2025-12-10 04:36:43 +00:00 · 2022-10-27 05:12:13 +02:00
parent 7f6094750e
commit d066e32719
9 changed files with 97 additions and 8 deletions
--- a/go.mod
+++ b/go.mod
@ -39,7 +39,7 @@ require (
 	github.com/stretchr/testify v1.8.0
 	github.com/urfave/cli/v2 v2.20.3
 	github.com/vmihailenco/msgpack/v5 v5.3.5
-	gitlab.protontech.ch/go/liteapi v0.36.1
+	gitlab.protontech.ch/go/liteapi v0.36.2
 	go.uber.org/goleak v1.2.0
 	golang.org/x/exp v0.0.0-20221023144134-a1e5550cf13e
 	golang.org/x/net v0.1.0
--- a/go.sum
+++ b/go.sum
@ -28,8 +28,6 @@ github.com/ProtonMail/bcrypt v0.0.0-20211005172633-e235017c1baf h1:yc9daCCYUefEs
 github.com/ProtonMail/bcrypt v0.0.0-20211005172633-e235017c1baf/go.mod h1:o0ESU9p83twszAU8LBeJKFAAMX14tISa0yk4Oo5TOqo=
 github.com/ProtonMail/docker-credential-helpers v1.1.0 h1:+kvUIpwWcbtP3WFv5sSvkFn/XLzSqPOB5AAthuk9xPk=
 github.com/ProtonMail/docker-credential-helpers v1.1.0/go.mod h1:mK0aBveCxhnQ756AmaTfXMZDeULvheYVhF/MWMErN5g=
-github.com/ProtonMail/gluon v0.13.1-0.20221025093924-86bbf0261eb8 h1:LKyiQdEsAxAocSYUWxSfwlxBwmzJYvO/9td/eAX3oFU=
-github.com/ProtonMail/gluon v0.13.1-0.20221025093924-86bbf0261eb8/go.mod h1:XW/gcr4jErc5bX5yMqkUq3U+AucC2QZHJ5L231k3Nw4=
 github.com/ProtonMail/gluon v0.13.1-0.20221026080908-3f1806709bdd h1:2R9kbvUVmGSHK4b0kGLyxruX9Ea0EO23km2xwYheWrY=
 github.com/ProtonMail/gluon v0.13.1-0.20221026080908-3f1806709bdd/go.mod h1:XW/gcr4jErc5bX5yMqkUq3U+AucC2QZHJ5L231k3Nw4=
 github.com/ProtonMail/go-autostart v0.0.0-20210130080809-00ed301c8e9a h1:D+aZah+k14Gn6kmL7eKxoo/4Dr/lK3ChBcwce2+SQP4=
@ -405,8 +403,8 @@ github.com/xrash/smetrics v0.0.0-20201216005158-039620a65673/go.mod h1:N3UwUGtsr
 github.com/yuin/goldmark v1.3.5/go.mod h1:mwnBkeHKe2W/ZEtQ+71ViKU8L12m81fl3OWwC1Zlc8k=
 github.com/zclconf/go-cty v1.11.0 h1:726SxLdi2SDnjY+BStqB9J1hNp4+2WlzyXLuimibIe0=
 github.com/zclconf/go-cty v1.11.0/go.mod h1:s9IfD1LK5ccNMSWCVFCE2rJfHiZgi7JijgeWIMfhLvA=
-gitlab.protontech.ch/go/liteapi v0.36.1 h1:OW+8hcfU95SBzT/3nRtQaNTRjaJyBb12ebz/WfFu5Lo=
-gitlab.protontech.ch/go/liteapi v0.36.1/go.mod h1:IM7ADWjgIL2hXopzx0WNamizEuMgM2QZl7QH12FNflk=
+gitlab.protontech.ch/go/liteapi v0.36.2 h1:Vu7zKIwJNQ46X7ou0n8wXDS9uoRTAhyyVnuH5tM2vVY=
+gitlab.protontech.ch/go/liteapi v0.36.2/go.mod h1:IM7ADWjgIL2hXopzx0WNamizEuMgM2QZl7QH12FNflk=
 go.etcd.io/bbolt v1.3.2/go.mod h1:IbVyRI1SCnLcuJnV2u8VeU0CEYM7e686BmAb1XKL+uU=
 go.opencensus.io v0.21.0/go.mod h1:mSImk1erAIZhrmZN+AvHh14ztQfjbGwt4TtuofqLduU=
 go.opencensus.io v0.22.0/go.mod h1:+kGneAE2xo2IficOXnaByMWTGM9T73dGwxeWcUqIpI8=
--- a/internal/bridge/bridge_test.go
+++ b/internal/bridge/bridge_test.go
@ -374,6 +374,39 @@ func TestBridge_MissingGluonDir(t *testing.T) {
 	})
 }

+func TestBridge_AddressWithoutKeys(t *testing.T) {
+	withEnv(t, func(ctx context.Context, s *server.Server, netCtl *liteapi.NetCtl, locator bridge.Locator, vaultKey []byte) {
+		withBridge(ctx, t, s.GetHostURL(), netCtl, locator, vaultKey, func(bridge *bridge.Bridge, mocks *bridge.Mocks) {
+			// Create a user which will have an address without keys.
+			userID, _, err := s.CreateUser("nokeys", "nokeys@pm.me", []byte("password"))
+			require.NoError(t, err)
+
+			// Create an additional address for the user; it will not have keys.
+			aliasAddrID, err := s.CreateAddress(userID, "alias@pm.me", []byte("password"))
+			require.NoError(t, err)
+
+			// Get the address key IDs.
+			aliasKeyIDs, err := s.GetAddressKeyIDs(userID, aliasAddrID)
+			require.NoError(t, err)
+			require.Len(t, aliasKeyIDs, 1)
+
+			// Remove the address keys.
+			require.NoError(t, s.RemoveAddressKey(userID, aliasAddrID, aliasKeyIDs[0]))
+
+			// Watch for sync finished event.
+			syncCh, done := chToType[events.Event, events.SyncFinished](bridge.GetEvents(events.SyncFinished{}))
+			defer done()
+
+			// We should be able to log the user in.
+			require.NoError(t, getErr(bridge.LoginFull(context.Background(), "nokeys", []byte("password"), nil, nil)))
+			require.NoError(t, err)
+
+			// The sync should eventually finish for the user without keys.
+			require.Equal(t, userID, (<-syncCh).UserID)
+		})
+	})
+}
+
 // withEnv creates the full test environment and runs the tests.
 func withEnv(t *testing.T, tests func(context.Context, *server.Server, *liteapi.NetCtl, bridge.Locator, []byte), opts ...server.Option) {
 	server := server.New(opts...)
--- a/internal/user/keys.go
+++ b/internal/user/keys.go
@ -21,6 +21,7 @@ import (
 	"fmt"

 	"github.com/ProtonMail/gopenpgp/v2/crypto"
+	"github.com/sirupsen/logrus"
 	"gitlab.protontech.ch/go/liteapi"
 )

@ -56,6 +57,10 @@ func withAddrKRs(apiUser liteapi.User, apiAddr map[string]liteapi.Address, keyPa
 		}
 		defer addrKR.ClearPrivateParams()

+		if addrKR.CountDecryptionEntities() == 0 {
+			logrus.WithField("addressID", addrID).Warn("Address keyring has no decryption entities")
+		}
+
 		addrKRs[addrID] = addrKR
 	}

--- a/tests/api_test.go
+++ b/tests/api_test.go
@ -33,6 +33,8 @@ type API interface {

 	CreateAddress(userID, address string, password []byte) (string, error)
 	RemoveAddress(userID, addrID string) error
+	GetAddressKeyIDs(userID, addrID string) ([]string, error)
+	RemoveAddressKey(userID, addrID, keyID string) error

 	Close()
 }
--- a/tests/bdd_test.go
+++ b/tests/bdd_test.go
@ -97,6 +97,7 @@ func TestFeatures(testingT *testing.T) {
 			ctx.Step(`^the account "([^"]*)" has the following custom mailboxes:$`, s.theAccountHasTheFollowingCustomMailboxes)
 			ctx.Step(`^the address "([^"]*)" of account "([^"]*)" has the following messages in "([^"]*)":$`, s.theAddressOfAccountHasTheFollowingMessagesInMailbox)
 			ctx.Step(`^the address "([^"]*)" of account "([^"]*)" has (\d+) messages in "([^"]*)"$`, s.theAddressOfAccountHasMessagesInMailbox)
+			ctx.Step(`^the address "([^"]*)" of account "([^"]*)" has no keys$`, s.theAddressOfAccountHasNoKeys)

 			// ==== BRIDGE ====
 			ctx.Step(`^bridge starts$`, s.bridgeStarts)
--- a/tests/features/user/sync.feature
+++ b/tests/features/user/sync.feature
@ -58,4 +58,22 @@ Feature: Bridge can fully sync an account
      | Folders/one  | 2     | 1      |
      | Folders/two  | 2     | 1      |
      | Labels       | 0     | 0      |
-      | Labels/three | 0     | 0      |
+      | Labels/three | 0     | 0      |
+
+  Scenario: If an address has no keys, the account is still synced
+    Given the account "user@pm.me" has additional address "alias@pm.me"
+    And the account "user@pm.me" has the following custom mailboxes:
+      | name      | type   |
+      | encrypted | folder |
+    And the address "alias@pm.me" of account "user@pm.me" has the following messages in "encrypted":
+      | from    | to      | subject |
+      | a@pm.me | a@pm.me | no key  |
+      | b@pm.me | b@pm.me | no key  |
+    And the address "alias@pm.me" of account "user@pm.me" has no keys
+    When the user logs in with username "user@pm.me" and password "password"
+    And user "user@pm.me" finishes syncing
+    When user "user@pm.me" connects and authenticates IMAP client "1"
+    Then IMAP client "1" eventually sees the following messages in "Folders/encrypted":
+      | from    | to      | subject | mime-type           |
+      | a@pm.me | a@pm.me | no key  | multipart/encrypted |
+      | b@pm.me | b@pm.me | no key  | multipart/encrypted |
--- a/tests/types_test.go
+++ b/tests/types_test.go
@ -18,7 +18,9 @@
 package tests

 import (
+	"bytes"
 	"fmt"
+	"io"
 	"reflect"
 	"strconv"
 	"strings"
@ -35,6 +37,7 @@ import (
 type Message struct {
 	Subject     string `bdd:"subject"`
 	Body        string `bdd:"body"`
+	MIMEType    string `bdd:"mime-type"`
 	Attachments string `bdd:"attachments"`
 	MessageID   string `bdd:"message-id"`

@ -80,7 +83,17 @@ func newMessageFromIMAP(msg *imap.Message) Message {
 		panic(err)
 	}

-	m, err := message.Parse(msg.GetBody(section))
+	literal, err := io.ReadAll(msg.GetBody(section))
+	if err != nil {
+		panic(err)
+	}
+
+	mimeType, _, err := rfc822.Parse(literal).ContentType()
+	if err != nil {
+		panic(err)
+	}
+
+	m, err := message.Parse(bytes.NewReader(literal))
 	if err != nil {
 		panic(err)
 	}
@ -96,6 +109,7 @@ func newMessageFromIMAP(msg *imap.Message) Message {
 	message := Message{
 		Subject:     msg.Envelope.Subject,
 		Body:        body,
+		MIMEType:    string(mimeType),
 		Attachments: strings.Join(xslices.Map(m.Attachments, func(att message.Attachment) string { return att.Name }), ", "),
 		MessageID:   msg.Envelope.MessageId,
 		Unread:      !slices.Contains(msg.Flags, imap.SeenFlag),
@ -169,7 +183,7 @@ func matchMailboxes(have, want []Mailbox) error {
 func eventually(condition func() error) error {
 	ch := make(chan error, 1)

-	timer := time.NewTimer(10 * time.Second)
+	timer := time.NewTimer(30 * time.Second)
 	defer timer.Stop()

 	ticker := time.NewTicker(100 * time.Millisecond)
--- a/tests/user_test.go
+++ b/tests/user_test.go
@ -200,6 +200,24 @@ func (s *scenario) theAddressOfAccountHasMessagesInMailbox(address, username str
 	})))
 }

+func (s *scenario) theAddressOfAccountHasNoKeys(address, username string) error {
+	userID := s.t.getUserID(username)
+	addrID := s.t.getUserAddrID(userID, address)
+
+	keyIDs, err := s.t.api.GetAddressKeyIDs(userID, addrID)
+	if err != nil {
+		return err
+	}
+
+	for _, keyID := range keyIDs {
+		if err := s.t.api.RemoveAddressKey(userID, addrID, keyID); err != nil {
+			return err
+		}
+	}
+
+	return nil
+}
+
 func (s *scenario) userLogsInWithUsernameAndPassword(username, password string) error {
 	userID, err := s.t.bridge.LoginFull(context.Background(), username, []byte(password), nil, nil)
 	if err != nil {