Move ::WriteAttribute validation inside Interaction Model Write Handling

src/app/WriteHandler.cpp

@@ -19,6 +19,8 @@
 #include <app/AppConfig.h>
 #include <app/AttributeAccessInterfaceRegistry.h>
 #include <app/AttributeValueDecoder.h>
+#include <app/ConcreteAttributePath.h>
+#include <app/GlobalAttributes.h>
 #include <app/InteractionModelEngine.h>
 #include <app/MessageDef/EventPathIB.h>
 #include <app/MessageDef/StatusIB.h>
@@ -759,8 +761,9 @@ void WriteHandler::MoveToState(const State aTargetState)
 }
 
 DataModel::ActionReturnStatus WriteHandler::CheckWriteAllowed(const Access::SubjectDescriptor & aSubject,
-                                                              const ConcreteDataAttributePath & aPath)
+                                                              const ConcreteAttributePath & aPath)
 {
+
     // TODO: ordering is to check writability/existence BEFORE ACL and this seems wrong, however
     //       existing unit tests (TC_AcessChecker.py) validate that we get UnsupportedWrite instead of UnsupportedAccess
     //
@@ -769,6 +772,10 @@ DataModel::ActionReturnStatus WriteHandler::CheckWriteAllowed(const Access::Subj
     //       and tests and implementation
     //
     //       Open issue that needs fixing: https://github.com/project-chip/connectedhomeip/issues/33735
+
+    // First check things that are not even in metadata. These are read.only.
+    VerifyOrReturnValue(!IsSupportedGlobalAttributeNotInMetadata(aPath.mAttributeId), Status::UnsupportedWrite);
+
     std::optional<DataModel::AttributeEntry> attributeEntry;
     DataModel::AttributeFinder finder(mDataModelProvider);
 
@@ -786,15 +793,10 @@ DataModel::ActionReturnStatus WriteHandler::CheckWriteAllowed(const Access::Subj
     bool checkAcl = true;
     if (mLastSuccessfullyWrittenPath.has_value())
     {
-        // NOTE: explicit cast/check only for attribute path and nothing else.
-        //
-        //       Specifically we DO NOT use `operator==` because `aPath` is a
-        //       DATA path (contains a list index) and we do not want
-        //       mLastSuccessfullyWrittenPath to be auto-cast to a
-        //       data path with a empty list and fail the compare.
+        // only validate ACL if path has changed
         //
-        //       This inline endpoint/cluster/attribute compare results in
-        //       smaller code as well.
+        // Note that this is NOT operator==: we could do `checkAcl == (aPath != *mLastSuccessfullyWrittenPath)`
+        // however that seems to use more flash.
         if ((aPath.mEndpointId == mLastSuccessfullyWrittenPath->mEndpointId) &&
             (aPath.mClusterId == mLastSuccessfullyWrittenPath->mClusterId) &&
             (aPath.mAttributeId == mLastSuccessfullyWrittenPath->mAttributeId))

src/app/WriteHandler.h

@@ -192,7 +192,7 @@ class WriteHandler : public Messaging::ExchangeDelegate
     ///
     /// Returns a success status if all is ok, failure otherwise.
     DataModel::ActionReturnStatus CheckWriteAllowed(const Access::SubjectDescriptor & aSubject,
-                                                    const ConcreteDataAttributePath & aPath);
+                                                    const ConcreteAttributePath & aPath);
 
     // Write the given data to the given path
     CHIP_ERROR WriteClusterData(const Access::SubjectDescriptor & aSubject, const ConcreteDataAttributePath & aPath,