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 1684638bc69d9f..dc49b5c7b4a728 100644 --- a/src/app/clusters/general-commissioning-server/general-commissioning-server.cpp +++ b/src/app/clusters/general-commissioning-server/general-commissioning-server.cpp @@ -169,45 +169,30 @@ CHIP_ERROR GeneralCommissioningAttrAccess::ReadSupportsConcurrentConnection(Attr return aEncoder.Encode(supportsConcurrentConnection); } -#if CHIP_CONFIG_TC_REQUIRED -CommissioningErrorEnum CheckTermsAndConditionsAcknowledgementsState(const Optional & requiredTermsAndConditions, - const Optional & acceptedTermsAndConditions) +CommissioningErrorEnum CheckTermsAndConditionsAcknowledgementsState(TermsAndConditionsProvider * const termsAndConditionsProvider, const Optional & acceptedTermsAndConditions) { - // No validation checks required if no required terms and conditions - if (!requiredTermsAndConditions.HasValue()) - { - return CommissioningErrorEnum::kOk; - } - - // Validate if we have received any terms and conditions acceptance - if (!acceptedTermsAndConditions.HasValue()) - { - ChipLogError(AppServer, "Failed to HasReceivedTermsAndConditionsAcknowledgements"); - return CommissioningErrorEnum::kTCAcknowledgementsNotReceived; - } + TermsAndConditionsState termsAndConditionsState; - // Validate the accepted version first... - if (requiredTermsAndConditions.Value().version > acceptedTermsAndConditions.Value().version) + CHIP_ERROR err = termsAndConditionsProvider->CheckAcceptance(acceptedTermsAndConditions, termsAndConditionsState); + if (CHIP_NO_ERROR != err) { - ChipLogProgress(AppServer, "Minimum terms and conditions version, 0x%04x, has not been accepted", - requiredTermsAndConditions.Value().version); - return CommissioningErrorEnum::kTCMinVersionNotMet; + return chip::app::Clusters::GeneralCommissioning::CommissioningErrorEnum::kUnknownEnumValue; } - // Validate the accepted bits second... - if (requiredTermsAndConditions.Value().value != - (requiredTermsAndConditions.Value().value & acceptedTermsAndConditions.Value().value)) + switch (termsAndConditionsState) { - ChipLogProgress(AppServer, "Required terms and conditions, 0x%04x,have not been accepted", - requiredTermsAndConditions.Value().value); - return CommissioningErrorEnum::kRequiredTCNotAccepted; + case OK: + return chip::app::Clusters::GeneralCommissioning::CommissioningErrorEnum::kOk; + case TC_ACKNOWLEDGEMENTS_NOT_RECEIVED: + return chip::app::Clusters::GeneralCommissioning::CommissioningErrorEnum::kTCAcknowledgementsNotReceived; + case TC_MIN_VERSION_NOT_MET: + return chip::app::Clusters::GeneralCommissioning::CommissioningErrorEnum::kTCMinVersionNotMet; + case REQUIRED_TC_NOT_ACCEPTED: + return chip::app::Clusters::GeneralCommissioning::CommissioningErrorEnum::kRequiredTCNotAccepted; } - // All validation check succeeded... - return CommissioningErrorEnum::kOk; + return chip::app::Clusters::GeneralCommissioning::CommissioningErrorEnum::kOk; } -#endif - } // anonymous namespace bool emberAfGeneralCommissioningClusterArmFailSafeCallback(app::CommandHandler * commandObj, @@ -282,6 +267,8 @@ bool emberAfGeneralCommissioningClusterCommissioningCompleteCallback( auto & failSafe = Server::GetInstance().GetFailSafeContext(); auto & fabricTable = Server::GetInstance().GetFabricTable(); + CHIP_ERROR err; + ChipLogProgress(FailSafe, "GeneralCommissioning: Received CommissioningComplete"); Commands::CommissioningCompleteResponse::Type response; @@ -305,27 +292,25 @@ bool emberAfGeneralCommissioningClusterCommissioningCompleteCallback( { #if CHIP_CONFIG_TC_REQUIRED TermsAndConditionsProvider * const termsAndConditionsProvider = Server::GetInstance().GetTermsAndConditionsProvider(); - Optional acceptedTermsAndConditions; - Optional requiredTermsAndConditions; - termsAndConditionsProvider->GetAcceptance(acceptedTermsAndConditions); - termsAndConditionsProvider->GetRequirements(requiredTermsAndConditions); + err = termsAndConditionsProvider->GetAcceptance(acceptedTermsAndConditions); + if (CHIP_NO_ERROR != err) + { + // + } - response.errorCode = - CheckTermsAndConditionsAcknowledgementsState(requiredTermsAndConditions, acceptedTermsAndConditions); + response.errorCode = CheckTermsAndConditionsAcknowledgementsState(termsAndConditionsProvider, acceptedTermsAndConditions); if (CommissioningErrorEnum::kOk != response.errorCode) { commandObj->AddResponse(commandPath, response); return true; } -#endif -#if CHIP_CONFIG_TC_REQUIRED if (failSafe.UpdateTermsAndConditionsHasBeenInvoked()) { // Commit terms and conditions acceptance on commissioning complete - CHIP_ERROR err = Server::GetInstance().GetTermsAndConditionsProvider()->CommitAcceptance(); + err = Server::GetInstance().GetTermsAndConditionsProvider()->CommitAcceptance(); if (err != CHIP_NO_ERROR) { ChipLogError(FailSafe, "GeneralCommissioning: Failed to commit terms and conditions: %" CHIP_ERROR_FORMAT, @@ -343,7 +328,7 @@ bool emberAfGeneralCommissioningClusterCommissioningCompleteCallback( if (failSafe.NocCommandHasBeenInvoked()) { // No need to revert on error: CommitPendingFabricData always reverts if not fully successful. - CHIP_ERROR err = fabricTable.CommitPendingFabricData(); + err = fabricTable.CommitPendingFabricData(); if (err != CHIP_NO_ERROR) { ChipLogError(FailSafe, "GeneralCommissioning: Failed to commit pending fabric data: %" CHIP_ERROR_FORMAT, @@ -440,20 +425,15 @@ bool emberAfGeneralCommissioningClusterSetTCAcknowledgementsCallback( MATTER_TRACE_SCOPE("SetTCAcknowledgements", "GeneralCommissioning"); auto & failSafeContext = Server::GetInstance().GetFailSafeContext(); - - Commands::SetTCAcknowledgementsResponse::Type response; TermsAndConditionsProvider * const termsAndConditionsProvider = Server::GetInstance().GetTermsAndConditionsProvider(); - Optional requiredTermsAndConditions; - - termsAndConditionsProvider->GetRequirements(requiredTermsAndConditions); - Optional acceptedTermsAndConditions = Optional({ .value = commandData.TCUserResponse, .version = commandData.TCVersion, }); - response.errorCode = CheckTermsAndConditionsAcknowledgementsState(requiredTermsAndConditions, acceptedTermsAndConditions); + Commands::SetTCAcknowledgementsResponse::Type response; + response.errorCode = CheckTermsAndConditionsAcknowledgementsState(termsAndConditionsProvider, acceptedTermsAndConditions); if (CommissioningErrorEnum::kOk == response.errorCode) { @@ -465,7 +445,7 @@ bool emberAfGeneralCommissioningClusterSetTCAcknowledgementsCallback( } else { - termsAndConditionsProvider->CommitAcceptance(); + CheckSuccess(termsAndConditionsProvider->CommitAcceptance(), Failure); } } diff --git a/src/app/server/DefaultTermsAndConditionsProvider.cpp b/src/app/server/DefaultTermsAndConditionsProvider.cpp index 0c1265c036908a..8310d6cb78a11f 100644 --- a/src/app/server/DefaultTermsAndConditionsProvider.cpp +++ b/src/app/server/DefaultTermsAndConditionsProvider.cpp @@ -41,12 +41,12 @@ constexpr size_t kEstimatedTlvBufferSize = chip::TLV::EstimateStructOverhead(siz CHIP_ERROR chip::app::DefaultTermsAndConditionsProvider::Init( chip::PersistentStorageDelegate * const inPersistentStorageDelegate, - const chip::Optional & inRequiredTermsAndConditions) + const chip::Optional & inmRequiredAcknowledgements) { VerifyOrReturnError(nullptr != inPersistentStorageDelegate, CHIP_ERROR_INVALID_ARGUMENT); mPersistentStorageDelegate = inPersistentStorageDelegate; - mRequiredAcknowledgements = inRequiredTermsAndConditions; + mRequiredAcknowledgements = inmRequiredAcknowledgements; if (CHIP_NO_ERROR == LoadAcceptance(mLatchedAcceptance)) { @@ -56,9 +56,49 @@ CHIP_ERROR chip::app::DefaultTermsAndConditionsProvider::Init( return CHIP_NO_ERROR; } -CHIP_ERROR chip::app::DefaultTermsAndConditionsProvider::RevertAcceptance() +CHIP_ERROR chip::app::DefaultTermsAndConditionsProvider::CheckAcceptance(const Optional & inTermsAndConditions, TermsAndConditionsState & outState) const { - mTemporalAcceptance = mLatchedAcceptance; + // No validation checks required if no required terms and conditions + if (!mRequiredAcknowledgements.HasValue()) + { + outState = TermsAndConditionsState::OK; + return CHIP_NO_ERROR; + } + + // Validate if we have received any terms and conditions acceptance + if (!inTermsAndConditions.HasValue()) + { + ChipLogError(AppServer, "Failed to HasReceivedTermsAndConditionsAcknowledgements"); + outState = TermsAndConditionsState::TC_ACKNOWLEDGEMENTS_NOT_RECEIVED; + return CHIP_NO_ERROR; + } + + // Validate the accepted version first... + if (mRequiredAcknowledgements.Value().version > inTermsAndConditions.Value().version) + { + ChipLogProgress(AppServer, "Minimum terms and conditions version, 0x%04x, has not been accepted", + mRequiredAcknowledgements.Value().version); + outState = TermsAndConditionsState::TC_MIN_VERSION_NOT_MET; + return CHIP_NO_ERROR; + } + + // Validate the accepted bits second... + if (mRequiredAcknowledgements.Value().value != (mRequiredAcknowledgements.Value().value & inTermsAndConditions.Value().value)) + { + ChipLogProgress(AppServer, "Required terms and conditions, 0x%04x,have not been accepted", + mRequiredAcknowledgements.Value().value); + outState = TermsAndConditionsState::REQUIRED_TC_NOT_ACCEPTED; + return CHIP_NO_ERROR; + } + + // All validation check succeeded... + outState = TermsAndConditionsState::OK; + return CHIP_NO_ERROR; +} + +CHIP_ERROR chip::app::DefaultTermsAndConditionsProvider::SetAcceptance(const Optional & inTermsAndConditions) +{ + mTemporalAcceptance.SetValue(inTermsAndConditions.Value()); return CHIP_NO_ERROR; } @@ -73,15 +113,9 @@ CHIP_ERROR chip::app::DefaultTermsAndConditionsProvider::CommitAcceptance() return StoreAcceptance(mLatchedAcceptance); } -CHIP_ERROR chip::app::DefaultTermsAndConditionsProvider::GetAcceptance(Optional & outTermsAndConditions) const -{ - outTermsAndConditions = mTemporalAcceptance; - return CHIP_NO_ERROR; -} - -CHIP_ERROR chip::app::DefaultTermsAndConditionsProvider::GetRequirements(Optional & outTermsAndConditions) const +CHIP_ERROR chip::app::DefaultTermsAndConditionsProvider::RevertAcceptance() { - outTermsAndConditions = mRequiredAcknowledgements; + mTemporalAcceptance = mLatchedAcceptance; return CHIP_NO_ERROR; } @@ -98,9 +132,15 @@ CHIP_ERROR chip::app::DefaultTermsAndConditionsProvider::ResetAcceptance() return CHIP_NO_ERROR; } -CHIP_ERROR chip::app::DefaultTermsAndConditionsProvider::SetAcceptance(const Optional & inTermsAndConditions) +CHIP_ERROR chip::app::DefaultTermsAndConditionsProvider::GetAcceptance(Optional & outTermsAndConditions) const { - mTemporalAcceptance.SetValue(inTermsAndConditions.Value()); + outTermsAndConditions = mTemporalAcceptance; + return CHIP_NO_ERROR; +} + +CHIP_ERROR chip::app::DefaultTermsAndConditionsProvider::GetRequirements(Optional & outTermsAndConditions) const +{ + outTermsAndConditions = mRequiredAcknowledgements; return CHIP_NO_ERROR; } diff --git a/src/app/server/DefaultTermsAndConditionsProvider.h b/src/app/server/DefaultTermsAndConditionsProvider.h index fc3e49e61f7613..7172a63b1199a0 100644 --- a/src/app/server/DefaultTermsAndConditionsProvider.h +++ b/src/app/server/DefaultTermsAndConditionsProvider.h @@ -39,6 +39,8 @@ class DefaultTermsAndConditionsProvider : public TermsAndConditionsProvider CHIP_ERROR Init(PersistentStorageDelegate * const inPersistentStorageDelegate, const chip::Optional & inRequiredTermsAndConditions); + CHIP_ERROR CheckAcceptance(const Optional & inTermsAndConditions, TermsAndConditionsState & outState) const override; + CHIP_ERROR SetAcceptance(const Optional & inTermsAndConditions) override; CHIP_ERROR CommitAcceptance() override; diff --git a/src/app/server/TermsAndConditionsProvider.h b/src/app/server/TermsAndConditionsProvider.h index 4cd9641641e4ef..9c952e937e3ed2 100644 --- a/src/app/server/TermsAndConditionsProvider.h +++ b/src/app/server/TermsAndConditionsProvider.h @@ -32,6 +32,14 @@ typedef struct sTermsAndConditions uint16_t version; } TermsAndConditions; +typedef enum eTermsAndConditionsState +{ + OK = 0, + TC_ACKNOWLEDGEMENTS_NOT_RECEIVED = 1, + TC_MIN_VERSION_NOT_MET = 2, + REQUIRED_TC_NOT_ACCEPTED = 3, +} TermsAndConditionsState; + /** * @brief Data access layer for the required terms and conditions and the store for the user acceptance. */ @@ -40,6 +48,11 @@ class TermsAndConditionsProvider public: virtual ~TermsAndConditionsProvider() = default; + /** + * @brief Checks the acceptance status without latching the value, nor temporal storage. + */ + virtual CHIP_ERROR CheckAcceptance(const Optional & inTermsAndConditions, TermsAndConditionsState & outState) const = 0; + /** * @brief Sets the acceptance status of the required terms and conditions. */