Replace several members of SecurityManager with pointers to Settings
Change-Id: I408bd5750879bb948cc8b8f6feac30e82e5446a9
Reviewed-on: https://weave-review.googlesource.com/2081
Reviewed-by: Vitaly Buka <vitalybuka@google.com>
diff --git a/src/privet/auth_manager.cc b/src/privet/auth_manager.cc
index 8df7e13..678faf7 100644
--- a/src/privet/auth_manager.cc
+++ b/src/privet/auth_manager.cc
@@ -315,20 +315,5 @@
return result;
}
-bool AuthManager::IsAnonymousAuthSupported() const {
- return !config_ ||
- config_->GetSettings().local_anonymous_access_role != AuthScope::kNone;
-}
-
-bool AuthManager::IsPairingAuthSupported() const {
- return !config_ || config_->GetSettings().local_pairing_enabled;
-}
-
-bool AuthManager::IsLocalAuthSupported() const {
- return !config_ ||
- config_->GetSettings().root_client_token_owner !=
- RootClientTokenOwner::kNone;
-}
-
} // namespace privet
} // namespace weave
diff --git a/src/privet/auth_manager.h b/src/privet/auth_manager.h
index 6a403b4..309d80e 100644
--- a/src/privet/auth_manager.h
+++ b/src/privet/auth_manager.h
@@ -69,10 +69,6 @@
std::vector<uint8_t> CreateSessionId();
- bool IsAnonymousAuthSupported() const;
- bool IsPairingAuthSupported() const;
- bool IsLocalAuthSupported() const;
-
private:
Config* config_{nullptr}; // Can be nullptr for tests.
base::DefaultClock default_clock_;
diff --git a/src/privet/privet_manager.cc b/src/privet/privet_manager.cc
index c3f3885..edc7907 100644
--- a/src/privet/privet_manager.cc
+++ b/src/privet/privet_manager.cc
@@ -62,9 +62,8 @@
CloudDelegate::CreateDefault(task_runner_, device, component_manager);
cloud_observer_.Add(cloud_.get());
- security_.reset(new SecurityManager(
- auth_manager, device->GetSettings().pairing_modes,
- device->GetSettings().embedded_code, disable_security_, task_runner_));
+ security_.reset(new SecurityManager(device->GetMutableConfig(), auth_manager,
+ task_runner_));
network->AddConnectionChangedCallback(
base::Bind(&Manager::OnConnectivityChanged, base::Unretained(this)));
diff --git a/src/privet/security_manager.cc b/src/privet/security_manager.cc
index caed476..4cd9276 100644
--- a/src/privet/security_manager.cc
+++ b/src/privet/security_manager.cc
@@ -18,7 +18,6 @@
#include <base/time/time.h>
#include <weave/provider/task_runner.h>
-#include "src/config.h"
#include "src/data_encoding.h"
#include "src/privet/auth_manager.h"
#include "src/privet/constants.h"
@@ -90,20 +89,16 @@
} // namespace
-SecurityManager::SecurityManager(AuthManager* auth_manager,
- const std::set<PairingType>& pairing_modes,
- const std::string& embedded_code,
- bool disable_security,
+SecurityManager::SecurityManager(const Config* config,
+ AuthManager* auth_manager,
provider::TaskRunner* task_runner)
- : auth_manager_{auth_manager},
- is_security_disabled_(disable_security),
- pairing_modes_(pairing_modes),
- embedded_code_(embedded_code),
- task_runner_{task_runner} {
+ : config_{config}, auth_manager_{auth_manager}, task_runner_{task_runner} {
CHECK(auth_manager_);
- CHECK_EQ(embedded_code_.empty(),
- std::find(pairing_modes_.begin(), pairing_modes_.end(),
- PairingType::kEmbeddedCode) == pairing_modes_.end());
+ CHECK_EQ(GetSettings().embedded_code.empty(),
+ std::find(GetSettings().pairing_modes.begin(),
+ GetSettings().pairing_modes.end(),
+ PairingType::kEmbeddedCode) ==
+ GetSettings().pairing_modes.end());
}
SecurityManager::~SecurityManager() {
@@ -151,12 +146,12 @@
switch (auth_type) {
case AuthType::kAnonymous:
- if (!auth_manager_->IsAnonymousAuthSupported())
+ if (!IsAnonymousAuthSupported())
return disabled_mode(error);
return CreateAccessTokenImpl(auth_type, desired_scope, access_token,
access_token_scope, access_token_ttl);
case AuthType::kPairing:
- if (!auth_manager_->IsPairingAuthSupported())
+ if (!IsPairingAuthSupported())
return disabled_mode(error);
if (!IsValidPairingCode(auth_code)) {
Error::AddTo(error, FROM_HERE, errors::kDomain,
@@ -166,7 +161,7 @@
return CreateAccessTokenImpl(auth_type, desired_scope, access_token,
access_token_scope, access_token_ttl);
case AuthType::kLocal:
- if (!auth_manager_->IsLocalAuthSupported())
+ if (!IsLocalAuthSupported())
return disabled_mode(error);
const base::TimeDelta kTtl =
base::TimeDelta::FromSeconds(kAccessTokenExpirationSeconds);
@@ -221,25 +216,25 @@
}
std::set<PairingType> SecurityManager::GetPairingTypes() const {
- return pairing_modes_;
+ return GetSettings().pairing_modes;
}
std::set<CryptoType> SecurityManager::GetCryptoTypes() const {
std::set<CryptoType> result{CryptoType::kSpake_p224};
- if (is_security_disabled_)
+ if (GetSettings().disable_security)
result.insert(CryptoType::kNone);
return result;
}
std::set<AuthType> SecurityManager::GetAuthTypes() const {
std::set<AuthType> result;
- if (auth_manager_->IsAnonymousAuthSupported())
+ if (IsAnonymousAuthSupported())
result.insert(AuthType::kAnonymous);
- if (auth_manager_->IsPairingAuthSupported())
+ if (IsPairingAuthSupported())
result.insert(AuthType::kPairing);
- if (auth_manager_->IsLocalAuthSupported())
+ if (IsLocalAuthSupported())
result.insert(AuthType::kLocal);
return result;
@@ -262,9 +257,13 @@
return auth_manager_->ConfirmClientAuthToken(token_decoded, error);
}
+const Config::Settings& SecurityManager::GetSettings() const {
+ return config_->GetSettings();
+}
+
bool SecurityManager::IsValidPairingCode(
const std::vector<uint8_t>& auth_code) const {
- if (is_security_disabled_)
+ if (GetSettings().disable_security)
return true;
for (const auto& session : confirmed_sessions_) {
const std::string& key = session.second->GetKey();
@@ -288,8 +287,9 @@
if (!CheckIfPairingAllowed(error))
return false;
- if (std::find(pairing_modes_.begin(), pairing_modes_.end(), mode) ==
- pairing_modes_.end()) {
+ const auto& pairing_modes = GetSettings().pairing_modes;
+ if (std::find(pairing_modes.begin(), pairing_modes.end(), mode) ==
+ pairing_modes.end()) {
Error::AddTo(error, FROM_HERE, errors::kDomain, errors::kInvalidParams,
"Pairing mode is not enabled");
return false;
@@ -298,8 +298,8 @@
std::string code;
switch (mode) {
case PairingType::kEmbeddedCode:
- CHECK(!embedded_code_.empty());
- code = embedded_code_;
+ CHECK(!GetSettings().embedded_code.empty());
+ code = GetSettings().embedded_code;
break;
case PairingType::kPinCode:
code = base::StringPrintf("%04i", base::RandInt(0, 9999));
@@ -316,7 +316,7 @@
spake.reset(new Spakep224Exchanger(code));
break;
case CryptoType::kNone:
- if (is_security_disabled_) {
+ if (GetSettings().disable_security) {
spake.reset(new UnsecureKeyExchanger(code));
break;
}
@@ -437,7 +437,7 @@
}
bool SecurityManager::CheckIfPairingAllowed(ErrorPtr* error) {
- if (is_security_disabled_)
+ if (GetSettings().disable_security)
return true;
if (block_pairing_until_ > auth_manager_->Now()) {
@@ -471,5 +471,17 @@
return confirmed_sessions_.erase(session_id) != 0;
}
+bool SecurityManager::IsAnonymousAuthSupported() const {
+ return GetSettings().local_anonymous_access_role != AuthScope::kNone;
+}
+
+bool SecurityManager::IsPairingAuthSupported() const {
+ return GetSettings().local_pairing_enabled;
+}
+
+bool SecurityManager::IsLocalAuthSupported() const {
+ return GetSettings().root_client_token_owner != RootClientTokenOwner::kNone;
+}
+
} // namespace privet
} // namespace weave
diff --git a/src/privet/security_manager.h b/src/privet/security_manager.h
index 79abf33..651089a 100644
--- a/src/privet/security_manager.h
+++ b/src/privet/security_manager.h
@@ -16,6 +16,7 @@
#include <base/memory/weak_ptr.h>
#include <weave/error.h>
+#include "src/config.h"
#include "src/privet/security_delegate.h"
namespace crypto {
@@ -51,10 +52,8 @@
virtual const std::string& GetKey() const = 0;
};
- SecurityManager(AuthManager* auth_manager,
- const std::set<PairingType>& pairing_modes,
- const std::string& embedded_code,
- bool disable_security,
+ SecurityManager(const Config* config,
+ AuthManager* auth_manager,
// TODO(vitalybuka): Remove task_runner.
provider::TaskRunner* task_runner);
~SecurityManager() override;
@@ -94,6 +93,7 @@
const PairingEndListener& on_end);
private:
+ const Config::Settings& GetSettings() const;
bool IsValidPairingCode(const std::vector<uint8_t>& auth_code) const;
FRIEND_TEST_ALL_PREFIXES(SecurityManagerTest, ThrottlePairing);
// Allows limited number of new sessions without successful authorization.
@@ -112,13 +112,13 @@
std::vector<uint8_t>* access_token,
AuthScope* access_token_scope,
base::TimeDelta* access_token_ttl);
+ bool IsAnonymousAuthSupported() const;
+ bool IsPairingAuthSupported() const;
+ bool IsLocalAuthSupported() const;
+ const Config* config_{nullptr};
AuthManager* auth_manager_{nullptr};
- // If true allows unencrypted pairing and accepts any access code.
- const bool is_security_disabled_{false};
- const std::set<PairingType> pairing_modes_;
- const std::string embedded_code_;
// TODO(vitalybuka): Session cleanup can be done without posting tasks.
provider::TaskRunner* task_runner_{nullptr};
std::map<std::string, std::unique_ptr<KeyExchanger>> pending_sessions_;
diff --git a/src/privet/security_manager_unittest.cc b/src/privet/security_manager_unittest.cc
index 3c15d7e..43b7f00 100644
--- a/src/privet/security_manager_unittest.cc
+++ b/src/privet/security_manager_unittest.cc
@@ -20,7 +20,9 @@
#include <gmock/gmock.h>
#include <gtest/gtest.h>
#include <weave/provider/test/fake_task_runner.h>
+#include <weave/provider/test/mock_config_store.h>
+#include "src/config.h"
#include "src/data_encoding.h"
#include "src/privet/auth_manager.h"
#include "src/privet/openssl_utils.h"
@@ -57,6 +59,25 @@
} // namespace
+class SecurityManagerConfigStore : public provider::test::MockConfigStore {
+ public:
+ SecurityManagerConfigStore() {
+ EXPECT_CALL(*this, LoadDefaults(_))
+ .WillRepeatedly(testing::Invoke([](Settings* settings) {
+ settings->embedded_code = "1234";
+ settings->pairing_modes = {PairingType::kEmbeddedCode};
+ settings->client_id = "TEST_CLIENT_ID";
+ settings->client_secret = "TEST_CLIENT_SECRET";
+ settings->api_key = "TEST_API_KEY";
+ settings->oem_name = "TEST_OEM";
+ settings->model_name = "TEST_MODEL";
+ settings->model_id = "ABCDE";
+ settings->name = "TEST_NAME";
+ return true;
+ }));
+ }
+};
+
class SecurityManagerTest : public testing::Test {
protected:
void SetUp() override {
@@ -113,6 +134,8 @@
const base::Time time_ = base::Time::FromTimeT(1410000000);
provider::test::FakeTaskRunner task_runner_;
test::MockClock clock_;
+ SecurityManagerConfigStore config_store_;
+ Config config_{&config_store_};
AuthManager auth_manager_{
{22, 47, 23, 77, 42, 98, 96, 25, 83, 16, 9, 14, 91, 44, 15, 75, 60, 62,
10, 18, 82, 35, 88, 100, 30, 45, 7, 46, 67, 84, 58, 85},
@@ -124,11 +147,8 @@
{22, 47, 23, 77, 42, 98, 96, 25, 83, 16, 9, 14, 91, 44, 15, 75, 60, 62,
10, 18, 82, 35, 88, 100, 30, 45, 7, 46, 67, 84, 58, 85},
&clock_};
- SecurityManager security_{&auth_manager_,
- {PairingType::kEmbeddedCode},
- "1234",
- false,
- &task_runner_};
+
+ SecurityManager security_{&config_, &auth_manager_, &task_runner_};
};
TEST_F(SecurityManagerTest, AccessToken) {
