-
Notifications
You must be signed in to change notification settings - Fork 8.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add a policy for profile sources #18009
Conversation
Love this. Will you provide special admx templates or will they be part of the os built-in templates? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
simple and glorious
So far, I wasn't planning on doing that, but if it helps you a lot, I could look into it. I've never written one, so I'd have to figure that out first. |
@lhecker You can look at the files in the PowerToys project. And if you have questions you can ask me for help. https://github.com/microsoft/PowerToys/tree/main/src%2Fgpo%2Fassets https://github.com/microsoft/PowerToys/blob/main/src%2Fgpo%2Fassets%2FPowerToys.admx#L609 => Multiline Textbox sample: MwbPolicyDefinedIpMappingRules policy. |
The |
Sure. But for managing and using them you either need ADMX templates or you have to create registry group policy preferences. So creating ADMX templates is a must have and makes it much easier for admins. Imagin you like to set the policy for some users (HKCU) in an enterprise environment where the users have no administrative permissions. This doesn't work without ADMX templates/Group Policy Preferences. |
@@ -0,0 +1,23 @@ | |||
<?xml version="1.0" encoding="utf-8"?> |
Check failure
Code scanning / check-spelling
Check File Path Error
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Check File Path
admx is not a recognized word. (check-file-path)
This comment has been minimized.
This comment has been minimized.
@lhecker |
@htcfreek if you're comfortable signing off or rejecting based on the ADMX changes, I would appreciate it! You know more about this than I do, that's for sure. 🙂 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A small improvement suggestion.
Will do a technical test of the ADMX template tomorrow. After that I will approve the PR.
policies/WindowsTerminal.admx
Outdated
<policies> | ||
<policy name="DisabledProfileSources" class="Both" displayName="$(string.DisabledProfileSources)" explainText="$(string.DisabledProfileSourcesText)" presentation="$(presentation.DisabledProfileSources)" key="Software\Policies\Microsoft\Windows\Terminal"> | ||
<parentCategory ref="WindowsTerminal" /> | ||
<supportedOn ref="terminal:SUPPORTED_WindowsTerminal_1_21" /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
<supportedOn ref="terminal:SUPPORTED_WindowsTerminal_1_21" /> | |
<supportedOn ref="SUPPORTED_WindowsTerminal_1_21" /> |
Is unique enough.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
1.21 is the current stable and I am sure it does not support the policy. Shouldn't it be 1.22 or 1.23?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We intend to backport this change to 1.21 along with other bug fixes to help a customer. 🙂
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But then please provide an exact version to avoid confusion: 1.21.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The .adml contains:
<string id="SUPPORTED_WindowsTerminal_1_21">At least Windows Terminal 1.21</string>
Is that not enough? I know that the supportedOn
element can contain a version, but I only saw it being used on the OS version, not on package versions...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Small text improvements
policies/en-US/WindowsTerminal.adml
Outdated
- Windows.Terminal.PowershellCore | ||
- Windows.Terminal.Wsl | ||
|
||
For instance, setting this policy to Windows.Terminal.Wsl will disable the builtin WSL integration of Windows Terminal.</string> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For instance, setting this policy to Windows.Terminal.Wsl will disable the builtin WSL integration of Windows Terminal.</string> | |
For instance, setting this policy to Windows.Terminal.Wsl will disable the builtin WSL integration of Windows Terminal. | |
Note: Allready existing profiles will disappear from Windows Terminal after adding their profile source to the policy.</string> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've adopted both of your suggestions with minor changes to them (on VS of, etc.). Thanks!
src/cascadia/TerminalSettingsModel/CascadiaSettingsSerialization.cpp
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ADMX/ADML and registry path lgtm.
Regarding the "Supported on" value of the ADMX: This should be as exact as possible. It would be a worst case for an admin reading 1.21, using 1.21.1 and the policy works not before 1.21.2 release.
Additional I suggest to add a page or something else to docs.msft.com describing how to install the admx files and what policies exist. As example you can look at the PowerToys documentation.
In that case, we'll just have to update the file after the update has shipped so that we know the correct number. Since the admx/adml files won't be part of the .msix package, we should have enough time to update it.
Thanks! I'll keep it in mind and write a page soon. 🙂 |
@htcfreek thank you for the review, and all the help! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm fine with this. IDK how we're shipping the translated .adml's, but looks fine to me.
That not a must have. But i imagine somehow that Visual Studio or VS Code has translated adml and an automated process. |
This adds a basic policy check for DisabledProfileSources, so that organizations can easily disable certain profiles like the Azure one. Closes #17964 * Add a policy to disable Azure under HKCU. Disabled ✅ * Add a policy to disable nothing under HKLM. Enabled ✅ (...because it overrides the HKCU setting.) (cherry picked from commit 3a06826) Service-Card-Id: PVTI_lADOAF3p4s4AmhmszgT6i0Y Service-Version: 1.21
This adds a basic policy check for DisabledProfileSources, so that organizations can easily disable certain profiles like the Azure one. Closes #17964 ## Validation Steps Performed * Add a policy to disable Azure under HKCU. Disabled ✅ * Add a policy to disable nothing under HKLM. Enabled ✅ (...because it overrides the HKCU setting.) (cherry picked from commit 3a06826) Service-Card-Id: PVTI_lADOAF3p4s4AmhmQzgT6i0c Service-Version: 1.22
Addresses microsoft/terminal#18095 and microsoft/terminal#18009 --------- Co-authored-by: Heiko <[email protected]> Co-authored-by: Matt Wojciakowski <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will upgrade soon
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks
This adds a basic policy check for DisabledProfileSources, so that
organizations can easily disable certain profiles like the Azure one.
Closes #17964
Validation Steps Performed
(...because it overrides the HKCU setting.)