Commit edc5b05c authored by Josh Nohle's avatar Josh Nohle Committed by Commit Bot

[DeviceSync v2] Do not bundle metadata-syncer results

To conform with the style of other DeviceSync v2 helper classes, send
the ResultCode and the new ClientDirective (if one was sent) as
individual callback arguments instead of bundling them in a
CryptAuthDeviceSyncResult. We reserve the CryptAuthDeviceSyncResult
to express the *final* result of the v2 DeviceSync flow.

This is a stylistic clean-up; no logic is affected.

Bug: 951969
Change-Id: I9cf32d1d2692c04721ccdb98da890db54b833739
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1778807Reviewed-by: default avatarKyle Horimoto <khorimoto@chromium.org>
Commit-Queue: Josh Nohle <nohle@chromium.org>
Cr-Commit-Position: refs/heads/master@{#692267}
parent fa0b4da0
...@@ -259,12 +259,13 @@ void CryptAuthDeviceSyncerImpl::OnSyncMetadataFinished( ...@@ -259,12 +259,13 @@ void CryptAuthDeviceSyncerImpl::OnSyncMetadataFinished(
std::unique_ptr<CryptAuthKey> new_group_key, std::unique_ptr<CryptAuthKey> new_group_key,
const base::Optional<cryptauthv2::EncryptedGroupPrivateKey>& const base::Optional<cryptauthv2::EncryptedGroupPrivateKey>&
encrypted_group_private_key, encrypted_group_private_key,
const CryptAuthDeviceSyncResult& device_sync_result) { const base::Optional<cryptauthv2::ClientDirective>& new_client_directive,
CryptAuthDeviceSyncResult::ResultCode device_sync_result_code) {
DCHECK_EQ(State::kWaitingForMetadataSync, state_); DCHECK_EQ(State::kWaitingForMetadataSync, state_);
id_to_device_metadata_packet_map_ = id_to_device_metadata_packet_map; id_to_device_metadata_packet_map_ = id_to_device_metadata_packet_map;
encrypted_group_private_key_ = encrypted_group_private_key; encrypted_group_private_key_ = encrypted_group_private_key;
new_client_directive_ = device_sync_result.client_directive(); new_client_directive_ = new_client_directive;
// If a new group key pair was created or if CryptAuth returned a new group // If a new group key pair was created or if CryptAuth returned a new group
// public key during the metadata sync, add the new group key to the key // public key during the metadata sync, add the new group key to the key
...@@ -272,7 +273,7 @@ void CryptAuthDeviceSyncerImpl::OnSyncMetadataFinished( ...@@ -272,7 +273,7 @@ void CryptAuthDeviceSyncerImpl::OnSyncMetadataFinished(
if (new_group_key) if (new_group_key)
SetGroupKey(*new_group_key); SetGroupKey(*new_group_key);
switch (device_sync_result.GetResultType()) { switch (CryptAuthDeviceSyncResult::GetResultType(device_sync_result_code)) {
case CryptAuthDeviceSyncResult::ResultType::kNonFatalError: case CryptAuthDeviceSyncResult::ResultType::kNonFatalError:
did_non_fatal_error_occur_ = true; did_non_fatal_error_occur_ = true;
FALLTHROUGH; FALLTHROUGH;
...@@ -289,7 +290,7 @@ void CryptAuthDeviceSyncerImpl::OnSyncMetadataFinished( ...@@ -289,7 +290,7 @@ void CryptAuthDeviceSyncerImpl::OnSyncMetadataFinished(
AttemptNextStep(); AttemptNextStep();
return; return;
case CryptAuthDeviceSyncResult::ResultType::kFatalError: case CryptAuthDeviceSyncResult::ResultType::kFatalError:
FinishAttempt(device_sync_result.result_code()); FinishAttempt(device_sync_result_code);
return; return;
} }
} }
......
...@@ -119,7 +119,8 @@ class CryptAuthDeviceSyncerImpl : public CryptAuthDeviceSyncer { ...@@ -119,7 +119,8 @@ class CryptAuthDeviceSyncerImpl : public CryptAuthDeviceSyncer {
std::unique_ptr<CryptAuthKey> new_group_key, std::unique_ptr<CryptAuthKey> new_group_key,
const base::Optional<cryptauthv2::EncryptedGroupPrivateKey>& const base::Optional<cryptauthv2::EncryptedGroupPrivateKey>&
encrypted_group_private_key, encrypted_group_private_key,
const CryptAuthDeviceSyncResult& device_sync_result); const base::Optional<cryptauthv2::ClientDirective>& new_client_directive,
CryptAuthDeviceSyncResult::ResultCode device_sync_result_code);
void SetGroupKey(const CryptAuthKey& new_group_key); void SetGroupKey(const CryptAuthKey& new_group_key);
......
...@@ -239,10 +239,7 @@ class DeviceSyncCryptAuthDeviceSyncerImplTest : public testing::Test { ...@@ -239,10 +239,7 @@ class DeviceSyncCryptAuthDeviceSyncerImplTest : public testing::Test {
: nullptr; : nullptr;
metadata_syncer()->FinishAttempt( metadata_syncer()->FinishAttempt(
id_to_device_metadata_packet_map, std::move(new_group_key_ptr), id_to_device_metadata_packet_map, std::move(new_group_key_ptr),
private_key, private_key, new_client_directive, device_sync_result_code);
CryptAuthDeviceSyncResult(device_sync_result_code,
false /* did_device_registry_change */,
new_client_directive));
} }
void VerifyFeatureStatusGetterInput( void VerifyFeatureStatusGetterInput(
......
...@@ -6,8 +6,6 @@ ...@@ -6,8 +6,6 @@
#include <utility> #include <utility>
#include "chromeos/services/device_sync/cryptauth_device_sync_result.h"
namespace chromeos { namespace chromeos {
namespace device_sync { namespace device_sync {
...@@ -35,11 +33,13 @@ void CryptAuthMetadataSyncer::OnAttemptFinished( ...@@ -35,11 +33,13 @@ void CryptAuthMetadataSyncer::OnAttemptFinished(
std::unique_ptr<CryptAuthKey> new_group_key, std::unique_ptr<CryptAuthKey> new_group_key,
const base::Optional<cryptauthv2::EncryptedGroupPrivateKey>& const base::Optional<cryptauthv2::EncryptedGroupPrivateKey>&
encrypted_group_private_key, encrypted_group_private_key,
const CryptAuthDeviceSyncResult& device_sync_result) { const base::Optional<cryptauthv2::ClientDirective>& new_client_directive,
CryptAuthDeviceSyncResult::ResultCode device_sync_result_code) {
DCHECK(callback_); DCHECK(callback_);
std::move(callback_).Run(id_to_device_metadata_packet_map, std::move(callback_).Run(id_to_device_metadata_packet_map,
std::move(new_group_key), std::move(new_group_key),
encrypted_group_private_key, device_sync_result); encrypted_group_private_key, new_client_directive,
device_sync_result_code);
} }
} // namespace device_sync } // namespace device_sync
......
...@@ -11,8 +11,10 @@ ...@@ -11,8 +11,10 @@
#include "base/containers/flat_map.h" #include "base/containers/flat_map.h"
#include "base/macros.h" #include "base/macros.h"
#include "base/optional.h" #include "base/optional.h"
#include "chromeos/services/device_sync/cryptauth_device_sync_result.h"
#include "chromeos/services/device_sync/cryptauth_key.h" #include "chromeos/services/device_sync/cryptauth_key.h"
#include "chromeos/services/device_sync/proto/cryptauth_devicesync.pb.h" #include "chromeos/services/device_sync/proto/cryptauth_devicesync.pb.h"
#include "chromeos/services/device_sync/proto/cryptauth_directive.pb.h"
namespace cryptauthv2 { namespace cryptauthv2 {
class BetterTogetherDeviceMetadata; class BetterTogetherDeviceMetadata;
...@@ -22,8 +24,6 @@ namespace chromeos { ...@@ -22,8 +24,6 @@ namespace chromeos {
namespace device_sync { namespace device_sync {
class CryptAuthDeviceSyncResult;
// Handles the SyncMetadata portion of the CryptAuth v2 DeviceSync protocol, // Handles the SyncMetadata portion of the CryptAuth v2 DeviceSync protocol,
// which consists of one or two SyncMetadata request/response interactions with // which consists of one or two SyncMetadata request/response interactions with
// the CryptAuth servers: // the CryptAuth servers:
...@@ -63,6 +63,7 @@ class CryptAuthDeviceSyncResult; ...@@ -63,6 +63,7 @@ class CryptAuthDeviceSyncResult;
// created. // created.
// - (Optional) A group private key, encrypted with this device's // - (Optional) A group private key, encrypted with this device's
// CryptAuthKeyBunde::kDeviceSyncBetterTogether public key. // CryptAuthKeyBunde::kDeviceSyncBetterTogether public key.
// - (Optional) A new ClientDirective sent from the CryptAuth server.
// //
// A CryptAuthMetadataSyncer object is designed to be used for only one // A CryptAuthMetadataSyncer object is designed to be used for only one
// SyncMetadata() call. For a new DeviceSync attempt, a new object should be // SyncMetadata() call. For a new DeviceSync attempt, a new object should be
...@@ -75,7 +76,8 @@ class CryptAuthMetadataSyncer { ...@@ -75,7 +76,8 @@ class CryptAuthMetadataSyncer {
const IdToDeviceMetadataPacketMap&, const IdToDeviceMetadataPacketMap&,
std::unique_ptr<CryptAuthKey>, std::unique_ptr<CryptAuthKey>,
const base::Optional<cryptauthv2::EncryptedGroupPrivateKey>&, const base::Optional<cryptauthv2::EncryptedGroupPrivateKey>&,
const CryptAuthDeviceSyncResult&)>; const base::Optional<cryptauthv2::ClientDirective>&,
CryptAuthDeviceSyncResult::ResultCode)>;
virtual ~CryptAuthMetadataSyncer(); virtual ~CryptAuthMetadataSyncer();
...@@ -99,7 +101,8 @@ class CryptAuthMetadataSyncer { ...@@ -99,7 +101,8 @@ class CryptAuthMetadataSyncer {
std::unique_ptr<CryptAuthKey> new_group_key, std::unique_ptr<CryptAuthKey> new_group_key,
const base::Optional<cryptauthv2::EncryptedGroupPrivateKey>& const base::Optional<cryptauthv2::EncryptedGroupPrivateKey>&
encrypted_group_private_key, encrypted_group_private_key,
const CryptAuthDeviceSyncResult& device_sync_result); const base::Optional<cryptauthv2::ClientDirective>& new_client_directive,
CryptAuthDeviceSyncResult::ResultCode device_sync_result_code);
private: private:
SyncMetadataAttemptFinishedCallback callback_; SyncMetadataAttemptFinishedCallback callback_;
......
...@@ -533,9 +533,7 @@ void CryptAuthMetadataSyncerImpl::FinishAttempt( ...@@ -533,9 +533,7 @@ void CryptAuthMetadataSyncerImpl::FinishAttempt(
OnAttemptFinished(id_to_device_metadata_packet_map_, OnAttemptFinished(id_to_device_metadata_packet_map_,
std::move(new_group_key_), encrypted_group_private_key, std::move(new_group_key_), encrypted_group_private_key,
CryptAuthDeviceSyncResult( new_client_directive, result_code);
result_code, false /* did_device_registry_change */,
new_client_directive));
} }
std::ostream& operator<<(std::ostream& stream, std::ostream& operator<<(std::ostream& stream,
......
...@@ -17,13 +17,15 @@ void FakeCryptAuthMetadataSyncer::FinishAttempt( ...@@ -17,13 +17,15 @@ void FakeCryptAuthMetadataSyncer::FinishAttempt(
std::unique_ptr<CryptAuthKey> new_group_key, std::unique_ptr<CryptAuthKey> new_group_key,
const base::Optional<cryptauthv2::EncryptedGroupPrivateKey>& const base::Optional<cryptauthv2::EncryptedGroupPrivateKey>&
encrypted_group_private_key, encrypted_group_private_key,
const CryptAuthDeviceSyncResult& device_sync_result) { const base::Optional<cryptauthv2::ClientDirective>& new_client_directive,
CryptAuthDeviceSyncResult::ResultCode device_sync_result_code) {
DCHECK(request_context_); DCHECK(request_context_);
DCHECK(local_device_metadata_); DCHECK(local_device_metadata_);
DCHECK(initial_group_key_); DCHECK(initial_group_key_);
OnAttemptFinished(id_to_device_metadata_packet_map, std::move(new_group_key), OnAttemptFinished(id_to_device_metadata_packet_map, std::move(new_group_key),
encrypted_group_private_key, device_sync_result); encrypted_group_private_key, new_client_directive,
device_sync_result_code);
} }
void FakeCryptAuthMetadataSyncer::OnAttemptStarted( void FakeCryptAuthMetadataSyncer::OnAttemptStarted(
......
...@@ -11,17 +11,18 @@ ...@@ -11,17 +11,18 @@
#include "base/macros.h" #include "base/macros.h"
#include "base/optional.h" #include "base/optional.h"
#include "base/timer/timer.h" #include "base/timer/timer.h"
#include "chromeos/services/device_sync/cryptauth_device_sync_result.h"
#include "chromeos/services/device_sync/cryptauth_metadata_syncer.h" #include "chromeos/services/device_sync/cryptauth_metadata_syncer.h"
#include "chromeos/services/device_sync/cryptauth_metadata_syncer_impl.h" #include "chromeos/services/device_sync/cryptauth_metadata_syncer_impl.h"
#include "chromeos/services/device_sync/proto/cryptauth_better_together_device_metadata.pb.h" #include "chromeos/services/device_sync/proto/cryptauth_better_together_device_metadata.pb.h"
#include "chromeos/services/device_sync/proto/cryptauth_devicesync.pb.h" #include "chromeos/services/device_sync/proto/cryptauth_devicesync.pb.h"
#include "chromeos/services/device_sync/proto/cryptauth_directive.pb.h"
namespace chromeos { namespace chromeos {
namespace device_sync { namespace device_sync {
class CryptAuthClientFactory; class CryptAuthClientFactory;
class CryptAuthDeviceSyncResult;
class CryptAuthKey; class CryptAuthKey;
class FakeCryptAuthMetadataSyncer : public CryptAuthMetadataSyncer { class FakeCryptAuthMetadataSyncer : public CryptAuthMetadataSyncer {
...@@ -53,7 +54,8 @@ class FakeCryptAuthMetadataSyncer : public CryptAuthMetadataSyncer { ...@@ -53,7 +54,8 @@ class FakeCryptAuthMetadataSyncer : public CryptAuthMetadataSyncer {
std::unique_ptr<CryptAuthKey> new_group_key, std::unique_ptr<CryptAuthKey> new_group_key,
const base::Optional<cryptauthv2::EncryptedGroupPrivateKey>& const base::Optional<cryptauthv2::EncryptedGroupPrivateKey>&
encrypted_group_private_key, encrypted_group_private_key,
const CryptAuthDeviceSyncResult& device_sync_result); const base::Optional<cryptauthv2::ClientDirective>& new_client_directive,
CryptAuthDeviceSyncResult::ResultCode device_sync_result_code);
private: private:
// CryptAuthMetadataSyncer: // CryptAuthMetadataSyncer:
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment