[Run, Enterprise] GPO for plugin enabled state

[htcfreek]

Summary of the Pull Request

This PR implements one or two GPOs to control the enabled state of PT Run plugins using

• a default enabled state value.
• a list with per plugin enabled state values to overwrite the default value.

On plugin init each plugin's id will be compared against a property in plugins code. We do this as protection against plugin.json manipulations. (⚠️ This is be a braking change!!)

Todo list

• Add policies to ADMX
  • Two policies for easy usage and most flexibility
• Implement GPO code
• Update Wox code
  • Evaluating GPO state
  • Write state to settings.json
  • Validating plugin id that we get from plugin.json [⚠ BREAKING CHANGE!!]
• Updating settings code and ui
  • Toggle switch behavior
  • Info bar "Some plugins are managed"
  • Fix bug of not reloading the settings ui on PT Run restart (separate PR ?) => (We are waiting on user feedback.)
• Finalize work
  • Update supportedOn version shortly before release
  • Add requires restart of Run note
  • Update ADMX docs
  • Update BugReportTool (separate PR ?) => Issue [BugReport Tool] Update GPO reports to include new PT Run plugin GPOs #29067

PR Checklist

• Closes: [Enterprise; PT Run] Add policy to manage plugins' enabled state #25438
• Communication: I've discussed this with core contributors already. If work hasn't been agreed, this work might be rejected
• Tests: Added/updated and all pass
• Localization: All end user facing strings can be localized
• Dev docs: Added/updated
• New binaries: Added on the required places
  • JSON for signing for new binaries
  • WXS for installer for new binaries and localization folder
  • YML for CI pipeline for new test projects
  • YML for signed pipeline
• Documentation updated: If checked, please file a pull request on our docs repo and link it here: #xxx

Detailed Description of the Pull Request / Additional comments

Validation Steps Performed

Tested locally on my computer:

• Global policy ✅
• Individual policy ✅
• Mixed policy configuration (only user and only client) ✅
• Mixed policy configuration (One part set for computer and one part set for user.) ✅

[htcfreek]

@stefansjfw, @jaimecbernardo
I tried to implement a string parameter in the new gpo.h utility method. But the code throws build errors. Can you help me here as I have not so much knowledge with cpp?

[davidegiacometti]

@htcfreek try to use String for IDL and winrt::hstring for C++.

[htcfreek]

@davidegiacometti

try to use String for IDL

Will try this.

and winrt::hstring for C++.

Only in the wrapper code or in gpo.h too?

[davidegiacometti]

winrt::hstring

@davidegiacometti

try to use String for IDL

Will try this.

and winrt::hstring for C++.

Only in the wrapper code or in gpo.h too?

Both

[jaimecbernardo]

Hi @htcfreek ,
The types that can be used in IDL are specified here: https://learn.microsoft.com/en-us/uwp/midl-3/intro#types
So in the IDL you actually want "String", which translates to "winrt:string", as davidegiacometti said.
Hope this helps.
Here's a git diff of the changes I did to have this building:

diff --git a/src/common/GPOWrapper/GPOWrapper.cpp b/src/common/GPOWrapper/GPOWrapper.cpp
index 54f8c1922..72a3ea17c 100644
--- a/src/common/GPOWrapper/GPOWrapper.cpp
+++ b/src/common/GPOWrapper/GPOWrapper.cpp
@@ -140,8 +140,8 @@ namespace winrt::PowerToys::GPOWrapper::implementation
     {
         return static_cast<GpoRuleConfigured>(powertoys_gpo::getAllowExperimentationValue());
     }
-    GpoRuleConfigured GPOWrapper::GetRunPluginEnabledValue(LPCWSTR pluginID)
+    GpoRuleConfigured GPOWrapper::GetRunPluginEnabledValue(winrt::hstring const& pluginID)
     {
-        return static_cast<GpoRuleConfigured>(powertoys_gpo)::getRunPluginEnabledValue(pluginID));
+        return static_cast<GpoRuleConfigured>(powertoys_gpo::getRunPluginEnabledValue(winrt::to_string(pluginID)));
     }
 }
diff --git a/src/common/GPOWrapper/GPOWrapper.h b/src/common/GPOWrapper/GPOWrapper.h
index f9f0703f9..4378f9f83 100644
--- a/src/common/GPOWrapper/GPOWrapper.h
+++ b/src/common/GPOWrapper/GPOWrapper.h
@@ -41,7 +41,7 @@ namespace winrt::PowerToys::GPOWrapper::implementation
         static GpoRuleConfigured GetConfiguredPeekEnabledValue();
         static GpoRuleConfigured GetDisableAutomaticUpdateDownloadValue();
         static GpoRuleConfigured GetAllowExperimentationValue();
-        static GpoRuleConfigured GetRunPluginEnabledValue(LPCWSTR pluginID);
+        static GpoRuleConfigured GetRunPluginEnabledValue(winrt::hstring const& pluginID);
     };
 }
 
diff --git a/src/common/GPOWrapper/GPOWrapper.idl b/src/common/GPOWrapper/GPOWrapper.idl
index d1c04c4d7..46b8de7b4 100644
--- a/src/common/GPOWrapper/GPOWrapper.idl
+++ b/src/common/GPOWrapper/GPOWrapper.idl
@@ -45,7 +45,7 @@ namespace PowerToys
             static GpoRuleConfigured GetConfiguredPeekEnabledValue();
             static GpoRuleConfigured GetDisableAutomaticUpdateDownloadValue();
             static GpoRuleConfigured GetAllowExperimentationValue();
-            static GpoRuleConfigured GetRunPluginEnabledValue(LPCWSTR pluginID);
+            static GpoRuleConfigured GetRunPluginEnabledValue(String pluginID);
         }
     }
 }
diff --git a/src/common/GPOWrapperProjection/GPOWrapper.cs b/src/common/GPOWrapperProjection/GPOWrapper.cs
index 48fd08b06..92196323e 100644
--- a/src/common/GPOWrapperProjection/GPOWrapper.cs
+++ b/src/common/GPOWrapperProjection/GPOWrapper.cs
@@ -52,9 +52,9 @@ namespace PowerToys.GPOWrapperProjection
             return (GpoRuleConfigured)PowerToys.GPOWrapper.GPOWrapper.GetConfiguredPeekEnabledValue();
         }
 
-        public static GpoRuleConfigured GetRunPluginEnabledValuel(string pluginID)
+        public static GpoRuleConfigured GetRunPluginEnabledValue(string pluginID)
         {
-            return (GpoRuleConfigured)PowerToys.GPOWrapper.GPOWrapper.GetRunPluginEnabledValuel(pluginID);
+            return (GpoRuleConfigured)PowerToys.GPOWrapper.GPOWrapper.GetRunPluginEnabledValue(pluginID);
         }
     }
 }
diff --git a/src/common/utils/gpo.h b/src/common/utils/gpo.h
index 0a7e503ec..4ba3ae1cc 100644
--- a/src/common/utils/gpo.h
+++ b/src/common/utils/gpo.h
@@ -299,7 +299,7 @@ namespace powertoys_gpo {
         return getConfiguredValue(POLICY_ALLOW_EXPERIMENTATION);
     }
 
-    inline gpo_rule_configured_t getRunPluginEnabledValue(LPCWSTR pluginID)
+    inline gpo_rule_configured_t getRunPluginEnabledValue(std::string pluginID)
     {
         // Later we read the registry value for the specified plugin ID or if it not exist the default enbaled state.
         return gpo_rule_configured_disabled;

[htcfreek]

@jaimecbernardo , @davidegiacometti
Thank you. I will try it. It was definitely to late for doing such coding yesterday evening. 😅

[htcfreek]

@jaimecbernardo
Thank you very much. Now it builds and seems to work.

But I still have two quirks in VS:

1. I can't debug gpo.h because VS says it has no symbols.
2. Intelisense complains about missing parameter overload in the method called from GPOWrapper.cs which was caused by an outdated autogenerated file. After deleting the wrong file VS doesn't generate it again. How can I fix this or should I delete the local clone an clone the repository again?

[jaimecbernardo]

@jaimecbernardo Thank you very much. Now it builds and seems to work.

But I still have two quirks in VS:

1. I can't debug gpo.h because VS says it has no symbols.
2. Intelisense complains about missing parameter overload in the method called from GPOWrapper.cs which was caused by an outdated autogenerated file. After deleting the wrong file VS doesn't generate it again. How can I fix this or should I delete the local clone an clone the repository again?

@htcfreek , for 1 I think you solve it by enabling native code debugging in the C# project you are debugging (I think you need it for PowerLauncher at least, not sure if you need it for individual libraries too).
For 2, I usually just git clean -fxd on the repo, after making sure I don't have any lose files that I forgot to add to git, since this deletes any files that are not tracked by git.

Hope this helps.

[htcfreek]

@jaimecbernardo Except of two things this PR is ready:

1. The settings page does not update the plugin settings on changing the settings.json file.
2. We need to add the new policies to the BugReport Tool which I can't do because we need to query registry dynamically. (I think it makes in general more sense to simply do a reg export of HKCU\Software\Policies\PowerToys and HKLM\Software\Policies\PowerToys instead of a hard coded export. With simple exports we have more information than with the final method result.)

Should I mark this PR as ready and we fix/implement the two things later? Or would you like to look into these two things?.

@jaimecbernardo, can you please answer on my question about marking the PR as ready? I can't work on this for the second half of Oktober for private reasons.

[jaimecbernardo]

@jaimecbernardo Except of two things this PR is ready:

1. The settings page does not update the plugin settings on changing the settings.json file.
2. We need to add the new policies to the BugReport Tool which I can't do because we need to query registry dynamically. (I think it makes in general more sense to simply do a reg export of HKCU\Software\Policies\PowerToys and HKLM\Software\Policies\PowerToys instead of a hard coded export. With simple exports we have more information than with the final method result.)

Should I mark this PR as ready and we fix/implement the two things later? Or would you like to look into these two things?.

Anyway we should merge this for the 0.75.0 release because of the braking changes in the plugin code requirements (the plugin ID property). I suggest to add a hint about this breaking change to the release notes of 0.74.0 to notify all third-party plugin developers about this change.

Hi For 1. you mean that it won't update well if you change the policy. But for example if you start PowerToys and don't change the GPO while PowerToys is running everything will makes sense, right? If that's the case, I'm OK with it. There's some soft expectations that GPO won't change during a run.

For 2. I'm OK with a reg export.
Thanks @htcfreek

[htcfreek]

@jaimecbernardo Except of two things this PR is ready:

1. The settings page does not update the plugin settings on changing the settings.json file.
2. We need to add the new policies to the BugReport Tool which I can't do because we need to query registry dynamically. (I think it makes in general more sense to simply do a reg export of HKCU\Software\Policies\PowerToys and HKLM\Software\Policies\PowerToys instead of a hard coded export. With simple exports we have more information than with the final method result.)

Should I mark this PR as ready and we fix/implement the two things later? Or would you like to look into these two things?.

Anyway we should merge this for the 0.75.0 release because of the braking changes in the plugin code requirements (the plugin ID property). I suggest to add a hint about this breaking change to the release notes of 0.74.0 to notify all third-party plugin developers about this change.

Hi For 1. you mean that it won't update well if you change the policy. But for example if you start PowerToys and don't change the GPO while PowerToys is running everything will makes sense, right? If that's the case, I'm OK with it. There's some soft expectations that GPO won't change during a run.

The problems is not only about GPOs.

Steps to reproduce:

1. Open settings for PT Run.
2. Disable PT Run.
3. Change GPO.
4. Enable PT Run.
   => Plugins in settings ui should update because PT Run executes

   PowerToys/src/modules/launcher/PowerLauncher/SettingsReader.cs

   Line 253 in d5eef57

   private static void UpdateSettings(PowerLauncherSettings settings)

   . But they didn't.

For 2. I'm OK with a reg export.
Thanks @htcfreek

Could you implement this? I don't have much time the next weeks. Or do we separate this work?

[jaimecbernardo]

@htcfreek ,
1 - I think the behavior is looking good, after some quick tests. I'd say our assurance is more on the side of gpo applies well after a complete PowerToys restart.
2 - OK with separating. Not crucial, tbh.
Mark it as ready to review whenever you think it makes sense ;) Perhaps after fixing conflicts. I could also look into those if you prefer. Thanks for all your work here.

[htcfreek]

@jaimecbernardo

1 - I think the behavior is looking good, after some quick tests. I'd say our assurance is more on the side of gpo applies well after a complete PowerToys restart.

Then let's keep as is and wait if we get feedback.

2 - OK with separating. Not crucial, tbh.

I will create a new issue.

Mark it as ready to review whenever you think it makes sense ;) Perhaps after fixing conflicts. I could also look into those if you prefer. Thanks for all your work here.

Will do it later after trying one small ui improvement. (I like the idea from the new dashboard to show look symbols near the on/off switch.)

[htcfreek]

@jaimecbernardo

• The PR is ready for review.
• I decided to not implement the look icon thing in settings ui to not overload the ui.
• All merge conflicts are fixed.

Let us get this in now. 🚀🔥

[jaimecbernardo]

LGTM! Thank you for the contribution!
Tested many combinations and behavior was as expected.
Added some nits. WDYT?

[jaimecbernardo]

LGTM! Thank you for the contribution!

[htcfreek]

@jaimecbernardo
I will create a PR to update user docs.