From 908a6ccaa11670f8fd5a2dcd076337a5ed82243b Mon Sep 17 00:00:00 2001 From: ShobhitAd Date: Mon, 7 Dec 2020 09:50:17 -0500 Subject: [PATCH 1/6] Add permissions check in OnAllowSDLFunctionalityNotification for application revoked case --- .../src/policies/policy_handler.cc | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/src/components/application_manager/src/policies/policy_handler.cc b/src/components/application_manager/src/policies/policy_handler.cc index b61a75d7def..407bf67efc7 100644 --- a/src/components/application_manager/src/policies/policy_handler.cc +++ b/src/components/application_manager/src/policies/policy_handler.cc @@ -1359,7 +1359,6 @@ void PolicyHandler::OnAllowSDLFunctionalityNotification( } #ifdef EXTERNAL_PROPRIETARY_MODE - ApplicationSet applications; { DataAccessor accessor = @@ -1405,6 +1404,8 @@ void PolicyHandler::OnAllowSDLFunctionalityNotification( } #ifdef EXTERNAL_PROPRIETARY_MODE + SDL_LOG_DEBUG("[!] OnAllowSDLFunctionality: is_allowed=" + << is_allowed << ", device_mac=" << device_mac); if (last_activated_app_id_) { ApplicationSharedPtr app = @@ -1416,7 +1417,19 @@ void PolicyHandler::OnAllowSDLFunctionalityNotification( << "' not found among registered applications."); return; } - if (is_allowed) { + + std::string policy_app_id = app->policy_app_id(); + AppPermissions permissions(policy_app_id); + const auto policy_manager = LoadPolicyManager(); + + bool is_allowed_by_policies = true; + if (PolicyEnabled()) { + permissions = policy_manager->GetAppPermissionsChanges(app->mac_address(), + policy_app_id); + is_allowed_by_policies = !permissions.appRevoked; + } + + if (is_allowed && is_allowed_by_policies) { // Send HMI status notification to mobile // Put application in full AudioStreamingState::eType audio_state = From f22dc98b472e1975a3cb92e225706623dbe4d34a Mon Sep 17 00:00:00 2001 From: ShobhitAd Date: Tue, 8 Dec 2020 08:50:28 -0500 Subject: [PATCH 2/6] Fix unit tests --- .../application_manager/src/policies/policy_handler.cc | 3 --- .../policy/policy_external/test/policy_manager_impl_test.cc | 2 ++ 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/src/components/application_manager/src/policies/policy_handler.cc b/src/components/application_manager/src/policies/policy_handler.cc index 407bf67efc7..e0be9c12eea 100644 --- a/src/components/application_manager/src/policies/policy_handler.cc +++ b/src/components/application_manager/src/policies/policy_handler.cc @@ -1404,9 +1404,6 @@ void PolicyHandler::OnAllowSDLFunctionalityNotification( } #ifdef EXTERNAL_PROPRIETARY_MODE - SDL_LOG_DEBUG("[!] OnAllowSDLFunctionality: is_allowed=" - << is_allowed << ", device_mac=" << device_mac); - if (last_activated_app_id_) { ApplicationSharedPtr app = application_manager_.application(last_activated_app_id_); diff --git a/src/components/policy/policy_external/test/policy_manager_impl_test.cc b/src/components/policy/policy_external/test/policy_manager_impl_test.cc index 4d39e699b52..95fc783b2d4 100644 --- a/src/components/policy/policy_external/test/policy_manager_impl_test.cc +++ b/src/components/policy/policy_external/test/policy_manager_impl_test.cc @@ -66,6 +66,7 @@ const int kServiceTypeInt = 0; const std::string kDeviceNumber = "XXX123456789ZZZ"; const std::string kAppStorageFolder = "app_storage_folder"; const std::string kValidAppId = "1234"; +const std::vector kDevices{kDeviceNumber}; } // namespace class PolicyManagerImplTest : public ::testing::Test { @@ -88,6 +89,7 @@ class PolicyManagerImplTest : public ::testing::Test { ON_CALL(policy_settings_, app_storage_folder()) .WillByDefault(ReturnRef(kAppStorageFolder)); + ON_CALL(listener_, GetDevicesIds(_)).WillByDefault(Return(kDevices)); } ::testing::AssertionResult IsValid(const policy_table::Table& table) { From 10eb2f6ed595ae3118cc7d6f4f234f49c009d75c Mon Sep 17 00:00:00 2001 From: ShobhitAd Date: Sun, 3 Jan 2021 15:26:26 -0500 Subject: [PATCH 3/6] Fix unit tests --- .../src/policies/policy_handler.cc | 6 ++--- .../test/policy_handler_test.cc | 22 +++++++++++++++++-- 2 files changed, 22 insertions(+), 6 deletions(-) diff --git a/src/components/application_manager/src/policies/policy_handler.cc b/src/components/application_manager/src/policies/policy_handler.cc index 2e08a653618..abef37d476b 100644 --- a/src/components/application_manager/src/policies/policy_handler.cc +++ b/src/components/application_manager/src/policies/policy_handler.cc @@ -1436,12 +1436,10 @@ void PolicyHandler::OnAllowSDLFunctionalityNotification( return; } - std::string policy_app_id = app->policy_app_id(); - AppPermissions permissions(policy_app_id); - const auto policy_manager = LoadPolicyManager(); - bool is_allowed_by_policies = true; if (PolicyEnabled()) { + std::string policy_app_id = app->policy_app_id(); + AppPermissions permissions(policy_app_id); permissions = policy_manager->GetAppPermissionsChanges(app->mac_address(), policy_app_id); is_allowed_by_policies = !permissions.appRevoked; diff --git a/src/components/application_manager/test/policy_handler_test.cc b/src/components/application_manager/test/policy_handler_test.cc index a66cff17270..7371fdbaa16 100644 --- a/src/components/application_manager/test/policy_handler_test.cc +++ b/src/components/application_manager/test/policy_handler_test.cc @@ -926,8 +926,8 @@ void PolicyHandlerTest::TestActivateApp(const uint32_t connection_key, _, _)); #endif // EXTERNAL_PROPRIETARY_MODE - - EXPECT_CALL(*application1, policy_app_id()).WillOnce(Return(kPolicyAppId_)); + EXPECT_CALL(*application1, policy_app_id()) + .WillRepeatedly(Return(kPolicyAppId_)); EXPECT_CALL(*mock_policy_manager_, GetAppPermissionsChanges(kMacAddr_, kPolicyAppId_)) .WillOnce(Return(permissions)); @@ -1798,6 +1798,12 @@ TEST_F(PolicyHandlerTest, EXPECT_CALL(conn_handler, GetDeviceID(kPolicyAppId_, _)) .WillRepeatedly(Return(true)); +#ifdef EXTERNAL_PROPRIETARY_MODE + AppPermissions permissions(kPolicyAppId_); + EXPECT_CALL(*mock_policy_manager_, GetAppPermissionsChanges(_, kPolicyAppId_)) + .WillOnce(Return(permissions)); +#endif // EXTERNAL_PROPRIETARY_MODE + policy_handler_.OnAllowSDLFunctionalityNotification(is_allowed, kPolicyAppId_); } @@ -1877,6 +1883,12 @@ TEST_F(PolicyHandlerTest, EXPECT_CALL(*mock_policy_manager_, SetUserConsentForDevice(kPolicyAppId_, is_allowed)); +#ifdef EXTERNAL_PROPRIETARY_MODE + AppPermissions permissions(kPolicyAppId_); + EXPECT_CALL(*mock_policy_manager_, GetAppPermissionsChanges(_, kPolicyAppId_)) + .WillOnce(Return(permissions)); +#endif // EXTERNAL_PROPRIETARY_MODE + policy_handler_.OnAllowSDLFunctionalityNotification(is_allowed, ""); } @@ -1893,6 +1905,12 @@ TEST_F(PolicyHandlerTest, EXPECT_CALL(*mock_policy_manager_, SetUserConsentForDevice(_, _)).Times(0); +#ifdef EXTERNAL_PROPRIETARY_MODE + AppPermissions permissions(kPolicyAppId_); + EXPECT_CALL(*mock_policy_manager_, GetAppPermissionsChanges(_, kPolicyAppId_)) + .WillOnce(Return(permissions)); +#endif // EXTERNAL_PROPRIETARY_MODE + const bool is_allowed = true; policy_handler_.OnAllowSDLFunctionalityNotification(is_allowed, ""); } From 01aff9e7561dbd06c51f7f7510b120cf7db9936b Mon Sep 17 00:00:00 2001 From: ShobhitAd Date: Fri, 8 Jan 2021 12:30:27 -0500 Subject: [PATCH 4/6] Use IsApplicationRevoked instead of GetAppPermissionChanges --- .../src/policies/policy_handler.cc | 7 ++----- .../test/policy_handler_test.cc | 18 ------------------ 2 files changed, 2 insertions(+), 23 deletions(-) diff --git a/src/components/application_manager/src/policies/policy_handler.cc b/src/components/application_manager/src/policies/policy_handler.cc index abef37d476b..74842d4cb5e 100644 --- a/src/components/application_manager/src/policies/policy_handler.cc +++ b/src/components/application_manager/src/policies/policy_handler.cc @@ -1438,11 +1438,8 @@ void PolicyHandler::OnAllowSDLFunctionalityNotification( bool is_allowed_by_policies = true; if (PolicyEnabled()) { - std::string policy_app_id = app->policy_app_id(); - AppPermissions permissions(policy_app_id); - permissions = policy_manager->GetAppPermissionsChanges(app->mac_address(), - policy_app_id); - is_allowed_by_policies = !permissions.appRevoked; + is_allowed_by_policies = + policy_manager->IsApplicationRevoked(app->policy_app_id()); } if (is_allowed && is_allowed_by_policies) { diff --git a/src/components/application_manager/test/policy_handler_test.cc b/src/components/application_manager/test/policy_handler_test.cc index 7371fdbaa16..f566bed9340 100644 --- a/src/components/application_manager/test/policy_handler_test.cc +++ b/src/components/application_manager/test/policy_handler_test.cc @@ -1798,12 +1798,6 @@ TEST_F(PolicyHandlerTest, EXPECT_CALL(conn_handler, GetDeviceID(kPolicyAppId_, _)) .WillRepeatedly(Return(true)); -#ifdef EXTERNAL_PROPRIETARY_MODE - AppPermissions permissions(kPolicyAppId_); - EXPECT_CALL(*mock_policy_manager_, GetAppPermissionsChanges(_, kPolicyAppId_)) - .WillOnce(Return(permissions)); -#endif // EXTERNAL_PROPRIETARY_MODE - policy_handler_.OnAllowSDLFunctionalityNotification(is_allowed, kPolicyAppId_); } @@ -1883,12 +1877,6 @@ TEST_F(PolicyHandlerTest, EXPECT_CALL(*mock_policy_manager_, SetUserConsentForDevice(kPolicyAppId_, is_allowed)); -#ifdef EXTERNAL_PROPRIETARY_MODE - AppPermissions permissions(kPolicyAppId_); - EXPECT_CALL(*mock_policy_manager_, GetAppPermissionsChanges(_, kPolicyAppId_)) - .WillOnce(Return(permissions)); -#endif // EXTERNAL_PROPRIETARY_MODE - policy_handler_.OnAllowSDLFunctionalityNotification(is_allowed, ""); } @@ -1905,12 +1893,6 @@ TEST_F(PolicyHandlerTest, EXPECT_CALL(*mock_policy_manager_, SetUserConsentForDevice(_, _)).Times(0); -#ifdef EXTERNAL_PROPRIETARY_MODE - AppPermissions permissions(kPolicyAppId_); - EXPECT_CALL(*mock_policy_manager_, GetAppPermissionsChanges(_, kPolicyAppId_)) - .WillOnce(Return(permissions)); -#endif // EXTERNAL_PROPRIETARY_MODE - const bool is_allowed = true; policy_handler_.OnAllowSDLFunctionalityNotification(is_allowed, ""); } From 842f85ca1ee94fbfb5abc40de8e2be42ac5a1479 Mon Sep 17 00:00:00 2001 From: ShobhitAd Date: Fri, 8 Jan 2021 13:01:45 -0500 Subject: [PATCH 5/6] Fix IsApplicationRevoked check --- .../application_manager/src/policies/policy_handler.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/application_manager/src/policies/policy_handler.cc b/src/components/application_manager/src/policies/policy_handler.cc index 74842d4cb5e..56f60ccbdc7 100644 --- a/src/components/application_manager/src/policies/policy_handler.cc +++ b/src/components/application_manager/src/policies/policy_handler.cc @@ -1439,7 +1439,7 @@ void PolicyHandler::OnAllowSDLFunctionalityNotification( bool is_allowed_by_policies = true; if (PolicyEnabled()) { is_allowed_by_policies = - policy_manager->IsApplicationRevoked(app->policy_app_id()); + !policy_manager->IsApplicationRevoked(app->policy_app_id()); } if (is_allowed && is_allowed_by_policies) { From 68f53030ea140d607e8fbe862cf1c97bde585995 Mon Sep 17 00:00:00 2001 From: ShobhitAd Date: Fri, 8 Jan 2021 13:37:39 -0500 Subject: [PATCH 6/6] Add check to prevent setting last_activated_app_id_ for revoked app --- .../application_manager/src/policies/policy_handler.cc | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/components/application_manager/src/policies/policy_handler.cc b/src/components/application_manager/src/policies/policy_handler.cc index 56f60ccbdc7..890dd5b57a8 100644 --- a/src/components/application_manager/src/policies/policy_handler.cc +++ b/src/components/application_manager/src/policies/policy_handler.cc @@ -1515,7 +1515,9 @@ void PolicyHandler::OnActivateApp(uint32_t connection_key, // is not allowed. if (!permissions.isSDLAllowed) { permissions.priority.clear(); - last_activated_app_id_ = connection_key; + if (!permissions.appRevoked) { + last_activated_app_id_ = connection_key; + } } if (permissions.appRevoked) {