[ICD] Finish ICDM server refactor (project-chip#30703)

mkardous-silabs · web-flow · commit b3ef405af9f7 · 2023-11-29T20:28:47.000Z
* clean up icdm server

* rename variables

* move CheckAdmin to anonymous namespace
diff --git a/src/app/clusters/icd-management-server/icd-management-server.cpp b/src/app/clusters/icd-management-server/icd-management-server.cpp
@@ -188,6 +188,30 @@ class IcdManagementFabricDelegate : public FabricTable::Delegate
 IcdManagementFabricDelegate gFabricDelegate;
 IcdManagementAttributeAccess gAttribute;
 
+/**
+ * @brief Function checks if the client as admin permissions to the cluster in the commandPath
+ *
+ * @param[out] isClientAdmin True : Client has admin permissions
+ *                           False : Client does not have admin permissions
+ *                           If an error ocurs, isClientAdmin is not changed
+ * @return CHIP_ERROR
+ */
+CHIP_ERROR CheckAdmin(CommandHandler * commandObj, const ConcreteCommandPath & commandPath, bool & isClientAdmin)
+{
+    RequestPath requestPath{ .cluster = commandPath.mClusterId, .endpoint = commandPath.mEndpointId };
+    CHIP_ERROR err = GetAccessControl().Check(commandObj->GetSubjectDescriptor(), requestPath, Privilege::kAdminister);
+    if (CHIP_NO_ERROR == err)
+    {
+        isClientAdmin = true;
+    }
+    else if (CHIP_ERROR_ACCESS_DENIED == err)
+    {
+        isClientAdmin = false;
+        err           = CHIP_NO_ERROR;
+    }
+    return err;
+}
+
 } // namespace
 
 /*
@@ -198,22 +222,32 @@ PersistentStorageDelegate * ICDManagementServer::mStorage           = nullptr;
 Crypto::SymmetricKeystore * ICDManagementServer::mSymmetricKeystore = nullptr;
 ICDConfigurationData * ICDManagementServer::mICDConfigurationData   = nullptr;
 
-Status ICDManagementServer::RegisterClient(FabricIndex fabric_index, NodeId node_id, uint64_t monitored_subject, ByteSpan key,
-                                           Optional<ByteSpan> verification_key, bool isAdmin, uint32_t & icdCounter)
+Status ICDManagementServer::RegisterClient(CommandHandler * commandObj, const ConcreteCommandPath & commandPath,
+                                           const Commands::RegisterClient::DecodableType & commandData, uint32_t & icdCounter)
 {
+    FabricIndex fabricIndex            = commandObj->GetAccessingFabricIndex();
+    NodeId nodeId                      = commandData.checkInNodeID;
+    uint64_t monitoredSubject          = commandData.monitoredSubject;
+    ByteSpan key                       = commandData.key;
+    Optional<ByteSpan> verificationKey = commandData.verificationKey;
+    bool isClientAdmin                 = false;
+
+    // Check if client is admin
+    VerifyOrReturnError(CHIP_NO_ERROR == CheckAdmin(commandObj, commandPath, isClientAdmin), InteractionModel::Status::Failure);
+
     bool isFirstEntryForFabric = false;
-    ICDMonitoringTable table(*mStorage, fabric_index, mICDConfigurationData->GetClientsSupportedPerFabric(), mSymmetricKeystore);
+    ICDMonitoringTable table(*mStorage, fabricIndex, mICDConfigurationData->GetClientsSupportedPerFabric(), mSymmetricKeystore);
 
     // Get current entry, if exists
     ICDMonitoringEntry entry(mSymmetricKeystore);
-    CHIP_ERROR err = table.Find(node_id, entry);
+    CHIP_ERROR err = table.Find(nodeId, entry);
     if (CHIP_NO_ERROR == err)
     {
         // Existing entry: Validate Key if, and only if, the ISD does NOT have administrator permissions
-        if (!isAdmin)
+        if (!isClientAdmin)
         {
-            VerifyOrReturnError(verification_key.HasValue(), InteractionModel::Status::Failure);
-            VerifyOrReturnError(entry.IsKeyEquivalent(verification_key.Value()), InteractionModel::Status::Failure);
+            VerifyOrReturnError(verificationKey.HasValue(), InteractionModel::Status::Failure);
+            VerifyOrReturnError(entry.IsKeyEquivalent(verificationKey.Value()), InteractionModel::Status::Failure);
         }
     }
     else if (CHIP_ERROR_NOT_FOUND == err)
@@ -231,8 +265,8 @@ Status ICDManagementServer::RegisterClient(FabricIndex fabric_index, NodeId node
     }
 
     // Save
-    entry.checkInNodeID    = node_id;
-    entry.monitoredSubject = monitored_subject;
+    entry.checkInNodeID    = nodeId;
+    entry.monitoredSubject = monitoredSubject;
     if (entry.keyHandleValid)
     {
         entry.DeleteKey();
@@ -262,19 +296,27 @@ Status ICDManagementServer::RegisterClient(FabricIndex fabric_index, NodeId node
     return InteractionModel::Status::Success;
 }
 
-Status ICDManagementServer::UnregisterClient(FabricIndex fabric_index, NodeId node_id, Optional<ByteSpan> verificationKey,
-                                             bool isAdmin)
+Status ICDManagementServer::UnregisterClient(CommandHandler * commandObj, const ConcreteCommandPath & commandPath,
+                                             const Commands::UnregisterClient::DecodableType & commandData)
 {
-    ICDMonitoringTable table(*mStorage, fabric_index, mICDConfigurationData->GetClientsSupportedPerFabric(), mSymmetricKeystore);
+    FabricIndex fabricIndex            = commandObj->GetAccessingFabricIndex();
+    NodeId nodeId                      = commandData.checkInNodeID;
+    Optional<ByteSpan> verificationKey = commandData.verificationKey;
+    bool isClientAdmin                 = false;
+
+    // Check if client is admin
+    VerifyOrReturnError(CHIP_NO_ERROR == CheckAdmin(commandObj, commandPath, isClientAdmin), InteractionModel::Status::Failure);
+
+    ICDMonitoringTable table(*mStorage, fabricIndex, mICDConfigurationData->GetClientsSupportedPerFabric(), mSymmetricKeystore);
 
     // Get current entry, if exists
     ICDMonitoringEntry entry(mSymmetricKeystore);
-    CHIP_ERROR err = table.Find(node_id, entry);
+    CHIP_ERROR err = table.Find(nodeId, entry);
     VerifyOrReturnError(CHIP_ERROR_NOT_FOUND != err, InteractionModel::Status::NotFound);
     VerifyOrReturnError(CHIP_NO_ERROR == err, InteractionModel::Status::Failure);
 
     // Existing entry: Validate Key if, and only if, the ISD has NOT administrator permissions
-    if (!isAdmin)
+    if (!isClientAdmin)
     {
         VerifyOrReturnError(verificationKey.HasValue(), InteractionModel::Status::Failure);
         VerifyOrReturnError(entry.IsKeyEquivalent(verificationKey.Value()), InteractionModel::Status::Failure);
@@ -291,7 +333,7 @@ Status ICDManagementServer::UnregisterClient(FabricIndex fabric_index, NodeId no
     return InteractionModel::Status::Success;
 }
 
-Status ICDManagementServer::StayActiveRequest(FabricIndex fabric_index)
+Status ICDManagementServer::StayActiveRequest(FabricIndex fabricIndex)
 {
     // TODO: Implementent stay awake logic for end device
     // https://github.com/project-chip/connectedhomeip/issues/24259
@@ -304,22 +346,6 @@ void ICDManagementServer::TriggerICDMTableUpdatedEvent()
     ICDNotifier::GetInstance().BroadcastICDManagementEvent(ICDListener::ICDManagementEvents::kTableUpdated);
 }
 
-CHIP_ERROR ICDManagementServer::CheckAdmin(CommandHandler * commandObj, const ConcreteCommandPath & commandPath, bool & isAdmin)
-{
-    RequestPath requestPath{ .cluster = commandPath.mClusterId, .endpoint = commandPath.mEndpointId };
-    CHIP_ERROR err = GetAccessControl().Check(commandObj->GetSubjectDescriptor(), requestPath, Privilege::kAdminister);
-    if (CHIP_NO_ERROR == err)
-    {
-        isAdmin = true;
-    }
-    else if (CHIP_ERROR_ACCESS_DENIED == err)
-    {
-        isAdmin = false;
-        err     = CHIP_NO_ERROR;
-    }
-    return err;
-}
-
 void ICDManagementServer::Init(PersistentStorageDelegate & storage, Crypto::SymmetricKeystore * symmetricKeystore,
                                ICDConfigurationData & icdConfigurationData)
 {
@@ -339,17 +365,10 @@ void ICDManagementServer::Init(PersistentStorageDelegate & storage, Crypto::Symm
 bool emberAfIcdManagementClusterRegisterClientCallback(CommandHandler * commandObj, const ConcreteCommandPath & commandPath,
                                                        const Commands::RegisterClient::DecodableType & commandData)
 {
-    InteractionModel::Status status = InteractionModel::Status::Failure;
-    bool isAdmin                    = false;
-    uint32_t icdCounter             = 0;
-    ICDManagementServer server;
+    uint32_t icdCounter = 0;
 
-    if (CHIP_NO_ERROR == server.CheckAdmin(commandObj, commandPath, isAdmin))
-    {
-        status =
-            server.RegisterClient(commandObj->GetAccessingFabricIndex(), commandData.checkInNodeID, commandData.monitoredSubject,
-                                  commandData.key, commandData.verificationKey, isAdmin, icdCounter);
-    }
+    ICDManagementServer server;
+    InteractionModel::Status status = server.RegisterClient(commandObj, commandPath, commandData, icdCounter);
 
     if (InteractionModel::Status::Success == status)
     {
@@ -371,15 +390,8 @@ bool emberAfIcdManagementClusterRegisterClientCallback(CommandHandler * commandO
 bool emberAfIcdManagementClusterUnregisterClientCallback(CommandHandler * commandObj, const ConcreteCommandPath & commandPath,
                                                          const Commands::UnregisterClient::DecodableType & commandData)
 {
-    InteractionModel::Status status = InteractionModel::Status::Failure;
-    bool isAdmin                    = false;
     ICDManagementServer server;
-
-    if (CHIP_NO_ERROR == server.CheckAdmin(commandObj, commandPath, isAdmin))
-    {
-        status = server.UnregisterClient(commandObj->GetAccessingFabricIndex(), commandData.checkInNodeID,
-                                         commandData.verificationKey, isAdmin);
-    }
+    InteractionModel::Status status = server.UnregisterClient(commandObj, commandPath, commandData);
 
     commandObj->AddStatus(commandPath, status);
     return true;
diff --git a/src/app/clusters/icd-management-server/icd-management-server.h b/src/app/clusters/icd-management-server/icd-management-server.h
@@ -17,6 +17,7 @@
 
 #pragma once
 
+#include <app-common/zap-generated/cluster-objects.h>
 #include <app/CommandHandler.h>
 #include <app/ConcreteAttributePath.h>
 #include <app/icd/ICDConfigurationData.h>
@@ -37,16 +38,22 @@ class ICDManagementServer
 
     static void Init(chip::PersistentStorageDelegate & storage, chip::Crypto::SymmetricKeystore * symmetricKeystore,
                      chip::ICDConfigurationData & ICDConfigurationData);
-    Status RegisterClient(chip::FabricIndex fabric_index, chip::NodeId node_id, uint64_t monitored_subject, chip::ByteSpan key,
-                          chip::Optional<chip::ByteSpan> verification_key, bool is_admin, uint32_t & icdCounter);
 
-    Status UnregisterClient(chip::FabricIndex fabric_index, chip::NodeId node_id, chip::Optional<chip::ByteSpan> verificationKey,
-                            bool is_admin);
+    /**
+     * @brief Function that executes the business logic of the RegisterClient Command
+     *
+     * @param[out] icdCounter If function succeeds, icdCounter will have the current value of the ICDCounter stored in the
+     * ICDConfigurationData If function fails, icdCounter will be unchanged
+     * @return Status
+     */
+    Status RegisterClient(chip::app::CommandHandler * commandObj, const chip::app::ConcreteCommandPath & commandPath,
+                          const chip::app::Clusters::IcdManagement::Commands::RegisterClient::DecodableType & commandData,
+                          uint32_t & icdCounter);
 
-    Status StayActiveRequest(chip::FabricIndex fabric_index);
+    Status UnregisterClient(chip::app::CommandHandler * commandObj, const chip::app::ConcreteCommandPath & commandPath,
+                            const chip::app::Clusters::IcdManagement::Commands::UnregisterClient::DecodableType & commandData);
 
-    CHIP_ERROR CheckAdmin(chip::app::CommandHandler * commandObj, const chip::app::ConcreteCommandPath & commandPath,
-                          bool & is_admin);
+    Status StayActiveRequest(chip::FabricIndex fabricIndex);
 
 private:
     /**
