From 7f7e360cd7224943c3e5a41e4f0397d5f7e4a150 Mon Sep 17 00:00:00 2001 From: Romain Le Jeune Date: Thu, 1 Jun 2023 15:27:56 +0000 Subject: [PATCH] feat(GODT-2673): Use NoClient as UserAgent without any client connected and... --- internal/bridge/bridge_test.go | 91 ++++++++++++++++++++++++++++++-- internal/bridge/imap.go | 7 +++ internal/bridge/smtp_backend.go | 5 ++ internal/useragent/useragent.go | 6 ++- internal/vault/settings.go | 3 +- internal/vault/settings_test.go | 3 +- internal/vault/types_settings.go | 4 +- tests/features/imap/id.feature | 7 ++- tests/features/smtp/auth.feature | 6 +++ 9 files changed, 122 insertions(+), 10 deletions(-) diff --git a/internal/bridge/bridge_test.go b/internal/bridge/bridge_test.go index 676eb612..f9be0e56 100644 --- a/internal/bridge/bridge_test.go +++ b/internal/bridge/bridge_test.go @@ -50,6 +50,8 @@ import ( "github.com/ProtonMail/proton-bridge/v3/tests" "github.com/bradenaw/juniper/xslices" imapid "github.com/emersion/go-imap-id" + "github.com/emersion/go-sasl" + "github.com/emersion/go-smtp" "github.com/stretchr/testify/require" ) @@ -184,14 +186,14 @@ func TestBridge_UserAgent_Persistence(t *testing.T) { smtpWaiter := waitForSMTPServerReady(b) defer smtpWaiter.Done() + currentUserAgent := b.GetCurrentUserAgent() + require.Contains(t, currentUserAgent, useragent.DefaultUserAgent) + require.NoError(t, getErr(b.LoginFull(ctx, otherUser, otherPassword, nil, nil))) imapWaiter.Wait() smtpWaiter.Wait() - currentUserAgent := b.GetCurrentUserAgent() - require.Contains(t, currentUserAgent, vault.DefaultUserAgent) - imapClient, err := eventuallyDial(fmt.Sprintf("%v:%v", constants.Host, b.GetIMAPPort())) require.NoError(t, err) defer func() { _ = imapClient.Logout() }() @@ -221,6 +223,89 @@ func TestBridge_UserAgent_Persistence(t *testing.T) { }) } +func TestBridge_UserAgentFromUnknownClient(t *testing.T) { + withEnv(t, func(ctx context.Context, s *server.Server, netCtl *proton.NetCtl, locator bridge.Locator, vaultKey []byte) { + otherPassword := []byte("bar") + otherUser := "foo" + _, _, err := s.CreateUser(otherUser, otherPassword) + require.NoError(t, err) + + withBridge(ctx, t, s.GetHostURL(), netCtl, locator, vaultKey, func(b *bridge.Bridge, mocks *bridge.Mocks) { + imapWaiter := waitForIMAPServerReady(b) + defer imapWaiter.Done() + + smtpWaiter := waitForSMTPServerReady(b) + defer smtpWaiter.Done() + + currentUserAgent := b.GetCurrentUserAgent() + require.Contains(t, currentUserAgent, useragent.DefaultUserAgent) + + userID, err := b.LoginFull(context.Background(), username, password, nil, nil) + require.NoError(t, err) + + imapWaiter.Wait() + smtpWaiter.Wait() + + imapClient, err := eventuallyDial(fmt.Sprintf("%v:%v", constants.Host, b.GetIMAPPort())) + require.NoError(t, err) + defer func() { _ = imapClient.Logout() }() + + info, err := b.GetUserInfo(userID) + require.NoError(t, err) + require.True(t, info.State == bridge.Connected) + + require.NoError(t, imapClient.Login(info.Addresses[0], string(info.BridgePass))) + + currentUserAgent = b.GetCurrentUserAgent() + require.Contains(t, currentUserAgent, "UnknownClient/0.0.1") + }) + }) +} + +func TestBridge_UserAgentFromSMTPClient(t *testing.T) { + withEnv(t, func(ctx context.Context, s *server.Server, netCtl *proton.NetCtl, locator bridge.Locator, vaultKey []byte) { + otherPassword := []byte("bar") + otherUser := "foo" + _, _, err := s.CreateUser(otherUser, otherPassword) + require.NoError(t, err) + + withBridge(ctx, t, s.GetHostURL(), netCtl, locator, vaultKey, func(b *bridge.Bridge, mocks *bridge.Mocks) { + imapWaiter := waitForIMAPServerReady(b) + defer imapWaiter.Done() + + smtpWaiter := waitForSMTPServerReady(b) + defer smtpWaiter.Done() + + currentUserAgent := b.GetCurrentUserAgent() + require.Contains(t, currentUserAgent, useragent.DefaultUserAgent) + + userID, err := b.LoginFull(context.Background(), username, password, nil, nil) + require.NoError(t, err) + + imapWaiter.Wait() + smtpWaiter.Wait() + + client, err := smtp.Dial(net.JoinHostPort(constants.Host, fmt.Sprint(b.GetSMTPPort()))) + require.NoError(t, err) + defer client.Close() //nolint:errcheck + + info, err := b.GetUserInfo(userID) + require.NoError(t, err) + require.True(t, info.State == bridge.Connected) + + // Upgrade to TLS. + require.NoError(t, client.StartTLS(&tls.Config{InsecureSkipVerify: true})) + require.NoError(t, client.Auth(sasl.NewLoginClient( + info.Addresses[0], + string(info.BridgePass)), + )) + + currentUserAgent = b.GetCurrentUserAgent() + require.Contains(t, currentUserAgent, "UnknownClient/0.0.1") + }) + }) +} + func TestBridge_UserAgentFromIMAPID(t *testing.T) { withEnv(t, func(ctx context.Context, s *server.Server, netCtl *proton.NetCtl, locator bridge.Locator, vaultKey []byte) { var ( diff --git a/internal/bridge/imap.go b/internal/bridge/imap.go index cf43c619..97eed175 100644 --- a/internal/bridge/imap.go +++ b/internal/bridge/imap.go @@ -23,6 +23,7 @@ import ( "io" "os" "path/filepath" + "strings" "github.com/Masterminds/semver/v3" "github.com/ProtonMail/gluon" @@ -36,6 +37,7 @@ import ( "github.com/ProtonMail/proton-bridge/v3/internal/events" "github.com/ProtonMail/proton-bridge/v3/internal/logging" "github.com/ProtonMail/proton-bridge/v3/internal/user" + "github.com/ProtonMail/proton-bridge/v3/internal/useragent" "github.com/sirupsen/logrus" ) @@ -82,6 +84,11 @@ func (bridge *Bridge) handleIMAPEvent(event imapEvents.Event) { "pkg": "imap", }).Error("Incorrect login credentials.") bridge.publish(events.IMAPLoginFailed{Username: event.Username}) + + case imapEvents.Login: + if strings.Contains(bridge.GetCurrentUserAgent(), useragent.DefaultUserAgent) { + bridge.setUserAgent(useragent.UnknownClient, useragent.DefaultVersion) + } } } diff --git a/internal/bridge/smtp_backend.go b/internal/bridge/smtp_backend.go index 996bf5be..5cf3d576 100644 --- a/internal/bridge/smtp_backend.go +++ b/internal/bridge/smtp_backend.go @@ -20,8 +20,10 @@ package bridge import ( "fmt" "io" + "strings" "github.com/ProtonMail/proton-bridge/v3/internal/safe" + "github.com/ProtonMail/proton-bridge/v3/internal/useragent" "github.com/emersion/go-smtp" "github.com/sirupsen/logrus" ) @@ -55,6 +57,9 @@ func (s *smtpSession) AuthPlain(username, password string) error { s.userID = user.ID() s.authID = addrID + if strings.Contains(s.Bridge.GetCurrentUserAgent(), useragent.DefaultUserAgent) { + s.Bridge.setUserAgent(useragent.UnknownClient, useragent.DefaultVersion) + } return nil } diff --git a/internal/useragent/useragent.go b/internal/useragent/useragent.go index 1e8b87bc..244ea66a 100644 --- a/internal/useragent/useragent.go +++ b/internal/useragent/useragent.go @@ -24,6 +24,10 @@ import ( "sync" ) +const DefaultUserAgent = "NoClient/0.0.1" +const DefaultVersion = "0.0.1" +const UnknownClient = "UnknownClient" + type UserAgent struct { client, platform string @@ -81,7 +85,7 @@ func (ua *UserAgent) GetUserAgent() string { if ua.client != "" { client = ua.client } else { - client = "NoClient/0.0.1" + client = DefaultUserAgent } return fmt.Sprintf("%v (%v)", client, ua.platform) diff --git a/internal/vault/settings.go b/internal/vault/settings.go index 9a7815b3..f566fe10 100644 --- a/internal/vault/settings.go +++ b/internal/vault/settings.go @@ -24,6 +24,7 @@ import ( "github.com/Masterminds/semver/v3" "github.com/ProtonMail/proton-bridge/v3/internal/updater" + "github.com/ProtonMail/proton-bridge/v3/internal/useragent" "github.com/sirupsen/logrus" ) @@ -245,7 +246,7 @@ func (vault *Vault) GetLastUserAgent() string { // Handle case where there may be no value. if len(v) == 0 { - v = DefaultUserAgent + v = useragent.DefaultUserAgent } return v diff --git a/internal/vault/settings_test.go b/internal/vault/settings_test.go index 37efda0f..c1679117 100644 --- a/internal/vault/settings_test.go +++ b/internal/vault/settings_test.go @@ -24,6 +24,7 @@ import ( "github.com/Masterminds/semver/v3" "github.com/ProtonMail/gluon/async" "github.com/ProtonMail/proton-bridge/v3/internal/updater" + "github.com/ProtonMail/proton-bridge/v3/internal/useragent" "github.com/ProtonMail/proton-bridge/v3/internal/vault" "github.com/stretchr/testify/require" ) @@ -236,7 +237,7 @@ func TestVault_Settings_LastUserAgent(t *testing.T) { s := newVault(t) // Check the default first start value. - require.Equal(t, vault.DefaultUserAgent, s.GetLastUserAgent()) + require.Equal(t, useragent.DefaultUserAgent, s.GetLastUserAgent()) } func Test_Settings_PasswordArchive(t *testing.T) { diff --git a/internal/vault/types_settings.go b/internal/vault/types_settings.go index 158791ea..5582b2e3 100644 --- a/internal/vault/types_settings.go +++ b/internal/vault/types_settings.go @@ -23,6 +23,7 @@ import ( "time" "github.com/ProtonMail/proton-bridge/v3/internal/updater" + "github.com/ProtonMail/proton-bridge/v3/internal/useragent" "github.com/ProtonMail/proton-bridge/v3/pkg/ports" ) @@ -61,7 +62,6 @@ type Settings struct { } const DefaultMaxSyncMemory = 2 * 1024 * uint64(1024*1024) -const DefaultUserAgent = "UnknownClient/0.0.1" func GetDefaultSyncWorkerCount() int { const minSyncWorkers = 16 @@ -105,7 +105,7 @@ func newDefaultSettings(gluonDir string) Settings { SyncWorkers: syncWorkers, SyncAttPool: syncWorkers, - LastUserAgent: DefaultUserAgent, + LastUserAgent: useragent.DefaultUserAgent, LastHeartbeatSent: time.Time{}, PasswordArchive: PasswordArchive{}, diff --git a/tests/features/imap/id.feature b/tests/features/imap/id.feature index 95d1ab83..9e3db21f 100644 --- a/tests/features/imap/id.feature +++ b/tests/features/imap/id.feature @@ -6,8 +6,11 @@ Feature: The IMAP ID is propagated to bridge And the user logs in with username "[user:user]" and password "password" Then it succeeds - Scenario: Initial user agent before an IMAP client announces its ID - When user "[user:user]" connects IMAP client "1" + Scenario: Initial user agent before an IMAP client connects + Then the user agent is "NoClient/0.0.1 ([GOOS])" + + Scenario: User agent before an IMAP client announces its ID + When user "[user:user]" connects and authenticates IMAP client "1" Then the user agent is "UnknownClient/0.0.1 ([GOOS])" Scenario: User agent after an IMAP client announces its ID diff --git a/tests/features/smtp/auth.feature b/tests/features/smtp/auth.feature index 067b9321..15163750 100644 --- a/tests/features/smtp/auth.feature +++ b/tests/features/smtp/auth.feature @@ -17,6 +17,12 @@ Feature: A user can authenticate an SMTP client When user "[user:user]" connects SMTP client "1" Then SMTP client "1" can authenticate + Scenario: User agent with only SMTP client connected + Then the user agent is "NoClient/0.0.1 ([GOOS])" + When user "[user:user]" connects SMTP client "1" + Then SMTP client "1" can authenticate + Then the user agent is "UnknownClient/0.0.1 ([GOOS])" + Scenario: SMTP client cannot authenticate with wrong username When user "[user:user]" connects SMTP client "1" Then SMTP client "1" cannot authenticate with incorrect username