jmap: do not keep invalid sessions in cache

If a session is found in the cache, check that it works by issuing
a GetIdentities request. If that request fails, invalidate the cache and
go through the authentication.

Ensure that the session is valid and explicitly fail if it is not.

Signed-off-by: Robin Jarry <robin@jarry.cc>
Reviewed-by: Tim Culverhouse <tim@timculverhouse.com>

8 files changed, 59 insertions, 58 deletions

diff --git a/worker/jmap/connect.go b/worker/jmap/connect.go
index affa9474..c5eabe63 100644
--- a/worker/jmap/connect.go
+++ b/worker/jmap/connect.go
@@ -2,9 +2,7 @@ package jmap
 
 import (
 	"encoding/json"
-	"fmt"
 	"io"
-	"net/url"
 	"strings"
 	"sync/atomic"
 
@@ -15,57 +13,61 @@ import (
 )
 
 func (w *JMAPWorker) handleConnect(msg *types.Connect) error {
-	client := &jmap.Client{SessionEndpoint: w.config.endpoint}
+	w.client = &jmap.Client{SessionEndpoint: w.config.endpoint}
 
 	if w.config.oauth {
 		pass, _ := w.config.user.Password()
-		client.WithAccessToken(pass)
+		w.client.WithAccessToken(pass)
 	} else {
 		user := w.config.user.Username()
 		pass, _ := w.config.user.Password()
-		client.WithBasicAuth(user, pass)
+		w.client.WithBasicAuth(user, pass)
 	}
 
-	if session, err := w.cache.GetSession(); err != nil {
-		if err := client.Authenticate(); err != nil {
+	if session, err := w.cache.GetSession(); err == nil {
+		w.client.Session = session
+		if w.GetIdentities() != nil {
+			w.client.Session = nil
+			w.identities = make(map[string]*identity.Identity)
+			if err := w.cache.DeleteSession(); err != nil {
+				w.w.Warnf("DeleteSession: %s", err)
+			}
+		}
+	}
+	if w.client.Session == nil {
+		if err := w.client.Authenticate(); err != nil {
 			return err
 		}
-		if err := w.cache.PutSession(client.Session); err != nil {
+		if err := w.cache.PutSession(w.client.Session); err != nil {
 			w.w.Warnf("PutSession: %s", err)
 		}
-	} else {
-		client.Session = session
 	}
+	if len(w.identities) == 0 {
+		if err := w.GetIdentities(); err != nil {
+			return err
+		}
+	}
+
+	return nil
+}
 
+func (w *JMAPWorker) AccountId() jmap.ID {
 	switch {
-	case client == nil:
+	case w.client == nil:
 		fallthrough
-	case client.Session == nil:
+	case w.client.Session == nil:
 		fallthrough
-	case client.Session.PrimaryAccounts == nil:
-		break
+	case w.client.Session.PrimaryAccounts == nil:
+		return ""
 	default:
-		w.accountId = client.Session.PrimaryAccounts[mail.URI]
+		return w.client.Session.PrimaryAccounts[mail.URI]
 	}
-
-	w.client = client
-
-	return w.GetIdentities()
 }
 
 func (w *JMAPWorker) GetIdentities() error {
-	u, err := url.Parse(w.config.account.Outgoing.Value)
-	if err != nil {
-		return fmt.Errorf("GetIdentities: %w", err)
-	}
-	if !strings.HasPrefix(u.Scheme, "jmap") {
-		// no need for identities
-		return nil
-	}
-
 	var req jmap.Request
 
-	req.Invoke(&identity.Get{Account: w.accountId})
+	req.Invoke(&identity.Get{Account: w.AccountId()})
 	resp, err := w.Do(&req)
 	if err != nil {
 		return err
@@ -102,14 +104,14 @@ func (w *JMAPWorker) Do(req *jmap.Request) (*jmap.Response, error) {
 func (w *JMAPWorker) Download(blobID jmap.ID) (io.ReadCloser, error) {
 	seq := atomic.AddUint64(&seqnum, 1)
 	replacer := strings.NewReplacer(
-		"{accountId}", string(w.accountId),
+		"{accountId}", string(w.AccountId()),
 		"{blobId}", string(blobID),
 		"{type}", "application/octet-stream",
 		"{name}", "filename",
 	)
 	url := replacer.Replace(w.client.Session.DownloadURL)
 	w.w.Debugf(">%d> GET %s", seq, url)
-	rd, err := w.client.Download(w.accountId, blobID)
+	rd, err := w.client.Download(w.AccountId(), blobID)
 	if err == nil {
 		w.w.Debugf("<%d< 200 OK", seq)
 	} else {
@@ -121,9 +123,9 @@ func (w *JMAPWorker) Download(blobID jmap.ID) (io.ReadCloser, error) {
 func (w *JMAPWorker) Upload(reader io.Reader) (*jmap.UploadResponse, error) {
 	seq := atomic.AddUint64(&seqnum, 1)
 	url := strings.ReplaceAll(w.client.Session.UploadURL,
-		"{accountId}", string(w.accountId))
+		"{accountId}", string(w.AccountId()))
 	w.w.Debugf(">%d> POST %s", seq, url)
-	resp, err := w.client.Upload(w.accountId, reader)
+	resp, err := w.client.Upload(w.AccountId(), reader)
 	if err == nil {
 		w.w.Debugf("<%d< 200 OK", seq)
 	} else {
diff --git a/worker/jmap/directories.go b/worker/jmap/directories.go
index 66875cc7..07bb0762 100644
--- a/worker/jmap/directories.go
+++ b/worker/jmap/directories.go
@@ -55,7 +55,7 @@ func (w *JMAPWorker) handleListDirectories(msg *types.ListDirectories) error {
 	if !consistentMailboxState || len(missing) > 0 {
 		var req jmap.Request
 
-		req.Invoke(&mailbox.Get{Account: w.accountId})
+		req.Invoke(&mailbox.Get{Account: w.AccountId()})
 		resp, err := w.Do(&req)
 		if err != nil {
 			return err
@@ -150,7 +150,7 @@ func (w *JMAPWorker) handleFetchDirectoryContents(msg *types.FetchDirectoryConte
 		var req jmap.Request
 
 		req.Invoke(&email.Query{
-			Account: w.accountId,
+			Account: w.AccountId(),
 			Filter:  w.translateSearch(w.selectedMbox, msg.Filter),
 			Sort:    translateSort(msg.SortCriteria),
 		})
@@ -202,7 +202,7 @@ func (w *JMAPWorker) handleSearchDirectory(msg *types.SearchDirectory) error {
 	var req jmap.Request
 
 	req.Invoke(&email.Query{
-		Account: w.accountId,
+		Account: w.AccountId(),
 		Filter:  w.translateSearch(w.selectedMbox, msg.Criteria),
 	})
 
@@ -256,7 +256,7 @@ func (w *JMAPWorker) handleCreateDirectory(msg *types.CreateDirectory) error {
 	id = jmap.ID(msg.Directory)
 
 	req.Invoke(&mailbox.Set{
-		Account: w.accountId,
+		Account: w.AccountId(),
 		Create: map[jmap.ID]*mailbox.Mailbox{
 			id: {
 				ParentID: parentId,
@@ -297,7 +297,7 @@ func (w *JMAPWorker) handleRemoveDirectory(msg *types.RemoveDirectory) error {
 	}
 
 	req.Invoke(&mailbox.Set{
-		Account:               w.accountId,
+		Account:               w.AccountId(),
 		Destroy:               []jmap.ID{id},
 		OnDestroyRemoveEmails: msg.Quiet,
 	})
diff --git a/worker/jmap/fetch.go b/worker/jmap/fetch.go
index 17b3fb2f..5d88e6a1 100644
--- a/worker/jmap/fetch.go
+++ b/worker/jmap/fetch.go
@@ -59,7 +59,7 @@ func (w *JMAPWorker) handleFetchMessageHeaders(msg *types.FetchMessageHeaders) e
 	}
 
 	req.Invoke(&email.Get{
-		Account:    w.accountId,
+		Account:    w.AccountId(),
 		IDs:        ids,
 		Properties: headersProperties,
 	})
@@ -186,7 +186,7 @@ func (w *JMAPWorker) handleFetchFullMessages(msg *types.FetchFullMessages) error
 
 func (w *JMAPWorker) wrapDownloadError(prefix string, blobId jmap.ID, err error) error {
 	urlRepl := strings.NewReplacer(
-		"{accountId}", string(w.accountId),
+		"{accountId}", string(w.AccountId()),
 		"{blobId}", string(blobId),
 		"{type}", "application/octet-stream",
 		"{name}", "filename",
diff --git a/worker/jmap/push.go b/worker/jmap/push.go
index cb3394d1..54e6375e 100644
--- a/worker/jmap/push.go
+++ b/worker/jmap/push.go
@@ -46,7 +46,7 @@ func (w *JMAPWorker) monitorChanges() {
 }
 
 func (w *JMAPWorker) handleChange(s *jmap.StateChange) {
-	changed, ok := s.Changed[w.accountId]
+	changed, ok := s.Changed[w.AccountId()]
 	if !ok {
 		return
 	}
@@ -63,11 +63,11 @@ func (w *JMAPWorker) refresh(newState jmap.TypeState) error {
 	}
 	if mboxState != "" && newState["Mailbox"] != mboxState {
 		callID := req.Invoke(&mailbox.Changes{
-			Account:    w.accountId,
+			Account:    w.AccountId(),
 			SinceState: mboxState,
 		})
 		req.Invoke(&mailbox.Get{
-			Account: w.accountId,
+			Account: w.AccountId(),
 			ReferenceIDs: &jmap.ResultReference{
 				ResultOf: callID,
 				Name:     "Mailbox/changes",
@@ -75,7 +75,7 @@ func (w *JMAPWorker) refresh(newState jmap.TypeState) error {
 			},
 		})
 		req.Invoke(&mailbox.Get{
-			Account: w.accountId,
+			Account: w.AccountId(),
 			ReferenceIDs: &jmap.ResultReference{
 				ResultOf: callID,
 				Name:     "Mailbox/changes",
@@ -109,11 +109,11 @@ func (w *JMAPWorker) refresh(newState jmap.TypeState) error {
 	}
 	if emailState != "" && newState["Email"] != emailState {
 		callID := req.Invoke(&email.Changes{
-			Account:    w.accountId,
+			Account:    w.AccountId(),
 			SinceState: emailState,
 		})
 		req.Invoke(&email.Get{
-			Account:    w.accountId,
+			Account:    w.AccountId(),
 			Properties: headersProperties,
 			ReferenceIDs: &jmap.ResultReference{
 				ResultOf: callID,
@@ -128,7 +128,7 @@ func (w *JMAPWorker) refresh(newState jmap.TypeState) error {
 				continue
 			}
 			callID = req.Invoke(&email.QueryChanges{
-				Account:         w.accountId,
+				Account:         w.AccountId(),
 				Filter:          w.translateSearch(id, contents.Filter),
 				Sort:            translateSort(contents.Sort),
 				SinceQueryState: contents.QueryState,
diff --git a/worker/jmap/send.go b/worker/jmap/send.go
index 7f5ffc4f..0791419c 100644
--- a/worker/jmap/send.go
+++ b/worker/jmap/send.go
@@ -43,7 +43,7 @@ func (w *JMAPWorker) handleStartSend(msg *types.StartSendingMessage) error {
 
 		// Import the blob into drafts
 		req.Invoke(&email.Import{
-			Account: w.accountId,
+			Account: w.AccountId(),
 			Emails: map[string]*email.EmailImport{
 				"aerc": {
 					BlobID: blob.ID,
@@ -68,7 +68,7 @@ func (w *JMAPWorker) handleStartSend(msg *types.StartSendingMessage) error {
 		envelope := &emailsubmission.Envelope{MailFrom: from, RcptTo: rcpts}
 		// Create the submission
 		req.Invoke(&emailsubmission.Set{
-			Account: w.accountId,
+			Account: w.AccountId(),
 			Create: map[jmap.ID]*emailsubmission.EmailSubmission{
 				"sub": {
 					IdentityID: identity,
diff --git a/worker/jmap/set.go b/worker/jmap/set.go
index 4495e428..36b23688 100644
--- a/worker/jmap/set.go
+++ b/worker/jmap/set.go
@@ -32,7 +32,7 @@ func (w *JMAPWorker) updateFlags(uids []uint32, flags models.Flags, enable bool)
 	}
 
 	req.Invoke(&email.Set{
-		Account: w.accountId,
+		Account: w.AccountId(),
 		Update:  patches,
 	})
 
@@ -81,7 +81,7 @@ func (w *JMAPWorker) moveCopy(uids []uint32, destDir string, deleteSrc bool) err
 	}
 
 	req.Invoke(&email.Set{
-		Account: w.accountId,
+		Account: w.AccountId(),
 		Update:  patches,
 		Destroy: destroy,
 	})
@@ -169,7 +169,7 @@ func (w *JMAPWorker) handleModifyLabels(msg *types.ModifyLabels) error {
 	}
 
 	req.Invoke(&email.Set{
-		Account: w.accountId,
+		Account: w.AccountId(),
 		Update:  patches,
 	})
 
@@ -211,7 +211,7 @@ func (w *JMAPWorker) handleAppendMessage(msg *types.AppendMessage) error {
 
 	// Import the blob into specified directory
 	req.Invoke(&email.Import{
-		Account: w.accountId,
+		Account: w.AccountId(),
 		Emails: map[string]*email.EmailImport{
 			"aerc": {
 				BlobID:     blob.ID,
diff --git a/worker/jmap/state.go b/worker/jmap/state.go
index 3dbab3fb..b86ec6ce 100644
--- a/worker/jmap/state.go
+++ b/worker/jmap/state.go
@@ -8,7 +8,7 @@ import (
 func (w *JMAPWorker) getMailboxState() (string, error) {
 	var req jmap.Request
 
-	req.Invoke(&mailbox.Get{Account: w.accountId, IDs: make([]jmap.ID, 0)})
+	req.Invoke(&mailbox.Get{Account: w.AccountId(), IDs: make([]jmap.ID, 0)})
 	resp, err := w.Do(&req)
 	if err != nil {
 		return "", err
diff --git a/worker/jmap/worker.go b/worker/jmap/worker.go
index 0b1c57ae..67553272 100644
--- a/worker/jmap/worker.go
+++ b/worker/jmap/worker.go
@@ -38,10 +38,9 @@ type JMAPWorker struct {
 		allMail    string
 	}
 
-	w         *types.Worker
-	client    *jmap.Client
-	cache     *cache.JMAPCache
-	accountId jmap.ID
+	w      *types.Worker
+	client *jmap.Client
+	cache  *cache.JMAPCache
 
 	selectedMbox jmap.ID
 	dir2mbox     map[string]jmap.ID