Move the access control ownership and initialization to Server. (#14680)

* Move the access control ownership and initialization to Server.

* Improvements from mlepage-google.

examples/platform/linux/AppMain.cpp

@@ -33,8 +33,6 @@
 #include <credentials/examples/DefaultDeviceAttestationVerifier.h>
 #include <credentials/examples/DeviceAttestationCredsExample.h>
 
-#include <access/examples/ExampleAccessControlDelegate.h>
-
 #include <lib/support/CHIPMem.h>
 #include <lib/support/ScopedBuffer.h>
 #include <setup_payload/QRCodeSetupPayloadGenerator.h>
@@ -66,27 +64,6 @@ using namespace chip::DeviceLayer;
 using namespace chip::Inet;
 using namespace chip::Transport;
 
-class GeneralStorageDelegate : public PersistentStorageDelegate
-{
-    CHIP_ERROR SyncGetKeyValue(const char * key, void * buffer, uint16_t & size) override
-    {
-        ChipLogProgress(NotSpecified, "Retrieved value from general storage.");
-        return PersistedStorage::KeyValueStoreMgr().Get(key, buffer, size);
-    }
-
-    CHIP_ERROR SyncSetKeyValue(const char * key, const void * value, uint16_t size) override
-    {
-        ChipLogProgress(NotSpecified, "Stored value in general storage");
-        return PersistedStorage::KeyValueStoreMgr().Put(key, value, size);
-    }
-
-    CHIP_ERROR SyncDeleteKeyValue(const char * key) override
-    {
-        ChipLogProgress(NotSpecified, "Delete value in general storage");
-        return PersistedStorage::KeyValueStoreMgr().Delete(key);
-    }
-};
-
 #if defined(ENABLE_CHIP_SHELL)
 using chip::Shell::Engine;
 #endif
@@ -111,8 +88,6 @@ void EventHandler(const DeviceLayer::ChipDeviceEvent * event, intptr_t arg)
         ChipLogProgress(DeviceLayer, "Receive kCHIPoBLEConnectionEstablished");
     }
 }
-
-GeneralStorageDelegate gAclStorageDelegate;
 } // namespace
 
 #if CHIP_DEVICE_CONFIG_ENABLE_WPA
@@ -161,8 +136,6 @@ int ChipLinuxAppInit(int argc, char ** argv)
 
     PrintOnboardingCodes(LinuxDeviceOptions::GetInstance().payload);
 
-    Access::Examples::SetAccessControlDelegateStorage(&gAclStorageDelegate);
-
 #if defined(PW_RPC_ENABLED)
     rpc::Init();
     ChipLogProgress(NotSpecified, "PW_RPC initialized.");

src/access/examples/ExampleAccessControlDelegate.cpp

@@ -1069,12 +1069,13 @@ class AccessControlDelegate : public AccessControl::Delegate
         CHIP_ERROR err = LoadFromFlash();
         if (err != CHIP_NO_ERROR)
         {
+            ChipLogDetail(DataManagement, "AccessControl: unable to load stored ACL entries; using empty list instead");
             for (auto & storage : EntryStorage::acl)
             {
                 storage.Clear();
             }
         }
-        return err;
+        return CHIP_NO_ERROR;
     }
 
     CHIP_ERROR Finish() override
@@ -1309,17 +1310,11 @@ namespace chip {
 namespace Access {
 namespace Examples {
 
-AccessControl::Delegate & GetAccessControlDelegate()
+AccessControl::Delegate & GetAccessControlDelegate(PersistentStorageDelegate * storageDelegate)
 {
     static AccessControlDelegate accessControlDelegate;
-    return accessControlDelegate;
-}
-
-void SetAccessControlDelegateStorage(chip::PersistentStorageDelegate * storageDelegate)
-{
-    ChipLogDetail(DataManagement, "Examples::SetAccessControlDelegateStorage");
-    AccessControlDelegate & accessControlDelegate = static_cast<AccessControlDelegate &>(GetAccessControlDelegate());
     accessControlDelegate.SetStorageDelegate(storageDelegate);
+    return accessControlDelegate;
 }
 
 } // namespace Examples

src/access/examples/ExampleAccessControlDelegate.h

@@ -23,9 +23,16 @@ namespace chip {
 namespace Access {
 namespace Examples {
 
-AccessControl::Delegate & GetAccessControlDelegate();
-
-void SetAccessControlDelegateStorage(chip::PersistentStorageDelegate * storageDelegate);
+/**
+ * @brief Get a global instance of the access control delegate implemented in this module.
+ *
+ * NOTE: This function should be followed by an ::Init() method call. This function does
+ *       not manage lifecycle considerations.
+ *
+ * @param storageDelegate Storage instance to access persisted ACL data.
+ * @return a reference to the AccessControl::Delegate singleton.
+ */
+AccessControl::Delegate & GetAccessControlDelegate(PersistentStorageDelegate * storageDelegate);
 
 } // namespace Examples
 } // namespace Access

src/access/tests/TestAccessControl.cpp

@@ -33,7 +33,7 @@ using Entry         = AccessControl::Entry;
 using EntryIterator = AccessControl::EntryIterator;
 using Target        = Entry::Target;
 
-AccessControl accessControl(Examples::GetAccessControlDelegate());
+AccessControl accessControl(Examples::GetAccessControlDelegate(nullptr));
 
 constexpr ClusterId kOnOffCluster         = 0x0006;
 constexpr ClusterId kLevelControlCluster  = 0x0008;

src/app/clusters/access-control-server/access-control-server.cpp

@@ -16,7 +16,6 @@
  */
 
 #include <access/AccessControl.h>
-#include <access/examples/ExampleAccessControlDelegate.h>
 
 #include <app-common/zap-generated/af-structs.h>
 #include <app-common/zap-generated/cluster-objects.h>
@@ -503,16 +502,9 @@ CHIP_ERROR AccessControlAttribute::WriteExtension(AttributeValueDecoder & aDecod
 
 AccessControlAttribute gAttribute;
 
-AccessControl gAccessControl(Examples::GetAccessControlDelegate());
-
 } // namespace
 
 void MatterAccessControlPluginServerInitCallback()
 {
     registerAttributeAccessOverride(&gAttribute);
-
-    // TODO: move access control setup to lower level
-    //       (it's OK and convenient here during development)
-    gAccessControl.Init();
-    SetAccessControl(gAccessControl);
 }

src/app/server/Server.cpp

@@ -17,6 +17,8 @@
 
 #include <app/server/Server.h>
 
+#include <access/examples/ExampleAccessControlDelegate.h>
+
 #include <app/EventManagement.h>
 #include <app/InteractionModelEngine.h>
 #include <app/server/Dnssd.h>
@@ -92,7 +94,7 @@ Server::Server() :
         .devicePool        = &mDevicePool,
         .dnsResolver       = nullptr,
     }), mCommissioningWindowManager(this), mGroupsProvider(mDeviceStorage),
-    mAttributePersister(mDeviceStorage)
+    mAttributePersister(mDeviceStorage), mAccessControl(Access::Examples::GetAccessControlDelegate(&mDeviceStorage))
 {}
 
 CHIP_ERROR Server::Init(AppDelegate * delegate, uint16_t secureServicePort, uint16_t unsecureServicePort)
@@ -128,6 +130,11 @@ CHIP_ERROR Server::Init(AppDelegate * delegate, uint16_t secureServicePort, uint
     SuccessOrExit(err);
     SetGroupDataProvider(&mGroupsProvider);
 
+    // Access control must be initialized after mDeviceStorage.
+    err = mAccessControl.Init();
+    SuccessOrExit(err);
+    Access::SetAccessControl(mAccessControl);
+
     // Init transport before operations with secure session mgr.
     err = mTransports.Init(UdpListenParameters(DeviceLayer::UDPEndPointManager())
                                .SetAddressType(IPAddressType::kIPv6)

src/app/server/Server.h

@@ -17,6 +17,7 @@
 
 #pragma once
 
+#include <access/AccessControl.h>
 #include <app/CASEClientPool.h>
 #include <app/CASESessionManager.h>
 #include <app/DefaultAttributePersistenceProvider.h>
@@ -194,6 +195,8 @@ class Server
     app::DefaultAttributePersistenceProvider mAttributePersister;
     GroupDataProviderListener mListener;
 
+    Access::AccessControl mAccessControl;
+
     // TODO @ceille: Maybe use OperationalServicePort and CommissionableServicePort
     uint16_t mSecuredServicePort;
     uint16_t mUnsecuredServicePort;