From 7d9753e2daf44fc760fc1b6739371a11878a7a89 Mon Sep 17 00:00:00 2001 From: Atanas Janeshliev Date: Wed, 6 Nov 2024 16:02:05 +0100 Subject: [PATCH] fix(BRIDGE-107): improved human verification UX --- internal/bridge/bridge.go | 10 ++++++++++ .../bridge-gui/bridge-gui-tester/GRPCService.cpp | 2 +- .../bridge-gui/bridge-gui/qml/SetupWizard/Login.qml | 7 ++++++- internal/frontend/cli/accounts.go | 5 ++++- internal/frontend/grpc/service.go | 7 +++++-- internal/frontend/grpc/service_methods.go | 3 ++- internal/hv/hv.go | 7 ++++++- 7 files changed, 34 insertions(+), 7 deletions(-) diff --git a/internal/bridge/bridge.go b/internal/bridge/bridge.go index 63387292..88ef7676 100644 --- a/internal/bridge/bridge.go +++ b/internal/bridge/bridge.go @@ -723,3 +723,13 @@ func (bridge *Bridge) PushDistinctObservabilityMetrics(errType observability.Dis func (bridge *Bridge) ModifyObservabilityHeartbeatInterval(duration time.Duration) { bridge.observabilityService.ModifyHeartbeatInterval(duration) } + +func (bridge *Bridge) ReportMessageWithContext(message string, messageCtx reporter.Context) { + if err := bridge.reporter.ReportMessageWithContext(message, messageCtx); err != nil { + logPkg.WithFields(logrus.Fields{ + "err": err, + "sentryMessage": message, + "messageCtx": messageCtx, + }).Info("Error occurred when sending Report to Sentry") + } +} diff --git a/internal/frontend/bridge-gui/bridge-gui-tester/GRPCService.cpp b/internal/frontend/bridge-gui/bridge-gui-tester/GRPCService.cpp index 9eb0fac2..590abeb7 100644 --- a/internal/frontend/bridge-gui/bridge-gui-tester/GRPCService.cpp +++ b/internal/frontend/bridge-gui/bridge-gui-tester/GRPCService.cpp @@ -30,7 +30,7 @@ using namespace bridgepp; namespace { QString const defaultKeychain = "defaultKeychain"; ///< The default keychain. -QString const HV_ERROR_TEMPLATE = "failed to create new API client: 422 POST https://mail-api.proton.me/auth/v4: CAPTCHA validation failed (Code=12087, Status=422)"; +QString const HV_ERROR_TEMPLATE = "Human verification failed. Please try again."; } diff --git a/internal/frontend/bridge-gui/bridge-gui/qml/SetupWizard/Login.qml b/internal/frontend/bridge-gui/bridge-gui/qml/SetupWizard/Login.qml index 2f1a86a9..c16529a4 100644 --- a/internal/frontend/bridge-gui/bridge-gui/qml/SetupWizard/Login.qml +++ b/internal/frontend/bridge-gui/bridge-gui/qml/SetupWizard/Login.qml @@ -29,6 +29,7 @@ FocusScope { property alias username: usernameTextField.text property var wizard property string hvLinkUrl: "" + property bool hvLinkClicked: false signal loginAbort(string username, bool wasSignedOut) @@ -49,6 +50,7 @@ FocusScope { } passwordTextField.hidePassword(); secondPasswordTextField.hidePassword(); + hvLinkClicked = false; } function resetViaHv() { usernameTextField.enabled = false; @@ -56,6 +58,7 @@ FocusScope { signInButton.loading = true; secondPasswordButton.loading = false; secondPasswordTextField.enabled = true; + hvLinkClicked = false; totpLayout.reset(); } @@ -562,6 +565,7 @@ FocusScope { cursorShape: Qt.PointingHandCursor onClicked: { Qt.openUrlExternally(hvLinkUrl); + hvLinkClicked = true; } } } @@ -574,7 +578,8 @@ FocusScope { id: hVContinueButton Layout.fillWidth: true colorScheme: wizard.colorScheme - text: qsTr("Continue") + text: qsTr("I’ve completed the verification") + enabled: hvLinkClicked function checkAndSignInHv() { console.assert(stackLayout.currentIndex === Login.RootStack.HV || stackLayout.currentIndex === Login.RootStack.MailboxPassword, "Unexpected checkInAndSignInHv") diff --git a/internal/frontend/cli/accounts.go b/internal/frontend/cli/accounts.go index 6dcaf440..2e8afb28 100644 --- a/internal/frontend/cli/accounts.go +++ b/internal/frontend/cli/accounts.go @@ -159,7 +159,10 @@ func (f *frontendCLI) loginAccount(c *ishell.Context) { hvDetails, hvErr := hv.VerifyAndExtractHvRequest(err) if hvErr != nil || hvDetails != nil { if hvErr != nil { - f.printAndLogError("Cannot login", hvErr) + f.printAndLogError("Cannot login:", hv.ExtractionErrorMsg) + f.bridge.ReportMessageWithContext("Unable to extract HV request details", map[string]any{ + "error": err.Error(), + }) return } f.promptHvURL(hvDetails) diff --git a/internal/frontend/grpc/service.go b/internal/frontend/grpc/service.go index 901fe61b..3b1b12d3 100644 --- a/internal/frontend/grpc/service.go +++ b/internal/frontend/grpc/service.go @@ -465,7 +465,7 @@ func (s *Service) finishLogin() { if apiErr := new(proton.APIError); errors.As(err, &apiErr) && apiErr.Code == proton.HumanValidationInvalidToken { s.hvDetails = nil - _ = s.SendEvent(NewLoginError(LoginErrorType_HV_ERROR, err.Error())) + _ = s.SendEvent(NewLoginError(LoginErrorType_HV_ERROR, hv.VerificationFailedErrorMsg)) return } @@ -643,7 +643,10 @@ func (s *Service) monitorParentPID() { func (s *Service) handleHvRequest(err error) { hvDet, hvErr := hv.VerifyAndExtractHvRequest(err) if hvErr != nil { - _ = s.SendEvent(NewLoginError(LoginErrorType_HV_ERROR, hvErr.Error())) + _ = s.SendEvent(NewLoginError(LoginErrorType_HV_ERROR, hv.ExtractionErrorMsg)) + s.bridge.ReportMessageWithContext("Unable to extract HV request details", map[string]any{ + "error": err.Error(), + }) return } diff --git a/internal/frontend/grpc/service_methods.go b/internal/frontend/grpc/service_methods.go index d59c675e..0faa3fa0 100644 --- a/internal/frontend/grpc/service_methods.go +++ b/internal/frontend/grpc/service_methods.go @@ -30,6 +30,7 @@ import ( "github.com/ProtonMail/proton-bridge/v3/internal/constants" "github.com/ProtonMail/proton-bridge/v3/internal/events" "github.com/ProtonMail/proton-bridge/v3/internal/frontend/theme" + "github.com/ProtonMail/proton-bridge/v3/internal/hv" "github.com/ProtonMail/proton-bridge/v3/internal/kb" "github.com/ProtonMail/proton-bridge/v3/internal/safe" "github.com/ProtonMail/proton-bridge/v3/internal/service" @@ -468,7 +469,7 @@ func (s *Service) Login(_ context.Context, login *LoginRequest) (*emptypb.Empty, case proton.HumanValidationInvalidToken: s.hvDetails = nil - _ = s.SendEvent(NewLoginError(LoginErrorType_HV_ERROR, err.Error())) + _ = s.SendEvent(NewLoginError(LoginErrorType_HV_ERROR, hv.VerificationFailedErrorMsg)) default: _ = s.SendEvent(NewLoginError(LoginErrorType_USERNAME_PASSWORD_ERROR, err.Error())) diff --git a/internal/hv/hv.go b/internal/hv/hv.go index 69e40bd6..5d312a51 100644 --- a/internal/hv/hv.go +++ b/internal/hv/hv.go @@ -21,6 +21,11 @@ import ( "github.com/ProtonMail/go-proton-api" ) +const ( + ExtractionErrorMsg = "Human verification requested, but an issue occurred. Please try again." + VerificationFailedErrorMsg = "Human verification failed. Please try again." +) + // VerifyAndExtractHvRequest expects an error request as input // determines whether the given error is a Proton human verification request; if it isn't then it returns -> nil, nil (no details, no error) // if it is a HV req. then it tries to parse the json data and verify that the captcha method is included; if either fails -> nil, err @@ -34,7 +39,7 @@ func VerifyAndExtractHvRequest(err error) (*proton.APIHVDetails, error) { if errors.As(err, &protonErr) && protonErr.IsHVError() { hvDetails, hvErr := protonErr.GetHVDetails() if hvErr != nil { - return nil, fmt.Errorf("received HV request, but can't decode HV details") + return nil, hvErr } return hvDetails, nil }