Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix ACL constraint error (return constraint error instead of failure when the ACL entry is invalid) #20736

Merged
merged 35 commits into from
Jul 18, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
baa8bd9
Define a test for access control constraints
andy31415 Jul 14, 2022
5996f4e
Typgo fixes
andy31415 Jul 14, 2022
1543c01
Restyle
andy31415 Jul 14, 2022
1e3f47f
Fix some text and formatting
andy31415 Jul 14, 2022
2342c31
Zap regen
andy31415 Jul 14, 2022
a7176fb
Make ACL cluster return constraint error if the ACL entry is not valid
andy31415 Jul 14, 2022
2b2edd3
Add one more test: Step 31
andy31415 Jul 14, 2022
fc5a873
Zap regen for the new test
andy31415 Jul 14, 2022
259e200
Added the rest of the tests from #20672
andy31415 Jul 14, 2022
42fe5d9
Restyle
andy31415 Jul 14, 2022
b0f56b4
Removed some steps: too large numbers for subjects, cannot be represe…
andy31415 Jul 14, 2022
ebe3c6f
Zap regen
andy31415 Jul 14, 2022
4658b23
Restyle
andy31415 Jul 14, 2022
5497e77
Fix build, add more ACL changes, zap regen after adjusting test case …
andy31415 Jul 14, 2022
db485b7
Fix more things to return CONSTRAINT_ERROR
andy31415 Jul 14, 2022
bac61d9
Convert more invalid argument to constraint errors. This is not ideal…
andy31415 Jul 14, 2022
e39f9bb
Restyle
andy31415 Jul 14, 2022
b6df250
Fix comments in yaml
andy31415 Jul 14, 2022
2e102d4
Restyle
andy31415 Jul 14, 2022
af66386
Restyle messed things up. Corrected it
andy31415 Jul 14, 2022
d0b9461
One more comment fix
andy31415 Jul 14, 2022
be0da15
Restyle
andy31415 Jul 14, 2022
0482a60
Split out IM status code header and cpp into a separate library for l…
andy31415 Jul 14, 2022
9587ced
Restyle
andy31415 Jul 14, 2022
3ec4f06
Also update TestAccessControlCluster
andy31415 Jul 14, 2022
96293ee
Zap regen
andy31415 Jul 14, 2022
5424f7d
Updated test ACL error codes a bit and zap regen
andy31415 Jul 14, 2022
ffa30f5
Update logic to centrailize error code processing location
andy31415 Jul 15, 2022
8a0b8b7
Added unit test for step 35 as well (pass)
andy31415 Jul 15, 2022
6751858
Added even more tests and updated formatting of ACL a bit for readabi…
andy31415 Jul 15, 2022
992b66c
Restyle
andy31415 Jul 15, 2022
ce8433d
One more test for invalid privilege
andy31415 Jul 15, 2022
fde974e
Restyle
andy31415 Jul 15, 2022
bbc438d
Merge branch 'master' into acl_constraints
andy31415 Jul 18, 2022
aced054
Disable the TestAccessControlConstraints test on darwin - it is uncle…
andy31415 Jul 18, 2022
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/darwin-tests.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ jobs:
./scripts/run_in_build_env.sh \
"./scripts/tests/run_test_suite.py \
--chip-tool ./out/darwin-x64-darwin-framework-tool-${BUILD_VARIANT}/darwin-framework-tool \
--target-skip-glob '{TestGroupMessaging}' \
--target-skip-glob '{TestGroupMessaging,TestAccessControlConstraints}' \
run \
--iterations 1 \
--test-timeout-seconds 200 \
Expand Down
58 changes: 56 additions & 2 deletions src/app/clusters/access-control-server/access-control-server.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -55,14 +55,27 @@ class AccessControlAttribute : public AttributeAccessInterface, public EntryList
public:
AccessControlAttribute() : AttributeAccessInterface(Optional<EndpointId>(0), AccessControlCluster::Id) {}

/// IM-level implementation of read
///
/// Returns appropriately mapped CHIP_ERROR if applicable (may return CHIP_IM_GLOBAL_STATUS errors)
CHIP_ERROR Read(const ConcreteReadAttributePath & aPath, AttributeValueEncoder & aEncoder) override;

/// IM-level implementation of write
///
/// Returns appropriately mapped CHIP_ERROR if applicable (may return CHIP_IM_GLOBAL_STATUS errors)
CHIP_ERROR Write(const ConcreteDataAttributePath & aPath, AttributeValueDecoder & aDecoder) override;

public:
void OnEntryChanged(const SubjectDescriptor * subjectDescriptor, FabricIndex fabric, size_t index, const Entry * entry,
ChangeType changeType) override;

private:
/// Business logic implementation of write, returns generic CHIP_ERROR.
CHIP_ERROR ReadImpl(const ConcreteReadAttributePath & aPath, AttributeValueEncoder & aEncoder);

/// Business logic implementation of write, returns generic CHIP_ERROR.
CHIP_ERROR WriteImpl(const ConcreteDataAttributePath & aPath, AttributeValueDecoder & aDecoder);

CHIP_ERROR ReadAcl(AttributeValueEncoder & aEncoder);
CHIP_ERROR ReadExtension(AttributeValueEncoder & aEncoder);
CHIP_ERROR WriteAcl(const ConcreteDataAttributePath & aPath, AttributeValueDecoder & aDecoder);
Expand Down Expand Up @@ -125,7 +138,7 @@ CHIP_ERROR CheckExtensionEntryDataFormat(const ByteSpan & data)
return CHIP_NO_ERROR;
}

CHIP_ERROR AccessControlAttribute::Read(const ConcreteReadAttributePath & aPath, AttributeValueEncoder & aEncoder)
CHIP_ERROR AccessControlAttribute::ReadImpl(const ConcreteReadAttributePath & aPath, AttributeValueEncoder & aEncoder)
{
switch (aPath.mAttributeId)
{
Expand Down Expand Up @@ -205,7 +218,7 @@ CHIP_ERROR AccessControlAttribute::ReadExtension(AttributeValueEncoder & aEncode
});
}

CHIP_ERROR AccessControlAttribute::Write(const ConcreteDataAttributePath & aPath, AttributeValueDecoder & aDecoder)
CHIP_ERROR AccessControlAttribute::WriteImpl(const ConcreteDataAttributePath & aPath, AttributeValueDecoder & aDecoder)
{
switch (aPath.mAttributeId)
{
Expand Down Expand Up @@ -415,6 +428,47 @@ void AccessControlAttribute::OnEntryChanged(const SubjectDescriptor * subjectDes
ChipLogError(DataManagement, "AccessControlCluster: event failed %" CHIP_ERROR_FORMAT, err.Format());
}

CHIP_ERROR ChipErrorToImErrorMap(CHIP_ERROR err)
andy31415 marked this conversation as resolved.
Show resolved Hide resolved
{
// Map some common errors into an underlying IM error
// Separate logging is done to not lose the original error location in case such
// this are available.
CHIP_ERROR mappedError = err;

if (err == CHIP_ERROR_INVALID_ARGUMENT)
{
mappedError = CHIP_IM_GLOBAL_STATUS(ConstraintError);
}
else if (err == CHIP_ERROR_NOT_FOUND)
{
// Not found is generally also illegal argument: caused a lookup into an invalid location,
// like invalid subjects or targets.
mappedError = CHIP_IM_GLOBAL_STATUS(ConstraintError);
}
else if (err == CHIP_ERROR_NO_MEMORY)
{
mappedError = CHIP_IM_GLOBAL_STATUS(ResourceExhausted);
}

if (mappedError != err)
{
ChipLogError(DataManagement, "Re-mapped %" CHIP_ERROR_FORMAT " into %" CHIP_ERROR_FORMAT " for IM return codes",
err.Format(), mappedError.Format());
}

return mappedError;
}

CHIP_ERROR AccessControlAttribute::Read(const ConcreteReadAttributePath & aPath, AttributeValueEncoder & aEncoder)
{
return ChipErrorToImErrorMap(ReadImpl(aPath, aEncoder));
}

CHIP_ERROR AccessControlAttribute::Write(const ConcreteDataAttributePath & aPath, AttributeValueDecoder & aDecoder)
{
return ChipErrorToImErrorMap(WriteImpl(aPath, aDecoder));
}

} // namespace

void MatterAccessControlPluginServerInitCallback()
Expand Down
12 changes: 6 additions & 6 deletions src/app/tests/suites/TestAccessControlCluster.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,7 @@ tests:
},
]
response:
error: 1
error: CONSTRAINT_ERROR

- label: "Verify"
command: "readAttribute"
Expand Down Expand Up @@ -214,7 +214,7 @@ tests:
},
]
response:
error: 1
error: CONSTRAINT_ERROR

- label: "Verify"
command: "readAttribute"
Expand Down Expand Up @@ -251,7 +251,7 @@ tests:
},
]
response:
error: 1
error: CONSTRAINT_ERROR

- label: "Verify"
command: "readAttribute"
Expand Down Expand Up @@ -289,7 +289,7 @@ tests:
},
]
response:
error: 1
error: CONSTRAINT_ERROR

- label: "Verify"
command: "readAttribute"
Expand Down Expand Up @@ -348,7 +348,7 @@ tests:
},
]
response:
error: 1
error: FAILURE

- label: "Verify"
command: "readAttribute"
Expand Down Expand Up @@ -407,7 +407,7 @@ tests:
},
]
response:
error: 1
error: FAILURE

- label: "Verify"
command: "readAttribute"
Expand Down
Loading