commit 741955e48ce2f069ab323f71e40dd1326654e7ab parent a9a63a35e4607571648f33d1d8800ad00117df64 Author: Luis Chacón <lechacon@users.noreply.github.com> Date: Fri, 2 Apr 2021 16:18:05 -0700 Group Policy for controlling sources (#841) Diffstat:
22 files changed, 1585 insertions(+), 317 deletions(-)
diff --git a/src/AppInstallerCLICore/Command.cpp b/src/AppInstallerCLICore/Command.cpp @@ -325,7 +325,7 @@ namespace AppInstaller::CLI { auto policy = TogglePolicy::GetPolicy(command->GroupPolicy()); AICLI_LOG(CLI, Error, << "Trying to use command: " << *itr << " disabled by group policy " << policy.RegValueName()); - throw CommandException(Resource::String::DisabledByGroupPolicy, policy.PolicyName()); + throw GroupPolicyException(command->GroupPolicy()); } AICLI_LOG(CLI, Info, << "Found subcommand: " << *itr); @@ -657,7 +657,7 @@ namespace AppInstaller::CLI { auto policy = TogglePolicy::GetPolicy(arg.GroupPolicy()); AICLI_LOG(CLI, Error, << "Trying to use argument: " << arg.Name() << " disabled by group policy " << policy.RegValueName()); - throw CommandException(Resource::String::DisabledByGroupPolicy, policy.PolicyName()); + throw GroupPolicyException(arg.GroupPolicy()); } if (arg.Required() && !execArgs.Contains(arg.ExecArgType())) @@ -783,10 +783,8 @@ namespace AppInstaller::CLI // Override the function to bypass this. if (!Settings::GroupPolicies().IsEnabled(Settings::TogglePolicy::Policy::WinGet)) { - auto policy = TogglePolicy::GetPolicy(Settings::TogglePolicy::Policy::WinGet); - AICLI_LOG(CLI, Error, << "WinGet is disabled by group policy " << policy.RegValueName()); - context.Reporter.Error() << Resource::String::DisabledByGroupPolicy << " : "_liv << policy.PolicyName() << std::endl; - AICLI_TERMINATE_CONTEXT(APPINSTALLER_CLI_ERROR_BLOCKED_BY_POLICY); + AICLI_LOG(CLI, Error, << "WinGet is disabled by group policy"); + throw GroupPolicyException(Settings::TogglePolicy::Policy::WinGet); } AICLI_LOG(CLI, Info, << "Executing command: " << Name()); @@ -883,6 +881,12 @@ namespace AppInstaller::CLI message << std::endl; return hre.code(); } + catch (const Settings::GroupPolicyException& e) + { + auto policy = Settings::TogglePolicy::GetPolicy(e.Policy()); + context.Reporter.Error() << Resource::String::DisabledByGroupPolicy << ": "_liv << policy.PolicyName() << std::endl; + return APPINSTALLER_CLI_ERROR_BLOCKED_BY_POLICY; + } catch (const std::exception& e) { Logging::Telemetry().LogException(command->FullName(), "std::exception", e.what()); diff --git a/src/AppInstallerCLICore/Commands/CompleteCommand.cpp b/src/AppInstallerCLICore/Commands/CompleteCommand.cpp @@ -69,6 +69,10 @@ namespace AppInstaller::CLI { AICLI_LOG(CLI, Info, << "Error encountered during completion, ignoring: " << ce.Message()); } + catch (const Settings::GroupPolicyException& e) + { + AICLI_LOG(CLI, Info, << "Error encountered during completion, ignoring: Blocked by Group Policy " << Settings::TogglePolicy::GetPolicy(e.Policy()).RegValueName()); + } catch (...) { AICLI_LOG(CLI, Info, << "Error encountered during completion, ignoring..."); diff --git a/src/AppInstallerCLICore/Commands/SourceCommand.cpp b/src/AppInstallerCLICore/Commands/SourceCommand.cpp @@ -22,6 +22,7 @@ namespace AppInstaller::CLI std::make_unique<SourceUpdateCommand>(FullName()), std::make_unique<SourceRemoveCommand>(FullName()), std::make_unique<SourceResetCommand>(FullName()), + std::make_unique<SourceExportCommand>(FullName()), }); } @@ -71,6 +72,8 @@ namespace AppInstaller::CLI void SourceAddCommand::ExecuteInternal(Context& context) const { + // Note: Group Policy for allowed sources is enforced at the RepositoryCore level + // as we need to validate the source data and handle sources that were already added. context << Workflow::EnsureRunningAsAdmin << Workflow::GetSourceList << @@ -187,6 +190,7 @@ namespace AppInstaller::CLI void SourceRemoveCommand::ExecuteInternal(Context& context) const { + // Note: Group Policy for unremovable sources is enforced at the RepositoryCore. context << Workflow::EnsureRunningAsAdmin << Workflow::GetSourceListWithFilter << @@ -242,4 +246,42 @@ namespace AppInstaller::CLI Workflow::ResetAllSources; } } + + std::vector<Argument> SourceExportCommand::GetArguments() const + { + return { + Argument::ForType(Args::Type::SourceName), + }; + } + + Resource::LocString SourceExportCommand::ShortDescription() const + { + return { Resource::String::SourceExportCommandShortDescription }; + } + + Resource::LocString SourceExportCommand::LongDescription() const + { + return { Resource::String::SourceExportCommandLongDescription }; + } + + void SourceExportCommand::Complete(Context& context, Args::Type valueType) const + { + if (valueType == Args::Type::SourceName) + { + context << + Workflow::CompleteSourceName; + } + } + + std::string SourceExportCommand::HelpLink() const + { + return std::string{ s_SourceCommand_HelpLink }; + } + + void SourceExportCommand::ExecuteInternal(Context& context) const + { + context << + Workflow::GetSourceListWithFilter << + Workflow::ExportSourceList; + } } diff --git a/src/AppInstallerCLICore/Commands/SourceCommand.h b/src/AppInstallerCLICore/Commands/SourceCommand.h @@ -22,7 +22,7 @@ namespace AppInstaller::CLI struct SourceAddCommand final : public Command { - SourceAddCommand(std::string_view parent) : Command("add", parent) {} + SourceAddCommand(std::string_view parent) : Command("add", parent, Settings::TogglePolicy::Policy::AllowedSources) {} std::vector<Argument> GetArguments() const override; @@ -71,6 +71,7 @@ namespace AppInstaller::CLI struct SourceRemoveCommand final : public Command { + // We can remove user or default sources, so this is not gated by any single policy. SourceRemoveCommand(std::string_view parent) : Command("remove", parent) {} std::vector<Argument> GetArguments() const override; @@ -102,4 +103,21 @@ namespace AppInstaller::CLI protected: void ExecuteInternal(Execution::Context& context) const override; }; + + struct SourceExportCommand final : public Command + { + SourceExportCommand(std::string_view parent) : Command("export", parent) {} + + std::vector<Argument> GetArguments() const override; + + Resource::LocString ShortDescription() const override; + Resource::LocString LongDescription() const override; + + void Complete(Execution::Context& context, Execution::Args::Type valueType) const override; + + std::string HelpLink() const override; + + protected: + void ExecuteInternal(Execution::Context& context) const override; + }; } diff --git a/src/AppInstallerCLICore/Core.cpp b/src/AppInstallerCLICore/Core.cpp @@ -115,6 +115,14 @@ namespace AppInstaller::CLI AICLI_LOG(CLI, Error, << "Error encountered parsing command line: " << ce.Message()); return APPINSTALLER_CLI_ERROR_INVALID_CL_ARGUMENTS; } + catch (const Settings::GroupPolicyException& e) + { + // Report any action blocked by Group Policy. + auto policy = Settings::TogglePolicy::GetPolicy(e.Policy()); + AICLI_LOG(CLI, Error, << "Operation blocked by Group Policy: " << policy.RegValueName()); + context.Reporter.Error() << Resource::String::DisabledByGroupPolicy << " : "_liv << policy.PolicyName() << std::endl; + return APPINSTALLER_CLI_ERROR_BLOCKED_BY_POLICY; + } return Execute(context, command); } diff --git a/src/AppInstallerCLICore/Resources.h b/src/AppInstallerCLICore/Resources.h @@ -188,6 +188,8 @@ namespace AppInstaller::CLI::Resource WINGET_DEFINE_RESOURCE_STRINGID(SourceArgumentDescription); WINGET_DEFINE_RESOURCE_STRINGID(SourceCommandLongDescription); WINGET_DEFINE_RESOURCE_STRINGID(SourceCommandShortDescription); + WINGET_DEFINE_RESOURCE_STRINGID(SourceExportCommandLongDescription); + WINGET_DEFINE_RESOURCE_STRINGID(SourceExportCommandShortDescription); WINGET_DEFINE_RESOURCE_STRINGID(SourceListArg); WINGET_DEFINE_RESOURCE_STRINGID(SourceListCommandLongDescription); WINGET_DEFINE_RESOURCE_STRINGID(SourceListCommandShortDescription); diff --git a/src/AppInstallerCLICore/Workflows/SourceFlow.cpp b/src/AppInstallerCLICore/Workflows/SourceFlow.cpp @@ -10,6 +10,7 @@ namespace AppInstaller::CLI::Workflow { using namespace AppInstaller::CLI::Execution; + using namespace AppInstaller::Settings; using namespace AppInstaller::Utility::literals; void GetSourceList(Execution::Context& context) @@ -166,6 +167,8 @@ namespace AppInstaller::CLI::Workflow void RemoveSources(Execution::Context& context) { + // TODO: We currently only allow removing a single source. If that changes, + // we need to check all sources with the Group Policy before removing any of them. if (!context.Args.Contains(Args::Type::SourceName)) { context.Reporter.Info() << Resource::String::SourceRemoveAll << std::endl; @@ -215,4 +218,27 @@ namespace AppInstaller::CLI::Workflow Repository::DropSource({}); context.Reporter.Info() << Resource::String::Done << std::endl; } + + void ExportSourceList(Execution::Context& context) + { + const std::vector<Repository::SourceDetails>& sources = context.Get<Data::SourceList>(); + + if (sources.empty()) + { + context.Reporter.Info() << Resource::String::SourceListNoSources << std::endl; + } + else + { + for (const auto& source : sources) + { + SourceFromPolicy s; + s.Name = source.Name; + s.Type = source.Type; + s.Arg = source.Arg; + s.Data = source.Data; + s.Identifier = source.Identifier; + context.Reporter.Info() << s.ToJsonString() << std::endl; + } + } + } } diff --git a/src/AppInstallerCLICore/Workflows/SourceFlow.h b/src/AppInstallerCLICore/Workflows/SourceFlow.h @@ -65,4 +65,10 @@ namespace AppInstaller::CLI::Workflow // Inputs: None // Outputs: None void ResetAllSources(Execution::Context& context); + + // Lists the sources in SourceList in a format appropriate for using in Group Policy + // Required Args: None + // Inputs: SourceList + // Outputs: None + void ExportSourceList(Execution::Context& context); } diff --git a/src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw b/src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw @@ -902,4 +902,10 @@ Configuration is disabled due to Group Policy.</value> <data name="PolicyEnableHashOverride" xml:space="preserve"> <value>Enable Windows App Installer Hash Override</value> </data> + <data name="SourceExportCommandLongDescription" xml:space="preserve"> + <value>Export current sources as JSON for Group Policy.</value> + </data> + <data name="SourceExportCommandShortDescription" xml:space="preserve"> + <value>Export current sources</value> + </data> </root> \ No newline at end of file diff --git a/src/AppInstallerCLITests/GroupPolicy.cpp b/src/AppInstallerCLITests/GroupPolicy.cpp @@ -9,6 +9,16 @@ using namespace TestCommon; using namespace AppInstaller::Settings; using namespace std::string_view_literals; +namespace +{ + std::wstring GetSourceJson(std::wstring_view name, std::wstring_view arg, std::wstring_view type, std::wstring_view data, std::wstring_view identifier) + { + std::wstringstream json; + json << L"{ \"Name\":\"" << name << L"\", \"Arg\":\"" << arg << L"\", \"Type\":\"" << type << L"\", \"Data\":\"" << data << L"\", \"Identifier\":\"" << identifier << L"\" }"; + return json.str(); + } +} + TEST_CASE("GroupPolicy_NoPolicies", "[groupPolicy]") { auto policiesKey = RegCreateVolatileTestRoot(); @@ -50,7 +60,164 @@ TEST_CASE("GroupPolicy_UpdateInterval", "[groupPolicy]") } } -// TODO: additional/allowed sources +TEST_CASE("GroupPolicy_Sources", "[groupPolicy]") +{ + auto policiesKey = RegCreateVolatileTestRoot(); + + // Note that the following tests mix using Additional/Allowed sources policy. + SECTION("Single source") + { + // We can read single source correctly + auto additionalSourcesKey = RegCreateVolatileSubKey(policiesKey.get(), AdditionalSourcesPolicyKeyName); + SetRegistryValue(additionalSourcesKey.get(), L"0", GetSourceJson(L"source-name", L"source-arg", L"source-type", L"source-data", L"source-identifier"), REG_SZ); + GroupPolicy groupPolicy{ policiesKey.get() }; + + auto policy = groupPolicy.GetValue<ValuePolicy::AdditionalSources>(); + REQUIRE(policy.has_value()); + REQUIRE(policy->size() == 1); + REQUIRE(policy.value()[0].Name == "source-name"); + REQUIRE(policy.value()[0].Arg == "source-arg"); + REQUIRE(policy.value()[0].Type == "source-type"); + REQUIRE(policy.value()[0].Data == "source-data"); + REQUIRE(policy.value()[0].Identifier == "source-identifier"); + } + SECTION("Missing field") + { + // A single missing field causes the source to not be read. + // "Type" is missing here. + std::wstring sourceJson = L"{ \"Name\":\"source_name\", \"Arg\":\"source_arg\", \"Data\":\"source_data\", \"Identifier\":\"source_identifier\" }"; + auto additionalSourcesKey = RegCreateVolatileSubKey(policiesKey.get(), AllowedSourcesPolicyKeyName); + SetRegistryValue(additionalSourcesKey.get(), L"0", sourceJson, REG_SZ); + GroupPolicy groupPolicy{ policiesKey.get() }; + + auto policy = groupPolicy.GetValue<ValuePolicy::AllowedSources>(); + REQUIRE(policy.has_value()); + REQUIRE(policy->empty()); + } + SECTION("Invalid field") + { + // A single invalid field causes the source to not be read. + // "Data" is invalid as it is an object, not a string. + std::wstring sourceJson = L"{ \"Name\":\"source_name\", \"Arg\":\"source_arg\", \"Data\":{}, \"Type\":\"source_type\", \"Identifier\":\"source_identifier\" }"; + auto additionalSourcesKey = RegCreateVolatileSubKey(policiesKey.get(), AdditionalSourcesPolicyKeyName); + SetRegistryValue(additionalSourcesKey.get(), L"0", sourceJson, REG_SZ); + GroupPolicy groupPolicy{ policiesKey.get() }; + + auto policy = groupPolicy.GetValue<ValuePolicy::AdditionalSources>(); + REQUIRE(policy.has_value()); + REQUIRE(policy->empty()); + } + SECTION("Invalid source JSON") + { + // An invalid source JSON causes the source to not be read. + auto additionalSourcesKey = RegCreateVolatileSubKey(policiesKey.get(), AllowedSourcesPolicyKeyName); + SetRegistryValue(additionalSourcesKey.get(), L"0", L"not a JSON", REG_SZ); + GroupPolicy groupPolicy{ policiesKey.get() }; + + auto policy = groupPolicy.GetValue<ValuePolicy::AllowedSources>(); + REQUIRE(policy.has_value()); + REQUIRE(policy->empty()); + } + SECTION("Missing key") + { + // If the key does not exist we should not get anything. + GroupPolicy groupPolicy{ policiesKey.get() }; + + auto policy = groupPolicy.GetValue<ValuePolicy::AdditionalSources>(); + REQUIRE_FALSE(policy.has_value()); + } + SECTION("Empty key") + { + // If the key is empty we should get an empty list. + // Note that the policy editor doesn't actually create empty keys. + auto additionalSourcesKey = RegCreateVolatileSubKey(policiesKey.get(), AllowedSourcesPolicyKeyName); + GroupPolicy groupPolicy{ policiesKey.get() }; + + auto policy = groupPolicy.GetValue<ValuePolicy::AllowedSources>(); + REQUIRE(policy.has_value()); + REQUIRE(policy->empty()); + } + SECTION("Valid list") + { + // We should be able to read multiple values. + // No specific order is required, but it will likely be the same. + auto additionalSourcesKey = RegCreateVolatileSubKey(policiesKey.get(), AdditionalSourcesPolicyKeyName); + SetRegistryValue(additionalSourcesKey.get(), L"0", GetSourceJson(L"s0-name", L"s0-arg", L"s0-type", L"s0-data", L"s0-identifier"), REG_SZ); + SetRegistryValue(additionalSourcesKey.get(), L"1", GetSourceJson(L"s1-name", L"s1-arg", L"s1-type", L"s1-data", L"s1-identifier"), REG_SZ); + SetRegistryValue(additionalSourcesKey.get(), L"2", GetSourceJson(L"s2-name", L"s2-arg", L"s2-type", L"s2-data", L"s2-identifier"), REG_SZ); + GroupPolicy groupPolicy{ policiesKey.get() }; + + auto policy = groupPolicy.GetValue<ValuePolicy::AdditionalSources>(); + REQUIRE(policy.has_value()); + REQUIRE(policy->size() == 3); + + REQUIRE(policy.value()[0].Name == "s0-name"); + REQUIRE(policy.value()[0].Arg == "s0-arg"); + REQUIRE(policy.value()[0].Type == "s0-type"); + REQUIRE(policy.value()[0].Data == "s0-data"); + REQUIRE(policy.value()[0].Identifier == "s0-identifier"); + + REQUIRE(policy.value()[1].Name == "s1-name"); + REQUIRE(policy.value()[1].Arg == "s1-arg"); + REQUIRE(policy.value()[1].Type == "s1-type"); + REQUIRE(policy.value()[1].Data == "s1-data"); + REQUIRE(policy.value()[1].Identifier == "s1-identifier"); + + REQUIRE(policy.value()[2].Name == "s2-name"); + REQUIRE(policy.value()[2].Arg == "s2-arg"); + REQUIRE(policy.value()[2].Type == "s2-type"); + REQUIRE(policy.value()[2].Data == "s2-data"); + REQUIRE(policy.value()[2].Identifier == "s2-identifier"); + } + SECTION("Invalid source in list") + { + // If a single source is invalid we should still get all others + auto additionalSourcesKey = RegCreateVolatileSubKey(policiesKey.get(), AdditionalSourcesPolicyKeyName); + SetRegistryValue(additionalSourcesKey.get(), L"0", GetSourceJson(L"s0-name", L"s0-arg", L"s0-type", L"s0-data", L"s0-identifier"), REG_SZ); + SetRegistryValue(additionalSourcesKey.get(), L"1", L"not a source", REG_SZ); + SetRegistryValue(additionalSourcesKey.get(), L"2", GetSourceJson(L"s2-name", L"s2-arg", L"s2-type", L"s2-data", L"s2-identifier"), REG_SZ); + GroupPolicy groupPolicy{ policiesKey.get() }; + + auto policy = groupPolicy.GetValue<ValuePolicy::AdditionalSources>(); + REQUIRE(policy.has_value()); + REQUIRE(policy->size() == 2); + + REQUIRE(policy.value()[0].Name == "s0-name"); + REQUIRE(policy.value()[0].Arg == "s0-arg"); + REQUIRE(policy.value()[0].Type == "s0-type"); + REQUIRE(policy.value()[0].Data == "s0-data"); + REQUIRE(policy.value()[0].Identifier == "s0-identifier"); + + REQUIRE(policy.value()[1].Name == "s2-name"); + REQUIRE(policy.value()[1].Arg == "s2-arg"); + REQUIRE(policy.value()[1].Type == "s2-type"); + REQUIRE(policy.value()[1].Data == "s2-data"); + REQUIRE(policy.value()[1].Identifier == "s2-identifier"); + } + SECTION("Exported JSON") + { + // Policy should be able to use an exported JSON strings + SourceFromPolicy source; + source.Name = "json-name"; + source.Type = "json-type"; + source.Arg = "json-arg"; + source.Data = "json-data"; + source.Identifier = "json-id"; + + auto additionalSourcesKey = RegCreateVolatileSubKey(policiesKey.get(), AllowedSourcesPolicyKeyName); + SetRegistryValue(additionalSourcesKey.get(), L"0", AppInstaller::Utility::ConvertToUTF16(source.ToJsonString())); + GroupPolicy groupPolicy{ policiesKey.get() }; + + auto policy = groupPolicy.GetValue<ValuePolicy::AllowedSources>(); + REQUIRE(policy.has_value()); + REQUIRE(policy->size() == 1); + REQUIRE(policy.value()[0].Name == source.Name); + REQUIRE(policy.value()[0].Arg == source.Arg); + REQUIRE(policy.value()[0].Type == source.Type); + REQUIRE(policy.value()[0].Data == source.Data); + REQUIRE(policy.value()[0].Identifier == source.Identifier); + } +} TEST_CASE("GroupPolicy_Toggle", "[groupPolicy]") { @@ -88,7 +255,7 @@ TEST_CASE("GroupPolicy_Toggle", "[groupPolicy]") } } -TEST_CASE("GroupPolicy_AllDisabled", "[groupPolicy]") +TEST_CASE("GroupPolicy_AllEnabled", "[groupPolicy]") { auto policiesKey = RegCreateVolatileTestRoot(); SetRegistryValue(policiesKey.get(), WinGetPolicyValueName, 1); diff --git a/src/AppInstallerCLITests/Sources.cpp b/src/AppInstallerCLITests/Sources.cpp @@ -3,6 +3,7 @@ #include "pch.h" #include "TestCommon.h" #include "TestHooks.h" +#include "TestSettings.h" #include "TestSource.h" #include <AppInstallerRepositorySource.h> @@ -108,6 +109,24 @@ Sources: IsTombstone: false )"sv; +constexpr std::string_view s_DefaultSourceAsUserSource = R"( +Sources: + - Name: not-winget + Type: Microsoft.PreIndexed.Package + Arg: https://winget.azureedge.net/cache + Data: Microsoft.Winget.Source_8wekyb3d8bbwe + IsTombstone: false +)"sv; + +constexpr std::string_view s_UserSourceNamedLikeDefault = R"( +Sources: + - Name: winget + Type: testType + Arg: testArg + Data: testData + IsTombstone: false +)"sv; + namespace { // Helper to create a simple source. @@ -554,4 +573,345 @@ TEST_CASE("RepoSources_SearchAcrossMultipleSources", "[sources]") REQUIRE((result.Matches[0].MatchCriteria.Type == MatchType::Exact && result.Matches[0].MatchCriteria.Field == PackageMatchField::Id)); REQUIRE((result.Matches[1].MatchCriteria.Type == MatchType::Exact && result.Matches[1].MatchCriteria.Field == PackageMatchField::Id)); REQUIRE((result.Matches[2].MatchCriteria.Type == MatchType::Exact && result.Matches[2].MatchCriteria.Field == PackageMatchField::Name)); -}- \ No newline at end of file +} + +TEST_CASE("RepoSources_GroupPolicy_DefaultSource", "[sources][groupPolicy]") +{ + WHEN("Default source is disabled") + { + GroupPolicyTestOverride policies; + policies.SetState(TogglePolicy::Policy::DefaultSource, PolicyState::Disabled); + + SECTION("Get source") + { + // Listing the sources should not return the default. + SetSetting(Streams::UserSources, s_EmptySources); + + auto sources = GetSources(); + REQUIRE(sources.empty()); + } + SECTION("Add default source") + { + // We should not be able to add the default source manually. + SetSetting(Streams::UserSources, s_EmptySources); + + ProgressCallback progress; + REQUIRE_POLICY_EXCEPTION( + AddSource("winget", "Microsoft.PreIndexed.Package", "https://winget.azureedge.net/cache", progress), + TogglePolicy::Policy::DefaultSource); + } + 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); + + auto sources = GetSources(); + REQUIRE(sources.empty()); + } + SECTION("Add same-name source from user") + { + // We should allow adding sources with the same name as the default but + // pointing somewhere else. + SetSetting(Streams::UserSources, s_EmptySources); + TestHook_ClearSourceFactoryOverrides(); + + std::string name = "winget"; + std::string type = "someType"; + std::string arg = "notWingetRealArg"; + std::string data = "someData"; + + bool addCalledOnFactory = false; + TestSourceFactory factory{ SourcesTestSource::Create }; + factory.OnAdd = [&](SourceDetails& sd) { addCalledOnFactory = true; sd.Data = data; }; + TestHook_SetSourceFactoryOverride(type, factory); + + ProgressCallback progress; + AddSource(name, type, arg, progress); + + REQUIRE(addCalledOnFactory); + + auto sources = GetSources(); + REQUIRE(sources.size() == 1); + + REQUIRE(sources[0].Name == name); + REQUIRE(sources[0].Type == type); + REQUIRE(sources[0].Arg == arg); + REQUIRE(sources[0].Data == data); + REQUIRE(sources[0].Origin == SourceOrigin::User); + } + SECTION("Allow same name source from user") + { + // 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); + + auto sources = GetSources(); + REQUIRE(sources.size() == 1); + + REQUIRE(sources[0].Name == "winget"); + REQUIRE(sources[0].Type == "testType"); + REQUIRE(sources[0].Arg == "testArg"); + REQUIRE(sources[0].Data == "testData"); + REQUIRE(sources[0].Origin == SourceOrigin::User); + } + } + + WHEN("Default source is enabled") + { + GroupPolicyTestOverride policies; + policies.SetState(TogglePolicy::Policy::DefaultSource, PolicyState::Enabled); + + SECTION("Remove source is blocked") + { + // We should not be able to remove the default source. + SetSetting(Streams::UserSources, s_EmptySources); + + ProgressCallback progress; + REQUIRE_POLICY_EXCEPTION( + RemoveSource("winget", progress), + TogglePolicy::Policy::DefaultSource); + } + SECTION("Tombstone is overridden") + { + // We should ignore if the default source was already deleted. + SetSetting(Streams::UserSources, s_DefaultSourceTombstoned); + + auto sources = GetSources(); + REQUIRE(sources.size() == 1); + REQUIRE(sources[0].Name == "winget"); + REQUIRE(sources[0].Origin == SourceOrigin::Default); + } + SECTION("Same name source is overridden") + { + // We should ignore existing user sources with the same name as the default. + SetSetting(Streams::UserSources, s_UserSourceNamedLikeDefault); + + auto sources = GetSources(); + REQUIRE(sources.size() == 1); + + REQUIRE(sources[0].Name == "winget"); + REQUIRE(sources[0].Arg == "https://winget.azureedge.net/cache"); + REQUIRE(sources[0].Origin == SourceOrigin::Default); + } + } +} + +TEST_CASE("RepoSources_GroupPolicy_AdditionalSources", "[sources][groupPolicy]") +{ + WHEN("Additional sources are enabled") + { + GroupPolicyTestOverride policies; + policies.SetState(TogglePolicy::Policy::AdditionalSources, PolicyState::Enabled); + + SECTION("Additional sources are listed") + { + // Getting the current sources should list the additional sources. + std::vector<SourceFromPolicy> policySources; + const std::string suffix[3] = { "", "2", "3" }; + for (size_t i = 0; i < 3; ++i) + { + SourceFromPolicy source; + source.Name = "name" + suffix[i]; + source.Type = "type" + suffix[i]; + source.Arg = "arg" + suffix[i]; + source.Data = "data" + suffix[i]; + source.Identifier = "id" + suffix[i]; + policySources.emplace_back(std::move(source)); + } + + policies.SetValue<ValuePolicy::AdditionalSources>(policySources); + SetSetting(Streams::UserSources, s_EmptySources); + + auto sources = GetSources(); + + // The source list includes the default source + REQUIRE(sources.size() == policySources.size() + 1); + REQUIRE(sources.back().Origin == SourceOrigin::Default); + + for (size_t i = 0; i < policySources.size(); ++i) + { + REQUIRE(sources[i].Name == policySources[i].Name); + REQUIRE(sources[i].Type == policySources[i].Type); + REQUIRE(sources[i].Arg == policySources[i].Arg); + REQUIRE(sources[i].Data == policySources[i].Data); + REQUIRE(sources[i].Identifier == policySources[i].Identifier); + REQUIRE(sources[i].Origin == SourceOrigin::GroupPolicy); + } + } + SECTION("Same-name user source is overridden") + { + // User sources with the same name as an additional source are ignored. + SourceFromPolicy policySource; + policySource.Name = "testName"; + policySource.Type = "notTestType"; + policySource.Arg = "notTestArg"; + policySource.Data = "notTestData"; + policySource.Identifier = "notTestId"; + + policies.SetValue<ValuePolicy::AdditionalSources>({ policySource }); + SetSetting(Streams::UserSources, s_SingleSource); + + auto sources = GetSources(); + + // The source list includes the default source + REQUIRE(sources.size() == 2); + REQUIRE(sources[1].Origin == SourceOrigin::Default); + + REQUIRE(sources[0].Name == policySource.Name); + REQUIRE(sources[0].Type == policySource.Type); + REQUIRE(sources[0].Arg == policySource.Arg); + REQUIRE(sources[0].Data == policySource.Data); + REQUIRE(sources[0].Identifier == policySource.Identifier); + REQUIRE(sources[0].Origin == SourceOrigin::GroupPolicy); + } + SECTION("Cannot remove additional source") + { + // An additional source cannot be removed. + SourceFromPolicy policySource; + policySource.Name = "name"; + policySource.Type = "type"; + policySource.Arg = "arg"; + policySource.Data = "data"; + policySource.Identifier = "id"; + + policies.SetValue<ValuePolicy::AdditionalSources>({ policySource }); + SetSetting(Streams::UserSources, s_EmptySources); + + ProgressCallback progress; + REQUIRE_POLICY_EXCEPTION( + RemoveSource(policySource.Name, progress), + TogglePolicy::Policy::AdditionalSources); + } + SECTION("Additional source overrides default") + { + // An additional source with the same name as a default overrides it. + SourceFromPolicy policySource; + policySource.Name = "winget"; + policySource.Type = "notDefaultType"; + policySource.Arg = "notDefaultArg"; + policySource.Data = "notDefaultData"; + policySource.Identifier = "notDefaultId"; + + policies.SetValue<ValuePolicy::AdditionalSources>({ policySource }); + SetSetting(Streams::UserSources, s_EmptySources); + + auto sources = GetSources(); + + REQUIRE(sources.size() == 1); + REQUIRE(sources[0].Name == policySource.Name); + REQUIRE(sources[0].Type == policySource.Type); + REQUIRE(sources[0].Arg == policySource.Arg); + REQUIRE(sources[0].Data == policySource.Data); + REQUIRE(sources[0].Identifier == policySource.Identifier); + REQUIRE(sources[0].Origin == SourceOrigin::GroupPolicy); + } + } +} + +TEST_CASE("RepoSources_GroupPolicy_AllowedSources", "[sources][groupPolicy]") +{ + WHEN("Allowed sources are enabled") + { + GroupPolicyTestOverride policies; + policies.SetState(TogglePolicy::Policy::AllowedSources, PolicyState::Enabled); + + SECTION("Add allowed source") + { + // We should be able to add sources in the allow list. + SourceFromPolicy policySource; + policySource.Name = "testName"; + policySource.Type = "testType"; + policySource.Arg = "testArg"; + policySource.Data = "testData"; + policySource.Identifier = "testId"; + + policies.SetValue<ValuePolicy::AllowedSources>({ policySource }); + SetSetting(Streams::UserSources, s_EmptySources); + TestHook_ClearSourceFactoryOverrides(); + + bool addCalledOnFactory = false; + TestSourceFactory factory{ SourcesTestSource::Create }; + factory.OnAdd = [&](SourceDetails& sd) + { + addCalledOnFactory = true; + sd.Data = policySource.Data; + sd.Identifier = policySource.Identifier; + }; + TestHook_SetSourceFactoryOverride(policySource.Type, factory); + + ProgressCallback progress; + AddSource(policySource.Name, policySource.Type, policySource.Arg, progress); + + REQUIRE(addCalledOnFactory); + + // The source list includes the default source + auto sources = GetSources(); + REQUIRE(sources.size() == 2); + REQUIRE(sources[1].Origin == SourceOrigin::Default); + + REQUIRE(sources[0].Name == policySource.Name); + REQUIRE(sources[0].Type == policySource.Type); + REQUIRE(sources[0].Arg == policySource.Arg); + REQUIRE(sources[0].Data == policySource.Data); + REQUIRE(sources[0].Identifier == policySource.Identifier); + REQUIRE(sources[0].Origin == SourceOrigin::User); + } + SECTION("Cannot add non-allowed source") + { + // We should not be allowed to add anything not matching the allow list. + SourceFromPolicy policySource; + policySource.Name = "testName"; + policySource.Type = "testType"; + policySource.Arg = "testArg"; + policySource.Data = "testData"; + policySource.Identifier = "testId"; + + policies.SetValue<ValuePolicy::AllowedSources>({ policySource }); + SetSetting(Streams::UserSources, s_EmptySources); + + bool addCalledOnFactory = false; + TestSourceFactory factory{ SourcesTestSource::Create }; + factory.OnAdd = [&](SourceDetails&) { addCalledOnFactory = true; }; + + ProgressCallback progress; + REQUIRE_POLICY_EXCEPTION( + AddSource("notAllowed", "type", "arg", progress), + TogglePolicy::Policy::AllowedSources); + REQUIRE_FALSE(addCalledOnFactory); + } + } + + WHEN("Allowed sources are disabled") + { + GroupPolicyTestOverride policies; + policies.SetState(TogglePolicy::Policy::AllowedSources, PolicyState::Disabled); + + SECTION("Cannot add any source") + { + SetSetting(Streams::UserSources, s_EmptySources); + + bool addCalledOnFactory = false; + TestSourceFactory factory{ SourcesTestSource::Create }; + factory.OnAdd = [&](SourceDetails&) { addCalledOnFactory = true; }; + + ProgressCallback progress; + REQUIRE_POLICY_EXCEPTION( + AddSource("name", "type", "arg", progress), + TogglePolicy::Policy::AllowedSources); + REQUIRE_FALSE(addCalledOnFactory); + + auto sources = GetSources(); + REQUIRE(sources.size() == 1); + REQUIRE(sources[0].Origin == SourceOrigin::Default); + } + SECTION("Existing sources are ignored") + { + SetSetting(Streams::UserSources, s_SingleSource); + + auto sources = GetSources(); + REQUIRE(sources.size() == 1); + REQUIRE(sources[0].Origin == SourceOrigin::Default); + } + } +} diff --git a/src/AppInstallerCLITests/TestSettings.cpp b/src/AppInstallerCLITests/TestSettings.cpp @@ -26,11 +26,16 @@ namespace TestCommon GroupPolicyTestOverride::GroupPolicyTestOverride(const AppInstaller::Registry::Key& key) : GroupPolicy(key) { - AppInstaller::Settings::GroupPolicy::OverrideInstance(this); + GroupPolicy::OverrideInstance(this); } GroupPolicyTestOverride::~GroupPolicyTestOverride() { - AppInstaller::Settings::GroupPolicy::ResetInstance(); + GroupPolicy::ResetInstance(); + } + + void GroupPolicyTestOverride::SetState(TogglePolicy::Policy policy, PolicyState state) + { + m_toggles[policy] = state; } } \ No newline at end of file diff --git a/src/AppInstallerCLITests/TestSettings.h b/src/AppInstallerCLITests/TestSettings.h @@ -14,12 +14,15 @@ namespace TestCommon const std::wstring LocalManifestsPolicyValueName = L"EnableLocalManifestFiles"; const std::wstring EnableHashOverridePolicyValueName = L"EnableHashOverride"; const std::wstring DefaultSourcePolicyValueName = L"EnableDefaultSource"; - const std::wstring MSStoreSourcePolicyValueName = L"EnableMSStoreSource"; + const std::wstring MSStoreSourcePolicyValueName = L"EnableMicrosoftStoreSource"; const std::wstring AdditionalSourcesPolicyValueName = L"EnableAdditionalSources"; const std::wstring AllowedSourcesPolicyValueName = L"EnableAllowedSources"; const std::wstring SourceUpdateIntervalPolicyValueName = L"SourceAutoUpdateIntervalInMinutes"; + const std::wstring AdditionalSourcesPolicyKeyName = L"AdditionalSources"; + const std::wstring AllowedSourcesPolicyKeyName = L"AllowedSources"; + void DeleteUserSettingsFiles(); struct UserSettingsTest : AppInstaller::Settings::UserSettings @@ -28,7 +31,45 @@ namespace TestCommon struct GroupPolicyTestOverride : AppInstaller::Settings::GroupPolicy { + GroupPolicyTestOverride() : GroupPolicyTestOverride(RegCreateVolatileTestRoot().get()) {} GroupPolicyTestOverride(const AppInstaller::Registry::Key& key); ~GroupPolicyTestOverride(); + + template<AppInstaller::Settings::ValuePolicy P> + void SetValue(const ValueType<P>& value) + { + m_values.Add<P>(value); + } + + template<AppInstaller::Settings::ValuePolicy P> + void SetValue(ValueType<P> &&value) + { + m_values.Add<P>(std::move(value)); + } + + void SetState(AppInstaller::Settings::TogglePolicy::Policy policy, AppInstaller::Settings::PolicyState state); }; + + // Matcher that lets us verify GroupPolicyExceptions. + struct GroupPolicyExceptionMatcher : public Catch::MatcherBase<AppInstaller::Settings::GroupPolicyException> + { + GroupPolicyExceptionMatcher(AppInstaller::Settings::TogglePolicy::Policy policy) : m_expectedPolicy(policy) {} + + bool match(const AppInstaller::Settings::GroupPolicyException& e) const override + { + return e.Policy() == m_expectedPolicy; + } + + std::string describe() const override + { + std::ostringstream result; + result << "has policy == " << m_expectedPolicy; + return result.str(); + } + + private: + AppInstaller::Settings::TogglePolicy::Policy m_expectedPolicy; + }; + +#define REQUIRE_POLICY_EXCEPTION(_expr_, _policy_) REQUIRE_THROWS_MATCHES(_expr_, AppInstaller::Settings::GroupPolicyException, TestCommon::GroupPolicyExceptionMatcher(_policy_)) } \ No newline at end of file diff --git a/src/AppInstallerCLITests/WorkflowGroupPolicy.cpp b/src/AppInstallerCLITests/WorkflowGroupPolicy.cpp @@ -18,9 +18,8 @@ using namespace std::string_view_literals; TEST_CASE("GroupPolicy_WinGet", "[groupPolicy]") { - auto policiesKey = RegCreateVolatileTestRoot(); - SetRegistryValue(policiesKey.get(), WinGetPolicyValueName, false); - GroupPolicyTestOverride policies{ policiesKey.get() }; + GroupPolicyTestOverride policies; + policies.SetState(TogglePolicy::Policy::WinGet, PolicyState::Disabled); SECTION("Install is blocked") { @@ -29,10 +28,9 @@ TEST_CASE("GroupPolicy_WinGet", "[groupPolicy]") context.Args.AddArg(Execution::Args::Type::Query, "Fake.Package"sv); InstallCommand installCommand({}); - installCommand.Execute(context); - - REQUIRE(context.IsTerminated()); - REQUIRE(context.GetTerminationHR() == APPINSTALLER_CLI_ERROR_BLOCKED_BY_POLICY); + REQUIRE_POLICY_EXCEPTION( + installCommand.Execute(context), + TogglePolicy::Policy::WinGet); } SECTION("Info is not blocked") { @@ -50,9 +48,8 @@ TEST_CASE("GroupPolicy_WinGet", "[groupPolicy]") TEST_CASE("GroupPolicy_SettingsCommand", "[groupPolicy]") { - auto policiesKey = RegCreateVolatileTestRoot(); - SetRegistryValue(policiesKey.get(), WinGetSettingsPolicyValueName, false); - GroupPolicyTestOverride policies{ policiesKey.get() }; + GroupPolicyTestOverride policies; + policies.SetState(TogglePolicy::Policy::Settings, PolicyState::Disabled); Invocation inv{ std::vector<std::string>{ "settings" } }; RootCommand rootCommand; @@ -61,9 +58,8 @@ TEST_CASE("GroupPolicy_SettingsCommand", "[groupPolicy]") TEST_CASE("GroupPolicy_LocalManifests", "[groupPolicy]") { - auto policiesKey = RegCreateVolatileTestRoot(); - SetRegistryValue(policiesKey.get(), LocalManifestsPolicyValueName, false); - GroupPolicyTestOverride policies{ policiesKey.get() }; + GroupPolicyTestOverride policies; + policies.SetState(TogglePolicy::Policy::LocalManifestFiles, PolicyState::Disabled); SECTION("Blocked on install") { diff --git a/src/AppInstallerCommonCore/GroupPolicy.cpp b/src/AppInstallerCommonCore/GroupPolicy.cpp @@ -104,26 +104,116 @@ namespace AppInstaller::Settings // Use folding to call each policy validate function. (FoldHelper{}, ..., Validate<static_cast<ValuePolicy>(P)>(policiesKey, policies)); } + + // Reads a list from a Group Policy. + // The list is stored in a sub-key of the policies key, and each value in that key is a list item. + // Cases not considered by this function because we don't use them: + // - When the list is in an arbitrary key, not a sub key. + // - When the list values are mixed with other values and are identified by a prefix in their names. + // - When the value names are relevant. + template<ValuePolicy P> + std::optional<typename details::ValuePolicyMapping<P>::value_t> ReadList(const Registry::Key& policiesKey) + { + using Mapping = details::ValuePolicyMapping<P>; + + auto listKey = policiesKey.SubKey(Mapping::KeyName); + if (!listKey.has_value()) + { + return std::nullopt; + } + + std::vector<Mapping::item_t> items; + for (const auto& value : listKey->Values()) + { + auto item = Mapping::ReadAndValidateItem(value); + if (item.has_value()) + { + items.emplace_back(std::move(item.value())); + } + else + { + AICLI_LOG(Core, Warning, << "Failed to read Group Policy list value. Policy [" << Mapping::KeyName << "], Value [" << value.Name() << ']'); + } + } + + return items; + } + + std::optional<SourceFromPolicy> ReadSourceFromRegistryValue(const Registry::Value& item) + { + auto jsonString = item.TryGetValue<Registry::Value::Type::String>(); + if (!jsonString.has_value()) + { + AICLI_LOG(Core, Warning, << "Registry value is not a string"); + return std::nullopt; + } + + int stringLength = static_cast<int>(jsonString->length()); + Json::Value sourceJson; + Json::CharReaderBuilder charReaderBuilder; + const std::unique_ptr<Json::CharReader> jsonReader(charReaderBuilder.newCharReader()); + Json::String jsonErrors; + if (!jsonReader->parse(jsonString->c_str(), jsonString->c_str() + stringLength, &sourceJson, &jsonErrors)) + { + AICLI_LOG(Core, Warning, << "Registry value does not contain a valid JSON: " << jsonErrors); + return std::nullopt; + } + + SourceFromPolicy source; + + auto readSourceAttribute = [&](const std::string& name, std::string SourceFromPolicy::* member) + { + if (sourceJson.isMember(name) && sourceJson[name].isString()) + { + source.*member = sourceJson[name].asString(); + return true; + } + else + { + AICLI_LOG(Core, Warning, << "Source JSON does not contain a string value for " << name); + return false; + } + }; + + bool allRead = readSourceAttribute("Name", &SourceFromPolicy::Name) + && readSourceAttribute("Arg", &SourceFromPolicy::Arg) + && readSourceAttribute("Type", &SourceFromPolicy::Type) + && readSourceAttribute("Data", &SourceFromPolicy::Data) + && readSourceAttribute("Identifier", &SourceFromPolicy::Identifier); + if (!allRead) + { + return std::nullopt; + } + + return source; + } } namespace details { +#define POLICY_MAPPING_DEFAULT_LIST_READ(_policy_) \ + std::optional<typename ValuePolicyMapping<_policy_>::value_t> ValuePolicyMapping<_policy_>::ReadAndValidate(const Registry::Key& policiesKey) \ + { \ + return ReadList<_policy_>(policiesKey); \ + } + + POLICY_MAPPING_DEFAULT_LIST_READ(ValuePolicy::AdditionalSources); + POLICY_MAPPING_DEFAULT_LIST_READ(ValuePolicy::AllowedSources); + std::optional<uint32_t> ValuePolicyMapping<ValuePolicy::SourceAutoUpdateIntervalInMinutes>::ReadAndValidate(const Registry::Key& policiesKey) { using Mapping = ValuePolicyMapping<ValuePolicy::SourceAutoUpdateIntervalInMinutes>; - return GetRegistryValue<Mapping::ValueType>(policiesKey , Mapping::ValueName); + return GetRegistryValue<Mapping::ValueType>(policiesKey, Mapping::ValueName); } - std::optional<std::vector<std::string>> ValuePolicyMapping<ValuePolicy::AdditionalSources>::ReadAndValidate(const Registry::Key&) + std::optional<SourceFromPolicy> ValuePolicyMapping<ValuePolicy::AdditionalSources>::ReadAndValidateItem(const Registry::Value& item) { - // TODO - return std::nullopt; + return ReadSourceFromRegistryValue(item); } - std::optional<std::vector<std::string>> ValuePolicyMapping<ValuePolicy::AllowedSources>::ReadAndValidate(const Registry::Key&) + std::optional<SourceFromPolicy> ValuePolicyMapping<ValuePolicy::AllowedSources>::ReadAndValidateItem(const Registry::Value& item) { - // TODO - return std::nullopt; + return ReadSourceFromRegistryValue(item); } } @@ -144,7 +234,7 @@ namespace AppInstaller::Settings case TogglePolicy::Policy::DefaultSource: return TogglePolicy(policy, "EnableDefaultSource"sv, String::PolicyEnableDefaultSource); case TogglePolicy::Policy::MSStoreSource: - return TogglePolicy(policy, "EnableMSStoreSource"sv, String::PolicyEnableMSStoreSource); + return TogglePolicy(policy, "EnableMicrosoftStoreSource"sv, String::PolicyEnableMSStoreSource); case TogglePolicy::Policy::AdditionalSources: return TogglePolicy(policy, "EnableAdditionalSources"sv, String::PolicyAdditionalSources); case TogglePolicy::Policy::AllowedSources: @@ -169,6 +259,20 @@ namespace AppInstaller::Settings return result; } + std::string SourceFromPolicy::ToJsonString() const + { + Json::Value json{ Json::ValueType::objectValue }; + json["Name"] = Name; + json["Type"] = Type; + json["Arg"] = Arg; + json["Data"] = Data; + json["Identifier"] = Identifier; + + Json::StreamWriterBuilder writerBuilder; + writerBuilder.settings_["indentation"] = ""; + return Json::writeString(writerBuilder, json); + } + GroupPolicy::GroupPolicy(const Registry::Key& key) { ValidateAllValuePolicies(key, m_values, std::make_index_sequence<static_cast<size_t>(ValuePolicy::Max)>()); diff --git a/src/AppInstallerCommonCore/JsonSchemaValidation.cpp b/src/AppInstallerCommonCore/JsonSchemaValidation.cpp @@ -41,7 +41,8 @@ namespace AppInstaller::JsonSchema Json::CharReaderBuilder charReaderBuilder; const std::unique_ptr<Json::CharReader> jsonReader(charReaderBuilder.newCharReader()); std::string errorMsg; - if (!jsonReader->parse(schemaStr.c_str(), schemaStr.c_str() + schemaLength, &schemaJson, &errorMsg)) { + if (!jsonReader->parse(schemaStr.c_str(), schemaStr.c_str() + schemaLength, &schemaJson, &errorMsg)) + { THROW_HR_MSG(E_UNEXPECTED, "Jsoncpp parser failed to parse the schema doc. Reason: %s", errorMsg.c_str()); } diff --git a/src/AppInstallerCommonCore/Public/winget/GroupPolicy.h b/src/AppInstallerCommonCore/Public/winget/GroupPolicy.h @@ -68,6 +68,19 @@ namespace AppInstaller::Settings Enabled, }; + // A source defined by Group Policy to be added or allowed + struct SourceFromPolicy + { + std::string Name; + std::string Arg; + std::string Type; + std::string Data; + std::string Identifier; + + std::string ToJsonString() const; + }; + + namespace details { @@ -81,32 +94,40 @@ namespace AppInstaller::Settings // ValueName - Name of the registry value // ValueType - Type of the registry value // reg_value_t - Type returned by the registry when reading the value + + // For lists: + // item_t - Type of each item + // KeyName -- Name of the sub-key containing the list + // ReadAndValidateItem() - Function that reads a single item from a subkey }; -#define POLICY_MAPPING_SPECIALIZATION(_policy_, _type_) \ +#define POLICY_MAPPING_SPECIALIZATION(_policy_, _type_, _extra_) \ template <> \ struct ValuePolicyMapping<_policy_> \ { \ using value_t = _type_; \ static std::optional<value_t> ReadAndValidate(const Registry::Key& policiesKey); \ + _extra_ \ } #define POLICY_MAPPING_VALUE_SPECIALIZATION(_policy_, _type_, _valueName_, _valueType_) \ - template<> \ - struct ValuePolicyMapping<_policy_> \ - { \ + POLICY_MAPPING_SPECIALIZATION(_policy_, _type_, \ static constexpr std::string_view ValueName = _valueName_; \ static constexpr Registry::Value::Type ValueType = _valueType_; \ - using value_t = _type_; \ using reg_value_t = decltype(std::declval<Registry::Value>().GetValue<ValueType>()); \ - static std::optional<value_t> ReadAndValidate(const Registry::Key& policiesKey); \ - } + ) + +#define POLICY_MAPPING_LIST_SPECIALIZATION(_policy_, _type_, _keyName_) \ + POLICY_MAPPING_SPECIALIZATION(_policy_, std::vector<_type_>, \ + static constexpr std::string_view KeyName = _keyName_; \ + using item_t = _type_; \ + static std::optional<item_t> ReadAndValidateItem(const Registry::Value& item); \ + ) POLICY_MAPPING_VALUE_SPECIALIZATION(ValuePolicy::SourceAutoUpdateIntervalInMinutes, uint32_t, "SourceAutoUpdateIntervalInMinutes"sv, Registry::Value::Type::DWord); - // TODO: Wire up policies for sources - POLICY_MAPPING_SPECIALIZATION(ValuePolicy::AdditionalSources, std::vector<std::string>); - POLICY_MAPPING_SPECIALIZATION(ValuePolicy::AllowedSources, std::vector<std::string>); + POLICY_MAPPING_LIST_SPECIALIZATION(ValuePolicy::AdditionalSources, SourceFromPolicy, "AdditionalSources"sv); + POLICY_MAPPING_LIST_SPECIALIZATION(ValuePolicy::AllowedSources, SourceFromPolicy, "AllowedSources"sv); } // Representation of the policies read from the registry. @@ -127,9 +148,12 @@ namespace AppInstaller::Settings GroupPolicy(GroupPolicy&&) = delete; GroupPolicy& operator=(GroupPolicy&&) = delete; + template<ValuePolicy P> + using ValueType = typename details::ValuePolicyMapping<P>::value_t; + // Gets the policy value if it is present template<ValuePolicy P> - std::optional<typename details::ValuePolicyMapping<P>::value_t> GetValue() const + std::optional<ValueType<P>> GetValue() const { if (m_values.Contains(P)) { @@ -141,25 +165,49 @@ namespace AppInstaller::Settings } } + template<ValuePolicy P> + std::optional<std::reference_wrapper<const ValueType<P>>> GetValueRef() const + { + if (m_values.Contains(P)) + { + return std::cref(m_values.Get<P>()); + } + else + { + return std::nullopt; + } + } + PolicyState GetState(TogglePolicy::Policy policy) const; // Checks whether a policy is enabled, using an appropriate default when not configured. // Should not be used when not configured means something different than enabled/disabled. bool IsEnabled(TogglePolicy::Policy policy) const; - private: - std::map<TogglePolicy::Policy, PolicyState> m_toggles; - ValuePoliciesMap m_values; - #ifndef AICLI_DISABLE_TEST_HOOKS protected: static void OverrideInstance(GroupPolicy* gp); static void ResetInstance(); +#else + private: #endif + std::map<TogglePolicy::Policy, PolicyState> m_toggles; + ValuePoliciesMap m_values; }; inline const GroupPolicy& GroupPolicies() { return GroupPolicy::Instance(); } + + struct GroupPolicyException + { + GroupPolicyException(TogglePolicy::Policy policy) : m_policy(policy) {} + + const TogglePolicy::Policy& Policy() const { return m_policy; } + + private: + TogglePolicy::Policy m_policy; + }; + } \ No newline at end of file diff --git a/src/AppInstallerCommonCore/Public/winget/Registry.h b/src/AppInstallerCommonCore/Public/winget/Registry.h @@ -61,11 +61,13 @@ namespace AppInstaller::Registry } struct Key; + struct ValueList; // A registry value. struct Value { friend Key; + friend ValueList; // The type of data stored in the Value. enum class Type : DWORD @@ -87,8 +89,13 @@ namespace AppInstaller::Registry template <Type T> typename details::ValueTypeSpecifics<static_cast<DWORD>(T)>::value_t GetValue() const { - THROW_HR_IF(E_INVALIDARG, !HasCompatibleType(T)); - return details::ValueTypeSpecifics<static_cast<DWORD>(T)>::Convert(m_data); + auto value = TryGetValue<T>(); + if (!value.has_value()) + { + THROW_HR(E_INVALIDARG); + } + + return std::move(value.value()); } template <Type T> @@ -113,6 +120,63 @@ namespace AppInstaller::Registry std::vector<BYTE> m_data; }; + // Value iteration + struct ValueList + { + friend Key; + + struct const_iterator; + + struct ValueRef : Value + { + friend const_iterator; + + // Gets the name of the value. + std::string Name() const; + + private: + ValueRef(std::wstring&& valueName, DWORD type, std::vector<BYTE>&& data); + + std::wstring m_valueName; + }; + + struct const_iterator + { + friend ValueList; + + const_iterator& operator++(); + const_iterator operator++(int); + + bool operator==(const const_iterator& other) const; + bool operator!=(const const_iterator& other) const; + + const ValueRef& operator*() const; + const ValueRef* operator->() const; + + private: + // Create an iterator + const_iterator(const wil::shared_hkey& key, DWORD index = 0); + + // Create an iterator for end + const_iterator() = default; + + void GetValue(); + + // An empty handle represents the end iterator. + wil::shared_hkey m_key; + DWORD m_index = 0; + std::optional<ValueRef> m_value; + }; + + const_iterator begin() const; + const_iterator end() const; + + private: + ValueList(wil::shared_hkey key); + + wil::shared_hkey m_key; + }; + // A registry key. struct Key { @@ -181,6 +245,11 @@ namespace AppInstaller::Registry std::optional<Value> operator[](std::string_view name) const; std::optional<Value> operator[](const std::wstring& name) const; + std::optional<Key> SubKey(std::string_view name, DWORD options = 0) const; + std::optional<Key> SubKey(const std::wstring& name, DWORD options = 0) const; + + ValueList Values() const; + operator bool() const { return m_key.operator bool(); } // Open a Key; will return an empty Key if the subkey does not exist. @@ -188,7 +257,8 @@ namespace AppInstaller::Registry static Key OpenIfExists(HKEY key, const std::wstring& subKey = {}, DWORD options = 0, REGSAM access = KEY_READ); private: - void Initialize(HKEY key, const std::wstring& subKey, DWORD options, REGSAM access, bool ignoreErrorIfDoesNotExist); + // When ignoring error, returns whether the key existed + bool Initialize(HKEY key, const std::wstring& subKey, DWORD options, REGSAM access, bool ignoreErrorIfDoesNotExist); wil::shared_hkey m_key; REGSAM m_access = KEY_READ; diff --git a/src/AppInstallerCommonCore/Public/winget/UserSettings.h b/src/AppInstallerCommonCore/Public/winget/UserSettings.h @@ -1,142 +1,142 @@ -// Copyright (c) Microsoft Corporation. -// Licensed under the MIT License. -#pragma once -#include "AppInstallerStrings.h" -#include "winget/GroupPolicy.h" -#include "winget/Resources.h" - -#include <filesystem> -#include <map> -#include <optional> -#include <string> -#include <type_traits> -#include <variant> -#include <vector> - -using namespace std::chrono_literals; -using namespace std::string_view_literals; - -namespace AppInstaller::Settings -{ - // The type of argument. - enum class UserSettingsType - { - // Settings files don't exist. A file is created on the first call to the settings command. - Default, - // Loaded settings.json - Standard, - // Loaded settings.json.backup - Backup, - }; - - // The visual style of the progress bar. - enum class VisualStyle - { - NoVT, - Retro, - Accent, - Rainbow, - }; - - // The preferred scope for installs. - enum class ScopePreference - { - None, - User, - Machine, - }; - - // Enum of settings. - // Must start at 0 to enable direct access to variant in UserSettings. - // Max must be last and unused. - // How to add a setting - // 1 - Add to enum. - // 2 - Implement SettingMap specialization via SETTINGMAPPING_SPECIALIZATION - // Validate will be called by ValidateAll without any more changes. - enum class Setting : size_t - { - ProgressBarVisualStyle, - AutoUpdateTimeInMinutes, - EFExperimentalCmd, - EFExperimentalArg, - EFExperimentalMSStore, - EFList, - EFExperimentalUpgrade, - EFUninstall, - EFImport, - EFExport, - TelemetryDisable, - EFRestSource, - InstallScopePreference, - InstallScopeRequirement, - Max - }; - - namespace details - { - template <Setting S> - struct SettingMapping - { - // json_t - type the setting in json. - // value_t - the type of this setting. - // DefaultValue - the value_t default value when setting is absent or semantically wrong. - // Path - json path to the property. See Json::Path in json.h for syntax. So far, this is sufficient - // but since is "brief" and "untested" we might implement our own if needed. - // Validate - Function that does semantic validation. - }; - -#define SETTINGMAPPING_SPECIALIZATION_EXTEND(_setting_, _json_, _value_, _default_, _path_, _extension_) \ - template <> \ - struct SettingMapping<_setting_> \ - { \ - using json_t = _json_; \ - using value_t = _value_; \ - static constexpr value_t DefaultValue = _default_; \ - static constexpr std::string_view Path = _path_; \ - static std::optional<value_t> Validate(const json_t& value); \ - _extension_ \ - } - -#define SETTINGMAPPING_SPECIALIZATION(_setting_, _json_, _value_, _default_, _path_) \ - SETTINGMAPPING_SPECIALIZATION_EXTEND(_setting_, _json_, _value_, _default_, _path_, ) - -#define SETTINGMAPPING_SPECIALIZATION_POLICY(_setting_, _json_, _value_, _default_, _path_, _valuePolicy_) \ - SETTINGMAPPING_SPECIALIZATION_EXTEND(_setting_, _json_, _value_, _default_, _path_, \ - static constexpr ValuePolicy Policy = _valuePolicy_; \ - using policy_t = decltype(std::declval<GroupPolicy>().GetValue<Policy>())::value_type; \ - static_assert(std::is_same<json_t, policy_t>::value); \ - ) - - SETTINGMAPPING_SPECIALIZATION(Setting::ProgressBarVisualStyle, std::string, VisualStyle, VisualStyle::Accent, ".visual.progressBar"sv); - SETTINGMAPPING_SPECIALIZATION_POLICY(Setting::AutoUpdateTimeInMinutes, uint32_t, std::chrono::minutes, 5min, ".source.autoUpdateIntervalInMinutes"sv, ValuePolicy::SourceAutoUpdateIntervalInMinutes); - SETTINGMAPPING_SPECIALIZATION(Setting::EFExperimentalCmd, bool, bool, false, ".experimentalFeatures.experimentalCmd"sv); - SETTINGMAPPING_SPECIALIZATION(Setting::EFExperimentalArg, bool, bool, false, ".experimentalFeatures.experimentalArg"sv); - SETTINGMAPPING_SPECIALIZATION(Setting::EFExperimentalMSStore, bool, bool, false, ".experimentalFeatures.experimentalMSStore"sv); - SETTINGMAPPING_SPECIALIZATION(Setting::EFList, bool, bool, false, ".experimentalFeatures.list"sv); - SETTINGMAPPING_SPECIALIZATION(Setting::EFExperimentalUpgrade, bool, bool, false, ".experimentalFeatures.upgrade"sv); - SETTINGMAPPING_SPECIALIZATION(Setting::EFUninstall, bool, bool, false, ".experimentalFeatures.uninstall"sv); - SETTINGMAPPING_SPECIALIZATION(Setting::EFImport, bool, bool, false, ".experimentalFeatures.import"sv); - SETTINGMAPPING_SPECIALIZATION(Setting::EFExport, bool, bool, false, ".experimentalFeatures.export"sv); - SETTINGMAPPING_SPECIALIZATION(Setting::TelemetryDisable, bool, bool, false, ".telemetry.disable"sv); - SETTINGMAPPING_SPECIALIZATION(Setting::EFRestSource, bool, bool, false, ".experimentalFeatures.restSource"sv); - SETTINGMAPPING_SPECIALIZATION(Setting::InstallScopePreference, std::string, ScopePreference, ScopePreference::User, ".installBehavior.preferences.scope"sv); - SETTINGMAPPING_SPECIALIZATION(Setting::InstallScopeRequirement, std::string, ScopePreference, ScopePreference::None, ".installBehavior.requirements.scope"sv); - - // Used to deduce the SettingVariant type; making a variant that includes std::monostate and all SettingMapping types. - template <size_t... I> - inline auto Deduce(std::index_sequence<I...>) { return std::variant<std::monostate, typename SettingMapping<static_cast<Setting>(I)>::value_t...>{}; } - - // Holds data of any type listed in a SettingMapping. - using SettingVariant = decltype(Deduce(std::make_index_sequence<static_cast<size_t>(Setting::Max)>())); - - // Gets the index into the variant for the given Setting. - constexpr inline size_t SettingIndex(Setting s) { return static_cast<size_t>(s) + 1; } - } - - // Representation of the parsed settings file. - struct UserSettings - { - // Jsoncpp doesn't provide line number and column for an individual Json::Value node. +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. +#pragma once +#include "AppInstallerStrings.h" +#include "winget/GroupPolicy.h" +#include "winget/Resources.h" + +#include <filesystem> +#include <map> +#include <optional> +#include <string> +#include <type_traits> +#include <variant> +#include <vector> + +using namespace std::chrono_literals; +using namespace std::string_view_literals; + +namespace AppInstaller::Settings +{ + // The type of argument. + enum class UserSettingsType + { + // Settings files don't exist. A file is created on the first call to the settings command. + Default, + // Loaded settings.json + Standard, + // Loaded settings.json.backup + Backup, + }; + + // The visual style of the progress bar. + enum class VisualStyle + { + NoVT, + Retro, + Accent, + Rainbow, + }; + + // The preferred scope for installs. + enum class ScopePreference + { + None, + User, + Machine, + }; + + // Enum of settings. + // Must start at 0 to enable direct access to variant in UserSettings. + // Max must be last and unused. + // How to add a setting + // 1 - Add to enum. + // 2 - Implement SettingMap specialization via SETTINGMAPPING_SPECIALIZATION + // Validate will be called by ValidateAll without any more changes. + enum class Setting : size_t + { + ProgressBarVisualStyle, + AutoUpdateTimeInMinutes, + EFExperimentalCmd, + EFExperimentalArg, + EFExperimentalMSStore, + EFList, + EFExperimentalUpgrade, + EFUninstall, + EFImport, + EFExport, + TelemetryDisable, + EFRestSource, + InstallScopePreference, + InstallScopeRequirement, + Max + }; + + namespace details + { + template <Setting S> + struct SettingMapping + { + // json_t - type the setting in json. + // value_t - the type of this setting. + // DefaultValue - the value_t default value when setting is absent or semantically wrong. + // Path - json path to the property. See Json::Path in json.h for syntax. So far, this is sufficient + // but since is "brief" and "untested" we might implement our own if needed. + // Validate - Function that does semantic validation. + }; + +#define SETTINGMAPPING_SPECIALIZATION_EXTEND(_setting_, _json_, _value_, _default_, _path_, _extension_) \ + template <> \ + struct SettingMapping<_setting_> \ + { \ + using json_t = _json_; \ + using value_t = _value_; \ + static constexpr value_t DefaultValue = _default_; \ + static constexpr std::string_view Path = _path_; \ + static std::optional<value_t> Validate(const json_t& value); \ + _extension_ \ + } + +#define SETTINGMAPPING_SPECIALIZATION(_setting_, _json_, _value_, _default_, _path_) \ + SETTINGMAPPING_SPECIALIZATION_EXTEND(_setting_, _json_, _value_, _default_, _path_, ) + +#define SETTINGMAPPING_SPECIALIZATION_POLICY(_setting_, _json_, _value_, _default_, _path_, _valuePolicy_) \ + SETTINGMAPPING_SPECIALIZATION_EXTEND(_setting_, _json_, _value_, _default_, _path_, \ + static constexpr ValuePolicy Policy = _valuePolicy_; \ + using policy_t = GroupPolicy::ValueType<Policy>; \ + static_assert(std::is_same<json_t, policy_t>::value); \ + ) + + SETTINGMAPPING_SPECIALIZATION(Setting::ProgressBarVisualStyle, std::string, VisualStyle, VisualStyle::Accent, ".visual.progressBar"sv); + SETTINGMAPPING_SPECIALIZATION_POLICY(Setting::AutoUpdateTimeInMinutes, uint32_t, std::chrono::minutes, 5min, ".source.autoUpdateIntervalInMinutes"sv, ValuePolicy::SourceAutoUpdateIntervalInMinutes); + SETTINGMAPPING_SPECIALIZATION(Setting::EFExperimentalCmd, bool, bool, false, ".experimentalFeatures.experimentalCmd"sv); + SETTINGMAPPING_SPECIALIZATION(Setting::EFExperimentalArg, bool, bool, false, ".experimentalFeatures.experimentalArg"sv); + SETTINGMAPPING_SPECIALIZATION(Setting::EFExperimentalMSStore, bool, bool, false, ".experimentalFeatures.experimentalMSStore"sv); + SETTINGMAPPING_SPECIALIZATION(Setting::EFList, bool, bool, false, ".experimentalFeatures.list"sv); + SETTINGMAPPING_SPECIALIZATION(Setting::EFExperimentalUpgrade, bool, bool, false, ".experimentalFeatures.upgrade"sv); + SETTINGMAPPING_SPECIALIZATION(Setting::EFUninstall, bool, bool, false, ".experimentalFeatures.uninstall"sv); + SETTINGMAPPING_SPECIALIZATION(Setting::EFImport, bool, bool, false, ".experimentalFeatures.import"sv); + SETTINGMAPPING_SPECIALIZATION(Setting::EFExport, bool, bool, false, ".experimentalFeatures.export"sv); + SETTINGMAPPING_SPECIALIZATION(Setting::TelemetryDisable, bool, bool, false, ".telemetry.disable"sv); + SETTINGMAPPING_SPECIALIZATION(Setting::EFRestSource, bool, bool, false, ".experimentalFeatures.restSource"sv); + SETTINGMAPPING_SPECIALIZATION(Setting::InstallScopePreference, std::string, ScopePreference, ScopePreference::User, ".installBehavior.preferences.scope"sv); + SETTINGMAPPING_SPECIALIZATION(Setting::InstallScopeRequirement, std::string, ScopePreference, ScopePreference::None, ".installBehavior.requirements.scope"sv); + + // Used to deduce the SettingVariant type; making a variant that includes std::monostate and all SettingMapping types. + template <size_t... I> + inline auto Deduce(std::index_sequence<I...>) { return std::variant<std::monostate, typename SettingMapping<static_cast<Setting>(I)>::value_t...>{}; } + + // Holds data of any type listed in a SettingMapping. + using SettingVariant = decltype(Deduce(std::make_index_sequence<static_cast<size_t>(Setting::Max)>())); + + // Gets the index into the variant for the given Setting. + constexpr inline size_t SettingIndex(Setting s) { return static_cast<size_t>(s) + 1; } + } + + // Representation of the parsed settings file. + struct UserSettings + { + // Jsoncpp doesn't provide line number and column for an individual Json::Value node. struct Warning { Warning(StringResource::StringId message) : Message(message) {} @@ -149,46 +149,46 @@ namespace AppInstaller::Settings std::string Data; bool IsFieldWarning = true; }; - - static UserSettings const& Instance(); - - static std::filesystem::path SettingsFilePath(); - - UserSettings(const UserSettings&) = delete; - UserSettings& operator=(const UserSettings&) = delete; - - UserSettings(UserSettings&&) = delete; - UserSettings& operator=(UserSettings&&) = delete; - - UserSettingsType GetType() const { return m_type; } - std::vector<Warning> const& GetWarnings() const { return m_warnings; } - - void PrepareToShellExecuteFile() const; - - // Gets setting value, if its not in the map it returns the default value. - template <Setting S> - typename details::SettingMapping<S>::value_t Get() const - { - auto itr = m_settings.find(S); - if (itr == m_settings.end()) - { - return details::SettingMapping<S>::DefaultValue; - } - - return std::get<details::SettingIndex(S)>(itr->second); - } - - protected: - UserSettingsType m_type = UserSettingsType::Default; - std::vector<Warning> m_warnings; - std::map<Setting, details::SettingVariant> m_settings; - - UserSettings(); - ~UserSettings() = default; - }; - - inline UserSettings const& User() - { - return UserSettings::Instance(); - } -} + + static UserSettings const& Instance(); + + static std::filesystem::path SettingsFilePath(); + + UserSettings(const UserSettings&) = delete; + UserSettings& operator=(const UserSettings&) = delete; + + UserSettings(UserSettings&&) = delete; + UserSettings& operator=(UserSettings&&) = delete; + + UserSettingsType GetType() const { return m_type; } + std::vector<Warning> const& GetWarnings() const { return m_warnings; } + + void PrepareToShellExecuteFile() const; + + // Gets setting value, if its not in the map it returns the default value. + template <Setting S> + typename details::SettingMapping<S>::value_t Get() const + { + auto itr = m_settings.find(S); + if (itr == m_settings.end()) + { + return details::SettingMapping<S>::DefaultValue; + } + + return std::get<details::SettingIndex(S)>(itr->second); + } + + protected: + UserSettingsType m_type = UserSettingsType::Default; + std::vector<Warning> m_warnings; + std::map<Setting, details::SettingVariant> m_settings; + + UserSettings(); + ~UserSettings() = default; + }; + + inline UserSettings const& User() + { + return UserSettings::Instance(); + } +} diff --git a/src/AppInstallerCommonCore/Registry.cpp b/src/AppInstallerCommonCore/Registry.cpp @@ -48,6 +48,90 @@ namespace AppInstaller::Registry return result; } + + bool TryGetRegistryValueNameFromIndex(const wil::shared_hkey& key, DWORD index, std::wstring& valueName) + { + constexpr DWORD MaxNameLength = 32767; + LSTATUS status = ERROR_SUCCESS; + DWORD charCount = 0; + valueName = L'\0'; + + while (valueName.size() <= MaxNameLength) + { + charCount = wil::safe_cast<DWORD>(valueName.size()); + + // We could also get the type and data here, but we read only the name instead + // to prevent duplication with the code that gets the data from the name. + status = RegEnumValueW(key.get(), index, &valueName[0], &charCount, nullptr, nullptr, nullptr, nullptr); + + if (status == ERROR_MORE_DATA) + { + // See if we can get away with the current capacity + if (valueName.size() < valueName.capacity()) + { + valueName.resize(valueName.capacity()); + } + else + { + valueName.resize(valueName.capacity() * 2); + } + } + else + { + break; + } + } + + if (status == ERROR_SUCCESS) + { + valueName.resize(wil::safe_cast<size_t>(charCount)); + return true; + } + else if (status == ERROR_NO_MORE_ITEMS) + { + return false; + } + else + { + THROW_IF_WIN32_ERROR(status); + return false; + } + } + + bool TryGetRegistryValueData(const wil::shared_hkey& key, const std::wstring& valueName, DWORD& type, std::vector<BYTE>& data) + { + data.resize(64); + + LSTATUS status = ERROR_SUCCESS; + DWORD byteCount = 0; + + while (data.size() < (64 << 20)) + { + byteCount = wil::safe_cast<DWORD>(data.size()); + status = RegGetValueW(key.get(), nullptr, valueName.c_str(), RRF_RT_ANY | RRF_NOEXPAND, &type, data.data(), &byteCount); + + if (status == ERROR_MORE_DATA && byteCount > data.size()) + { + data.resize(byteCount); + } + else + { + break; + } + } + + if (status == ERROR_FILE_NOT_FOUND) + { + return false; + } + + THROW_IF_WIN32_ERROR(status); + + // Resize to actual data size + data.resize(byteCount); + + return true; + } } namespace details @@ -93,6 +177,87 @@ namespace AppInstaller::Registry return m_type == type; } + ValueList::ValueRef::ValueRef(std::wstring&& valueName, DWORD type, std::vector<BYTE>&& data) : Value(type, std::move(data)), m_valueName(std::move(valueName)) {} + + std::string ValueList::ValueRef::Name() const + { + return Utility::ConvertToUTF8(m_valueName); + } + + ValueList::const_iterator& ValueList::const_iterator::operator++() + { + ++m_index; + GetValue(); + return *this; + } + + ValueList::const_iterator ValueList::const_iterator::operator++(int) + { + const_iterator result; + result.m_key = m_key; + result.m_index = m_index++; + result.m_value = std::nullopt; + std::swap(m_value, result.m_value); + GetValue(); + return result; + } + + bool ValueList::const_iterator::operator==(const const_iterator& other) const + { + return (!m_key && !other.m_key) || (m_key.get() == other.m_key.get() && m_index == other.m_index); + } + + bool ValueList::const_iterator::operator!=(const const_iterator& other) const + { + return !operator==(other); + } + + void ValueList::const_iterator::GetValue() + { + std::wstring valueName; + if (!TryGetRegistryValueNameFromIndex(m_key, m_index, valueName)) + { + m_key.reset(); + return; + } + + DWORD type; + std::vector<BYTE> data; + if (!TryGetRegistryValueData(m_key, valueName, type, data)) + { + THROW_HR(E_UNEXPECTED); + } + + m_value = ValueRef{ std::move(valueName), type, std::move(data) }; + } + + const ValueList::ValueRef& ValueList::const_iterator::operator*() const + { + return m_value.value(); + } + + const ValueList::ValueRef* ValueList::const_iterator::operator->() const + { + return &m_value.value(); + } + + ValueList::const_iterator::const_iterator(const wil::shared_hkey& key, DWORD index) : m_key(key), m_index(index) + { + GetValue(); + } + + ValueList::const_iterator ValueList::begin() const + { + return { m_key }; + } + + ValueList::const_iterator ValueList::end() const + { + return {}; + } + + ValueList::ValueList(wil::shared_hkey key) : m_key(key) {} + Key::Key(HKEY key) { Initialize(key, {}, 0, KEY_READ, false); @@ -221,39 +386,45 @@ namespace AppInstaller::Registry std::optional<Value> Key::operator[](const std::wstring& name) const { + DWORD type; std::vector<BYTE> data; - data.resize(64); - LSTATUS status = ERROR_SUCCESS; - DWORD type = 0; - DWORD byteCount = 0; - - while (data.size() < (64 << 20)) + if (TryGetRegistryValueData(m_key, name, type, data)) { - byteCount = wil::safe_cast<DWORD>(data.size()); - status = RegGetValueW(m_key.get(), nullptr, name.c_str(), RRF_RT_ANY | RRF_NOEXPAND, &type, data.data(), &byteCount); - - if (status == ERROR_MORE_DATA && byteCount > data.size()) - { - data.resize(byteCount); - } - else - { - break; - } + return Value{ type, std::move(data) }; } - - if (status == ERROR_FILE_NOT_FOUND) + else { return {}; } + } - THROW_IF_WIN32_ERROR(status); + std::optional<Key> Key::SubKey(std::string_view subKey, DWORD options) const + { + return SubKey(Utility::ConvertToUTF16(subKey), options); + } - // Resize to actual data size - data.resize(byteCount); + std::optional<Key> Key::SubKey(const std::wstring& subKey, DWORD options) const + { + if (!m_key) + { + return std::nullopt; + } + + Key result; + if (result.Initialize(m_key.get(), subKey, options, m_access, true)) + { + return result; + } + else + { + return std::nullopt; + } + } - return Value{ type, std::move(data) }; + ValueList Key::Values() const + { + return { m_key }; } Key Key::OpenIfExists(HKEY key, std::string_view subKey, DWORD options, REGSAM access) @@ -268,16 +439,18 @@ namespace AppInstaller::Registry return result; } - void Key::Initialize(HKEY key, const std::wstring& subKey, DWORD options, REGSAM access, bool ignoreErrorIfDoesNotExist) + bool Key::Initialize(HKEY key, const std::wstring& subKey, DWORD options, REGSAM access, bool ignoreErrorIfDoesNotExist) { + m_access = access; LSTATUS status = RegOpenKeyExW(key, subKey.c_str(), options, access, &m_key); if (ignoreErrorIfDoesNotExist && status == ERROR_FILE_NOT_FOUND) { AICLI_LOG(Core, Verbose, << "Subkey '" << Utility::ConvertToUTF8(subKey) << "' was not found"); - return; + return false; } THROW_IF_WIN32_ERROR(status); + return true; } } diff --git a/src/AppInstallerRepositoryCore/Public/AppInstallerRepositorySource.h b/src/AppInstallerRepositoryCore/Public/AppInstallerRepositorySource.h @@ -21,6 +21,7 @@ namespace AppInstaller::Repository Default, User, Predefined, + GroupPolicy, }; // Defines the trust level of the source. diff --git a/src/AppInstallerRepositoryCore/RepositorySource.cpp b/src/AppInstallerRepositoryCore/RepositorySource.cpp @@ -49,6 +49,194 @@ namespace AppInstaller::Repository 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<ValuePolicy P> + std::optional<SourceFromPolicy> FindSourceInPolicy(std::string_view name, std::string_view type, std::string_view arg) + { + auto sourcesOpt = GroupPolicies().GetValueRef<P>(); + 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<ValuePolicy P> + bool IsSourceInPolicy(std::string_view name, std::string_view type, std::string_view arg) + { + return FindSourceInPolicy<P>(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<ValuePolicy::AllowedSources>(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<typename Value> bool TryReadScalar(std::string_view settingName, const std::string& settingValue, const YAML::Node& sourceNode, std::string_view name, Value& value, bool required = true) @@ -197,16 +385,6 @@ namespace AppInstaller::Repository return true; } - bool IsWingetCommunityDefaultSourceEnabled() - { - return IsDefaultSourceEnabled(s_Source_WingetCommunityDefault_Name, ExperimentalFeature::Feature::None, TogglePolicy::Policy::DefaultSource); - } - - bool IsWingetMSStoreDefaultSourceEnabled() - { - return IsDefaultSourceEnabled(s_Source_WingetMSStoreDefault_Name, ExperimentalFeature::Feature::ExperimentalMSStore, TogglePolicy::Policy::MSStoreSource); - } - // Gets the sources from a particular origin. std::vector<SourceDetailsInternal> GetSourcesByOrigin(SourceOrigin origin) { @@ -230,18 +408,17 @@ namespace AppInstaller::Repository if (IsWingetMSStoreDefaultSourceEnabled()) { - SourceDetailsInternal storeDetails; - storeDetails.Name = s_Source_WingetMSStoreDefault_Name; - storeDetails.Type = Microsoft::PreIndexedPackageSourceFactory::Type(); - storeDetails.Arg = s_Source_WingetMSStoreDefault_Arg; - storeDetails.Data = s_Source_WingetMSStoreDefault_Data; - storeDetails.Identifier = s_Source_WingetMSStoreDefault_Identifier; - storeDetails.TrustLevel = SourceTrustLevel::Trusted; - result.emplace_back(std::move(storeDetails)); + 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; + result.emplace_back(std::move(details)); } - - break; } + break; case SourceOrigin::User: { std::vector<SourceDetailsInternal> userSources = GetSourcesFromSetting( @@ -267,10 +444,40 @@ namespace AppInstaller::Repository continue; } + // 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<ValuePolicy::AdditionalSources>(); + 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); } @@ -483,7 +690,7 @@ namespace AppInstaller::Repository SourceListInternal::SourceListInternal() { - for (SourceOrigin origin : { SourceOrigin::User, SourceOrigin::Default }) + for (SourceOrigin origin : { SourceOrigin::GroupPolicy, SourceOrigin::User, SourceOrigin::Default }) { auto forOrigin = GetSourcesByOrigin(origin); @@ -586,6 +793,10 @@ namespace AppInstaller::Repository 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); } @@ -597,39 +808,6 @@ namespace AppInstaller::Repository { SetMetadata(m_sourceList); } - - // Checks whether the group policy allows this source. - // Reasons for not allowing: - // - The source is a default source that is disabled - // - Allowed sources are disabled, blocking everything - // - There is an explicit list of allowed sources and this source is not in it - bool IsSourceAllowedByPolicy(std::string_view, std::string_view, std::string_view arg) - { - if (Utility::CaseInsensitiveEquals(arg, s_Source_WingetCommunityDefault_Arg)) - { - return IsWingetCommunityDefaultSourceEnabled(); - } - - if (Utility::CaseInsensitiveEquals(arg, s_Source_WingetMSStoreDefault_Arg)) - { - return IsWingetMSStoreDefaultSourceEnabled(); - } - - auto allowedSourcesPolicy = GroupPolicies().GetState(TogglePolicy::Policy::AllowedSources); - if (allowedSourcesPolicy == PolicyState::Disabled) - { - // We check here but this should already be blocked higher in the stack - AICLI_LOG(Repo, Warning, << "Additional sources are blocked by group policy"); - return false; - } - - if (allowedSourcesPolicy == PolicyState::Enabled) - { - // TODO: Allowed sources - } - - return true; - } } std::string_view ToString(SourceOrigin origin) @@ -640,6 +818,8 @@ namespace AppInstaller::Repository return "Default"sv; case SourceOrigin::User: return "User"sv; + case SourceOrigin::GroupPolicy: + return "GroupPolicy"sv; default: THROW_HR(E_UNEXPECTED); } @@ -686,7 +866,12 @@ namespace AppInstaller::Repository auto source = sourceList.GetCurrentSource(name); THROW_HR_IF(APPINSTALLER_CLI_ERROR_SOURCE_NAME_ALREADY_EXISTS, source != nullptr); - THROW_HR_IF(APPINSTALLER_CLI_ERROR_BLOCKED_BY_POLICY, !IsSourceAllowedByPolicy(name, type, arg)); + // Check sources allowed by group policy + auto blockingPolicy = GetPolicyBlockingUserSource(name, type, arg, false); + if (blockingPolicy != TogglePolicy::Policy::None) + { + throw GroupPolicyException(blockingPolicy); + } SourceDetailsInternal details; details.Name = name; @@ -878,8 +1063,9 @@ namespace AppInstaller::Repository else { AICLI_LOG(Repo, Info, << "Named source to be removed, found: " << source->Name << " [" << ToString(source->Origin) << ']'); - RemoveSourceFromDetails(*source, progress); + EnsureSourceIsRemovable(*source); + RemoveSourceFromDetails(*source, progress); sourceList.RemoveSource(*source); return true; @@ -908,6 +1094,7 @@ namespace AppInstaller::Repository { AICLI_LOG(Repo, Info, << "Named source to be dropped, found: " << source->Name); + EnsureSourceIsRemovable(*source); sourceList.RemoveSource(*source); return true;