VPN-5855 - Part 2 - Update TaskGetFeatureList to use new Guardian endpoint and scheduling

src/app.cpp

@@ -53,18 +53,6 @@ void App::setUserState(UserState state) {
   }
 }
 
-// static
-QByteArray App::authorizationHeader() {
-  if (SettingsHolder::instance()->token().isEmpty()) {
-    logger.error() << "INVALID TOKEN! This network request is going to fail.";
-    Q_ASSERT(false);
-  }
-
-  QByteArray authorizationHeader = "Bearer ";
-  authorizationHeader.append(SettingsHolder::instance()->token().toLocal8Bit());
-  return authorizationHeader;
-}
-
 void App::quit() {
   logger.debug() << "quit";
   TaskScheduler::forceDeleteTasks();

src/cmake/shared-sources.cmake

@@ -84,6 +84,10 @@ target_sources(shared-sources INTERFACE
     ${CMAKE_SOURCE_DIR}/src/errorhandler.h
     ${CMAKE_SOURCE_DIR}/src/feature/feature.cpp
     ${CMAKE_SOURCE_DIR}/src/feature/feature.h
+    ${CMAKE_SOURCE_DIR}/src/feature/taskgetfeaturelist.cpp
+    ${CMAKE_SOURCE_DIR}/src/feature/taskgetfeaturelist.h
+    ${CMAKE_SOURCE_DIR}/src/feature/taskgetfeaturelistworker.cpp
+    ${CMAKE_SOURCE_DIR}/src/feature/taskgetfeaturelistworker.h
     ${CMAKE_SOURCE_DIR}/src/fontloader.cpp
     ${CMAKE_SOURCE_DIR}/src/fontloader.h
     ${CMAKE_SOURCE_DIR}/src/frontend/navigator.cpp
@@ -181,8 +185,6 @@ target_sources(shared-sources INTERFACE
     ${CMAKE_SOURCE_DIR}/src/tasks/authenticate/taskauthenticate.h
     ${CMAKE_SOURCE_DIR}/src/tasks/deleteaccount/taskdeleteaccount.cpp
     ${CMAKE_SOURCE_DIR}/src/tasks/deleteaccount/taskdeleteaccount.h
-    ${CMAKE_SOURCE_DIR}/src/tasks/getfeaturelist/taskgetfeaturelist.cpp
-    ${CMAKE_SOURCE_DIR}/src/tasks/getfeaturelist/taskgetfeaturelist.h
     ${CMAKE_SOURCE_DIR}/src/tasks/function/taskfunction.cpp
     ${CMAKE_SOURCE_DIR}/src/tasks/function/taskfunction.h
     ${CMAKE_SOURCE_DIR}/src/tasks/group/taskgroup.cpp

src/constants.cpp

@@ -85,7 +85,7 @@ QString Constants::apiUrl(ApiEndpoint endpoint) {
       {ApiEndpoint::Device, "/api/v1/vpn/device"},
       {ApiEndpoint::DeviceWithPublicKeyArgument, "/api/v1/vpn/device/%1"},
       {ApiEndpoint::DNSDetectPortal, "/api/v1/vpn/dns/detectportal"},
-      {ApiEndpoint::FeatureList, "/api/v1/vpn/featurelist"},
+      {ApiEndpoint::FeatureList, "/api/v2/vpn/featurelist"},
       {ApiEndpoint::Heartbeat, "/__heartbeat__"},
       {ApiEndpoint::IPInfo, "/api/v1/vpn/ipinfo"},
       {ApiEndpoint::LoginVerify, "/api/v2/vpn/login/verify"},

src/controller.cpp

@@ -978,7 +978,7 @@ bool Controller::activate(const ServerData& serverData,
     // replicate the behavior of a TaskAccount.
     TaskFunction* task = new TaskFunction([]() {});
     NetworkRequest* request = new NetworkRequest(task, 200);
-    request->auth(App::authorizationHeader());
+    request->auth();
     request->get(Constants::apiUrl(Constants::Account));
 
     connect(request, &NetworkRequest::requestFailed, this,

src/tasks/getfeaturelist/taskgetfeaturelist.cpp → src/feature/taskgetfeaturelist.cpp

@@ -6,12 +6,15 @@
 
 #include <QJsonDocument>
 #include <QJsonObject>
+#include <QUuid>
 
+#include "app.h"
 #include "constants.h"
 #include "feature/featuremodel.h"
 #include "leakdetector.h"
 #include "logger.h"
 #include "networkrequest.h"
+#include "settingsholder.h"
 
 namespace {
 Logger logger("TaskGetFeatureList");
@@ -25,17 +28,34 @@ TaskGetFeatureList::~TaskGetFeatureList() { MZ_COUNT_DTOR(TaskGetFeatureList); }
 
 void TaskGetFeatureList::run() {
   NetworkRequest* request = new NetworkRequest(this, 200);
-  request->get(Constants::apiUrl(Constants::FeatureList));
+
+  QJsonObject body;
+  if (SettingsHolder::instance()->token().isEmpty()) {
+    auto unauthedExperimenterId =
+        SettingsHolder::instance()->unauthedExperimenterId();
+
+    if (unauthedExperimenterId.isEmpty()) {
+      unauthedExperimenterId = QUuid::createUuid().toString();
+      SettingsHolder::instance()->setUnauthedExperimenterId(
+          unauthedExperimenterId);
+    }
+
+    body["experimenterId"] = unauthedExperimenterId;
+  } else {
+    request->auth();
+  }
+
+  request->post(Constants::apiUrl(Constants::FeatureList), body);
 
   connect(request, &NetworkRequest::requestFailed, this,
           [this](QNetworkReply::NetworkError error, const QByteArray&) {
-            logger.error() << "Get feature list is failed" << error;
+            logger.error() << "Get feature list has failed" << error;
             emit completed();
           });
 
   connect(request, &NetworkRequest::requestCompleted, this,
           [this](const QByteArray& data) {
-            logger.debug() << "Get feature list is completed" << data;
+            logger.debug() << "Get feature list completed" << data;
             FeatureModel::instance()->updateFeatureList(data);
             emit completed();
           });

src/tasks/getfeaturelist/taskgetfeaturelist.h → src/feature/taskgetfeaturelist.h

@@ -9,6 +9,21 @@
 
 #include "task.h"
 
+/**
+ * @brief Task to access the Guardian endpoint /featurelist.
+ *
+ * This is a POST endpoint. The request must either include the Authorization
+ * header or the exerimenterId property in the body.
+ *
+ * When the user is logged in, the Authorization header will be included.
+ *
+ * When the user is logged out the value under `unauthedExperimenterId` will be
+ * sent in the body.
+ *
+ * If the `unauthedExperimenterId` settings has never been set, this task
+ * will set it.
+ *
+ */
 class TaskGetFeatureList final : public Task {
   Q_DISABLE_COPY_MOVE(TaskGetFeatureList)
 

src/feature/taskgetfeaturelistworker.cpp

@@ -0,0 +1,41 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+#include "taskgetfeaturelistworker.h"
+
+#include "leakdetector.h"
+#include "settingsholder.h"
+#include "taskgetfeaturelist.h"
+#include "taskscheduler.h"
+
+TaskGetFeatureListWorker::TaskGetFeatureListWorker(QObject* parent)
+    : QObject(parent) {
+  MZ_COUNT_CTOR(TaskGetFeatureListWorker);
+}
+
+TaskGetFeatureListWorker::~TaskGetFeatureListWorker() {
+  MZ_COUNT_DTOR(TaskGetFeatureListWorker);
+
+  qDebug() << "Destructing!";
+}
+
+void TaskGetFeatureListWorker::start(std::chrono::milliseconds interval) {
+  scheduleTask();
+
+  // The `token` setting is the setting that contains the JWT token for authed
+  // users. When it changes it means the user has either logged in or out.
+  //
+  // Experiments are tied to users, so when the user changes or signs out,
+  // the active experiments may also change.
+  connect(SettingsHolder::instance(), &SettingsHolder::tokenChanged, this,
+          &TaskGetFeatureListWorker::scheduleTask);
+  connect(&m_timer, &QTimer::timeout, this,
+          &TaskGetFeatureListWorker::scheduleTask);
+
+  m_timer.start(interval);
+}
+
+void TaskGetFeatureListWorker::scheduleTask() {
+  TaskScheduler::scheduleTask(new TaskGetFeatureList());
+}

src/feature/taskgetfeaturelistworker.h

@@ -0,0 +1,56 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+#ifndef TASKGETFEATURELISTWORKER_H
+#define TASKGETFEATURELISTWORKER_H
+
+#include <QObject>
+#include <QTimer>
+
+/**
+ * @brief Worker responsible for executing TaskGetFeatureList
+ * at the appropriate times.
+ *
+ * What are the appropriate times?
+ *
+ * * On initialization
+ * * Whenever the experimenter id changes -- the feature list depends on it
+ * * Periodically on a specified interval
+ *
+ * What is the experimenter id?
+ *
+ * The experimenter id is an id associated to a user session and used by Cirrus
+ * to determine which experimental features should be enabled for this user.
+ *
+ * On a logged out session of the VPN app, that will be a randomly generated
+ * UUID stored under the unauthedExperimenterId setting.
+ *
+ * On a logged in session of the VPN app, that will be the FxA id which is not
+ * passed in the request in plain text, but is gleaned from the JWT user token
+ * sent to the server through the Authorization header.
+ */
+class TaskGetFeatureListWorker final : public QObject {
+  Q_OBJECT
+
+ public:
+  TaskGetFeatureListWorker(QObject* parent = nullptr);
+  ~TaskGetFeatureListWorker();
+
+  /**
+   * @brief Starts the worker.
+   *
+   * 1. Schedules TaskGetFeatureList to run ASAP
+   * 2. Starts a timer to re-run TaskGetFeatureList on a given interval
+   * 3. Starts observers for re-triggering the task on experimenter id change
+   */
+  void start(std::chrono::milliseconds interval);
+
+ private:
+  void scheduleTask();
+
+ private:
+  QTimer m_timer;
+};
+
+#endif  // TASKGETFEATURELISTWORKER_H

src/glean/mzglean.cpp

@@ -141,16 +141,26 @@ void MZGlean::setUploadEnabled(bool isTelemetryEnabled) {
 
   broadcastUploadEnabledChange(isTelemetryEnabled);
 
-#if defined(MZ_ANDROID) || defined(MZ_IOS)
   if (isTelemetryEnabled) {
+#if defined(MZ_ANDROID) || defined(MZ_IOS)
     // need to reset installation ID, as it would have been cleared
     QString uuid = mozilla::glean::session::installation_id.generateAndSet();
     SettingsHolder::instance()->setInstallationId(uuid);
+#endif
   } else {
-    // clear out the former installation ID immediately
+    // Note: Identifiers should be rotated when telemetry is disabled.
+    //
+    // Whenever telemetry is disabled, that generates a data deletion request
+    // on the backend. If telemtry is re-enabled, all new identifiers are
+    // generated by Glean, so we also rotate our identifiers to comply.
+
+    // Clear out the experimentation ID
+    SettingsHolder::instance()->removeUnauthedExperimenterId();
+#if defined(MZ_ANDROID) || defined(MZ_IOS)
+    // Clear out the former installation ID
     SettingsHolder::instance()->removeInstallationId();
-  }
 #endif
+  }
 }
 
 // static

src/mozillavpn.cpp

@@ -13,6 +13,7 @@
 #include "controller.h"
 #include "dnshelper.h"
 #include "feature/feature.h"
+#include "feature/taskgetfeaturelistworker.h"
 #include "frontend/navigationbarmodel.h"
 #include "frontend/navigator.h"
 #include "glean/generated/metrics.h"
@@ -53,7 +54,6 @@
 #include "tasks/createsupportticket/taskcreatesupportticket.h"
 #include "tasks/deleteaccount/taskdeleteaccount.h"
 #include "tasks/function/taskfunction.h"
-#include "tasks/getfeaturelist/taskgetfeaturelist.h"
 #include "tasks/getlocation/taskgetlocation.h"
 #include "tasks/getsubscriptiondetails/taskgetsubscriptiondetails.h"
 #include "tasks/group/taskgroup.h"
@@ -124,7 +124,7 @@ MozillaVPN::MozillaVPN() : App(nullptr), m_private(new MozillaVPNPrivate()) {
         {new TaskAccount(ErrorHandler::DoNotPropagateError),
          new TaskServers(ErrorHandler::DoNotPropagateError),
          new TaskCaptivePortalLookup(ErrorHandler::DoNotPropagateError),
-         new TaskHeartbeat(), new TaskGetFeatureList(), new TaskAddonIndex(),
+         new TaskHeartbeat(), new TaskAddonIndex(),
          new TaskGetSubscriptionDetails(
              TaskGetSubscriptionDetails::NoAuthenticationFlow,
              ErrorHandler::PropagateError)}));
@@ -248,6 +248,9 @@ void MozillaVPN::initialize() {
   // This is our first state.
   Q_ASSERT(state() == StateInitialize);
 
+  m_private->m_taskGetFeatureListWorker.start(
+      Constants::Timers::schedulePeriodicTask());
+
   m_private->m_releaseMonitor.runSoon();
 
   m_private->m_telemetry.initialize();
@@ -263,7 +266,7 @@ void MozillaVPN::initialize() {
   RecentConnections::instance()->initialize();
   RecommendedLocationModel::instance()->initialize();
 
-  QList<Task*> initTasks{new TaskAddonIndex(), new TaskGetFeatureList()};
+  QList<Task*> initTasks{new TaskAddonIndex()};
 
 #ifdef MZ_ADJUST
   logger.debug() << "Adjust included in build.";

src/mozillavpn_p.h

@@ -9,6 +9,7 @@
 #include "captiveportal/captiveportaldetection.h"
 #include "connectionhealth.h"
 #include "controller.h"
+#include "feature/taskgetfeaturelistworker.h"
 #include "ipaddresslookup.h"
 #include "models/devicemodel.h"
 #include "models/keys.h"
@@ -45,6 +46,7 @@ struct MozillaVPNPrivate {
   ProfileFlow m_profileFlow;
   Telemetry m_telemetry;
   User m_user;
+  TaskGetFeatureListWorker m_taskGetFeatureListWorker;
 };
 
 #endif  // MOZILLAVPN_PRIVATE_H

src/networkrequest.cpp

@@ -55,6 +55,18 @@ void NetworkRequest::setRequestHandler(
   s_postResourceIODeviceCallback = std::move(postResourceIODeviceCallback);
 }
 
+#ifdef UNIT_TEST
+// static
+void NetworkRequest::resetRequestHandler() {
+  std::function<bool(NetworkRequest*)> s_deleteResourceCallback = nullptr;
+  std::function<bool(NetworkRequest*)> s_getResourceCallback = nullptr;
+  std::function<bool(NetworkRequest*, const QByteArray&)>
+      s_postResourceCallback = nullptr;
+  std::function<bool(NetworkRequest*, QIODevice*)>
+      s_postResourceIODeviceCallback = nullptr;
+}
+#endif
+
 NetworkRequest::NetworkRequest(Task* parent, int status)
     : QObject(parent), m_expectedStatusCode(status) {
   MZ_COUNT_CTOR(NetworkRequest);
@@ -99,7 +111,26 @@ NetworkRequest::~NetworkRequest() {
   }
 }
 
+// static
+const QByteArray NetworkRequest::authorizationHeader() {
+  if (SettingsHolder::instance()->token().isEmpty()) {
+    logger.error() << "INVALID TOKEN! This network request is going to fail.";
+    Q_ASSERT(false);
+  }
+
+  QByteArray authorizationHeader = "Bearer ";
+  authorizationHeader.append(SettingsHolder::instance()->token().toLocal8Bit());
+
+  return authorizationHeader;
+}
+
 void NetworkRequest::auth(const QByteArray& authorizationHeader) {
+  auto finalAuthorizationHeader = authorizationHeader;
+
+  if (finalAuthorizationHeader.isEmpty()) {
+    finalAuthorizationHeader = NetworkRequest::authorizationHeader();
+  }
+
   m_request.setRawHeader("Authorization", authorizationHeader);
 }