Network traffic annotation added to OAuth2ApiCallFlow and its subclasses.

components/cryptauth/cryptauth_device_manager.cc

 // Copyright 2015 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "components/cryptauth/cryptauth_device_manager.h"
 
 #include <stddef.h>
 #include <stdexcept>
 #include <utility>
 
 #include "base/base64url.h"
 #include "base/memory/ptr_util.h"
 #include "base/strings/string_number_conversions.h"
 #include "components/cryptauth/cryptauth_client.h"
 #include "components/cryptauth/pref_names.h"
 #include "components/cryptauth/sync_scheduler_impl.h"
 #include "components/prefs/pref_registry_simple.h"
 #include "components/prefs/pref_service.h"
 #include "components/prefs/scoped_user_pref_update.h"
 #include "components/proximity_auth/logging/logging.h"
+#include "net/traffic_annotation/network_traffic_annotation.h"
 
 namespace cryptauth {
 
 namespace {
 
 // The normal period between successful syncs, in hours.
 const int kRefreshPeriodHours = 24;
 
 // A more aggressive period between sync attempts to recover when the last
 // sync attempt fails, in minutes. This is a base time that increases for each
@@ skipping 468 matching lines @@
         PA_LOG(ERROR) << "Unable to deserialize unlock key dictionary "
                       << "(index=" << i << "):\n" << *unlock_key_dictionary;
       }
     } else {
       PA_LOG(ERROR) << "Can not get dictionary in list of unlock keys "
                     << "(index=" << i << "):\n" << *unlock_key_list;
     }
   }
 }
 
 void CryptAuthDeviceManager::OnSyncRequested(

[sacomoto, 2017/06/19 18:25:38]

There are essentially two way this method can be called (and then trigger an
GetMyDevices API call): periodic or forced. In both cases, the only difference
in the annotation should be in the "trigger" value.

For the periodic one, we already have all the information to another the
request. But this is not true for the forced sync. Any user of this class can
call the |ForceSyncNow| method and trigger an API call.

For the forced sync case, I think there are two alternative here:
1) Be a little bit more vague, and set the ""trigger" to forced sync whenever
|invocation_reason != INVOCATION_REASON_PERIODIC && invocation_reason !=
INVOCATION_REASON_INITIALIZATION|.
2) Let the clients of this class set the "trigger" value. Concretely, add a
|trigger| parameter to the |ForceSyncNow| method.

[Ramin Halavati, 2017/06/20 05:35:34]

I think it would be good if we give a general description of both trigger
situations here, like 'Periodically every 24h, or by API request'.
We don't have the required structure to define more than 2 level partial
annotations now, and I don't feel that we need to expand the structures because
of this case as the data and policy are very much the same and it is very
improbable that one would like to stop this request based on a specific trigger
case. But if you believe so, I can start it that way.

[sacomoto, 2017/06/20 09:48:18]

I'm fine with the general description.

[Ramin Halavati, 2017/06/20 10:46:16]

Acknowledged.

     std::unique_ptr<SyncScheduler::SyncRequest> sync_request) {
   for (auto& observer : observers_)
     observer.OnSyncStarted();
 
   sync_request_ = std::move(sync_request);
   cryptauth_client_ = client_factory_->CreateInstance();
 
   InvocationReason invocation_reason =
       INVOCATION_REASON_UNKNOWN;
 
@@ skipping 14 matching lines @@
     invocation_reason = INVOCATION_REASON_INITIALIZATION;
   } else if (IsRecoveringFromFailure()) {
     invocation_reason = INVOCATION_REASON_FAILURE_RECOVERY;
   } else {
     invocation_reason = INVOCATION_REASON_PERIODIC;
   }
 
   GetMyDevicesRequest request;
   request.set_invocation_reason(invocation_reason);
   request.set_allow_stale_read(is_sync_speculative);
+  net::PartialNetworkTrafficAnnotationTag partial_traffic_annotation =
+      net::DefinePartialNetworkTrafficAnnotation("...", "oauth2_api_call_flow",

[sacomoto, 2017/06/20 09:48:18]

cryptauth_get_my_devices

[Ramin Halavati, 2017/06/20 10:46:16]

Done.

+                                                 R"(
+      semantics {
+        sender: "..."

[sacomoto, 2017/06/20 09:48:18]

CryptAuth Device Manager

[Ramin Halavati, 2017/06/20 10:46:16]

Done.

+        description: "..."

[sacomoto, 2017/06/20 09:48:18]

Gets a list of the devices registered (for the same user) on CryptAuth

[Ramin Halavati, 2017/06/20 10:46:16]

Done.

+        trigger: "..."

[sacomoto, 2017/06/20 09:48:18]

Periodically every 24h, or by API request

[Ramin Halavati, 2017/06/20 10:46:16]

Done.

+        data: "..."

[sacomoto, 2017/06/20 09:48:18]

None

[Ramin Halavati, 2017/06/20 10:46:15]

If sign-in is required, shouldn't this be user authentication token?
Also for other requests that are dependent on sign-in.

+        destination: GOOGLE_OWNED_SERVICE
+      }
+      policy {
+        setting: "..."

[sacomoto, 2017/06/20 09:48:18]

This feature cannot be disabled in settings.

[Ramin Halavati, 2017/06/20 10:46:15]

Done.

+        chrome_policy {
+          [POLICY_NAME] {

[sacomoto, 2017/06/20 09:48:18]

SigninAllowed

[Ramin Halavati, 2017/06/20 10:46:16]

Done.

+            policy_options {mode: MANDATORY/RECOMMENDED/UNSET}
+            [POLICY_NAME]: ... //(value to disable it)

[sacomoto, 2017/06/20 09:48:18]

SigninAllowed: false

[Ramin Halavati, 2017/06/20 10:46:15]

Done.

+          }
+        }
+        policy_exception_justification: "..."
+      })");
   cryptauth_client_->GetMyDevices(
-      request, base::Bind(&CryptAuthDeviceManager::OnGetMyDevicesSuccess,
-                          weak_ptr_factory_.GetWeakPtr()),
+      request,
+      base::Bind(&CryptAuthDeviceManager::OnGetMyDevicesSuccess,
+                 weak_ptr_factory_.GetWeakPtr()),
       base::Bind(&CryptAuthDeviceManager::OnGetMyDevicesFailure,
-                 weak_ptr_factory_.GetWeakPtr()));
+                 weak_ptr_factory_.GetWeakPtr()),
+      partial_traffic_annotation);
 }
 
 }  // namespace cryptauth