Skip to content

Commit

Permalink
Hookup SubjectDescriptor in CommandHandler (#12953)
Browse files Browse the repository at this point in the history
Get the subject descriptor (from the exchange context session handle)
in the command handler so an access control check can be performed
before executing a command.

Unit tests did not always provide an exchange context for tests of
command interaction.

This required some special handling for test cases in actual
command handling code, and blocked cases (e.g. access control)
which require an exchange context.

So provide an exchange context even in test cases.
  • Loading branch information
mlepage-google authored Jan 14, 2022
1 parent 3b8a3c7 commit f12831d
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 15 deletions.
9 changes: 5 additions & 4 deletions src/app/CommandHandler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -98,16 +98,16 @@ CHIP_ERROR CommandHandler::ProcessInvokeRequest(System::PacketBufferHandle && pa
ReturnErrorOnFailure(invokeRequestMessage.GetTimedRequest(&mTimedRequest));
ReturnErrorOnFailure(invokeRequestMessage.GetInvokeRequests(&invokeRequests));

VerifyOrReturnError(mpExchangeCtx != nullptr, CHIP_ERROR_INCORRECT_STATE);

if (mTimedRequest != isTimedInvoke)
{
// The message thinks it should be part of a timed interaction but it's
// not, or vice versa. Spec says to Respond with UNSUPPORTED_ACCESS.
err = StatusResponse::Send(Protocols::InteractionModel::Status::UnsupportedAccess, mpExchangeCtx,
/* aExpectResponse = */ false);

// Some unit tests call this function in an abnormal state when we don't
// even have an exchange.
if (err != CHIP_NO_ERROR && mpExchangeCtx)
if (err != CHIP_NO_ERROR)
{
// We have to manually close the exchange, because we called
// WillSendMessage already.
Expand Down Expand Up @@ -252,9 +252,10 @@ CHIP_ERROR CommandHandler::ProcessCommandDataIB(CommandDataIB::Parser & aCommand
SuccessOrExit(err);

VerifyOrExit(mpCallback->CommandExists(concretePath), err = CHIP_ERROR_INVALID_PROFILE_ID);
VerifyOrExit(mpExchangeCtx != nullptr && mpExchangeCtx->HasSessionHandle(), err = CHIP_ERROR_INCORRECT_STATE);

{
Access::SubjectDescriptor subjectDescriptor; // TODO: get actual subject descriptor
Access::SubjectDescriptor subjectDescriptor = mpExchangeCtx->GetSessionHandle()->GetSubjectDescriptor();
Access::RequestPath requestPath{ .cluster = concretePath.mClusterId, .endpoint = concretePath.mEndpointId };
Access::Privilege requestPrivilege = Access::Privilege::kOperate; // TODO: get actual request privilege
err = Access::GetAccessControl().Check(subjectDescriptor, requestPath, requestPrivilege);
Expand Down
23 changes: 12 additions & 11 deletions src/app/tests/TestCommandInteraction.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -542,12 +542,18 @@ void TestCommandInteraction::TestCommandHandlerWithSendSimpleStatusCode(nlTestSu

void TestCommandInteraction::TestCommandHandlerWithProcessReceivedMsg(nlTestSuite * apSuite, void * apContext)
{
CHIP_ERROR err = CHIP_NO_ERROR;
TestContext & ctx = *static_cast<TestContext *>(apContext);
CHIP_ERROR err = CHIP_NO_ERROR;
app::CommandHandler commandHandler(&mockCommandHandlerDelegate);
System::PacketBufferHandle commandDatabuf = System::PacketBufferHandle::New(System::PacketBuffer::kMaxSize);

TestExchangeDelegate delegate;
commandHandler.mpExchangeCtx = ctx.NewExchangeToAlice(&delegate);

GenerateInvokeRequest(apSuite, apContext, commandDatabuf, true /*aNeedCommandData*/, /* aIsTimedRequest = */ false);
err = commandHandler.ProcessInvokeRequest(std::move(commandDatabuf), false);

ChipLogDetail(DataManagement, "###################################### %s", err.AsString());
NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR);
}

Expand All @@ -570,6 +576,7 @@ void TestCommandInteraction::TestCommandHandlerWithProcessReceivedNotExistComman

void TestCommandInteraction::TestCommandHandlerWithProcessReceivedEmptyDataMsg(nlTestSuite * apSuite, void * apContext)
{
TestContext & ctx = *static_cast<TestContext *>(apContext);
bool allBooleans[] = { true, false };
for (auto messageIsTimed : allBooleans)
{
Expand All @@ -579,19 +586,13 @@ void TestCommandInteraction::TestCommandHandlerWithProcessReceivedEmptyDataMsg(n
app::CommandHandler commandHandler(&mockCommandHandlerDelegate);
System::PacketBufferHandle commandDatabuf = System::PacketBufferHandle::New(System::PacketBuffer::kMaxSize);

chip::isCommandDispatched = false;
TestExchangeDelegate delegate;
commandHandler.mpExchangeCtx = ctx.NewExchangeToAlice(&delegate);

chip::isCommandDispatched = false;
GenerateInvokeRequest(apSuite, apContext, commandDatabuf, false /*aNeedCommandData*/, messageIsTimed);

err = commandHandler.ProcessInvokeRequest(std::move(commandDatabuf), transactionIsTimed);
if (messageIsTimed == transactionIsTimed)
{
NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR && chip::isCommandDispatched);
}
else
{
NL_TEST_ASSERT(apSuite, err != CHIP_NO_ERROR && !chip::isCommandDispatched);
}
NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR && chip::isCommandDispatched == (messageIsTimed == transactionIsTimed));
}
}
}
Expand Down

0 comments on commit f12831d

Please sign in to comment.