[Fabric-Admin] Refactor to use API methods instead of PushCommand (2/3)

examples/fabric-admin/commands/common/CHIPCommand.cpp

@@ -182,7 +182,7 @@ CHIP_ERROR CHIPCommand::MaybeSetUpStack()
     mCredIssuerCmds->SetCredentialIssuerOption(CredentialIssuerCommands::CredentialIssuerOptions::kAllowTestCdSigningKey,
                                                allowTestCdSigningKey);
 
-    PairingManager::Instance().Init(&CurrentCommissioner());
+    ReturnLogErrorOnFailure(PairingManager::Instance().Init(&CurrentCommissioner(), mCredIssuerCmds));
 
     return CHIP_NO_ERROR;
 }

examples/fabric-admin/commands/common/CHIPCommand.h

@@ -120,6 +120,8 @@ class CHIPCommand : public Command
         StopWaiting();
     }
 
+    static chip::app::DefaultICDClientStorage sICDClientStorage;
+
 protected:
     // Will be called in a setting in which it's safe to touch the CHIP
     // stack. The rules for Run() are as follows:
@@ -167,7 +169,6 @@ class CHIPCommand : public Command
     static chip::Crypto::RawKeySessionKeystore sSessionKeystore;
 
     static chip::Credentials::GroupDataProviderImpl sGroupDataProvider;
-    static chip::app::DefaultICDClientStorage sICDClientStorage;
     static chip::app::CheckInHandler sCheckInHandler;
     CredentialIssuerCommands * mCredIssuerCmds;
 

examples/fabric-admin/commands/fabric-sync/FabricSyncCommand.cpp

@@ -94,15 +94,8 @@ CHIP_ERROR FabricSyncAddBridgeCommand::RunCommand(NodeId remoteId)
         return CHIP_NO_ERROR;
     }
 
-    PairingCommand * pairingCommand = static_cast<PairingCommand *>(CommandMgr().GetCommandByName("pairing", "already-discovered"));
+    PairingManager::Instance().SetCommissioningDelegate(this);
 
-    if (pairingCommand == nullptr)
-    {
-        ChipLogError(NotSpecified, "Pairing already-discovered command is not available");
-        return CHIP_ERROR_NOT_IMPLEMENTED;
-    }
-
-    pairingCommand->RegisterCommissioningDelegate(this);
     mBridgeNodeId = remoteId;
 
     DeviceMgr().PairRemoteFabricBridge(remoteId, mSetupPINCode, reinterpret_cast<const char *>(mRemoteAddr.data()), mRemotePort);
@@ -146,16 +139,7 @@ CHIP_ERROR FabricSyncRemoveBridgeCommand::RunCommand()
 
     mBridgeNodeId = bridgeNodeId;
 
-    PairingCommand * pairingCommand = static_cast<PairingCommand *>(CommandMgr().GetCommandByName("pairing", "unpair"));
-
-    if (pairingCommand == nullptr)
-    {
-        ChipLogError(NotSpecified, "Pairing unpair command is not available");
-        return CHIP_ERROR_NOT_IMPLEMENTED;
-    }
-
-    pairingCommand->RegisterPairingDelegate(this);
-
+    PairingManager::Instance().SetPairingDelegate(this);
     DeviceMgr().UnpairRemoteFabricBridge();
 
     return CHIP_NO_ERROR;
@@ -203,10 +187,7 @@ CHIP_ERROR FabricSyncAddLocalBridgeCommand::RunCommand(NodeId deviceId)
         return CHIP_NO_ERROR;
     }
 
-    PairingCommand * pairingCommand = static_cast<PairingCommand *>(CommandMgr().GetCommandByName("pairing", "already-discovered"));
-    VerifyOrDie(pairingCommand != nullptr);
-
-    pairingCommand->RegisterCommissioningDelegate(this);
+    PairingManager::Instance().SetCommissioningDelegate(this);
     mLocalBridgeNodeId = deviceId;
 
     if (mSetupPINCode.HasValue())
@@ -259,16 +240,7 @@ CHIP_ERROR FabricSyncRemoveLocalBridgeCommand::RunCommand()
 
     mLocalBridgeNodeId = bridgeNodeId;
 
-    PairingCommand * pairingCommand = static_cast<PairingCommand *>(CommandMgr().GetCommandByName("pairing", "unpair"));
-
-    if (pairingCommand == nullptr)
-    {
-        ChipLogError(NotSpecified, "Pairing unpair command is not available");
-        return CHIP_ERROR_NOT_IMPLEMENTED;
-    }
-
-    pairingCommand->RegisterPairingDelegate(this);
-
+    PairingManager::Instance().SetPairingDelegate(this);
     DeviceMgr().UnpairLocalFabricBridge();
 
     return CHIP_NO_ERROR;
@@ -287,15 +259,7 @@ void FabricSyncDeviceCommand::OnCommissioningWindowOpened(NodeId deviceId, CHIP_
         {
             NodeId nodeId = DeviceMgr().GetNextAvailableNodeId();
 
-            PairingCommand * pairingCommand = static_cast<PairingCommand *>(CommandMgr().GetCommandByName("pairing", "code"));
-
-            if (pairingCommand == nullptr)
-            {
-                ChipLogError(NotSpecified, "Pairing code command is not available");
-                return;
-            }
-
-            pairingCommand->RegisterCommissioningDelegate(this);
+            PairingManager::Instance().SetCommissioningDelegate(this);
             mAssignedNodeId = nodeId;
 
             usleep(kCommissionPrepareTimeMs * 1000);

examples/fabric-admin/commands/fabric-sync/FabricSyncCommand.h

@@ -19,12 +19,10 @@
 #pragma once
 
 #include <commands/common/CHIPCommand.h>
-#include <commands/pairing/PairingCommand.h>
 #include <device_manager/PairingManager.h>
 
 // Constants
 constexpr uint32_t kCommissionPrepareTimeMs = 500;
-constexpr uint16_t kMaxManualCodeLength     = 21;
 
 class FabricSyncAddBridgeCommand : public CHIPCommand, public CommissioningDelegate
 {

examples/fabric-admin/commands/pairing/PairingCommand.cpp

@@ -30,7 +30,6 @@
 #include <protocols/secure_channel/PASESession.h>
 #include <setup_payload/ManualSetupPayloadParser.h>
 #include <setup_payload/QRCodeSetupPayloadParser.h>
-#include <setup_payload/SetupPayload.h>
 
 #include <string>
 
@@ -41,28 +40,6 @@
 using namespace ::chip;
 using namespace ::chip::Controller;
 
-namespace {
-
-CHIP_ERROR GetPayload(const char * setUpCode, SetupPayload & payload)
-{
-    VerifyOrReturnValue(setUpCode, CHIP_ERROR_INVALID_ARGUMENT);
-    bool isQRCode = strncmp(setUpCode, kQRCodePrefix, strlen(kQRCodePrefix)) == 0;
-    if (isQRCode)
-    {
-        ReturnErrorOnFailure(QRCodeSetupPayloadParser(setUpCode).populatePayload(payload));
-        VerifyOrReturnError(payload.isValidQRCodePayload(), CHIP_ERROR_INVALID_ARGUMENT);
-    }
-    else
-    {
-        ReturnErrorOnFailure(ManualSetupPayloadParser(setUpCode).populatePayload(payload));
-        VerifyOrReturnError(payload.isValidManualCode(), CHIP_ERROR_INVALID_ARGUMENT);
-    }
-
-    return CHIP_NO_ERROR;
-}
-
-} // namespace
-
 CHIP_ERROR PairingCommand::RunCommand()
 {
     CurrentCommissioner().RegisterPairingDelegate(this);
@@ -128,7 +105,10 @@ CommissioningParameters PairingCommand::GetCommissioningParameters()
 {
     auto params = CommissioningParameters();
     params.SetSkipCommissioningComplete(mSkipCommissioningComplete.ValueOr(false));
-    params.SetDeviceAttestationDelegate(this);
+    if (mBypassAttestationVerifier.ValueOr(false))
+    {
+        params.SetDeviceAttestationDelegate(this);
+    }
 
     switch (mNetworkType)
     {
@@ -442,12 +422,6 @@ void PairingCommand::OnCommissioningComplete(NodeId nodeId, CHIP_ERROR err)
         ChipLogProgress(NotSpecified, "Device commissioning Failure: %s", ErrorStr(err));
     }
 
-    if (mCommissioningDelegate)
-    {
-        mCommissioningDelegate->OnCommissioningComplete(nodeId, err);
-        this->UnregisterCommissioningDelegate();
-    }
-
     SetCommandExitStatus(err);
 }
 
@@ -576,13 +550,6 @@ void PairingCommand::OnCurrentFabricRemove(void * context, NodeId nodeId, CHIP_E
         ChipLogProgress(NotSpecified, "Device unpair Failure: " ChipLogFormatX64 " %s", ChipLogValueX64(nodeId), ErrorStr(err));
     }
 
-    PairingDelegate * pairingDelegate = command->GetPairingDelegate();
-    if (pairingDelegate)
-    {
-        pairingDelegate->OnDeviceRemoved(nodeId, err);
-        command->UnregisterPairingDelegate();
-    }
-
     command->SetCommandExitStatus(err);
 }
 
@@ -592,77 +559,13 @@ Optional<uint16_t> PairingCommand::FailSafeExpiryTimeoutSecs() const
     return Optional<uint16_t>();
 }
 
-bool PairingCommand::ShouldWaitAfterDeviceAttestation()
-{
-    // If there is a vendor ID and product ID, request OnDeviceAttestationCompleted().
-    // Currently this is added in the case that the example is performing reverse commissioning,
-    // but it would be an improvement to store that explicitly.
-    // TODO: Issue #35297 - [Fabric Sync] Improve where we get VID and PID when validating CCTRL CommissionNode command
-    SetupPayload payload;
-    CHIP_ERROR err = GetPayload(mOnboardingPayload, payload);
-    return err == CHIP_NO_ERROR && (payload.vendorID != 0 || payload.productID != 0);
-}
-
 void PairingCommand::OnDeviceAttestationCompleted(Controller::DeviceCommissioner * deviceCommissioner, DeviceProxy * device,
                                                   const Credentials::DeviceAttestationVerifier::AttestationDeviceInfo & info,
                                                   Credentials::AttestationVerificationResult attestationResult)
 {
-    SetupPayload payload;
-    CHIP_ERROR parse_error = GetPayload(mOnboardingPayload, payload);
-    if (parse_error == CHIP_NO_ERROR && (payload.vendorID != 0 || payload.productID != 0))
-    {
-        if (payload.vendorID == 0 || payload.productID == 0)
-        {
-            ChipLogProgress(NotSpecified,
-                            "Failed validation: vendorID or productID must not be 0."
-                            "Requested VID: %u, Requested PID: %u.",
-                            payload.vendorID, payload.productID);
-            deviceCommissioner->ContinueCommissioningAfterDeviceAttestation(
-                device, Credentials::AttestationVerificationResult::kInvalidArgument);
-            return;
-        }
-
-        if (payload.vendorID != info.BasicInformationVendorId() || payload.productID != info.BasicInformationProductId())
-        {
-            ChipLogProgress(NotSpecified,
-                            "Failed validation of vendorID or productID."
-                            "Requested VID: %u, Requested PID: %u,"
-                            "Detected VID: %u, Detected PID %u.",
-                            payload.vendorID, payload.productID, info.BasicInformationVendorId(), info.BasicInformationProductId());
-            deviceCommissioner->ContinueCommissioningAfterDeviceAttestation(
-                device,
-                payload.vendorID == info.BasicInformationVendorId()
-                    ? Credentials::AttestationVerificationResult::kDacProductIdMismatch
-                    : Credentials::AttestationVerificationResult::kDacVendorIdMismatch);
-            return;
-        }
-
-        // NOTE: This will log errors even if the attestion was successful.
-        auto err = deviceCommissioner->ContinueCommissioningAfterDeviceAttestation(device, attestationResult);
-        if (CHIP_NO_ERROR != err)
-        {
-            SetCommandExitStatus(err);
-        }
-        return;
-    }
-
-    // OnDeviceAttestationCompleted() is called if ShouldWaitAfterDeviceAttestation() returns true
-    // or if there is an attestation error. The conditions for ShouldWaitAfterDeviceAttestation() have
-    // already been checked, so the call to OnDeviceAttestationCompleted() was an error.
-    if (mBypassAttestationVerifier.ValueOr(false))
-    {
-        // Bypass attestation verification, continue with success
-        auto err = deviceCommissioner->ContinueCommissioningAfterDeviceAttestation(
-            device, Credentials::AttestationVerificationResult::kSuccess);
-        if (CHIP_NO_ERROR != err)
-        {
-            SetCommandExitStatus(err);
-        }
-        return;
-    }
-
-    // Don't bypass attestation, continue with error.
-    auto err = deviceCommissioner->ContinueCommissioningAfterDeviceAttestation(device, attestationResult);
+    // Bypass attestation verification, continue with success
+    auto err = deviceCommissioner->ContinueCommissioningAfterDeviceAttestation(
+        device, Credentials::AttestationVerificationResult::kSuccess);
     if (CHIP_NO_ERROR != err)
     {
         SetCommandExitStatus(err);

examples/fabric-admin/commands/pairing/PairingCommand.h

@@ -45,20 +45,6 @@ enum class PairingNetworkType
     Thread,
 };
 
-class CommissioningDelegate
-{
-public:
-    virtual void OnCommissioningComplete(chip::NodeId deviceId, CHIP_ERROR err) = 0;
-    virtual ~CommissioningDelegate()                                            = default;
-};
-
-class PairingDelegate
-{
-public:
-    virtual void OnDeviceRemoved(chip::NodeId deviceId, CHIP_ERROR err) = 0;
-    virtual ~PairingDelegate()                                          = default;
-};
-
 class PairingCommand : public CHIPCommand,
                        public chip::Controller::DevicePairingDelegate,
                        public chip::Controller::DeviceDiscoveryDelegate,
@@ -221,20 +207,10 @@ class PairingCommand : public CHIPCommand,
 
     /////////// DeviceAttestationDelegate Interface /////////
     chip::Optional<uint16_t> FailSafeExpiryTimeoutSecs() const override;
-    bool ShouldWaitAfterDeviceAttestation() override;
     void OnDeviceAttestationCompleted(chip::Controller::DeviceCommissioner * deviceCommissioner, chip::DeviceProxy * device,
                                       const chip::Credentials::DeviceAttestationVerifier::AttestationDeviceInfo & info,
                                       chip::Credentials::AttestationVerificationResult attestationResult) override;
 
-    /////////// CommissioningDelegate /////////
-    void RegisterCommissioningDelegate(CommissioningDelegate * delegate) { mCommissioningDelegate = delegate; }
-    void UnregisterCommissioningDelegate() { mCommissioningDelegate = nullptr; }
-
-    /////////// PairingDelegate /////////
-    void RegisterPairingDelegate(PairingDelegate * delegate) { mPairingDelegate = delegate; }
-    void UnregisterPairingDelegate() { mPairingDelegate = nullptr; }
-    PairingDelegate * GetPairingDelegate() { return mPairingDelegate; }
-
 private:
     CHIP_ERROR RunInternal(NodeId remoteId);
     CHIP_ERROR Pair(NodeId remoteId, PeerAddress address);
@@ -290,9 +266,6 @@ class PairingCommand : public CHIPCommand,
     chip::Platform::UniquePtr<chip::Controller::CurrentFabricRemover> mCurrentFabricRemover;
     chip::Callback::Callback<chip::Controller::OnCurrentFabricRemove> mCurrentFabricRemoveCallback;
 
-    CommissioningDelegate * mCommissioningDelegate = nullptr;
-    PairingDelegate * mPairingDelegate             = nullptr;
-
     static void OnCurrentFabricRemove(void * context, NodeId remoteNodeId, CHIP_ERROR status);
     void PersistIcdInfo();
 };