diff --git a/src/mozillavpn.cpp b/src/mozillavpn.cpp index be7b792bc0..8039eaa013 100644 --- a/src/mozillavpn.cpp +++ b/src/mozillavpn.cpp @@ -1213,7 +1213,17 @@ void MozillaVPN::backendServiceRestore() { void MozillaVPN::heartbeatCompleted(bool success) { logger.debug() << "Server-side check done:" << success; - if (!success) { + // In the event of a Guardian error during authentication, + // the "something went wrong" screen is displayed to the user. + // This is important because the service used for authentication is + // unavailable and causes an interruption in the authentication process. + // Alternatively if the user is already authenticated, we intentionally avoid + // presenting them with the "something went wrong" screen to avoid unnecessary + // interruptions. If the user is already authenticated, it is possible that a + // Guardian service may momentarily become unavailable but there is nothing + // for the user to do and it may not affect the connectivity at all so it is + // not necessary to take additional actions. + if (!success && state() == StateAuthenticating) { m_private->m_connectionManager.backendFailure(); return; } diff --git a/src/tasks/heartbeat/taskheartbeat.cpp b/src/tasks/heartbeat/taskheartbeat.cpp index 3417c360ff..ecc8e03834 100644 --- a/src/tasks/heartbeat/taskheartbeat.cpp +++ b/src/tasks/heartbeat/taskheartbeat.cpp @@ -29,19 +29,19 @@ void TaskHeartbeat::run() { connect(request, &NetworkRequest::requestFailed, this, [this, request](QNetworkReply::NetworkError, const QByteArray&) { - logger.error() << "Failed to talk with the server"; + int statusCode = request->statusCode(); + logger.error() << "Failed to talk with the server. Status code: " + << statusCode; MozillaVPN* vpn = MozillaVPN::instance(); Q_ASSERT(vpn); - int statusCode = request->statusCode(); - // Internal server errors. if (statusCode >= 500 && statusCode <= 509) { vpn->heartbeatCompleted(false); } - // Request failure ((?!?) + // Client errors. else if (statusCode >= 400 && statusCode <= 409) { vpn->heartbeatCompleted(false); } diff --git a/tests/functional/testHeartbeat.js b/tests/functional/testHeartbeat.js index 175fe7523d..d0aa2570b1 100644 --- a/tests/functional/testHeartbeat.js +++ b/tests/functional/testHeartbeat.js @@ -9,31 +9,26 @@ const vpn = require('./helper.js'); describe('Backend failure', function() { this.timeout(300000); - async function backendFailureAndRestore() { - await vpn.forceHeartbeatFailure(); - await vpn.waitForQueryAndClick( - queries.screenBackendFailure.HEARTBEAT_TRY_BUTTON.visible()); - } - it('Backend failure during the main view', async () => { await vpn.waitForInitialView(); - await backendFailureAndRestore(); + await vpn.forceHeartbeatFailure(); await vpn.waitForQueryAndClick(queries.screenInitialize.GET_HELP_LINK); await vpn.waitForQuery(queries.screenGetHelp.LINKS.visible()); }); - /* TODO: - it('Backend failure in the help menu', async () => { + it('Backend failure in the help menu', async () => { await vpn.waitForQueryAndClick( queries.screenInitialize.GET_HELP_LINK.visible()); await vpn.waitForQuery(queries.screenGetHelp.BACK_BUTTON.visible()); - await backendFailureAndRestore(); - }); - */ + await vpn.forceHeartbeatFailure(); + + await + vpn.waitForQuery(queries.screenGetHelp.BACK_BUTTON.visible()); + }); it('BackendFailure during browser authentication', async () => { if (this.ctx.wasm) { @@ -54,21 +49,26 @@ describe('Backend failure', function() { await vpn.waitForQuery( queries.screenInitialize.AUTHENTICATE_VIEW.visible()); - await backendFailureAndRestore(); + // Ensure that in the event of a Guardian error during authentication, + // the "something went wrong" screen is displayed to the user. + await vpn.forceHeartbeatFailure(); + await vpn.waitForQueryAndClick( + queries.screenBackendFailure.HEARTBEAT_TRY_BUTTON.visible()); + await vpn.waitForQuery(queries.screenInitialize.GET_HELP_LINK.visible()); }); it('BackendFailure in the Post authentication view', async () => { await vpn.authenticateInApp(); await vpn.waitForQuery(queries.screenPostAuthentication.BUTTON.visible()); - await backendFailureAndRestore(); + await vpn.forceHeartbeatFailure(); await vpn.waitForQuery(queries.screenPostAuthentication.BUTTON.visible()); }); it('BackendFailure in the Telemetry policy view', async () => { await vpn.authenticateInApp(true, false); await vpn.waitForQuery(queries.screenTelemetry.BUTTON.visible()); - await backendFailureAndRestore(); + await vpn.forceHeartbeatFailure(); await vpn.waitForQuery(queries.screenTelemetry.BUTTON.visible()); }); @@ -82,7 +82,7 @@ describe('Backend failure', function() { queries.screenHome.CONTROLLER_TITLE.visible(), 'text'), 'VPN is off'); - await backendFailureAndRestore(); + await vpn.forceHeartbeatFailure(); await vpn.waitForQuery(queries.screenHome.CONTROLLER_TITLE.visible()); assert.equal( @@ -105,13 +105,18 @@ describe('Backend failure', function() { queries.screenHome.CONTROLLER_SUBTITLE, 'text'), 'Masking connection and location'); - await backendFailureAndRestore(); + await vpn.forceHeartbeatFailure(); await vpn.waitForQuery(queries.screenHome.CONTROLLER_TITLE.visible()); - assert.equal( - await vpn.getQueryProperty( - queries.screenHome.CONTROLLER_TITLE.visible(), 'text'), - 'VPN is off'); + + // VPN should not be disconnected after a heartbeat failure. + // This ensures that the VPN remains on to avoid potentially leaking traffic + // and leaving the user unprotected. + await vpn.waitForCondition(async () => { + return await vpn.getQueryProperty( + queries.screenHome.CONTROLLER_TITLE.visible(), 'text') === + 'VPN is on'; + }); }); it('BackendFailure when connected', async () => { @@ -122,16 +127,19 @@ describe('Backend failure', function() { 'VPN is on'; }); - await backendFailureAndRestore(); + await vpn.forceHeartbeatFailure(); await vpn.waitForQuery(queries.screenHome.CONTROLLER_TITLE.visible()); + + // VPN should not be disconnected after a heartbeat failure. + // This ensures that the VPN remains on to avoid potentially leaking traffic + // and leaving the user unprotected. assert.equal( await vpn.getQueryProperty( queries.screenHome.CONTROLLER_TITLE.visible(), 'text'), - 'VPN is off'); + 'VPN is on'); }); - it('disconnecting', async () => { await vpn.activate(); await vpn.waitForCondition(() => { @@ -145,13 +153,16 @@ describe('Backend failure', function() { return msg === 'Disconnecting…' || msg === 'VPN is off'; }); - await backendFailureAndRestore(); + await vpn.forceHeartbeatFailure(); - await vpn.waitForQuery(queries.screenHome.CONTROLLER_TITLE.visible()); - assert.equal( - await vpn.getQueryProperty( - queries.screenHome.CONTROLLER_TITLE.visible(), 'text'), - 'VPN is off'); + // Because the user has already initiated the deactivation of the VPN + // even if we encounter a heartbeat failure during the process, + // we proceed with deactivation as usual. + await vpn.waitForCondition(async () => { + const msg = await vpn.getQueryProperty( + queries.screenHome.CONTROLLER_TITLE.visible(), 'text'); + return msg === 'VPN is off'; + }); }); }); });