Skip to content

Commit

Permalink
Access control cluster server improvements (#12690)
Browse files Browse the repository at this point in the history
* Access control cluster server improvements

- move implementation into anonymous namespace
- handle more errors during list encode
- move extension attribute storage to external (still not implemented)
- move cluster revision storage to external

* Regen and restyle again

* Regen and restyle again again

* Fix iteration in ACL read

Sentinel error is expected when iteration ends.
  • Loading branch information
mlepage-google authored and pull[bot] committed May 16, 2022
1 parent 7cb1a74 commit 4162170
Show file tree
Hide file tree
Showing 6 changed files with 468 additions and 464 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -979,7 +979,7 @@
"mfgCode": null,
"side": "server",
"included": 1,
"storageOption": "RAM",
"storageOption": "External",
"singleton": 0,
"bounded": 0,
"defaultValue": "",
Expand All @@ -994,7 +994,7 @@
"mfgCode": null,
"side": "server",
"included": 1,
"storageOption": "RAM",
"storageOption": "External",
"singleton": 0,
"bounded": 0,
"defaultValue": "1",
Expand Down
36 changes: 24 additions & 12 deletions src/app/clusters/access-control-server/access-control-server.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@ using namespace chip::Access;

namespace AccessControlCluster = chip::app::Clusters::AccessControl;

namespace {

struct AccessControlEntryCodec
{
static CHIP_ERROR Convert(AuthMode from, AccessControlCluster::AuthMode & to)
Expand Down Expand Up @@ -318,25 +320,31 @@ struct AccessControlEntryCodec
class AccessControlAttribute : public chip::app::AttributeAccessInterface
{
public:
AccessControlAttribute() : AttributeAccessInterface(Optional<EndpointId>(0), Clusters::AccessControl::Id) {}
AccessControlAttribute() : AttributeAccessInterface(Optional<EndpointId>(0), AccessControlCluster::Id) {}

CHIP_ERROR Read(const ConcreteReadAttributePath & aPath, AttributeValueEncoder & aEncoder) override;
CHIP_ERROR Write(const ConcreteDataAttributePath & aPath, AttributeValueDecoder & aDecoder) override;

static constexpr uint16_t ClusterRevision = 1;

private:
CHIP_ERROR ReadAcl(AttributeValueEncoder & aEncoder);
CHIP_ERROR ReadExtension(AttributeValueEncoder & aEncoder);
CHIP_ERROR WriteAcl(AttributeValueDecoder & aDecoder);
CHIP_ERROR WriteExtension(AttributeValueDecoder & aDecoder);
};

constexpr uint16_t AccessControlAttribute::ClusterRevision;

CHIP_ERROR AccessControlAttribute::Read(const ConcreteReadAttributePath & aPath, AttributeValueEncoder & aEncoder)
{
switch (aPath.mAttributeId)
{
case Clusters::AccessControl::Attributes::Acl::Id:
case AccessControlCluster::Attributes::Acl::Id:
return ReadAcl(aEncoder);
case Clusters::AccessControl::Attributes::Extension::Id:
case AccessControlCluster::Attributes::ClusterRevision::Id:
return aEncoder.Encode(ClusterRevision);
case AccessControlCluster::Attributes::Extension::Id:
return ReadExtension(aEncoder);
}

Expand All @@ -348,31 +356,31 @@ CHIP_ERROR AccessControlAttribute::ReadAcl(AttributeValueEncoder & aEncoder)
AccessControlEntryCodec codec;
AccessControl::EntryIterator iterator;

GetAccessControl().Entries(iterator);
ReturnErrorOnFailure(GetAccessControl().Entries(iterator));

CHIP_ERROR err = aEncoder.EncodeList([&](const auto & encoder) -> CHIP_ERROR {
while (iterator.Next(codec.entry) == CHIP_NO_ERROR)
return aEncoder.EncodeList([&](const auto & encoder) -> CHIP_ERROR {
CHIP_ERROR err;
while ((err = iterator.Next(codec.entry)) == CHIP_NO_ERROR)
{
encoder.Encode(codec);
ReturnErrorOnFailure(encoder.Encode(codec));
}
ReturnErrorCodeIf(err != CHIP_NO_ERROR && err != CHIP_ERROR_SENTINEL, err);
return CHIP_NO_ERROR;
});

return err;
}

CHIP_ERROR AccessControlAttribute::ReadExtension(AttributeValueEncoder & aEncoder)
{
return CHIP_NO_ERROR;
return aEncoder.EncodeList([&](const auto & encoder) -> CHIP_ERROR { return CHIP_NO_ERROR; });
}

CHIP_ERROR AccessControlAttribute::Write(const ConcreteDataAttributePath & aPath, AttributeValueDecoder & aDecoder)
{
switch (aPath.mAttributeId)
{
case Clusters::AccessControl::Attributes::Acl::Id:
case AccessControlCluster::Attributes::Acl::Id:
return WriteAcl(aDecoder);
case Clusters::AccessControl::Attributes::Extension::Id:
case AccessControlCluster::Attributes::Extension::Id:
return WriteExtension(aDecoder);
}

Expand Down Expand Up @@ -419,13 +427,17 @@ CHIP_ERROR AccessControlAttribute::WriteAcl(AttributeValueDecoder & aDecoder)

CHIP_ERROR AccessControlAttribute::WriteExtension(AttributeValueDecoder & aDecoder)
{
DataModel::DecodableList<AccessControlCluster::Structs::ExtensionEntry::DecodableType> list;
ReturnErrorOnFailure(aDecoder.Decode(list));
return CHIP_NO_ERROR;
}

AccessControlAttribute gAttribute;

AccessControl gAccessControl(Examples::GetAccessControlDelegate());

} // namespace

void MatterAccessControlPluginServerInitCallback()
{
registerAttributeAccessOverride(&gAttribute);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ limitations under the License.
<access op="write" privilege="administer"/>
<access modifier="fabric-scoped"/>
</attribute>
<attribute side="server" code="0x0001" define="EXTENSION" type="ARRAY" length="4" entryType="ExtensionEntry" writable="true">
<attribute side="server" code="0x0001" define="EXTENSION" type="ARRAY" entryType="ExtensionEntry" writable="true">
<description>Extension</description>
<access op="read" privilege="administer"/>
<access op="write" privilege="administer"/>
Expand Down
6 changes: 3 additions & 3 deletions src/controller/data_model/controller-clusters.zap
Original file line number Diff line number Diff line change
Expand Up @@ -1427,7 +1427,7 @@
"mfgCode": null,
"side": "client",
"included": 1,
"storageOption": "RAM",
"storageOption": "External",
"singleton": 0,
"bounded": 0,
"defaultValue": "1",
Expand Down Expand Up @@ -1468,7 +1468,7 @@
"mfgCode": null,
"side": "server",
"included": 1,
"storageOption": "RAM",
"storageOption": "External",
"singleton": 0,
"bounded": 0,
"defaultValue": "",
Expand All @@ -1483,7 +1483,7 @@
"mfgCode": null,
"side": "server",
"included": 1,
"storageOption": "RAM",
"storageOption": "External",
"singleton": 0,
"bounded": 0,
"defaultValue": "1",
Expand Down
Loading

0 comments on commit 4162170

Please sign in to comment.