From 71e6516cbca93deca1e673616a6eb09e6a6a4a2c Mon Sep 17 00:00:00 2001 From: James Swan <122404367+swan-amazon@users.noreply.github.com> Date: Tue, 27 Aug 2024 23:35:55 +0000 Subject: [PATCH] wip --- src/app/BUILD.gn | 13 +- src/app/FailSafeContext.h | 3 +- .../general-commissioning-server.cpp | 57 ++-- src/app/common_flags.gni | 19 ++ src/app/server/BUILD.gn | 7 +- .../DefaultEnhancedSetupFlowProvider.cpp | 72 ---- .../server/DefaultEnhancedSetupFlowProvider.h | 57 ---- .../DefaultTermsAndConditionsProvider.cpp | 26 +- .../DefaultTermsAndConditionsProvider.h | 3 +- src/app/server/EnhancedSetupFlowProvider.h | 50 --- src/app/server/Server.cpp | 4 +- src/app/server/Server.h | 24 +- src/app/server/TermsAndConditionsProvider.h | 1 - src/app/tests/BUILD.gn | 2 - .../TestDefaultEnhancedSetupFlowProvider.cpp | 312 ------------------ .../TestDefaultTermsAndConditionsProvider.cpp | 193 ++++++----- src/controller/CommissioningDelegate.h | 2 +- src/lib/core/CHIPConfig.h | 40 +-- src/python_testing/matter_testing_support.py | 148 ++++----- 19 files changed, 263 insertions(+), 770 deletions(-) delete mode 100644 src/app/server/DefaultEnhancedSetupFlowProvider.cpp delete mode 100644 src/app/server/DefaultEnhancedSetupFlowProvider.h delete mode 100644 src/app/server/EnhancedSetupFlowProvider.h delete mode 100644 src/app/tests/TestDefaultEnhancedSetupFlowProvider.cpp diff --git a/src/app/BUILD.gn b/src/app/BUILD.gn index 79a20235132133..1ec4fbf43e26f2 100644 --- a/src/app/BUILD.gn +++ b/src/app/BUILD.gn @@ -63,11 +63,7 @@ declare_args() { config("enhanced_setup_flow_config") { defines = [] if (chip_config_tc_required) { - defines += [ - "CHIP_CONFIG_TC_REQUIRED=1", - "CHIP_CONFIG_TC_REQUIRED_ACKNOWLEDGEMENTS=${chip_config_tc_required_acknowledgements}", - "CHIP_CONFIG_TC_REQUIRED_ACKNOWLEDGEMENTS_VERSION=${chip_config_tc_required_acknowledgements_version}", - ] + defines += [ "CHIP_CONFIG_TC_REQUIRED=1" ] } else { defines += [ "CHIP_CONFIG_TC_REQUIRED=0" ] } @@ -93,6 +89,13 @@ buildconfig_header("app_buildconfig") { "CHIP_CONFIG_ENABLE_BUSY_HANDLING_FOR_OPERATIONAL_SESSION_SETUP=${chip_enable_busy_handling_for_operational_session_setup}", ] + if (chip_config_tc_required) { + defines += [ + "CHIP_CONFIG_TC_REQUIRED_ACKNOWLEDGEMENTS=${chip_config_tc_required_acknowledgements}", + "CHIP_CONFIG_TC_REQUIRED_ACKNOWLEDGEMENTS_VERSION=${chip_config_tc_required_acknowledgements_version}", + ] + } + if (chip_use_data_model_interface == "disabled") { defines += [ "CHIP_CONFIG_USE_DATA_MODEL_INTERFACE=0", diff --git a/src/app/FailSafeContext.h b/src/app/FailSafeContext.h index 421774a521450a..db5f163c29c904 100644 --- a/src/app/FailSafeContext.h +++ b/src/app/FailSafeContext.h @@ -56,6 +56,7 @@ class FailSafeContext void SetUpdateNocCommandInvoked() { mUpdateNocCommandHasBeenInvoked = true; } void SetAddTrustedRootCertInvoked() { mAddTrustedRootCertHasBeenInvoked = true; } void SetCsrRequestForUpdateNoc(bool isForUpdateNoc) { mIsCsrRequestForUpdateNoc = isForUpdateNoc; } + #if CHIP_CONFIG_TC_REQUIRED void SetUpdateTermsAndConditionsHasBeenInvoked() { mUpdateTermsAndConditionsHasBeenInvoked = true; } #endif @@ -115,10 +116,10 @@ class FailSafeContext bool mAddTrustedRootCertHasBeenInvoked = false; // The fact of whether a CSR occurred at all is stored elsewhere. bool mIsCsrRequestForUpdateNoc = false; + FabricIndex mFabricIndex = kUndefinedFabricIndex; #if CHIP_CONFIG_TC_REQUIRED bool mUpdateTermsAndConditionsHasBeenInvoked = false; #endif - FabricIndex mFabricIndex = kUndefinedFabricIndex; /** * @brief diff --git a/src/app/clusters/general-commissioning-server/general-commissioning-server.cpp b/src/app/clusters/general-commissioning-server/general-commissioning-server.cpp index a8d8b979643a9b..1684638bc69d9f 100644 --- a/src/app/clusters/general-commissioning-server/general-commissioning-server.cpp +++ b/src/app/clusters/general-commissioning-server/general-commissioning-server.cpp @@ -28,7 +28,7 @@ #include #include #if CHIP_CONFIG_TC_REQUIRED -#include +#include #endif #include #include @@ -101,22 +101,22 @@ CHIP_ERROR GeneralCommissioningAttrAccess::Read(const ConcreteReadAttributePath #if CHIP_CONFIG_TC_REQUIRED case TCAcceptedVersion::Id: { Optional outTermsAndConditions; - Server::GetInstance().GetEnhancedSetupFlowProvider()->GetTermsAndConditionsAcknowledgements(outTermsAndConditions); + Server::GetInstance().GetTermsAndConditionsProvider()->GetAcceptance(outTermsAndConditions); return !outTermsAndConditions.HasValue() ? aEncoder.Encode(0) : aEncoder.Encode(outTermsAndConditions.Value().version); } case TCMinRequiredVersion::Id: { Optional outTermsAndConditions; - Server::GetInstance().GetEnhancedSetupFlowProvider()->GetTermsAndConditionsRequirements(outTermsAndConditions); + Server::GetInstance().GetTermsAndConditionsProvider()->GetRequirements(outTermsAndConditions); return !outTermsAndConditions.HasValue() ? aEncoder.Encode(0) : aEncoder.Encode(outTermsAndConditions.Value().version); } case TCAcknowledgements::Id: { Optional outTermsAndConditions; - Server::GetInstance().GetEnhancedSetupFlowProvider()->GetTermsAndConditionsAcknowledgements(outTermsAndConditions); + Server::GetInstance().GetTermsAndConditionsProvider()->GetAcceptance(outTermsAndConditions); return !outTermsAndConditions.HasValue() ? aEncoder.Encode(0) : aEncoder.Encode(outTermsAndConditions.Value().value); } case TCAcknowledgementsRequired::Id: { Optional outTermsAndConditions; - Server::GetInstance().GetEnhancedSetupFlowProvider()->GetTermsAndConditionsRequirements(outTermsAndConditions); + Server::GetInstance().GetTermsAndConditionsProvider()->GetRequirements(outTermsAndConditions); return aEncoder.Encode(outTermsAndConditions.HasValue()); } #endif @@ -304,15 +304,13 @@ bool emberAfGeneralCommissioningClusterCommissioningCompleteCallback( else { #if CHIP_CONFIG_TC_REQUIRED - // CheckSuccess(CheckTermsAndConditionsAcknowledgementsStateOnCommissioningComplete(response.errorCode), Failure); - - EnhancedSetupFlowProvider * const enhancedSetupFlowProvider = Server::GetInstance().GetEnhancedSetupFlowProvider(); + TermsAndConditionsProvider * const termsAndConditionsProvider = Server::GetInstance().GetTermsAndConditionsProvider(); Optional acceptedTermsAndConditions; Optional requiredTermsAndConditions; - enhancedSetupFlowProvider->GetTermsAndConditionsAcknowledgements(acceptedTermsAndConditions); - enhancedSetupFlowProvider->GetTermsAndConditionsRequirements(requiredTermsAndConditions); + termsAndConditionsProvider->GetAcceptance(acceptedTermsAndConditions); + termsAndConditionsProvider->GetRequirements(requiredTermsAndConditions); response.errorCode = CheckTermsAndConditionsAcknowledgementsState(requiredTermsAndConditions, acceptedTermsAndConditions); @@ -323,12 +321,31 @@ bool emberAfGeneralCommissioningClusterCommissioningCompleteCallback( } #endif +#if CHIP_CONFIG_TC_REQUIRED + if (failSafe.UpdateTermsAndConditionsHasBeenInvoked()) + { + // Commit terms and conditions acceptance on commissioning complete + CHIP_ERROR err = Server::GetInstance().GetTermsAndConditionsProvider()->CommitAcceptance(); + if (err != CHIP_NO_ERROR) + { + ChipLogError(FailSafe, "GeneralCommissioning: Failed to commit terms and conditions: %" CHIP_ERROR_FORMAT, + err.Format()); + fabricTable.RevertPendingFabricData(); + } + else + { + ChipLogProgress(FailSafe, "GeneralCommissioning: Successfully commited terms and conditions"); + } + CheckSuccess(err, Failure); + } +#endif + if (failSafe.NocCommandHasBeenInvoked()) { + // No need to revert on error: CommitPendingFabricData always reverts if not fully successful. CHIP_ERROR err = fabricTable.CommitPendingFabricData(); if (err != CHIP_NO_ERROR) { - // No need to revert on error: CommitPendingFabricData always reverts if not fully successful. ChipLogError(FailSafe, "GeneralCommissioning: Failed to commit pending fabric data: %" CHIP_ERROR_FORMAT, err.Format()); } @@ -339,14 +356,6 @@ bool emberAfGeneralCommissioningClusterCommissioningCompleteCallback( CheckSuccess(err, Failure); } -#if CHIP_CONFIG_TC_REQUIRED - if (failSafe.UpdateTermsAndConditionsHasBeenInvoked()) - { - // Commit terms and conditions acceptance on commissioning complete - Server::GetInstance().GetEnhancedSetupFlowProvider()->CommitTermsAndConditionsAcceptance(); - } -#endif - /* * Pass fabric of commissioner to DeviceControlSvr. * This allows device to send messages back to commissioner. @@ -433,11 +442,11 @@ bool emberAfGeneralCommissioningClusterSetTCAcknowledgementsCallback( auto & failSafeContext = Server::GetInstance().GetFailSafeContext(); Commands::SetTCAcknowledgementsResponse::Type response; - EnhancedSetupFlowProvider * const enhancedSetupFlowProvider = Server::GetInstance().GetEnhancedSetupFlowProvider(); + TermsAndConditionsProvider * const termsAndConditionsProvider = Server::GetInstance().GetTermsAndConditionsProvider(); Optional requiredTermsAndConditions; - enhancedSetupFlowProvider->GetTermsAndConditionsRequirements(requiredTermsAndConditions); + termsAndConditionsProvider->GetRequirements(requiredTermsAndConditions); Optional acceptedTermsAndConditions = Optional({ .value = commandData.TCUserResponse, @@ -448,7 +457,7 @@ bool emberAfGeneralCommissioningClusterSetTCAcknowledgementsCallback( if (CommissioningErrorEnum::kOk == response.errorCode) { - CheckSuccess(enhancedSetupFlowProvider->SetTermsAndConditionsAcceptance(acceptedTermsAndConditions), Failure); + CheckSuccess(termsAndConditionsProvider->SetAcceptance(acceptedTermsAndConditions), Failure); if (failSafeContext.IsFailSafeArmed()) { @@ -456,7 +465,7 @@ bool emberAfGeneralCommissioningClusterSetTCAcknowledgementsCallback( } else { - enhancedSetupFlowProvider->CommitTermsAndConditionsAcceptance(); + termsAndConditionsProvider->CommitAcceptance(); } } @@ -478,7 +487,7 @@ void OnPlatformEventHandler(const DeviceLayer::ChipDeviceEvent * event, intptr_t if (event->FailSafeTimerExpired.updateTermsAndConditionsHasBeenInvoked) { // Clear terms and conditions acceptance on failsafe timer expiration - Server::GetInstance().GetEnhancedSetupFlowProvider()->RevertTermsAndConditionsAcceptance(); + Server::GetInstance().GetTermsAndConditionsProvider()->RevertAcceptance(); } #endif } diff --git a/src/app/common_flags.gni b/src/app/common_flags.gni index 34e8549a35d1cd..bffdc22a21e442 100644 --- a/src/app/common_flags.gni +++ b/src/app/common_flags.gni @@ -18,8 +18,27 @@ declare_args() { chip_enable_read_client = true chip_build_controller_dynamic_server = false + # Configures whether terms and conditions acknowledgements are required. + # + # When set to true, the application will enforce the requirement of + # terms and conditions acknowledgements during commissioning. chip_config_tc_required = false + + # Configures the required terms and conditions acknowledgements bitmask. + # + # This setting defines the required terms and conditions acknowledgements bitmask. + # The bit-field is 16 bits long, so the possible value range is [0, 65535). + # This setting can be used to require that terms and conditions are presented + # to the user during commissioning. chip_config_tc_required_acknowledgements = 0 + + # Configures the latest known version of the terms and conditions. + # + # This setting defines the version number of the latest terms and conditions. + # It allows the application to iterate on revisions of the terms and conditions. + # A value of 0 indicates that no specific version is required. This setting can + # be used to enforce version-specific terms and conditions acknowledgements in + # the application. chip_config_tc_required_acknowledgements_version = 0 # Flag that controls whether the time-to-wait from BUSY responses is diff --git a/src/app/server/BUILD.gn b/src/app/server/BUILD.gn index 81193f35512df8..326db2f44fc6aa 100644 --- a/src/app/server/BUILD.gn +++ b/src/app/server/BUILD.gn @@ -27,9 +27,7 @@ config("server_config") { source_set("enhanced_setup_flow_interface") { sources = [ - "DefaultEnhancedSetupFlowProvider.h", "DefaultTermsAndConditionsProvider.h", - "EnhancedSetupFlowProvider.h", "TermsAndConditionsProvider.h", ] @@ -39,10 +37,7 @@ source_set("enhanced_setup_flow_interface") { } source_set("enhanced_setup_flow") { - sources = [ - "DefaultEnhancedSetupFlowProvider.cpp", - "DefaultTermsAndConditionsProvider.cpp", - ] + sources = [ "DefaultTermsAndConditionsProvider.cpp" ] deps = [ ":enhanced_setup_flow_interface" ] } diff --git a/src/app/server/DefaultEnhancedSetupFlowProvider.cpp b/src/app/server/DefaultEnhancedSetupFlowProvider.cpp deleted file mode 100644 index f90ca2cb609140..00000000000000 --- a/src/app/server/DefaultEnhancedSetupFlowProvider.cpp +++ /dev/null @@ -1,72 +0,0 @@ -/* - * - * Copyright (c) 2024 Project CHIP Authors - * All rights reserved. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -#include "DefaultEnhancedSetupFlowProvider.h" - -#include -#include - -CHIP_ERROR chip::app::DefaultEnhancedSetupFlowProvider::Init(TermsAndConditionsProvider * const inTermsAndConditionsProvider) -{ - VerifyOrReturnError(nullptr != inTermsAndConditionsProvider, CHIP_ERROR_INVALID_ARGUMENT); - - mTermsAndConditionsProvider = inTermsAndConditionsProvider; - - return CHIP_NO_ERROR; -} - -CHIP_ERROR chip::app::DefaultEnhancedSetupFlowProvider::GetTermsAndConditionsAcknowledgements( - Optional & outTermsAndConditions) const -{ - VerifyOrReturnError(nullptr != mTermsAndConditionsProvider, CHIP_ERROR_UNINITIALIZED); - ReturnErrorOnFailure(mTermsAndConditionsProvider->GetAcceptance(outTermsAndConditions)); - return CHIP_NO_ERROR; -} - -CHIP_ERROR -chip::app::DefaultEnhancedSetupFlowProvider::GetTermsAndConditionsRequirements( - Optional & outTermsAndConditions) const -{ - VerifyOrReturnError(nullptr != mTermsAndConditionsProvider, CHIP_ERROR_UNINITIALIZED); - ReturnErrorOnFailure(mTermsAndConditionsProvider->GetRequirements(outTermsAndConditions)); - return CHIP_NO_ERROR; -} - -CHIP_ERROR chip::app::DefaultEnhancedSetupFlowProvider::SetTermsAndConditionsAcceptance( - const Optional & inTermsAndConditions) -{ - VerifyOrReturnError(nullptr != mTermsAndConditionsProvider, CHIP_ERROR_UNINITIALIZED); - ReturnErrorOnFailure(mTermsAndConditionsProvider->SetAcceptance(inTermsAndConditions)); - return CHIP_NO_ERROR; -} - -CHIP_ERROR chip::app::DefaultEnhancedSetupFlowProvider::RevertTermsAndConditionsAcceptance() -{ - VerifyOrReturnError(nullptr != mTermsAndConditionsProvider, CHIP_ERROR_UNINITIALIZED); - ReturnErrorOnFailure(mTermsAndConditionsProvider->RevertAcceptance()); - - return CHIP_NO_ERROR; -} - -CHIP_ERROR chip::app::DefaultEnhancedSetupFlowProvider::CommitTermsAndConditionsAcceptance() -{ - VerifyOrReturnError(nullptr != mTermsAndConditionsProvider, CHIP_ERROR_UNINITIALIZED); - ReturnErrorOnFailure(mTermsAndConditionsProvider->CommitAcceptance()); - - return CHIP_NO_ERROR; -} diff --git a/src/app/server/DefaultEnhancedSetupFlowProvider.h b/src/app/server/DefaultEnhancedSetupFlowProvider.h deleted file mode 100644 index 0dba132d306ec4..00000000000000 --- a/src/app/server/DefaultEnhancedSetupFlowProvider.h +++ /dev/null @@ -1,57 +0,0 @@ -/* - * - * Copyright (c) 2024 Project CHIP Authors - * All rights reserved. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -#pragma once - -#include "EnhancedSetupFlowProvider.h" - -#include - -#include -#include - -#include "TermsAndConditionsProvider.h" - -namespace chip { -namespace app { -class DefaultEnhancedSetupFlowProvider : public EnhancedSetupFlowProvider -{ -public: - /** - * @brief Initializes the EnhancedSetupFlowProvider. - * - * @param[in] inTermsAndConditionsProvider The terms and conditions provide dependency. - */ - CHIP_ERROR Init(TermsAndConditionsProvider * const inTermsAndConditionsProvider); - - virtual CHIP_ERROR GetTermsAndConditionsAcknowledgements(Optional & outTermsAndConditions) const override; - - virtual CHIP_ERROR GetTermsAndConditionsRequirements(Optional & outTermsAndConditions) const override; - - virtual CHIP_ERROR SetTermsAndConditionsAcceptance(const Optional & inTermsAndConditions) override; - - virtual CHIP_ERROR RevertTermsAndConditionsAcceptance() override; - - virtual CHIP_ERROR CommitTermsAndConditionsAcceptance() override; - -private: - TermsAndConditionsProvider * mTermsAndConditionsProvider; -}; - -} // namespace app -} // namespace chip diff --git a/src/app/server/DefaultTermsAndConditionsProvider.cpp b/src/app/server/DefaultTermsAndConditionsProvider.cpp index e3acb855948ce5..0c1265c036908a 100644 --- a/src/app/server/DefaultTermsAndConditionsProvider.cpp +++ b/src/app/server/DefaultTermsAndConditionsProvider.cpp @@ -39,16 +39,14 @@ constexpr size_t kEstimatedTlvBufferSize = chip::TLV::EstimateStructOverhead(siz 2; // Extra space for rollback compatibility } // namespace -CHIP_ERROR chip::app::DefaultTermsAndConditionsProvider::Init(chip::PersistentStorageDelegate * const inPersistentStorageDelegate) +CHIP_ERROR chip::app::DefaultTermsAndConditionsProvider::Init( + chip::PersistentStorageDelegate * const inPersistentStorageDelegate, + const chip::Optional & inRequiredTermsAndConditions) { VerifyOrReturnError(nullptr != inPersistentStorageDelegate, CHIP_ERROR_INVALID_ARGUMENT); mPersistentStorageDelegate = inPersistentStorageDelegate; - - mRequiredAcknowledgements = Optional({ - .value = CHIP_CONFIG_TC_REQUIRED_ACKNOWLEDGEMENTS, - .version = CHIP_CONFIG_TC_REQUIRED_ACKNOWLEDGEMENTS_VERSION, - }); + mRequiredAcknowledgements = inRequiredTermsAndConditions; if (CHIP_NO_ERROR == LoadAcceptance(mLatchedAcceptance)) { @@ -60,7 +58,7 @@ CHIP_ERROR chip::app::DefaultTermsAndConditionsProvider::Init(chip::PersistentSt CHIP_ERROR chip::app::DefaultTermsAndConditionsProvider::RevertAcceptance() { - mTemporalAcceptance.SetValue(mLatchedAcceptance.Value()); + mTemporalAcceptance = mLatchedAcceptance; return CHIP_NO_ERROR; } @@ -77,23 +75,13 @@ CHIP_ERROR chip::app::DefaultTermsAndConditionsProvider::CommitAcceptance() CHIP_ERROR chip::app::DefaultTermsAndConditionsProvider::GetAcceptance(Optional & outTermsAndConditions) const { - if (!mTemporalAcceptance.HasValue()) - { - return CHIP_ERROR_INCORRECT_STATE; - } - - outTermsAndConditions.SetValue(mTemporalAcceptance.Value()); + outTermsAndConditions = mTemporalAcceptance; return CHIP_NO_ERROR; } CHIP_ERROR chip::app::DefaultTermsAndConditionsProvider::GetRequirements(Optional & outTermsAndConditions) const { - if (!mRequiredAcknowledgements.HasValue()) - { - return CHIP_ERROR_INCORRECT_STATE; - } - - outTermsAndConditions.SetValue(mRequiredAcknowledgements.Value()); + outTermsAndConditions = mRequiredAcknowledgements; return CHIP_NO_ERROR; } diff --git a/src/app/server/DefaultTermsAndConditionsProvider.h b/src/app/server/DefaultTermsAndConditionsProvider.h index af86f8a4c95fed..fc3e49e61f7613 100644 --- a/src/app/server/DefaultTermsAndConditionsProvider.h +++ b/src/app/server/DefaultTermsAndConditionsProvider.h @@ -36,7 +36,8 @@ class DefaultTermsAndConditionsProvider : public TermsAndConditionsProvider * * @param[in] inPersistentStorageDelegate Persistent storage delegate dependency. */ - CHIP_ERROR Init(PersistentStorageDelegate * const inPersistentStorageDelegate); + CHIP_ERROR Init(PersistentStorageDelegate * const inPersistentStorageDelegate, + const chip::Optional & inRequiredTermsAndConditions); CHIP_ERROR SetAcceptance(const Optional & inTermsAndConditions) override; diff --git a/src/app/server/EnhancedSetupFlowProvider.h b/src/app/server/EnhancedSetupFlowProvider.h deleted file mode 100644 index e4b5ce7ce89864..00000000000000 --- a/src/app/server/EnhancedSetupFlowProvider.h +++ /dev/null @@ -1,50 +0,0 @@ -/* - * - * Copyright (c) 2024 Project CHIP Authors - * All rights reserved. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -#pragma once - -#include - -#include - -#include "TermsAndConditionsProvider.h" - -namespace chip { -namespace app { - -/** - * This class provides access to the state of the Enhanced Setup Flow feature. - */ -class EnhancedSetupFlowProvider -{ -public: - virtual ~EnhancedSetupFlowProvider() = default; - - virtual CHIP_ERROR GetTermsAndConditionsAcknowledgements(Optional & outTermsAndConditions) const = 0; - - virtual CHIP_ERROR GetTermsAndConditionsRequirements(Optional & outTermsAndConditions) const = 0; - - virtual CHIP_ERROR SetTermsAndConditionsAcceptance(const Optional & inTermsAndConditions) = 0; - - virtual CHIP_ERROR RevertTermsAndConditionsAcceptance() = 0; - - virtual CHIP_ERROR CommitTermsAndConditionsAcceptance() = 0; -}; - -} // namespace app -} // namespace chip diff --git a/src/app/server/Server.cpp b/src/app/server/Server.cpp index 2e6e961f1f87b3..f34754d97a86ef 100644 --- a/src/app/server/Server.cpp +++ b/src/app/server/Server.cpp @@ -194,7 +194,6 @@ CHIP_ERROR Server::Init(const ServerInitParams & initParams) #if CHIP_CONFIG_TC_REQUIRED mTermsAndConditionsProvider = initParams.termsAndConditionsProvider; - mEnhancedSetupFlowProvider = initParams.enhancedSetupFlowProvider; #endif mTestEventTriggerDelegate = initParams.testEventTriggerDelegate; @@ -599,8 +598,7 @@ void Server::ScheduleFactoryReset() PlatformMgr().HandleServerShuttingDown(); ConfigurationMgr().InitiateFactoryReset(); #if CHIP_CONFIG_TC_REQUIRED - // Clear accepted terms and conditions - GetInstance().GetEnhancedSetupFlowProvider()->RevertTermsAndConditionsAcceptance(); + GetInstance().GetTermsAndConditionsProvider()->ResetAcceptance(); #endif }); } diff --git a/src/app/server/Server.h b/src/app/server/Server.h index f9dd82071dcfd3..ea9d33f8b0b355 100644 --- a/src/app/server/Server.h +++ b/src/app/server/Server.h @@ -34,10 +34,7 @@ #include #include #if CHIP_CONFIG_TC_REQUIRED -#include #include -#include -#include #endif #include #include @@ -193,8 +190,6 @@ struct ServerInitParams // If the ICD Check-In protocol use-case is supported and no strategy is provided, server will use the default strategy. app::ICDCheckInBackOffStrategy * icdCheckInBackOffStrategy = nullptr; #if CHIP_CONFIG_TC_REQUIRED - // Optional. Enhanced setup flow provider to support terms and conditions acceptance check. - app::EnhancedSetupFlowProvider * enhancedSetupFlowProvider = nullptr; // Optional. Terms and conditions provider to support enhanced setup flow feature. app::TermsAndConditionsProvider * termsAndConditionsProvider = nullptr; #endif @@ -320,18 +315,18 @@ struct CommonCaseDeviceServerInitParams : public ServerInitParams #endif #if CHIP_CONFIG_TC_REQUIRED - static app::DefaultEnhancedSetupFlowProvider sDefaultEnhancedSetupFlowProviderInstance; static app::DefaultTermsAndConditionsProvider sDefaultTermsAndConditionsProviderInstance; + if (this->termsAndConditionsProvider == nullptr) { - ReturnErrorOnFailure(sDefaultTermsAndConditionsProviderInstance.Init(this->persistentStorageDelegate)); - this->termsAndConditionsProvider = &sDefaultTermsAndConditionsProviderInstance; - } + Optional termsAndConditions = Optional({ + .value = CHIP_CONFIG_TC_REQUIRED_ACKNOWLEDGEMENTS, + .version = CHIP_CONFIG_TC_REQUIRED_ACKNOWLEDGEMENTS_VERSION, + }); - if (this->enhancedSetupFlowProvider == nullptr) - { - ReturnErrorOnFailure(sDefaultEnhancedSetupFlowProviderInstance.Init(this->termsAndConditionsProvider)); - this->enhancedSetupFlowProvider = &sDefaultEnhancedSetupFlowProviderInstance; + ReturnErrorOnFailure( + sDefaultTermsAndConditionsProviderInstance.Init(this->persistentStorageDelegate, termsAndConditions)); + this->termsAndConditionsProvider = &sDefaultTermsAndConditionsProviderInstance; } #endif @@ -438,7 +433,7 @@ class Server app::reporting::ReportScheduler * GetReportScheduler() { return mReportScheduler; } #if CHIP_CONFIG_TC_REQUIRED - app::EnhancedSetupFlowProvider * GetEnhancedSetupFlowProvider() { return mEnhancedSetupFlowProvider; } + app::TermsAndConditionsProvider * GetTermsAndConditionsProvider() { return mTermsAndConditionsProvider; } #endif #if CHIP_CONFIG_ENABLE_ICD_SERVER @@ -715,7 +710,6 @@ class Server ServerFabricDelegate mFabricDelegate; app::reporting::ReportScheduler * mReportScheduler; #if CHIP_CONFIG_TC_REQUIRED - app::EnhancedSetupFlowProvider * mEnhancedSetupFlowProvider; app::TermsAndConditionsProvider * mTermsAndConditionsProvider; #endif diff --git a/src/app/server/TermsAndConditionsProvider.h b/src/app/server/TermsAndConditionsProvider.h index 5a1909e9c51e6b..4cd9641641e4ef 100644 --- a/src/app/server/TermsAndConditionsProvider.h +++ b/src/app/server/TermsAndConditionsProvider.h @@ -20,7 +20,6 @@ #include -#include #include #include diff --git a/src/app/tests/BUILD.gn b/src/app/tests/BUILD.gn index 40cf05ecda478a..302799f56f6f93 100644 --- a/src/app/tests/BUILD.gn +++ b/src/app/tests/BUILD.gn @@ -199,7 +199,6 @@ chip_test_suite("tests") { "TestCommandPathParams.cpp", "TestConcreteAttributePath.cpp", "TestDataModelSerialization.cpp", - "TestDefaultEnhancedSetupFlowProvider.cpp", "TestDefaultOTARequestorStorage.cpp", "TestDefaultTermsAndConditionsProvider.cpp", "TestDefaultThreadNetworkDirectoryStorage.cpp", @@ -240,7 +239,6 @@ chip_test_suite("tests") { "${chip_root}/src/app/common:cluster-objects", "${chip_root}/src/app/icd/client:manager", "${chip_root}/src/app/server:enhanced_setup_flow", - "${chip_root}/src/app/server:server", "${chip_root}/src/app/tests:helpers", "${chip_root}/src/app/util/mock:mock_codegen_data_model", "${chip_root}/src/app/util/mock:mock_ember", diff --git a/src/app/tests/TestDefaultEnhancedSetupFlowProvider.cpp b/src/app/tests/TestDefaultEnhancedSetupFlowProvider.cpp deleted file mode 100644 index 24213068e6b1a2..00000000000000 --- a/src/app/tests/TestDefaultEnhancedSetupFlowProvider.cpp +++ /dev/null @@ -1,312 +0,0 @@ -/* - * Copyright (c) 2024 Project CHIP Authors - * All rights reserved. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -#include "app/server/DefaultEnhancedSetupFlowProvider.h" - -#include -#include -#include - -class FakeTermsAndConditionsProvider : public chip::app::TermsAndConditionsProvider -{ -public: - FakeTermsAndConditionsProvider(uint16_t inAcceptedAcknowledgements, uint16_t inAcceptedAcknowledgementsVersion, - uint16_t inRequiredAcknowledgements, uint16_t inRequiredAcknowledgementsVersion) : - mAcceptedAcknowledgements(inAcceptedAcknowledgements), - mAcceptedAcknowledgementsVersion(inAcceptedAcknowledgementsVersion), mRequiredAcknowledgements(inRequiredAcknowledgements), - mRequiredAcknowledgementsVersion(inRequiredAcknowledgementsVersion) - {} - - CHIP_ERROR RevertAcceptance() override - { - mAcceptedAcknowledgements = 0; - mAcceptedAcknowledgementsVersion = 0; - return CHIP_NO_ERROR; - } - - CHIP_ERROR CommitAcceptance() override { return CHIP_NO_ERROR; } - - CHIP_ERROR GetAcceptance(uint16_t & outAcknowledgements, uint16_t & outAcknowledgementsVersion) const override - { - outAcknowledgements = mAcceptedAcknowledgements; - outAcknowledgementsVersion = mAcceptedAcknowledgementsVersion; - return CHIP_NO_ERROR; - } - - CHIP_ERROR GetRequirements(uint16_t & outAcknowledgements, uint16_t & outAcknowledgementsVersion) const override - { - outAcknowledgements = mRequiredAcknowledgements; - outAcknowledgementsVersion = mRequiredAcknowledgementsVersion; - return CHIP_NO_ERROR; - } - - CHIP_ERROR HasAcceptance(bool & outHasAcceptance) const override { return CHIP_NO_ERROR; } - - CHIP_ERROR SetAcceptance(uint16_t inAcknowledgements, uint16_t inAcknowledgementsVersion) override - { - mAcceptedAcknowledgements = inAcknowledgements; - mAcceptedAcknowledgementsVersion = inAcknowledgementsVersion; - return CHIP_NO_ERROR; - } - - CHIP_ERROR ResetAcceptance() override { return CHIP_NO_ERROR; } - -private: - uint16_t mAcceptedAcknowledgements; - uint16_t mAcceptedAcknowledgementsVersion; - uint16_t mRequiredAcknowledgements; - uint16_t mRequiredAcknowledgementsVersion; -}; - -TEST(DefaultEnhancedSetupFlowProvider, TestNoAcceptanceRequiredCheckAcknowledgementsAcceptedSuccess) -{ - CHIP_ERROR err; - bool hasTermsBeenAccepted; - - FakeTermsAndConditionsProvider tncProvider(0, 0, 0, 0); - chip::app::DefaultEnhancedSetupFlowProvider esfProvider; - - err = esfProvider.Init(&tncProvider); - EXPECT_EQ(CHIP_NO_ERROR, err); - - err = esfProvider.HasTermsAndConditionsRequiredAcknowledgementsBeenAccepted(hasTermsBeenAccepted); - EXPECT_EQ(CHIP_NO_ERROR, err); - EXPECT_TRUE(hasTermsBeenAccepted); -} - -TEST(DefaultEnhancedSetupFlowProvider, TestNoAcceptanceRequiredCheckAcknowledgementsVersionAcceptedSuccess) -{ - CHIP_ERROR err; - bool hasTermsVersionBeenAccepted; - - FakeTermsAndConditionsProvider tncProvider(0, 0, 0, 0); - chip::app::DefaultEnhancedSetupFlowProvider esfProvider; - - err = esfProvider.Init(&tncProvider); - EXPECT_EQ(CHIP_NO_ERROR, err); - - err = esfProvider.HasTermsAndConditionsRequiredAcknowledgementsVersionBeenAccepted(hasTermsVersionBeenAccepted); - EXPECT_EQ(CHIP_NO_ERROR, err); - EXPECT_TRUE(hasTermsVersionBeenAccepted); -} - -TEST(DefaultEnhancedSetupFlowProvider, TestAcceptanceRequiredNoTermsAcceptedCheckAcknowledgementsAcceptedFailure) -{ - CHIP_ERROR err; - bool hasTermsBeenAccepted; - - FakeTermsAndConditionsProvider tncProvider(0, 0, 1, 1); - chip::app::DefaultEnhancedSetupFlowProvider esfProvider; - - err = esfProvider.Init(&tncProvider); - EXPECT_EQ(CHIP_NO_ERROR, err); - - err = esfProvider.HasTermsAndConditionsRequiredAcknowledgementsBeenAccepted(hasTermsBeenAccepted); - EXPECT_EQ(CHIP_NO_ERROR, err); - EXPECT_FALSE(hasTermsBeenAccepted); -} - -TEST(DefaultEnhancedSetupFlowProvider, - TestAcceptanceRequiredTermsAcceptedTermsVersionOutdatedCheckAcknowledgementsVersionAcceptedFailure) -{ - CHIP_ERROR err; - bool hasTermsBeenAccepted; - bool hasTermsVersionBeenAccepted; - - FakeTermsAndConditionsProvider tncProvider(0, 0, 1, 1); - chip::app::DefaultEnhancedSetupFlowProvider esfProvider; - - err = esfProvider.Init(&tncProvider); - EXPECT_EQ(CHIP_NO_ERROR, err); - - err = esfProvider.SetTermsAndConditionsAcceptance(1, 0); - EXPECT_EQ(CHIP_NO_ERROR, err); - - err = esfProvider.HasTermsAndConditionsRequiredAcknowledgementsBeenAccepted(hasTermsBeenAccepted); - EXPECT_EQ(CHIP_NO_ERROR, err); - EXPECT_TRUE(hasTermsBeenAccepted); - - err = esfProvider.HasTermsAndConditionsRequiredAcknowledgementsVersionBeenAccepted(hasTermsVersionBeenAccepted); - EXPECT_EQ(CHIP_NO_ERROR, err); - EXPECT_FALSE(hasTermsVersionBeenAccepted); -} - -TEST(DefaultEnhancedSetupFlowProvider, TestAcceptanceRequiredTermsAcceptedFutureVersionCheckAcknowledgementsAcceptedSuccess) -{ - CHIP_ERROR err; - bool hasTermsBeenAccepted; - bool hasTermsVersionBeenAccepted; - - uint16_t acceptedTerms = 1; - uint16_t requiredTerms = 1; - uint16_t acceptedTermsVersion = 2; - uint16_t requiredTermsVersion = 1; - - FakeTermsAndConditionsProvider tncProvider(acceptedTerms, acceptedTermsVersion, requiredTerms, requiredTermsVersion); - chip::app::DefaultEnhancedSetupFlowProvider esfProvider; - - err = esfProvider.Init(&tncProvider); - EXPECT_EQ(CHIP_NO_ERROR, err); - - err = esfProvider.HasTermsAndConditionsRequiredAcknowledgementsBeenAccepted(hasTermsBeenAccepted); - EXPECT_EQ(CHIP_NO_ERROR, err); - EXPECT_TRUE(hasTermsBeenAccepted); - - err = esfProvider.HasTermsAndConditionsRequiredAcknowledgementsVersionBeenAccepted(hasTermsVersionBeenAccepted); - EXPECT_EQ(CHIP_NO_ERROR, err); - EXPECT_TRUE(hasTermsVersionBeenAccepted); -} - -TEST(DefaultEnhancedSetupFlowProvider, TestAcceptanceRequiredTermsAcceptedSuccess) -{ - CHIP_ERROR err; - bool hasTermsBeenAccepted; - bool hasTermsVersionBeenAccepted; - - FakeTermsAndConditionsProvider tncProvider(0, 0, 1, 1); - chip::app::DefaultEnhancedSetupFlowProvider esfProvider; - - err = esfProvider.Init(&tncProvider); - EXPECT_EQ(CHIP_NO_ERROR, err); - - err = esfProvider.SetTermsAndConditionsAcceptance(1, 1); - EXPECT_EQ(CHIP_NO_ERROR, err); - - err = esfProvider.HasTermsAndConditionsRequiredAcknowledgementsBeenAccepted(hasTermsBeenAccepted); - EXPECT_EQ(CHIP_NO_ERROR, err); - EXPECT_TRUE(hasTermsBeenAccepted); - - err = esfProvider.HasTermsAndConditionsRequiredAcknowledgementsVersionBeenAccepted(hasTermsVersionBeenAccepted); - EXPECT_EQ(CHIP_NO_ERROR, err); - EXPECT_TRUE(hasTermsVersionBeenAccepted); -} - -TEST(DefaultEnhancedSetupFlowProvider, TestAcceptanceRequiredTermsMissingFailure) -{ - CHIP_ERROR err; - bool hasTermsBeenAccepted; - bool hasTermsVersionBeenAccepted; - - uint16_t acceptedTerms = 0b0111'1111'1111'1111; - uint16_t requiredTerms = 0b1111'1111'1111'1111; - uint16_t acceptedTermsVersion = 1; - uint16_t requiredTermsVersion = 1; - - FakeTermsAndConditionsProvider tncProvider(acceptedTerms, acceptedTermsVersion, requiredTerms, requiredTermsVersion); - chip::app::DefaultEnhancedSetupFlowProvider esfProvider; - - err = esfProvider.Init(&tncProvider); - EXPECT_EQ(CHIP_NO_ERROR, err); - - err = esfProvider.HasTermsAndConditionsRequiredAcknowledgementsBeenAccepted(hasTermsBeenAccepted); - EXPECT_EQ(CHIP_NO_ERROR, err); - EXPECT_FALSE(hasTermsBeenAccepted); - - err = esfProvider.HasTermsAndConditionsRequiredAcknowledgementsVersionBeenAccepted(hasTermsVersionBeenAccepted); - EXPECT_EQ(CHIP_NO_ERROR, err); - EXPECT_TRUE(hasTermsVersionBeenAccepted); -} - -TEST(DefaultEnhancedSetupFlowProvider, TestAcceptanceRequiredAllTermsAcceptedCheckAcknowledgementsAcceptedSuccess) -{ - CHIP_ERROR err; - bool hasTermsBeenAccepted; - bool hasTermsVersionBeenAccepted; - - uint16_t acceptedTerms = 0b1111'1111'1111'1111; - uint16_t requiredTerms = 0b1111'1111'1111'1111; - uint16_t acceptedTermsVersion = 1; - uint16_t requiredTermsVersion = 1; - - FakeTermsAndConditionsProvider tncProvider(acceptedTerms, acceptedTermsVersion, requiredTerms, requiredTermsVersion); - chip::app::DefaultEnhancedSetupFlowProvider esfProvider; - - err = esfProvider.Init(&tncProvider); - EXPECT_EQ(CHIP_NO_ERROR, err); - - err = esfProvider.HasTermsAndConditionsRequiredAcknowledgementsBeenAccepted(hasTermsBeenAccepted); - EXPECT_EQ(CHIP_NO_ERROR, err); - EXPECT_TRUE(hasTermsBeenAccepted); - - err = esfProvider.HasTermsAndConditionsRequiredAcknowledgementsVersionBeenAccepted(hasTermsVersionBeenAccepted); - EXPECT_EQ(CHIP_NO_ERROR, err); - EXPECT_TRUE(hasTermsVersionBeenAccepted); -} - -TEST(DefaultEnhancedSetupFlowProvider, TestRevertAcceptanceRetainsRequirements) -{ - CHIP_ERROR err; - - uint16_t initialAcceptedTermsAndConditions = 0; - uint16_t initialRequiredTermsAndConditions = 0b1111'1111'1111'1111; - uint16_t initialAcceptedTermsAndConditionsVersion = 0; - uint16_t initialRequiredTermsAndConditionsVersion = 1; - - uint16_t outAcceptedTermsAndConditions; - uint16_t outRequiredTermsAndConditions; - uint16_t outAcceptedTermsAndConditionsVersion; - uint16_t outRequiredTermsAndConditionsVersion; - - uint16_t updatedAcceptedTermsAndConditions = 0b1111'1111'1111'1111; - uint16_t updatedAcceptedTermsAndConditionsVersion = 1; - - FakeTermsAndConditionsProvider tncProvider(initialAcceptedTermsAndConditions, initialAcceptedTermsAndConditionsVersion, - initialRequiredTermsAndConditions, initialRequiredTermsAndConditionsVersion); - - chip::app::DefaultEnhancedSetupFlowProvider esfProvider; - - err = esfProvider.Init(&tncProvider); - EXPECT_EQ(CHIP_NO_ERROR, err); - - err = esfProvider.SetTermsAndConditionsAcceptance(updatedAcceptedTermsAndConditions, updatedAcceptedTermsAndConditionsVersion); - EXPECT_EQ(CHIP_NO_ERROR, err); - - err = esfProvider.GetTermsAndConditionsRequiredAcknowledgements(outRequiredTermsAndConditions); - EXPECT_EQ(CHIP_NO_ERROR, err); - EXPECT_EQ(outRequiredTermsAndConditions, initialRequiredTermsAndConditions); - - err = esfProvider.GetTermsAndConditionsRequiredAcknowledgementsVersion(outRequiredTermsAndConditionsVersion); - EXPECT_EQ(CHIP_NO_ERROR, err); - EXPECT_EQ(outRequiredTermsAndConditionsVersion, initialRequiredTermsAndConditionsVersion); - - err = esfProvider.GetTermsAndConditionsAcceptedAcknowledgements(outAcceptedTermsAndConditions); - EXPECT_EQ(CHIP_NO_ERROR, err); - EXPECT_EQ(outAcceptedTermsAndConditions, updatedAcceptedTermsAndConditions); - - err = esfProvider.GetTermsAndConditionsAcceptedAcknowledgementsVersion(outAcceptedTermsAndConditionsVersion); - EXPECT_EQ(CHIP_NO_ERROR, err); - EXPECT_EQ(outAcceptedTermsAndConditionsVersion, updatedAcceptedTermsAndConditionsVersion); - - err = esfProvider.RevertTermsAndConditionsAcceptance(); - EXPECT_EQ(CHIP_NO_ERROR, err); - - err = esfProvider.GetTermsAndConditionsRequiredAcknowledgements(outRequiredTermsAndConditions); - EXPECT_EQ(CHIP_NO_ERROR, err); - EXPECT_EQ(outRequiredTermsAndConditions, initialRequiredTermsAndConditions); - - err = esfProvider.GetTermsAndConditionsRequiredAcknowledgementsVersion(outRequiredTermsAndConditionsVersion); - EXPECT_EQ(CHIP_NO_ERROR, err); - EXPECT_EQ(outRequiredTermsAndConditionsVersion, initialRequiredTermsAndConditionsVersion); - - err = esfProvider.GetTermsAndConditionsAcceptedAcknowledgements(outAcceptedTermsAndConditions); - EXPECT_EQ(CHIP_NO_ERROR, err); - EXPECT_EQ(outAcceptedTermsAndConditions, 0); - - err = esfProvider.GetTermsAndConditionsAcceptedAcknowledgementsVersion(outAcceptedTermsAndConditionsVersion); - EXPECT_EQ(CHIP_NO_ERROR, err); - EXPECT_EQ(outAcceptedTermsAndConditionsVersion, 0); -} diff --git a/src/app/tests/TestDefaultTermsAndConditionsProvider.cpp b/src/app/tests/TestDefaultTermsAndConditionsProvider.cpp index 474dda7a6b2785..bb1fff24806b2a 100644 --- a/src/app/tests/TestDefaultTermsAndConditionsProvider.cpp +++ b/src/app/tests/TestDefaultTermsAndConditionsProvider.cpp @@ -17,6 +17,7 @@ */ #include "app/server/DefaultTermsAndConditionsProvider.h" +#include "app/server/TermsAndConditionsProvider.h" #include #include @@ -30,9 +31,12 @@ TEST(DefaultTermsAndConditionsProvider, TestInitSuccess) chip::TestPersistentStorageDelegate storageDelegate; chip::app::DefaultTermsAndConditionsProvider tncProvider; - uint16_t requiredAcknowledgements = 1; - uint16_t requiredAcknowledgementsVersion = 1; - err = tncProvider.Init(&storageDelegate, requiredAcknowledgements, requiredAcknowledgementsVersion); + chip::Optional termsAndConditions = chip::Optional({ + .value = 1, + .version = 1, + }); + + err = tncProvider.Init(&storageDelegate, termsAndConditions); EXPECT_EQ(CHIP_NO_ERROR, err); } @@ -43,17 +47,15 @@ TEST(DefaultTermsAndConditionsProvider, TestNoRequirementsGetRequirementsSuccess chip::TestPersistentStorageDelegate storageDelegate; chip::app::DefaultTermsAndConditionsProvider tncProvider; - uint16_t requiredAcknowledgements = 0; - uint16_t requiredAcknowledgementsVersion = 0; - err = tncProvider.Init(&storageDelegate, requiredAcknowledgements, requiredAcknowledgementsVersion); + chip::Optional termsAndConditions = chip::Optional(); + + err = tncProvider.Init(&storageDelegate, termsAndConditions); EXPECT_EQ(CHIP_NO_ERROR, err); - uint16_t outAcceptance; - uint16_t outAcknowledgementsVersion; - err = tncProvider.GetAcceptance(outAcceptance, outAcknowledgementsVersion); + chip::Optional outTermsAndConditions; + err = tncProvider.GetAcceptance(outTermsAndConditions); EXPECT_EQ(CHIP_NO_ERROR, err); - EXPECT_EQ(0, outAcceptance); - EXPECT_EQ(0, outAcknowledgementsVersion); + EXPECT_FALSE(outTermsAndConditions.HasValue()); } TEST(DefaultTermsAndConditionsProvider, TestNeverAcceptanceGetAcceptanceSuccess) @@ -63,17 +65,18 @@ TEST(DefaultTermsAndConditionsProvider, TestNeverAcceptanceGetAcceptanceSuccess) chip::TestPersistentStorageDelegate storageDelegate; chip::app::DefaultTermsAndConditionsProvider tncProvider; - uint16_t requiredAcknowledgements = 0b1111'1111'1111'1111; - uint16_t requiredAcknowledgementsVersion = 1; - err = tncProvider.Init(&storageDelegate, requiredAcknowledgements, requiredAcknowledgementsVersion); + chip::Optional termsAndConditions = chip::Optional({ + .value = 0b1111'1111'1111'1111, + .version = 1, + }); + + err = tncProvider.Init(&storageDelegate, termsAndConditions); EXPECT_EQ(CHIP_NO_ERROR, err); - uint16_t outAcceptance; - uint16_t outAcknowledgementsVersion; - err = tncProvider.GetAcceptance(outAcceptance, outAcknowledgementsVersion); + chip::Optional outTermsAndConditions; + err = tncProvider.GetAcceptance(outTermsAndConditions); EXPECT_EQ(CHIP_NO_ERROR, err); - EXPECT_EQ(0, outAcceptance); - EXPECT_EQ(0, outAcknowledgementsVersion); + EXPECT_FALSE(outTermsAndConditions.HasValue()); } TEST(DefaultTermsAndConditionsProvider, TestTermsAcceptedPersistsSuccess) @@ -84,35 +87,41 @@ TEST(DefaultTermsAndConditionsProvider, TestTermsAcceptedPersistsSuccess) chip::app::DefaultTermsAndConditionsProvider tncProvider; chip::app::DefaultTermsAndConditionsProvider anotherTncProvider; - uint16_t requiredAcknowledgements = 1; - uint16_t requiredAcknowledgementsVersion = 1; - err = tncProvider.Init(&storageDelegate, requiredAcknowledgements, requiredAcknowledgementsVersion); + chip::Optional termsAndConditions = chip::Optional({ + .value = 1, + .version = 1, + }); + + err = tncProvider.Init(&storageDelegate, termsAndConditions); EXPECT_EQ(CHIP_NO_ERROR, err); - uint16_t acceptedTermsAndConditions = 1; - uint16_t acceptedTermsAndConditionsVersion = 1; - err = tncProvider.SetAcceptance(acceptedTermsAndConditions, acceptedTermsAndConditionsVersion); + chip::Optional newTermsAndConditions = chip::Optional({ + .value = 1, + .version = 1, + }); + + err = tncProvider.SetAcceptance(newTermsAndConditions); EXPECT_EQ(CHIP_NO_ERROR, err); - uint16_t outAcceptance; - uint16_t outAcknowledgementsVersion; - err = tncProvider.GetAcceptance(outAcceptance, outAcknowledgementsVersion); + chip::Optional outTermsAndConditions; + err = tncProvider.GetAcceptance(outTermsAndConditions); EXPECT_EQ(CHIP_NO_ERROR, err); - EXPECT_EQ(1, outAcceptance); - EXPECT_EQ(1, outAcknowledgementsVersion); + EXPECT_EQ(1, outTermsAndConditions.Value().value); + EXPECT_EQ(1, outTermsAndConditions.Value().version); err = tncProvider.CommitAcceptance(); + err = tncProvider.GetAcceptance(outTermsAndConditions); EXPECT_EQ(CHIP_NO_ERROR, err); - EXPECT_EQ(1, outAcceptance); - EXPECT_EQ(1, outAcknowledgementsVersion); + EXPECT_EQ(1, outTermsAndConditions.Value().value); + EXPECT_EQ(1, outTermsAndConditions.Value().version); - err = anotherTncProvider.Init(&storageDelegate, requiredAcknowledgements, requiredAcknowledgementsVersion); + err = anotherTncProvider.Init(&storageDelegate, termsAndConditions); EXPECT_EQ(CHIP_NO_ERROR, err); - err = anotherTncProvider.GetAcceptance(outAcceptance, outAcknowledgementsVersion); + err = anotherTncProvider.GetAcceptance(outTermsAndConditions); EXPECT_EQ(CHIP_NO_ERROR, err); - EXPECT_EQ(1, outAcceptance); - EXPECT_EQ(1, outAcknowledgementsVersion); + EXPECT_EQ(1, outTermsAndConditions.Value().value); + EXPECT_EQ(1, outTermsAndConditions.Value().version); } TEST(DefaultTermsAndConditionsProvider, TestTermsRequiredGetRequirementsSuccess) @@ -122,17 +131,19 @@ TEST(DefaultTermsAndConditionsProvider, TestTermsRequiredGetRequirementsSuccess) chip::TestPersistentStorageDelegate storageDelegate; chip::app::DefaultTermsAndConditionsProvider tncProvider; - uint16_t initialRequiredAcknowledgements = 1; - uint16_t initialRequiredAcknowledgementsVersion = 1; - err = tncProvider.Init(&storageDelegate, initialRequiredAcknowledgements, initialRequiredAcknowledgementsVersion); + chip::Optional termsAndConditions = chip::Optional({ + .value = 1, + .version = 1, + }); + + err = tncProvider.Init(&storageDelegate, termsAndConditions); EXPECT_EQ(CHIP_NO_ERROR, err); - uint16_t outRequiredAcknowledgements; - uint16_t outRequiredAcknowledgementsVersion; - err = tncProvider.GetRequirements(outRequiredAcknowledgements, outRequiredAcknowledgementsVersion); + chip::Optional outTermsAndConditions; + err = tncProvider.GetRequirements(outTermsAndConditions); EXPECT_EQ(CHIP_NO_ERROR, err); - EXPECT_EQ(1, outRequiredAcknowledgements); - EXPECT_EQ(1, outRequiredAcknowledgementsVersion); + EXPECT_EQ(1, outTermsAndConditions.Value().value); + EXPECT_EQ(1, outTermsAndConditions.Value().version); } TEST(DefaultTermsAndConditionsProvider, TestSetAcceptanceGetAcceptanceSuccess) @@ -142,22 +153,26 @@ TEST(DefaultTermsAndConditionsProvider, TestSetAcceptanceGetAcceptanceSuccess) chip::TestPersistentStorageDelegate storageDelegate; chip::app::DefaultTermsAndConditionsProvider tncProvider; - uint16_t requiredAcknowledgements = 1; - uint16_t requiredAcknowledgementsVersion = 1; - err = tncProvider.Init(&storageDelegate, requiredAcknowledgements, requiredAcknowledgementsVersion); + chip::Optional termsAndConditions = chip::Optional({ + .value = 1, + .version = 1, + }); + + err = tncProvider.Init(&storageDelegate, termsAndConditions); EXPECT_EQ(CHIP_NO_ERROR, err); - uint16_t acceptedTermsAndConditions = 1; - uint16_t acceptedTermsAndConditionsVersion = 1; - err = tncProvider.SetAcceptance(acceptedTermsAndConditions, acceptedTermsAndConditionsVersion); + chip::Optional acceptedTermsAndConditions = chip::Optional({ + .value = 1, + .version = 1, + }); + err = tncProvider.SetAcceptance(acceptedTermsAndConditions); EXPECT_EQ(CHIP_NO_ERROR, err); - uint16_t outAcceptance; - uint16_t outAcknowledgementsVersion; - err = tncProvider.GetAcceptance(outAcceptance, outAcknowledgementsVersion); + chip::Optional outTermsAndConditions; + err = tncProvider.GetRequirements(outTermsAndConditions); EXPECT_EQ(CHIP_NO_ERROR, err); - EXPECT_EQ(1, outAcceptance); - EXPECT_EQ(1, outAcknowledgementsVersion); + EXPECT_EQ(1, outTermsAndConditions.Value().value); + EXPECT_EQ(1, outTermsAndConditions.Value().version); } TEST(DefaultTermsAndConditionsProvider, TestRevertAcceptanceGetAcceptanceSuccess) @@ -167,32 +182,34 @@ TEST(DefaultTermsAndConditionsProvider, TestRevertAcceptanceGetAcceptanceSuccess chip::TestPersistentStorageDelegate storageDelegate; chip::app::DefaultTermsAndConditionsProvider tncProvider; - uint16_t requiredAcknowledgements = 1; - uint16_t requiredAcknowledgementsVersion = 1; - err = tncProvider.Init(&storageDelegate, requiredAcknowledgements, requiredAcknowledgementsVersion); + chip::Optional termsAndConditions = chip::Optional({ + .value = 1, + .version = 1, + }); + + err = tncProvider.Init(&storageDelegate, termsAndConditions); EXPECT_EQ(CHIP_NO_ERROR, err); - uint16_t acceptedTermsAndConditions = 1; - uint16_t acceptedTermsAndConditionsVersion = 1; - err = tncProvider.SetAcceptance(acceptedTermsAndConditions, acceptedTermsAndConditionsVersion); + chip::Optional acceptedTermsAndConditions = chip::Optional({ + .value = 1, + .version = 1, + }); + err = tncProvider.SetAcceptance(acceptedTermsAndConditions); EXPECT_EQ(CHIP_NO_ERROR, err); - uint16_t outAcceptance; - uint16_t outAcknowledgementsVersion; - err = tncProvider.GetAcceptance(outAcceptance, outAcknowledgementsVersion); + chip::Optional outTermsAndConditions; + err = tncProvider.GetRequirements(outTermsAndConditions); EXPECT_EQ(CHIP_NO_ERROR, err); - EXPECT_EQ(1, outAcceptance); - EXPECT_EQ(1, outAcknowledgementsVersion); + EXPECT_EQ(1, outTermsAndConditions.Value().value); + EXPECT_EQ(1, outTermsAndConditions.Value().version); err = tncProvider.RevertAcceptance(); EXPECT_EQ(CHIP_NO_ERROR, err); - uint16_t outAcceptance2; - uint16_t outAcknowledgementsVersion2; - err = tncProvider.GetAcceptance(outAcceptance2, outAcknowledgementsVersion2); + chip::Optional outAcceptance2; + err = tncProvider.GetAcceptance(outAcceptance2); EXPECT_EQ(CHIP_NO_ERROR, err); - EXPECT_EQ(0, outAcceptance2); - EXPECT_EQ(0, outAcknowledgementsVersion2); + EXPECT_FALSE(outAcceptance2.HasValue()); } TEST(DefaultTermsAndConditionsProvider, TestAcceptanceRequiredTermsMissingFailure) @@ -202,30 +219,32 @@ TEST(DefaultTermsAndConditionsProvider, TestAcceptanceRequiredTermsMissingFailur chip::TestPersistentStorageDelegate storageDelegate; chip::app::DefaultTermsAndConditionsProvider tncProvider; - uint16_t requiredAcknowledgements = 1; - uint16_t requiredAcknowledgementsVersion = 1; - err = tncProvider.Init(&storageDelegate, requiredAcknowledgements, requiredAcknowledgementsVersion); + chip::Optional requiredTermsAndConditions = chip::Optional({ + .value = 1, + .version = 1, + }); + err = tncProvider.Init(&storageDelegate, requiredTermsAndConditions); EXPECT_EQ(CHIP_NO_ERROR, err); - uint16_t acceptedTermsAndConditions = 1; - uint16_t acceptedTermsAndConditionsVersion = 1; - err = tncProvider.SetAcceptance(acceptedTermsAndConditions, acceptedTermsAndConditionsVersion); + chip::Optional acceptedTermsAndConditions = chip::Optional({ + .value = 1, + .version = 1, + }); + err = tncProvider.SetAcceptance(acceptedTermsAndConditions); EXPECT_EQ(CHIP_NO_ERROR, err); - uint16_t outAcceptance; - uint16_t outAcknowledgementsVersion; - err = tncProvider.GetAcceptance(outAcceptance, outAcknowledgementsVersion); + chip::Optional outAcknowledgementTermsAndConditions; + err = tncProvider.GetAcceptance(outAcknowledgementTermsAndConditions); EXPECT_EQ(CHIP_NO_ERROR, err); - EXPECT_EQ(1, outAcceptance); - EXPECT_EQ(1, outAcknowledgementsVersion); + EXPECT_EQ(1, outAcknowledgementTermsAndConditions.Value().value); + EXPECT_EQ(1, outAcknowledgementTermsAndConditions.Value().version); err = tncProvider.RevertAcceptance(); EXPECT_EQ(CHIP_NO_ERROR, err); - uint16_t outRequiredAcknowledgements; - uint16_t outRequiredAcknowledgementsVersion; - err = tncProvider.GetRequirements(outRequiredAcknowledgements, outRequiredAcknowledgementsVersion); + chip::Optional outRequiredTermsAndConditions; + err = tncProvider.GetRequirements(outRequiredTermsAndConditions); EXPECT_EQ(CHIP_NO_ERROR, err); - EXPECT_EQ(1, outRequiredAcknowledgements); - EXPECT_EQ(1, outRequiredAcknowledgementsVersion); + EXPECT_EQ(1, outRequiredTermsAndConditions.Value().value); + EXPECT_EQ(1, outRequiredTermsAndConditions.Value().version); } diff --git a/src/controller/CommissioningDelegate.h b/src/controller/CommissioningDelegate.h index a707b05f873385..3d5a6bea646b8e 100644 --- a/src/controller/CommissioningDelegate.h +++ b/src/controller/CommissioningDelegate.h @@ -1,6 +1,6 @@ /* * - * Copyright (c) 2021 Project CHIP Authors + * Copyright (c) 2021-2024 Project CHIP Authors * All rights reserved. * * Licensed under the Apache License, Version 2.0 (the "License"); diff --git a/src/lib/core/CHIPConfig.h b/src/lib/core/CHIPConfig.h index 2cfd53a314b772..87127f99cf6bc4 100644 --- a/src/lib/core/CHIPConfig.h +++ b/src/lib/core/CHIPConfig.h @@ -1,6 +1,6 @@ /* * - * Copyright (c) 2020-2022 Project CHIP Authors + * Copyright (c) 2020-2024 Project CHIP Authors * Copyright (c) 2019 Google LLC. * Copyright (c) 2013-2018 Nest Labs, Inc. * @@ -1841,47 +1841,11 @@ extern const char CHIP_NON_PRODUCTION_MARKER[]; * This macro defines whether the device commissioning process requires the user to acknowledge terms and conditions. * - 1: Terms and conditions are required. * - 0: Terms and conditions are not required. - * - * If this is set to 1, both CHIP_CONFIG_TC_REQUIRED_ACKNOWLEDGEMENTS and - * CHIP_CONFIG_TC_REQUIRED_ACKNOWLEDGEMENTS_VERSION must be defined. */ #ifndef CHIP_CONFIG_TC_REQUIRED -#define CHIP_CONFIG_TC_REQUIRED (0) -#endif - -#if CHIP_CONFIG_TC_REQUIRED - -/** - * @def CHIP_CONFIG_TC_REQUIRED_ACKNOWLEDGEMENTS - * - * @brief Configures the required terms and conditions acknowledgements bitmask. - * - * This macro defines the required terms and conditions acknowledgements bitmask. The bit-field is 16 bits long, so the possible - * value range is [0, 65535). This setting can be used to require that terms and conditions are presented to the user during - * commissioning. - */ -#ifndef CHIP_CONFIG_TC_REQUIRED_ACKNOWLEDGEMENTS -#error "CHIP_CONFIG_TC_REQUIRED_ACKNOWLEDGEMENTS must be defined when CHIP_CONFIG_TC_REQUIRED is enabled." +#define CHIP_CONFIG_TC_REQUIRED 0 #endif -/** - * @def CHIP_CONFIG_TC_REQUIRED_ACKNOWLEDGEMENTS_VERSION - * - * @brief Configures the latest known version of the terms and conditions. - * - * This macro defines the version number of the latest terms and conditions. It allows the application to iterate on revisions of - * the terms and conditions. A value of 0 indicates that no specific version is required. This setting can be used to enforce - * version-specific terms and conditions acknowledgements in the application. When the set of terms and conditions needs to be - * changed, the version number should be monotonically increased. If the latest terms and conditions version is updated (most - * likely during an OTA), then this may signal to the Administrator that updated terms and conditions should be presented to the - * user. - */ -#ifndef CHIP_CONFIG_TC_REQUIRED_ACKNOWLEDGEMENTS_VERSION -#error "CHIP_CONFIG_TC_REQUIRED_ACKNOWLEDGEMENTS_VERSION must be defined when CHIP_CONFIG_TC_REQUIRED is enabled." -#endif - -#endif // CHIP_CONFIG_TC_REQUIRED - /** * @} */ diff --git a/src/python_testing/matter_testing_support.py b/src/python_testing/matter_testing_support.py index df3762e902d2ff..3efaad19beef45 100644 --- a/src/python_testing/matter_testing_support.py +++ b/src/python_testing/matter_testing_support.py @@ -1,5 +1,5 @@ # -# Copyright (c) 2022 Project CHIP Authors +# Copyright (c) 2022-2024 Project CHIP Authors # All rights reserved. # # Licensed under the Apache License, Version 2.0 (the "License"); @@ -915,73 +915,7 @@ def __init__(self, *args): self.problems = [] self.is_commissioning = False - async def commission_device(self, dut_node_idx: int) -> bool: - dev_ctrl = self.default_controller - conf = self.matter_test_config - - info = self.get_setup_payload_info()[dut_node_idx] - - if conf.tc_version is not None and conf.tc_user_response is not None: - logging.debug(f"Setting TC Acknowledgements to version {conf.tc_version} with user response {conf.tc_user_response}.") - dev_ctrl.SetTCAcknowledgements(conf.tc_version, conf.tc_user_response) - dev_ctrl.SetTCRequired(True) - else: - dev_ctrl.SetTCRequired(False) - - if conf.commissioning_method == "on-network": - try: - await dev_ctrl.CommissionOnNetwork( - nodeId=conf.dut_node_ids[dut_node_idx], - setupPinCode=info.passcode, - filterType=info.filter_type, - filter=info.filter_value - ) - return True - except ChipStackError as e: - logging.error("Commissioning failed: %s" % e) - return False - elif conf.commissioning_method == "ble-wifi": - try: - await dev_ctrl.CommissionWiFi( - info.filter_value, - info.passcode, - conf.dut_node_ids[dut_node_idx], - conf.wifi_ssid, - conf.wifi_passphrase, - isShortDiscriminator=(info.filter_type == DiscoveryFilterType.SHORT_DISCRIMINATOR) - ) - return True - except ChipStackError as e: - logging.error("Commissioning failed: %s" % e) - return False - elif conf.commissioning_method == "ble-thread": - try: - await dev_ctrl.CommissionThread( - info.filter_value, - info.passcode, - conf.dut_node_ids[dut_node_idx], - conf.thread_operational_dataset, - isShortDiscriminator=(info.filter_type == DiscoveryFilterType.SHORT_DISCRIMINATOR) - ) - return True - except ChipStackError as e: - logging.error("Commissioning failed: %s" % e) - return False - elif conf.commissioning_method == "on-network-ip": - try: - logging.warning("==== USING A DIRECT IP COMMISSIONING METHOD NOT SUPPORTED IN THE LONG TERM ====") - await dev_ctrl.CommissionIP( - ipaddr=conf.commissionee_ip_address_just_for_testing, - setupPinCode=info.passcode, nodeid=conf.dut_node_ids[dut_node_idx] - ) - return True - except ChipStackError as e: - logging.error("Commissioning failed: %s" % e) - return False - else: - raise ValueError("Invalid commissioning method %s!" % conf.commissioning_method) - - async def commission_devices(self): + async def commission_devices(self) -> bool: conf = self.matter_test_config for commission_idx, node_id in enumerate(conf.dut_node_ids): @@ -989,7 +923,9 @@ async def commission_devices(self): (conf.root_of_trust_index, conf.fabric_id, node_id)) logging.info("Commissioning method: %s" % conf.commissioning_method) - await self.commission_device(commission_idx) + await CommissionDeviceTest.commission_device(self, commission_idx) + + return True def get_test_steps(self, test: str) -> list[TestStep]: ''' Retrieves the test step list for the given test @@ -2195,14 +2131,74 @@ def __init__(self, *args): self.is_commissioning = True def test_run_commissioning(self): - conf = self.matter_test_config - for commission_idx, node_id in enumerate(conf.dut_node_ids): - logging.info("Starting commissioning for root index %d, fabric ID 0x%016X, node ID 0x%016X" % - (conf.root_of_trust_index, conf.fabric_id, node_id)) - logging.info("Commissioning method: %s" % conf.commissioning_method) + if not asyncio.run(self.commission_devices()): + raise signals.TestAbortAll("Failed to commission node") + + async def commission_device(instance: MatterBaseTest, i) -> bool: + dev_ctrl = instance.default_controller + conf = instance.matter_test_config + + info = instance.get_setup_payload_info()[i] + + if conf.tc_version is not None and conf.tc_user_response is not None: + logging.debug(f"Setting TC Acknowledgements to version {conf.tc_version} with user response {conf.tc_user_response}.") + dev_ctrl.SetTCAcknowledgements(conf.tc_version, conf.tc_user_response) + dev_ctrl.SetTCRequired(True) + else: + dev_ctrl.SetTCRequired(False) - if not asyncio.run(self.commission_device(commission_idx)): - raise signals.TestAbortAll("Failed to commission node") + if conf.commissioning_method == "on-network": + try: + await dev_ctrl.CommissionOnNetwork( + nodeId=conf.dut_node_ids[i], + setupPinCode=info.passcode, + filterType=info.filter_type, + filter=info.filter_value + ) + return True + except ChipStackError as e: + logging.error("Commissioning failed: %s" % e) + return False + elif conf.commissioning_method == "ble-wifi": + try: + await dev_ctrl.CommissionWiFi( + info.filter_value, + info.passcode, + conf.dut_node_ids[i], + conf.wifi_ssid, + conf.wifi_passphrase, + isShortDiscriminator=(info.filter_type == DiscoveryFilterType.SHORT_DISCRIMINATOR) + ) + return True + except ChipStackError as e: + logging.error("Commissioning failed: %s" % e) + return False + elif conf.commissioning_method == "ble-thread": + try: + await dev_ctrl.CommissionThread( + info.filter_value, + info.passcode, + conf.dut_node_ids[i], + conf.thread_operational_dataset, + isShortDiscriminator=(info.filter_type == DiscoveryFilterType.SHORT_DISCRIMINATOR) + ) + return True + except ChipStackError as e: + logging.error("Commissioning failed: %s" % e) + return False + elif conf.commissioning_method == "on-network-ip": + try: + logging.warning("==== USING A DIRECT IP COMMISSIONING METHOD NOT SUPPORTED IN THE LONG TERM ====") + await dev_ctrl.CommissionIP( + ipaddr=conf.commissionee_ip_address_just_for_testing, + setupPinCode=info.passcode, nodeid=conf.dut_node_ids[i] + ) + return True + except ChipStackError as e: + logging.error("Commissioning failed: %s" % e) + return False + else: + raise ValueError("Invalid commissioning method %s!" % conf.commissioning_method) def default_matter_test_main(override_matter_test_config: dict = {}):