From 6269b1ab88489543cdb57c9ba75d32257c3a0248 Mon Sep 17 00:00:00 2001 From: Romain Le Jeune Date: Fri, 8 Sep 2023 11:53:24 +0000 Subject: [PATCH] fix(GODT-2913): Reduce the number of configuration failure detected. --- internal/services/imapservice/connector.go | 3 ++- internal/services/imapservice/service.go | 1 + internal/services/imapservice/shared_identity.go | 6 +++--- internal/services/smtp/service.go | 2 +- internal/services/useridentity/service.go | 2 +- internal/services/useridentity/state.go | 8 ++------ internal/user/user.go | 2 +- 7 files changed, 11 insertions(+), 13 deletions(-) diff --git a/internal/services/imapservice/connector.go b/internal/services/imapservice/connector.go index 2fcb96a3..03ca7dbe 100644 --- a/internal/services/imapservice/connector.go +++ b/internal/services/imapservice/connector.go @@ -119,8 +119,9 @@ func (s *Connector) Init(_ context.Context, cache connector.IMAPState) error { } func (s *Connector) Authorize(ctx context.Context, username string, password []byte) bool { - addrID, err := s.identityState.CheckAuth(username, password, s.telemetry) + addrID, err := s.identityState.CheckAuth(username, password) if err != nil { + s.telemetry.ReportConfigStatusFailure("IMAP " + err.Error()) return false } diff --git a/internal/services/imapservice/service.go b/internal/services/imapservice/service.go index d7c2fba6..701a62c6 100644 --- a/internal/services/imapservice/service.go +++ b/internal/services/imapservice/service.go @@ -48,6 +48,7 @@ type EventProvider interface { type Telemetry interface { useridentity.Telemetry SendConfigStatusSuccess(ctx context.Context) + ReportConfigStatusFailure(errDetails string) } type GluonIDProvider interface { diff --git a/internal/services/imapservice/shared_identity.go b/internal/services/imapservice/shared_identity.go index c53575fb..4117faf4 100644 --- a/internal/services/imapservice/shared_identity.go +++ b/internal/services/imapservice/shared_identity.go @@ -32,7 +32,7 @@ type sharedIdentity interface { GetPrimaryAddress() (proton.Address, error) GetAddresses() []proton.Address WithAddrKR(addrID string, fn func(userKR, addrKR *crypto.KeyRing) error) error - CheckAuth(email string, password []byte, telemetry Telemetry) (string, error) + CheckAuth(email string, password []byte) (string, error) } type rwIdentity struct { @@ -102,11 +102,11 @@ func (r *rwIdentity) WithAddrKRs(fn func(*crypto.KeyRing, map[string]*crypto.Key return r.identity.WithAddrKRs(r.keyPassProvider.KeyPass(), fn) } -func (r *rwIdentity) CheckAuth(email string, password []byte, telemetry Telemetry) (string, error) { +func (r *rwIdentity) CheckAuth(email string, password []byte) (string, error) { r.lock.RLock() defer r.lock.RUnlock() - return r.identity.CheckAuth(email, password, r.bridgePassProvider, telemetry) + return r.identity.CheckAuth(email, password, r.bridgePassProvider) } func (r *rwIdentity) Write(f func(identity *useridentity.State) error) error { diff --git a/internal/services/smtp/service.go b/internal/services/smtp/service.go index 1705c389..0be2f6c6 100644 --- a/internal/services/smtp/service.go +++ b/internal/services/smtp/service.go @@ -221,7 +221,7 @@ func (s *Service) run(ctx context.Context) { case *checkAuthReq: s.log.WithField("email", bridgelogging.Sensitive(r.email)).Debug("Checking authentication") - addrID, err := s.identityState.CheckAuth(r.email, r.password, s.bridgePassProvider, s.telemetry) + addrID, err := s.identityState.CheckAuth(r.email, r.password, s.bridgePassProvider) request.Reply(ctx, addrID, err) case *resyncReq: diff --git a/internal/services/useridentity/service.go b/internal/services/useridentity/service.go index a57c0a9a..4a359816 100644 --- a/internal/services/useridentity/service.go +++ b/internal/services/useridentity/service.go @@ -249,7 +249,7 @@ func (s *Service) run(ctx context.Context) { r.Reply(ctx, maps.Clone(s.identity.Addresses), nil) case *checkAuthReq: - id, err := s.identity.CheckAuth(req.email, req.password, s.bridgePassProvider, s.telemetry) + id, err := s.identity.CheckAuth(req.email, req.password, s.bridgePassProvider) r.Reply(ctx, id, err) default: diff --git a/internal/services/useridentity/state.go b/internal/services/useridentity/state.go index 5bb5ca44..873c0db8 100644 --- a/internal/services/useridentity/state.go +++ b/internal/services/useridentity/state.go @@ -226,7 +226,7 @@ func (s *State) Clone() *State { // CheckAuth returns whether the given email and password can be used to authenticate over IMAP or SMTP with this user. // It returns the address ID of the authenticated address. -func (s *State) CheckAuth(email string, password []byte, bridgePassProvider BridgePassProvider, telemetry Telemetry) (string, error) { +func (s *State) CheckAuth(email string, password []byte, bridgePassProvider BridgePassProvider) (string, error) { if email == "crash@bandicoot" { panic("your wish is my command.. I crash") } @@ -237,11 +237,7 @@ func (s *State) CheckAuth(email string, password []byte, bridgePassProvider Brid } if subtle.ConstantTimeCompare(bridgePassProvider.BridgePass(), dec) != 1 { - err := fmt.Errorf("invalid password") - if telemetry != nil { - telemetry.ReportConfigStatusFailure(err.Error()) - } - return "", err + return "", fmt.Errorf("invalid password") } for _, addr := range s.AddressesSorted { diff --git a/internal/user/user.go b/internal/user/user.go index c983dcce..4e1470a6 100644 --- a/internal/user/user.go +++ b/internal/user/user.go @@ -674,7 +674,7 @@ func (user *User) ReportSMTPAuthFailed(username string) { emails := user.Emails() for _, mail := range emails { if mail == username { - user.ReportConfigStatusFailure("invalid username or password") + user.ReportConfigStatusFailure("SMTP invalid username or password") } } }