Merge "Parse user flags as flags instead of enum." into sc-v2-dev

2 files changed, 72 insertions, 5 deletions

diff --git a/automotive/vehicle/2.0/utils/UserHalHelper.cpp b/automotive/vehicle/2.0/utils/UserHalHelper.cpp
index abf59b74e..dccdb2b84 100644
--- a/automotive/vehicle/2.0/utils/UserHalHelper.cpp
+++ b/automotive/vehicle/2.0/utils/UserHalHelper.cpp
@@ -60,11 +60,22 @@ Result<void> parseUserInfo(const hidl_vec<int32_t>& int32Values, size_t startPos
                        << int32Values.size();
     }
     userInfo->userId = int32Values[startPos];
-    auto userFlags = verifyAndCast<UserFlags>(int32Values[startPos + 1]);
-    if (!userFlags.ok()) {
-        return Error() << "Invalid user flags: " << userFlags.error();
+    int32_t intUserFlags = int32Values[startPos + 1];
+    int32_t expectedUserFlags = 0;
+    for (const auto& v : hidl_enum_range<UserFlags>()) {
+        int32_t intEnumUserFlag = static_cast<int32_t>(v);
+        if ((intUserFlags & intEnumUserFlag) != 0) {
+            expectedUserFlags |= intEnumUserFlag;
+        }
+    }
+    if (intUserFlags != expectedUserFlags) {
+        return Error() << "Invalid user flags: " << intUserFlags << ", must be '|' of UserFlags";
     }
-    userInfo->flags = *userFlags;
+    // intUserFlags is actually not a valid UserFlags enum, instead, it is a 'bit or' of possible
+    // multiple UserFlags. However, because the HAL interface was defined incorrectly, we have to
+    // cast it to UserFlags here, which is defined behavior because the underlying type for
+    // UserFlags is int32_t and our intUserFlags is within the range of int32_t.
+    userInfo->flags = static_cast<UserFlags>(intUserFlags);
     return {};
 }
 
diff --git a/automotive/vehicle/2.0/utils/tests/UserHalHelper_test.cpp b/automotive/vehicle/2.0/utils/tests/UserHalHelper_test.cpp
index 7da87a23c..0562a54ce 100644
--- a/automotive/vehicle/2.0/utils/tests/UserHalHelper_test.cpp
+++ b/automotive/vehicle/2.0/utils/tests/UserHalHelper_test.cpp
@@ -54,6 +54,10 @@ constexpr int32_t VEHICLE_REQUEST = static_cast<int32_t>(SwitchUserMessageType::
 constexpr int32_t GUEST_USER = static_cast<int32_t>(UserFlags::GUEST);
 constexpr int32_t NONE_USER = static_cast<int32_t>(UserFlags::NONE);
 constexpr int32_t SYSTEM_USER = static_cast<int32_t>(UserFlags::SYSTEM);
+constexpr int32_t ADMIN_USER = static_cast<int32_t>(UserFlags::ADMIN);
+constexpr int32_t SYSTEM_ADMIN_USER = static_cast<int32_t>(UserFlags::SYSTEM | UserFlags::ADMIN);
+// 0x1111 is not a valid UserFlags combination.
+constexpr int32_t INVALID_USER_FLAG = 0x1111;
 
 constexpr int32_t USER_ID_ASSOC_KEY_FOB =
         static_cast<int32_t>(UserIdentificationAssociationType::KEY_FOB);
@@ -72,7 +76,7 @@ constexpr int32_t USER_ID_ASSOC_NO_USER =
 
 }  // namespace
 
-TEST(UserHalHelperTest, TestToInitialUserInfoRequest) {
+TEST(UserHalHelperTest, TestToInitialUserInfoRequestSystemUser) {
     VehiclePropValue propValue{
             .prop = INITIAL_USER_INFO,
             .value = {.int32Values = {23, FIRST_BOOT_AFTER_OTA, 10, NONE_USER, 2, 0, SYSTEM_USER,
@@ -92,6 +96,58 @@ TEST(UserHalHelperTest, TestToInitialUserInfoRequest) {
     EXPECT_THAT(actual.value(), Eq(expected));
 }
 
+TEST(UserHalHelperTest, TestToInitialUserInfoRequestAdminUser) {
+    VehiclePropValue propValue{
+            .prop = INITIAL_USER_INFO,
+            .value = {.int32Values = {23, FIRST_BOOT_AFTER_OTA, 10, NONE_USER, 2, 0, ADMIN_USER, 10,
+                                      NONE_USER}},
+    };
+    InitialUserInfoRequest expected{
+            .requestId = 23,
+            .requestType = InitialUserInfoRequestType::FIRST_BOOT_AFTER_OTA,
+            .usersInfo = {{10, UserFlags::NONE}, 2, {{0, UserFlags::ADMIN}, {10, UserFlags::NONE}}},
+    };
+
+    auto actual = toInitialUserInfoRequest(propValue);
+
+    ASSERT_TRUE(actual.ok()) << actual.error().message();
+    EXPECT_THAT(actual.value(), Eq(expected));
+}
+
+TEST(UserHalHelperTest, TestToInitialUserInfoRequestUserFlagsBitCombination) {
+    // SYSTEM_ADMIN_USER is two UserFlags combined and is itself not a defined UserFlags enum.
+    VehiclePropValue propValue{
+            .prop = INITIAL_USER_INFO,
+            .value = {.int32Values = {23, FIRST_BOOT_AFTER_OTA, 10, NONE_USER, 2, 0,
+                                      SYSTEM_ADMIN_USER, 10, NONE_USER}},
+    };
+    InitialUserInfoRequest expected{
+            .requestId = 23,
+            .requestType = InitialUserInfoRequestType::FIRST_BOOT_AFTER_OTA,
+            .usersInfo = {{10, UserFlags::NONE},
+                          2,
+                          {{0, static_cast<UserFlags>(SYSTEM_ADMIN_USER)}, {10, UserFlags::NONE}}},
+    };
+
+    auto actual = toInitialUserInfoRequest(propValue);
+
+    ASSERT_TRUE(actual.ok()) << actual.error().message();
+    EXPECT_THAT(actual.value(), Eq(expected));
+}
+
+TEST(UserHalHelperTest, TestToInitialUserInfoRequestUserInvalidUserFlag) {
+    // 0x1111 is not a valid UserFlags flag combination.
+    VehiclePropValue propValue{
+            .prop = INITIAL_USER_INFO,
+            .value = {.int32Values = {23, FIRST_BOOT_AFTER_OTA, 10, NONE_USER, 2, 0,
+                                      INVALID_USER_FLAG, 10, NONE_USER}},
+    };
+
+    auto actual = toInitialUserInfoRequest(propValue);
+
+    EXPECT_FALSE(actual.ok()) << "No error returned on invalid user flags";
+}
+
 TEST(UserHalHelperTest, TestFailsToInitialUserInfoRequestWithMismatchingPropType) {
     VehiclePropValue propValue{
             .prop = INT32_MAX,