Move most of auth logic into SecurityDelegate::CreateAccessToken With local auth we will need to extract most of information from macaroon auth code. BUG=25768507 Change-Id: If7b31a1ba9a081dfae0cf8e9df6c8ed27bfe79c4 Reviewed-on: https://weave-review.googlesource.com/2049 Reviewed-by: Vitaly Buka <vitalybuka@google.com>
diff --git a/src/privet/mock_delegates.h b/src/privet/mock_delegates.h index bc7ec93..1586846 100644 --- a/src/privet/mock_delegates.h +++ b/src/privet/mock_delegates.h
@@ -62,8 +62,14 @@ class MockSecurityDelegate : public SecurityDelegate { public: - MOCK_CONST_METHOD2(CreateAccessToken, - std::string(const UserInfo&, base::TimeDelta)); + MOCK_METHOD7(CreateAccessToken, + bool(AuthType, + const std::string&, + AuthScope, + std::string*, + AuthScope*, + base::TimeDelta*, + ErrorPtr*)); MOCK_CONST_METHOD3(ParseAccessToken, bool(const std::string&, UserInfo*, ErrorPtr*)); MOCK_CONST_METHOD0(GetPairingTypes, std::set<PairingType>()); @@ -84,8 +90,11 @@ MOCK_METHOD0(CreateSessionId, std::string()); MockSecurityDelegate() { - EXPECT_CALL(*this, CreateAccessToken(_, _)) - .WillRepeatedly(Return("GuestAccessToken")); + EXPECT_CALL(*this, CreateAccessToken(_, _, _, _, _, _, _)) + .WillRepeatedly(DoAll( + SetArgPointee<3>("GuestAccessToken"), + SetArgPointee<4>(AuthScope::kViewer), + SetArgPointee<5>(base::TimeDelta::FromSeconds(15)), Return(true))); EXPECT_CALL(*this, ClaimRootClientAuthToken(_)) .WillRepeatedly(Return("RootClientAuthToken"));
diff --git a/src/privet/privet_handler.cc b/src/privet/privet_handler.cc index b55053d..ef0c54e 100644 --- a/src/privet/privet_handler.cc +++ b/src/privet/privet_handler.cc
@@ -116,8 +116,6 @@ const char kInvalidParamValueFormat[] = "Invalid parameter: '%s'='%s'"; -const int kAccessTokenExpirationSeconds = 3600; - template <class Container> std::unique_ptr<base::ListValue> ToValue(const Container& list) { std::unique_ptr<base::ListValue> value_list(new base::ListValue()); @@ -151,14 +149,6 @@ {errors::kAlreadyClaimed, http::kDenied}, }; -AuthScope AuthScopeFromString(const std::string& scope, AuthScope auto_scope) { - if (scope == kAuthScopeAutoValue) - return auto_scope; - AuthScope scope_id = AuthScope::kNone; - StringToEnum(scope, &scope_id); - return scope_id; -} - std::string GetAuthTokenFromAuthHeader(const std::string& auth_header) { return SplitAtFirst(auth_header, " ", true).second; } @@ -684,58 +674,49 @@ return ReturnError(*error, callback); } - std::string auth_code; - input.GetString(kAuthCodeKey, &auth_code); - - AuthScope max_auth_scope = AuthScope::kNone; - switch (auth_type) { - case AuthType::kAnonymous: - max_auth_scope = GetAnonymousMaxScope(*cloud_, wifi_); - break; - case AuthType::kPairing: - if (!security_->IsValidPairingCode(auth_code)) { - Error::AddToPrintf(&error, FROM_HERE, errors::kDomain, - errors::kInvalidAuthCode, kInvalidParamValueFormat, - kAuthCodeKey, auth_code.c_str()); - return ReturnError(*error, callback); - } - max_auth_scope = AuthScope::kOwner; - break; - default: - Error::AddToPrintf(&error, FROM_HERE, errors::kDomain, - errors::kInvalidAuthMode, kInvalidParamValueFormat, - kAuthModeKey, auth_code_type.c_str()); - return ReturnError(*error, callback); - } + AuthScope desired_scope = AuthScope::kOwner; + AuthScope acceptable_scope = AuthScope::kViewer; std::string requested_scope; input.GetString(kAuthRequestedScopeKey, &requested_scope); + if (requested_scope != kAuthScopeAutoValue) { + if (!StringToEnum(requested_scope, &desired_scope)) { + Error::AddToPrintf(&error, FROM_HERE, errors::kDomain, + errors::kInvalidRequestedScope, + kInvalidParamValueFormat, kAuthRequestedScopeKey, + requested_scope.c_str()); + return ReturnError(*error, callback); + } + acceptable_scope = std::max(desired_scope, acceptable_scope); + } - AuthScope requested_auth_scope = - AuthScopeFromString(requested_scope, max_auth_scope); - if (requested_auth_scope == AuthScope::kNone) { - Error::AddToPrintf(&error, FROM_HERE, errors::kDomain, - errors::kInvalidRequestedScope, kInvalidParamValueFormat, - kAuthRequestedScopeKey, requested_scope.c_str()); + if (auth_type == AuthType::kAnonymous) + desired_scope = GetAnonymousMaxScope(*cloud_, wifi_); + + std::string auth_code; + input.GetString(kAuthCodeKey, &auth_code); + + std::string access_token; + base::TimeDelta access_token_ttl; + AuthScope access_token_scope = AuthScope::kNone; + if (!security_->CreateAccessToken(auth_type, auth_code, desired_scope, + &access_token, &access_token_scope, + &access_token_ttl, &error)) { return ReturnError(*error, callback); } - if (requested_auth_scope > max_auth_scope) { + if (access_token_scope < acceptable_scope) { Error::AddToPrintf(&error, FROM_HERE, errors::kDomain, errors::kAccessDenied, "Scope '%s' is not allowed", - EnumToString(requested_auth_scope).c_str()); + EnumToString(access_token_scope).c_str()); return ReturnError(*error, callback); } base::DictionaryValue output; - output.SetString( - kAuthAccessTokenKey, - security_->CreateAccessToken( - UserInfo{requested_auth_scope, ++last_user_id_}, - base::TimeDelta::FromSeconds(kAccessTokenExpirationSeconds))); + output.SetString(kAuthAccessTokenKey, access_token); output.SetString(kAuthTokenTypeKey, kAuthorizationHeaderPrefix); - output.SetInteger(kAuthExpiresInKey, kAccessTokenExpirationSeconds); - output.SetString(kAuthScopeKey, EnumToString(requested_auth_scope)); + output.SetInteger(kAuthExpiresInKey, access_token_ttl.InSeconds()); + output.SetString(kAuthScopeKey, EnumToString(access_token_scope)); callback.Run(http::kOk, output); }
diff --git a/src/privet/privet_handler.h b/src/privet/privet_handler.h index 646cab2..4eba329 100644 --- a/src/privet/privet_handler.h +++ b/src/privet/privet_handler.h
@@ -162,7 +162,6 @@ std::vector<UpdateRequestParameters> update_requests_; int last_update_request_id_{0}; - uint64_t last_user_id_{0}; uint64_t state_fingerprint_{1}; uint64_t traits_fingerprint_{1}; uint64_t components_fingerprint_{1};
diff --git a/src/privet/privet_handler_unittest.cc b/src/privet/privet_handler_unittest.cc index 53f1a05..7c9cf33 100644 --- a/src/privet/privet_handler_unittest.cc +++ b/src/privet/privet_handler_unittest.cc
@@ -377,8 +377,11 @@ } TEST_F(PrivetHandlerTest, AuthErrorInvalidAuthCode) { - EXPECT_CALL(security_, IsValidPairingCode("testToken")) - .WillRepeatedly(Return(false)); + auto set_error = [](ErrorPtr* error) { + Error::AddTo(error, FROM_HERE, errors::kDomain, "invalidAuthCode", ""); + }; + EXPECT_CALL(security_, CreateAccessToken(_, "testToken", _, _, _, _, _)) + .WillRepeatedly(DoAll(WithArgs<6>(Invoke(set_error)), Return(false))); const char kInput[] = R"({ 'mode': 'pairing', 'requestedScope': 'user', @@ -391,8 +394,8 @@ TEST_F(PrivetHandlerTest, AuthAnonymous) { const char kExpected[] = R"({ 'accessToken': 'GuestAccessToken', - 'expiresIn': 3600, - 'scope': 'user', + 'expiresIn': 15, + 'scope': 'viewer', 'tokenType': 'Privet' })"; EXPECT_JSON_EQ(kExpected, @@ -403,8 +406,11 @@ TEST_F(PrivetHandlerTest, AuthPairing) { EXPECT_CALL(security_, IsValidPairingCode("testToken")) .WillRepeatedly(Return(true)); - EXPECT_CALL(security_, CreateAccessToken(_, _)) - .WillRepeatedly(Return("OwnerAccessToken")); + EXPECT_CALL(security_, CreateAccessToken(_, _, _, _, _, _, _)) + .WillRepeatedly(DoAll(SetArgPointee<3>("OwnerAccessToken"), + SetArgPointee<4>(AuthScope::kOwner), + SetArgPointee<5>(base::TimeDelta::FromSeconds(15)), + Return(true))); const char kInput[] = R"({ 'mode': 'pairing', 'requestedScope': 'owner', @@ -412,7 +418,7 @@ })"; const char kExpected[] = R"({ 'accessToken': 'OwnerAccessToken', - 'expiresIn': 3600, + 'expiresIn': 15, 'scope': 'owner', 'tokenType': 'Privet' })";
diff --git a/src/privet/security_delegate.h b/src/privet/security_delegate.h index fdf9a84..c07b782 100644 --- a/src/privet/security_delegate.h +++ b/src/privet/security_delegate.h
@@ -22,8 +22,13 @@ virtual ~SecurityDelegate() {} // Creates access token for the given scope, user id and |time|. - virtual std::string CreateAccessToken(const UserInfo& user_info, - base::TimeDelta ttl) const = 0; + virtual bool CreateAccessToken(AuthType auth_type, + const std::string& auth_code, + AuthScope desired_scope, + std::string* access_token, + AuthScope* granted_scope, + base::TimeDelta* ttl, + ErrorPtr* error) = 0; // Validates |token| and returns scope, user id parsed from that. virtual bool ParseAccessToken(const std::string& token,
diff --git a/src/privet/security_manager.cc b/src/privet/security_manager.cc index d2b7b35..b30bb04 100644 --- a/src/privet/security_manager.cc +++ b/src/privet/security_manager.cc
@@ -36,6 +36,8 @@ const int kMaxAllowedPairingAttemts = 3; const int kPairingBlockingTimeMinutes = 1; +const int kAccessTokenExpirationSeconds = 3600; + class Spakep224Exchanger : public SecurityManager::KeyExchanger { public: explicit Spakep224Exchanger(const std::string& password) @@ -109,9 +111,45 @@ ClosePendingSession(pending_sessions_.begin()->first); } -std::string SecurityManager::CreateAccessToken(const UserInfo& user_info, - base::TimeDelta ttl) const { - return Base64Encode(auth_manager_->CreateAccessToken(user_info, ttl)); +bool SecurityManager::CreateAccessToken(AuthType auth_type, + const std::string& auth_code, + AuthScope desired_scope, + std::string* access_token, + AuthScope* access_token_scope, + base::TimeDelta* access_token_ttl, + ErrorPtr* error) { + switch (auth_type) { + case AuthType::kAnonymous: + break; + case AuthType::kPairing: + if (!IsValidPairingCode(auth_code)) { + Error::AddTo(error, FROM_HERE, errors::kDomain, + errors::kInvalidAuthCode, "Invalid authCode"); + return false; + } + break; + default: + Error::AddToPrintf(error, FROM_HERE, errors::kDomain, + errors::kInvalidAuthMode, "Unsupported auth mode"); + return false; + } + + UserInfo user_info{desired_scope, ++last_user_id_}; + base::TimeDelta ttl = + base::TimeDelta::FromSeconds(kAccessTokenExpirationSeconds); + + if (access_token) { + *access_token = + Base64Encode(auth_manager_->CreateAccessToken(user_info, ttl)); + } + + if (access_token_scope) + *access_token_scope = user_info.scope(); + + if (access_token_ttl) + *access_token_ttl = ttl; + + return true; } bool SecurityManager::ParseAccessToken(const std::string& token,
diff --git a/src/privet/security_manager.h b/src/privet/security_manager.h index f65f7bd..beb7955 100644 --- a/src/privet/security_manager.h +++ b/src/privet/security_manager.h
@@ -60,8 +60,13 @@ ~SecurityManager() override; // SecurityDelegate methods - std::string CreateAccessToken(const UserInfo& user_info, - base::TimeDelta ttl) const override; + bool CreateAccessToken(AuthType auth_type, + const std::string& auth_code, + AuthScope desired_scope, + std::string* access_token, + AuthScope* granted_scope, + base::TimeDelta* ttl, + ErrorPtr* error) override; bool ParseAccessToken(const std::string& token, UserInfo* user_info, ErrorPtr* error) const override; @@ -109,6 +114,7 @@ mutable base::Time block_pairing_until_; PairingStartListener on_start_; PairingEndListener on_end_; + uint64_t last_user_id_{0}; base::WeakPtrFactory<SecurityManager> weak_ptr_factory_{this};
diff --git a/src/privet/security_manager_unittest.cc b/src/privet/security_manager_unittest.cc index f48ec7f..6fc6b4c 100644 --- a/src/privet/security_manager_unittest.cc +++ b/src/privet/security_manager_unittest.cc
@@ -97,6 +97,19 @@ std::string auth_code_base64{Base64Encode(auth_code)}; EXPECT_TRUE(security_.IsValidPairingCode(auth_code_base64)); + + std::string token; + AuthScope scope; + base::TimeDelta ttl; + EXPECT_TRUE(security_.CreateAccessToken(AuthType::kPairing, + auth_code_base64, AuthScope::kOwner, + &token, &scope, &ttl, nullptr)); + EXPECT_EQ(AuthScope::kOwner, scope); + EXPECT_EQ(base::TimeDelta::FromHours(1), ttl); + + UserInfo info; + EXPECT_TRUE(security_.ParseAccessToken(token, &info, nullptr)); + EXPECT_EQ(AuthScope::kOwner, info.scope()); } const base::Time time_ = base::Time::FromTimeT(1410000000); @@ -119,9 +132,16 @@ }; TEST_F(SecurityManagerTest, CreateAccessToken) { - EXPECT_EQ("TV18I+N7cDPah7Nq6o7pl5H7DjDu5nCDf/cbdE4FZFEyOjc6MTQxMDAwMDA2MA==", - security_.CreateAccessToken(UserInfo{AuthScope::kUser, 7}, - base::TimeDelta::FromMinutes(1))); + std::string token; + AuthScope scope; + base::TimeDelta ttl; + EXPECT_TRUE(security_.CreateAccessToken(AuthType::kAnonymous, "", + AuthScope::kUser, &token, &scope, + &ttl, nullptr)); + EXPECT_EQ("Cfz+qcndz8/Mo3ytgYlD7zn8qImkkdPsJVUNBmSOiXwyOjE6MTQxMDAwMzYwMA==", + token); + EXPECT_EQ(AuthScope::kUser, scope); + EXPECT_EQ(base::TimeDelta::FromHours(1), ttl); } TEST_F(SecurityManagerTest, ParseAccessToken) {