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

Use DataModel::Provider provided command information to handle command validation #35897

Merged
merged 30 commits into from
Oct 9, 2024
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
17dcef6
A first pass to implement better invoke logic decoupling
andreilitvin Oct 3, 2024
3ed30ea
Fix some includes
andreilitvin Oct 3, 2024
6bd969d
Fix some includes
andreilitvin Oct 3, 2024
b63c918
More include fixes
andreilitvin Oct 3, 2024
0912e9a
Fix dependency to make things build
andreilitvin Oct 3, 2024
6386bf7
Fixed tests
andreilitvin Oct 3, 2024
4188ae5
Fix includes
andreilitvin Oct 3, 2024
af7552f
Restyle
andreilitvin Oct 3, 2024
129ad24
Fix naming typo
andreilitvin Oct 3, 2024
b08d16e
Fix tizen build
andreilitvin Oct 3, 2024
03dcc81
Fix condition typo
andy31415 Oct 4, 2024
d197a94
Keep group command logic to use continue instead of status return ...…
andy31415 Oct 4, 2024
f83397f
Merge branch 'master' into use_dm_for_command_validation
andy31415 Oct 4, 2024
4ec72e5
Minor update to kick restyler
andy31415 Oct 4, 2024
3d3e7f6
one more decoupling update: sligtly more inefficient, however likely …
andy31415 Oct 4, 2024
5ca78df
Renamed based on code review feedback
andy31415 Oct 4, 2024
e03197d
Restyled by clang-format
restyled-commits Oct 4, 2024
3cf6ff3
Fix some code review comments
andreilitvin Oct 8, 2024
6e77713
Review comment: default privilege to operate
andreilitvin Oct 8, 2024
99d0629
Fix up renames
andreilitvin Oct 8, 2024
5981a57
Fix hex prefix
andreilitvin Oct 8, 2024
f0968ae
Restyled by clang-format
restyled-commits Oct 8, 2024
649c50c
Merge branch 'master' into use_dm_for_command_validation
andy31415 Oct 8, 2024
c8efbbf
Fix cirque after log formatting change. Load bearing log ....
andreilitvin Oct 8, 2024
ae63895
Merge branch 'use_dm_for_command_validation' of github.com:andy31415/…
andreilitvin Oct 8, 2024
4f1f196
Revert file change that I did not mean to change
andreilitvin Oct 8, 2024
bab965a
Update src/app/CommandHandlerImpl.cpp
andy31415 Oct 9, 2024
fcdb555
Merge branch 'master' into use_dm_for_command_validation
andy31415 Oct 9, 2024
38e3bdb
Remove separate member for accessing fabric index
andy31415 Oct 9, 2024
e497eec
Restyled by clang-format
restyled-commits Oct 9, 2024
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
4 changes: 1 addition & 3 deletions src/app/CommandHandlerImpl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -524,9 +524,7 @@ Status CommandHandlerImpl::ProcessGroupCommandDataIB(CommandDataIB::Parser & aCo
Status preCheckStatus = mpCallback->ValidateCommandCanBeDispatched(request);
andy31415 marked this conversation as resolved.
Show resolved Hide resolved
if (preCheckStatus != Status::Success)
{
// NOTE: an expected error is CHIP_ERROR_ACCESS_DENIED, but there could be other unexpected errors;
// therefore, keep processing subsequent commands, and if any errors continue, those subsequent
// commands will likewise fail.
// Command failed for a specific path, keep trying the rest of the commands still
andy31415 marked this conversation as resolved.
Show resolved Hide resolved
continue;
}
}
Expand Down
22 changes: 10 additions & 12 deletions src/app/InteractionModelEngine.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1685,22 +1685,22 @@ void InteractionModelEngine::DispatchCommand(CommandHandlerImpl & apCommandObj,
Protocols::InteractionModel::Status InteractionModelEngine::ValidateCommandCanBeDispatched(const DataModel::InvokeRequest & request)
{

Status status = CommandCheckExists(request.path);
Status status = CheckCommandExistence(request.path);

if (status != Status::Success)
{
ChipLogDetail(DataManagement, "No command " ChipLogFormatMEI " in Cluster " ChipLogFormatMEI " on Endpoint 0x%x",
ChipLogDetail(DataManagement, "No command " ChipLogFormatMEI " in Cluster " ChipLogFormatMEI " on Endpoint %u",
ChipLogValueMEI(request.path.mCommandId), ChipLogValueMEI(request.path.mClusterId), request.path.mEndpointId);
return status;
}

status = CommandCheckACL(request);
status = CheckCommandAccess(request);
VerifyOrReturnValue(status == Status::Success, status);

return CommandCheckFlags(request);
return CheckCommandFlags(request);
}

Protocols::InteractionModel::Status InteractionModelEngine::CommandCheckACL(const DataModel::InvokeRequest & aRequest)
Protocols::InteractionModel::Status InteractionModelEngine::CheckCommandAccess(const DataModel::InvokeRequest & aRequest)
{
if (!aRequest.subjectDescriptor.has_value())
bzbarsky-apple marked this conversation as resolved.
Show resolved Hide resolved
{
Expand All @@ -1713,10 +1713,8 @@ Protocols::InteractionModel::Status InteractionModelEngine::CommandCheckACL(cons
.entityId = aRequest.path.mCommandId };
#if CHIP_CONFIG_USE_DATA_MODEL_INTERFACE
std::optional<DataModel::CommandInfo> commandInfo = mDataModelProvider->GetAcceptedCommandInfo(aRequest.path);
// This is checked by previous validations, so it should not happen
VerifyOrReturnValue(commandInfo.has_value(), Status::UnsupportedCommand);

Access::Privilege minimumRequiredPrivilege = commandInfo->invokePrivilege;
Access::Privilege minimumRequiredPrivilege =
commandInfo.has_value() ? commandInfo->invokePrivilege : Access::Privilege::kOperate;
#else
Access::Privilege minimumRequiredPrivilege = RequiredPrivilege::ForInvokeCommand(aRequest.path);
#endif
Expand All @@ -1733,12 +1731,12 @@ Protocols::InteractionModel::Status InteractionModelEngine::CommandCheckACL(cons
return Status::Success;
}

Protocols::InteractionModel::Status InteractionModelEngine::CommandCheckFlags(const DataModel::InvokeRequest & aRequest)
Protocols::InteractionModel::Status InteractionModelEngine::CheckCommandFlags(const DataModel::InvokeRequest & aRequest)
{
#if CHIP_CONFIG_USE_DATA_MODEL_INTERFACE
std::optional<DataModel::CommandInfo> commandInfo = mDataModelProvider->GetAcceptedCommandInfo(aRequest.path);
// This is checked by previous validations, so it should not happen
andy31415 marked this conversation as resolved.
Show resolved Hide resolved
VerifyOrReturnValue(commandInfo.has_value(), Status::UnsupportedCommand);
VerifyOrDie(commandInfo.has_value());

const bool commandNeedsTimedInvoke = commandInfo->flags.Has(DataModel::CommandQualityFlags::kTimed);
const bool commandIsFabricScoped = commandInfo->flags.Has(DataModel::CommandQualityFlags::kFabricScoped);
Expand Down Expand Up @@ -1768,7 +1766,7 @@ Protocols::InteractionModel::Status InteractionModelEngine::CommandCheckFlags(co
return Status::Success;
}

Protocols::InteractionModel::Status InteractionModelEngine::CommandCheckExists(const ConcreteCommandPath & aCommandPath)
Protocols::InteractionModel::Status InteractionModelEngine::CheckCommandExistence(const ConcreteCommandPath & aCommandPath)
{
#if CHIP_CONFIG_USE_DATA_MODEL_INTERFACE
auto provider = GetDataModelProvider();
Expand Down
6 changes: 3 additions & 3 deletions src/app/InteractionModelEngine.h
Original file line number Diff line number Diff line change
Expand Up @@ -613,9 +613,9 @@ class InteractionModelEngine : public Messaging::UnsolicitedMessageHandler,
void ShutdownMatchingSubscriptions(const Optional<FabricIndex> & aFabricIndex = NullOptional,
const Optional<NodeId> & aPeerNodeId = NullOptional);

Status CommandCheckExists(const ConcreteCommandPath & aCommandPath);
Status CommandCheckACL(const DataModel::InvokeRequest & aRequest);
Status CommandCheckFlags(const DataModel::InvokeRequest & aRequest);
Status CheckCommandExistence(const ConcreteCommandPath & aCommandPath);
Status CheckCommandAccess(const DataModel::InvokeRequest & aRequest);
Status CheckCommandFlags(const DataModel::InvokeRequest & aRequest);

/**
* Check if the given attribute path is a valid path in the data model provider.
Expand Down
2 changes: 1 addition & 1 deletion src/test_driver/linux-cirque/MobileDeviceTest.py
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ def run_controller_test(self):
"Toggle ep1 on/off from state 0 to 1",
"Received command for Endpoint=1 Cluster=0x0000_0006 Command=0x0000_0000",
"Toggle ep1 on/off from state 1 to 0",
"No command 0x0000_0001 in Cluster 0x0000_0006 on Endpoint 0xe9"]),
"No command 0x0000_0001 in Cluster 0x0000_0006 on Endpoint 233"]),
"Datamodel test failed: cannot find matching string from device {}".format(device_id))


Expand Down
Loading