Skip to content

Commit

Permalink
VPN-5612 Do not deactivate VPN upon encountering backendfailure (#8300)
Browse files Browse the repository at this point in the history 
* Do not deactivate VPN upon encountering backendfailure

* fixup! Do not deactivate VPN upon encountering backendfailure

* limit fix to taskHeartbeat instead of changing what states would call deactivate() to minimize unknown impact

* fixup! limit fix to taskHeartbeat instead of changing what states would call deactivate() to minimize unknown impact

* fixup! limit fix to taskHeartbeat instead of changing what states would call deactivate() to minimize unknown impact

* If user is not yet authenticated when a Guardian error happens, show them the -something went wrong- screen

* fixup! If user is not yet authenticated when a Guardian error happens, show them the -something went wrong- screen

* Add comments to functional tests and the code changes
  • Loading branch information
Gela authored Oct 18, 2023
1 parent 998ed1f commit 0121fca
Show file tree
Hide file tree
Showing 3 changed files with 56 additions and 35 deletions.
12 changes: 11 additions & 1 deletion src/mozillavpn.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
8 changes: 4 additions & 4 deletions src/tasks/heartbeat/taskheartbeat.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
71 changes: 41 additions & 30 deletions tests/functional/testHeartbeat.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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());
});

Expand All @@ -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(
Expand All @@ -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 () => {
Expand All @@ -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(() => {
Expand All @@ -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';
});
});
});
});

0 comments on commit 0121fca

Please sign in to comment.