Add ability to specify default settings values from the registry on Windows #404
Conversation
Introduce the concept of another layer of settings (lowest precedence; default values only) below the existing environment variable and Git configuration file mechanisms. Implement on Windows using the Registry, under key: HKLM\SOFTWARE\GitCredentialManager\Configuration
Add documentation about the new enterprise/registry settings.
mjcheetham
requested review from
vtbassmatt,
derrickstolee and
dscho
and removed request for
vtbassmatt
|
This is cool. My only question is, how certain are we that this is the correct area of the registry? (Genuine question -- you've probably done your homework, I just don't know how MDM systems expect to configure the registry.) Also, as a future thought -- when we build some diagnostics, it would be nice to emit configuration and where those config values came from. Env vars are obvious, Git config can be slightly obscure, but the registry is really hidden from the average user's view (even a power user). Doesn't block this work at all, thought. |
This was my immediate thought as well! 😉 I'll add a read out of all keys in the upcoming |
Group Policy and/or Intune can set any arbitrary registry value from what I understand. The Local Machine hive feels like the correct place based on MDM policies applying to the entire machine (not just the current user). Since the settings pertain to GCM, the settings should logically exist under a GCM namespace in the registry. Applications should place their root key under the |
| #if NETFRAMEWORK | ||
| // Check for machine (HKLM) registry keys that match the Git configuration name. | ||
| // These can be set by system administrators via Group Policy, so make useful defaults. | ||
| using (Win32.RegistryKey configKey = Win32.Registry.LocalMachine.OpenSubKey(Constants.WindowsRegistry.HKConfigurationPath)) |
There was a problem hiding this comment.
Why is this limited to NETFRAMEWORK projects? It looks like both RegistryKey and Registry exist for .NET 5.
There was a problem hiding this comment.
Although the Registry* types exist in .NET 5 (/.NET Core), they exist in a separate assembly/NuGet package that must be referenced to use. For the .NET Framework target the Registry* types are included by default in the framework references.
Since this feature only pertains to Windows, and we only release on .NET Framework for Windows there's no need to bring the extra assembly along for the ride in all the other targets (Mac and Linux).
If/once we move to .NET 5 (or 6) on Windows then there'll need to be a review of all #if NETFRAMEWORK code, at which point we should hopefully have better support from the linker/trimmer to keep distribution bloat to a minimum on other platforms.
There was a problem hiding this comment.
I mostly have comments on the docs. The existing discussion of config settings could be reorganized to make the new landscape a bit clearer.
I built this branch locally and confirmed that I was able to load the config setting from a registry key. Excellent work!
Clarify that the new registry settings are aimed at enterprises and system administrators, _and_ that the settings are at the lowest precedence; they can always be overridden by the user.
|
@derrickstolee & @jeffhostetler I've made some edits to the documentation and naming of the feature to stress the audience and scope: "enterprise system administrator defaults". |
|
Is there a feature in GCM to list the various settings and where they came from (something like The Admin could use to ensure that they got everything the way they wanted (especially WRT the 32/64 registry hive stuff). And later, the Admin could ask individual users to run it and see what they're getting whenever there's a problem (or maybe we include it in Just asking. It doesn't have to be a part of this. |
That might be a good idea actually. I can throw together an issue tracking this idea (#407). |
Dynamically skip the macOS Keychain read/write/delete test if the Keychain is in a "strange" state. There is an unknown issue that the keychain can sometimes get itself in where all API calls result in an errSecAuthFailed error. The only solution seems to be a machine restart; not possible in CI! The problem has plagued others who are calling the same Keychain APIs from C# such as the MSAL.NET team - they don't know either. It might have something to do with the code signing signature of the binary (our collective best theory). It's probably only diagnosable at this point by Apple, but we don't have a reliable way to reproduce the problem.
Introduce the concept of another layer of settings (lowest precedence; default values only) below the existing environment variable and Git configuration file mechanisms.
Implement on Windows using the Registry, under key:
HKLM\SOFTWARE\GitCredentialManager\ConfigurationThe primary target for this feature are enterprise system administrators who wish to set defaults for GCM via MDM technologies such as Group Policy on Windows.
Note: We believe the user should always be at liberty to configure Git and GCM exactly as they wish. By preferring environment variables and Git configuration files over OS store values, these only act as default values that can always be overridden by the user in the usual ways.