From f12831df26b2abf0e8f6f5cf7d3bb857009631bf Mon Sep 17 00:00:00 2001
From: Marc Lepage <67919234+mlepage-google@users.noreply.github.com>
Date: Fri, 14 Jan 2022 13:59:58 -0500
Subject: [PATCH] Hookup SubjectDescriptor in CommandHandler (#12953)

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.
---
 src/app/CommandHandler.cpp               |  9 +++++----
 src/app/tests/TestCommandInteraction.cpp | 23 ++++++++++++-----------
 2 files changed, 17 insertions(+), 15 deletions(-)

diff --git a/src/app/CommandHandler.cpp b/src/app/CommandHandler.cpp
index cabddbb1f8b57c..4a2b77aeb983cd 100644
--- a/src/app/CommandHandler.cpp
+++ b/src/app/CommandHandler.cpp
@@ -98,6 +98,8 @@ 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
@@ -105,9 +107,7 @@ CHIP_ERROR CommandHandler::ProcessInvokeRequest(System::PacketBufferHandle && pa
         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.
@@ -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);
diff --git a/src/app/tests/TestCommandInteraction.cpp b/src/app/tests/TestCommandInteraction.cpp
index 2af13c597ce36e..67fcd195fe2c8b 100644
--- a/src/app/tests/TestCommandInteraction.cpp
+++ b/src/app/tests/TestCommandInteraction.cpp
@@ -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);
 }
 
@@ -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)
     {
@@ -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));
         }
     }
 }