From 7f8a2b4d5eabd4a97719c4f6dcc516f482505736 Mon Sep 17 00:00:00 2001 From: JohnMcPMS Date: Mon, 7 Jun 2021 21:02:33 -0700 Subject: [PATCH 1/7] Checkpoint, needs implementation of sync and possibly more code moved around --- .../Public/winget/Settings.h | 40 +- src/AppInstallerCommonCore/UserSettings.cpp | 20 +- .../AppInstallerRepositoryCore.vcxproj | 4 + ...AppInstallerRepositoryCore.vcxproj.filters | 12 + .../RepositorySource.cpp | 682 +----------------- src/AppInstallerRepositoryCore/SourceList.cpp | 645 +++++++++++++++++ src/AppInstallerRepositoryCore/SourceList.h | 57 ++ .../SourcePolicy.cpp | 169 +++++ src/AppInstallerRepositoryCore/SourcePolicy.h | 16 + 9 files changed, 954 insertions(+), 691 deletions(-) create mode 100644 src/AppInstallerRepositoryCore/SourceList.cpp create mode 100644 src/AppInstallerRepositoryCore/SourceList.h create mode 100644 src/AppInstallerRepositoryCore/SourcePolicy.cpp create mode 100644 src/AppInstallerRepositoryCore/SourcePolicy.h diff --git a/src/AppInstallerCommonCore/Public/winget/Settings.h b/src/AppInstallerCommonCore/Public/winget/Settings.h index 5648a3a8d9..d30189981e 100644 --- a/src/AppInstallerCommonCore/Public/winget/Settings.h +++ b/src/AppInstallerCommonCore/Public/winget/Settings.h @@ -35,10 +35,12 @@ namespace AppInstaller::Settings std::string_view Path; }; - // The set of well known settings streams. - // Changing these values can result in data loss. - struct Streams + // A setting stream; provides access to functionality on the stream. + struct Stream { + // The set of well known settings streams. + // Changing these values can result in data loss. + // The set of sources as defined by the user. constexpr static StreamDefinition UserSources{ Type::Secure, "user_sources"sv }; // The metadata about all sources. @@ -47,18 +49,30 @@ namespace AppInstaller::Settings constexpr static StreamDefinition PrimaryUserSettings{ Type::UserFile, "settings.json"sv }; // The backup user settings file. constexpr static StreamDefinition BackupUserSettings{ Type::UserFile, "settings.json.backup"sv }; - }; - // Gets a stream containing the named setting's value, if present. - // If the setting does not exist, returns an empty value. - std::unique_ptr GetSettingStream(const StreamDefinition& def); + // Gets a Stream for the StreamDefinition. + // If the setting stream does not exist, returns an empty value (see operator bool). + // If the stream is synchronized, attempts to Set the value can fail due to another writer + // having changed the underlying stream. + Stream(const StreamDefinition& streamDefinition, bool synchronize = true); + + const StreamDefinition& Definition() const { return m_streamDefinition; } - // Sets the named setting to the given value. - void SetSetting(const StreamDefinition& def, std::string_view value); + // Gets the actual stream if present; throws if not. + std::unique_ptr Get(); - // Deletes the given setting. - void RemoveSetting(const StreamDefinition& def); + // Sets the stream to the given value. + // Returns true if successful; false if the underlying stream has changed. + bool Set(std::string_view value); - // Gets the path to the given stream definition. - std::filesystem::path GetPathTo(const StreamDefinition& def); + // Deletes the setting stream. + void Remove(); + + // Gets the path to the stream. + std::filesystem::path GetPath() const; + + private: + const StreamDefinition& m_streamDefinition; + bool m_synchronize; + }; } diff --git a/src/AppInstallerCommonCore/UserSettings.cpp b/src/AppInstallerCommonCore/UserSettings.cpp index 8cac00105e..0913e994df 100644 --- a/src/AppInstallerCommonCore/UserSettings.cpp +++ b/src/AppInstallerCommonCore/UserSettings.cpp @@ -71,7 +71,7 @@ namespace AppInstaller::Settings std::optional ParseFile(const StreamDefinition& setting, std::vector& warnings) { - auto stream = GetSettingStream(setting); + auto stream = Stream{ setting, false }.Get(); if (stream) { Json::Value root; @@ -334,10 +334,10 @@ namespace AppInstaller::Settings return; } - auto settingsJson = ParseFile(Streams::PrimaryUserSettings, m_warnings); + auto settingsJson = ParseFile(Stream::PrimaryUserSettings, m_warnings); if (settingsJson.has_value()) { - AICLI_LOG(Core, Info, << "Settings loaded from " << Streams::PrimaryUserSettings.Path); + AICLI_LOG(Core, Info, << "Settings loaded from " << Stream::PrimaryUserSettings.Path); m_type = UserSettingsType::Standard; settingsRoot = settingsJson.value(); } @@ -345,10 +345,10 @@ namespace AppInstaller::Settings // Settings didn't parse or doesn't exist, try with backup. if (settingsRoot.isNull()) { - auto settingsBackupJson = ParseFile(Streams::BackupUserSettings, m_warnings); + auto settingsBackupJson = ParseFile(Stream::BackupUserSettings, m_warnings); if (settingsBackupJson.has_value()) { - AICLI_LOG(Core, Info, << "Settings loaded from " << Streams::BackupUserSettings.Path); + AICLI_LOG(Core, Info, << "Settings loaded from " << Stream::BackupUserSettings.Path); m_warnings.emplace_back(StringResource::String::SettingsWarningLoadedBackupSettings); m_type = UserSettingsType::Backup; settingsRoot = settingsBackupJson.value(); @@ -371,10 +371,12 @@ namespace AppInstaller::Settings if (userSettingType == UserSettingsType::Default) { + Stream primarySettings{ Stream::PrimaryUserSettings }; + // Create settings file if it doesn't exist. - if (!std::filesystem::exists(UserSettings::SettingsFilePath())) + if (!std::filesystem::exists(primarySettings.GetPath())) { - SetSetting(Streams::PrimaryUserSettings, s_SettingEmpty); + primarySettings.Set(s_SettingEmpty); AICLI_LOG(Core, Info, << "Created new settings file"); } } @@ -382,7 +384,7 @@ namespace AppInstaller::Settings { // Settings file was loaded correctly, create backup. auto from = SettingsFilePath(); - auto to = GetPathTo(Streams::BackupUserSettings); + auto to = Stream{ Stream::BackupUserSettings }.GetPath(); std::filesystem::copy_file(from, to, std::filesystem::copy_options::overwrite_existing); AICLI_LOG(Core, Info, << "Copied settings to backup file"); } @@ -390,6 +392,6 @@ namespace AppInstaller::Settings std::filesystem::path UserSettings::SettingsFilePath() { - return GetPathTo(Streams::PrimaryUserSettings); + return Stream{ Stream::PrimaryUserSettings }.GetPath(); } } diff --git a/src/AppInstallerRepositoryCore/AppInstallerRepositoryCore.vcxproj b/src/AppInstallerRepositoryCore/AppInstallerRepositoryCore.vcxproj index a40933659a..8bc9ddf1b5 100644 --- a/src/AppInstallerRepositoryCore/AppInstallerRepositoryCore.vcxproj +++ b/src/AppInstallerRepositoryCore/AppInstallerRepositoryCore.vcxproj @@ -274,6 +274,8 @@ + + @@ -326,6 +328,8 @@ + + diff --git a/src/AppInstallerRepositoryCore/AppInstallerRepositoryCore.vcxproj.filters b/src/AppInstallerRepositoryCore/AppInstallerRepositoryCore.vcxproj.filters index 11971b5953..677c758037 100644 --- a/src/AppInstallerRepositoryCore/AppInstallerRepositoryCore.vcxproj.filters +++ b/src/AppInstallerRepositoryCore/AppInstallerRepositoryCore.vcxproj.filters @@ -216,6 +216,12 @@ Microsoft\Schema\1_3 + + Header Files + + + Header Files + @@ -329,6 +335,12 @@ Microsoft\Schema\1_3 + + Source Files + + + Source Files + diff --git a/src/AppInstallerRepositoryCore/RepositorySource.cpp b/src/AppInstallerRepositoryCore/RepositorySource.cpp index c6abea4e11..b2ab47251f 100644 --- a/src/AppInstallerRepositoryCore/RepositorySource.cpp +++ b/src/AppInstallerRepositoryCore/RepositorySource.cpp @@ -2,13 +2,13 @@ // Licensed under the MIT License. #include "pch.h" #include "Public/AppInstallerRepositorySource.h" - +#include "SourceList.h" +#include "SourcePolicy.h" #include "CompositeSource.h" #include "SourceFactory.h" #include "Microsoft/PredefinedInstalledSourceFactory.h" #include "Microsoft/PreIndexedPackageSourceFactory.h" #include "Rest/RestSourceFactory.h" - #include namespace AppInstaller::Repository @@ -16,201 +16,9 @@ namespace AppInstaller::Repository using namespace Settings; using namespace std::chrono_literals; - using namespace std::string_view_literals; - - constexpr std::string_view s_SourcesYaml_Sources = "Sources"sv; - constexpr std::string_view s_SourcesYaml_Source_Name = "Name"sv; - constexpr std::string_view s_SourcesYaml_Source_Type = "Type"sv; - constexpr std::string_view s_SourcesYaml_Source_Arg = "Arg"sv; - constexpr std::string_view s_SourcesYaml_Source_Data = "Data"sv; - constexpr std::string_view s_SourcesYaml_Source_Identifier = "Identifier"sv; - constexpr std::string_view s_SourcesYaml_Source_IsTombstone = "IsTombstone"sv; - - constexpr std::string_view s_MetadataYaml_Sources = "Sources"sv; - constexpr std::string_view s_MetadataYaml_Source_Name = "Name"sv; - constexpr std::string_view s_MetadataYaml_Source_LastUpdate = "LastUpdate"sv; - - constexpr std::string_view s_Source_WingetCommunityDefault_Name = "winget"sv; - constexpr std::string_view s_Source_WingetCommunityDefault_Arg = "https://winget.azureedge.net/cache"sv; - constexpr std::string_view s_Source_WingetCommunityDefault_Data = "Microsoft.Winget.Source_8wekyb3d8bbwe"sv; - constexpr std::string_view s_Source_WingetCommunityDefault_Identifier = "Microsoft.Winget.Source_8wekyb3d8bbwe"sv; - - constexpr std::string_view s_Source_WingetMSStoreDefault_Name = "msstore"sv; - constexpr std::string_view s_Source_WingetMSStoreDefault_Arg = "https://winget.azureedge.net/msstore"sv; - constexpr std::string_view s_Source_WingetMSStoreDefault_Data = "Microsoft.Winget.MSStore.Source_8wekyb3d8bbwe"sv; - constexpr std::string_view s_Source_WingetMSStoreDefault_Identifier = "Microsoft.Winget.MSStore.Source_8wekyb3d8bbwe"sv; namespace { - // SourceDetails with additional data used by this file. - struct SourceDetailsInternal : public SourceDetails - { - // If true, this is a tombstone, marking the deletion of a source at a lower priority origin. - bool IsTombstone = false; - }; - - // Checks whether a default source is enabled with the current settings. - // onlyExplicit determines whether we consider the not-configured state to be enabled or not. - bool IsDefaultSourceEnabled(std::string_view sourceToLog, ExperimentalFeature::Feature feature, bool onlyExplicit, TogglePolicy::Policy policy) - { - if (!ExperimentalFeature::IsEnabled(feature)) - { - // No need to log here - return false; - } - - if (onlyExplicit) - { - // No need to log here - return GroupPolicies().GetState(policy) == PolicyState::Enabled; - } - - if (!GroupPolicies().IsEnabled(policy)) - { - AICLI_LOG(Repo, Info, << "The default source " << sourceToLog << " is disabled due to Group Policy"); - return false; - } - - return true; - } - - bool IsWingetCommunityDefaultSourceEnabled(bool onlyExplicit = false) - { - return IsDefaultSourceEnabled(s_Source_WingetCommunityDefault_Name, ExperimentalFeature::Feature::None, onlyExplicit, TogglePolicy::Policy::DefaultSource); - } - - bool IsWingetMSStoreDefaultSourceEnabled(bool onlyExplicit = false) - { - return IsDefaultSourceEnabled(s_Source_WingetMSStoreDefault_Name, ExperimentalFeature::Feature::ExperimentalMSStore, onlyExplicit, TogglePolicy::Policy::MSStoreSource); - } - - template - std::optional FindSourceInPolicy(std::string_view name, std::string_view type, std::string_view arg) - { - auto sourcesOpt = GroupPolicies().GetValueRef

(); - if (!sourcesOpt.has_value()) - { - return std::nullopt; - } - - const auto& sources = sourcesOpt->get(); - auto source = std::find_if( - sources.begin(), - sources.end(), - [&](const SourceFromPolicy& policySource) - { - return Utility::ICUCaseInsensitiveEquals(name, policySource.Name) && Utility::ICUCaseInsensitiveEquals(type, policySource.Type) && arg == policySource.Arg; - }); - - if (source == sources.end()) - { - return std::nullopt; - } - - return *source; - } - - template - bool IsSourceInPolicy(std::string_view name, std::string_view type, std::string_view arg) - { - return FindSourceInPolicy

(name, type, arg).has_value(); - } - - // Checks whether the Group Policy allows this user source. - // If it does it returns None, otherwise it returns which policy is blocking it. - // Note that this applies to user sources that are being added as well as user sources - // that already existed when the Group Policy came into effect. - TogglePolicy::Policy GetPolicyBlockingUserSource(std::string_view name, std::string_view type, std::string_view arg, bool isTombstone) - { - // Reasons for not allowing: - // 1. The source is a tombstone for default source that is explicitly enabled - // 2. The source is a default source that is disabled - // 3. The source has the same name as a default source that is explicitly enabled (to prevent shadowing) - // 4. Allowed sources are disabled, blocking all user sources - // 5. There is an explicit list of allowed sources and this source is not in it - // - // We don't need to check sources added by policy as those have higher priority. - // - // Use the name and arg to match sources as we don't have the identifier before adding. - - // Case 1: - // The source is a tombstone and we need the policy to be explicitly enabled. - if (isTombstone) - { - if (name == s_Source_WingetCommunityDefault_Name && IsWingetCommunityDefaultSourceEnabled(true)) - { - return TogglePolicy::Policy::DefaultSource; - } - - if (name == s_Source_WingetMSStoreDefault_Name && IsWingetMSStoreDefaultSourceEnabled(true)) - { - return TogglePolicy::Policy::MSStoreSource; - } - - // Any other tombstone is allowed - return TogglePolicy::Policy::None; - } - - // Case 2: - // - The source is not a tombstone and we don't need the policy to be explicitly enabled. - // - Check only against the source argument and type as the user source may have a different name. - // - Do a case insensitive check as the domain portion of the URL is case insensitive, - // and we don't need case sensitivity for the rest as we control the domain. - if (Utility::CaseInsensitiveEquals(arg, s_Source_WingetCommunityDefault_Arg) && - Utility::CaseInsensitiveEquals(type, Microsoft::PreIndexedPackageSourceFactory::Type())) - { - return IsWingetCommunityDefaultSourceEnabled(false) ? TogglePolicy::Policy::None : TogglePolicy::Policy::DefaultSource; - } - - if (Utility::CaseInsensitiveEquals(arg, s_Source_WingetMSStoreDefault_Arg) && - Utility::CaseInsensitiveEquals(type, Microsoft::PreIndexedPackageSourceFactory::Type())) - { - return IsWingetMSStoreDefaultSourceEnabled(false) ? TogglePolicy::Policy::None : TogglePolicy::Policy::MSStoreSource; - } - - // Case 3: - // If the source has the same name as a default source, it is shadowing with a different argument - // (as it didn't match above). We only care if Group Policy requires the default source. - if (name == s_Source_WingetCommunityDefault_Name && IsWingetCommunityDefaultSourceEnabled(true)) - { - AICLI_LOG(Repo, Warning, << "User source is not allowed as it shadows the default source. Name [" << name << "]. Arg [" << arg << "] Type [" << type << ']'); - return TogglePolicy::Policy::DefaultSource; - } - - if (name == s_Source_WingetMSStoreDefault_Name && IsWingetMSStoreDefaultSourceEnabled(true)) - { - AICLI_LOG(Repo, Warning, << "User source is not allowed as it shadows the default MS Store source. Name [" << name << "]. Arg [" << arg << "] Type [" << type << ']'); - return TogglePolicy::Policy::MSStoreSource; - } - - // Case 4: - // The guard in the source add command should already block adding. - // This check drops existing user sources. - auto allowedSourcesPolicy = GroupPolicies().GetState(TogglePolicy::Policy::AllowedSources); - if (allowedSourcesPolicy == PolicyState::Disabled) - { - AICLI_LOG(Repo, Warning, << "User sources are disabled by Group Policy"); - return TogglePolicy::Policy::AllowedSources; - } - - // Case 5: - if (allowedSourcesPolicy == PolicyState::Enabled) - { - if (!IsSourceInPolicy(name, type, arg)) - { - AICLI_LOG(Repo, Warning, << "Source is not in the Group Policy allowed list. Name [" << name << "]. Arg [" << arg << "] Type [" << type << ']'); - return TogglePolicy::Policy::AllowedSources; - } - } - - return TogglePolicy::Policy::None; - } - - bool IsUserSourceAllowedByPolicy(std::string_view name, std::string_view type, std::string_view arg, bool isTombstone) - { - return GetPolicyBlockingUserSource(name, type, arg, isTombstone) == TogglePolicy::Policy::None; - } - void EnsureSourceIsRemovable(const SourceDetailsInternal& source) { // Block removing sources added by Group Policy @@ -224,332 +32,19 @@ namespace AppInstaller::Repository if (source.Origin == SourceOrigin::Default) { if (GroupPolicies().GetState(TogglePolicy::Policy::DefaultSource) == PolicyState::Enabled && - source.Identifier == s_Source_WingetCommunityDefault_Identifier) + source.Identifier == WingetCommunityDefaultSourceId()) { throw GroupPolicyException(TogglePolicy::Policy::DefaultSource); } if (GroupPolicies().GetState(TogglePolicy::Policy::MSStoreSource) == PolicyState::Enabled && - source.Identifier == s_Source_WingetMSStoreDefault_Identifier) + source.Identifier == WingetMSStoreDefaultSourceId()) { throw GroupPolicyException(TogglePolicy::Policy::MSStoreSource); } } } - // Attempts to read a single scalar value from the node. - template - bool TryReadScalar(std::string_view settingName, const std::string& settingValue, const YAML::Node& sourceNode, std::string_view name, Value& value, bool required = true) - { - YAML::Node valueNode = sourceNode[std::string{ name }]; - - if (!valueNode || !valueNode.IsScalar()) - { - if (required) - { - AICLI_LOG(Repo, Error, << "Setting '" << settingName << "' did not contain the expected format (" << name << " is invalid within a source):\n" << settingValue); - } - return false; - } - - value = valueNode.as(); - return true; - } - - // Attempts to read the source details from the given stream. - // Results are all or nothing; if any failures occur, no details are returned. - bool TryReadSourceDetails( - std::string_view settingName, - std::istream& stream, - std::string_view rootName, - std::function parse, - std::vector& sourceDetails) - { - std::vector result; - std::string settingValue = Utility::ReadEntireStream(stream); - - YAML::Node document; - try - { - document = YAML::Load(settingValue); - } - catch (const std::exception& e) - { - AICLI_LOG(YAML, Error, << "Setting '" << settingName << "' contained invalid YAML (" << e.what() << "):\n" << settingValue); - return false; - } - - try - { - YAML::Node sources = document[rootName]; - if (!sources) - { - AICLI_LOG(Repo, Error, << "Setting '" << settingName << "' did not contain the expected format (missing " << rootName << "):\n" << settingValue); - return false; - } - - if (sources.IsNull()) - { - // An empty sources is an acceptable thing. - return true; - } - - if (!sources.IsSequence()) - { - AICLI_LOG(Repo, Error, << "Setting '" << settingName << "' did not contain the expected format (" << rootName << " was not a sequence):\n" << settingValue); - return false; - } - - for (const auto& source : sources.Sequence()) - { - SourceDetailsInternal details; - if (!parse(details, settingValue, source)) - { - return false; - } - - result.emplace_back(std::move(details)); - } - } - catch (const std::exception& e) - { - AICLI_LOG(YAML, Error, << "Setting '" << settingName << "' contained unexpected YAML (" << e.what() << "):\n" << settingValue); - return false; - } - - sourceDetails = std::move(result); - return true; - } - - // Gets the source details from a particular setting, or an empty optional if no setting exists. - std::optional> TryGetSourcesFromSetting( - const Settings::StreamDefinition& setting, - std::string_view rootName, - std::function parse) - { - auto sourcesStream = Settings::GetSettingStream(setting); - if (!sourcesStream) - { - // Note that this case is different than the one in which all sources have been removed. - return {}; - } - else - { - std::vector result; - THROW_HR_IF(APPINSTALLER_CLI_ERROR_SOURCES_INVALID, !TryReadSourceDetails(setting.Path, *sourcesStream, rootName, parse, result)); - return result; - } - } - - // Gets the source details from a particular setting. - std::vector GetSourcesFromSetting( - const Settings::StreamDefinition& setting, - std::string_view rootName, - std::function parse) - { - return TryGetSourcesFromSetting(setting, rootName, parse).value_or(std::vector{}); - } - - // Gets the metadata. - std::vector GetMetadata() - { - return GetSourcesFromSetting( - Settings::Streams::SourcesMetadata, - s_MetadataYaml_Sources, - [&](SourceDetailsInternal& details, const std::string& settingValue, const YAML::Node& source) - { - std::string_view name = Settings::Streams::SourcesMetadata.Path; - if (!TryReadScalar(name, settingValue, source, s_MetadataYaml_Source_Name, details.Name)) { return false; } - int64_t lastUpdateInEpoch{}; - if (!TryReadScalar(name, settingValue, source, s_MetadataYaml_Source_LastUpdate, lastUpdateInEpoch)) { return false; } - details.LastUpdateTime = Utility::ConvertUnixEpochToSystemClock(lastUpdateInEpoch); - return true; - }); - } - - // Checks whether a default source is enabled with the current settings - bool IsDefaultSourceEnabled(std::string_view sourceToLog, ExperimentalFeature::Feature feature, TogglePolicy::Policy policy) - { - if (!ExperimentalFeature::IsEnabled(feature)) - { - // No need to log here - return false; - } - - if (!GroupPolicies().IsEnabled(policy)) - { - AICLI_LOG(Repo, Info, << "The default source " << sourceToLog << " is disabled due to Group Policy"); - return false; - } - - return true; - } - - // Gets the sources from a particular origin. - std::vector GetSourcesByOrigin(SourceOrigin origin) - { - std::vector result; - - switch (origin) - { - case SourceOrigin::Default: - { - if (IsWingetCommunityDefaultSourceEnabled()) - { - SourceDetailsInternal details; - details.Name = s_Source_WingetCommunityDefault_Name; - details.Type = Microsoft::PreIndexedPackageSourceFactory::Type(); - details.Arg = s_Source_WingetCommunityDefault_Arg; - details.Data = s_Source_WingetCommunityDefault_Data; - details.Identifier = s_Source_WingetCommunityDefault_Identifier; - details.TrustLevel = SourceTrustLevel::Trusted | SourceTrustLevel::StoreOrigin; - result.emplace_back(std::move(details)); - } - - if (IsWingetMSStoreDefaultSourceEnabled()) - { - SourceDetailsInternal details; - details.Name = s_Source_WingetMSStoreDefault_Name; - details.Type = Microsoft::PreIndexedPackageSourceFactory::Type(); - details.Arg = s_Source_WingetMSStoreDefault_Arg; - details.Data = s_Source_WingetMSStoreDefault_Data; - details.Identifier = s_Source_WingetMSStoreDefault_Identifier; - details.TrustLevel = SourceTrustLevel::Trusted | SourceTrustLevel::StoreOrigin; - result.emplace_back(std::move(details)); - } - } - break; - case SourceOrigin::User: - { - std::vector userSources = GetSourcesFromSetting( - Settings::Streams::UserSources, - s_SourcesYaml_Sources, - [&](SourceDetailsInternal& details, const std::string& settingValue, const YAML::Node& source) - { - std::string_view name = Settings::Streams::UserSources.Path; - if (!TryReadScalar(name, settingValue, source, s_SourcesYaml_Source_Name, details.Name)) { return false; } - if (!TryReadScalar(name, settingValue, source, s_SourcesYaml_Source_Type, details.Type)) { return false; } - if (!TryReadScalar(name, settingValue, source, s_SourcesYaml_Source_Arg, details.Arg)) { return false; } - if (!TryReadScalar(name, settingValue, source, s_SourcesYaml_Source_Data, details.Data)) { return false; } - if (!TryReadScalar(name, settingValue, source, s_SourcesYaml_Source_IsTombstone, details.IsTombstone)) { return false; } - TryReadScalar(name, settingValue, source, s_SourcesYaml_Source_Identifier, details.Identifier); - return true; - }); - - for (auto& source : userSources) - { - // Check source against list of allowed sources and drop tombstones for required sources - if (!IsUserSourceAllowedByPolicy(source.Name, source.Type, source.Arg, source.IsTombstone)) - { - AICLI_LOG(Repo, Warning, << "User source " << source.Name << " dropped because of group policy"); - continue; - } - - result.emplace_back(std::move(source)); - } - } - break; - case SourceOrigin::GroupPolicy: - { - if (GroupPolicies().GetState(TogglePolicy::Policy::AdditionalSources) == PolicyState::Enabled) - { - auto additionalSourcesOpt = GroupPolicies().GetValueRef(); - if (additionalSourcesOpt.has_value()) - { - const auto& additionalSources = additionalSourcesOpt->get(); - for (const auto& additionalSource : additionalSources) - { - SourceDetailsInternal details; - details.Name = additionalSource.Name; - details.Type = additionalSource.Type; - details.Arg = additionalSource.Arg; - details.Data = additionalSource.Data; - details.Identifier = additionalSource.Identifier; - details.Origin = SourceOrigin::GroupPolicy; - result.emplace_back(std::move(details)); - } - } - } - } - break; - default: - THROW_HR(E_UNEXPECTED); - } - - for (auto& source : result) - { - source.Origin = origin; - } - - return result; - } - - // Sets the sources for a particular setting, from a particular origin. - void SetSourcesToSettingWithFilter(const Settings::StreamDefinition& setting, SourceOrigin origin, const std::vector& sources) - { - YAML::Emitter out; - out << YAML::BeginMap; - out << YAML::Key << s_SourcesYaml_Sources; - out << YAML::BeginSeq; - - for (const auto& details : sources) - { - if (details.Origin == origin) - { - out << YAML::BeginMap; - out << YAML::Key << s_SourcesYaml_Source_Name << YAML::Value << details.Name; - out << YAML::Key << s_SourcesYaml_Source_Type << YAML::Value << details.Type; - out << YAML::Key << s_SourcesYaml_Source_Arg << YAML::Value << details.Arg; - out << YAML::Key << s_SourcesYaml_Source_Data << YAML::Value << details.Data; - out << YAML::Key << s_SourcesYaml_Source_Identifier << YAML::Value << details.Identifier; - out << YAML::Key << s_SourcesYaml_Source_IsTombstone << YAML::Value << details.IsTombstone; - out << YAML::EndMap; - } - } - - out << YAML::EndSeq; - out << YAML::EndMap; - - Settings::SetSetting(setting, out.str()); - } - - // Sets the metadata only (which is not a secure setting and can be set unprivileged) - void SetMetadata(const std::vector& sources) - { - YAML::Emitter out; - out << YAML::BeginMap; - out << YAML::Key << s_MetadataYaml_Sources; - out << YAML::BeginSeq; - - for (const auto& details : sources) - { - out << YAML::BeginMap; - out << YAML::Key << s_MetadataYaml_Source_Name << YAML::Value << details.Name; - out << YAML::Key << s_MetadataYaml_Source_LastUpdate << YAML::Value << Utility::ConvertSystemClockToUnixEpoch(details.LastUpdateTime); - out << YAML::EndMap; - } - - out << YAML::EndSeq; - out << YAML::EndMap; - - Settings::SetSetting(Settings::Streams::SourcesMetadata, out.str()); - } - - // Sets the sources for a given origin. - void SetSourcesByOrigin(SourceOrigin origin, const std::vector& sources) - { - switch (origin) - { - case SourceOrigin::User: - SetSourcesToSettingWithFilter(Settings::Streams::UserSources, SourceOrigin::User, sources); - break; - default: - THROW_HR(E_UNEXPECTED); - } - - SetMetadata(sources); - } - #ifndef AICLI_DISABLE_TEST_HOOKS static std::map()>> s_Sources_TestHook_SourceFactories; #endif @@ -663,157 +158,6 @@ namespace AppInstaller::Repository return false; } - - // Struct containing internal implementation of source list - // This contains all sources including tombstoned sources - struct SourceListInternal - { - SourceListInternal(); - - // Get a list of current sources references which can be used to update the contents in place. - // e.g. update the LastTimeUpdated value of sources. - std::vector> GetCurrentSourceRefs(); - - // Current source means source that's not in tombstone - SourceDetailsInternal* GetCurrentSource(std::string_view name); - - // Source includes ones in tombstone - SourceDetailsInternal* GetSource(std::string_view name); - - // Add/remove a current source - void AddSource(const SourceDetailsInternal& source); - void RemoveSource(const SourceDetailsInternal& source); - - // Save source metadata. Currently only LastTimeUpdated is used. - void SaveMetadata() const; - - private: - std::vector m_sourceList; - - // calls std::find_if and return the iterator. - auto FindSource(std::string_view name, bool includeTombstone = false); - }; - - SourceListInternal::SourceListInternal() - { - for (SourceOrigin origin : { SourceOrigin::GroupPolicy, SourceOrigin::User, SourceOrigin::Default }) - { - auto forOrigin = GetSourcesByOrigin(origin); - - for (auto&& source : forOrigin) - { - auto foundSource = GetSource(source.Name); - if (!foundSource) - { - // Name not already defined, add it - m_sourceList.emplace_back(std::move(source)); - } - else - { - AICLI_LOG(Repo, Info, << "Source named '" << foundSource->Name << "' is already defined at origin " << ToString(foundSource->Origin) << - ". The source from origin " << ToString(origin) << " is dropped."); - } - } - } - - auto metadata = GetMetadata(); - for (const auto& metaSource : metadata) - { - auto source = GetSource(metaSource.Name); - if (source) - { - source->LastUpdateTime = metaSource.LastUpdateTime; - } - } - } - - std::vector> SourceListInternal::GetCurrentSourceRefs() - { - std::vector> result; - - for (auto& s : m_sourceList) - { - if (!s.IsTombstone) - { - result.emplace_back(std::ref(s)); - } - else - { - AICLI_LOG(Repo, Info, << "GetCurrentSourceRefs: Source named '" << s.Name << "' from origin " << ToString(s.Origin) << " is a tombstone and is dropped."); - } - } - - return result; - } - - auto SourceListInternal::FindSource(std::string_view name, bool includeTombstone) - { - return std::find_if(m_sourceList.begin(), m_sourceList.end(), - [name, includeTombstone](const SourceDetailsInternal& sd) - { - return Utility::ICUCaseInsensitiveEquals(sd.Name, name) && - (!sd.IsTombstone || includeTombstone); - }); - } - - SourceDetailsInternal* SourceListInternal::GetCurrentSource(std::string_view name) - { - auto itr = FindSource(name); - return itr == m_sourceList.end() ? nullptr : &(*itr); - } - - SourceDetailsInternal* SourceListInternal::GetSource(std::string_view name) - { - auto itr = FindSource(name, true); - return itr == m_sourceList.end() ? nullptr : &(*itr); - } - - void SourceListInternal::AddSource(const SourceDetailsInternal& details) - { - // Erase the source's tombstone entry if applicable - auto itr = FindSource(details.Name, true); - if (itr != m_sourceList.end()) - { - THROW_HR_IF(HRESULT_FROM_WIN32(ERROR_INVALID_STATE), !itr->IsTombstone); - m_sourceList.erase(itr); - } - - m_sourceList.emplace_back(details); - - SetSourcesByOrigin(SourceOrigin::User, m_sourceList); - } - - void SourceListInternal::RemoveSource(const SourceDetailsInternal& source) - { - switch (source.Origin) - { - case SourceOrigin::Default: - { - SourceDetailsInternal tombstone; - tombstone.Name = source.Name; - tombstone.IsTombstone = true; - tombstone.Origin = SourceOrigin::User; - m_sourceList.emplace_back(std::move(tombstone)); - } - break; - case SourceOrigin::User: - m_sourceList.erase(FindSource(source.Name)); - break; - case SourceOrigin::GroupPolicy: - // This should have already been blocked higher up. - AICLI_LOG(Repo, Error, << "Attempting to remove Group Policy source: " << source.Name); - THROW_HR(E_UNEXPECTED); - default: - THROW_HR(E_UNEXPECTED); - } - - SetSourcesByOrigin(SourceOrigin::User, m_sourceList); - } - - void SourceListInternal::SaveMetadata() const - { - SetMetadata(m_sourceList); - } } std::string_view ToString(SourceOrigin origin) @@ -833,7 +177,7 @@ namespace AppInstaller::Repository std::vector GetSources() { - SourceListInternal sourceList; + SourceList sourceList; std::vector result; for (auto&& source : sourceList.GetCurrentSourceRefs()) @@ -847,7 +191,7 @@ namespace AppInstaller::Repository std::optional GetSource(std::string_view name) { // Check all sources for the given name. - SourceListInternal sourceList; + SourceList sourceList; auto source = sourceList.GetCurrentSource(name); if (!source) @@ -867,7 +211,7 @@ namespace AppInstaller::Repository AICLI_LOG(Repo, Info, << "Adding source: Name[" << name << "], Type[" << type << "], Arg[" << arg << "]"); // Check all sources for the given name. - SourceListInternal sourceList; + SourceList sourceList; auto source = sourceList.GetCurrentSource(name); THROW_HR_IF(APPINSTALLER_CLI_ERROR_SOURCE_NAME_ALREADY_EXISTS, source != nullptr); @@ -900,7 +244,7 @@ namespace AppInstaller::Repository OpenSourceResult OpenSource(std::string_view name, IProgressCallback& progress) { - SourceListInternal sourceList; + SourceList sourceList; auto currentSources = sourceList.GetCurrentSourceRefs(); if (name.empty()) @@ -1031,7 +375,7 @@ namespace AppInstaller::Repository { THROW_HR_IF(E_INVALIDARG, name.empty()); - SourceListInternal sourceList; + SourceList sourceList; auto source = sourceList.GetCurrentSource(name); if (!source) @@ -1057,7 +401,7 @@ namespace AppInstaller::Repository { THROW_HR_IF(E_INVALIDARG, name.empty()); - SourceListInternal sourceList; + SourceList sourceList; auto source = sourceList.GetCurrentSource(name); if (!source) @@ -1085,13 +429,13 @@ namespace AppInstaller::Repository { if (name.empty()) { - Settings::RemoveSetting(Settings::Streams::UserSources); - Settings::RemoveSetting(Settings::Streams::SourcesMetadata); + Settings::Stream{ Settings::Stream::UserSources }.Remove(); + Settings::Stream{ Settings::Stream::SourcesMetadata }.Remove(); return true; } else { - SourceListInternal sourceList; + SourceList sourceList; auto source = sourceList.GetCurrentSource(name); if (!source) diff --git a/src/AppInstallerRepositoryCore/SourceList.cpp b/src/AppInstallerRepositoryCore/SourceList.cpp new file mode 100644 index 0000000000..3fb213364c --- /dev/null +++ b/src/AppInstallerRepositoryCore/SourceList.cpp @@ -0,0 +1,645 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. +#include "pch.h" +#include "SourceList.h" + + +namespace AppInstaller::Repository +{ + using namespace Settings; + + using namespace std::chrono_literals; + using namespace std::string_view_literals; + + constexpr std::string_view s_SourcesYaml_Sources = "Sources"sv; + constexpr std::string_view s_SourcesYaml_Source_Name = "Name"sv; + constexpr std::string_view s_SourcesYaml_Source_Type = "Type"sv; + constexpr std::string_view s_SourcesYaml_Source_Arg = "Arg"sv; + constexpr std::string_view s_SourcesYaml_Source_Data = "Data"sv; + constexpr std::string_view s_SourcesYaml_Source_Identifier = "Identifier"sv; + constexpr std::string_view s_SourcesYaml_Source_IsTombstone = "IsTombstone"sv; + + constexpr std::string_view s_MetadataYaml_Sources = "Sources"sv; + constexpr std::string_view s_MetadataYaml_Source_Name = "Name"sv; + constexpr std::string_view s_MetadataYaml_Source_LastUpdate = "LastUpdate"sv; + + constexpr std::string_view s_Source_WingetCommunityDefault_Name = "winget"sv; + constexpr std::string_view s_Source_WingetCommunityDefault_Arg = "https://winget.azureedge.net/cache"sv; + constexpr std::string_view s_Source_WingetCommunityDefault_Data = "Microsoft.Winget.Source_8wekyb3d8bbwe"sv; + constexpr std::string_view s_Source_WingetCommunityDefault_Identifier = "Microsoft.Winget.Source_8wekyb3d8bbwe"sv; + + constexpr std::string_view s_Source_WingetMSStoreDefault_Name = "msstore"sv; + constexpr std::string_view s_Source_WingetMSStoreDefault_Arg = "https://winget.azureedge.net/msstore"sv; + constexpr std::string_view s_Source_WingetMSStoreDefault_Data = "Microsoft.Winget.MSStore.Source_8wekyb3d8bbwe"sv; + constexpr std::string_view s_Source_WingetMSStoreDefault_Identifier = "Microsoft.Winget.MSStore.Source_8wekyb3d8bbwe"sv; + + namespace + { + // Checks whether a default source is enabled with the current settings. + // onlyExplicit determines whether we consider the not-configured state to be enabled or not. + bool IsDefaultSourceEnabled(std::string_view sourceToLog, ExperimentalFeature::Feature feature, bool onlyExplicit, TogglePolicy::Policy policy) + { + if (!ExperimentalFeature::IsEnabled(feature)) + { + // No need to log here + return false; + } + + if (onlyExplicit) + { + // No need to log here + return GroupPolicies().GetState(policy) == PolicyState::Enabled; + } + + if (!GroupPolicies().IsEnabled(policy)) + { + AICLI_LOG(Repo, Info, << "The default source " << sourceToLog << " is disabled due to Group Policy"); + return false; + } + + return true; + } + + bool IsWingetCommunityDefaultSourceEnabled(bool onlyExplicit = false) + { + return IsDefaultSourceEnabled(s_Source_WingetCommunityDefault_Name, ExperimentalFeature::Feature::None, onlyExplicit, TogglePolicy::Policy::DefaultSource); + } + + bool IsWingetMSStoreDefaultSourceEnabled(bool onlyExplicit = false) + { + return IsDefaultSourceEnabled(s_Source_WingetMSStoreDefault_Name, ExperimentalFeature::Feature::ExperimentalMSStore, onlyExplicit, TogglePolicy::Policy::MSStoreSource); + } + + template + std::optional FindSourceInPolicy(std::string_view name, std::string_view type, std::string_view arg) + { + auto sourcesOpt = GroupPolicies().GetValueRef

(); + if (!sourcesOpt.has_value()) + { + return std::nullopt; + } + + const auto& sources = sourcesOpt->get(); + auto source = std::find_if( + sources.begin(), + sources.end(), + [&](const SourceFromPolicy& policySource) + { + return Utility::ICUCaseInsensitiveEquals(name, policySource.Name) && Utility::ICUCaseInsensitiveEquals(type, policySource.Type) && arg == policySource.Arg; + }); + + if (source == sources.end()) + { + return std::nullopt; + } + + return *source; + } + + template + bool IsSourceInPolicy(std::string_view name, std::string_view type, std::string_view arg) + { + return FindSourceInPolicy

(name, type, arg).has_value(); + } + + // Checks whether the Group Policy allows this user source. + // If it does it returns None, otherwise it returns which policy is blocking it. + // Note that this applies to user sources that are being added as well as user sources + // that already existed when the Group Policy came into effect. + TogglePolicy::Policy GetPolicyBlockingUserSource(std::string_view name, std::string_view type, std::string_view arg, bool isTombstone) + { + // Reasons for not allowing: + // 1. The source is a tombstone for default source that is explicitly enabled + // 2. The source is a default source that is disabled + // 3. The source has the same name as a default source that is explicitly enabled (to prevent shadowing) + // 4. Allowed sources are disabled, blocking all user sources + // 5. There is an explicit list of allowed sources and this source is not in it + // + // We don't need to check sources added by policy as those have higher priority. + // + // Use the name and arg to match sources as we don't have the identifier before adding. + + // Case 1: + // The source is a tombstone and we need the policy to be explicitly enabled. + if (isTombstone) + { + if (name == s_Source_WingetCommunityDefault_Name && IsWingetCommunityDefaultSourceEnabled(true)) + { + return TogglePolicy::Policy::DefaultSource; + } + + if (name == s_Source_WingetMSStoreDefault_Name && IsWingetMSStoreDefaultSourceEnabled(true)) + { + return TogglePolicy::Policy::MSStoreSource; + } + + // Any other tombstone is allowed + return TogglePolicy::Policy::None; + } + + // Case 2: + // - The source is not a tombstone and we don't need the policy to be explicitly enabled. + // - Check only against the source argument and type as the user source may have a different name. + // - Do a case insensitive check as the domain portion of the URL is case insensitive, + // and we don't need case sensitivity for the rest as we control the domain. + if (Utility::CaseInsensitiveEquals(arg, s_Source_WingetCommunityDefault_Arg) && + Utility::CaseInsensitiveEquals(type, Microsoft::PreIndexedPackageSourceFactory::Type())) + { + return IsWingetCommunityDefaultSourceEnabled(false) ? TogglePolicy::Policy::None : TogglePolicy::Policy::DefaultSource; + } + + if (Utility::CaseInsensitiveEquals(arg, s_Source_WingetMSStoreDefault_Arg) && + Utility::CaseInsensitiveEquals(type, Microsoft::PreIndexedPackageSourceFactory::Type())) + { + return IsWingetMSStoreDefaultSourceEnabled(false) ? TogglePolicy::Policy::None : TogglePolicy::Policy::MSStoreSource; + } + + // Case 3: + // If the source has the same name as a default source, it is shadowing with a different argument + // (as it didn't match above). We only care if Group Policy requires the default source. + if (name == s_Source_WingetCommunityDefault_Name && IsWingetCommunityDefaultSourceEnabled(true)) + { + AICLI_LOG(Repo, Warning, << "User source is not allowed as it shadows the default source. Name [" << name << "]. Arg [" << arg << "] Type [" << type << ']'); + return TogglePolicy::Policy::DefaultSource; + } + + if (name == s_Source_WingetMSStoreDefault_Name && IsWingetMSStoreDefaultSourceEnabled(true)) + { + AICLI_LOG(Repo, Warning, << "User source is not allowed as it shadows the default MS Store source. Name [" << name << "]. Arg [" << arg << "] Type [" << type << ']'); + return TogglePolicy::Policy::MSStoreSource; + } + + // Case 4: + // The guard in the source add command should already block adding. + // This check drops existing user sources. + auto allowedSourcesPolicy = GroupPolicies().GetState(TogglePolicy::Policy::AllowedSources); + if (allowedSourcesPolicy == PolicyState::Disabled) + { + AICLI_LOG(Repo, Warning, << "User sources are disabled by Group Policy"); + return TogglePolicy::Policy::AllowedSources; + } + + // Case 5: + if (allowedSourcesPolicy == PolicyState::Enabled) + { + if (!IsSourceInPolicy(name, type, arg)) + { + AICLI_LOG(Repo, Warning, << "Source is not in the Group Policy allowed list. Name [" << name << "]. Arg [" << arg << "] Type [" << type << ']'); + return TogglePolicy::Policy::AllowedSources; + } + } + + return TogglePolicy::Policy::None; + } + + bool IsUserSourceAllowedByPolicy(std::string_view name, std::string_view type, std::string_view arg, bool isTombstone) + { + return GetPolicyBlockingUserSource(name, type, arg, isTombstone) == TogglePolicy::Policy::None; + } + + void EnsureSourceIsRemovable(const SourceDetailsInternal& source) + { + // Block removing sources added by Group Policy + if (source.Origin == SourceOrigin::GroupPolicy) + { + AICLI_LOG(Repo, Error, << "Cannot remove source added by Group Policy"); + throw GroupPolicyException(TogglePolicy::Policy::AdditionalSources); + } + + // Block removing default sources required by Group Policy. + if (source.Origin == SourceOrigin::Default) + { + if (GroupPolicies().GetState(TogglePolicy::Policy::DefaultSource) == PolicyState::Enabled && + source.Identifier == s_Source_WingetCommunityDefault_Identifier) + { + throw GroupPolicyException(TogglePolicy::Policy::DefaultSource); + } + + if (GroupPolicies().GetState(TogglePolicy::Policy::MSStoreSource) == PolicyState::Enabled && + source.Identifier == s_Source_WingetMSStoreDefault_Identifier) + { + throw GroupPolicyException(TogglePolicy::Policy::MSStoreSource); + } + } + } + + // Attempts to read a single scalar value from the node. + template + bool TryReadScalar(std::string_view settingName, const std::string& settingValue, const YAML::Node& sourceNode, std::string_view name, Value& value, bool required = true) + { + YAML::Node valueNode = sourceNode[std::string{ name }]; + + if (!valueNode || !valueNode.IsScalar()) + { + if (required) + { + AICLI_LOG(Repo, Error, << "Setting '" << settingName << "' did not contain the expected format (" << name << " is invalid within a source):\n" << settingValue); + } + return false; + } + + value = valueNode.as(); + return true; + } + + // Attempts to read the source details from the given stream. + // Results are all or nothing; if any failures occur, no details are returned. + bool TryReadSourceDetails( + std::string_view settingName, + std::istream& stream, + std::string_view rootName, + std::function parse, + std::vector& sourceDetails) + { + std::vector result; + std::string settingValue = Utility::ReadEntireStream(stream); + + YAML::Node document; + try + { + document = YAML::Load(settingValue); + } + catch (const std::exception& e) + { + AICLI_LOG(YAML, Error, << "Setting '" << settingName << "' contained invalid YAML (" << e.what() << "):\n" << settingValue); + return false; + } + + try + { + YAML::Node sources = document[rootName]; + if (!sources) + { + AICLI_LOG(Repo, Error, << "Setting '" << settingName << "' did not contain the expected format (missing " << rootName << "):\n" << settingValue); + return false; + } + + if (sources.IsNull()) + { + // An empty sources is an acceptable thing. + return true; + } + + if (!sources.IsSequence()) + { + AICLI_LOG(Repo, Error, << "Setting '" << settingName << "' did not contain the expected format (" << rootName << " was not a sequence):\n" << settingValue); + return false; + } + + for (const auto& source : sources.Sequence()) + { + SourceDetailsInternal details; + if (!parse(details, settingValue, source)) + { + return false; + } + + result.emplace_back(std::move(details)); + } + } + catch (const std::exception& e) + { + AICLI_LOG(YAML, Error, << "Setting '" << settingName << "' contained unexpected YAML (" << e.what() << "):\n" << settingValue); + return false; + } + + sourceDetails = std::move(result); + return true; + } + + // Gets the source details from a particular setting, or an empty optional if no setting exists. + std::optional> TryGetSourcesFromSetting( + Settings::Stream& stream, + std::string_view rootName, + std::function parse) + { + auto sourcesStream = stream.Get(); + if (!sourcesStream) + { + // Note that this case is different than the one in which all sources have been removed. + return {}; + } + else + { + std::vector result; + THROW_HR_IF(APPINSTALLER_CLI_ERROR_SOURCES_INVALID, !TryReadSourceDetails(stream.Definition().Path, *sourcesStream, rootName, parse, result)); + return result; + } + } + + // Gets the source details from a particular setting. + std::vector GetSourcesFromSetting( + Settings::Stream& stream, + std::string_view rootName, + std::function parse) + { + return TryGetSourcesFromSetting(stream, rootName, parse).value_or(std::vector{}); + } + + // Gets the metadata. + std::vector GetMetadata(Settings::Stream& stream) + { + return GetSourcesFromSetting( + stream, + s_MetadataYaml_Sources, + [&](SourceDetailsInternal& details, const std::string& settingValue, const YAML::Node& source) + { + std::string_view name = Settings::Stream::SourcesMetadata.Path; + if (!TryReadScalar(name, settingValue, source, s_MetadataYaml_Source_Name, details.Name)) { return false; } + int64_t lastUpdateInEpoch{}; + if (!TryReadScalar(name, settingValue, source, s_MetadataYaml_Source_LastUpdate, lastUpdateInEpoch)) { return false; } + details.LastUpdateTime = Utility::ConvertUnixEpochToSystemClock(lastUpdateInEpoch); + return true; + }); + } + + // Gets the sources from a particular origin. + std::vector GetSourcesByOrigin(SourceOrigin origin, Settings::Stream& userSettingStream) + { + std::vector result; + + switch (origin) + { + case SourceOrigin::Default: + { + if (IsWingetCommunityDefaultSourceEnabled()) + { + SourceDetailsInternal details; + details.Name = s_Source_WingetCommunityDefault_Name; + details.Type = Microsoft::PreIndexedPackageSourceFactory::Type(); + details.Arg = s_Source_WingetCommunityDefault_Arg; + details.Data = s_Source_WingetCommunityDefault_Data; + details.Identifier = s_Source_WingetCommunityDefault_Identifier; + details.TrustLevel = SourceTrustLevel::Trusted | SourceTrustLevel::StoreOrigin; + result.emplace_back(std::move(details)); + } + + if (IsWingetMSStoreDefaultSourceEnabled()) + { + SourceDetailsInternal details; + details.Name = s_Source_WingetMSStoreDefault_Name; + details.Type = Microsoft::PreIndexedPackageSourceFactory::Type(); + details.Arg = s_Source_WingetMSStoreDefault_Arg; + details.Data = s_Source_WingetMSStoreDefault_Data; + details.Identifier = s_Source_WingetMSStoreDefault_Identifier; + details.TrustLevel = SourceTrustLevel::Trusted | SourceTrustLevel::StoreOrigin; + result.emplace_back(std::move(details)); + } + } + break; + case SourceOrigin::User: + { + std::vector userSources = GetSourcesFromSetting( + userSettingStream, + s_SourcesYaml_Sources, + [&](SourceDetailsInternal& details, const std::string& settingValue, const YAML::Node& source) + { + std::string_view name = Settings::Stream::UserSources.Path; + if (!TryReadScalar(name, settingValue, source, s_SourcesYaml_Source_Name, details.Name)) { return false; } + if (!TryReadScalar(name, settingValue, source, s_SourcesYaml_Source_Type, details.Type)) { return false; } + if (!TryReadScalar(name, settingValue, source, s_SourcesYaml_Source_Arg, details.Arg)) { return false; } + if (!TryReadScalar(name, settingValue, source, s_SourcesYaml_Source_Data, details.Data)) { return false; } + if (!TryReadScalar(name, settingValue, source, s_SourcesYaml_Source_IsTombstone, details.IsTombstone)) { return false; } + TryReadScalar(name, settingValue, source, s_SourcesYaml_Source_Identifier, details.Identifier); + return true; + }); + + for (auto& source : userSources) + { + // Check source against list of allowed sources and drop tombstones for required sources + if (!IsUserSourceAllowedByPolicy(source.Name, source.Type, source.Arg, source.IsTombstone)) + { + AICLI_LOG(Repo, Warning, << "User source " << source.Name << " dropped because of group policy"); + continue; + } + + result.emplace_back(std::move(source)); + } + } + break; + case SourceOrigin::GroupPolicy: + { + if (GroupPolicies().GetState(TogglePolicy::Policy::AdditionalSources) == PolicyState::Enabled) + { + auto additionalSourcesOpt = GroupPolicies().GetValueRef(); + if (additionalSourcesOpt.has_value()) + { + const auto& additionalSources = additionalSourcesOpt->get(); + for (const auto& additionalSource : additionalSources) + { + SourceDetailsInternal details; + details.Name = additionalSource.Name; + details.Type = additionalSource.Type; + details.Arg = additionalSource.Arg; + details.Data = additionalSource.Data; + details.Identifier = additionalSource.Identifier; + details.Origin = SourceOrigin::GroupPolicy; + result.emplace_back(std::move(details)); + } + } + } + } + break; + default: + THROW_HR(E_UNEXPECTED); + } + + for (auto& source : result) + { + source.Origin = origin; + } + + return result; + } + + // Sets the sources for a particular setting, from a particular origin. + bool SetSourcesToSettingWithFilter(Settings::Stream& stream, SourceOrigin origin, const std::vector& sources) + { + YAML::Emitter out; + out << YAML::BeginMap; + out << YAML::Key << s_SourcesYaml_Sources; + out << YAML::BeginSeq; + + for (const auto& details : sources) + { + if (details.Origin == origin) + { + out << YAML::BeginMap; + out << YAML::Key << s_SourcesYaml_Source_Name << YAML::Value << details.Name; + out << YAML::Key << s_SourcesYaml_Source_Type << YAML::Value << details.Type; + out << YAML::Key << s_SourcesYaml_Source_Arg << YAML::Value << details.Arg; + out << YAML::Key << s_SourcesYaml_Source_Data << YAML::Value << details.Data; + out << YAML::Key << s_SourcesYaml_Source_Identifier << YAML::Value << details.Identifier; + out << YAML::Key << s_SourcesYaml_Source_IsTombstone << YAML::Value << details.IsTombstone; + out << YAML::EndMap; + } + } + + out << YAML::EndSeq; + out << YAML::EndMap; + + return stream.Set(out.str()); + } + + // Sets the metadata only (which is not a secure setting and can be set unprivileged) + void SetMetadata(const std::vector& sources) + { + YAML::Emitter out; + out << YAML::BeginMap; + out << YAML::Key << s_MetadataYaml_Sources; + out << YAML::BeginSeq; + + for (const auto& details : sources) + { + out << YAML::BeginMap; + out << YAML::Key << s_MetadataYaml_Source_Name << YAML::Value << details.Name; + out << YAML::Key << s_MetadataYaml_Source_LastUpdate << YAML::Value << Utility::ConvertSystemClockToUnixEpoch(details.LastUpdateTime); + out << YAML::EndMap; + } + + out << YAML::EndSeq; + out << YAML::EndMap; + + Settings::SetSetting(Settings::Stream::SourcesMetadata, out.str()); + } + + // Sets the sources for a given origin. + bool SetSourcesByOrigin(SourceOrigin origin, Settings::Stream& userSettingStream, const std::vector& sources) + { + switch (origin) + { + case SourceOrigin::User: + if (!SetSourcesToSettingWithFilter(userSettingStream, SourceOrigin::User, sources)) + { + return false; + } + break; + default: + THROW_HR(E_UNEXPECTED); + } + + return SetMetadata(sources); + } + } + + SourceList::SourceList() : m_userSourcesStream(Settings::Stream::UserSources), m_metadataStream(Settings::Stream::SourcesMetadata) + { + for (SourceOrigin origin : { SourceOrigin::GroupPolicy, SourceOrigin::User, SourceOrigin::Default }) + { + auto forOrigin = GetSourcesByOrigin(origin, m_userSourcesStream); + + for (auto&& source : forOrigin) + { + auto foundSource = GetSource(source.Name); + if (!foundSource) + { + // Name not already defined, add it + m_sourceList.emplace_back(std::move(source)); + } + else + { + AICLI_LOG(Repo, Info, << "Source named '" << foundSource->Name << "' is already defined at origin " << ToString(foundSource->Origin) << + ". The source from origin " << ToString(origin) << " is dropped."); + } + } + } + + auto metadata = GetMetadata(m_metadataStream); + for (const auto& metaSource : metadata) + { + auto source = GetSource(metaSource.Name); + if (source) + { + source->LastUpdateTime = metaSource.LastUpdateTime; + } + } + } + + std::vector> SourceList::GetCurrentSourceRefs() + { + std::vector> result; + + for (auto& s : m_sourceList) + { + if (!s.IsTombstone) + { + result.emplace_back(std::ref(s)); + } + else + { + AICLI_LOG(Repo, Info, << "GetCurrentSourceRefs: Source named '" << s.Name << "' from origin " << ToString(s.Origin) << " is a tombstone and is dropped."); + } + } + + return result; + } + + auto SourceList::FindSource(std::string_view name, bool includeTombstone) + { + return std::find_if(m_sourceList.begin(), m_sourceList.end(), + [name, includeTombstone](const SourceDetailsInternal& sd) + { + return Utility::ICUCaseInsensitiveEquals(sd.Name, name) && + (!sd.IsTombstone || includeTombstone); + }); + } + + SourceDetailsInternal* SourceList::GetCurrentSource(std::string_view name) + { + auto itr = FindSource(name); + return itr == m_sourceList.end() ? nullptr : &(*itr); + } + + SourceDetailsInternal* SourceList::GetSource(std::string_view name) + { + auto itr = FindSource(name, true); + return itr == m_sourceList.end() ? nullptr : &(*itr); + } + + void SourceList::AddSource(const SourceDetailsInternal& details) + { + // Erase the source's tombstone entry if applicable + auto itr = FindSource(details.Name, true); + if (itr != m_sourceList.end()) + { + THROW_HR_IF(HRESULT_FROM_WIN32(ERROR_INVALID_STATE), !itr->IsTombstone); + m_sourceList.erase(itr); + } + + m_sourceList.emplace_back(details); + + SetSourcesByOrigin(SourceOrigin::User, m_sourceList); + } + + void SourceList::RemoveSource(const SourceDetailsInternal& source) + { + switch (source.Origin) + { + case SourceOrigin::Default: + { + SourceDetailsInternal tombstone; + tombstone.Name = source.Name; + tombstone.IsTombstone = true; + tombstone.Origin = SourceOrigin::User; + m_sourceList.emplace_back(std::move(tombstone)); + } + break; + case SourceOrigin::User: + m_sourceList.erase(FindSource(source.Name)); + break; + case SourceOrigin::GroupPolicy: + // This should have already been blocked higher up. + AICLI_LOG(Repo, Error, << "Attempting to remove Group Policy source: " << source.Name); + THROW_HR(E_UNEXPECTED); + default: + THROW_HR(E_UNEXPECTED); + } + + SetSourcesByOrigin(SourceOrigin::User, m_sourceList); + } + + void SourceList::SaveMetadata() const + { + SetMetadata(m_sourceList); + } +} diff --git a/src/AppInstallerRepositoryCore/SourceList.h b/src/AppInstallerRepositoryCore/SourceList.h new file mode 100644 index 0000000000..7df090c42d --- /dev/null +++ b/src/AppInstallerRepositoryCore/SourceList.h @@ -0,0 +1,57 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. +#pragma once +#include "AppInstallerRepositorySource.h" +#include + + +namespace AppInstaller::Repository +{ + // Built-in values for default sources + std::string_view WingetCommunityDefaultSourceName(); + std::string_view WingetCommunityDefaultSourceArg(); + std::string_view WingetCommunityDefaultSourceId(); + + std::string_view WingetMSStoreDefaultSourceName(); + std::string_view WingetMSStoreDefaultSourceArg(); + std::string_view WingetMSStoreDefaultSourceId(); + + // SourceDetails with additional data used internally. + struct SourceDetailsInternal : public SourceDetails + { + // If true, this is a tombstone, marking the deletion of a source at a lower priority origin. + bool IsTombstone = false; + }; + + // Struct containing internal implementation of the source list. + // This contains all source data; including tombstoned sources. + struct SourceList + { + SourceList(); + + // Get a list of current sources references which can be used to update the contents in place. + // e.g. update the LastTimeUpdated value of sources. + std::vector> GetCurrentSourceRefs(); + + // Current source means source that's not in tombstone + SourceDetailsInternal* GetCurrentSource(std::string_view name); + + // Source includes ones in tombstone + SourceDetailsInternal* GetSource(std::string_view name); + + // Add/remove a current source + void AddSource(const SourceDetailsInternal& source); + void RemoveSource(const SourceDetailsInternal& source); + + // Save source metadata. Currently only LastTimeUpdated is used. + void SaveMetadata() const; + + private: + // calls std::find_if and return the iterator. + auto FindSource(std::string_view name, bool includeTombstone = false); + + std::vector m_sourceList; + Settings::Stream m_userSourcesStream; + Settings::Stream m_metadataStream; + }; +} diff --git a/src/AppInstallerRepositoryCore/SourcePolicy.cpp b/src/AppInstallerRepositoryCore/SourcePolicy.cpp new file mode 100644 index 0000000000..ba3a8ef031 --- /dev/null +++ b/src/AppInstallerRepositoryCore/SourcePolicy.cpp @@ -0,0 +1,169 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. +#include "pch.h" +#include "SourcePolicy.h" +#include "SourceList.h" +#include "Microsoft/PreIndexedPackageSourceFactory.h" +#include + + +namespace AppInstaller::Repository +{ + using namespace Settings; + + namespace + { + // Checks whether a default source is enabled with the current settings. + // onlyExplicit determines whether we consider the not-configured state to be enabled or not. + bool IsDefaultSourceEnabled(std::string_view sourceToLog, ExperimentalFeature::Feature feature, bool onlyExplicit, TogglePolicy::Policy policy) + { + if (!ExperimentalFeature::IsEnabled(feature)) + { + // No need to log here + return false; + } + + if (onlyExplicit) + { + // No need to log here + return GroupPolicies().GetState(policy) == PolicyState::Enabled; + } + + if (!GroupPolicies().IsEnabled(policy)) + { + AICLI_LOG(Repo, Info, << "The default source " << sourceToLog << " is disabled due to Group Policy"); + return false; + } + + return true; + } + + bool IsWingetCommunityDefaultSourceEnabled(bool onlyExplicit = false) + { + return IsDefaultSourceEnabled(WingetCommunityDefaultSourceName(), ExperimentalFeature::Feature::None, onlyExplicit, TogglePolicy::Policy::DefaultSource); + } + + bool IsWingetMSStoreDefaultSourceEnabled(bool onlyExplicit = false) + { + return IsDefaultSourceEnabled(WingetMSStoreDefaultSourceName(), ExperimentalFeature::Feature::ExperimentalMSStore, onlyExplicit, TogglePolicy::Policy::MSStoreSource); + } + + template + std::optional FindSourceInPolicy(std::string_view name, std::string_view type, std::string_view arg) + { + auto sourcesOpt = GroupPolicies().GetValueRef

(); + if (!sourcesOpt.has_value()) + { + return std::nullopt; + } + + const auto& sources = sourcesOpt->get(); + auto source = std::find_if( + sources.begin(), + sources.end(), + [&](const SourceFromPolicy& policySource) + { + return Utility::ICUCaseInsensitiveEquals(name, policySource.Name) && Utility::ICUCaseInsensitiveEquals(type, policySource.Type) && arg == policySource.Arg; + }); + + if (source == sources.end()) + { + return std::nullopt; + } + + return *source; + } + + template + bool IsSourceInPolicy(std::string_view name, std::string_view type, std::string_view arg) + { + return FindSourceInPolicy

(name, type, arg).has_value(); + } + } + + TogglePolicy::Policy GetPolicyBlockingUserSource(std::string_view name, std::string_view type, std::string_view arg, bool isTombstone) + { + // Reasons for not allowing: + // 1. The source is a tombstone for default source that is explicitly enabled + // 2. The source is a default source that is disabled + // 3. The source has the same name as a default source that is explicitly enabled (to prevent shadowing) + // 4. Allowed sources are disabled, blocking all user sources + // 5. There is an explicit list of allowed sources and this source is not in it + // + // We don't need to check sources added by policy as those have higher priority. + // + // Use the name and arg to match sources as we don't have the identifier before adding. + + // Case 1: + // The source is a tombstone and we need the policy to be explicitly enabled. + if (isTombstone) + { + if (name == WingetCommunityDefaultSourceName() && IsWingetCommunityDefaultSourceEnabled(true)) + { + return TogglePolicy::Policy::DefaultSource; + } + + if (name == WingetMSStoreDefaultSourceName() && IsWingetMSStoreDefaultSourceEnabled(true)) + { + return TogglePolicy::Policy::MSStoreSource; + } + + // Any other tombstone is allowed + return TogglePolicy::Policy::None; + } + + // Case 2: + // - The source is not a tombstone and we don't need the policy to be explicitly enabled. + // - Check only against the source argument and type as the user source may have a different name. + // - Do a case insensitive check as the domain portion of the URL is case insensitive, + // and we don't need case sensitivity for the rest as we control the domain. + if (Utility::CaseInsensitiveEquals(arg, WingetCommunityDefaultSourceArg()) && + Utility::CaseInsensitiveEquals(type, Microsoft::PreIndexedPackageSourceFactory::Type())) + { + return IsWingetCommunityDefaultSourceEnabled(false) ? TogglePolicy::Policy::None : TogglePolicy::Policy::DefaultSource; + } + + if (Utility::CaseInsensitiveEquals(arg, WingetMSStoreDefaultSourceArg()) && + Utility::CaseInsensitiveEquals(type, Microsoft::PreIndexedPackageSourceFactory::Type())) + { + return IsWingetMSStoreDefaultSourceEnabled(false) ? TogglePolicy::Policy::None : TogglePolicy::Policy::MSStoreSource; + } + + // Case 3: + // If the source has the same name as a default source, it is shadowing with a different argument + // (as it didn't match above). We only care if Group Policy requires the default source. + if (name == WingetCommunityDefaultSourceName() && IsWingetCommunityDefaultSourceEnabled(true)) + { + AICLI_LOG(Repo, Warning, << "User source is not allowed as it shadows the default source. Name [" << name << "]. Arg [" << arg << "] Type [" << type << ']'); + return TogglePolicy::Policy::DefaultSource; + } + + if (name == WingetMSStoreDefaultSourceName() && IsWingetMSStoreDefaultSourceEnabled(true)) + { + AICLI_LOG(Repo, Warning, << "User source is not allowed as it shadows the default MS Store source. Name [" << name << "]. Arg [" << arg << "] Type [" << type << ']'); + return TogglePolicy::Policy::MSStoreSource; + } + + // Case 4: + // The guard in the source add command should already block adding. + // This check drops existing user sources. + auto allowedSourcesPolicy = GroupPolicies().GetState(TogglePolicy::Policy::AllowedSources); + if (allowedSourcesPolicy == PolicyState::Disabled) + { + AICLI_LOG(Repo, Warning, << "User sources are disabled by Group Policy"); + return TogglePolicy::Policy::AllowedSources; + } + + // Case 5: + if (allowedSourcesPolicy == PolicyState::Enabled) + { + if (!IsSourceInPolicy(name, type, arg)) + { + AICLI_LOG(Repo, Warning, << "Source is not in the Group Policy allowed list. Name [" << name << "]. Arg [" << arg << "] Type [" << type << ']'); + return TogglePolicy::Policy::AllowedSources; + } + } + + return TogglePolicy::Policy::None; + } +} diff --git a/src/AppInstallerRepositoryCore/SourcePolicy.h b/src/AppInstallerRepositoryCore/SourcePolicy.h new file mode 100644 index 0000000000..b80de64703 --- /dev/null +++ b/src/AppInstallerRepositoryCore/SourcePolicy.h @@ -0,0 +1,16 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. +#pragma once +#include + +#include + + +namespace AppInstaller::Repository +{ + // Checks whether the Group Policy allows this user source. + // If it does it returns None, otherwise it returns which policy is blocking it. + // Note that this applies to user sources that are being added as well as user sources + // that already existed when the Group Policy came into effect. + Settings::TogglePolicy::Policy GetPolicyBlockingUserSource(std::string_view name, std::string_view type, std::string_view arg, bool isTombstone); +} From acd698692c1651d69eb1b666b3709857981dea39 Mon Sep 17 00:00:00 2001 From: JohnMcPMS Date: Tue, 28 Sep 2021 16:46:20 -0700 Subject: [PATCH 2/7] Settings stream callers updated; need to implement stream internals --- src/AppInstallerCommonCore/AdminSettings.cpp | 36 +- .../Public/winget/Settings.h | 18 +- src/AppInstallerCommonCore/Settings.cpp | 16 +- src/AppInstallerCommonCore/UserSettings.cpp | 12 +- .../Public/AppInstallerRepositorySource.h | 1 + .../RepositorySource.cpp | 849 +---------------- src/AppInstallerRepositoryCore/SourceList.cpp | 894 ++++++++++-------- src/AppInstallerRepositoryCore/SourceList.h | 56 +- .../SourcePolicy.cpp | 89 +- src/AppInstallerRepositoryCore/SourcePolicy.h | 10 + 10 files changed, 674 insertions(+), 1307 deletions(-) diff --git a/src/AppInstallerCommonCore/AdminSettings.cpp b/src/AppInstallerCommonCore/AdminSettings.cpp index 9d3cefbcac..7224463bc7 100644 --- a/src/AppInstallerCommonCore/AdminSettings.cpp +++ b/src/AppInstallerCommonCore/AdminSettings.cpp @@ -48,28 +48,40 @@ namespace AppInstaller::Settings private: void LoadAdminSettings(); - void SaveAdminSettings() const; + [[nodiscard]] bool SaveAdminSettings(); + Stream m_settingStream; AdminSettingValues m_settingValues; }; - AdminSettingsInternal::AdminSettingsInternal() + AdminSettingsInternal::AdminSettingsInternal() : m_settingStream(Stream::AdminSettings) { LoadAdminSettings(); } void AdminSettingsInternal::SetAdminSetting(AdminSetting setting, bool enabled) { - switch (setting) + for (size_t i = 0; i < 10; ++i) { - case AdminSetting::LocalManifestFiles: - m_settingValues.LocalManifestFiles = enabled; - break; - default: - return; + switch (setting) + { + case AdminSetting::LocalManifestFiles: + m_settingValues.LocalManifestFiles = enabled; + break; + default: + return; + } + + if (SaveAdminSettings()) + { + return; + } + + // We need to reload the settings as they have changed + LoadAdminSettings(); } - SaveAdminSettings(); + THROW_HR_MSG(E_UNEXPECTED, "Too many attempts at SaveAdminSettings"); } bool AdminSettingsInternal::GetAdminSettingBoolValue(AdminSetting setting) const @@ -85,7 +97,7 @@ namespace AppInstaller::Settings void AdminSettingsInternal::LoadAdminSettings() { - auto stream = Settings::GetSettingStream(Settings::Streams::AdminSettings); + auto stream = m_settingStream.Get(); if (!stream) { AICLI_LOG(Core, Verbose, << "Admin settings was not found"); @@ -120,14 +132,14 @@ namespace AppInstaller::Settings TryReadScalar(document, s_AdminSettingsYaml_LocalManifestFiles, m_settingValues.LocalManifestFiles); } - void AdminSettingsInternal::SaveAdminSettings() const + bool AdminSettingsInternal::SaveAdminSettings() { YAML::Emitter out; out << YAML::BeginMap; out << YAML::Key << s_AdminSettingsYaml_LocalManifestFiles << YAML::Value << m_settingValues.LocalManifestFiles; out << YAML::EndMap; - Settings::SetSetting(Settings::Streams::AdminSettings, out.str()); + return m_settingStream.Set(out.str()); } } diff --git a/src/AppInstallerCommonCore/Public/winget/Settings.h b/src/AppInstallerCommonCore/Public/winget/Settings.h index 2d30eb6583..7f072c3889 100644 --- a/src/AppInstallerCommonCore/Public/winget/Settings.h +++ b/src/AppInstallerCommonCore/Public/winget/Settings.h @@ -29,10 +29,13 @@ namespace AppInstaller::Settings // The well known values in Streams should be used by product code, while tests may directly create them. struct StreamDefinition { - constexpr StreamDefinition(Type type, std::string_view path) : Type(type), Path(path) {} + constexpr StreamDefinition(Type type, std::string_view name) : Type(type), Name(name) {} + // The type of stream. Type Type; - std::string_view Path; + + // The name is used as a file name in some situations. + std::string_view Name; }; // A setting stream; provides access to functionality on the stream. @@ -53,23 +56,26 @@ namespace AppInstaller::Settings constexpr static StreamDefinition AdminSettings{ Type::Secure, "admin_settings"sv }; // Gets a Stream for the StreamDefinition. - // If the setting stream does not exist, returns an empty value (see operator bool). // If the stream is synchronized, attempts to Set the value can fail due to another writer // having changed the underlying stream. - Stream(const StreamDefinition& streamDefinition, bool synchronize = true); + Stream(const StreamDefinition& streamDefinition); const StreamDefinition& Definition() const { return m_streamDefinition; } - // Gets the actual stream if present; throws if not. + // Gets the stream if present. + // If the setting stream does not exist, returns an empty value (see operator bool). std::unique_ptr Get(); // Sets the stream to the given value. // Returns true if successful; false if the underlying stream has changed. - bool Set(std::string_view value); + [[nodiscard]] bool Set(std::string_view value); // Deletes the setting stream. void Remove(); + // Gets the name of the stream. + std::string_view GetName() const; + // Gets the path to the stream. std::filesystem::path GetPath() const; diff --git a/src/AppInstallerCommonCore/Settings.cpp b/src/AppInstallerCommonCore/Settings.cpp index d44de54580..1116504d34 100644 --- a/src/AppInstallerCommonCore/Settings.cpp +++ b/src/AppInstallerCommonCore/Settings.cpp @@ -25,7 +25,7 @@ namespace AppInstaller::Settings void LogSettingAction(std::string_view action, const StreamDefinition& def) { - AICLI_LOG(Core, Verbose, << "Setting action: " << action << ", Type: " << ToString(def.Type) << ", Name: " << def.Path); + AICLI_LOG(Core, Verbose, << "Setting action: " << action << ", Type: " << ToString(def.Type) << ", Name: " << def.Name); } // A settings container. @@ -347,26 +347,26 @@ namespace AppInstaller::Settings std::unique_ptr GetSettingStream(const StreamDefinition& def) { LogSettingAction("Get", def); - ValidateSettingNamePath(def.Path); - return GetSettingsContainer(def.Type)->Get(def.Path); + ValidateSettingNamePath(def.Name); + return GetSettingsContainer(def.Type)->Get(def.Name); } void SetSetting(const StreamDefinition& def, std::string_view value) { LogSettingAction("Set", def); - ValidateSettingNamePath(def.Path); - GetSettingsContainer(def.Type)->Set(def.Path, value); + ValidateSettingNamePath(def.Name); + GetSettingsContainer(def.Type)->Set(def.Name, value); } void RemoveSetting(const StreamDefinition& def) { LogSettingAction("Remove", def); - ValidateSettingNamePath(def.Path); - GetSettingsContainer(def.Type)->Remove(def.Path); + ValidateSettingNamePath(def.Name); + GetSettingsContainer(def.Type)->Remove(def.Name); } std::filesystem::path GetPathTo(const StreamDefinition& def) { - return GetSettingsContainer(def.Type)->PathTo(def.Path); + return GetSettingsContainer(def.Type)->PathTo(def.Name); } } diff --git a/src/AppInstallerCommonCore/UserSettings.cpp b/src/AppInstallerCommonCore/UserSettings.cpp index d043e69574..7f9a3dc2f5 100644 --- a/src/AppInstallerCommonCore/UserSettings.cpp +++ b/src/AppInstallerCommonCore/UserSettings.cpp @@ -71,7 +71,7 @@ namespace AppInstaller::Settings std::optional ParseFile(const StreamDefinition& setting, std::vector& warnings) { - auto stream = Stream{ setting, false }.Get(); + auto stream = Stream{ setting }.Get(); if (stream) { Json::Value root; @@ -86,8 +86,8 @@ namespace AppInstaller::Settings return root; } - AICLI_LOG(Core, Error, << "Error parsing " << setting.Path << ": " << error); - warnings.emplace_back(StringResource::String::SettingsWarningParseError, setting.Path, error, false); + AICLI_LOG(Core, Error, << "Error parsing " << setting.Name << ": " << error); + warnings.emplace_back(StringResource::String::SettingsWarningParseError, setting.Name, error, false); } return {}; @@ -338,7 +338,7 @@ namespace AppInstaller::Settings auto settingsJson = ParseFile(Stream::PrimaryUserSettings, m_warnings); if (settingsJson.has_value()) { - AICLI_LOG(Core, Info, << "Settings loaded from " << Stream::PrimaryUserSettings.Path); + AICLI_LOG(Core, Info, << "Settings loaded from " << Stream::PrimaryUserSettings.Name); m_type = UserSettingsType::Standard; settingsRoot = settingsJson.value(); } @@ -349,7 +349,7 @@ namespace AppInstaller::Settings auto settingsBackupJson = ParseFile(Stream::BackupUserSettings, m_warnings); if (settingsBackupJson.has_value()) { - AICLI_LOG(Core, Info, << "Settings loaded from " << Stream::BackupUserSettings.Path); + AICLI_LOG(Core, Info, << "Settings loaded from " << Stream::BackupUserSettings.Name); m_warnings.emplace_back(StringResource::String::SettingsWarningLoadedBackupSettings); m_type = UserSettingsType::Backup; settingsRoot = settingsBackupJson.value(); @@ -377,7 +377,7 @@ namespace AppInstaller::Settings // Create settings file if it doesn't exist. if (!std::filesystem::exists(primarySettings.GetPath())) { - primarySettings.Set(s_SettingEmpty); + std::ignore = primarySettings.Set(s_SettingEmpty); AICLI_LOG(Core, Info, << "Created new settings file"); } } diff --git a/src/AppInstallerRepositoryCore/Public/AppInstallerRepositorySource.h b/src/AppInstallerRepositoryCore/Public/AppInstallerRepositorySource.h index caecaa7975..bfd533b022 100644 --- a/src/AppInstallerRepositoryCore/Public/AppInstallerRepositorySource.h +++ b/src/AppInstallerRepositoryCore/Public/AppInstallerRepositorySource.h @@ -22,6 +22,7 @@ namespace AppInstaller::Repository User, Predefined, GroupPolicy, + Metadata, }; // Defines the trust level of the source. diff --git a/src/AppInstallerRepositoryCore/RepositorySource.cpp b/src/AppInstallerRepositoryCore/RepositorySource.cpp index 0c1f03cfa8..cbb0afebc5 100644 --- a/src/AppInstallerRepositoryCore/RepositorySource.cpp +++ b/src/AppInstallerRepositoryCore/RepositorySource.cpp @@ -5,6 +5,8 @@ #include "CompositeSource.h" #include "SourceFactory.h" +#include "SourceList.h" +#include "SourcePolicy.h" #include "Microsoft/PredefinedInstalledSourceFactory.h" #include "Microsoft/PredefinedWriteableSourceFactory.h" #include "Microsoft/PreIndexedPackageSourceFactory.h" @@ -16,603 +18,13 @@ #include +using namespace AppInstaller::Settings; +using namespace std::chrono_literals; + namespace AppInstaller::Repository { - using namespace Settings; - - using namespace std::chrono_literals; - using namespace std::string_view_literals; - - constexpr std::string_view s_SourcesYaml_Sources = "Sources"sv; - constexpr std::string_view s_SourcesYaml_Source_Name = "Name"sv; - constexpr std::string_view s_SourcesYaml_Source_Type = "Type"sv; - constexpr std::string_view s_SourcesYaml_Source_Arg = "Arg"sv; - constexpr std::string_view s_SourcesYaml_Source_Data = "Data"sv; - constexpr std::string_view s_SourcesYaml_Source_Identifier = "Identifier"sv; - constexpr std::string_view s_SourcesYaml_Source_IsTombstone = "IsTombstone"sv; - - constexpr std::string_view s_MetadataYaml_Sources = "Sources"sv; - constexpr std::string_view s_MetadataYaml_Source_Name = "Name"sv; - constexpr std::string_view s_MetadataYaml_Source_LastUpdate = "LastUpdate"sv; - constexpr std::string_view s_MetadataYaml_Source_AcceptedAgreementsIdentifier = "AcceptedAgreementsIdentifier"sv; - constexpr std::string_view s_MetadataYaml_Source_AcceptedAgreementFields = "AcceptedAgreementFields"sv; - - constexpr std::string_view s_Source_WingetCommunityDefault_Name = "winget"sv; - constexpr std::string_view s_Source_WingetCommunityDefault_Arg = "https://winget.azureedge.net/cache"sv; - constexpr std::string_view s_Source_WingetCommunityDefault_Data = "Microsoft.Winget.Source_8wekyb3d8bbwe"sv; - constexpr std::string_view s_Source_WingetCommunityDefault_Identifier = "Microsoft.Winget.Source_8wekyb3d8bbwe"sv; - - constexpr std::string_view s_Source_MSStoreDefault_Name = "msstore"sv; - constexpr std::string_view s_Source_MSStoreDefault_Arg = "https://storeedgefd.dsx.mp.microsoft.com/v9.0"sv; - constexpr std::string_view s_Source_MSStoreDefault_Identifier = "StoreEdgeFD"sv; - - constexpr std::string_view s_Source_DesktopFrameworks_Name = "microsoft.builtin.desktop.frameworks"sv; - constexpr std::string_view s_Source_DesktopFrameworks_Arg = "https://winget.azureedge.net/platform"sv; - constexpr std::string_view s_Source_DesktopFrameworks_Data = "Microsoft.Winget.Platform.Source_8wekyb3d8bbwe"sv; - constexpr std::string_view s_Source_DesktopFrameworks_Identifier = "Microsoft.Winget.Platform.Source_8wekyb3d8bbwe"sv; - namespace { - // SourceDetails with additional data. - struct SourceDetailsInternal : public SourceDetails - { - // If true, this is a tombstone, marking the deletion of a source at a lower priority origin. - bool IsTombstone = false; - - std::string AcceptedAgreementsIdentifier; - - int AcceptedAgreementFields = 0; - - SourceDetailsInternal() = default; - - SourceDetailsInternal(const SourceDetails& details) : SourceDetails(details) {}; - }; - - SourceDetailsInternal GetWellKnownSourceDetailsInternal(WellKnownSource source) - { - switch (source) - { - case WellKnownSource::WinGet: - { - SourceDetailsInternal details; - details.Origin = SourceOrigin::Default; - details.Name = s_Source_WingetCommunityDefault_Name; - details.Type = Microsoft::PreIndexedPackageSourceFactory::Type(); - details.Arg = s_Source_WingetCommunityDefault_Arg; - details.Data = s_Source_WingetCommunityDefault_Data; - details.Identifier = s_Source_WingetCommunityDefault_Identifier; - details.TrustLevel = SourceTrustLevel::Trusted | SourceTrustLevel::StoreOrigin; - return details; - } - case WellKnownSource::MicrosoftStore: - { - SourceDetailsInternal details; - details.Origin = SourceOrigin::Default; - details.Name = s_Source_MSStoreDefault_Name; - details.Type = Rest::RestSourceFactory::Type(); - details.Arg = s_Source_MSStoreDefault_Arg; - details.Identifier = s_Source_MSStoreDefault_Identifier; - details.TrustLevel = SourceTrustLevel::Trusted; - details.SupportInstalledSearchCorrelation = false; - return details; - } - case WellKnownSource::DesktopFrameworks: - { - SourceDetailsInternal details; - details.Origin = SourceOrigin::Default; - details.Name = s_Source_DesktopFrameworks_Name; - details.Type = Microsoft::PreIndexedPackageSourceFactory::Type(); - details.Arg = s_Source_DesktopFrameworks_Arg; - details.Data = s_Source_DesktopFrameworks_Data; - details.Identifier = s_Source_DesktopFrameworks_Identifier; - details.TrustLevel = SourceTrustLevel::Trusted | SourceTrustLevel::StoreOrigin; - // Cheat the system and call this a tombstone. This effectively hides it from everything outside - // of this file, while still allowing it to properly save metadata. There might be problems - // if someone chooses the exact same name as this, which is why its name is very long. - // TODO: When refactoring the source interface, handle this with Visibility or similar. - details.IsTombstone = true; - return details; - } - } - - THROW_HR(E_UNEXPECTED); - } - - // Checks whether a default source is enabled with the current settings. - // onlyExplicit determines whether we consider the not-configured state to be enabled or not. - bool IsDefaultSourceEnabled(std::string_view sourceToLog, ExperimentalFeature::Feature feature, bool onlyExplicit, TogglePolicy::Policy policy) - { - if (!ExperimentalFeature::IsEnabled(feature)) - { - // No need to log here - return false; - } - - if (onlyExplicit) - { - // No need to log here - return GroupPolicies().GetState(policy) == PolicyState::Enabled; - } - - if (!GroupPolicies().IsEnabled(policy)) - { - AICLI_LOG(Repo, Info, << "The default source " << sourceToLog << " is disabled due to Group Policy"); - return false; - } - - return true; - } - - bool IsWingetCommunityDefaultSourceEnabled(bool onlyExplicit = false) - { - return IsDefaultSourceEnabled(s_Source_WingetCommunityDefault_Name, ExperimentalFeature::Feature::None, onlyExplicit, TogglePolicy::Policy::DefaultSource); - } - - bool IsMSStoreDefaultSourceEnabled(bool onlyExplicit = false) - { - return IsDefaultSourceEnabled(s_Source_MSStoreDefault_Name, ExperimentalFeature::Feature::None, onlyExplicit, TogglePolicy::Policy::MSStoreSource); - } - - template - std::optional FindSourceInPolicy(std::string_view name, std::string_view type, std::string_view arg) - { - auto sourcesOpt = GroupPolicies().GetValueRef

(); - if (!sourcesOpt.has_value()) - { - return std::nullopt; - } - - const auto& sources = sourcesOpt->get(); - auto source = std::find_if( - sources.begin(), - sources.end(), - [&](const SourceFromPolicy& policySource) - { - return Utility::ICUCaseInsensitiveEquals(name, policySource.Name) && Utility::ICUCaseInsensitiveEquals(type, policySource.Type) && arg == policySource.Arg; - }); - - if (source == sources.end()) - { - return std::nullopt; - } - - return *source; - } - - template - bool IsSourceInPolicy(std::string_view name, std::string_view type, std::string_view arg) - { - return FindSourceInPolicy

(name, type, arg).has_value(); - } - - // Checks whether the Group Policy allows this user source. - // If it does it returns None, otherwise it returns which policy is blocking it. - // Note that this applies to user sources that are being added as well as user sources - // that already existed when the Group Policy came into effect. - TogglePolicy::Policy GetPolicyBlockingUserSource(std::string_view name, std::string_view type, std::string_view arg, bool isTombstone) - { - // Reasons for not allowing: - // 1. The source is a tombstone for default source that is explicitly enabled - // 2. The source is a default source that is disabled - // 3. The source has the same name as a default source that is explicitly enabled (to prevent shadowing) - // 4. Allowed sources are disabled, blocking all user sources - // 5. There is an explicit list of allowed sources and this source is not in it - // - // We don't need to check sources added by policy as those have higher priority. - // - // Use the name and arg to match sources as we don't have the identifier before adding. - - // Case 1: - // The source is a tombstone and we need the policy to be explicitly enabled. - if (isTombstone) - { - if (name == s_Source_WingetCommunityDefault_Name && IsWingetCommunityDefaultSourceEnabled(true)) - { - return TogglePolicy::Policy::DefaultSource; - } - - if (name == s_Source_MSStoreDefault_Name && IsMSStoreDefaultSourceEnabled(true)) - { - return TogglePolicy::Policy::MSStoreSource; - } - - // Any other tombstone is allowed - return TogglePolicy::Policy::None; - } - - // Case 2: - // - The source is not a tombstone and we don't need the policy to be explicitly enabled. - // - Check only against the source argument and type as the user source may have a different name. - // - Do a case insensitive check as the domain portion of the URL is case insensitive, - // and we don't need case sensitivity for the rest as we control the domain. - if (Utility::CaseInsensitiveEquals(arg, s_Source_WingetCommunityDefault_Arg) && - Utility::CaseInsensitiveEquals(type, Microsoft::PreIndexedPackageSourceFactory::Type())) - { - return IsWingetCommunityDefaultSourceEnabled(false) ? TogglePolicy::Policy::None : TogglePolicy::Policy::DefaultSource; - } - - if (Utility::CaseInsensitiveEquals(arg, s_Source_MSStoreDefault_Arg) && - Utility::CaseInsensitiveEquals(type, Rest::RestSourceFactory::Type())) - { - return IsMSStoreDefaultSourceEnabled(false) ? TogglePolicy::Policy::None : TogglePolicy::Policy::MSStoreSource; - } - - // Case 3: - // If the source has the same name as a default source, it is shadowing with a different argument - // (as it didn't match above). We only care if Group Policy requires the default source. - if (name == s_Source_WingetCommunityDefault_Name && IsWingetCommunityDefaultSourceEnabled(true)) - { - AICLI_LOG(Repo, Warning, << "User source is not allowed as it shadows the default source. Name [" << name << "]. Arg [" << arg << "] Type [" << type << ']'); - return TogglePolicy::Policy::DefaultSource; - } - - if (name == s_Source_MSStoreDefault_Name && IsMSStoreDefaultSourceEnabled(true)) - { - AICLI_LOG(Repo, Warning, << "User source is not allowed as it shadows a default MS Store source. Name [" << name << "]. Arg [" << arg << "] Type [" << type << ']'); - return TogglePolicy::Policy::MSStoreSource; - } - - // Case 4: - // The guard in the source add command should already block adding. - // This check drops existing user sources. - auto allowedSourcesPolicy = GroupPolicies().GetState(TogglePolicy::Policy::AllowedSources); - if (allowedSourcesPolicy == PolicyState::Disabled) - { - AICLI_LOG(Repo, Warning, << "User sources are disabled by Group Policy"); - return TogglePolicy::Policy::AllowedSources; - } - - // Case 5: - if (allowedSourcesPolicy == PolicyState::Enabled) - { - if (!IsSourceInPolicy(name, type, arg)) - { - AICLI_LOG(Repo, Warning, << "Source is not in the Group Policy allowed list. Name [" << name << "]. Arg [" << arg << "] Type [" << type << ']'); - return TogglePolicy::Policy::AllowedSources; - } - } - - return TogglePolicy::Policy::None; - } - - bool IsUserSourceAllowedByPolicy(std::string_view name, std::string_view type, std::string_view arg, bool isTombstone) - { - return GetPolicyBlockingUserSource(name, type, arg, isTombstone) == TogglePolicy::Policy::None; - } - - void EnsureSourceIsRemovable(const SourceDetailsInternal& source) - { - // Block removing sources added by Group Policy - if (source.Origin == SourceOrigin::GroupPolicy) - { - AICLI_LOG(Repo, Error, << "Cannot remove source added by Group Policy"); - throw GroupPolicyException(TogglePolicy::Policy::AdditionalSources); - } - - // Block removing default sources required by Group Policy. - if (source.Origin == SourceOrigin::Default) - { - if (GroupPolicies().GetState(TogglePolicy::Policy::DefaultSource) == PolicyState::Enabled && - source.Identifier == s_Source_WingetCommunityDefault_Identifier) - { - throw GroupPolicyException(TogglePolicy::Policy::DefaultSource); - } - - if (GroupPolicies().GetState(TogglePolicy::Policy::MSStoreSource) == PolicyState::Enabled && - source.Identifier == s_Source_MSStoreDefault_Identifier) - { - throw GroupPolicyException(TogglePolicy::Policy::MSStoreSource); - } - } - } - - // Attempts to read a single scalar value from the node. - template - bool TryReadScalar(std::string_view settingName, const std::string& settingValue, const YAML::Node& sourceNode, std::string_view name, Value& value, bool required = true) - { - YAML::Node valueNode = sourceNode[std::string{ name }]; - - if (!valueNode || !valueNode.IsScalar()) - { - if (required) - { - AICLI_LOG(Repo, Error, << "Setting '" << settingName << "' did not contain the expected format (" << name << " is invalid within a source):\n" << settingValue); - } - return false; - } - - value = valueNode.as(); - return true; - } - - // Attempts to read the source details from the given stream. - // Results are all or nothing; if any failures occur, no details are returned. - bool TryReadSourceDetails( - std::string_view settingName, - std::istream& stream, - std::string_view rootName, - std::function parse, - std::vector& sourceDetails) - { - std::vector result; - std::string settingValue = Utility::ReadEntireStream(stream); - - YAML::Node document; - try - { - document = YAML::Load(settingValue); - } - catch (const std::exception& e) - { - AICLI_LOG(YAML, Error, << "Setting '" << settingName << "' contained invalid YAML (" << e.what() << "):\n" << settingValue); - return false; - } - - try - { - YAML::Node sources = document[rootName]; - if (!sources) - { - AICLI_LOG(Repo, Error, << "Setting '" << settingName << "' did not contain the expected format (missing " << rootName << "):\n" << settingValue); - return false; - } - - if (sources.IsNull()) - { - // An empty sources is an acceptable thing. - return true; - } - - if (!sources.IsSequence()) - { - AICLI_LOG(Repo, Error, << "Setting '" << settingName << "' did not contain the expected format (" << rootName << " was not a sequence):\n" << settingValue); - return false; - } - - for (const auto& source : sources.Sequence()) - { - SourceDetailsInternal details; - if (!parse(details, settingValue, source)) - { - return false; - } - - result.emplace_back(std::move(details)); - } - } - catch (const std::exception& e) - { - AICLI_LOG(YAML, Error, << "Setting '" << settingName << "' contained unexpected YAML (" << e.what() << "):\n" << settingValue); - return false; - } - - sourceDetails = std::move(result); - return true; - } - - // Gets the source details from a particular setting, or an empty optional if no setting exists. - std::optional> TryGetSourcesFromSetting( - const Settings::StreamDefinition& setting, - std::string_view rootName, - std::function parse) - { - auto sourcesStream = Settings::GetSettingStream(setting); - if (!sourcesStream) - { - // Note that this case is different than the one in which all sources have been removed. - return {}; - } - else - { - std::vector result; - THROW_HR_IF(APPINSTALLER_CLI_ERROR_SOURCES_INVALID, !TryReadSourceDetails(setting.Path, *sourcesStream, rootName, parse, result)); - return result; - } - } - - // Gets the source details from a particular setting. - std::vector GetSourcesFromSetting( - const Settings::StreamDefinition& setting, - std::string_view rootName, - std::function parse) - { - return TryGetSourcesFromSetting(setting, rootName, parse).value_or(std::vector{}); - } - - // Gets the metadata. - std::vector GetMetadata() - { - return GetSourcesFromSetting( - Settings::Streams::SourcesMetadata, - s_MetadataYaml_Sources, - [&](SourceDetailsInternal& details, const std::string& settingValue, const YAML::Node& source) - { - std::string_view name = Settings::Streams::SourcesMetadata.Path; - if (!TryReadScalar(name, settingValue, source, s_MetadataYaml_Source_Name, details.Name)) { return false; } - int64_t lastUpdateInEpoch{}; - if (!TryReadScalar(name, settingValue, source, s_MetadataYaml_Source_LastUpdate, lastUpdateInEpoch)) { return false; } - details.LastUpdateTime = Utility::ConvertUnixEpochToSystemClock(lastUpdateInEpoch); - TryReadScalar(name, settingValue, source, s_MetadataYaml_Source_AcceptedAgreementsIdentifier, details.AcceptedAgreementsIdentifier, false); - TryReadScalar(name, settingValue, source, s_MetadataYaml_Source_AcceptedAgreementFields, details.AcceptedAgreementFields, false); - return true; - }); - } - - // Checks whether a default source is enabled with the current settings - bool IsDefaultSourceEnabled(std::string_view sourceToLog, ExperimentalFeature::Feature feature, TogglePolicy::Policy policy) - { - if (!ExperimentalFeature::IsEnabled(feature)) - { - // No need to log here - return false; - } - - if (!GroupPolicies().IsEnabled(policy)) - { - AICLI_LOG(Repo, Info, << "The default source " << sourceToLog << " is disabled due to Group Policy"); - return false; - } - - return true; - } - - // Gets the sources from a particular origin. - std::vector GetSourcesByOrigin(SourceOrigin origin) - { - std::vector result; - - switch (origin) - { - case SourceOrigin::Default: - { - if (IsMSStoreDefaultSourceEnabled()) - { - result.emplace_back(GetWellKnownSourceDetailsInternal(WellKnownSource::MicrosoftStore)); - } - - if (IsWingetCommunityDefaultSourceEnabled()) - { - result.emplace_back(GetWellKnownSourceDetailsInternal(WellKnownSource::WinGet)); - } - - // Since we are using the tombstone trick, this is added just to have the source in the internal - // list for tracking updates. Thus there is no need to check a policy. - result.emplace_back(GetWellKnownSourceDetailsInternal(WellKnownSource::DesktopFrameworks)); - } - break; - case SourceOrigin::User: - { - std::vector userSources = GetSourcesFromSetting( - Settings::Streams::UserSources, - s_SourcesYaml_Sources, - [&](SourceDetailsInternal& details, const std::string& settingValue, const YAML::Node& source) - { - std::string_view name = Settings::Streams::UserSources.Path; - if (!TryReadScalar(name, settingValue, source, s_SourcesYaml_Source_Name, details.Name)) { return false; } - if (!TryReadScalar(name, settingValue, source, s_SourcesYaml_Source_Type, details.Type)) { return false; } - if (!TryReadScalar(name, settingValue, source, s_SourcesYaml_Source_Arg, details.Arg)) { return false; } - if (!TryReadScalar(name, settingValue, source, s_SourcesYaml_Source_Data, details.Data)) { return false; } - if (!TryReadScalar(name, settingValue, source, s_SourcesYaml_Source_IsTombstone, details.IsTombstone)) { return false; } - TryReadScalar(name, settingValue, source, s_SourcesYaml_Source_Identifier, details.Identifier, false); - return true; - }); - - for (auto& source : userSources) - { - // Check source against list of allowed sources and drop tombstones for required sources - if (!IsUserSourceAllowedByPolicy(source.Name, source.Type, source.Arg, source.IsTombstone)) - { - AICLI_LOG(Repo, Warning, << "User source " << source.Name << " dropped because of group policy"); - continue; - } - - result.emplace_back(std::move(source)); - } - } - break; - case SourceOrigin::GroupPolicy: - { - if (GroupPolicies().GetState(TogglePolicy::Policy::AdditionalSources) == PolicyState::Enabled) - { - auto additionalSourcesOpt = GroupPolicies().GetValueRef(); - if (additionalSourcesOpt.has_value()) - { - const auto& additionalSources = additionalSourcesOpt->get(); - for (const auto& additionalSource : additionalSources) - { - SourceDetailsInternal details; - details.Name = additionalSource.Name; - details.Type = additionalSource.Type; - details.Arg = additionalSource.Arg; - details.Data = additionalSource.Data; - details.Identifier = additionalSource.Identifier; - details.Origin = SourceOrigin::GroupPolicy; - result.emplace_back(std::move(details)); - } - } - } - } - break; - default: - THROW_HR(E_UNEXPECTED); - } - - for (auto& source : result) - { - source.Origin = origin; - } - - return result; - } - - // Sets the sources for a particular setting, from a particular origin. - void SetSourcesToSettingWithFilter(const Settings::StreamDefinition& setting, SourceOrigin origin, const std::vector& sources) - { - YAML::Emitter out; - out << YAML::BeginMap; - out << YAML::Key << s_SourcesYaml_Sources; - out << YAML::BeginSeq; - - for (const auto& details : sources) - { - if (details.Origin == origin) - { - out << YAML::BeginMap; - out << YAML::Key << s_SourcesYaml_Source_Name << YAML::Value << details.Name; - out << YAML::Key << s_SourcesYaml_Source_Type << YAML::Value << details.Type; - out << YAML::Key << s_SourcesYaml_Source_Arg << YAML::Value << details.Arg; - out << YAML::Key << s_SourcesYaml_Source_Data << YAML::Value << details.Data; - out << YAML::Key << s_SourcesYaml_Source_Identifier << YAML::Value << details.Identifier; - out << YAML::Key << s_SourcesYaml_Source_IsTombstone << YAML::Value << details.IsTombstone; - out << YAML::EndMap; - } - } - - out << YAML::EndSeq; - out << YAML::EndMap; - - Settings::SetSetting(setting, out.str()); - } - - // Sets the metadata only (which is not a secure setting and can be set unprivileged) - void SetMetadata(const std::vector& sources) - { - YAML::Emitter out; - out << YAML::BeginMap; - out << YAML::Key << s_MetadataYaml_Sources; - out << YAML::BeginSeq; - - for (const auto& details : sources) - { - out << YAML::BeginMap; - out << YAML::Key << s_MetadataYaml_Source_Name << YAML::Value << details.Name; - out << YAML::Key << s_MetadataYaml_Source_LastUpdate << YAML::Value << Utility::ConvertSystemClockToUnixEpoch(details.LastUpdateTime); - out << YAML::Key << s_MetadataYaml_Source_AcceptedAgreementsIdentifier << YAML::Value << details.AcceptedAgreementsIdentifier; - out << YAML::Key << s_MetadataYaml_Source_AcceptedAgreementFields << YAML::Value << details.AcceptedAgreementFields; - out << YAML::EndMap; - } - - out << YAML::EndSeq; - out << YAML::EndMap; - - Settings::SetSetting(Settings::Streams::SourcesMetadata, out.str()); - } - - // Sets the sources for a given origin. - void SetSourcesByOrigin(SourceOrigin origin, const std::vector& sources) - { - switch (origin) - { - case SourceOrigin::User: - SetSourcesToSettingWithFilter(Settings::Streams::UserSources, SourceOrigin::User, sources); - break; - default: - THROW_HR(E_UNEXPECTED); - } - - SetMetadata(sources); - } - #ifndef AICLI_DISABLE_TEST_HOOKS static std::map()>> s_Sources_TestHook_SourceFactories; #endif @@ -737,208 +149,6 @@ namespace AppInstaller::Repository return false; } - // Struct containing internal implementation of source list - // This contains all sources including tombstoned sources - struct SourceListInternal - { - SourceListInternal(); - - // Get a list of current sources references which can be used to update the contents in place. - // e.g. update the LastTimeUpdated value of sources. - std::vector> GetCurrentSourceRefs(); - - // Current source means source that's not in tombstone - SourceDetailsInternal* GetCurrentSource(std::string_view name); - - // Source includes ones in tombstone - SourceDetailsInternal* GetSource(std::string_view name); - - // Add/remove a current source - void AddSource(const SourceDetailsInternal& source); - void RemoveSource(const SourceDetailsInternal& source); - - // Save source metadata. Currently only LastTimeUpdated is used. - void SaveMetadata() const; - - bool CheckSourceAgreements(const SourceDetails& details); - - // SaveMetadata() should be called after all accepted source agreements are updated. - void SaveAcceptedSourceAgreements(const SourceDetails& details); - - private: - std::vector m_sourceList; - - // calls std::find_if and return the iterator. - auto FindSource(std::string_view name, bool includeTombstone = false); - }; - - SourceListInternal::SourceListInternal() - { - for (SourceOrigin origin : { SourceOrigin::GroupPolicy, SourceOrigin::User, SourceOrigin::Default }) - { - auto forOrigin = GetSourcesByOrigin(origin); - - for (auto&& source : forOrigin) - { - auto foundSource = GetSource(source.Name); - if (!foundSource) - { - // Name not already defined, add it - m_sourceList.emplace_back(std::move(source)); - } - else - { - AICLI_LOG(Repo, Info, << "Source named '" << foundSource->Name << "' is already defined at origin " << ToString(foundSource->Origin) << - ". The source from origin " << ToString(origin) << " is dropped."); - } - } - } - - auto metadata = GetMetadata(); - for (const auto& metaSource : metadata) - { - auto source = GetSource(metaSource.Name); - if (source) - { - source->LastUpdateTime = metaSource.LastUpdateTime; - source->AcceptedAgreementFields = metaSource.AcceptedAgreementFields; - source->AcceptedAgreementsIdentifier = metaSource.AcceptedAgreementsIdentifier; - } - } - } - - std::vector> SourceListInternal::GetCurrentSourceRefs() - { - std::vector> result; - - for (auto& s : m_sourceList) - { - if (!s.IsTombstone) - { - result.emplace_back(std::ref(s)); - } - else - { - AICLI_LOG(Repo, Info, << "GetCurrentSourceRefs: Source named '" << s.Name << "' from origin " << ToString(s.Origin) << " is a tombstone and is dropped."); - } - } - - return result; - } - - auto SourceListInternal::FindSource(std::string_view name, bool includeTombstone) - { - return std::find_if(m_sourceList.begin(), m_sourceList.end(), - [name, includeTombstone](const SourceDetailsInternal& sd) - { - return Utility::ICUCaseInsensitiveEquals(sd.Name, name) && - (!sd.IsTombstone || includeTombstone); - }); - } - - SourceDetailsInternal* SourceListInternal::GetCurrentSource(std::string_view name) - { - auto itr = FindSource(name); - return itr == m_sourceList.end() ? nullptr : &(*itr); - } - - SourceDetailsInternal* SourceListInternal::GetSource(std::string_view name) - { - auto itr = FindSource(name, true); - return itr == m_sourceList.end() ? nullptr : &(*itr); - } - - void SourceListInternal::AddSource(const SourceDetailsInternal& details) - { - // Erase the source's tombstone entry if applicable - auto itr = FindSource(details.Name, true); - if (itr != m_sourceList.end()) - { - THROW_HR_IF(HRESULT_FROM_WIN32(ERROR_INVALID_STATE), !itr->IsTombstone); - m_sourceList.erase(itr); - } - - m_sourceList.emplace_back(details); - - SetSourcesByOrigin(SourceOrigin::User, m_sourceList); - } - - void SourceListInternal::RemoveSource(const SourceDetailsInternal& source) - { - switch (source.Origin) - { - case SourceOrigin::Default: - { - SourceDetailsInternal tombstone; - tombstone.Name = source.Name; - tombstone.IsTombstone = true; - tombstone.Origin = SourceOrigin::User; - m_sourceList.emplace_back(std::move(tombstone)); - } - break; - case SourceOrigin::User: - m_sourceList.erase(FindSource(source.Name)); - break; - case SourceOrigin::GroupPolicy: - // This should have already been blocked higher up. - AICLI_LOG(Repo, Error, << "Attempting to remove Group Policy source: " << source.Name); - THROW_HR(E_UNEXPECTED); - default: - THROW_HR(E_UNEXPECTED); - } - - SetSourcesByOrigin(SourceOrigin::User, m_sourceList); - } - - void SourceListInternal::SaveMetadata() const - { - SetMetadata(m_sourceList); - } - - bool SourceListInternal::CheckSourceAgreements(const SourceDetails& details) - { - auto agreementFields = GetAgreementFieldsFromSourceInformation(details.Information); - - if (agreementFields == ImplicitAgreementFieldEnum::None && details.Information.SourceAgreementsIdentifier.empty()) - { - // No agreements to be accepted. - return true; - } - - auto detailsInternal = GetCurrentSource(details.Name); - if (!detailsInternal) - { - // Source not found. - return false; - } - - return static_cast(agreementFields) == detailsInternal->AcceptedAgreementFields && - details.Information.SourceAgreementsIdentifier == detailsInternal->AcceptedAgreementsIdentifier; - } - - void SourceListInternal::SaveAcceptedSourceAgreements(const SourceDetails& details) - { - auto agreementFields = GetAgreementFieldsFromSourceInformation(details.Information); - - if (agreementFields == ImplicitAgreementFieldEnum::None && details.Information.SourceAgreementsIdentifier.empty()) - { - // No agreements to be accepted. - return; - } - - auto detailsInternal = GetCurrentSource(details.Name); - if (!detailsInternal) - { - // No source to update. - return; - } - - detailsInternal->AcceptedAgreementFields = static_cast(agreementFields); - detailsInternal->AcceptedAgreementsIdentifier = details.Information.SourceAgreementsIdentifier; - - SaveMetadata(); - } - std::string GetStringVectorMessage(const std::vector& input) { std::string result; @@ -991,6 +201,8 @@ namespace AppInstaller::Repository return "User"sv; case SourceOrigin::GroupPolicy: return "GroupPolicy"sv; + case SourceOrigin::Metadata: + return "Metadata"sv; default: THROW_HR(E_UNEXPECTED); } @@ -1011,7 +223,7 @@ namespace AppInstaller::Repository std::vector GetSources() { - SourceListInternal sourceList; + SourceList sourceList; std::vector result; for (auto&& source : sourceList.GetCurrentSourceRefs()) @@ -1025,7 +237,7 @@ namespace AppInstaller::Repository std::optional GetSource(std::string_view name) { // Check all sources for the given name. - SourceListInternal sourceList; + SourceList sourceList; auto source = sourceList.GetCurrentSource(name); if (!source) @@ -1045,15 +257,16 @@ namespace AppInstaller::Repository AICLI_LOG(Repo, Info, << "Adding source: Name[" << sourceDetails.Name << "], Type[" << sourceDetails.Type << "], Arg[" << sourceDetails.Arg << "]"); // Check all sources for the given name. - SourceListInternal sourceList; + SourceList sourceList; auto source = sourceList.GetCurrentSource(sourceDetails.Name); THROW_HR_IF(APPINSTALLER_CLI_ERROR_SOURCE_NAME_ALREADY_EXISTS, source != nullptr); - // Check for a non-user tombstone; hidden source data that we don't want to collide. + // Check for a hidden source data that we don't want to collide. // TODO: Refactor the source interface so that we don't do this - auto tombstoneSource = sourceList.GetSource(sourceDetails.Name); - THROW_HR_IF(APPINSTALLER_CLI_ERROR_SOURCE_NAME_ALREADY_EXISTS, tombstoneSource && tombstoneSource->Origin != SourceOrigin::User); + auto hiddenSource = GetSource(sourceDetails.Name); + THROW_HR_IF(APPINSTALLER_CLI_ERROR_SOURCE_NAME_ALREADY_EXISTS, + hiddenSource && hiddenSource->Origin != SourceOrigin::User && hiddenSource->Origin != SourceOrigin::Metadata); // Check sources allowed by group policy auto blockingPolicy = GetPolicyBlockingUserSource(sourceDetails.Name, sourceDetails.Type, sourceDetails.Arg, false); @@ -1079,7 +292,7 @@ namespace AppInstaller::Repository OpenSourceResult OpenSource(std::string_view name, IProgressCallback& progress) { - SourceListInternal sourceList; + SourceList sourceList; if (name.empty()) { @@ -1101,7 +314,6 @@ namespace AppInstaller::Repository OpenSourceResult result; std::vector> openExceptionProxies; - bool sourceUpdated = false; for (auto& source : currentSources) { AICLI_LOG(Repo, Info, << "Adding to aggregated source: " << source.get().Name); @@ -1112,7 +324,10 @@ namespace AppInstaller::Repository { // TODO: Consider adding a context callback to indicate we are doing the same action // to avoid the progress bar fill up multiple times. - sourceUpdated = BackgroundUpdateSourceFromDetails(source, progress) || sourceUpdated; + if (BackgroundUpdateSourceFromDetails(source, progress)) + { + sourceList.SaveMetadata(source); + } } catch (...) { @@ -1141,11 +356,6 @@ namespace AppInstaller::Repository } } - if (sourceUpdated) - { - sourceList.SaveMetadata(); - } - // If all sources failed to open, then throw an exception that is specific to this case. THROW_HR_IF(APPINSTALLER_CLI_ERROR_FAILED_TO_OPEN_ALL_SOURCES, !aggregatedSource->HasAvailableSource()); @@ -1178,7 +388,7 @@ namespace AppInstaller::Repository { if (BackgroundUpdateSourceFromDetails(*source, progress)) { - sourceList.SaveMetadata(); + sourceList.SaveMetadata(*source); } } catch (...) @@ -1204,7 +414,7 @@ namespace AppInstaller::Repository // Restricted sources don't have full functionality if (!details.Name.empty()) { - SourceListInternal sourceList; + SourceList sourceList; auto source = sourceList.GetSource(details.Name); if (!source) { @@ -1218,7 +428,7 @@ namespace AppInstaller::Repository { if (BackgroundUpdateSourceFromDetails(*source, progress)) { - sourceList.SaveMetadata(); + sourceList.SaveMetadata(*source); } } catch (...) @@ -1313,7 +523,7 @@ namespace AppInstaller::Repository { THROW_HR_IF(E_INVALIDARG, name.empty()); - SourceListInternal sourceList; + SourceList sourceList; auto source = sourceList.GetCurrentSource(name); if (!source) @@ -1328,7 +538,7 @@ namespace AppInstaller::Repository bool result = UpdateSourceFromDetails(*source, progress); if (result) { - sourceList.SaveMetadata(); + sourceList.SaveMetadata(*source); } return result; @@ -1339,7 +549,7 @@ namespace AppInstaller::Repository { THROW_HR_IF(E_INVALIDARG, name.empty()); - SourceListInternal sourceList; + SourceList sourceList; auto source = sourceList.GetCurrentSource(name); if (!source) @@ -1367,13 +577,12 @@ namespace AppInstaller::Repository { if (name.empty()) { - Settings::RemoveSetting(Settings::Streams::UserSources); - Settings::RemoveSetting(Settings::Streams::SourcesMetadata); + SourceList::RemoveSettingsStreams(); return true; } else { - SourceListInternal sourceList; + SourceList sourceList; auto source = sourceList.GetCurrentSource(name); if (!source) @@ -1407,13 +616,13 @@ namespace AppInstaller::Repository bool CheckSourceAgreements(const SourceDetails& source) { - SourceListInternal sourceList; + SourceList sourceList; return sourceList.CheckSourceAgreements(source); } void SaveAcceptedSourceAgreements(const SourceDetails& source) { - SourceListInternal sourceList; + SourceList sourceList; sourceList.SaveAcceptedSourceAgreements(source); } diff --git a/src/AppInstallerRepositoryCore/SourceList.cpp b/src/AppInstallerRepositoryCore/SourceList.cpp index 3fb213364c..bdf94cf3ff 100644 --- a/src/AppInstallerRepositoryCore/SourceList.cpp +++ b/src/AppInstallerRepositoryCore/SourceList.cpp @@ -2,226 +2,44 @@ // Licensed under the MIT License. #include "pch.h" #include "SourceList.h" +#include "SourcePolicy.h" +#include "Microsoft/PreIndexedPackageSourceFactory.h" +#include "Rest/RestSourceFactory.h" +using namespace AppInstaller::Settings; +using namespace std::string_view_literals; namespace AppInstaller::Repository { - using namespace Settings; - - using namespace std::chrono_literals; - using namespace std::string_view_literals; - - constexpr std::string_view s_SourcesYaml_Sources = "Sources"sv; - constexpr std::string_view s_SourcesYaml_Source_Name = "Name"sv; - constexpr std::string_view s_SourcesYaml_Source_Type = "Type"sv; - constexpr std::string_view s_SourcesYaml_Source_Arg = "Arg"sv; - constexpr std::string_view s_SourcesYaml_Source_Data = "Data"sv; - constexpr std::string_view s_SourcesYaml_Source_Identifier = "Identifier"sv; - constexpr std::string_view s_SourcesYaml_Source_IsTombstone = "IsTombstone"sv; - - constexpr std::string_view s_MetadataYaml_Sources = "Sources"sv; - constexpr std::string_view s_MetadataYaml_Source_Name = "Name"sv; - constexpr std::string_view s_MetadataYaml_Source_LastUpdate = "LastUpdate"sv; - - constexpr std::string_view s_Source_WingetCommunityDefault_Name = "winget"sv; - constexpr std::string_view s_Source_WingetCommunityDefault_Arg = "https://winget.azureedge.net/cache"sv; - constexpr std::string_view s_Source_WingetCommunityDefault_Data = "Microsoft.Winget.Source_8wekyb3d8bbwe"sv; - constexpr std::string_view s_Source_WingetCommunityDefault_Identifier = "Microsoft.Winget.Source_8wekyb3d8bbwe"sv; - - constexpr std::string_view s_Source_WingetMSStoreDefault_Name = "msstore"sv; - constexpr std::string_view s_Source_WingetMSStoreDefault_Arg = "https://winget.azureedge.net/msstore"sv; - constexpr std::string_view s_Source_WingetMSStoreDefault_Data = "Microsoft.Winget.MSStore.Source_8wekyb3d8bbwe"sv; - constexpr std::string_view s_Source_WingetMSStoreDefault_Identifier = "Microsoft.Winget.MSStore.Source_8wekyb3d8bbwe"sv; - namespace { - // Checks whether a default source is enabled with the current settings. - // onlyExplicit determines whether we consider the not-configured state to be enabled or not. - bool IsDefaultSourceEnabled(std::string_view sourceToLog, ExperimentalFeature::Feature feature, bool onlyExplicit, TogglePolicy::Policy policy) - { - if (!ExperimentalFeature::IsEnabled(feature)) - { - // No need to log here - return false; - } - - if (onlyExplicit) - { - // No need to log here - return GroupPolicies().GetState(policy) == PolicyState::Enabled; - } - - if (!GroupPolicies().IsEnabled(policy)) - { - AICLI_LOG(Repo, Info, << "The default source " << sourceToLog << " is disabled due to Group Policy"); - return false; - } - - return true; - } - - bool IsWingetCommunityDefaultSourceEnabled(bool onlyExplicit = false) - { - return IsDefaultSourceEnabled(s_Source_WingetCommunityDefault_Name, ExperimentalFeature::Feature::None, onlyExplicit, TogglePolicy::Policy::DefaultSource); - } - - bool IsWingetMSStoreDefaultSourceEnabled(bool onlyExplicit = false) - { - return IsDefaultSourceEnabled(s_Source_WingetMSStoreDefault_Name, ExperimentalFeature::Feature::ExperimentalMSStore, onlyExplicit, TogglePolicy::Policy::MSStoreSource); - } - - template - std::optional FindSourceInPolicy(std::string_view name, std::string_view type, std::string_view arg) - { - auto sourcesOpt = GroupPolicies().GetValueRef

(); - if (!sourcesOpt.has_value()) - { - return std::nullopt; - } - - const auto& sources = sourcesOpt->get(); - auto source = std::find_if( - sources.begin(), - sources.end(), - [&](const SourceFromPolicy& policySource) - { - return Utility::ICUCaseInsensitiveEquals(name, policySource.Name) && Utility::ICUCaseInsensitiveEquals(type, policySource.Type) && arg == policySource.Arg; - }); - - if (source == sources.end()) - { - return std::nullopt; - } - - return *source; - } - - template - bool IsSourceInPolicy(std::string_view name, std::string_view type, std::string_view arg) - { - return FindSourceInPolicy

(name, type, arg).has_value(); - } - - // Checks whether the Group Policy allows this user source. - // If it does it returns None, otherwise it returns which policy is blocking it. - // Note that this applies to user sources that are being added as well as user sources - // that already existed when the Group Policy came into effect. - TogglePolicy::Policy GetPolicyBlockingUserSource(std::string_view name, std::string_view type, std::string_view arg, bool isTombstone) - { - // Reasons for not allowing: - // 1. The source is a tombstone for default source that is explicitly enabled - // 2. The source is a default source that is disabled - // 3. The source has the same name as a default source that is explicitly enabled (to prevent shadowing) - // 4. Allowed sources are disabled, blocking all user sources - // 5. There is an explicit list of allowed sources and this source is not in it - // - // We don't need to check sources added by policy as those have higher priority. - // - // Use the name and arg to match sources as we don't have the identifier before adding. - - // Case 1: - // The source is a tombstone and we need the policy to be explicitly enabled. - if (isTombstone) - { - if (name == s_Source_WingetCommunityDefault_Name && IsWingetCommunityDefaultSourceEnabled(true)) - { - return TogglePolicy::Policy::DefaultSource; - } - - if (name == s_Source_WingetMSStoreDefault_Name && IsWingetMSStoreDefaultSourceEnabled(true)) - { - return TogglePolicy::Policy::MSStoreSource; - } - - // Any other tombstone is allowed - return TogglePolicy::Policy::None; - } - - // Case 2: - // - The source is not a tombstone and we don't need the policy to be explicitly enabled. - // - Check only against the source argument and type as the user source may have a different name. - // - Do a case insensitive check as the domain portion of the URL is case insensitive, - // and we don't need case sensitivity for the rest as we control the domain. - if (Utility::CaseInsensitiveEquals(arg, s_Source_WingetCommunityDefault_Arg) && - Utility::CaseInsensitiveEquals(type, Microsoft::PreIndexedPackageSourceFactory::Type())) - { - return IsWingetCommunityDefaultSourceEnabled(false) ? TogglePolicy::Policy::None : TogglePolicy::Policy::DefaultSource; - } - - if (Utility::CaseInsensitiveEquals(arg, s_Source_WingetMSStoreDefault_Arg) && - Utility::CaseInsensitiveEquals(type, Microsoft::PreIndexedPackageSourceFactory::Type())) - { - return IsWingetMSStoreDefaultSourceEnabled(false) ? TogglePolicy::Policy::None : TogglePolicy::Policy::MSStoreSource; - } - - // Case 3: - // If the source has the same name as a default source, it is shadowing with a different argument - // (as it didn't match above). We only care if Group Policy requires the default source. - if (name == s_Source_WingetCommunityDefault_Name && IsWingetCommunityDefaultSourceEnabled(true)) - { - AICLI_LOG(Repo, Warning, << "User source is not allowed as it shadows the default source. Name [" << name << "]. Arg [" << arg << "] Type [" << type << ']'); - return TogglePolicy::Policy::DefaultSource; - } - - if (name == s_Source_WingetMSStoreDefault_Name && IsWingetMSStoreDefaultSourceEnabled(true)) - { - AICLI_LOG(Repo, Warning, << "User source is not allowed as it shadows the default MS Store source. Name [" << name << "]. Arg [" << arg << "] Type [" << type << ']'); - return TogglePolicy::Policy::MSStoreSource; - } - - // Case 4: - // The guard in the source add command should already block adding. - // This check drops existing user sources. - auto allowedSourcesPolicy = GroupPolicies().GetState(TogglePolicy::Policy::AllowedSources); - if (allowedSourcesPolicy == PolicyState::Disabled) - { - AICLI_LOG(Repo, Warning, << "User sources are disabled by Group Policy"); - return TogglePolicy::Policy::AllowedSources; - } - - // Case 5: - if (allowedSourcesPolicy == PolicyState::Enabled) - { - if (!IsSourceInPolicy(name, type, arg)) - { - AICLI_LOG(Repo, Warning, << "Source is not in the Group Policy allowed list. Name [" << name << "]. Arg [" << arg << "] Type [" << type << ']'); - return TogglePolicy::Policy::AllowedSources; - } - } - - return TogglePolicy::Policy::None; - } - - bool IsUserSourceAllowedByPolicy(std::string_view name, std::string_view type, std::string_view arg, bool isTombstone) - { - return GetPolicyBlockingUserSource(name, type, arg, isTombstone) == TogglePolicy::Policy::None; - } - - void EnsureSourceIsRemovable(const SourceDetailsInternal& source) - { - // Block removing sources added by Group Policy - if (source.Origin == SourceOrigin::GroupPolicy) - { - AICLI_LOG(Repo, Error, << "Cannot remove source added by Group Policy"); - throw GroupPolicyException(TogglePolicy::Policy::AdditionalSources); - } - - // Block removing default sources required by Group Policy. - if (source.Origin == SourceOrigin::Default) - { - if (GroupPolicies().GetState(TogglePolicy::Policy::DefaultSource) == PolicyState::Enabled && - source.Identifier == s_Source_WingetCommunityDefault_Identifier) - { - throw GroupPolicyException(TogglePolicy::Policy::DefaultSource); - } - - if (GroupPolicies().GetState(TogglePolicy::Policy::MSStoreSource) == PolicyState::Enabled && - source.Identifier == s_Source_WingetMSStoreDefault_Identifier) - { - throw GroupPolicyException(TogglePolicy::Policy::MSStoreSource); - } - } - } + constexpr std::string_view s_SourcesYaml_Sources = "Sources"sv; + constexpr std::string_view s_SourcesYaml_Source_Name = "Name"sv; + constexpr std::string_view s_SourcesYaml_Source_Type = "Type"sv; + constexpr std::string_view s_SourcesYaml_Source_Arg = "Arg"sv; + constexpr std::string_view s_SourcesYaml_Source_Data = "Data"sv; + constexpr std::string_view s_SourcesYaml_Source_Identifier = "Identifier"sv; + constexpr std::string_view s_SourcesYaml_Source_IsTombstone = "IsTombstone"sv; + + constexpr std::string_view s_MetadataYaml_Sources = "Sources"sv; + constexpr std::string_view s_MetadataYaml_Source_Name = "Name"sv; + constexpr std::string_view s_MetadataYaml_Source_LastUpdate = "LastUpdate"sv; + constexpr std::string_view s_MetadataYaml_Source_AcceptedAgreementsIdentifier = "AcceptedAgreementsIdentifier"sv; + constexpr std::string_view s_MetadataYaml_Source_AcceptedAgreementFields = "AcceptedAgreementFields"sv; + + constexpr std::string_view s_Source_WingetCommunityDefault_Name = "winget"sv; + constexpr std::string_view s_Source_WingetCommunityDefault_Arg = "https://winget.azureedge.net/cache"sv; + constexpr std::string_view s_Source_WingetCommunityDefault_Data = "Microsoft.Winget.Source_8wekyb3d8bbwe"sv; + constexpr std::string_view s_Source_WingetCommunityDefault_Identifier = "Microsoft.Winget.Source_8wekyb3d8bbwe"sv; + + constexpr std::string_view s_Source_MSStoreDefault_Name = "msstore"sv; + constexpr std::string_view s_Source_MSStoreDefault_Arg = "https://storeedgefd.dsx.mp.microsoft.com/v9.0"sv; + constexpr std::string_view s_Source_MSStoreDefault_Identifier = "StoreEdgeFD"sv; + + constexpr std::string_view s_Source_DesktopFrameworks_Name = "microsoft.builtin.desktop.frameworks"sv; + constexpr std::string_view s_Source_DesktopFrameworks_Arg = "https://winget.azureedge.net/platform"sv; + constexpr std::string_view s_Source_DesktopFrameworks_Data = "Microsoft.Winget.Platform.Source_8wekyb3d8bbwe"sv; + constexpr std::string_view s_Source_DesktopFrameworks_Identifier = "Microsoft.Winget.Platform.Source_8wekyb3d8bbwe"sv; // Attempts to read a single scalar value from the node. template @@ -309,11 +127,11 @@ namespace AppInstaller::Repository // Gets the source details from a particular setting, or an empty optional if no setting exists. std::optional> TryGetSourcesFromSetting( - Settings::Stream& stream, + Settings::Stream& setting, std::string_view rootName, std::function parse) { - auto sourcesStream = stream.Get(); + auto sourcesStream = setting.Get(); if (!sourcesStream) { // Note that this case is different than the one in which all sources have been removed. @@ -322,138 +140,22 @@ namespace AppInstaller::Repository else { std::vector result; - THROW_HR_IF(APPINSTALLER_CLI_ERROR_SOURCES_INVALID, !TryReadSourceDetails(stream.Definition().Path, *sourcesStream, rootName, parse, result)); + THROW_HR_IF(APPINSTALLER_CLI_ERROR_SOURCES_INVALID, !TryReadSourceDetails(setting.GetName(), *sourcesStream, rootName, parse, result)); return result; } } // Gets the source details from a particular setting. std::vector GetSourcesFromSetting( - Settings::Stream& stream, + Settings::Stream& setting, std::string_view rootName, std::function parse) { - return TryGetSourcesFromSetting(stream, rootName, parse).value_or(std::vector{}); - } - - // Gets the metadata. - std::vector GetMetadata(Settings::Stream& stream) - { - return GetSourcesFromSetting( - stream, - s_MetadataYaml_Sources, - [&](SourceDetailsInternal& details, const std::string& settingValue, const YAML::Node& source) - { - std::string_view name = Settings::Stream::SourcesMetadata.Path; - if (!TryReadScalar(name, settingValue, source, s_MetadataYaml_Source_Name, details.Name)) { return false; } - int64_t lastUpdateInEpoch{}; - if (!TryReadScalar(name, settingValue, source, s_MetadataYaml_Source_LastUpdate, lastUpdateInEpoch)) { return false; } - details.LastUpdateTime = Utility::ConvertUnixEpochToSystemClock(lastUpdateInEpoch); - return true; - }); - } - - // Gets the sources from a particular origin. - std::vector GetSourcesByOrigin(SourceOrigin origin, Settings::Stream& userSettingStream) - { - std::vector result; - - switch (origin) - { - case SourceOrigin::Default: - { - if (IsWingetCommunityDefaultSourceEnabled()) - { - SourceDetailsInternal details; - details.Name = s_Source_WingetCommunityDefault_Name; - details.Type = Microsoft::PreIndexedPackageSourceFactory::Type(); - details.Arg = s_Source_WingetCommunityDefault_Arg; - details.Data = s_Source_WingetCommunityDefault_Data; - details.Identifier = s_Source_WingetCommunityDefault_Identifier; - details.TrustLevel = SourceTrustLevel::Trusted | SourceTrustLevel::StoreOrigin; - result.emplace_back(std::move(details)); - } - - if (IsWingetMSStoreDefaultSourceEnabled()) - { - SourceDetailsInternal details; - details.Name = s_Source_WingetMSStoreDefault_Name; - details.Type = Microsoft::PreIndexedPackageSourceFactory::Type(); - details.Arg = s_Source_WingetMSStoreDefault_Arg; - details.Data = s_Source_WingetMSStoreDefault_Data; - details.Identifier = s_Source_WingetMSStoreDefault_Identifier; - details.TrustLevel = SourceTrustLevel::Trusted | SourceTrustLevel::StoreOrigin; - result.emplace_back(std::move(details)); - } - } - break; - case SourceOrigin::User: - { - std::vector userSources = GetSourcesFromSetting( - userSettingStream, - s_SourcesYaml_Sources, - [&](SourceDetailsInternal& details, const std::string& settingValue, const YAML::Node& source) - { - std::string_view name = Settings::Stream::UserSources.Path; - if (!TryReadScalar(name, settingValue, source, s_SourcesYaml_Source_Name, details.Name)) { return false; } - if (!TryReadScalar(name, settingValue, source, s_SourcesYaml_Source_Type, details.Type)) { return false; } - if (!TryReadScalar(name, settingValue, source, s_SourcesYaml_Source_Arg, details.Arg)) { return false; } - if (!TryReadScalar(name, settingValue, source, s_SourcesYaml_Source_Data, details.Data)) { return false; } - if (!TryReadScalar(name, settingValue, source, s_SourcesYaml_Source_IsTombstone, details.IsTombstone)) { return false; } - TryReadScalar(name, settingValue, source, s_SourcesYaml_Source_Identifier, details.Identifier); - return true; - }); - - for (auto& source : userSources) - { - // Check source against list of allowed sources and drop tombstones for required sources - if (!IsUserSourceAllowedByPolicy(source.Name, source.Type, source.Arg, source.IsTombstone)) - { - AICLI_LOG(Repo, Warning, << "User source " << source.Name << " dropped because of group policy"); - continue; - } - - result.emplace_back(std::move(source)); - } - } - break; - case SourceOrigin::GroupPolicy: - { - if (GroupPolicies().GetState(TogglePolicy::Policy::AdditionalSources) == PolicyState::Enabled) - { - auto additionalSourcesOpt = GroupPolicies().GetValueRef(); - if (additionalSourcesOpt.has_value()) - { - const auto& additionalSources = additionalSourcesOpt->get(); - for (const auto& additionalSource : additionalSources) - { - SourceDetailsInternal details; - details.Name = additionalSource.Name; - details.Type = additionalSource.Type; - details.Arg = additionalSource.Arg; - details.Data = additionalSource.Data; - details.Identifier = additionalSource.Identifier; - details.Origin = SourceOrigin::GroupPolicy; - result.emplace_back(std::move(details)); - } - } - } - } - break; - default: - THROW_HR(E_UNEXPECTED); - } - - for (auto& source : result) - { - source.Origin = origin; - } - - return result; + return TryGetSourcesFromSetting(setting, rootName, parse).value_or(std::vector{}); } // Sets the sources for a particular setting, from a particular origin. - bool SetSourcesToSettingWithFilter(Settings::Stream& stream, SourceOrigin origin, const std::vector& sources) + [[nodiscard]] bool SetSourcesToSettingWithFilter(Settings::Stream& setting, SourceOrigin origin, const std::vector& sources) { YAML::Emitter out; out << YAML::BeginMap; @@ -478,55 +180,319 @@ namespace AppInstaller::Repository out << YAML::EndSeq; out << YAML::EndMap; - return stream.Set(out.str()); + return setting.Set(out.str()); } - // Sets the metadata only (which is not a secure setting and can be set unprivileged) - void SetMetadata(const std::vector& sources) + // Assumes that names match already + bool DoSourceDetailsInternalMatch(const SourceDetailsInternal& left, const SourceDetailsInternal& right) { - YAML::Emitter out; - out << YAML::BeginMap; - out << YAML::Key << s_MetadataYaml_Sources; - out << YAML::BeginSeq; + return left.Arg == right.Arg && + left.Identifier == right.Identifier && + Utility::CaseInsensitiveEquals(left.Type, right.Type); + } + } - for (const auto& details : sources) + std::string_view GetWellKnownSourceName(WellKnownSource source) + { + switch (source) + { + case WellKnownSource::WinGet: + return s_Source_WingetCommunityDefault_Name; + case WellKnownSource::MicrosoftStore: + return s_Source_MSStoreDefault_Name; + case WellKnownSource::DesktopFrameworks: + return s_Source_DesktopFrameworks_Name; + } + + return {}; + } + + std::string_view GetWellKnownSourceArg(WellKnownSource source) + { + switch (source) + { + case WellKnownSource::WinGet: + return s_Source_WingetCommunityDefault_Arg; + case WellKnownSource::MicrosoftStore: + return s_Source_MSStoreDefault_Arg; + case WellKnownSource::DesktopFrameworks: + return s_Source_DesktopFrameworks_Arg; + } + + return {}; + } + + std::string_view GetWellKnownSourceIdentifier(WellKnownSource source) + { + switch (source) + { + case WellKnownSource::WinGet: + return s_Source_WingetCommunityDefault_Identifier; + case WellKnownSource::MicrosoftStore: + return s_Source_MSStoreDefault_Identifier; + case WellKnownSource::DesktopFrameworks: + return s_Source_DesktopFrameworks_Identifier; + } + + return {}; + } + + SourceDetailsInternal GetWellKnownSourceDetailsInternal(WellKnownSource source) + { + switch (source) + { + case WellKnownSource::WinGet: + { + SourceDetailsInternal details; + details.Origin = SourceOrigin::Default; + details.Name = s_Source_WingetCommunityDefault_Name; + details.Type = Microsoft::PreIndexedPackageSourceFactory::Type(); + details.Arg = s_Source_WingetCommunityDefault_Arg; + details.Data = s_Source_WingetCommunityDefault_Data; + details.Identifier = s_Source_WingetCommunityDefault_Identifier; + details.TrustLevel = SourceTrustLevel::Trusted | SourceTrustLevel::StoreOrigin; + return details; + } + case WellKnownSource::MicrosoftStore: + { + SourceDetailsInternal details; + details.Origin = SourceOrigin::Default; + details.Name = s_Source_MSStoreDefault_Name; + details.Type = Rest::RestSourceFactory::Type(); + details.Arg = s_Source_MSStoreDefault_Arg; + details.Identifier = s_Source_MSStoreDefault_Identifier; + details.TrustLevel = SourceTrustLevel::Trusted; + details.SupportInstalledSearchCorrelation = false; + return details; + } + case WellKnownSource::DesktopFrameworks: + { + SourceDetailsInternal details; + details.Origin = SourceOrigin::Default; + details.Name = s_Source_DesktopFrameworks_Name; + details.Type = Microsoft::PreIndexedPackageSourceFactory::Type(); + details.Arg = s_Source_DesktopFrameworks_Arg; + details.Data = s_Source_DesktopFrameworks_Data; + details.Identifier = s_Source_DesktopFrameworks_Identifier; + details.TrustLevel = SourceTrustLevel::Trusted | SourceTrustLevel::StoreOrigin; + // Cheat the system and call this a tombstone. This effectively hides it from everything outside + // of this file, while still allowing it to properly save metadata. There might be problems + // if someone chooses the exact same name as this, which is why its name is very long. + // TODO: When refactoring the source interface, handle this with Visibility or similar. + details.IsTombstone = true; + return details; + } + } + + THROW_HR(E_UNEXPECTED); + } + + SourceList::SourceList() : m_userSourcesStream(Stream::UserSources), m_metadataStream(Stream::SourcesMetadata) + { + OverwriteSourceList(); + OverwriteMetadata(); + } + + std::vector> SourceList::GetCurrentSourceRefs() + { + std::vector> result; + + for (auto& s : m_sourceList) + { + if (!s.IsTombstone) + { + result.emplace_back(std::ref(s)); + } + else { - out << YAML::BeginMap; - out << YAML::Key << s_MetadataYaml_Source_Name << YAML::Value << details.Name; - out << YAML::Key << s_MetadataYaml_Source_LastUpdate << YAML::Value << Utility::ConvertSystemClockToUnixEpoch(details.LastUpdateTime); - out << YAML::EndMap; + AICLI_LOG(Repo, Info, << "GetCurrentSourceRefs: Source named '" << s.Name << "' from origin " << ToString(s.Origin) << " is a tombstone and is dropped."); } + } - out << YAML::EndSeq; - out << YAML::EndMap; + return result; + } + + auto SourceList::FindSource(std::string_view name, bool includeHidden) + { + return std::find_if(m_sourceList.begin(), m_sourceList.end(), + [name, includeHidden](const SourceDetailsInternal& sd) + { + return Utility::ICUCaseInsensitiveEquals(sd.Name, name) && + (includeHidden || (!sd.IsTombstone && sd.Origin != SourceOrigin::Metadata)); + }); + } + + SourceDetailsInternal* SourceList::GetCurrentSource(std::string_view name) + { + auto itr = FindSource(name); + return itr == m_sourceList.end() ? nullptr : &(*itr); + } + + SourceDetailsInternal* SourceList::GetSource(std::string_view name) + { + auto itr = FindSource(name, true); + return itr == m_sourceList.end() ? nullptr : &(*itr); + } + + void SourceList::AddSource(const SourceDetailsInternal& details) + { + bool sourcesSet = false; + + for (size_t i = 0; !sourcesSet && i < 10; ++i) + { + auto currentSource = GetCurrentSource(details.Name); + THROW_HR_IF(APPINSTALLER_CLI_ERROR_SOURCE_NAME_ALREADY_EXISTS, currentSource != nullptr && !DoSourceDetailsInternalMatch(details, *currentSource)); + + // Check for a hidden source data that we don't want to collide. + // TODO: Refactor the source interface so that we don't do this + auto itr = FindSource(details.Name, true); + THROW_HR_IF(APPINSTALLER_CLI_ERROR_SOURCE_NAME_ALREADY_EXISTS, + itr != m_sourceList.end() && itr->Origin != SourceOrigin::User && itr->Origin != SourceOrigin::Metadata); - Settings::SetSetting(Settings::Stream::SourcesMetadata, out.str()); + // Erase the source's entry if applicable + if (itr != m_sourceList.end()) + { + m_sourceList.erase(itr); + } + + m_sourceList.emplace_back(details); + + sourcesSet = SetSourcesByOrigin(SourceOrigin::User, m_sourceList); + + if (!sourcesSet) + { + OverwriteSourceList(); + OverwriteMetadata(); + } } - // Sets the sources for a given origin. - bool SetSourcesByOrigin(SourceOrigin origin, Settings::Stream& userSettingStream, const std::vector& sources) + THROW_HR_IF_MSG(E_UNEXPECTED, !sourcesSet, "Too many attempts at SetSourcesByOrigin"); + + SaveMetadataInternal(details); + } + + void SourceList::RemoveSource(const SourceDetailsInternal& details) + { + bool sourcesSet = false; + + for (size_t i = 0; !sourcesSet && i < 10; ++i) { - switch (origin) + switch (details.Origin) { + case SourceOrigin::Default: + { + auto target = FindSource(details.Name, true); + if (target == m_sourceList.end()) + { + THROW_HR_MSG(E_UNEXPECTED, "Default source not in SourceList"); + } + + if (!target->IsTombstone) + { + SourceDetailsInternal tombstone; + tombstone.Name = details.Name; + tombstone.IsTombstone = true; + tombstone.Origin = SourceOrigin::User; + m_sourceList.emplace_back(std::move(tombstone)); + } + } + break; case SourceOrigin::User: - if (!SetSourcesToSettingWithFilter(userSettingStream, SourceOrigin::User, sources)) + { + auto target = FindSource(details.Name); + if (target == m_sourceList.end()) { - return false; + // Assumed that an update to the sources removed it first + return; } + + m_sourceList.erase(target); + } break; + case SourceOrigin::GroupPolicy: + // This should have already been blocked higher up. + AICLI_LOG(Repo, Error, << "Attempting to remove Group Policy source: " << details.Name); + THROW_HR(E_UNEXPECTED); default: THROW_HR(E_UNEXPECTED); } - return SetMetadata(sources); + sourcesSet = SetSourcesByOrigin(SourceOrigin::User, m_sourceList); + + if (!sourcesSet) + { + OverwriteSourceList(); + OverwriteMetadata(); + } + } + + THROW_HR_IF_MSG(E_UNEXPECTED, !sourcesSet, "Too many attempts at SetSourcesByOrigin"); + + SaveMetadataInternal(details, true); + } + + void SourceList::SaveMetadata(const SourceDetailsInternal& details) + { + SaveMetadataInternal(details); + } + + bool SourceList::CheckSourceAgreements(const SourceDetails& details) + { + auto agreementFields = GetAgreementFieldsFromSourceInformation(details.Information); + + if (agreementFields == ImplicitAgreementFieldEnum::None && details.Information.SourceAgreementsIdentifier.empty()) + { + // No agreements to be accepted. + return true; + } + + auto detailsInternal = GetCurrentSource(details.Name); + if (!detailsInternal) + { + // Source not found. + return false; } + + return static_cast(agreementFields) == detailsInternal->AcceptedAgreementFields && + details.Information.SourceAgreementsIdentifier == detailsInternal->AcceptedAgreementsIdentifier; + } + + void SourceList::SaveAcceptedSourceAgreements(const SourceDetails& details) + { + auto agreementFields = GetAgreementFieldsFromSourceInformation(details.Information); + + if (agreementFields == ImplicitAgreementFieldEnum::None && details.Information.SourceAgreementsIdentifier.empty()) + { + // No agreements to be accepted. + return; + } + + auto detailsInternal = GetCurrentSource(details.Name); + if (!detailsInternal) + { + // No source to update. + return; + } + + detailsInternal->AcceptedAgreementFields = static_cast(agreementFields); + detailsInternal->AcceptedAgreementsIdentifier = details.Information.SourceAgreementsIdentifier; + + SaveMetadataInternal(details); + } + + void SourceList::RemoveSettingsStreams() + { + Stream{ Stream::UserSources }.Remove(); + Stream{ Stream::SourcesMetadata }.Remove(); } - SourceList::SourceList() : m_userSourcesStream(Settings::Stream::UserSources), m_metadataStream(Settings::Stream::SourcesMetadata) + void SourceList::OverwriteSourceList() { + m_sourceList.clear(); + for (SourceOrigin origin : { SourceOrigin::GroupPolicy, SourceOrigin::User, SourceOrigin::Default }) { - auto forOrigin = GetSourcesByOrigin(origin, m_userSourcesStream); + auto forOrigin = GetSourcesByOrigin(origin); for (auto&& source : forOrigin) { @@ -543,103 +509,197 @@ namespace AppInstaller::Repository } } } + } - auto metadata = GetMetadata(m_metadataStream); - for (const auto& metaSource : metadata) + void SourceList::OverwriteMetadata() + { + auto metadata = GetMetadata(); + for (auto& metaSource : metadata) { auto source = GetSource(metaSource.Name); if (source) { source->LastUpdateTime = metaSource.LastUpdateTime; + source->AcceptedAgreementFields = metaSource.AcceptedAgreementFields; + source->AcceptedAgreementsIdentifier = metaSource.AcceptedAgreementsIdentifier; + } + else + { + m_sourceList.emplace_back(std::move(metaSource)); } } } - std::vector> SourceList::GetCurrentSourceRefs() + // Gets the sources from a particular origin. + std::vector SourceList::GetSourcesByOrigin(SourceOrigin origin) { - std::vector> result; + std::vector result; - for (auto& s : m_sourceList) + switch (origin) { - if (!s.IsTombstone) + case SourceOrigin::Default: + { + if (IsWellKnownSourceEnabled(WellKnownSource::MicrosoftStore)) { - result.emplace_back(std::ref(s)); + result.emplace_back(GetWellKnownSourceDetailsInternal(WellKnownSource::MicrosoftStore)); } - else + + if (IsWellKnownSourceEnabled(WellKnownSource::WinGet)) { - AICLI_LOG(Repo, Info, << "GetCurrentSourceRefs: Source named '" << s.Name << "' from origin " << ToString(s.Origin) << " is a tombstone and is dropped."); + result.emplace_back(GetWellKnownSourceDetailsInternal(WellKnownSource::WinGet)); } + + // Since we are using the tombstone trick, this is added just to have the source in the internal + // list for tracking updates. Thus there is no need to check a policy. + result.emplace_back(GetWellKnownSourceDetailsInternal(WellKnownSource::DesktopFrameworks)); } + break; + case SourceOrigin::User: + { + std::vector userSources = GetSourcesFromSetting( + m_userSourcesStream, + s_SourcesYaml_Sources, + [&](SourceDetailsInternal& details, const std::string& settingValue, const YAML::Node& source) + { + std::string_view name = m_userSourcesStream.GetName(); + if (!TryReadScalar(name, settingValue, source, s_SourcesYaml_Source_Name, details.Name)) { return false; } + if (!TryReadScalar(name, settingValue, source, s_SourcesYaml_Source_Type, details.Type)) { return false; } + if (!TryReadScalar(name, settingValue, source, s_SourcesYaml_Source_Arg, details.Arg)) { return false; } + if (!TryReadScalar(name, settingValue, source, s_SourcesYaml_Source_Data, details.Data)) { return false; } + if (!TryReadScalar(name, settingValue, source, s_SourcesYaml_Source_IsTombstone, details.IsTombstone)) { return false; } + TryReadScalar(name, settingValue, source, s_SourcesYaml_Source_Identifier, details.Identifier, false); + return true; + }); - return result; - } + for (auto& source : userSources) + { + // Check source against list of allowed sources and drop tombstones for required sources + if (!IsUserSourceAllowedByPolicy(source.Name, source.Type, source.Arg, source.IsTombstone)) + { + AICLI_LOG(Repo, Warning, << "User source " << source.Name << " dropped because of group policy"); + continue; + } - auto SourceList::FindSource(std::string_view name, bool includeTombstone) - { - return std::find_if(m_sourceList.begin(), m_sourceList.end(), - [name, includeTombstone](const SourceDetailsInternal& sd) + result.emplace_back(std::move(source)); + } + } + break; + case SourceOrigin::GroupPolicy: + { + if (GroupPolicies().GetState(TogglePolicy::Policy::AdditionalSources) == PolicyState::Enabled) { - return Utility::ICUCaseInsensitiveEquals(sd.Name, name) && - (!sd.IsTombstone || includeTombstone); - }); + auto additionalSourcesOpt = GroupPolicies().GetValueRef(); + if (additionalSourcesOpt.has_value()) + { + const auto& additionalSources = additionalSourcesOpt->get(); + for (const auto& additionalSource : additionalSources) + { + SourceDetailsInternal details; + details.Name = additionalSource.Name; + details.Type = additionalSource.Type; + details.Arg = additionalSource.Arg; + details.Data = additionalSource.Data; + details.Identifier = additionalSource.Identifier; + details.Origin = SourceOrigin::GroupPolicy; + result.emplace_back(std::move(details)); + } + } + } + } + break; + default: + THROW_HR(E_UNEXPECTED); + } + + for (auto& source : result) + { + source.Origin = origin; + } + + return result; } - SourceDetailsInternal* SourceList::GetCurrentSource(std::string_view name) + bool SourceList::SetSourcesByOrigin(SourceOrigin origin, const std::vector& sources) { - auto itr = FindSource(name); - return itr == m_sourceList.end() ? nullptr : &(*itr); + switch (origin) + { + case SourceOrigin::User: + return SetSourcesToSettingWithFilter(m_userSourcesStream, SourceOrigin::User, sources); + } + + THROW_HR(E_UNEXPECTED); } - SourceDetailsInternal* SourceList::GetSource(std::string_view name) + std::vector SourceList::GetMetadata() { - auto itr = FindSource(name, true); - return itr == m_sourceList.end() ? nullptr : &(*itr); + return GetSourcesFromSetting( + m_metadataStream, + s_MetadataYaml_Sources, + [&](SourceDetailsInternal& details, const std::string& settingValue, const YAML::Node& source) + { + details.Origin = SourceOrigin::Metadata; + std::string_view name = m_metadataStream.GetName(); + if (!TryReadScalar(name, settingValue, source, s_MetadataYaml_Source_Name, details.Name)) { return false; } + int64_t lastUpdateInEpoch{}; + if (!TryReadScalar(name, settingValue, source, s_MetadataYaml_Source_LastUpdate, lastUpdateInEpoch)) { return false; } + details.LastUpdateTime = Utility::ConvertUnixEpochToSystemClock(lastUpdateInEpoch); + TryReadScalar(name, settingValue, source, s_MetadataYaml_Source_AcceptedAgreementsIdentifier, details.AcceptedAgreementsIdentifier, false); + TryReadScalar(name, settingValue, source, s_MetadataYaml_Source_AcceptedAgreementFields, details.AcceptedAgreementFields, false); + return true; + }); } - void SourceList::AddSource(const SourceDetailsInternal& details) + bool SourceList::SetMetadata(const std::vector& sources) { - // Erase the source's tombstone entry if applicable - auto itr = FindSource(details.Name, true); - if (itr != m_sourceList.end()) + YAML::Emitter out; + out << YAML::BeginMap; + out << YAML::Key << s_MetadataYaml_Sources; + out << YAML::BeginSeq; + + for (const auto& details : sources) { - THROW_HR_IF(HRESULT_FROM_WIN32(ERROR_INVALID_STATE), !itr->IsTombstone); - m_sourceList.erase(itr); + out << YAML::BeginMap; + out << YAML::Key << s_MetadataYaml_Source_Name << YAML::Value << details.Name; + out << YAML::Key << s_MetadataYaml_Source_LastUpdate << YAML::Value << Utility::ConvertSystemClockToUnixEpoch(details.LastUpdateTime); + out << YAML::Key << s_MetadataYaml_Source_AcceptedAgreementsIdentifier << YAML::Value << details.AcceptedAgreementsIdentifier; + out << YAML::Key << s_MetadataYaml_Source_AcceptedAgreementFields << YAML::Value << details.AcceptedAgreementFields; + out << YAML::EndMap; } - m_sourceList.emplace_back(details); + out << YAML::EndSeq; + out << YAML::EndMap; - SetSourcesByOrigin(SourceOrigin::User, m_sourceList); + return m_metadataStream.Set(out.str()); } - void SourceList::RemoveSource(const SourceDetailsInternal& source) + void SourceList::SaveMetadataInternal(const SourceDetailsInternal& details, bool remove) { - switch (source.Origin) - { - case SourceOrigin::Default: + bool metadataSet = false; + + for (size_t i = 0; !metadataSet && i < 10; ++i) { - SourceDetailsInternal tombstone; - tombstone.Name = source.Name; - tombstone.IsTombstone = true; - tombstone.Origin = SourceOrigin::User; - m_sourceList.emplace_back(std::move(tombstone)); - } - break; - case SourceOrigin::User: - m_sourceList.erase(FindSource(source.Name)); - break; - case SourceOrigin::GroupPolicy: - // This should have already been blocked higher up. - AICLI_LOG(Repo, Error, << "Attempting to remove Group Policy source: " << source.Name); - THROW_HR(E_UNEXPECTED); - default: - THROW_HR(E_UNEXPECTED); - } + metadataSet = SetMetadata(m_sourceList); - SetSourcesByOrigin(SourceOrigin::User, m_sourceList); - } + if (!metadataSet) + { + OverwriteMetadata(); - void SourceList::SaveMetadata() const - { - SetMetadata(m_sourceList); + // The remove will have removed the source but not the metadata. + // Remove it again here. + if (remove) + { + auto target = FindSource(details.Name, true); + if (target == m_sourceList.end()) + { + // Didn't find the metadata, so we consider this a success + return; + } + + m_sourceList.erase(target); + } + } + } + + THROW_HR_IF_MSG(E_UNEXPECTED, !metadataSet, "Too many attempts at SetMetadata"); } } diff --git a/src/AppInstallerRepositoryCore/SourceList.h b/src/AppInstallerRepositoryCore/SourceList.h index 7df090c42d..1c5a5a2eec 100644 --- a/src/AppInstallerRepositoryCore/SourceList.h +++ b/src/AppInstallerRepositoryCore/SourceList.h @@ -8,21 +8,25 @@ namespace AppInstaller::Repository { // Built-in values for default sources - std::string_view WingetCommunityDefaultSourceName(); - std::string_view WingetCommunityDefaultSourceArg(); - std::string_view WingetCommunityDefaultSourceId(); - - std::string_view WingetMSStoreDefaultSourceName(); - std::string_view WingetMSStoreDefaultSourceArg(); - std::string_view WingetMSStoreDefaultSourceId(); + std::string_view GetWellKnownSourceName(WellKnownSource source); + std::string_view GetWellKnownSourceArg(WellKnownSource source); + std::string_view GetWellKnownSourceIdentifier(WellKnownSource source); // SourceDetails with additional data used internally. struct SourceDetailsInternal : public SourceDetails { + SourceDetailsInternal() = default; + SourceDetailsInternal(const SourceDetails& details) : SourceDetails(details) {} + // If true, this is a tombstone, marking the deletion of a source at a lower priority origin. bool IsTombstone = false; + std::string AcceptedAgreementsIdentifier; + int AcceptedAgreementFields = 0; }; + // Gets the internal details for a well known source. + SourceDetailsInternal GetWellKnownSourceDetailsInternal(WellKnownSource source); + // Struct containing internal implementation of the source list. // This contains all source data; including tombstoned sources. struct SourceList @@ -40,15 +44,43 @@ namespace AppInstaller::Repository SourceDetailsInternal* GetSource(std::string_view name); // Add/remove a current source - void AddSource(const SourceDetailsInternal& source); - void RemoveSource(const SourceDetailsInternal& source); + void AddSource(const SourceDetailsInternal& details); + void RemoveSource(const SourceDetailsInternal& details); + + // Save source metadata; the particular source with the metadata update is given. + // The given source must already be in the internal source list. + void SaveMetadata(const SourceDetailsInternal& details); + + // Checks the source agreements and returns if agreements are satisfied. + bool CheckSourceAgreements(const SourceDetails& details); + + // Save agreements information. + void SaveAcceptedSourceAgreements(const SourceDetails& details); - // Save source metadata. Currently only LastTimeUpdated is used. - void SaveMetadata() const; + // Removes all settings streams associated with the source list. + // Implements `winget source reset --force`. + static void RemoveSettingsStreams(); private: + // Overwrites the source list with all sources. + void OverwriteSourceList(); + + // Overwrites the source list with the current metadata. + void OverwriteMetadata(); + // calls std::find_if and return the iterator. - auto FindSource(std::string_view name, bool includeTombstone = false); + auto FindSource(std::string_view name, bool includeHidden = false); + + std::vector GetSourcesByOrigin(SourceOrigin origin); + // Does *NOT* set metadata; call SaveMetadataInternal afterward. + [[nodiscard]] bool SetSourcesByOrigin(SourceOrigin origin, const std::vector& sources); + + std::vector GetMetadata(); + [[nodiscard]] bool SetMetadata(const std::vector& sources); + + // Save source metadata; the particular source with the metadata update is given. + // If remove is true, the given source is being removed. + void SaveMetadataInternal(const SourceDetailsInternal& details, bool remove = false); std::vector m_sourceList; Settings::Stream m_userSourcesStream; diff --git a/src/AppInstallerRepositoryCore/SourcePolicy.cpp b/src/AppInstallerRepositoryCore/SourcePolicy.cpp index ba3a8ef031..a5324d1226 100644 --- a/src/AppInstallerRepositoryCore/SourcePolicy.cpp +++ b/src/AppInstallerRepositoryCore/SourcePolicy.cpp @@ -2,20 +2,19 @@ // Licensed under the MIT License. #include "pch.h" #include "SourcePolicy.h" -#include "SourceList.h" #include "Microsoft/PreIndexedPackageSourceFactory.h" -#include +#include "Rest/RestSourceFactory.h" + +using namespace AppInstaller::Settings; namespace AppInstaller::Repository { - using namespace Settings; - namespace { // Checks whether a default source is enabled with the current settings. // onlyExplicit determines whether we consider the not-configured state to be enabled or not. - bool IsDefaultSourceEnabled(std::string_view sourceToLog, ExperimentalFeature::Feature feature, bool onlyExplicit, TogglePolicy::Policy policy) + bool IsDefaultSourceEnabled(WellKnownSource sourceToLog, ExperimentalFeature::Feature feature, bool onlyExplicit, TogglePolicy::Policy policy) { if (!ExperimentalFeature::IsEnabled(feature)) { @@ -31,23 +30,13 @@ namespace AppInstaller::Repository if (!GroupPolicies().IsEnabled(policy)) { - AICLI_LOG(Repo, Info, << "The default source " << sourceToLog << " is disabled due to Group Policy"); + AICLI_LOG(Repo, Info, << "The default source " << GetWellKnownSourceName(sourceToLog) << " is disabled due to Group Policy"); return false; } return true; } - bool IsWingetCommunityDefaultSourceEnabled(bool onlyExplicit = false) - { - return IsDefaultSourceEnabled(WingetCommunityDefaultSourceName(), ExperimentalFeature::Feature::None, onlyExplicit, TogglePolicy::Policy::DefaultSource); - } - - bool IsWingetMSStoreDefaultSourceEnabled(bool onlyExplicit = false) - { - return IsDefaultSourceEnabled(WingetMSStoreDefaultSourceName(), ExperimentalFeature::Feature::ExperimentalMSStore, onlyExplicit, TogglePolicy::Policy::MSStoreSource); - } - template std::optional FindSourceInPolicy(std::string_view name, std::string_view type, std::string_view arg) { @@ -81,6 +70,10 @@ namespace AppInstaller::Repository } } + // Checks whether the Group Policy allows this user source. + // If it does it returns None, otherwise it returns which policy is blocking it. + // Note that this applies to user sources that are being added as well as user sources + // that already existed when the Group Policy came into effect. TogglePolicy::Policy GetPolicyBlockingUserSource(std::string_view name, std::string_view type, std::string_view arg, bool isTombstone) { // Reasons for not allowing: @@ -98,12 +91,12 @@ namespace AppInstaller::Repository // The source is a tombstone and we need the policy to be explicitly enabled. if (isTombstone) { - if (name == WingetCommunityDefaultSourceName() && IsWingetCommunityDefaultSourceEnabled(true)) + if (name == GetWellKnownSourceName(WellKnownSource::WinGet) && IsWellKnownSourceEnabled(WellKnownSource::WinGet, true)) { return TogglePolicy::Policy::DefaultSource; } - if (name == WingetMSStoreDefaultSourceName() && IsWingetMSStoreDefaultSourceEnabled(true)) + if (name == GetWellKnownSourceName(WellKnownSource::MicrosoftStore) && IsWellKnownSourceEnabled(WellKnownSource::MicrosoftStore, true)) { return TogglePolicy::Policy::MSStoreSource; } @@ -117,30 +110,30 @@ namespace AppInstaller::Repository // - Check only against the source argument and type as the user source may have a different name. // - Do a case insensitive check as the domain portion of the URL is case insensitive, // and we don't need case sensitivity for the rest as we control the domain. - if (Utility::CaseInsensitiveEquals(arg, WingetCommunityDefaultSourceArg()) && + if (Utility::CaseInsensitiveEquals(arg, GetWellKnownSourceArg(WellKnownSource::WinGet)) && Utility::CaseInsensitiveEquals(type, Microsoft::PreIndexedPackageSourceFactory::Type())) { - return IsWingetCommunityDefaultSourceEnabled(false) ? TogglePolicy::Policy::None : TogglePolicy::Policy::DefaultSource; + return IsWellKnownSourceEnabled(WellKnownSource::WinGet) ? TogglePolicy::Policy::None : TogglePolicy::Policy::DefaultSource; } - if (Utility::CaseInsensitiveEquals(arg, WingetMSStoreDefaultSourceArg()) && - Utility::CaseInsensitiveEquals(type, Microsoft::PreIndexedPackageSourceFactory::Type())) + if (Utility::CaseInsensitiveEquals(arg, GetWellKnownSourceArg(WellKnownSource::MicrosoftStore)) && + Utility::CaseInsensitiveEquals(type, Rest::RestSourceFactory::Type())) { - return IsWingetMSStoreDefaultSourceEnabled(false) ? TogglePolicy::Policy::None : TogglePolicy::Policy::MSStoreSource; + return IsWellKnownSourceEnabled(WellKnownSource::MicrosoftStore) ? TogglePolicy::Policy::None : TogglePolicy::Policy::MSStoreSource; } // Case 3: // If the source has the same name as a default source, it is shadowing with a different argument // (as it didn't match above). We only care if Group Policy requires the default source. - if (name == WingetCommunityDefaultSourceName() && IsWingetCommunityDefaultSourceEnabled(true)) + if (name == GetWellKnownSourceName(WellKnownSource::WinGet) && IsWellKnownSourceEnabled(WellKnownSource::WinGet, true)) { AICLI_LOG(Repo, Warning, << "User source is not allowed as it shadows the default source. Name [" << name << "]. Arg [" << arg << "] Type [" << type << ']'); return TogglePolicy::Policy::DefaultSource; } - if (name == WingetMSStoreDefaultSourceName() && IsWingetMSStoreDefaultSourceEnabled(true)) + if (name == GetWellKnownSourceName(WellKnownSource::MicrosoftStore) && IsWellKnownSourceEnabled(WellKnownSource::MicrosoftStore, true)) { - AICLI_LOG(Repo, Warning, << "User source is not allowed as it shadows the default MS Store source. Name [" << name << "]. Arg [" << arg << "] Type [" << type << ']'); + AICLI_LOG(Repo, Warning, << "User source is not allowed as it shadows a default MS Store source. Name [" << name << "]. Arg [" << arg << "] Type [" << type << ']'); return TogglePolicy::Policy::MSStoreSource; } @@ -166,4 +159,48 @@ namespace AppInstaller::Repository return TogglePolicy::Policy::None; } + + bool IsUserSourceAllowedByPolicy(std::string_view name, std::string_view type, std::string_view arg, bool isTombstone) + { + return GetPolicyBlockingUserSource(name, type, arg, isTombstone) == TogglePolicy::Policy::None; + } + + bool IsWellKnownSourceEnabled(WellKnownSource source, bool onlyExplicit) + { + switch (source) + { + case AppInstaller::Repository::WellKnownSource::WinGet: + return IsDefaultSourceEnabled(source, ExperimentalFeature::Feature::None, onlyExplicit, TogglePolicy::Policy::DefaultSource); + case AppInstaller::Repository::WellKnownSource::MicrosoftStore: + return IsDefaultSourceEnabled(source, ExperimentalFeature::Feature::None, onlyExplicit, TogglePolicy::Policy::MSStoreSource); + } + + return false; + } + + void EnsureSourceIsRemovable(const SourceDetailsInternal& source) + { + // Block removing sources added by Group Policy + if (source.Origin == SourceOrigin::GroupPolicy) + { + AICLI_LOG(Repo, Error, << "Cannot remove source added by Group Policy"); + throw GroupPolicyException(TogglePolicy::Policy::AdditionalSources); + } + + // Block removing default sources required by Group Policy. + if (source.Origin == SourceOrigin::Default) + { + if (GroupPolicies().GetState(TogglePolicy::Policy::DefaultSource) == PolicyState::Enabled && + source.Identifier == GetWellKnownSourceIdentifier(WellKnownSource::WinGet)) + { + throw GroupPolicyException(TogglePolicy::Policy::DefaultSource); + } + + if (GroupPolicies().GetState(TogglePolicy::Policy::MSStoreSource) == PolicyState::Enabled && + source.Identifier == GetWellKnownSourceIdentifier(WellKnownSource::MicrosoftStore)) + { + throw GroupPolicyException(TogglePolicy::Policy::MSStoreSource); + } + } + } } diff --git a/src/AppInstallerRepositoryCore/SourcePolicy.h b/src/AppInstallerRepositoryCore/SourcePolicy.h index b80de64703..535949c608 100644 --- a/src/AppInstallerRepositoryCore/SourcePolicy.h +++ b/src/AppInstallerRepositoryCore/SourcePolicy.h @@ -1,6 +1,7 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. #pragma once +#include "SourceList.h" #include #include @@ -13,4 +14,13 @@ namespace AppInstaller::Repository // Note that this applies to user sources that are being added as well as user sources // that already existed when the Group Policy came into effect. Settings::TogglePolicy::Policy GetPolicyBlockingUserSource(std::string_view name, std::string_view type, std::string_view arg, bool isTombstone); + + // Helper that converts the result of GetPolicyBlockingUserSource into a bool. + bool IsUserSourceAllowedByPolicy(std::string_view name, std::string_view type, std::string_view arg, bool isTombstone); + + // Determines if a well known source is enabled; if onlyExplicit is true, it must be explicitly enabled by group policy. + bool IsWellKnownSourceEnabled(WellKnownSource source, bool onlyExplicit = false); + + // Checks that the specified source is removable per policy; throwing if it is not. + void EnsureSourceIsRemovable(const SourceDetailsInternal& source); } From 5e3a431617d45e99d41df93b22eefd7599a56314 Mon Sep 17 00:00:00 2001 From: JohnMcPMS Date: Wed, 29 Sep 2021 17:16:05 -0700 Subject: [PATCH 3/7] Starting on implementing settings part --- .../Public/winget/Settings.h | 26 +++++++++- src/AppInstallerCommonCore/Settings.cpp | 51 ++++++++++--------- 2 files changed, 52 insertions(+), 25 deletions(-) diff --git a/src/AppInstallerCommonCore/Public/winget/Settings.h b/src/AppInstallerCommonCore/Public/winget/Settings.h index 7f072c3889..100c7c879f 100644 --- a/src/AppInstallerCommonCore/Public/winget/Settings.h +++ b/src/AppInstallerCommonCore/Public/winget/Settings.h @@ -10,6 +10,28 @@ namespace AppInstaller::Settings { using namespace std::string_view_literals; + namespace details + { + // A settings container. + struct ISettingsContainer + { + virtual ~ISettingsContainer() = default; + + // Gets a stream containing the setting's value, if present. + // If the setting does not exist, returns an empty value. + virtual std::unique_ptr Get() = 0; + + // Sets the setting to the given value. + virtual void Set(std::string_view value) = 0; + + // Deletes the setting. + virtual void Remove() = 0; + + // Gets the path to the setting, if reasonable. + virtual std::filesystem::path PathTo() = 0; + }; + } + // Allows settings to be classified and treated differently base on any number of factors. // Names should still be unique, as there is no guarantee made about types mapping to unique roots. enum class Type @@ -80,7 +102,7 @@ namespace AppInstaller::Settings std::filesystem::path GetPath() const; private: - const StreamDefinition& m_streamDefinition; - bool m_synchronize; + const StreamDefinition m_streamDefinition; + std::unique_ptr m_container; }; } diff --git a/src/AppInstallerCommonCore/Settings.cpp b/src/AppInstallerCommonCore/Settings.cpp index 1116504d34..51533fde6b 100644 --- a/src/AppInstallerCommonCore/Settings.cpp +++ b/src/AppInstallerCommonCore/Settings.cpp @@ -28,25 +28,6 @@ namespace AppInstaller::Settings AICLI_LOG(Core, Verbose, << "Setting action: " << action << ", Type: " << ToString(def.Type) << ", Name: " << def.Name); } - // A settings container. - struct ISettingsContainer - { - virtual ~ISettingsContainer() = default; - - // Gets a stream containing the named setting's value, if present. - // If the setting does not exist, returns an empty value. - virtual std::unique_ptr Get(const std::filesystem::path& name) = 0; - - // Sets the named setting to the given value. - virtual void Set(const std::filesystem::path& name, std::string_view value) = 0; - - // Deletes the given setting. - virtual void Remove(const std::filesystem::path& name) = 0; - - // Gets the path to the named setting, if reasonable. - virtual std::filesystem::path PathTo(const std::filesystem::path& name) = 0; - }; - #ifndef WINGET_DISABLE_FOR_FUZZING // A settings container backed by the ApplicationDataContainer functionality. struct ApplicationDataSettingsContainer : public ISettingsContainer @@ -291,17 +272,19 @@ namespace AppInstaller::Settings FileSettingsContainer m_secure; }; - std::unique_ptr GetSettingsContainer(Type type) + std::unique_ptr GetSettingsContainer(const StreamDefinition& streamDefinition) { - if (type == Type::Secure) + if (streamDefinition.Type == Type::Secure) { - return std::make_unique(GetSettingsContainer(Type::Standard)); + StreamDefinition standardDefinition = streamDefinition; + standardDefinition.Type = Type::Standard; + return std::make_unique(GetSettingsContainer(standardDefinition)); } #ifndef WINGET_DISABLE_FOR_FUZZING if (IsRunningInPackagedContext()) { - switch (type) + switch (streamDefinition.Type) { case Type::Standard: return std::make_unique( @@ -369,4 +352,26 @@ namespace AppInstaller::Settings { return GetSettingsContainer(def.Type)->PathTo(def.Name); } + + Stream::Stream(const StreamDefinition& streamDefinition) : + m_streamDefinition(streamDefinition), m_container(GetSettingsContainer(streamDefinition)) + { + } + + // Gets the stream if present. + // If the setting stream does not exist, returns an empty value (see operator bool). + std::unique_ptr Get(); + + // Sets the stream to the given value. + // Returns true if successful; false if the underlying stream has changed. + [[nodiscard]] bool Set(std::string_view value); + + // Deletes the setting stream. + void Remove(); + + // Gets the name of the stream. + std::string_view GetName() const; + + // Gets the path to the stream. + std::filesystem::path GetPath() const; } From 4766c8cc18921dc13adf1fbc47acab7f394f557d Mon Sep 17 00:00:00 2001 From: JohnMcPMS Date: Thu, 30 Sep 2021 12:56:44 -0700 Subject: [PATCH 4/7] Compiling --- src/AppInstallerCLITests/CustomHeader.cpp | 6 +- .../ExperimentalFeature.cpp | 8 +- .../PreIndexedPackageSource.cpp | 5 +- src/AppInstallerCLITests/Settings.cpp | 65 ++-- src/AppInstallerCLITests/Sources.cpp | 76 ++--- src/AppInstallerCLITests/TestSettings.cpp | 17 +- src/AppInstallerCLITests/TestSettings.h | 5 + src/AppInstallerCLITests/UserSettings.cpp | 58 ++-- src/AppInstallerCLITests/WorkFlow.cpp | 4 +- .../Public/winget/Settings.h | 2 +- src/AppInstallerCommonCore/Settings.cpp | 317 +++++++++++------- 11 files changed, 336 insertions(+), 227 deletions(-) diff --git a/src/AppInstallerCLITests/CustomHeader.cpp b/src/AppInstallerCLITests/CustomHeader.cpp index 9741b197c5..1c5501b7ef 100644 --- a/src/AppInstallerCLITests/CustomHeader.cpp +++ b/src/AppInstallerCLITests/CustomHeader.cpp @@ -86,7 +86,7 @@ TEST_CASE("RestClient_CustomHeader", "[RestSource][CustomHeader]") TEST_CASE("AddSource_CustomHeader", "[RestSource][CustomHeader]") { - SetSetting(Streams::UserSources, s_EmptySources); + SetSetting(Stream::UserSources, s_EmptySources); TestHook_ClearSourceFactoryOverrides(); std::string customHeader = "Testing custom header with open source"; @@ -110,7 +110,7 @@ TEST_CASE("AddSource_CustomHeader", "[RestSource][CustomHeader]") TEST_CASE("CreateSource_CustomHeader", "[RestSource][CustomHeader]") { - SetSetting(Streams::UserSources, s_EmptySources); + SetSetting(Stream::UserSources, s_EmptySources); TestHook_ClearSourceFactoryOverrides(); std::string customHeader = "Testing custom header with open source"; @@ -137,7 +137,7 @@ TEST_CASE("CreateSource_CustomHeader", "[RestSource][CustomHeader]") TEST_CASE("CreateSource_CustomHeaderNotApplicable", "[RestSource][CustomHeader]") { - SetSetting(Streams::UserSources, s_EmptySources); + SetSetting(Stream::UserSources, s_EmptySources); TestHook_ClearSourceFactoryOverrides(); std::string customHeader = "Testing custom header with open source"; diff --git a/src/AppInstallerCLITests/ExperimentalFeature.cpp b/src/AppInstallerCLITests/ExperimentalFeature.cpp index 45892ec0c2..0440b173d7 100644 --- a/src/AppInstallerCLITests/ExperimentalFeature.cpp +++ b/src/AppInstallerCLITests/ExperimentalFeature.cpp @@ -39,7 +39,7 @@ TEST_CASE("ExperimentalFeature ExperimentalCmd", "[experimentalFeature]") SECTION("Feature on") { std::string_view json = R"({ "experimentalFeatures": { "experimentalCmd": true } })"; - SetSetting(Streams::PrimaryUserSettings, json); + SetSetting(Stream::PrimaryUserSettings, json); UserSettingsTest userSettingTest; REQUIRE(ExperimentalFeature::IsEnabled(ExperimentalFeature::Feature::ExperimentalCmd, userSettingTest)); @@ -47,7 +47,7 @@ TEST_CASE("ExperimentalFeature ExperimentalCmd", "[experimentalFeature]") SECTION("Feature off") { std::string_view json = R"({ "experimentalFeatures": { "experimentalCmd": false } })"; - SetSetting(Streams::PrimaryUserSettings, json); + SetSetting(Stream::PrimaryUserSettings, json); UserSettingsTest userSettingTest; REQUIRE_FALSE(ExperimentalFeature::IsEnabled(ExperimentalFeature::Feature::ExperimentalCmd, userSettingTest)); @@ -55,7 +55,7 @@ TEST_CASE("ExperimentalFeature ExperimentalCmd", "[experimentalFeature]") SECTION("Invalid value") { std::string_view json = R"({ "experimentalFeatures": { "experimentalCmd": "string" } })"; - SetSetting(Streams::PrimaryUserSettings, json); + SetSetting(Stream::PrimaryUserSettings, json); UserSettingsTest userSettingTest; REQUIRE_FALSE(ExperimentalFeature::IsEnabled(ExperimentalFeature::Feature::ExperimentalCmd, userSettingTest)); @@ -67,7 +67,7 @@ TEST_CASE("ExperimentalFeature ExperimentalCmd", "[experimentalFeature]") GroupPolicyTestOverride policies{ policiesKey.get() }; std::string_view json = R"({ "experimentalFeatures": { "experimentalCmd": true } })"; - SetSetting(Streams::PrimaryUserSettings, json); + SetSetting(Stream::PrimaryUserSettings, json); UserSettingsTest userSettingTest; REQUIRE_FALSE(ExperimentalFeature::IsEnabled(ExperimentalFeature::Feature::ExperimentalCmd, userSettingTest)); diff --git a/src/AppInstallerCLITests/PreIndexedPackageSource.cpp b/src/AppInstallerCLITests/PreIndexedPackageSource.cpp index 95eab8438c..ab01a96b1d 100644 --- a/src/AppInstallerCLITests/PreIndexedPackageSource.cpp +++ b/src/AppInstallerCLITests/PreIndexedPackageSource.cpp @@ -2,6 +2,7 @@ // Licensed under the MIT License. #include "pch.h" #include "TestCommon.h" +#include "TestSettings.h" #include #include #include @@ -56,8 +57,8 @@ std::string GetContents(const fs::path& file) void CleanSources() { - RemoveSetting(Streams::UserSources); - RemoveSetting(Streams::SourcesMetadata); + RemoveSetting(Stream::UserSources); + RemoveSetting(Stream::SourcesMetadata); fs::remove_all(GetPathToFileDir()); } diff --git a/src/AppInstallerCLITests/Settings.cpp b/src/AppInstallerCLITests/Settings.cpp index 2355d9f8ec..a0bb5c4fac 100644 --- a/src/AppInstallerCLITests/Settings.cpp +++ b/src/AppInstallerCLITests/Settings.cpp @@ -15,7 +15,7 @@ TEST_CASE("ReadEmptySetting", "[settings]") { StreamDefinition name{ Type::Standard, "nonexistentsetting" }; - auto result = GetSettingStream(name); + auto result = Stream{ name }.Get(); REQUIRE(!result); } @@ -24,9 +24,10 @@ TEST_CASE("SetAndReadSetting", "[settings]") StreamDefinition name{ Type::Standard, "testsettingname" }; std::string value = "This is the test setting value"; - SetSetting(name, value); + Stream stream{ name }; + REQUIRE(stream.Set(value)); - auto result = GetSettingStream(name); + auto result = stream.Get(); REQUIRE(static_cast(result)); std::string settingValue = ReadEntireStream(*result); @@ -38,9 +39,10 @@ TEST_CASE("SetAndReadSettingInContainer", "[settings]") StreamDefinition name{ Type::Standard, "testcontainer/testsettingname" }; std::string value = "This is the test setting value from inside a container"; - SetSetting(name, value); + Stream stream{ name }; + REQUIRE(stream.Set(value)); - auto result = GetSettingStream(name); + auto result = stream.Get(); REQUIRE(static_cast(result)); std::string settingValue = ReadEntireStream(*result); @@ -52,19 +54,20 @@ TEST_CASE("RemoveSetting", "[settings]") StreamDefinition name{ Type::Standard, "testsettingname" }; std::string value = "This is the test setting value to be removed"; - SetSetting(name, value); + Stream stream{ name }; + REQUIRE(stream.Set(value)); { - auto result = GetSettingStream(name); + auto result = stream.Get(); REQUIRE(static_cast(result)); std::string settingValue = ReadEntireStream(*result); REQUIRE(value == settingValue); } - RemoveSetting( name); + stream.Remove(); - auto result = GetSettingStream(name); + auto result = stream.Get(); REQUIRE(!static_cast(result)); } @@ -73,9 +76,10 @@ TEST_CASE("SetAndReadUserFileSetting", "[settings]") StreamDefinition name{ Type::UserFile, "userfilesetting" }; std::string value = "This is the test setting value for a user file"; - SetSetting(name, value); + Stream stream{ name }; + REQUIRE(stream.Set(value)); - auto result = GetSettingStream(name); + auto result = stream.Get(); REQUIRE(static_cast(result)); std::string settingValue = ReadEntireStream(*result); @@ -86,7 +90,7 @@ TEST_CASE("ReadEmptySecureSetting", "[settings]") { StreamDefinition name{ Type::Secure, "secure_nonexistentsetting" }; - auto result = GetSettingStream(name); + auto result = Stream{ name }.Get(); REQUIRE(!result); } @@ -95,9 +99,10 @@ TEST_CASE("SetAndReadSecureSetting", "[settings]") StreamDefinition name{ Type::Secure, "secure_testsettingname" }; std::string value = "This is the test setting value"; - SetSetting(name, value); + Stream stream{ name }; + REQUIRE(stream.Set(value)); - auto result = GetSettingStream(name); + auto result = stream.Get(); REQUIRE(static_cast(result)); std::string settingValue = ReadEntireStream(*result); @@ -109,9 +114,10 @@ TEST_CASE("SetAndReadSecureSettingInContainer", "[settings]") StreamDefinition name{ Type::Secure, "testcontainer/secure_testsettingname" }; std::string value = "This is the test setting value from inside a container"; - SetSetting(name, value); + Stream stream{ name }; + REQUIRE(stream.Set(value)); - auto result = GetSettingStream(name); + auto result = stream.Get(); REQUIRE(static_cast(result)); std::string settingValue = ReadEntireStream(*result); @@ -123,19 +129,20 @@ TEST_CASE("RemoveSecureSetting", "[settings]") StreamDefinition name{ Type::Secure, "secure_testsettingname" }; std::string value = "This is the test setting value to be removed"; - SetSetting(name, value); + Stream stream{ name }; + REQUIRE(stream.Set(value)); { - auto result = GetSettingStream(name); + auto result = stream.Get(); REQUIRE(static_cast(result)); std::string settingValue = ReadEntireStream(*result); REQUIRE(value == settingValue); } - RemoveSetting(name); + stream.Remove(); - auto result = GetSettingStream(name); + auto result = stream.Get(); REQUIRE(!static_cast(result)); } @@ -144,17 +151,18 @@ TEST_CASE("SetAndReadSecureSetting_SecureDataRemoved", "[settings]") StreamDefinition name{ Type::Secure, "secure_testsettingname" }; std::string value = "This is the test setting value"; - SetSetting(name, value); + Stream stream{ name }; + REQUIRE(stream.Set(value)); - auto result = GetSettingStream(name); + auto result = stream.Get(); REQUIRE(static_cast(result)); std::string settingValue = ReadEntireStream(*result); REQUIRE(value == settingValue); - std::filesystem::remove(GetPathTo(PathName::SecureSettings) / name.Path); + std::filesystem::remove(GetPathTo(PathName::SecureSettings) / name.Name); - REQUIRE_THROWS_HR(GetSettingStream(name), SPAPI_E_FILE_HASH_NOT_IN_CATALOG); + REQUIRE_THROWS_HR(stream.Get(), SPAPI_E_FILE_HASH_NOT_IN_CATALOG); } TEST_CASE("SetAndReadSecureSetting_DataTampered", "[settings]") @@ -162,9 +170,10 @@ TEST_CASE("SetAndReadSecureSetting_DataTampered", "[settings]") StreamDefinition name{ Type::Secure, "secure_testsettingname" }; std::string value = "This is the test setting value"; - SetSetting(name, value); + Stream stream{ name }; + REQUIRE(stream.Set(value)); - auto result = GetSettingStream(name); + auto result = stream.Get(); REQUIRE(static_cast(result)); std::string settingValue = ReadEntireStream(*result); @@ -173,7 +182,7 @@ TEST_CASE("SetAndReadSecureSetting_DataTampered", "[settings]") StreamDefinition insecureName = name; insecureName.Type = Type::Standard; - SetSetting(insecureName, "Tampered data"); + REQUIRE(Stream{ insecureName }.Set("Tampered data")); - REQUIRE_THROWS_HR(GetSettingStream(name), HRESULT_FROM_WIN32(ERROR_DATA_CHECKSUM_ERROR)); + REQUIRE_THROWS_HR(stream.Get(), HRESULT_FROM_WIN32(ERROR_DATA_CHECKSUM_ERROR)); } diff --git a/src/AppInstallerCLITests/Sources.cpp b/src/AppInstallerCLITests/Sources.cpp index e63a98b09a..2bbe51d16e 100644 --- a/src/AppInstallerCLITests/Sources.cpp +++ b/src/AppInstallerCLITests/Sources.cpp @@ -211,7 +211,7 @@ namespace TEST_CASE("RepoSources_UserSettingDoesNotExist", "[sources]") { - RemoveSetting(Streams::UserSources); + RemoveSetting(Stream::UserSources); std::vector sources = GetSources(); REQUIRE(sources.size() == c_DefaultSourceCount); @@ -220,7 +220,7 @@ TEST_CASE("RepoSources_UserSettingDoesNotExist", "[sources]") TEST_CASE("RepoSources_EmptySourcesList", "[sources]") { - SetSetting(Streams::UserSources, s_EmptySources); + SetSetting(Stream::UserSources, s_EmptySources); std::vector sources = GetSources(); REQUIRE(sources.size() == c_DefaultSourceCount); @@ -229,7 +229,7 @@ TEST_CASE("RepoSources_EmptySourcesList", "[sources]") TEST_CASE("RepoSources_DefaultSourcesTombstoned", "[sources]") { - SetSetting(Streams::UserSources, s_DefaultSourcesTombstoned); + SetSetting(Stream::UserSources, s_DefaultSourcesTombstoned); std::vector sources = GetSources(); REQUIRE(sources.empty()); @@ -237,8 +237,8 @@ TEST_CASE("RepoSources_DefaultSourcesTombstoned", "[sources]") TEST_CASE("RepoSources_SingleSource", "[sources]") { - SetSetting(Streams::UserSources, s_SingleSource); - RemoveSetting(Streams::SourcesMetadata); + SetSetting(Stream::UserSources, s_SingleSource); + RemoveSetting(Stream::SourcesMetadata); std::vector sources = GetSources(); REQUIRE(sources.size() == c_DefaultSourceCount + 1); @@ -255,8 +255,8 @@ TEST_CASE("RepoSources_SingleSource", "[sources]") TEST_CASE("RepoSources_ThreeSources", "[sources]") { - SetSetting(Streams::UserSources, s_ThreeSources); - SetSetting(Streams::SourcesMetadata, s_ThreeSourcesMetadata); + SetSetting(Stream::UserSources, s_ThreeSources); + SetSetting(Stream::SourcesMetadata, s_ThreeSourcesMetadata); std::vector sources = GetSources(); REQUIRE(sources.size() == 3); @@ -277,21 +277,21 @@ TEST_CASE("RepoSources_ThreeSources", "[sources]") TEST_CASE("RepoSources_InvalidYAML", "[sources]") { - SetSetting(Streams::UserSources, "Name: Value : BAD"); + SetSetting(Stream::UserSources, "Name: Value : BAD"); REQUIRE_THROWS_HR(GetSources(), APPINSTALLER_CLI_ERROR_SOURCES_INVALID); } TEST_CASE("RepoSources_MissingField", "[sources]") { - SetSetting(Streams::UserSources, s_SingleSource_MissingArg); + SetSetting(Stream::UserSources, s_SingleSource_MissingArg); REQUIRE_THROWS_HR(GetSources(), APPINSTALLER_CLI_ERROR_SOURCES_INVALID); } TEST_CASE("RepoSources_AddSource", "[sources]") { - SetSetting(Streams::UserSources, s_EmptySources); + SetSetting(Stream::UserSources, s_EmptySources); TestHook_ClearSourceFactoryOverrides(); SourceDetails details; @@ -325,7 +325,7 @@ TEST_CASE("RepoSources_AddSource", "[sources]") TEST_CASE("RepoSources_AddMultipleSources", "[sources]") { - SetSetting(Streams::UserSources, s_EmptySources); + SetSetting(Stream::UserSources, s_EmptySources); SourceDetails details; details.Name = "thisIsTheName"; @@ -386,7 +386,7 @@ TEST_CASE("RepoSources_UpdateSource", "[sources]") { using namespace std::chrono_literals; - SetSetting(Streams::UserSources, s_EmptySources); + SetSetting(Stream::UserSources, s_EmptySources); TestHook_ClearSourceFactoryOverrides(); SourceDetails details; @@ -440,7 +440,7 @@ TEST_CASE("RepoSources_UpdateSourceRetries", "[sources]") { using namespace std::chrono_literals; - SetSetting(Streams::UserSources, s_EmptySources); + SetSetting(Stream::UserSources, s_EmptySources); TestHook_ClearSourceFactoryOverrides(); SourceDetails details; @@ -476,7 +476,7 @@ TEST_CASE("RepoSources_UpdateSourceRetries", "[sources]") TEST_CASE("RepoSources_RemoveSource", "[sources]") { - SetSetting(Streams::UserSources, s_EmptySources); + SetSetting(Stream::UserSources, s_EmptySources); TestHook_ClearSourceFactoryOverrides(); SourceDetails details; @@ -506,7 +506,7 @@ TEST_CASE("RepoSources_RemoveSource", "[sources]") TEST_CASE("RepoSources_RemoveDefaultSource", "[sources]") { - SetSetting(Streams::UserSources, s_EmptySources); + SetSetting(Stream::UserSources, s_EmptySources); TestHook_ClearSourceFactoryOverrides(); std::vector sources = GetSources(); @@ -544,7 +544,7 @@ TEST_CASE("RepoSources_UpdateOnOpen", "[sources]") factory.OnUpdate = [&](const SourceDetails&) { updateCalledOnFactory = true; }; TestHook_SetSourceFactoryOverride(type, factory); - SetSetting(Streams::UserSources, s_SingleSource); + SetSetting(Stream::UserSources, s_SingleSource); ProgressCallback progress; auto source = OpenSource(name, progress).Source; @@ -563,8 +563,8 @@ TEST_CASE("RepoSources_UpdateOnOpen", "[sources]") TEST_CASE("RepoSources_DropSourceByName", "[sources]") { - SetSetting(Streams::UserSources, s_ThreeSources); - SetSetting(Streams::SourcesMetadata, s_ThreeSourcesMetadata); + SetSetting(Stream::UserSources, s_ThreeSources); + SetSetting(Stream::SourcesMetadata, s_ThreeSourcesMetadata); std::vector sources = GetSources(); REQUIRE(sources.size() == 3); @@ -590,7 +590,7 @@ TEST_CASE("RepoSources_DropSourceByName", "[sources]") TEST_CASE("RepoSources_DropAllSources", "[sources]") { - SetSetting(Streams::UserSources, s_ThreeSources); + SetSetting(Stream::UserSources, s_ThreeSources); std::vector sources = GetSources(); REQUIRE(sources.size() == 3); @@ -608,7 +608,7 @@ TEST_CASE("RepoSources_SearchAcrossMultipleSources", "[sources]") TestSourceFactory factory{ SourcesTestSource::Create }; TestHook_SetSourceFactoryOverride("testType", factory); - SetSetting(Streams::UserSources, s_TwoSource_AggregateSourceTest); + SetSetting(Stream::UserSources, s_TwoSource_AggregateSourceTest); ProgressCallback progress; auto source = OpenSource("", progress).Source; @@ -646,7 +646,7 @@ TEST_CASE("RepoSources_GroupPolicy_DefaultSource", "[sources][groupPolicy]") SECTION("Get source") { // Listing the sources should not return the default. - SetSetting(Streams::UserSources, s_EmptySources); + SetSetting(Stream::UserSources, s_EmptySources); auto sources = GetSources(); REQUIRE(sources.size() == c_DefaultSourceCount - 1); @@ -654,7 +654,7 @@ TEST_CASE("RepoSources_GroupPolicy_DefaultSource", "[sources][groupPolicy]") SECTION("Add default source") { // We should not be able to add the default source manually. - SetSetting(Streams::UserSources, s_EmptySources); + SetSetting(Stream::UserSources, s_EmptySources); ProgressCallback progress; SourceDetails details; @@ -668,7 +668,7 @@ TEST_CASE("RepoSources_GroupPolicy_DefaultSource", "[sources][groupPolicy]") SECTION("Ignore default source from user") { // We should ignore any existing user source that is the same as the default. - SetSetting(Streams::UserSources, s_DefaultSourceAsUserSource); + SetSetting(Stream::UserSources, s_DefaultSourceAsUserSource); auto sources = GetSources(); REQUIRE(sources.size() == c_DefaultSourceCount - 1); @@ -677,7 +677,7 @@ TEST_CASE("RepoSources_GroupPolicy_DefaultSource", "[sources][groupPolicy]") { // We should allow adding sources with the same name as the default but // pointing somewhere else. - SetSetting(Streams::UserSources, s_EmptySources); + SetSetting(Stream::UserSources, s_EmptySources); TestHook_ClearSourceFactoryOverrides(); SourceDetails details; @@ -710,7 +710,7 @@ TEST_CASE("RepoSources_GroupPolicy_DefaultSource", "[sources][groupPolicy]") // We should respect existing user sources with the same name. // We should allow adding sources with the same name as the default but // pointing somewhere else. - SetSetting(Streams::UserSources, s_UserSourceNamedLikeDefault); + SetSetting(Stream::UserSources, s_UserSourceNamedLikeDefault); auto sources = GetSources(); REQUIRE(sources.size() == c_DefaultSourceCount); @@ -731,7 +731,7 @@ TEST_CASE("RepoSources_GroupPolicy_DefaultSource", "[sources][groupPolicy]") SECTION("Remove source is blocked") { // We should not be able to remove the default source. - SetSetting(Streams::UserSources, s_EmptySources); + SetSetting(Stream::UserSources, s_EmptySources); ProgressCallback progress; REQUIRE_POLICY_EXCEPTION( @@ -741,7 +741,7 @@ TEST_CASE("RepoSources_GroupPolicy_DefaultSource", "[sources][groupPolicy]") SECTION("Tombstone is overridden") { // We should ignore if the default source was already deleted. - SetSetting(Streams::UserSources, s_DefaultSourcesTombstoned); + SetSetting(Stream::UserSources, s_DefaultSourcesTombstoned); auto sources = GetSources(); REQUIRE(sources.size() == 1); @@ -751,7 +751,7 @@ TEST_CASE("RepoSources_GroupPolicy_DefaultSource", "[sources][groupPolicy]") SECTION("Same name source is overridden") { // We should ignore existing user sources with the same name as the default. - SetSetting(Streams::UserSources, s_UserSourceNamedLikeDefault); + SetSetting(Stream::UserSources, s_UserSourceNamedLikeDefault); auto sources = GetSources(); REQUIRE(sources.size() == c_DefaultSourceCount); @@ -787,7 +787,7 @@ TEST_CASE("RepoSources_GroupPolicy_AdditionalSources", "[sources][groupPolicy]") } policies.SetValue(policySources); - SetSetting(Streams::UserSources, s_EmptySources); + SetSetting(Stream::UserSources, s_EmptySources); auto sources = GetSources(); @@ -816,7 +816,7 @@ TEST_CASE("RepoSources_GroupPolicy_AdditionalSources", "[sources][groupPolicy]") policySource.Identifier = "notTestId"; policies.SetValue({ policySource }); - SetSetting(Streams::UserSources, s_SingleSource); + SetSetting(Stream::UserSources, s_SingleSource); auto sources = GetSources(); @@ -842,7 +842,7 @@ TEST_CASE("RepoSources_GroupPolicy_AdditionalSources", "[sources][groupPolicy]") policySource.Identifier = "id"; policies.SetValue({ policySource }); - SetSetting(Streams::UserSources, s_EmptySources); + SetSetting(Stream::UserSources, s_EmptySources); ProgressCallback progress; REQUIRE_POLICY_EXCEPTION( @@ -860,7 +860,7 @@ TEST_CASE("RepoSources_GroupPolicy_AdditionalSources", "[sources][groupPolicy]") policySource.Identifier = "notDefaultId"; policies.SetValue({ policySource }); - SetSetting(Streams::UserSources, s_EmptySources); + SetSetting(Stream::UserSources, s_EmptySources); auto sources = GetSources(); @@ -893,7 +893,7 @@ TEST_CASE("RepoSources_GroupPolicy_AllowedSources", "[sources][groupPolicy]") policySource.Identifier = "testId"; policies.SetValue({ policySource }); - SetSetting(Streams::UserSources, s_EmptySources); + SetSetting(Stream::UserSources, s_EmptySources); TestHook_ClearSourceFactoryOverrides(); bool addCalledOnFactory = false; @@ -938,7 +938,7 @@ TEST_CASE("RepoSources_GroupPolicy_AllowedSources", "[sources][groupPolicy]") policySource.Identifier = "testId"; policies.SetValue({ policySource }); - SetSetting(Streams::UserSources, s_EmptySources); + SetSetting(Stream::UserSources, s_EmptySources); bool addCalledOnFactory = false; TestSourceFactory factory{ SourcesTestSource::Create }; @@ -963,7 +963,7 @@ TEST_CASE("RepoSources_GroupPolicy_AllowedSources", "[sources][groupPolicy]") SECTION("Cannot add any source") { - SetSetting(Streams::UserSources, s_EmptySources); + SetSetting(Stream::UserSources, s_EmptySources); bool addCalledOnFactory = false; TestSourceFactory factory{ SourcesTestSource::Create }; @@ -985,7 +985,7 @@ TEST_CASE("RepoSources_GroupPolicy_AllowedSources", "[sources][groupPolicy]") } SECTION("Existing sources are ignored") { - SetSetting(Streams::UserSources, s_SingleSource); + SetSetting(Stream::UserSources, s_SingleSource); auto sources = GetSources(); REQUIRE(sources.size() == c_DefaultSourceCount); @@ -1000,7 +1000,7 @@ TEST_CASE("RepoSources_OpenMultipleWithSingleFailure", "[sources]") TestSourceFactory factory{ FailingSourcesTestSource::CreateFailWinget }; TestHook_SetSourceFactoryOverride("testType", factory); - SetSetting(Streams::UserSources, s_TwoSource_AggregateSourceTest); + SetSetting(Stream::UserSources, s_TwoSource_AggregateSourceTest); ProgressCallback progress; auto result = OpenSource("", progress); @@ -1032,7 +1032,7 @@ TEST_CASE("RepoSources_OpenMultipleWithTotalFailure", "[sources]") TestSourceFactory factory{ FailingSourcesTestSource::CreateFailAll }; TestHook_SetSourceFactoryOverride("testType", factory); - SetSetting(Streams::UserSources, s_TwoSource_AggregateSourceTest); + SetSetting(Stream::UserSources, s_TwoSource_AggregateSourceTest); ProgressCallback progress; REQUIRE_THROWS_HR(OpenSource("", progress), APPINSTALLER_CLI_ERROR_FAILED_TO_OPEN_ALL_SOURCES); diff --git a/src/AppInstallerCLITests/TestSettings.cpp b/src/AppInstallerCLITests/TestSettings.cpp index c26d1e944a..19e31ef316 100644 --- a/src/AppInstallerCLITests/TestSettings.cpp +++ b/src/AppInstallerCLITests/TestSettings.cpp @@ -9,6 +9,21 @@ using namespace AppInstaller::Settings; namespace TestCommon { + void SetSetting(const AppInstaller::Settings::StreamDefinition& stream, std::string_view value) + { + REQUIRE(Stream{ stream }.Set(value)); + } + + void RemoveSetting(const AppInstaller::Settings::StreamDefinition& stream) + { + Stream{ stream }.Remove(); + } + + std::filesystem::path GetPathTo(const AppInstaller::Settings::StreamDefinition& stream) + { + return Stream{ stream }.GetPath(); + } + void DeleteUserSettingsFiles() { auto settingsPath = UserSettings::SettingsFilePath(); @@ -17,7 +32,7 @@ namespace TestCommon std::filesystem::remove(settingsPath); } - auto settingsBackupPath = GetPathTo(Streams::BackupUserSettings); + auto settingsBackupPath = GetPathTo(Stream::BackupUserSettings); if (std::filesystem::exists(settingsBackupPath)) { std::filesystem::remove(settingsBackupPath); diff --git a/src/AppInstallerCLITests/TestSettings.h b/src/AppInstallerCLITests/TestSettings.h index 1bc953f524..45a517b063 100644 --- a/src/AppInstallerCLITests/TestSettings.h +++ b/src/AppInstallerCLITests/TestSettings.h @@ -1,6 +1,7 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. #pragma once +#include #include #include #include @@ -23,6 +24,10 @@ namespace TestCommon const std::wstring AdditionalSourcesPolicyKeyName = L"AdditionalSources"; const std::wstring AllowedSourcesPolicyKeyName = L"AllowedSources"; + void SetSetting(const AppInstaller::Settings::StreamDefinition& stream, std::string_view value); + void RemoveSetting(const AppInstaller::Settings::StreamDefinition& stream); + std::filesystem::path GetPathTo(const AppInstaller::Settings::StreamDefinition& stream); + void DeleteUserSettingsFiles(); struct UserSettingsTest : AppInstaller::Settings::UserSettings diff --git a/src/AppInstallerCLITests/UserSettings.cpp b/src/AppInstallerCLITests/UserSettings.cpp index 4f1dfc4d24..02cb75c6bb 100644 --- a/src/AppInstallerCLITests/UserSettings.cpp +++ b/src/AppInstallerCLITests/UserSettings.cpp @@ -50,60 +50,60 @@ TEST_CASE("UserSettingsType", "[settings]") } SECTION("No setting.json Bad setting.json.backup") { - SetSetting(Streams::BackupUserSettings, s_badJson); + SetSetting(Stream::BackupUserSettings, s_badJson); UserSettingsTest userSettingTest; REQUIRE(userSettingTest.GetType() == UserSettingsType::Default); } SECTION("No setting.json Good setting.json.backup") { - SetSetting(Streams::BackupUserSettings, s_goodJson); + SetSetting(Stream::BackupUserSettings, s_goodJson); UserSettingsTest userSettingTest; REQUIRE(userSettingTest.GetType() == UserSettingsType::Backup); } SECTION("Bad setting.json No setting.json.backup") { - SetSetting(Streams::PrimaryUserSettings, s_badJson); + SetSetting(Stream::PrimaryUserSettings, s_badJson); UserSettingsTest userSettingTest; REQUIRE(userSettingTest.GetType() == UserSettingsType::Default); } SECTION("Bad setting.json Bad setting.json.backup") { - SetSetting(Streams::PrimaryUserSettings, s_badJson); - SetSetting(Streams::BackupUserSettings, s_badJson); + SetSetting(Stream::PrimaryUserSettings, s_badJson); + SetSetting(Stream::BackupUserSettings, s_badJson); UserSettingsTest userSettingTest; REQUIRE(userSettingTest.GetType() == UserSettingsType::Default); } SECTION("Bad setting.json Good setting.json.backup") { - SetSetting(Streams::PrimaryUserSettings, s_badJson); - SetSetting(Streams::BackupUserSettings, s_goodJson); + SetSetting(Stream::PrimaryUserSettings, s_badJson); + SetSetting(Stream::BackupUserSettings, s_goodJson); UserSettingsTest userSettingTest; REQUIRE(userSettingTest.GetType() == UserSettingsType::Backup); } SECTION("Good setting.json No setting.json.backup") { - SetSetting(Streams::PrimaryUserSettings, s_goodJson); + SetSetting(Stream::PrimaryUserSettings, s_goodJson); UserSettingsTest userSettingTest; REQUIRE(userSettingTest.GetType() == UserSettingsType::Standard); } SECTION("Good setting.json Bad setting.json.backup") { - SetSetting(Streams::PrimaryUserSettings, s_goodJson); - SetSetting(Streams::BackupUserSettings, s_badJson); + SetSetting(Stream::PrimaryUserSettings, s_goodJson); + SetSetting(Stream::BackupUserSettings, s_badJson); UserSettingsTest userSettingTest; REQUIRE(userSettingTest.GetType() == UserSettingsType::Standard); } SECTION("Good setting.json Good setting.json.backup") { - SetSetting(Streams::PrimaryUserSettings, s_goodJson); - SetSetting(Streams::BackupUserSettings, s_goodJson); + SetSetting(Stream::PrimaryUserSettings, s_goodJson); + SetSetting(Stream::BackupUserSettings, s_goodJson); UserSettingsTest userSettingTest; REQUIRE(userSettingTest.GetType() == UserSettingsType::Standard); @@ -115,7 +115,7 @@ TEST_CASE("UserSettingsCreateFiles", "[settings]") DeleteUserSettingsFiles(); auto settingsPath = UserSettings::SettingsFilePath(); - auto settingsBackupPath = GetPathTo(Streams::BackupUserSettings); + auto settingsBackupPath = GetPathTo(Stream::BackupUserSettings); SECTION("No settings.json create new") { @@ -131,7 +131,7 @@ TEST_CASE("UserSettingsCreateFiles", "[settings]") } SECTION("Good settings.json create new backup") { - SetSetting(Streams::PrimaryUserSettings, s_goodJson); + SetSetting(Stream::PrimaryUserSettings, s_goodJson); REQUIRE(std::filesystem::exists(settingsPath)); REQUIRE(!std::filesystem::exists(settingsBackupPath)); @@ -158,7 +158,7 @@ TEST_CASE("SettingProgressBar", "[settings]") SECTION("Accent") { std::string_view json = R"({ "visual": { "progressBar": "accent" } })"; - SetSetting(Streams::PrimaryUserSettings, json); + SetSetting(Stream::PrimaryUserSettings, json); UserSettingsTest userSettingTest; REQUIRE(userSettingTest.Get() == VisualStyle::Accent); @@ -167,7 +167,7 @@ TEST_CASE("SettingProgressBar", "[settings]") SECTION("Rainbow") { std::string_view json = R"({ "visual": { "progressBar": "rainbow" } })"; - SetSetting(Streams::PrimaryUserSettings, json); + SetSetting(Stream::PrimaryUserSettings, json); UserSettingsTest userSettingTest; REQUIRE(userSettingTest.Get() == VisualStyle::Rainbow); @@ -176,7 +176,7 @@ TEST_CASE("SettingProgressBar", "[settings]") SECTION("retro") { std::string_view json = R"({ "visual": { "progressBar": "retro" } })"; - SetSetting(Streams::PrimaryUserSettings, json); + SetSetting(Stream::PrimaryUserSettings, json); UserSettingsTest userSettingTest; REQUIRE(userSettingTest.Get() == VisualStyle::Retro); @@ -185,7 +185,7 @@ TEST_CASE("SettingProgressBar", "[settings]") SECTION("Bad value") { std::string_view json = R"({ "visual": { "progressBar": "fake" } })"; - SetSetting(Streams::PrimaryUserSettings, json); + SetSetting(Stream::PrimaryUserSettings, json); UserSettingsTest userSettingTest; REQUIRE(userSettingTest.Get() == VisualStyle::Accent); @@ -194,7 +194,7 @@ TEST_CASE("SettingProgressBar", "[settings]") SECTION("Bad value type") { std::string_view json = R"({ "visual": { "progressBar": 5 } })"; - SetSetting(Streams::PrimaryUserSettings, json); + SetSetting(Stream::PrimaryUserSettings, json); UserSettingsTest userSettingTest; REQUIRE(userSettingTest.Get() == VisualStyle::Accent); @@ -220,7 +220,7 @@ TEST_CASE("SettingAutoUpdateIntervalInMinutes", "[settings]") SECTION("Valid value") { std::string_view json = R"({ "source": { "autoUpdateIntervalInMinutes": 0 } })"; - SetSetting(Streams::PrimaryUserSettings, json); + SetSetting(Stream::PrimaryUserSettings, json); UserSettingsTest userSettingTest; REQUIRE(userSettingTest.Get() == cero); @@ -229,7 +229,7 @@ TEST_CASE("SettingAutoUpdateIntervalInMinutes", "[settings]") SECTION("Valid value 0") { std::string_view json = R"({ "source": { "autoUpdateIntervalInMinutes": 300 } })"; - SetSetting(Streams::PrimaryUserSettings, json); + SetSetting(Stream::PrimaryUserSettings, json); UserSettingsTest userSettingTest; REQUIRE(userSettingTest.Get() == threehundred); @@ -238,7 +238,7 @@ TEST_CASE("SettingAutoUpdateIntervalInMinutes", "[settings]") SECTION("Invalid type negative integer") { std::string_view json = R"({ "source": { "autoUpdateIntervalInMinutes": -20 } })"; - SetSetting(Streams::PrimaryUserSettings, json); + SetSetting(Stream::PrimaryUserSettings, json); UserSettingsTest userSettingTest; REQUIRE(userSettingTest.Get() == cinq); @@ -247,7 +247,7 @@ TEST_CASE("SettingAutoUpdateIntervalInMinutes", "[settings]") SECTION("Invalid type string") { std::string_view json = R"({ "source": { "autoUpdateIntervalInMinutes": "not a number" } })"; - SetSetting(Streams::PrimaryUserSettings, json); + SetSetting(Stream::PrimaryUserSettings, json); UserSettingsTest userSettingTest; REQUIRE(userSettingTest.Get() == cinq); @@ -260,7 +260,7 @@ TEST_CASE("SettingAutoUpdateIntervalInMinutes", "[settings]") GroupPolicyTestOverride policies{ policiesKey.get() }; std::string_view json = R"({ "source": { "autoUpdateIntervalInMinutes": 5 } })"; - SetSetting(Streams::PrimaryUserSettings, json); + SetSetting(Stream::PrimaryUserSettings, json); UserSettingsTest userSettingTest; REQUIRE(userSettingTest.Get() == threehundred); @@ -273,7 +273,7 @@ TEST_CASE("SettingAutoUpdateIntervalInMinutes", "[settings]") GroupPolicyTestOverride policies{ policiesKey.get() }; std::string_view json = R"({ "source": { "autoUpdateIntervalInMinutes": 5 } })"; - SetSetting(Streams::PrimaryUserSettings, json); + SetSetting(Stream::PrimaryUserSettings, json); UserSettingsTest userSettingTest; REQUIRE(userSettingTest.Get() == cinq); @@ -295,7 +295,7 @@ TEST_CASE("SettingsExperimentalCmd", "[settings]") SECTION("Feature on") { std::string_view json = R"({ "experimentalFeatures": { "experimentalCmd": true } })"; - SetSetting(Streams::PrimaryUserSettings, json); + SetSetting(Stream::PrimaryUserSettings, json); UserSettingsTest userSettingTest; REQUIRE(userSettingTest.Get()); @@ -304,7 +304,7 @@ TEST_CASE("SettingsExperimentalCmd", "[settings]") SECTION("Feature off") { std::string_view json = R"({ "experimentalFeatures": { "experimentalCmd": false } })"; - SetSetting(Streams::PrimaryUserSettings, json); + SetSetting(Stream::PrimaryUserSettings, json); UserSettingsTest userSettingTest; REQUIRE(!userSettingTest.Get()); @@ -313,7 +313,7 @@ TEST_CASE("SettingsExperimentalCmd", "[settings]") SECTION("Invalid value") { std::string_view json = R"({ "experimentalFeatures": { "experimentalCmd": "string" } })"; - SetSetting(Streams::PrimaryUserSettings, json); + SetSetting(Stream::PrimaryUserSettings, json); UserSettingsTest userSettingTest; REQUIRE(!userSettingTest.Get()); @@ -326,7 +326,7 @@ TEST_CASE("SettingsExperimentalCmd", "[settings]") GroupPolicyTestOverride policies{ policiesKey.get() }; std::string_view json = R"({ "experimentalFeatures": { "experimentalCmd": true } })"; - SetSetting(Streams::PrimaryUserSettings, json); + SetSetting(Stream::PrimaryUserSettings, json); UserSettingsTest userSettingTest; // Experimental features group policy is applied at the ExperimentalFeature level, diff --git a/src/AppInstallerCLITests/WorkFlow.cpp b/src/AppInstallerCLITests/WorkFlow.cpp index f76630a481..09af22f7a4 100644 --- a/src/AppInstallerCLITests/WorkFlow.cpp +++ b/src/AppInstallerCLITests/WorkFlow.cpp @@ -2163,7 +2163,7 @@ TEST_CASE("SourceAddFlow_Agreement_Prompt_No", "[SourceAddFlow][workflow]") TEST_CASE("OpenSource_WithCustomHeader", "[OpenSource][CustomHeader]") { - SetSetting(Streams::UserSources, R"(Sources:)"sv); + SetSetting(Stream::UserSources, R"(Sources:)"sv); TestHook_ClearSourceFactoryOverrides(); SourceDetails details; @@ -2196,7 +2196,7 @@ TEST_CASE("OpenSource_WithCustomHeader", "[OpenSource][CustomHeader]") TEST_CASE("AdminSetting_LocalManifestFiles", "[LocalManifests][workflow]") { - RemoveSetting(Streams::AdminSettings); + RemoveSetting(Stream::AdminSettings); // If there's no admin setting, using local manifest should fail. Execution::Args args; diff --git a/src/AppInstallerCommonCore/Public/winget/Settings.h b/src/AppInstallerCommonCore/Public/winget/Settings.h index 100c7c879f..befbaeca4b 100644 --- a/src/AppInstallerCommonCore/Public/winget/Settings.h +++ b/src/AppInstallerCommonCore/Public/winget/Settings.h @@ -22,7 +22,7 @@ namespace AppInstaller::Settings virtual std::unique_ptr Get() = 0; // Sets the setting to the given value. - virtual void Set(std::string_view value) = 0; + virtual bool Set(std::string_view value) = 0; // Deletes the setting. virtual void Remove() = 0; diff --git a/src/AppInstallerCommonCore/Settings.cpp b/src/AppInstallerCommonCore/Settings.cpp index 51533fde6b..b4069fbbdc 100644 --- a/src/AppInstallerCommonCore/Settings.cpp +++ b/src/AppInstallerCommonCore/Settings.cpp @@ -30,11 +30,15 @@ namespace AppInstaller::Settings #ifndef WINGET_DISABLE_FOR_FUZZING // A settings container backed by the ApplicationDataContainer functionality. - struct ApplicationDataSettingsContainer : public ISettingsContainer + struct ApplicationDataSettingsContainer : public details::ISettingsContainer { using Container = winrt::Windows::Storage::ApplicationDataContainer; - ApplicationDataSettingsContainer(Container&& container) : m_root(std::move(container)) {} + ApplicationDataSettingsContainer(const Container& container, const std::filesystem::path& name) + { + m_parentContainer = GetRelativeContainer(container, name.parent_path()); + m_settingName = winrt::to_hstring(name.filename().c_str()); + } static Container GetRelativeContainer(const Container& container, const std::filesystem::path& offset) { @@ -49,15 +53,12 @@ namespace AppInstaller::Settings return result; } - std::unique_ptr Get(const std::filesystem::path& name) override + std::unique_ptr Get() override { - Container parent = GetRelativeContainer(m_root, name.parent_path()); - - auto filenameHstring = winrt::to_hstring(name.filename().c_str()); - auto settingsValues = parent.Values(); - if (settingsValues.HasKey(filenameHstring)) + auto settingsValues = m_parentContainer.Values(); + if (settingsValues.HasKey(m_settingName)) { - auto value = winrt::unbox_value(settingsValues.Lookup(filenameHstring)); + auto value = winrt::unbox_value(settingsValues.Lookup(m_settingName)); return std::make_unique(Utility::ConvertToUTF8(value.c_str())); } else @@ -66,40 +67,41 @@ namespace AppInstaller::Settings } } - void Set(const std::filesystem::path& name, std::string_view value) override + bool Set(std::string_view value) override { - Container parent = GetRelativeContainer(m_root, name.parent_path()); - parent.Values().Insert(winrt::to_hstring(name.filename().c_str()), winrt::box_value(winrt::to_hstring(value))); + m_parentContainer.Values().Insert(m_settingName, winrt::box_value(winrt::to_hstring(value))); + return true; } - void Remove(const std::filesystem::path& name) override + void Remove() override { - Container parent = GetRelativeContainer(m_root, name.parent_path()); - parent.Values().Remove(winrt::to_hstring(name.filename().c_str())); + m_parentContainer.Values().Remove(m_settingName); } - std::filesystem::path PathTo(const std::filesystem::path&) override + std::filesystem::path PathTo() override { THROW_HR(E_UNEXPECTED); } private: - Container m_root; + Container m_parentContainer = nullptr; + winrt::hstring m_settingName; }; #endif // A settings container backed by the filesystem. - struct FileSettingsContainer : public ISettingsContainer + struct FileSettingsContainer : public details::ISettingsContainer { - FileSettingsContainer(std::filesystem::path root) : m_root(std::move(root)) {} - - std::unique_ptr Get(const std::filesystem::path& name) override + FileSettingsContainer(std::filesystem::path root, const std::filesystem::path& name) : m_settingFile(std::move(root)) { - std::filesystem::path settingFileName = GetPath(name); + m_settingFile /= name; + } - if (std::filesystem::exists(settingFileName)) + std::unique_ptr Get() override + { + if (std::filesystem::exists(m_settingFile)) { - auto result = std::make_unique(settingFileName); + auto result = std::make_unique(m_settingFile); THROW_LAST_ERROR_IF(result->fail()); return result; } @@ -109,66 +111,142 @@ namespace AppInstaller::Settings } } - void Set(const std::filesystem::path& name, std::string_view value) override + bool Set(std::string_view value) override { - std::filesystem::path settingFileName = GetPath(name, true); + EnsureParentPath(); - std::ofstream stream(settingFileName, std::ios_base::out | std::ios_base::binary | std::ios_base::trunc); + std::ofstream stream(m_settingFile, std::ios_base::out | std::ios_base::binary | std::ios_base::trunc); THROW_LAST_ERROR_IF(stream.fail()); stream << value << std::flush; THROW_LAST_ERROR_IF(stream.fail()); + + return true; } - void Remove(const std::filesystem::path& name) override + void Remove() override { - std::filesystem::path settingFileName = GetPath(name); - - std::filesystem::remove(settingFileName); + std::filesystem::remove(m_settingFile); } - std::filesystem::path PathTo(const std::filesystem::path& name) override + std::filesystem::path PathTo() override { - return GetPath(name); + return m_settingFile; } private: - std::filesystem::path GetPath(const std::filesystem::path& name, bool createParent = false) + void EnsureParentPath() + { + std::filesystem::create_directories(m_settingFile.parent_path()); + } + + std::filesystem::path m_settingFile; + }; + + // A settings container that manages safely writing to its value with exchange semantics. + // Only allows Set to succeed if the hash value of the setting is the same as the last time it was read. + struct ExchangeSettingsContainer : public details::ISettingsContainer + { + ExchangeSettingsContainer(std::unique_ptr&& container, const std::string_view& name) : + m_container(std::move(container)), m_name(name) {} + + std::unique_ptr Get() override { - std::filesystem::path result = m_root; + return GetInternal(); + } - if (name.has_parent_path()) + bool Set(std::string_view value) override + { + THROW_HR_IF(E_UNEXPECTED, value.size() > std::numeric_limits::max()); + + // If Set is called without ever reading the value, then we can assume that caller wants + // to overwrite it regardless. Also, we don't have any previous value to compare against + // anyway so the only other option would be to always reject it, but then we would need + // to distinguish the state where the value has never existed. + if (m_hash) { - result /= name.parent_path(); + SHA256::HashBuffer previousHash = m_hash.value(); + std::ignore = GetInternal(); + + if (m_hash && !AreHashesEqual(previousHash, m_hash.value())) + { + AICLI_LOG(Core, Verbose, << "Setting value for '" << m_name << "' has changed since last read; rejecting Set"); + return false; + } } - if (createParent) + SHA256::HashBuffer newHash = SHA256::ComputeHash(reinterpret_cast(value.data()), static_cast(value.size())); + if (m_container->Set(value)) + { + m_hash = std::move(newHash); + return true; + } + else { - std::filesystem::create_directories(result); + return false; } + } - result /= name.filename(); - return result; + void Remove() override + { + m_container->Remove(); + m_hash.reset(); + } + + std::filesystem::path PathTo() override + { + return m_container->PathTo(); + } + + protected: + std::unique_ptr GetInternal() + { + std::unique_ptr stream = m_container->Get(); + + if (!stream) + { + // If no stream exists, then no hashing needs to be done. + return stream; + } + + std::string streamContents = Utility::ReadEntireStream(*stream); + THROW_HR_IF(E_UNEXPECTED, streamContents.size() > std::numeric_limits::max()); + + m_hash = SHA256::ComputeHash(reinterpret_cast(streamContents.c_str()), static_cast(streamContents.size())); + + // Return a stream over the contents that we read in and hashed, to prevent a race. + return std::make_unique(streamContents); + } + + static bool AreHashesEqual(const SHA256::HashBuffer& a, const SHA256::HashBuffer& b) + { + return std::equal(a.begin(), a.end(), b.begin()); } - std::filesystem::path m_root; + std::string_view m_name; + std::optional m_hash; + + private: + std::unique_ptr m_container; }; // A settings container wrapper that enforces security. - struct SecureSettingsContainer : public ISettingsContainer + struct SecureSettingsContainer : public ExchangeSettingsContainer { constexpr static std::string_view NodeName_Sha256 = "SHA256"sv; - SecureSettingsContainer(std::unique_ptr&& container) : m_container(std::move(container)), m_secure(GetPathTo(PathName::SecureSettings)) {} + SecureSettingsContainer(std::unique_ptr&& container, const std::string_view& name) : + ExchangeSettingsContainer(std::move(container), name), m_secure(GetPathTo(PathName::SecureSettings), name) {} + private: struct VerificationData { bool Found = false; SHA256::HashBuffer Hash; }; - VerificationData GetVerificationData(const std::filesystem::path& name) + VerificationData GetVerificationData() { - std::unique_ptr stream = m_secure.Get(name); + std::unique_ptr stream = m_secure.Get(); if (!stream) { @@ -184,7 +262,7 @@ namespace AppInstaller::Settings } catch (const std::runtime_error& e) { - AICLI_LOG(YAML, Error, << "Secure setting metadata for '" << name << "' contained invalid YAML (" << e.what() << "):\n" << streamContents); + AICLI_LOG(Core, Error, << "Secure setting metadata for '" << m_name << "' contained invalid YAML (" << e.what() << "):\n" << streamContents); return {}; } @@ -196,7 +274,7 @@ namespace AppInstaller::Settings } catch (const std::runtime_error& e) { - AICLI_LOG(YAML, Error, << "Secure setting metadata for '" << name << "' contained invalid YAML (" << e.what() << "):\n" << streamContents); + AICLI_LOG(Core, Error, << "Secure setting metadata for '" << m_name << "' contained invalid YAML (" << e.what() << "):\n" << streamContents); return {}; } @@ -207,19 +285,20 @@ namespace AppInstaller::Settings return result; } - void SetVerificationData(const std::filesystem::path& name, VerificationData data) + void SetVerificationData(VerificationData data) { YAML::Emitter out; out << YAML::BeginMap; out << YAML::Key << NodeName_Sha256 << YAML::Value << SHA256::ConvertToString(data.Hash); out << YAML::EndMap; - m_secure.Set(name, out.str()); + m_secure.Set(out.str()); } - std::unique_ptr Get(const std::filesystem::path& name) override + public: + std::unique_ptr Get() override { - std::unique_ptr stream = m_container->Get(name); + std::unique_ptr stream = GetInternal(); if (!stream) { @@ -227,71 +306,60 @@ namespace AppInstaller::Settings return stream; } - VerificationData verData = GetVerificationData(name); + VerificationData verData = GetVerificationData(); // This case should be very rare, so a very identifiable error is helpful. // Plus the text for this one is fairly on point for what has happened. THROW_HR_IF(SPAPI_E_FILE_HASH_NOT_IN_CATALOG, !verData.Found); - std::string streamContents = Utility::ReadEntireStream(*stream); - THROW_HR_IF(E_UNEXPECTED, streamContents.size() > std::numeric_limits::max()); - - auto streamHash = SHA256::ComputeHash(reinterpret_cast(streamContents.c_str()), static_cast(streamContents.size())); + THROW_HR_IF(HRESULT_FROM_WIN32(ERROR_DATA_CHECKSUM_ERROR), !AreHashesEqual(m_hash.value(), verData.Hash)); - THROW_HR_IF(HRESULT_FROM_WIN32(ERROR_DATA_CHECKSUM_ERROR), !std::equal(streamHash.begin(), streamHash.end(), verData.Hash.begin())); - - // Return a stream over the contents that we read in and verified, to prevent a race attack. - return std::make_unique(streamContents); + // ExchangeSettingsContainer already produces an in memory stream that we can use. + return stream; } - void Set(const std::filesystem::path& name, std::string_view value) override + bool Set(std::string_view value) override { - THROW_HR_IF(E_UNEXPECTED, value.size() > std::numeric_limits::max()); + bool exchangeResult = ExchangeSettingsContainer::Set(value); - VerificationData verData; - verData.Hash = SHA256::ComputeHash(reinterpret_cast(value.data()), static_cast(value.size())); + if (exchangeResult) + { + VerificationData verData; + verData.Hash = m_hash.value(); - SetVerificationData(name, verData); + SetVerificationData(verData); + } - m_container->Set(name, value); + return exchangeResult; } - void Remove(const std::filesystem::path& name) override + void Remove() override { - m_secure.Remove(name); - m_container->Remove(name); + ExchangeSettingsContainer::Remove(); + m_secure.Remove(); } - std::filesystem::path PathTo(const std::filesystem::path&) override + std::filesystem::path PathTo() override { THROW_HR(E_UNEXPECTED); } private: - std::unique_ptr m_container; FileSettingsContainer m_secure; }; - std::unique_ptr GetSettingsContainer(const StreamDefinition& streamDefinition) + std::unique_ptr GetRawSettingsContainer(Type type, const std::string_view& name) { - if (streamDefinition.Type == Type::Secure) - { - StreamDefinition standardDefinition = streamDefinition; - standardDefinition.Type = Type::Standard; - return std::make_unique(GetSettingsContainer(standardDefinition)); - } - #ifndef WINGET_DISABLE_FOR_FUZZING if (IsRunningInPackagedContext()) { - switch (streamDefinition.Type) + switch (type) { case Type::Standard: return std::make_unique( ApplicationDataSettingsContainer::GetRelativeContainer( - winrt::Windows::Storage::ApplicationData::Current().LocalSettings(), GetPathTo(PathName::StandardSettings))); - case Type::UserFile: - return std::make_unique(GetPathTo(PathName::UserFileSettings)); + winrt::Windows::Storage::ApplicationData::Current().LocalSettings(), GetPathTo(PathName::StandardSettings)), + name); default: THROW_HR(E_UNEXPECTED); } @@ -302,14 +370,39 @@ namespace AppInstaller::Settings switch (type) { case Type::Standard: - return std::make_unique(GetPathTo(PathName::StandardSettings)); - case Type::UserFile: - return std::make_unique(GetPathTo(PathName::UserFileSettings)); + return std::make_unique(GetPathTo(PathName::StandardSettings), name); default: THROW_HR(E_UNEXPECTED); } } } + + // The default is not a raw container, so we wrap some of the underlying containers to enable higher order behaviors. + std::unique_ptr GetSettingsContainer(Type type, const std::string_view& name) + { + switch (type) + { + case Type::Standard: + // Standard settings should use exchange semantics to prevent overwrites + return std::make_unique(GetRawSettingsContainer(type, name), name); + + case Type::UserFile: + // User file settings are not typically modified by us, so there is no need for exchange + return std::make_unique(GetPathTo(PathName::UserFileSettings), name); + + case Type::Secure: + // Secure settings add hash verification on reads on top of exchange semantics + return std::make_unique(GetRawSettingsContainer(Type::Standard, name), name); + + default: + THROW_HR(E_UNEXPECTED); + } + } + + std::unique_ptr GetSettingsContainer(const StreamDefinition& streamDefinition) + { + return GetSettingsContainer(streamDefinition.Type, streamDefinition.Name); + } } std::string_view ToString(Type type) @@ -327,51 +420,37 @@ namespace AppInstaller::Settings } } - std::unique_ptr GetSettingStream(const StreamDefinition& def) + Stream::Stream(const StreamDefinition& streamDefinition) : + m_streamDefinition(streamDefinition), m_container(GetSettingsContainer(streamDefinition)) { - LogSettingAction("Get", def); - ValidateSettingNamePath(def.Name); - return GetSettingsContainer(def.Type)->Get(def.Name); + ValidateSettingNamePath(m_streamDefinition.Name); } - void SetSetting(const StreamDefinition& def, std::string_view value) + std::unique_ptr Stream::Get() { - LogSettingAction("Set", def); - ValidateSettingNamePath(def.Name); - GetSettingsContainer(def.Type)->Set(def.Name, value); + LogSettingAction("Get", m_streamDefinition); + return m_container->Get(); } - void RemoveSetting(const StreamDefinition& def) + [[nodiscard]] bool Stream::Set(std::string_view value) { - LogSettingAction("Remove", def); - ValidateSettingNamePath(def.Name); - GetSettingsContainer(def.Type)->Remove(def.Name); + LogSettingAction("Set", m_streamDefinition); + return m_container->Set(value); } - std::filesystem::path GetPathTo(const StreamDefinition& def) + void Stream::Remove() { - return GetSettingsContainer(def.Type)->PathTo(def.Name); + LogSettingAction("Remove", m_streamDefinition); + m_container->Remove(); } - Stream::Stream(const StreamDefinition& streamDefinition) : - m_streamDefinition(streamDefinition), m_container(GetSettingsContainer(streamDefinition)) + std::string_view Stream::GetName() const { + return m_streamDefinition.Name; } - // Gets the stream if present. - // If the setting stream does not exist, returns an empty value (see operator bool). - std::unique_ptr Get(); - - // Sets the stream to the given value. - // Returns true if successful; false if the underlying stream has changed. - [[nodiscard]] bool Set(std::string_view value); - - // Deletes the setting stream. - void Remove(); - - // Gets the name of the stream. - std::string_view GetName() const; - - // Gets the path to the stream. - std::filesystem::path GetPath() const; + std::filesystem::path Stream::GetPath() const + { + return m_container->PathTo(); + } } From 8b9268f034b221e3c159d551b26f0b64b7b481ad Mon Sep 17 00:00:00 2001 From: JohnMcPMS Date: Thu, 30 Sep 2021 14:24:12 -0700 Subject: [PATCH 5/7] Existing unit tests pass --- src/AppInstallerRepositoryCore/SourceList.cpp | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/AppInstallerRepositoryCore/SourceList.cpp b/src/AppInstallerRepositoryCore/SourceList.cpp index bdf94cf3ff..d0b8375057 100644 --- a/src/AppInstallerRepositoryCore/SourceList.cpp +++ b/src/AppInstallerRepositoryCore/SourceList.cpp @@ -190,6 +190,11 @@ namespace AppInstaller::Repository left.Identifier == right.Identifier && Utility::CaseInsensitiveEquals(left.Type, right.Type); } + + bool ShouldBeHidden(const SourceDetailsInternal& details) + { + return details.IsTombstone || details.Origin == SourceOrigin::Metadata; + } } std::string_view GetWellKnownSourceName(WellKnownSource source) @@ -299,13 +304,13 @@ namespace AppInstaller::Repository for (auto& s : m_sourceList) { - if (!s.IsTombstone) + if (!ShouldBeHidden(s)) { result.emplace_back(std::ref(s)); } else { - AICLI_LOG(Repo, Info, << "GetCurrentSourceRefs: Source named '" << s.Name << "' from origin " << ToString(s.Origin) << " is a tombstone and is dropped."); + AICLI_LOG(Repo, Info, << "GetCurrentSourceRefs: Source named '" << s.Name << "' from origin " << ToString(s.Origin) << " is hidden and is dropped."); } } @@ -318,7 +323,7 @@ namespace AppInstaller::Repository [name, includeHidden](const SourceDetailsInternal& sd) { return Utility::ICUCaseInsensitiveEquals(sd.Name, name) && - (includeHidden || (!sd.IsTombstone && sd.Origin != SourceOrigin::Metadata)); + (includeHidden || !ShouldBeHidden(sd)); }); } From 420751b9adf03c9843c28c015dc53b9a3c9717e7 Mon Sep 17 00:00:00 2001 From: JohnMcPMS Date: Thu, 30 Sep 2021 19:34:09 -0700 Subject: [PATCH 6/7] Unit tests added and pass --- src/AppInstallerCLITests/Settings.cpp | 95 ++++++++++ src/AppInstallerCLITests/Sources.cpp | 179 ++++++++++++++++++ src/AppInstallerCommonCore/Settings.cpp | 37 ++-- src/AppInstallerRepositoryCore/SourceList.cpp | 44 +++-- src/AppInstallerRepositoryCore/SourceList.h | 3 + 5 files changed, 326 insertions(+), 32 deletions(-) diff --git a/src/AppInstallerCLITests/Settings.cpp b/src/AppInstallerCLITests/Settings.cpp index a0bb5c4fac..146b41cff8 100644 --- a/src/AppInstallerCLITests/Settings.cpp +++ b/src/AppInstallerCLITests/Settings.cpp @@ -186,3 +186,98 @@ TEST_CASE("SetAndReadSecureSetting_DataTampered", "[settings]") REQUIRE_THROWS_HR(stream.Get(), HRESULT_FROM_WIN32(ERROR_DATA_CHECKSUM_ERROR)); } + +TEST_CASE("SetChangeAndReadSetting", "[settings]") +{ + StreamDefinition name{ Type::Standard, "testsettingname" }; + std::string value1 = "This is the test setting value1"; + std::string value2 = "This is the test setting value2, which is different"; + std::string value3 = "This is the test setting value3; also different"; + + name.Type = GENERATE(Type::Standard, Type::Secure); + INFO(ToString(name.Type)); + + // Set the value on stream 1 + Stream stream1{ name }; + REQUIRE(stream1.Set(value1)); + + // Read the value on stream 2 to verify + { + Stream stream2{ name }; + + auto result = stream2.Get(); + REQUIRE(static_cast(result)); + + std::string settingValue = ReadEntireStream(*result); + REQUIRE(value1 == settingValue); + + // Set the value on stream 2 + REQUIRE(stream2.Set(value2)); + } + + // Attempt to set the value on stream 1 again + REQUIRE(!stream1.Set(value3)); + + // Attempting to set again should still not work + REQUIRE(!stream1.Set(value3)); + + // Ensure that the value remains value 2 + auto result = stream1.Get(); + REQUIRE(static_cast(result)); + + std::string settingValue = ReadEntireStream(*result); + REQUIRE(value2 == settingValue); + + // Now that we have read it, we can update it + REQUIRE(stream1.Set(value3)); + + result = stream1.Get(); + REQUIRE(static_cast(result)); + + settingValue = ReadEntireStream(*result); + REQUIRE(value3 == settingValue); +} + +TEST_CASE("AttemptSetOnNewValue", "[settings]") +{ + StreamDefinition name{ Type::Standard, "testsettingname" }; + std::string value1 = "This is the test setting value1"; + std::string value2 = "This is the test setting value2, which is different"; + + name.Type = GENERATE(Type::Standard, Type::Secure); + INFO(ToString(name.Type)); + + // Remove the stream + Stream{ name }.Remove(); + + Stream stream1{ name }; + REQUIRE(!stream1.Get()); + + // Set the value on stream 2 + { + Stream stream2{ name }; + REQUIRE(stream2.Set(value1)); + } + + // Attempt to set the value on stream 1 again + REQUIRE(!stream1.Set(value2)); + + // Attempting to set again should still not work + REQUIRE(!stream1.Set(value2)); + + // Ensure that the value remains value 2 + auto result = stream1.Get(); + REQUIRE(static_cast(result)); + + std::string settingValue = ReadEntireStream(*result); + REQUIRE(value1 == settingValue); + + // Now that we have read it, we can update it + REQUIRE(stream1.Set(value2)); + + result = stream1.Get(); + REQUIRE(static_cast(result)); + + settingValue = ReadEntireStream(*result); + REQUIRE(value2 == settingValue); +} diff --git a/src/AppInstallerCLITests/Sources.cpp b/src/AppInstallerCLITests/Sources.cpp index 2bbe51d16e..d30ef49304 100644 --- a/src/AppInstallerCLITests/Sources.cpp +++ b/src/AppInstallerCLITests/Sources.cpp @@ -60,6 +60,40 @@ constexpr std::string_view s_SingleSource = R"( IsTombstone: false )"sv; +constexpr std::string_view s_SingleSourceMetadata = R"( +Sources: + - Name: testName + LastUpdate: 100 +)"sv; + +constexpr std::string_view s_SingleSourceMetadataUpdate = R"( +Sources: + - Name: testName + LastUpdate: 101 +)"sv; + +constexpr std::string_view s_DoubleSource = R"( +Sources: + - Name: testName + Type: testType + Arg: testArg + Data: testData + IsTombstone: false + - Name: testName2 + Type: testType + Arg: testArg2 + Data: testData2 + IsTombstone: false +)"sv; + +constexpr std::string_view s_DoubleSourceMetadata = R"( +Sources: + - Name: testName + LastUpdate: 100 + - Name: testName2 + LastUpdate: 200 +)"sv; + constexpr std::string_view s_ThreeSources = R"( Sources: - Name: testName @@ -1037,3 +1071,148 @@ TEST_CASE("RepoSources_OpenMultipleWithTotalFailure", "[sources]") ProgressCallback progress; REQUIRE_THROWS_HR(OpenSource("", progress), APPINSTALLER_CLI_ERROR_FAILED_TO_OPEN_ALL_SOURCES); } + +TEST_CASE("RepoSources_UpdateSettingsDuringAction_SourcesUpdate", "[sources]") +{ + SetSetting(Stream::UserSources, s_SingleSource); + SetSetting(Stream::SourcesMetadata, s_SingleSourceMetadata); + + std::string userSourcesUpdate{ s_DoubleSource }; + std::string sourcesMetadataUpdate{ s_DoubleSourceMetadata }; + + std::string singleSourceName = "testName"; + std::string doubleSourceName = "testName2"; + + std::string unusedSourceName = "unusedName"; + std::string unusedSourceArg = "unusedArg"; + std::string testSourceType = "testType"; + + TestHook_ClearSourceFactoryOverrides(); + TestSourceFactory factory{ FailingSourcesTestSource::CreateFailAll }; + auto settingsUpdate = [&](const AppInstaller::Repository::SourceDetails&) + { + SetSetting(Stream::UserSources, userSourcesUpdate); + SetSetting(Stream::SourcesMetadata, sourcesMetadataUpdate); + }; + factory.OnAdd = settingsUpdate; + factory.OnUpdate = settingsUpdate; + factory.OnRemove = settingsUpdate; + TestHook_SetSourceFactoryOverride(testSourceType, factory); + + ProgressCallback progress; + + SECTION("Add") + { + SourceDetails addedSource; + addedSource.Name = unusedSourceName; + addedSource.Type = testSourceType; + addedSource.Arg = unusedSourceArg; + AddSource(addedSource, progress); + + auto sources = GetSources(); + REQUIRE(sources.size() == 3 + c_DefaultSourceCount); + + REQUIRE(sources[0].Name == singleSourceName); + REQUIRE(sources[1].Name == doubleSourceName); + REQUIRE(sources[2].Name == addedSource.Name); + } + SECTION("Add conflicting") + { + SourceDetails addedSource; + addedSource.Name = doubleSourceName; + addedSource.Type = testSourceType; + addedSource.Arg = unusedSourceArg; + REQUIRE_THROWS_HR(AddSource(addedSource, progress), APPINSTALLER_CLI_ERROR_SOURCE_NAME_ALREADY_EXISTS); + } + SECTION("Update") + { + UpdateSource(singleSourceName, progress); + + auto sources = GetSources(); + REQUIRE(sources.size() == 2 + c_DefaultSourceCount); + + REQUIRE(sources[0].Name == singleSourceName); + REQUIRE(sources[1].Name == doubleSourceName); + } + SECTION("Remove") + { + RemoveSource(singleSourceName, progress); + + auto sources = GetSources(); + REQUIRE(sources.size() == 1 + c_DefaultSourceCount); + + REQUIRE(sources[0].Name == doubleSourceName); + } + SECTION("Remove already removed") + { + userSourcesUpdate = s_EmptySources; + sourcesMetadataUpdate = s_EmptySources; + + RemoveSource(singleSourceName, progress); + + auto sources = GetSources(); + REQUIRE(sources.size() == c_DefaultSourceCount); + } +} + +TEST_CASE("RepoSources_UpdateSettingsDuringAction_MetadataUpdate", "[sources]") +{ + SetSetting(Stream::UserSources, s_SingleSource); + SetSetting(Stream::SourcesMetadata, s_SingleSourceMetadata); + + std::string sourcesMetadataUpdate{ s_SingleSourceMetadataUpdate }; + int64_t updateTime = 101; + + std::string singleSourceName = "testName"; + std::string doubleSourceName = "testName2"; + + std::string unusedSourceName = "unusedName"; + std::string unusedSourceArg = "unusedArg"; + std::string testSourceType = "testType"; + + TestHook_ClearSourceFactoryOverrides(); + TestSourceFactory factory{ FailingSourcesTestSource::CreateFailAll }; + auto settingsUpdate = [&](const AppInstaller::Repository::SourceDetails&) + { + SetSetting(Stream::SourcesMetadata, sourcesMetadataUpdate); + }; + factory.OnAdd = settingsUpdate; + factory.OnUpdate = settingsUpdate; + factory.OnRemove = settingsUpdate; + TestHook_SetSourceFactoryOverride(testSourceType, factory); + + ProgressCallback progress; + + SECTION("Add") + { + SourceDetails addedSource; + addedSource.Name = unusedSourceName; + addedSource.Type = testSourceType; + addedSource.Arg = unusedSourceArg; + AddSource(addedSource, progress); + + auto sources = GetSources(); + REQUIRE(sources.size() == 2 + c_DefaultSourceCount); + + REQUIRE(sources[0].Name == singleSourceName); + REQUIRE(ConvertSystemClockToUnixEpoch(sources[0].LastUpdateTime) == updateTime); + REQUIRE(sources[1].Name == addedSource.Name); + } + SECTION("Update") + { + UpdateSource(singleSourceName, progress); + + auto sources = GetSources(); + REQUIRE(sources.size() == 1 + c_DefaultSourceCount); + + REQUIRE(sources[0].Name == singleSourceName); + REQUIRE(ConvertSystemClockToUnixEpoch(sources[0].LastUpdateTime) > updateTime); + } + SECTION("Remove") + { + RemoveSource(singleSourceName, progress); + + auto sources = GetSources(); + REQUIRE(sources.size() == c_DefaultSourceCount); + } +} diff --git a/src/AppInstallerCommonCore/Settings.cpp b/src/AppInstallerCommonCore/Settings.cpp index b4069fbbdc..9894dcb0b6 100644 --- a/src/AppInstallerCommonCore/Settings.cpp +++ b/src/AppInstallerCommonCore/Settings.cpp @@ -151,7 +151,7 @@ namespace AppInstaller::Settings std::unique_ptr Get() override { - return GetInternal(); + return GetInternal(m_hash); } bool Set(std::string_view value) override @@ -160,14 +160,13 @@ namespace AppInstaller::Settings // If Set is called without ever reading the value, then we can assume that caller wants // to overwrite it regardless. Also, we don't have any previous value to compare against - // anyway so the only other option would be to always reject it, but then we would need - // to distinguish the state where the value has never existed. + // anyway so the only other option would be to always reject it. if (m_hash) { - SHA256::HashBuffer previousHash = m_hash.value(); - std::ignore = GetInternal(); + std::optional currentHash; + std::ignore = GetInternal(currentHash); - if (m_hash && !AreHashesEqual(previousHash, m_hash.value())) + if (currentHash && !AreHashesEqual(m_hash.value(), currentHash.value())) { AICLI_LOG(Core, Verbose, << "Setting value for '" << m_name << "' has changed since last read; rejecting Set"); return false; @@ -198,34 +197,36 @@ namespace AppInstaller::Settings } protected: - std::unique_ptr GetInternal() + static bool AreHashesEqual(const SHA256::HashBuffer& a, const SHA256::HashBuffer& b) + { + return (a.size() == b.size() && std::equal(a.begin(), a.end(), b.begin())); + } + + std::string_view m_name; + std::optional m_hash; + + private: + std::unique_ptr GetInternal(std::optional& hashStorage) { std::unique_ptr stream = m_container->Get(); if (!stream) { // If no stream exists, then no hashing needs to be done. + // Return an empty hash vector to indicate the attempted read but no result. + hashStorage.emplace(); return stream; } std::string streamContents = Utility::ReadEntireStream(*stream); THROW_HR_IF(E_UNEXPECTED, streamContents.size() > std::numeric_limits::max()); - m_hash = SHA256::ComputeHash(reinterpret_cast(streamContents.c_str()), static_cast(streamContents.size())); + hashStorage = SHA256::ComputeHash(reinterpret_cast(streamContents.c_str()), static_cast(streamContents.size())); // Return a stream over the contents that we read in and hashed, to prevent a race. return std::make_unique(streamContents); } - static bool AreHashesEqual(const SHA256::HashBuffer& a, const SHA256::HashBuffer& b) - { - return std::equal(a.begin(), a.end(), b.begin()); - } - - std::string_view m_name; - std::optional m_hash; - - private: std::unique_ptr m_container; }; @@ -298,7 +299,7 @@ namespace AppInstaller::Settings public: std::unique_ptr Get() override { - std::unique_ptr stream = GetInternal(); + std::unique_ptr stream = ExchangeSettingsContainer::Get(); if (!stream) { diff --git a/src/AppInstallerRepositoryCore/SourceList.cpp b/src/AppInstallerRepositoryCore/SourceList.cpp index d0b8375057..92d10c7126 100644 --- a/src/AppInstallerRepositoryCore/SourceList.cpp +++ b/src/AppInstallerRepositoryCore/SourceList.cpp @@ -197,6 +197,13 @@ namespace AppInstaller::Repository } } + void SourceDetailsInternal::CopyMetadataFieldsTo(SourceDetailsInternal& target) + { + target.LastUpdateTime = LastUpdateTime; + target.AcceptedAgreementFields = AcceptedAgreementFields; + target.AcceptedAgreementsIdentifier = AcceptedAgreementsIdentifier; + } + std::string_view GetWellKnownSourceName(WellKnownSource source) { switch (source) @@ -376,8 +383,11 @@ namespace AppInstaller::Repository SaveMetadataInternal(details); } - void SourceList::RemoveSource(const SourceDetailsInternal& details) + void SourceList::RemoveSource(const SourceDetailsInternal& detailsRef) { + // Copy the incoming details because we might destroy the referenced structure + // when reloading the source details from settings. + SourceDetailsInternal details = detailsRef; bool sourcesSet = false; for (size_t i = 0; !sourcesSet && i < 10; ++i) @@ -524,9 +534,7 @@ namespace AppInstaller::Repository auto source = GetSource(metaSource.Name); if (source) { - source->LastUpdateTime = metaSource.LastUpdateTime; - source->AcceptedAgreementFields = metaSource.AcceptedAgreementFields; - source->AcceptedAgreementsIdentifier = metaSource.AcceptedAgreementsIdentifier; + metaSource.CopyMetadataFieldsTo(*source); } else { @@ -677,8 +685,11 @@ namespace AppInstaller::Repository return m_metadataStream.Set(out.str()); } - void SourceList::SaveMetadataInternal(const SourceDetailsInternal& details, bool remove) + void SourceList::SaveMetadataInternal(const SourceDetailsInternal& detailsRef, bool remove) { + // Copy the incoming details because we might overwrite the metadata + // when reloading the source details from settings. + SourceDetailsInternal details = detailsRef; bool metadataSet = false; for (size_t i = 0; !metadataSet && i < 10; ++i) @@ -689,19 +700,24 @@ namespace AppInstaller::Repository { OverwriteMetadata(); - // The remove will have removed the source but not the metadata. - // Remove it again here. - if (remove) + auto target = FindSource(details.Name, true); + if (target == m_sourceList.end()) { - auto target = FindSource(details.Name, true); - if (target == m_sourceList.end()) - { - // Didn't find the metadata, so we consider this a success - return; - } + // Didn't find the metadata, so we consider this a success + return; + } + if (remove) + { + // The remove will have removed the source but not the metadata. + // Remove it again here. m_sourceList.erase(target); } + else + { + // Update the freshly read metadata with the update that was requested. + details.CopyMetadataFieldsTo(*target); + } } } diff --git a/src/AppInstallerRepositoryCore/SourceList.h b/src/AppInstallerRepositoryCore/SourceList.h index 1c5a5a2eec..3a0c92678f 100644 --- a/src/AppInstallerRepositoryCore/SourceList.h +++ b/src/AppInstallerRepositoryCore/SourceList.h @@ -18,6 +18,9 @@ namespace AppInstaller::Repository SourceDetailsInternal() = default; SourceDetailsInternal(const SourceDetails& details) : SourceDetails(details) {} + // Copies the metadata fields from this to target. + void CopyMetadataFieldsTo(SourceDetailsInternal& target); + // If true, this is a tombstone, marking the deletion of a source at a lower priority origin. bool IsTombstone = false; std::string AcceptedAgreementsIdentifier; From 62a216b2105c85282159747854c6b31e2cc0e38e Mon Sep 17 00:00:00 2001 From: JohnMcPMS Date: Mon, 4 Oct 2021 11:38:27 -0700 Subject: [PATCH 7/7] Move function to more reusable place --- src/AppInstallerCommonCore/Public/AppInstallerSHA256.h | 3 +++ src/AppInstallerCommonCore/SHA256.cpp | 5 +++++ src/AppInstallerCommonCore/Settings.cpp | 9 ++------- 3 files changed, 10 insertions(+), 7 deletions(-) diff --git a/src/AppInstallerCommonCore/Public/AppInstallerSHA256.h b/src/AppInstallerCommonCore/Public/AppInstallerSHA256.h index d6049b3021..45a65044df 100644 --- a/src/AppInstallerCommonCore/Public/AppInstallerSHA256.h +++ b/src/AppInstallerCommonCore/Public/AppInstallerSHA256.h @@ -53,6 +53,9 @@ namespace AppInstaller::Utility { static HashBuffer ConvertToBytes(const std::string& hashStr); + // Returns a value indicating whether the two hashes are equal. + static bool AreEqual(const HashBuffer& first, const HashBuffer& second); + private: void EnsureNotFinished() const; diff --git a/src/AppInstallerCommonCore/SHA256.cpp b/src/AppInstallerCommonCore/SHA256.cpp index ff21a176cd..9b97ec9399 100644 --- a/src/AppInstallerCommonCore/SHA256.cpp +++ b/src/AppInstallerCommonCore/SHA256.cpp @@ -171,6 +171,11 @@ namespace AppInstaller::Utility { delete context; } + bool SHA256::AreEqual(const HashBuffer& first, const HashBuffer& second) + { + return (first.size() == second.size() && std::equal(first.begin(), first.end(), second.begin())); + } + void SHA256::EnsureNotFinished() const { if (!context) diff --git a/src/AppInstallerCommonCore/Settings.cpp b/src/AppInstallerCommonCore/Settings.cpp index 9894dcb0b6..94edc39bef 100644 --- a/src/AppInstallerCommonCore/Settings.cpp +++ b/src/AppInstallerCommonCore/Settings.cpp @@ -166,7 +166,7 @@ namespace AppInstaller::Settings std::optional currentHash; std::ignore = GetInternal(currentHash); - if (currentHash && !AreHashesEqual(m_hash.value(), currentHash.value())) + if (currentHash && !SHA256::AreEqual(m_hash.value(), currentHash.value())) { AICLI_LOG(Core, Verbose, << "Setting value for '" << m_name << "' has changed since last read; rejecting Set"); return false; @@ -197,11 +197,6 @@ namespace AppInstaller::Settings } protected: - static bool AreHashesEqual(const SHA256::HashBuffer& a, const SHA256::HashBuffer& b) - { - return (a.size() == b.size() && std::equal(a.begin(), a.end(), b.begin())); - } - std::string_view m_name; std::optional m_hash; @@ -313,7 +308,7 @@ namespace AppInstaller::Settings // Plus the text for this one is fairly on point for what has happened. THROW_HR_IF(SPAPI_E_FILE_HASH_NOT_IN_CATALOG, !verData.Found); - THROW_HR_IF(HRESULT_FROM_WIN32(ERROR_DATA_CHECKSUM_ERROR), !AreHashesEqual(m_hash.value(), verData.Hash)); + THROW_HR_IF(HRESULT_FROM_WIN32(ERROR_DATA_CHECKSUM_ERROR), !SHA256::AreEqual(m_hash.value(), verData.Hash)); // ExchangeSettingsContainer already produces an in memory stream that we can use. return stream;