User agent detected by fake IMAP extension instead of AUTH callback

Changelog.md

@@ -13,6 +13,7 @@ Changelog [format](http://keepachangelog.com/en/1.0.0/)
 * GODT-511 User agent format changed.
 * Unsilent errors reading mbox files.
 * GODT-692 QA build with option to change API URL by ENV variable.
+* GODT-704 User agent detected by fake IMAP extension instead of AUTH callback (some clients use LOGIN instead of AUTH).
 
 ### Removed
 * GODT-519 Unused AUTH scope parsing methods.

internal/imap/backend.go

@@ -23,7 +23,6 @@ import (
 	"sync"
 	"time"
 
-	imapid "github.com/ProtonMail/go-imap-id"
 	"github.com/ProtonMail/proton-bridge/internal/bridge"
 	"github.com/ProtonMail/proton-bridge/internal/events"
 	"github.com/ProtonMail/proton-bridge/pkg/listener"
@@ -45,9 +44,6 @@ type imapBackend struct {
 	users       map[string]*imapUser
 	usersLocker sync.Locker
 
-	lastMailClient       imapid.ID
-	lastMailClientLocker sync.Locker
-
 	imapCache     map[string]map[string]string
 	imapCachePath string
 	imapCacheLock *sync.RWMutex
@@ -87,9 +83,6 @@ func newIMAPBackend(
 		users:       map[string]*imapUser{},
 		usersLocker: &sync.Mutex{},
 
-		lastMailClient:       imapid.ID{imapid.FieldName: clientNone},
-		lastMailClientLocker: &sync.Mutex{},
-
 		imapCachePath: cfg.GetIMAPCachePath(),
 		imapCacheLock: &sync.RWMutex{},
 	}
@@ -194,23 +187,6 @@ func (ib *imapBackend) CreateMessageLimit() *uint32 {
 	return nil
 }
 
-func (ib *imapBackend) setLastMailClient(id imapid.ID) {
-	ib.lastMailClientLocker.Lock()
-	defer ib.lastMailClientLocker.Unlock()
-
-	if name, ok := id[imapid.FieldName]; ok && ib.lastMailClient[imapid.FieldName] != name {
-		ib.lastMailClient = imapid.ID{}
-		for k, v := range id {
-			ib.lastMailClient[k] = v
-		}
-		log.Warn("Mail Client ID changed to ", ib.lastMailClient)
-		ib.bridge.SetCurrentClient(
-			ib.lastMailClient[imapid.FieldName],
-			ib.lastMailClient[imapid.FieldVersion],
-		)
-	}
-}
-
 // monitorDisconnectedUsers removes users when it receives a close connection event for them.
 func (ib *imapBackend) monitorDisconnectedUsers() {
 	ch := make(chan string)

internal/imap/currentclient/extension.go

@@ -0,0 +1,102 @@
+// Copyright (c) 2020 Proton Technologies AG
+//
+// This file is part of ProtonMail Bridge.
+//
+// ProtonMail 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.
+//
+// ProtonMail 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 ProtonMail Bridge.  If not, see <https://www.gnu.org/licenses/>.
+
+// Package currentclient implements setting client's ID to backend.
+package currentclient
+
+import (
+	"sync"
+	"time"
+
+	imapid "github.com/ProtonMail/go-imap-id"
+	imapserver "github.com/emersion/go-imap/server"
+	"github.com/sirupsen/logrus"
+)
+
+var log = logrus.WithField("pkg", "imap/currentclient") //nolint[gochecknoglobals]
+
+type currentClientSetter interface {
+	SetCurrentClient(name, version string)
+}
+
+type extension struct {
+	backend currentClientSetter
+
+	lastID       imapid.ID
+	lastIDLocker sync.Locker
+}
+
+// NewExtension prepares extension reading IMAP ID from connection
+// and setting it to backend. This is not standard IMAP extension.
+func NewExtension(backend currentClientSetter) imapserver.Extension {
+	return &extension{
+		backend: backend,
+
+		lastID:       imapid.ID{imapid.FieldName: ""},
+		lastIDLocker: &sync.Mutex{},
+	}
+}
+
+func (ext *extension) Capabilities(conn imapserver.Conn) []string {
+	ext.readID(conn)
+	return nil
+}
+
+func (ext *extension) Command(name string) imapserver.HandlerFactory {
+	return nil
+}
+
+func (ext *extension) readID(conn imapserver.Conn) {
+	conn.Server().ForEachConn(func(candidate imapserver.Conn) {
+		if id, ok := candidate.(imapid.Conn); ok {
+			if conn.Context() == candidate.Context() {
+				// ID is not available right at the beginning of the connection.
+				// Clients send ID quickly after AUTH. We need to wait for it.
+				go func() {
+					start := time.Now()
+					for {
+						if id.ID() != nil {
+							ext.setLastID(id.ID())
+							break
+						}
+						if time.Since(start) > 10*time.Second {
+							break
+						}
+						time.Sleep(100 * time.Millisecond)
+					}
+				}()
+			}
+		}
+	})
+}
+
+func (ext *extension) setLastID(id imapid.ID) {
+	ext.lastIDLocker.Lock()
+	defer ext.lastIDLocker.Unlock()
+
+	if name, ok := id[imapid.FieldName]; ok && ext.lastID[imapid.FieldName] != name {
+		ext.lastID = imapid.ID{}
+		for k, v := range id {
+			ext.lastID[k] = v
+		}
+		log.Warn("Mail Client ID changed to ", ext.lastID)
+		ext.backend.SetCurrentClient(
+			ext.lastID[imapid.FieldName],
+			ext.lastID[imapid.FieldVersion],
+		)
+	}
+}

internal/imap/imap.go

@@ -22,12 +22,6 @@ import "github.com/sirupsen/logrus"
 const (
 	fetchMessagesWorkers    = 5 // In how many workers to fetch message (group list on IMAP).
 	fetchAttachmentsWorkers = 5 // In how many workers to fetch attachments (for one message).
-
-	clientAppleMail   = "Mac OS X Mail"             //nolint[deadcode]
-	clientThunderbird = "Thunderbird"               //nolint[deadcode]
-	clientOutlookMac  = "Microsoft Outlook for Mac" //nolint[deadcode]
-	clientOutlookWin  = "Microsoft Outlook"         //nolint[deadcode]
-	clientNone        = ""
 )
 
 var (

internal/imap/server.go

@@ -28,6 +28,7 @@ import (
 	imapid "github.com/ProtonMail/go-imap-id"
 	"github.com/ProtonMail/proton-bridge/internal/bridge"
 	"github.com/ProtonMail/proton-bridge/internal/events"
+	"github.com/ProtonMail/proton-bridge/internal/imap/currentclient"
 	"github.com/ProtonMail/proton-bridge/internal/imap/uidplus"
 	"github.com/ProtonMail/proton-bridge/pkg/listener"
 	"github.com/emersion/go-imap"
@@ -66,28 +67,6 @@ func NewIMAPServer(debugClient, debugServer bool, port int, tls *tls.Config, ima
 	}
 
 	s.EnableAuth(sasl.Login, func(conn imapserver.Conn) sasl.Server {
-		conn.Server().ForEachConn(func(candidate imapserver.Conn) {
-			if id, ok := candidate.(imapid.Conn); ok {
-				if conn.Context() == candidate.Context() {
-					// ID is not available right at the beginning of the connection.
-					// Clients send ID quickly after AUTH. We need to wait for it.
-					go func() {
-						start := time.Now()
-						for {
-							if id.ID() != nil {
-								imapBackend.setLastMailClient(id.ID())
-								break
-							}
-							if time.Since(start) > 10*time.Second {
-								break
-							}
-							time.Sleep(100 * time.Millisecond)
-						}
-					}()
-				}
-			}
-		})
-
 		return sasl.NewLoginServer(func(address, password string) error {
 			user, err := conn.Server().Backend.Login(nil, address, password)
 			if err != nil {
@@ -109,6 +88,7 @@ func NewIMAPServer(debugClient, debugServer bool, port int, tls *tls.Config, ima
 		imapquota.NewExtension(),
 		imapappendlimit.NewExtension(),
 		imapunselect.NewExtension(),
+		currentclient.NewExtension(imapBackend.bridge),
 		uidplus.NewExtension(),
 	)