diff --git a/examples/tv-app/android/include/account-login/AccountLoginManager.cpp b/examples/tv-app/android/include/account-login/AccountLoginManager.cpp index 71c1bcc88e14e2..52ffc9341e392c 100644 --- a/examples/tv-app/android/include/account-login/AccountLoginManager.cpp +++ b/examples/tv-app/android/include/account-login/AccountLoginManager.cpp @@ -58,7 +58,7 @@ AccountLoginManager::AccountLoginManager(ContentAppCommandDelegate * commandDele CopyString(mSetupPin, sizeof(mSetupPin), setupPin); } -bool AccountLoginManager::HandleLogin(const CharSpan & tempAccountIdentifier, const CharSpan & setupPin, +bool AccountLoginManager::HandleLogin(const CharSpan & tempAccountId, const CharSpan & setupPIN, const chip::Optional & nodeId) { ChipLogProgress(DeviceLayer, "AccountLoginManager::HandleLogin called for endpoint %d", mEndpointId); @@ -76,7 +76,7 @@ bool AccountLoginManager::HandleLogin(const CharSpan & tempAccountIdentifier, co Json::Value response; bool commandHandled = true; - AccountLogin::Commands::Login::Type cmd = { tempAccountIdentifier, setupPin, nodeId }; + AccountLogin::Commands::Login::Type cmd = { tempAccountId, setupPIN, nodeId }; // Deliberately ignore returned status mCommandDelegate->InvokeCommand(mEndpointId, AccountLogin::Id, diff --git a/examples/tv-app/android/java/TVApp-JNI.cpp b/examples/tv-app/android/java/TVApp-JNI.cpp index 04b5f4199edcaa..57e6a51dd271c1 100644 --- a/examples/tv-app/android/java/TVApp-JNI.cpp +++ b/examples/tv-app/android/java/TVApp-JNI.cpp @@ -252,13 +252,13 @@ SampleTvAppInstallationService gSampleTvAppInstallationService; class MyPostCommissioningListener : public PostCommissioningListener { - void CommissioningCompleted(uint16_t vendorId, uint16_t productId, NodeId nodeId, Messaging::ExchangeManager & exchangeMgr, - const SessionHandle & sessionHandle) override + void CommissioningCompleted(uint16_t vendorId, uint16_t productId, NodeId nodeId, std::string rotatingId, uint32_t passcode, + Messaging::ExchangeManager & exchangeMgr, const SessionHandle & sessionHandle) override { // read current binding list chip::Controller::ClusterBase cluster(exchangeMgr, sessionHandle, kTargetBindingClusterEndpointId); - cacheContext(vendorId, productId, nodeId, exchangeMgr, sessionHandle); + cacheContext(vendorId, productId, nodeId, std::move(rotatingId), passcode, exchangeMgr, sessionHandle); CHIP_ERROR err = cluster.ReadAttribute(this, OnReadSuccessResponse, OnReadFailureResponse); @@ -342,17 +342,19 @@ class MyPostCommissioningListener : public PostCommissioningListener Optional opt = mSecureSession.Get(); SessionHandle & sessionHandle = opt.Value(); - ContentAppPlatform::GetInstance().ManageClientAccess(*mExchangeMgr, sessionHandle, mVendorId, mProductId, localNodeId, + ContentAppPlatform::GetInstance().ManageClientAccess(*mExchangeMgr, sessionHandle, mVendorId, mProductId, localNodeId, mRotatingId, mPasscode, bindings, OnSuccessResponse, OnFailureResponse); clearContext(); } - void cacheContext(uint16_t vendorId, uint16_t productId, NodeId nodeId, Messaging::ExchangeManager & exchangeMgr, + void cacheContext(uint16_t vendorId, uint16_t productId, NodeId nodeId, std::string rotatingId, uint32_t passcode, Messaging::ExchangeManager & exchangeMgr, const SessionHandle & sessionHandle) { mVendorId = vendorId; mProductId = productId; mNodeId = nodeId; + mRotatingId = std::move(rotatingId); + mPasscode = passcode; mExchangeMgr = &exchangeMgr; mSecureSession.ShiftToSession(sessionHandle); } @@ -368,6 +370,8 @@ class MyPostCommissioningListener : public PostCommissioningListener uint16_t mVendorId = 0; uint16_t mProductId = 0; NodeId mNodeId = 0; + std::string mRotatingId; + uint32_t mPasscode = 0; Messaging::ExchangeManager * mExchangeMgr = nullptr; SessionHolder mSecureSession; }; diff --git a/examples/tv-app/tv-common/src/AppTv.cpp b/examples/tv-app/tv-common/src/AppTv.cpp index 03422c3fa462e7..6022c5d33f3cdf 100644 --- a/examples/tv-app/tv-common/src/AppTv.cpp +++ b/examples/tv-app/tv-common/src/AppTv.cpp @@ -158,13 +158,13 @@ MyAppInstallationService gMyAppInstallationService; class MyPostCommissioningListener : public PostCommissioningListener { - void CommissioningCompleted(uint16_t vendorId, uint16_t productId, NodeId nodeId, Messaging::ExchangeManager & exchangeMgr, - const SessionHandle & sessionHandle) override + void CommissioningCompleted(uint16_t vendorId, uint16_t productId, NodeId nodeId, std::string rotatingId, uint32_t passcode, + Messaging::ExchangeManager & exchangeMgr, const SessionHandle & sessionHandle) override { // read current binding list chip::Controller::ClusterBase cluster(exchangeMgr, sessionHandle, kTargetBindingClusterEndpointId); - cacheContext(vendorId, productId, nodeId, exchangeMgr, sessionHandle); + cacheContext(vendorId, productId, nodeId, std::move(rotatingId), passcode, exchangeMgr, sessionHandle); CHIP_ERROR err = cluster.ReadAttribute(this, OnReadSuccessResponse, OnReadFailureResponse); @@ -248,17 +248,19 @@ class MyPostCommissioningListener : public PostCommissioningListener Optional opt = mSecureSession.Get(); SessionHandle & sessionHandle = opt.Value(); - ContentAppPlatform::GetInstance().ManageClientAccess(*mExchangeMgr, sessionHandle, mVendorId, mProductId, localNodeId, + ContentAppPlatform::GetInstance().ManageClientAccess(*mExchangeMgr, sessionHandle, mVendorId, mProductId, localNodeId, rotatingId, passcode, bindings, OnSuccessResponse, OnFailureResponse); clearContext(); } - void cacheContext(uint16_t vendorId, uint16_t productId, NodeId nodeId, Messaging::ExchangeManager & exchangeMgr, - const SessionHandle & sessionHandle) + void cacheContext(uint16_t vendorId, uint16_t productId, NodeId nodeId, std::string rotatingId, uint32_t passcode, + Messaging::ExchangeManager & exchangeMgr, const SessionHandle & sessionHandle) { mVendorId = vendorId; mProductId = productId; mNodeId = nodeId; + mRotatingId = std::move(rotatingId); + mPassocde = passcode; mExchangeMgr = &exchangeMgr; mSecureSession.ShiftToSession(sessionHandle); } @@ -274,6 +276,8 @@ class MyPostCommissioningListener : public PostCommissioningListener uint16_t mVendorId = 0; uint16_t mProductId = 0; NodeId mNodeId = 0; + std::string rotatingId; + uint32_t passcode = 0; Messaging::ExchangeManager * mExchangeMgr = nullptr; SessionHolder mSecureSession; }; diff --git a/src/app/app-platform/ContentAppPlatform.cpp b/src/app/app-platform/ContentAppPlatform.cpp index fb2d6ab39a1b5f..edf1e466e03ae9 100644 --- a/src/app/app-platform/ContentAppPlatform.cpp +++ b/src/app/app-platform/ContentAppPlatform.cpp @@ -609,6 +609,7 @@ CHIP_ERROR ContentAppPlatform::GetACLEntryIndex(size_t * foundIndex, FabricIndex // and create bindings on the given client so that it knows what it has access to. CHIP_ERROR ContentAppPlatform::ManageClientAccess(Messaging::ExchangeManager & exchangeMgr, SessionHandle & sessionHandle, uint16_t targetVendorId, uint16_t targetProductId, NodeId localNodeId, + const std::string& rotatingId, uint32_t passcode, std::vector bindings, Controller::WriteResponseSuccessCallback successCb, Controller::WriteResponseFailureCallback failureCb) @@ -755,6 +756,19 @@ CHIP_ERROR ContentAppPlatform::ManageClientAccess(Messaging::ExchangeManager & e { // notify content app about this nodeId app->AddClientNode(subjectNodeId); + + auto tempAccountId = CharSpan::fromCharString(rotatingId.c_str()); + auto setupPIN = CharSpan::fromCharString(std::to_string(passcode).c_str()); + auto nodeId = MakeOptional(subjectNodeId); // Q: Is subjectNodeId correct NodeId to use? + + // send login command to content app + if (!tempAccountId.empty() && !setupPIN.empty()) { + // Q: Should we only do this if (commissioner) passcode is not 0, e.g when user was shown PIN prompt? + auto status = app->GetAccountLoginDelegate()->HandleLogin(tempAccountId, setupPIN, nodeId); + ChipLogProgress(Controller, "AccountLogin::Login command sent and returned with status: %d", status); + } else { + ChipLogError(Controller, "Failed to get tempAccountId or setupPIN to set AccountLogin::Login command"); + } } } } diff --git a/src/app/app-platform/ContentAppPlatform.h b/src/app/app-platform/ContentAppPlatform.h index 16a47c26a3b991..4fd5284f78457c 100644 --- a/src/app/app-platform/ContentAppPlatform.h +++ b/src/app/app-platform/ContentAppPlatform.h @@ -176,6 +176,8 @@ class DLL_EXPORT ContentAppPlatform * @param[in] targetVendorId Vendor ID for the target device. * @param[in] targetProductId Product ID for the target device. * @param[in] localNodeId The NodeId for the local device. + * @param[in] rotatingId The rotating account ID to handle account login. + * @param[in] passcode The passcode to handle account login. * @param[in] bindings Any additional bindings to include. This may include current bindings. * @param[in] successCb The function to be called on success of adding the binding. * @param[in] failureCb The function to be called on failure of adding the binding. @@ -184,6 +186,7 @@ class DLL_EXPORT ContentAppPlatform */ CHIP_ERROR ManageClientAccess(Messaging::ExchangeManager & exchangeMgr, SessionHandle & sessionHandle, uint16_t targetVendorId, uint16_t targetProductId, NodeId localNodeId, + const std::string& rotatingId, uint32_t passcode, std::vector bindings, Controller::WriteResponseSuccessCallback successCb, Controller::WriteResponseFailureCallback failureCb); diff --git a/src/controller/CommissionerDiscoveryController.cpp b/src/controller/CommissionerDiscoveryController.cpp index 1d9421da09af38..a34d527aad42a2 100644 --- a/src/controller/CommissionerDiscoveryController.cpp +++ b/src/controller/CommissionerDiscoveryController.cpp @@ -32,6 +32,33 @@ #if CHIP_DEVICE_CONFIG_ENABLE_COMMISSIONER_DISCOVERY +namespace { + +bool fillRotatingIdBuffer(UDCClientState* client, char[] buffer) { + auto rotatingIdLength = client->GetRotatingIdLength(); + auto hexBufferByteCount = 2 * rotatingIdLength; + if (sizeof(buffer) < hexBufferByteCount) { + return false; + } + auto err = Encoding::BytesToUppercaseHexBuffer(client->GetRotatingId(), rotatingIdLength, buffer, sizeof(buffer)); + return err == CHIP_NO_ERROR; +} + +// Returned CharSpan valid lifetime equals buffer lifetime. +CharSpan getRotatingIdSpan(UDCClientState* client, char[] buffer) { + auto ok = fillRotatingIdBuffer(client, buffer); + return ok ? { buffer, hexBufferByteCount } : {}; +} + +// Allocates memory for rotating ID string and copies to string +std::string getRotatingIdString(UDCClientState* client) { + auto buffer = char[Dnssd::kMaxRotatingIdLen * 2]; + auto ok = fillRotatingIdBuffer(client, buffer); + return ok ? { buffer, hexBufferByteCount } : {}; +} + +} + using namespace ::chip; using namespace chip::Protocols::UserDirectedCommissioning; @@ -246,15 +273,11 @@ void CommissionerDiscoveryController::InternalOk() } char rotatingIdBuffer[Dnssd::kMaxRotatingIdLen * 2]; - size_t rotatingIdLength = client->GetRotatingIdLength(); - CHIP_ERROR err = - Encoding::BytesToUppercaseHexBuffer(client->GetRotatingId(), rotatingIdLength, rotatingIdBuffer, sizeof(rotatingIdBuffer)); - if (err != CHIP_NO_ERROR) - { + CharSpan rotatingIdSpan = getRotatingIdSpan(client, rotatingIdBuffer); + if (rotatingIdSpan.empty()) { ChipLogError(AppServer, "UX InternalOk: could not convert rotating id to hex"); return; } - CharSpan rotatingIdSpan(rotatingIdBuffer, 2 * rotatingIdLength); uint8_t targetAppCount = client->GetNumTargetAppInfos(); @@ -406,7 +429,6 @@ void CommissionerDiscoveryController::InternalHandleContentAppPasscodeResponse() ValidateSession(); uint32_t passcode = mPasscode; - // Q: Seems like getting rotating ID is done twice - once here and once in InternalOk. Is this necessary or could it be cached? if (mUdcServer == nullptr) { ChipLogError(AppServer, "UX Ok - HandleContentAppPasscodeResponse: no udc server"); @@ -432,18 +454,11 @@ void CommissionerDiscoveryController::InternalHandleContentAppPasscodeResponse() if (passcode == 0 && client->GetCommissionerPasscode() && client->GetCdPort() != 0) { char rotatingIdBuffer[Dnssd::kMaxRotatingIdLen * 2]; - size_t rotatingIdLength = client->GetRotatingIdLength(); - CHIP_ERROR err = Encoding::BytesToUppercaseHexBuffer(client->GetRotatingId(), rotatingIdLength, rotatingIdBuffer, - sizeof(rotatingIdBuffer)); - if (err != CHIP_NO_ERROR) - { + CharSpan rotatingIdSpan = getRotatingIdSpan(client, rotatingIdBuffer); + if (rotatingIdSpan.empty()) { ChipLogError(AppServer, "UX Ok - HandleContentAppPasscodeResponse: could not convert rotating id to hex"); return; } - CharSpan rotatingIdSpan(rotatingIdBuffer, 2 * rotatingIdLength); - - // Store rotating ID as tempAccountIdentifier to use in AccountLogin::Login payload later. - mTempAccountIdentifier = rotatingIdSpan; // first step of commissioner passcode ChipLogError(AppServer, "UX Ok: commissioner passcode, sending CDC"); @@ -460,9 +475,6 @@ void CommissionerDiscoveryController::InternalHandleContentAppPasscodeResponse() cd, Transport::PeerAddress::UDP(client->GetPeerAddress().GetIPAddress(), client->GetCdPort())); return; } - // Store commissioner passcode as setup PIN (string/charspan). - // If this PIN is not empty it signals that user prompt was shown to enter PIN and AccountLogin::Login command has to be sent. - mCommissionerSetupPin = CharSpan::fromCharString(std::to_string(passcode)); client->SetCachedCommissionerPasscode(passcode); client->SetUDCClientProcessingState(UDCClientProcessingState::kWaitingForCommissionerPasscodeReady); @@ -616,26 +628,22 @@ void CommissionerDiscoveryController::CommissioningSucceeded(uint16_t vendorId, mProductId = productId; mNodeId = nodeId; - // Send AccountLogin::Login command if user was prompted to enter setup PIN manually. - // Q: Is this the correct place to have this logic? - // Q: Is there an easier way call AccountLoginDelegate from here? - if (!mCommissionerSetupPIN.empty()) { - ChipLogProgress(AppServer, "UX ComissioningSucceeded with setupPIN prompt flow"); - auto app = ContentAppPlatform::GetInstance().LoadContentAppByClient(vendorId, productId); - if (app == nullptr) { - ChipLogError(AppServer, "UX ComissioningSucceeded with setupPIN prompt flow: Failed to get ContentApp"); - // Q: Any action to take? - } else { - auto status = app->GetAccountLoginDelegate()->HandleLogin(mTempAccountIdentifier, mCommissionerSetupPin, {mNodeId}); - ChipLogProgress(AppServer, "UX ComissioningSucceeded with setupPIN prompt flow: HandleLogin response status: %d", status); - // Q: Any action to take here if status is true/false? - } + auto rotatingId = std::string{}; + auto passcode = uint32_t{ 0 }; + + auto client = GetUDCClientState(); + if (client != nullptr) + { + // Get rotating ID and cached (commissioner?) passcode to handle AccountLogin + rotatingId = getRotatingIdString(client); + // Q: Should we use mPasscode, client->GetCommissionerPasscode, or client->GetCachedCommissionerPasscode? + passcode = std::to_string(client->GetCachedCommissionerPasscode()); } if (mPostCommissioningListener != nullptr) { ChipLogDetail(Controller, "CommissionerDiscoveryController calling listener"); - mPostCommissioningListener->CommissioningCompleted(vendorId, productId, nodeId, exchangeMgr, sessionHandle); + mPostCommissioningListener->CommissioningCompleted(vendorId, productId, nodeId, std::move(rotatingId), passcode, exchangeMgr, sessionHandle); } else { diff --git a/src/controller/CommissionerDiscoveryController.h b/src/controller/CommissionerDiscoveryController.h index a23cd153d567cf..48624cb24a56ab 100644 --- a/src/controller/CommissionerDiscoveryController.h +++ b/src/controller/CommissionerDiscoveryController.h @@ -38,7 +38,6 @@ #if CHIP_DEVICE_CONFIG_ENABLE_COMMISSIONER_DISCOVERY -using chip::CharSpan; using chip::NodeId; using chip::OperationalSessionSetup; using chip::Protocols::UserDirectedCommissioning::UDCClientState; @@ -233,11 +232,14 @@ class DLL_EXPORT PostCommissioningListener * @param[in] vendorId The vendorid from the DAC of the new node. * @param[in] productId The productid from the DAC of the new node. * @param[in] nodeId The node id for the newly commissioned node. + * @param[in] rotatingId The rotating ID to handle account login. + * @param[in] passcode The passcode to handle account login. * @param[in] exchangeMgr The exchange manager to be used to get an exchange context. * @param[in] sessionHandle A reference to an established session. * */ virtual void CommissioningCompleted(uint16_t vendorId, uint16_t productId, NodeId nodeId, + std::string rotatingId, uint32_t passcode, chip::Messaging::ExchangeManager & exchangeMgr, const chip::SessionHandle & sessionHandle) = 0; @@ -452,8 +454,6 @@ class CommissionerDiscoveryController : public chip::Protocols::UserDirectedComm uint16_t mProductId = 0; NodeId mNodeId = 0; uint32_t mPasscode = 0; - CharSpan mTempAccountIdentifier; - CharSpan mCommissionerSetupPin; UserDirectedCommissioningServer * mUdcServer = nullptr; UserPrompter * mUserPrompter = nullptr;