Enforce constraints on UserStatus in SetUser/SetCredential commands. (#…

…21657) * Enforce constraints on UserStatus in SetUser/SetCredential commands. Fixes #21577 * Fix incorrect userStatus in precondition setup in cert YAMLs.
project-chip · Jun 27, 2023 · 6020100 · 6020100
1 parent 5ec29e2
commit 6020100
Show file tree

Hide file tree

Showing 7 changed files with 10,244 additions and 29 deletions.
diff --git a/src/app/clusters/door-lock-server/door-lock-server.cpp b/src/app/clusters/door-lock-server/door-lock-server.cpp
@@ -304,6 +304,16 @@ bool DoorLockServer::SendLockAlarmEvent(chip::EndpointId endpointId, DlAlarmCode
     return true;
 }
 
+namespace {
+// Check whether this is valid UserStatus for a SetUser or SetCredential
+// command.
+bool IsValidUserStatusForSet(const Nullable<DlUserStatus> & userStatus)
+{
+    return userStatus.IsNull() || (userStatus.Value() == DlUserStatus::kOccupiedEnabled) ||
+        (userStatus.Value() == DlUserStatus::kOccupiedDisabled);
+}
+} // anonymous namespace
+
 void DoorLockServer::setUserCommandHandler(chip::app::CommandHandler * commandObj,
                                            const chip::app::ConcreteCommandPath & commandPath,
                                            const chip::app::Clusters::DoorLock::Commands::SetUser::DecodableType & commandData)
@@ -362,8 +372,7 @@ void DoorLockServer::setUserCommandHandler(chip::app::CommandHandler * commandOb
         return;
     }
 
-    if (!userStatus.IsNull() &&
-        (userStatus.Value() < DlUserStatus::kAvailable || userStatus.Value() > DlUserStatus::kOccupiedDisabled))
+    if (!IsValidUserStatusForSet(userStatus))
     {
         emberAfDoorLockClusterPrintln(
             "[SetUser] Unable to set the user: user status is out of range [endpointId=%d,userIndex=%d,userStatus=%u]",
@@ -677,9 +686,7 @@ void DoorLockServer::setCredentialCommandHandler(
         return;
     }
 
-    // OPTIMIZE: We can unify the checks for enum validity here and in set user command handler
-    if (!userStatus.IsNull() &&
-        (userStatus.Value() < DlUserStatus::kAvailable || userStatus.Value() > DlUserStatus::kOccupiedDisabled))
+    if (!IsValidUserStatusForSet(userStatus))
     {
         emberAfDoorLockClusterPrintln("[SetCredential] Unable to set the credential: user status is out of range "
                                       "[endpointId=%d,credentialIndex=%d,userStatus=%u]",

diff --git a/src/app/tests/suites/DL_UsersAndCredentials.yaml b/src/app/tests/suites/DL_UsersAndCredentials.yaml
@@ -497,6 +497,157 @@ tests:
               - name: "nextUserIndex"
                 value: null
 
+    - label: "Try to add a user with userStatus 0"
+      command: "SetUser"
+      timedInteractionTimeoutMs: 10000
+      arguments:
+          values:
+              - name: "operationType"
+                value: 0
+              - name: "userIndex"
+                value: 3
+              - name: "userName"
+                value: "test_user3"
+              - name: "userUniqueId"
+                value: 0xBABA
+              - name: "userStatus"
+                value: 0
+              - name: "userType"
+                value: null
+              - name: "credentialRule"
+                value: null
+      response:
+          error: INVALID_COMMAND
+
+    - label: "Make sure the user did not get created"
+      command: "GetUser"
+      arguments:
+          values:
+              - name: "userIndex"
+                value: 3
+      response:
+          values:
+              - name: "userIndex"
+                value: 3
+              - name: "userName"
+                value: null
+              - name: "userUniqueId"
+                value: null
+              - name: "userStatus"
+                value: null
+              - name: "userType"
+                value: null
+              - name: "credentialRule"
+                value: null
+              - name: "credentials"
+                value: null
+              - name: "creatorFabricIndex"
+                value: null
+              - name: "lastModifiedFabricIndex"
+                value: null
+              - name: "nextUserIndex"
+                value: null
+
+    - label: "Try to add a user with userStatus 2"
+      command: "SetUser"
+      timedInteractionTimeoutMs: 10000
+      arguments:
+          values:
+              - name: "operationType"
+                value: 0
+              - name: "userIndex"
+                value: 3
+              - name: "userName"
+                value: "test_user3"
+              - name: "userUniqueId"
+                value: 0xBABA
+              - name: "userStatus"
+                value: 2
+              - name: "userType"
+                value: null
+              - name: "credentialRule"
+                value: null
+      response:
+          error: INVALID_COMMAND
+
+    - label: "Make sure the user did not get created"
+      command: "GetUser"
+      arguments:
+          values:
+              - name: "userIndex"
+                value: 3
+      response:
+          values:
+              - name: "userIndex"
+                value: 3
+              - name: "userName"
+                value: null
+              - name: "userUniqueId"
+                value: null
+              - name: "userStatus"
+                value: null
+              - name: "userType"
+                value: null
+              - name: "credentialRule"
+                value: null
+              - name: "credentials"
+                value: null
+              - name: "creatorFabricIndex"
+                value: null
+              - name: "lastModifiedFabricIndex"
+                value: null
+              - name: "nextUserIndex"
+                value: null
+
+    - label: "Try to add a user with userStatus 3"
+      command: "SetUser"
+      timedInteractionTimeoutMs: 10000
+      arguments:
+          values:
+              - name: "operationType"
+                value: 0
+              - name: "userIndex"
+                value: 3
+              - name: "userName"
+                value: "test_user3"
+              - name: "userUniqueId"
+                value: 0xBABA
+              - name: "userStatus"
+                value: 3
+              - name: "userType"
+                value: null
+              - name: "credentialRule"
+                value: null
+
+    - label: "Read the new third user back and verify its fields"
+      command: "GetUser"
+      arguments:
+          values:
+              - name: "userIndex"
+                value: 3
+      response:
+          values:
+              - name: "userIndex"
+                value: 3
+              - name: "userName"
+                value: "test_user3"
+              - name: "userUniqueId"
+                value: 0xBABA
+              - name: "userStatus"
+                value: 3
+              - name: "userType"
+                value: 0
+              - name: "credentialRule"
+                value: 0
+              - name: "credentials"
+                value: null
+              - name: "creatorFabricIndex"
+                value: 1
+              - name: "lastModifiedFabricIndex"
+                value: 1
+              - name: "nextUserIndex"
+                value: null
+
     - label: "Create user in the last slot"
       command: "SetUser"
       timedInteractionTimeoutMs: 10000
@@ -793,7 +944,7 @@ tests:
               - name: "nextCredentialIndex"
                 value: null
 
-    - label: "Reading PIN credential with index 0 fails"
+    - label: "Reading PIN credential with index 0 returns no credential"
       command: "GetCredentialStatus"
       arguments:
           values:
@@ -812,7 +963,8 @@ tests:
               - name: "nextCredentialIndex"
                 value: null
 
-    - label: "Reading PIN credential with out-of-bounds index fails"
+    - label:
+          "Reading PIN credential with out-of-bounds index returns no credential"
       command: "GetCredentialStatus"
       arguments:
           values:
@@ -823,7 +975,73 @@ tests:
                         CredentialIndex: NumberOfPINUsersSupported + 1,
                     }
       response:
-          error: INVALID_COMMAND
+          values:
+              - name: "credentialExists"
+                value: false
+              - name: "userIndex"
+                value: null
+              - name: "creatorFabricIndex"
+                value: null
+              - name: "lastModifiedFabricIndex"
+                value: null
+              - name: "nextCredentialIndex"
+                value: null
+
+    - label:
+          "Verify that a user with UserStatus = 0 cannot be added via
+          SetCredential"
+      command: "SetCredential"
+      timedInteractionTimeoutMs: 10000
+      arguments:
+          values:
+              - name: "operationType"
+                value: 0
+              - name: "credential"
+                value: { CredentialType: 1, CredentialIndex: 1 }
+              - name: "credentialData"
+                value: "000000"
+              - name: "userIndex"
+                value: null
+              - name: "userStatus"
+                value: 0
+              - name: "userType"
+                value: null
+      response:
+          values:
+              - name: "status"
+                value: 0x85 # INVALID_COMMAND
+              - name: "userIndex"
+                value: null
+              - name: "nextCredentialIndex"
+                value: 2
+
+    - label:
+          "Verify that a user with UserStatus = 2 cannot be added via
+          SetCredential"
+      command: "SetCredential"
+      timedInteractionTimeoutMs: 10000
+      arguments:
+          values:
+              - name: "operationType"
+                value: 0
+              - name: "credential"
+                value: { CredentialType: 1, CredentialIndex: 1 }
+              - name: "credentialData"
+                value: "000000"
+              - name: "userIndex"
+                value: null
+              - name: "userStatus"
+                value: 2
+              - name: "userType"
+                value: null
+      response:
+          values:
+              - name: "status"
+                value: 0x85 # INVALID_COMMAND
+              - name: "userIndex"
+                value: null
+              - name: "nextCredentialIndex"
+                value: 2
 
     - label: "Create new PIN credential and user"
       command: "SetCredential"
@@ -962,16 +1180,51 @@ tests:
           saveAs: NumberOfRFIDUsersSupported
           value: 10
 
-    - label: "Reading RFID credential with index 0 fails"
+    # Disabled because due to https://github.com/project-chip/connectedhomeip/issues/21656: that causes the nextCredentialIndex
+    # here to have the wrong value.
+    - label: "Reading RFID credential with index 0 returns no credential"
+      disabled: true
       command: "GetCredentialStatus"
       arguments:
           values:
               - name: "credential"
                 value: { CredentialType: 2, CredentialIndex: 0 }
       response:
-          error: INVALID_COMMAND
+          values:
+              - name: "credentialExists"
+                value: false
+              - name: "userIndex"
+                value: null
+              - name: "creatorFabricIndex"
+                value: null
+              - name: "lastModifiedFabricIndex"
+                value: null
+              - name: "nextCredentialIndex"
+                value: null
 
-    - label: "Reading RFID credential with out-of-bounds index fails"
+    # Duplicate of the previous test that does not check the value of nextCredentialIndex
+    - label:
+          "Reading RFID credential with index 0 returns no credential duplicate
+          with bug workaround"
+      command: "GetCredentialStatus"
+      arguments:
+          values:
+              - name: "credential"
+                value: { CredentialType: 2, CredentialIndex: 0 }
+      response:
+          values:
+              - name: "credentialExists"
+                value: false
+              - name: "userIndex"
+                value: null
+              - name: "creatorFabricIndex"
+                value: null
+              - name: "lastModifiedFabricIndex"
+                value: null
+
+    - label:
+          "Reading RFID credential with out-of-bounds index returns no
+          credential"
       command: "GetCredentialStatus"
       arguments:
           values:
@@ -982,7 +1235,17 @@ tests:
                         CredentialIndex: NumberOfRFIDUsersSupported + 1,
                     }
       response:
-          error: INVALID_COMMAND
+          values:
+              - name: "credentialExists"
+                value: false
+              - name: "userIndex"
+                value: null
+              - name: "creatorFabricIndex"
+                value: null
+              - name: "lastModifiedFabricIndex"
+                value: null
+              - name: "nextCredentialIndex"
+                value: null
 
     - label: "Check that RFID credential does not exist"
       command: "GetCredentialStatus"

diff --git a/src/app/tests/suites/certification/Test_TC_DRLK_2_2.yaml b/src/app/tests/suites/certification/Test_TC_DRLK_2_2.yaml
@@ -91,9 +91,9 @@ tests:
               - name: "userIndex"
                 value: 1
               - name: "userStatus"
-                value: 0
+                value: null
               - name: "userType"
-                value: 0
+                value: null
       response:
           values:
               - name: "status"

diff --git a/src/app/tests/suites/certification/Test_TC_DRLK_2_3.yaml b/src/app/tests/suites/certification/Test_TC_DRLK_2_3.yaml
@@ -91,9 +91,9 @@ tests:
               - name: "userIndex"
                 value: 1
               - name: "userStatus"
-                value: 0
+                value: null
               - name: "userType"
-                value: 0
+                value: null
       response:
           values:
               - name: "status"

diff --git a/src/app/tests/suites/tests.js b/src/app/tests/suites/tests.js
@@ -865,7 +865,7 @@ function getTests() {
     ];
 
     const DoorLock = [
-        //"DL_UsersAndCredentials", TODO: This test is not aligned with spec
+        "DL_UsersAndCredentials",
         "DL_LockUnlock",
         "DL_Schedules",
         "Test_TC_DRLK_2_2",