From 977a52eef591ec80ea890193fd1c451edccf27c6 Mon Sep 17 00:00:00 2001 From: Alberto Spelta Date: Mon, 13 Jul 2026 14:24:53 +0200 Subject: [PATCH] Refactor policy handling into a dedicated module --- src/Infrastructure/AppInstance.cs | 4 +- src/Infrastructure/AppWindow.cs | 7 +- .../Settings/ExperimentalSettings.cs | 10 - .../Configuration/Settings/UserSettings.cs | 55 +--- .../Extensions/RegistryExtensions.cs | 6 - src/Infrastructure/Policies/IPolicySource.cs | 48 ++++ src/Infrastructure/Policies/Policies.cs | 29 ++ .../Policies/PoliciesFactory.cs | 47 ++++ .../Policies/RegistryPolicySource.cs | 19 ++ .../Policies/ServiceCollectionExtensions.cs | 14 + .../ADMX => Policies/Templates}/bravo.admx | 0 .../Templates}/en-US/bravo.adml | 0 .../Templates}/en-US/sqlbi.adml | 0 .../ADMX => Policies/Templates}/sqlbi.admx | 0 .../CloudAuthenticationClient.cs | 15 +- .../PowerBI/ServiceCollectionExtensions.cs | 2 +- .../Security/Policies/PolicyManager.cs | 154 ----------- .../Security/Policies/PolicyStatus.cs | 15 - .../DaxTemplate/DaxTemplateManager.cs | 15 +- .../Telemetry/TelemetryService.cs | 91 +++--- src/Models/BravoPolicies.cs | 113 -------- src/Program.cs | 3 +- src/Scripts/@types/global.d.ts | 5 +- src/Scripts/controllers/app.ts | 8 +- src/Scripts/controllers/options.ts | 68 +++-- src/Scripts/controllers/telemetry.ts | 4 +- src/Scripts/view/options-dialog-about.ts | 6 +- src/Scripts/view/options-dialog-dev.ts | 1 + .../view/scene-manage-dates-calendar.ts | 2 +- src/Scripts/view/scene-manage-dates.ts | 5 +- src/Services/ManageDatesService.cs | 8 +- src/Services/TemplateDevelopmentService.cs | 48 ++-- src/Startup.cs | 8 +- test/Bravo.Tests/GlobalUsings.cs | 13 + .../Policies/PoliciesFactoryTests.cs | 260 ++++++++++++++++++ .../Policies/RegistryPolicySourceTests.cs | 89 ++++++ 36 files changed, 686 insertions(+), 486 deletions(-) delete mode 100644 src/Infrastructure/Configuration/Settings/ExperimentalSettings.cs create mode 100644 src/Infrastructure/Policies/IPolicySource.cs create mode 100644 src/Infrastructure/Policies/Policies.cs create mode 100644 src/Infrastructure/Policies/PoliciesFactory.cs create mode 100644 src/Infrastructure/Policies/RegistryPolicySource.cs create mode 100644 src/Infrastructure/Policies/ServiceCollectionExtensions.cs rename src/Infrastructure/{Security/Policies/ADMX => Policies/Templates}/bravo.admx (100%) rename src/Infrastructure/{Security/Policies/ADMX => Policies/Templates}/en-US/bravo.adml (100%) rename src/Infrastructure/{Security/Policies/ADMX => Policies/Templates}/en-US/sqlbi.adml (100%) rename src/Infrastructure/{Security/Policies/ADMX => Policies/Templates}/sqlbi.admx (100%) delete mode 100644 src/Infrastructure/Security/Policies/PolicyManager.cs delete mode 100644 src/Infrastructure/Security/Policies/PolicyStatus.cs delete mode 100644 src/Models/BravoPolicies.cs create mode 100644 test/Bravo.Tests/GlobalUsings.cs create mode 100644 test/Bravo.Tests/Infrastructure/Policies/PoliciesFactoryTests.cs create mode 100644 test/Bravo.Tests/Infrastructure/Policies/RegistryPolicySourceTests.cs diff --git a/src/Infrastructure/AppInstance.cs b/src/Infrastructure/AppInstance.cs index e94f2e32..e6bf0259 100644 --- a/src/Infrastructure/AppInstance.cs +++ b/src/Infrastructure/AppInstance.cs @@ -73,7 +73,7 @@ public void NotifyOwner() catch (Exception ex) when (ex is IOException || ex is TimeoutException) { ExceptionHelper.WriteToEventLog(ex, EventLogEntryType.Warning); - TelemetryService.TrackFatalException(ex); + TelemetryService.Instance.TrackException(ex); return; } @@ -90,7 +90,7 @@ public void NotifyOwner() catch (Exception ex) when (ex is ObjectDisposedException || ex is InvalidOperationException || ex is IOException) { ExceptionHelper.WriteToEventLog(ex, EventLogEntryType.Warning); - TelemetryService.TrackFatalException(ex); + TelemetryService.Instance.TrackException(ex); return; } } diff --git a/src/Infrastructure/AppWindow.cs b/src/Infrastructure/AppWindow.cs index 2358ac8b..f605e683 100644 --- a/src/Infrastructure/AppWindow.cs +++ b/src/Infrastructure/AppWindow.cs @@ -10,6 +10,7 @@ using Sqlbi.Bravo.Infrastructure.Extensions; using Sqlbi.Bravo.Infrastructure.Helpers; using Sqlbi.Bravo.Infrastructure.Messages; + using Sqlbi.Bravo.Infrastructure.Policies; using Sqlbi.Bravo.Infrastructure.Services; using Sqlbi.Bravo.Infrastructure.Telemetry; using Sqlbi.Bravo.Infrastructure.Windows.Interop; @@ -37,12 +38,14 @@ internal partial class AppWindow : Form private readonly IOptions _startupSettingsOptionsAccessor; private readonly WebView2ProxyAuthHandler _proxyAuthHandler; private readonly Color _startupThemeColor; + private readonly IPolicies _policies; public AppWindow(IServiceProvider services, AppInstance instance) { _instance = instance; _serverAddressProvider = services.GetRequiredService(); _startupSettingsOptionsAccessor = services.GetRequiredService>(); + _policies = services.GetRequiredService(); _proxyAuthHandler = new WebView2ProxyAuthHandler(WebProxyWrapper.Current); _startupThemeColor = ThemeHelper.ShouldUseDarkMode(UserPreferences.Current.Theme) ? AppEnvironment.ThemeColorDark : AppEnvironment.ThemeColorLight; @@ -307,7 +310,7 @@ private MemoryStream GetConfigJs() token = AppEnvironment.ApiAuthenticationToken, version = AppEnvironment.VersionInfo.Version, options = BravoOptions.CreateFromUserPreferences(), - policies = BravoPolicies.Current, + policies = _policies, culture = new { ietfLanguageTag = CultureInfo.CurrentCulture.IetfLanguageTag, @@ -323,7 +326,7 @@ private MemoryStream GetConfigJs() }, }; - var script = $@"var CONFIG = { JsonSerializer.Serialize(config) };"; + var script = $@"var CONFIG = { JsonSerializer.Serialize(config, options: new JsonSerializerOptions(JsonSerializerDefaults.Web)) };"; return new MemoryStream(Encoding.UTF8.GetBytes(script)); } diff --git a/src/Infrastructure/Configuration/Settings/ExperimentalSettings.cs b/src/Infrastructure/Configuration/Settings/ExperimentalSettings.cs deleted file mode 100644 index d3c19e36..00000000 --- a/src/Infrastructure/Configuration/Settings/ExperimentalSettings.cs +++ /dev/null @@ -1,10 +0,0 @@ -namespace Sqlbi.Bravo.Infrastructure.Configuration.Settings -{ - using System.Text.Json.Serialization; - - public class ExperimentalSettings - { - //[JsonPropertyName("useIntegratedWindowsAuthenticationSso")] - //public bool? UseIntegratedWindowsAuthenticationSso { get; } - } -} diff --git a/src/Infrastructure/Configuration/Settings/UserSettings.cs b/src/Infrastructure/Configuration/Settings/UserSettings.cs index 944173c9..06a1dcaa 100644 --- a/src/Infrastructure/Configuration/Settings/UserSettings.cs +++ b/src/Infrastructure/Configuration/Settings/UserSettings.cs @@ -1,7 +1,5 @@ namespace Sqlbi.Bravo.Infrastructure.Configuration.Settings { - using Sqlbi.Bravo.Infrastructure.Security.Policies; - using Sqlbi.Bravo.Models; using System.Text.Json; using System.Text.Json.Serialization; @@ -36,35 +34,17 @@ public class UserSettings : IUserSettings public const bool DefaultUseSystemBrowserForAuthentication = false; public const bool DefaultCustomTemplatesEnabled = true; - private bool _telemetryEnabled = DefaultTelemetryEnabled; - private UpdateChannelType _updateChannel = DefaultUpdateChannel; - private bool _updateCheckEnabled = DefaultUpdateCheckEnabled; - private bool _useSystemBrowserForAuthentication = DefaultUseSystemBrowserForAuthentication; - private bool _customTemplatesEnabled = DefaultCustomTemplatesEnabled; - [JsonPropertyName("telemetryEnabled")] - public bool TelemetryEnabled - { - get => _telemetryEnabled; - set => _telemetryEnabled = GetSetterValue(value, BravoPolicies.Current.TelemetryEnabledPolicy, BravoPolicies.Current.TelemetryEnabled); - } + public bool TelemetryEnabled { get; set; } = DefaultTelemetryEnabled; [JsonPropertyName("diagnosticLevel")] public DiagnosticLevelType DiagnosticLevel { get; set; } = DefaultDiagnosticLevel; [JsonPropertyName("updateChannel")] - public UpdateChannelType UpdateChannel - { - get => _updateChannel; - set => _updateChannel = GetSetterValue(value, BravoPolicies.Current.UpdateChannelPolicy, BravoPolicies.Current.UpdateChannel); - } + public UpdateChannelType UpdateChannel { get; set; } = DefaultUpdateChannel; [JsonPropertyName("updateCheckEnabled")] - public bool UpdateCheckEnabled - { - get => _updateCheckEnabled; - set => _updateCheckEnabled = GetSetterValue(value, BravoPolicies.Current.UpdateCheckEnabledPolicy, BravoPolicies.Current.UpdateCheckEnabled); - } + public bool UpdateCheckEnabled { get; set; } = DefaultUpdateCheckEnabled; [JsonPropertyName("theme")] public ThemeType Theme { get; set; } = DefaultTheme; @@ -73,38 +53,13 @@ public bool UpdateCheckEnabled public ProxySettings? Proxy { get; set; } [JsonPropertyName("useSystemBrowserForAuthentication")] - public bool UseSystemBrowserForAuthentication - { - get => _useSystemBrowserForAuthentication; - set => _useSystemBrowserForAuthentication = GetSetterValue(value, BravoPolicies.Current.UseSystemBrowserForAuthenticationPolicy, BravoPolicies.Current.UseSystemBrowserForAuthentication); - } + public bool UseSystemBrowserForAuthentication { get; set; } = DefaultUseSystemBrowserForAuthentication; [JsonPropertyName("customTemplatesEnabled")] - public bool CustomTemplatesEnabled - { - get => _customTemplatesEnabled; - set => _customTemplatesEnabled = GetSetterValue(value, BravoPolicies.Current.CustomTemplatesEnabledPolicy, BravoPolicies.Current.CustomTemplatesEnabled); - } + public bool CustomTemplatesEnabled { get; set; } = DefaultCustomTemplatesEnabled; [JsonPropertyName("customOptions")] public JsonElement? CustomOptions { get; set; } - - //[JsonPropertyName("experimental")] - //public ExperimentalSettings? Experimental { get; set; } - - private T GetSetterValue(T setterValue, PolicyStatus policyStatus, T policyValue) - { - if (policyStatus == PolicyStatus.Forced) - { - return policyValue; - } - else if (policyStatus == PolicyStatus.NotConfigured) - { - return setterValue; - } - - throw new BravoUnexpectedException($"Unexpected { nameof(PolicyStatus) } value ({ policyStatus })"); - } } public enum ThemeType diff --git a/src/Infrastructure/Extensions/RegistryExtensions.cs b/src/Infrastructure/Extensions/RegistryExtensions.cs index 52525529..533c1f85 100644 --- a/src/Infrastructure/Extensions/RegistryExtensions.cs +++ b/src/Infrastructure/Extensions/RegistryExtensions.cs @@ -4,12 +4,6 @@ internal static class RegistryExtensions { - public static bool SubKeyExists(this RegistryKey registryKey, string subkeyName) - { - using var registrySubKey = registryKey.OpenSubKey(subkeyName); - return registrySubKey != null; - } - public static bool GetBoolValue(this RegistryKey registryKey, string subkeyName, string valueName) { var valueInt = GetIntValue(registryKey, subkeyName, valueName); diff --git a/src/Infrastructure/Policies/IPolicySource.cs b/src/Infrastructure/Policies/IPolicySource.cs new file mode 100644 index 00000000..7464d793 --- /dev/null +++ b/src/Infrastructure/Policies/IPolicySource.cs @@ -0,0 +1,48 @@ +namespace Sqlbi.Bravo.Infrastructure.Policies +{ + using Microsoft.Win32; + + /// + /// Abstraction over a single raw policy value store (e.g. a registry key), so that + /// ' parsing/precedence logic does not depend on + /// directly and can be unit tested against a fake, without touching the real registry. + /// + internal interface IPolicySource + { + int? GetInt(string name); + string? GetString(string name); + } + + /// + /// Typed reading conventions shared by every : a policy is a + /// DWORD (0/1 -> bool, or a defined enum member) or a string. Kept as extensions rather than + /// interface members so itself stays minimal (raw int/string only). + /// + internal static class PolicySourceExtensions + { + private const int PolicyDisabledValue = 0; + private const int PolicyEnabledValue = 1; + + extension(IPolicySource source) + { + public bool? GetBool(string name) + { + return source.GetInt(name) switch + { + null => null, // Policy not set + PolicyDisabledValue => false, + PolicyEnabledValue => true, + _ => null, // Invalid policy value + }; + } + + public T? GetEnum(string name) where T : struct, Enum + { + if (source.GetInt(name) is { } value && Enum.IsDefined(typeof(T), value)) + return (T)(object)value; + + return null; // Policy not set or invalid + } + } + } +} diff --git a/src/Infrastructure/Policies/Policies.cs b/src/Infrastructure/Policies/Policies.cs new file mode 100644 index 00000000..9201b321 --- /dev/null +++ b/src/Infrastructure/Policies/Policies.cs @@ -0,0 +1,29 @@ +namespace Sqlbi.Bravo.Infrastructure.Policies +{ + using Sqlbi.Bravo.Infrastructure.Configuration.Settings; + + internal interface IPolicies + { + bool? TelemetryEnabled { get; } + UpdateChannelType? UpdateChannel { get; } + bool? UpdateCheckEnabled { get; } + bool? UseSystemBrowserForAuthentication { get; } + bool? BuiltInTemplatesEnabled { get; } + bool? CustomTemplatesEnabled { get; } + string? CustomTemplatesOrganizationRepositoryPath { get; } + } + + /// + /// Immutable snapshot of Bravo's effective policy values. Pure data - see + /// for how instances are read from the registry, parsed, + /// and merged with LocalMachine/CurrentUser precedence. + /// + internal sealed record Policies( + bool? TelemetryEnabled, + UpdateChannelType? UpdateChannel, + bool? UpdateCheckEnabled, + bool? UseSystemBrowserForAuthentication, + bool? BuiltInTemplatesEnabled, + bool? CustomTemplatesEnabled, + string? CustomTemplatesOrganizationRepositoryPath) : IPolicies; +} diff --git a/src/Infrastructure/Policies/PoliciesFactory.cs b/src/Infrastructure/Policies/PoliciesFactory.cs new file mode 100644 index 00000000..0bf6344d --- /dev/null +++ b/src/Infrastructure/Policies/PoliciesFactory.cs @@ -0,0 +1,47 @@ +namespace Sqlbi.Bravo.Infrastructure.Policies +{ + using Microsoft.Win32; + using Sqlbi.Bravo.Infrastructure.Configuration.Settings; + + /// + /// Builds instances: parses a single , + /// and composes the effective policy set from LocalMachine + CurrentUser with precedence. + /// + internal static class PoliciesFactory + { + private const string OptionSettingsSubKeyName = @"SOFTWARE\Policies\SQLBI\Bravo\OptionSettings"; + + public static Policies Create() + { + using var machineKey = Registry.LocalMachine.OpenSubKey(OptionSettingsSubKeyName); + var machinePolicies = FromSource(new RegistryPolicySource(machineKey)); + + using var userKey = Registry.CurrentUser.OpenSubKey(OptionSettingsSubKeyName); + var userPolicies = FromSource(new RegistryPolicySource(userKey)); + + return Merge(machinePolicies, userPolicies); + } + + internal static Policies FromSource(IPolicySource source) => new( + TelemetryEnabled: source.GetBool("TelemetryEnabled"), + UpdateChannel: source.GetEnum("UpdateChannel"), + UpdateCheckEnabled: source.GetBool("UpdateCheckEnabled"), + UseSystemBrowserForAuthentication: source.GetBool("UseSystemBrowserForAuthentication"), + BuiltInTemplatesEnabled: source.GetBool("BuiltInTemplatesEnabled"), + CustomTemplatesEnabled: source.GetBool("CustomTemplatesEnabled"), + CustomTemplatesOrganizationRepositoryPath: source.GetString("CustomTemplatesOrganizationRepositoryPath")); + + internal static Policies Merge(Policies machinePolicies, Policies userPolicies) + { + // LocalMachine takes precedence over CurrentUser when both are configured + return new Policies( + TelemetryEnabled: machinePolicies.TelemetryEnabled ?? userPolicies.TelemetryEnabled, + UpdateChannel: machinePolicies.UpdateChannel ?? userPolicies.UpdateChannel, + UpdateCheckEnabled: machinePolicies.UpdateCheckEnabled ?? userPolicies.UpdateCheckEnabled, + UseSystemBrowserForAuthentication: machinePolicies.UseSystemBrowserForAuthentication ?? userPolicies.UseSystemBrowserForAuthentication, + BuiltInTemplatesEnabled: machinePolicies.BuiltInTemplatesEnabled ?? userPolicies.BuiltInTemplatesEnabled, + CustomTemplatesEnabled: machinePolicies.CustomTemplatesEnabled ?? userPolicies.CustomTemplatesEnabled, + CustomTemplatesOrganizationRepositoryPath: machinePolicies.CustomTemplatesOrganizationRepositoryPath ?? userPolicies.CustomTemplatesOrganizationRepositoryPath); + } + } +} diff --git a/src/Infrastructure/Policies/RegistryPolicySource.cs b/src/Infrastructure/Policies/RegistryPolicySource.cs new file mode 100644 index 00000000..8770f502 --- /dev/null +++ b/src/Infrastructure/Policies/RegistryPolicySource.cs @@ -0,0 +1,19 @@ +namespace Sqlbi.Bravo.Infrastructure.Policies +{ + using Microsoft.Win32; + + /// + /// Adapter that bridges to a real . + /// Intentionally a thin pass-through with no logic of its own. + /// + internal sealed class RegistryPolicySource(RegistryKey? key) : IPolicySource + { + private readonly RegistryKey? _key = key; + + public int? GetInt(string name) + => _key?.GetValue(name) is int value ? value : null; + + public string? GetString(string name) + => _key?.GetValue(name) as string; + } +} diff --git a/src/Infrastructure/Policies/ServiceCollectionExtensions.cs b/src/Infrastructure/Policies/ServiceCollectionExtensions.cs new file mode 100644 index 00000000..469ac404 --- /dev/null +++ b/src/Infrastructure/Policies/ServiceCollectionExtensions.cs @@ -0,0 +1,14 @@ +namespace Sqlbi.Bravo.Infrastructure.Policies +{ + using Microsoft.Extensions.DependencyInjection; + + internal static class ServiceCollectionExtensions + { + public static IServiceCollection AddGroupPolicies(this IServiceCollection services) + { + services.AddSingleton(_ => PoliciesFactory.Create()); + + return services; + } + } +} diff --git a/src/Infrastructure/Security/Policies/ADMX/bravo.admx b/src/Infrastructure/Policies/Templates/bravo.admx similarity index 100% rename from src/Infrastructure/Security/Policies/ADMX/bravo.admx rename to src/Infrastructure/Policies/Templates/bravo.admx diff --git a/src/Infrastructure/Security/Policies/ADMX/en-US/bravo.adml b/src/Infrastructure/Policies/Templates/en-US/bravo.adml similarity index 100% rename from src/Infrastructure/Security/Policies/ADMX/en-US/bravo.adml rename to src/Infrastructure/Policies/Templates/en-US/bravo.adml diff --git a/src/Infrastructure/Security/Policies/ADMX/en-US/sqlbi.adml b/src/Infrastructure/Policies/Templates/en-US/sqlbi.adml similarity index 100% rename from src/Infrastructure/Security/Policies/ADMX/en-US/sqlbi.adml rename to src/Infrastructure/Policies/Templates/en-US/sqlbi.adml diff --git a/src/Infrastructure/Security/Policies/ADMX/sqlbi.admx b/src/Infrastructure/Policies/Templates/sqlbi.admx similarity index 100% rename from src/Infrastructure/Security/Policies/ADMX/sqlbi.admx rename to src/Infrastructure/Policies/Templates/sqlbi.admx diff --git a/src/Infrastructure/PowerBI/Cloud/Authentication/CloudAuthenticationClient.cs b/src/Infrastructure/PowerBI/Cloud/Authentication/CloudAuthenticationClient.cs index 385ed257..d0b07ec3 100644 --- a/src/Infrastructure/PowerBI/Cloud/Authentication/CloudAuthenticationClient.cs +++ b/src/Infrastructure/PowerBI/Cloud/Authentication/CloudAuthenticationClient.cs @@ -5,6 +5,7 @@ namespace Sqlbi.Bravo.Infrastructure.PowerBI.Cloud.Authentication using Sqlbi.Bravo.Infrastructure.Configuration; using Sqlbi.Bravo.Infrastructure.Extensions; using Sqlbi.Bravo.Infrastructure.Helpers; + using Sqlbi.Bravo.Infrastructure.Policies; using Sqlbi.Bravo.Infrastructure.PowerBI.Cloud; using Msal = Microsoft.Identity.Client; @@ -18,11 +19,13 @@ public interface ICloudAuthenticationClient /// /// Handles authentication with Microsoft Entra ID (Azure AD) using MSAL.NET, including token acquisition and cache management. /// - internal sealed class CloudAuthenticationClient : ICloudAuthenticationClient + internal sealed class CloudAuthenticationClient(IPolicies policies) : ICloudAuthenticationClient { private const string SystemBrowserRedirectUri = "http://localhost"; private const string OrganizationalAccountsOnlyQueryParameter = "msafed=0"; // no Microsoft accounts (MSA) allowed + private readonly IPolicies _policies = policies; + public async Task AcquireTokenAsync( CloudEnvironment environment, string email, CancellationToken cancellationToken) { @@ -68,10 +71,11 @@ public async Task ClearTokenCacheAsync(CloudEnvironment environment) return await builder.ExecuteAsync(cancellationToken).ConfigureAwait(false); } - private static async Task AcquireTokenInteractiveAsync( + private async Task AcquireTokenInteractiveAsync( IPublicClientApplication client, string[] scopes, string email, string claims, CancellationToken cancellationToken) { - var useEmbeddedBrowser = !UserPreferences.Current.UseSystemBrowserForAuthentication; + var useSystemBrowser = _policies.UseSystemBrowserForAuthentication ?? UserPreferences.Current.UseSystemBrowserForAuthentication; + var useEmbeddedBrowser = !useSystemBrowser; var extraQueryParameters = OrganizationalAccountsOnlyQueryParameter; var prompt = Prompt.SelectAccount; var loginHint = email; @@ -116,9 +120,10 @@ public async Task ClearTokenCacheAsync(CloudEnvironment environment) } } - private static IPublicClientApplication CreatePublicClient(CloudEnvironment environment) + private IPublicClientApplication CreatePublicClient(CloudEnvironment environment) { - var useEmbeddedBrowser = !UserPreferences.Current.UseSystemBrowserForAuthentication; + var useSystemBrowser = _policies.UseSystemBrowserForAuthentication ?? UserPreferences.Current.UseSystemBrowserForAuthentication; + var useEmbeddedBrowser = !useSystemBrowser; var redirectUri = (useEmbeddedBrowser ? environment.RedirectUri : SystemBrowserRedirectUri); var builder = PublicClientApplicationBuilder.Create(environment.ClientId) diff --git a/src/Infrastructure/PowerBI/ServiceCollectionExtensions.cs b/src/Infrastructure/PowerBI/ServiceCollectionExtensions.cs index 373b998b..113ef84d 100644 --- a/src/Infrastructure/PowerBI/ServiceCollectionExtensions.cs +++ b/src/Infrastructure/PowerBI/ServiceCollectionExtensions.cs @@ -9,7 +9,7 @@ internal static class ServiceCollectionExtensions { internal const string PowerBIApiHttpClientName = "PowerBIApi"; - public static IServiceCollection AddPowerBIServices(this IServiceCollection services) + public static IServiceCollection AddPowerBI(this IServiceCollection services) { services.AddHttpClient(PowerBIApiHttpClientName, (client) => { diff --git a/src/Infrastructure/Security/Policies/PolicyManager.cs b/src/Infrastructure/Security/Policies/PolicyManager.cs deleted file mode 100644 index 3c1f904c..00000000 --- a/src/Infrastructure/Security/Policies/PolicyManager.cs +++ /dev/null @@ -1,154 +0,0 @@ -namespace Sqlbi.Bravo.Infrastructure.Security.Policies -{ - using Microsoft.Win32; - using Sqlbi.Bravo.Infrastructure.Configuration.Settings; - using Sqlbi.Bravo.Infrastructure.Extensions; - using System.IO; - - internal sealed class PolicyManager - { - private const string PolicySubKeyName = @"Software\Policies\SQLBI\Bravo"; - private const string OptionSettingsName = "OptionSettings"; - private const int PolicyDisabledValue = 0; - private const int PolicyEnabledValue = 1; - - public bool PoliciesEnabled => GetPoliciesEnabled(); - - public (PolicyStatus Policy, bool Value) GetTelemetryEnabledPolicy() => GetBoolPolicy(valueName: "TelemetryEnabled", relativeSubkeyName: OptionSettingsName); - - public (PolicyStatus Policy, bool Value) GetUseSystemBrowserForAuthenticationPolicy() => GetBoolPolicy(valueName: "UseSystemBrowserForAuthentication", relativeSubkeyName: OptionSettingsName); - - public (PolicyStatus Policy, UpdateChannelType Value) GetUpdateChannelPolicy() - { - var policyValue = GetIntValue(valueName: "UpdateChannel", relativeSubkeyName: OptionSettingsName); - - if (IsPolicyNotConfigured(policyValue)) - { - return (PolicyStatus.NotConfigured, Value: UpdateChannelType.Stable); - } - else - { - var updateChannel = policyValue.TryParseTo(); - - if (updateChannel is null) - updateChannel = UpdateChannelType.Stable; - - return (PolicyStatus.Forced, updateChannel.Value); - } - } - - public (PolicyStatus Policy, bool Value) GetUpdateCheckEnabledPolicy() => GetBoolPolicy(valueName: "UpdateCheckEnabled", relativeSubkeyName: OptionSettingsName); - - public (PolicyStatus Policy, bool Value) GetBuiltInTemplatesEnabledPolicy() => GetBoolPolicy(valueName: "BuiltInTemplatesEnabled", relativeSubkeyName: OptionSettingsName); - - public (PolicyStatus Policy, bool Value) GetCustomTemplatesEnabledPolicy() => GetBoolPolicy(valueName: "CustomTemplatesEnabled", relativeSubkeyName: OptionSettingsName); - - public (PolicyStatus Policy, string? Value) GetCustomTemplatesOrganizationRepositoryPathPolicy() => GetStringPolicy(valueName: "CustomTemplatesOrganizationRepositoryPath", relativeSubkeyName: OptionSettingsName); - - private static (PolicyStatus Policy, bool Value) GetBoolPolicy(string valueName, string relativeSubkeyName) - { - var policyValue = GetIntValue(valueName, relativeSubkeyName); - - if (IsPolicyNotConfigured(policyValue)) - { - return (PolicyStatus.NotConfigured, Value: true); - } - else if (IsPolicyEnabled(policyValue)) - { - return (PolicyStatus.Forced, Value: true); - } - else if (IsPolicyDisabled(policyValue)) - { - return (PolicyStatus.Forced, Value: false); - } - else - { - throw new BravoUnexpectedException($"Unexpected {nameof(PolicyStatus)} value ({policyValue})"); - } - } - - private static (PolicyStatus Policy, string? Value) GetStringPolicy(string valueName, string relativeSubkeyName) - { - var policyValue = GetStringValue(valueName, relativeSubkeyName); - - if (IsPolicyNotConfigured(policyValue)) - { - return (PolicyStatus.NotConfigured, Value: null); - } - else - { - return (PolicyStatus.Forced, Value: policyValue); - } - } - - private static string? GetStringValue(string valueName, string relativeSubkeyName) - { - var subkeyName = Path.Combine(PolicySubKeyName, relativeSubkeyName); - var value = Registry.LocalMachine.GetStringValue(subkeyName, valueName); - - if (value is null) - value = Registry.CurrentUser.GetStringValue(subkeyName, valueName); - - return value; - } - - private static int? GetIntValue(string valueName, string relativeSubkeyName) - { - var subkeyName = Path.Combine(PolicySubKeyName, relativeSubkeyName); - var value = Registry.LocalMachine.GetIntValue(subkeyName, valueName); - - if (value is null) - value = Registry.CurrentUser.GetIntValue(subkeyName, valueName); - - return value; - } - - private static bool GetPoliciesEnabled() - { - var enabled = Registry.LocalMachine.SubKeyExists(PolicySubKeyName); - - if (enabled == false) - enabled = Registry.CurrentUser.SubKeyExists(PolicySubKeyName); - - return enabled; - } - - private static bool IsPolicyEnabled(int? policyValue) - { - if (policyValue is null) - return false; - - if (policyValue == PolicyEnabledValue) - return true; - - return false; - } - - private static bool IsPolicyDisabled(int? policyValue) - { - if (policyValue is null) - return false; - - if (policyValue == PolicyDisabledValue) - return true; - - return false; - } - - private static bool IsPolicyNotConfigured(int? policyValue) - { - if (policyValue is null) - return true; - - return false; - } - - private static bool IsPolicyNotConfigured(string? policyValue) - { - if (policyValue is null) - return true; - - return false; - } - } -} diff --git a/src/Infrastructure/Security/Policies/PolicyStatus.cs b/src/Infrastructure/Security/Policies/PolicyStatus.cs deleted file mode 100644 index ff62d63b..00000000 --- a/src/Infrastructure/Security/Policies/PolicyStatus.cs +++ /dev/null @@ -1,15 +0,0 @@ -namespace Sqlbi.Bravo.Infrastructure.Security.Policies -{ - public enum PolicyStatus - { - /// - /// No policy has been enforced - /// - NotConfigured = 0, - - /// - /// A policy has been applied for the property scope - /// - Forced = 1, - } -} diff --git a/src/Infrastructure/Services/DaxTemplate/DaxTemplateManager.cs b/src/Infrastructure/Services/DaxTemplate/DaxTemplateManager.cs index 43e00a01..03a8bf57 100644 --- a/src/Infrastructure/Services/DaxTemplate/DaxTemplateManager.cs +++ b/src/Infrastructure/Services/DaxTemplate/DaxTemplateManager.cs @@ -2,9 +2,9 @@ { using Dax.Template; using Dax.Template.Model; + using Sqlbi.Bravo.Infrastructure; using Sqlbi.Bravo.Infrastructure.Extensions; - using Sqlbi.Bravo.Infrastructure.Security.Policies; - using Sqlbi.Bravo.Models; + using Sqlbi.Bravo.Infrastructure.Policies; using Sqlbi.Bravo.Models.ManageDates; internal class DaxTemplateManager @@ -27,9 +27,11 @@ internal class DaxTemplateManager internal static readonly string UserPath = Path.Combine(AppEnvironment.ApplicationDataPath, @"ManageDates\Templates"); private readonly object _cacheSyncLock = new(); + private readonly IPolicies _policies; - public DaxTemplateManager() + public DaxTemplateManager(IPolicies policies) { + _policies = policies; InitializeCache(); } @@ -40,12 +42,9 @@ public Package GetPackage(string path) public IEnumerable GetPackages() { - if (BravoPolicies.Current.BuiltInTemplatesEnabledPolicy == PolicyStatus.Forced) + if (_policies.BuiltInTemplatesEnabled is false) { - if (BravoPolicies.Current.BuiltInTemplatesEnabled == false) - { - return Array.Empty(); - } + return Array.Empty(); } var files = Package.FindTemplateFiles(CachePath); diff --git a/src/Infrastructure/Telemetry/TelemetryService.cs b/src/Infrastructure/Telemetry/TelemetryService.cs index dbe075b1..26bd7923 100644 --- a/src/Infrastructure/Telemetry/TelemetryService.cs +++ b/src/Infrastructure/Telemetry/TelemetryService.cs @@ -1,8 +1,9 @@ -namespace Sqlbi.Bravo.Infrastructure.Telemetry; +namespace Sqlbi.Bravo.Infrastructure.Telemetry; using Microsoft.ApplicationInsights; using Microsoft.ApplicationInsights.Extensibility; using Sqlbi.Bravo.Infrastructure.Configuration; +using Sqlbi.Bravo.Infrastructure.Policies; public interface ITelemetryService : IDisposable { @@ -14,17 +15,52 @@ internal sealed class TelemetryService : ITelemetryService { private readonly TelemetryConfiguration _configuration; private readonly TelemetryClient _client; + private readonly IPolicies _policies; - public TelemetryService() + private static readonly Lazy _instance = new(CreateInstance, isThreadSafe: true); + + public static TelemetryService Instance => _instance.Value; + + private static TelemetryService CreateInstance() + { + // Use parameterless constructor to avoid default processors (e.g. adaptive sampling) + // that TelemetryConfiguration.CreateDefault() would register. + var configuration = new TelemetryConfiguration(); + + // Remark: ensure no sampling is applied — every exception must be recorded + var processorChain = configuration.DefaultTelemetrySink.TelemetryProcessorChainBuilder; + processorChain.Use((next) => new DefaultTelemetryProcessor(next)); + processorChain.Build(); + + configuration.TelemetryInitializers.Add(new DefaultTelemetryInitializer()); + configuration.ConnectionString = TelemetrySessionInfo.ConnectionString; + + var policies = PoliciesFactory.Create(); + + // Determine whether telemetry is enabled based on policies and user settings. + var telemetryEnabled = policies.TelemetryEnabled ?? UserPreferences.Current.TelemetryEnabled; + configuration.DisableTelemetry = !telemetryEnabled; +#if DEBUG + configuration.TelemetryChannel.DeveloperMode = Debugger.IsAttached; +#endif + return new TelemetryService(configuration, policies); + } + + private TelemetryService(TelemetryConfiguration configuration, IPolicies policies) { - _configuration = CreateConfiguration(); - _client = new TelemetryClient(_configuration); + _configuration = configuration; + _client = new TelemetryClient(configuration); + _policies = policies; } public bool TelemetryEnabled { - get => _configuration.DisableTelemetry == false; - set => _configuration.DisableTelemetry = !value; + get => !_configuration.DisableTelemetry; + set + { + var telemetryEnabled = _policies.TelemetryEnabled ?? value; + _configuration.DisableTelemetry = !telemetryEnabled; + } } public void TrackException(Exception exception) @@ -33,52 +69,11 @@ public void TrackException(Exception exception) exception = aex.GetBaseException(); _client.TrackException(exception); + _client.Flush(); } public void Dispose() { - _client.Flush(); _configuration.Dispose(); } - - /// - /// Tracks a fatal exception in scenarios where the DI container is unavailable. - /// - public static void TrackFatalException(Exception exception) - { - if (exception is AggregateException aex) - exception = aex.GetBaseException(); - - try - { - using var configuration = CreateConfiguration(); - var client = new TelemetryClient(configuration); - client.TrackException(exception); - client.Flush(); // Blocking flush is acceptable — app is terminating - } - catch - { - // Telemetry failure must not mask the original exception - } - } - - private static TelemetryConfiguration CreateConfiguration() - { - // Use parameterless constructor to avoid default processors (e.g. adaptive sampling) - // that TelemetryConfiguration.CreateDefault() would register. - var configuration = new TelemetryConfiguration(); - - // Remark: ensure no sampling is applied — every exception must be recorded - var processorChain = configuration.DefaultTelemetrySink.TelemetryProcessorChainBuilder; - processorChain.Use((next) => new DefaultTelemetryProcessor(next)); - processorChain.Build(); - - configuration.TelemetryInitializers.Add(new DefaultTelemetryInitializer()); - configuration.ConnectionString = TelemetrySessionInfo.ConnectionString; - configuration.DisableTelemetry = UserPreferences.Current.TelemetryEnabled == false; -#if DEBUG - configuration.TelemetryChannel.DeveloperMode = Debugger.IsAttached; -#endif - return configuration; - } } diff --git a/src/Models/BravoPolicies.cs b/src/Models/BravoPolicies.cs deleted file mode 100644 index 0cdaaf1a..00000000 --- a/src/Models/BravoPolicies.cs +++ /dev/null @@ -1,113 +0,0 @@ -namespace Sqlbi.Bravo.Models -{ - using Sqlbi.Bravo.Infrastructure.Configuration.Settings; - using Sqlbi.Bravo.Infrastructure.Security.Policies; - using System; - using System.Text.Json.Serialization; - - public class BravoPolicies // : IUserSettings - { - private static readonly Lazy _instance; - - static BravoPolicies() - { - _instance = new Lazy(CreateInstance, isThreadSafe: true); - } - - public static BravoPolicies Current => _instance.Value; - - private static BravoPolicies CreateInstance() - { - var bravoPolicies = new BravoPolicies(); - return bravoPolicies; - } - - private BravoPolicies() - { - var policyManager = new PolicyManager(); - if (policyManager.PoliciesEnabled) - { - { - var (policy, value) = policyManager.GetTelemetryEnabledPolicy(); - TelemetryEnabled = value; - TelemetryEnabledPolicy = policy; - } - { - var (policy, value) = policyManager.GetUpdateChannelPolicy(); - UpdateChannel = value; - UpdateChannelPolicy = policy; - } - { - var (policy, value) = policyManager.GetUpdateCheckEnabledPolicy(); - UpdateCheckEnabled = value; - UpdateCheckEnabledPolicy = policy; - } - { - var (policy, value) = policyManager.GetUseSystemBrowserForAuthenticationPolicy(); - UseSystemBrowserForAuthentication = value; - UseSystemBrowserForAuthenticationPolicy = policy; - } - { - var (policy, value) = policyManager.GetCustomTemplatesEnabledPolicy(); - CustomTemplatesEnabled = value; - CustomTemplatesEnabledPolicy = policy; - } - { - var (policy, value) = policyManager.GetBuiltInTemplatesEnabledPolicy(); - BuiltInTemplatesEnabled = value; - BuiltInTemplatesEnabledPolicy = policy; - } - { - var (policy, value) = policyManager.GetCustomTemplatesOrganizationRepositoryPathPolicy(); - CustomTemplatesOrganizationRepositoryPath = value; - CustomTemplatesOrganizationRepositoryPathPolicy = policy; - } - } - } - - [JsonIgnore] - public bool TelemetryEnabled { get; } - - [JsonPropertyName("telemetryEnabledPolicy")] - public PolicyStatus TelemetryEnabledPolicy { get; } = PolicyStatus.NotConfigured; - - [JsonIgnore] - public UpdateChannelType UpdateChannel { get; } - - [JsonPropertyName("updateChannelPolicy")] - public PolicyStatus UpdateChannelPolicy { get; } = PolicyStatus.NotConfigured; - - [JsonIgnore] - public bool UpdateCheckEnabled { get; } - - [JsonPropertyName("updateCheckEnabledPolicy")] - public PolicyStatus UpdateCheckEnabledPolicy { get; } = PolicyStatus.NotConfigured; - - [JsonIgnore] - public bool UseSystemBrowserForAuthentication { get; } - - [JsonPropertyName("useSystemBrowserForAuthenticationPolicy")] - public PolicyStatus UseSystemBrowserForAuthenticationPolicy { get; } = PolicyStatus.NotConfigured; - - /// This property is not exposed in the because it is not to be set by the user. It's serialized in for the sole purpose of allowing the UI to read its value - [JsonPropertyName("builtInTemplatesEnabled")] - public bool BuiltInTemplatesEnabled { get; } // TODO: Add policy to the ADMX template - - [JsonPropertyName("builtInTemplatesEnabledPolicy")] - public PolicyStatus BuiltInTemplatesEnabledPolicy { get; } = PolicyStatus.NotConfigured; - - /// This property is not exposed in the because it is not to be set by the user. It's serialized in for the sole purpose of allowing the UI to read its value - [JsonPropertyName("customTemplatesEnabled")] - public bool CustomTemplatesEnabled { get; } // TODO: Add policy to the ADMX template - - [JsonPropertyName("customTemplatesEnabledPolicy")] - public PolicyStatus CustomTemplatesEnabledPolicy { get; } = PolicyStatus.NotConfigured; - - /// This property is not exposed in the because it is not to be set by the user. It's serialized in for the sole purpose of allowing the UI to read its value - [JsonPropertyName("customTemplatesOrganizationRepositoryPath")] - public string? CustomTemplatesOrganizationRepositoryPath { get; } // TODO: Add policy to the ADMX template - - [JsonPropertyName("customTemplatesOrganizationRepositoryPathPolicy")] - public PolicyStatus CustomTemplatesOrganizationRepositoryPathPolicy { get; } = PolicyStatus.NotConfigured; - } -} diff --git a/src/Program.cs b/src/Program.cs index 5591ef9f..b11cfd4e 100644 --- a/src/Program.cs +++ b/src/Program.cs @@ -5,7 +5,6 @@ using Sqlbi.Bravo.Infrastructure.Configuration; using Sqlbi.Bravo.Infrastructure.Helpers; using Sqlbi.Bravo.Infrastructure.Telemetry; - using System; using System.Windows.Forms; internal partial class Program @@ -35,7 +34,7 @@ public static void Main() } catch (Exception ex) { - TelemetryService.TrackFatalException(ex); + TelemetryService.Instance.TrackException(ex); ExceptionHelper.ShowDialog(ex); throw; } diff --git a/src/Scripts/@types/global.d.ts b/src/Scripts/@types/global.d.ts index 41e70b7d..d3749134 100644 --- a/src/Scripts/@types/global.d.ts +++ b/src/Scripts/@types/global.d.ts @@ -1,7 +1,6 @@ import * as CodeMirror from 'codemirror'; -import { Options, PolicyStatus } from '../controllers/options'; +import { Options, Policies } from '../controllers/options'; import { TelemetryConfig } from '../controllers/telemetry'; -import { Dic } from '../helpers/utils'; declare global { var CONFIG: { @@ -9,7 +8,7 @@ declare global { address: string version: string, options: Options, - policies?: Dic, + policies?: Policies, token?: string, telemetry?: TelemetryConfig culture: { diff --git a/src/Scripts/controllers/app.ts b/src/Scripts/controllers/app.ts index df88bea6..d311a31b 100644 --- a/src/Scripts/controllers/app.ts +++ b/src/Scripts/controllers/app.ts @@ -420,13 +420,13 @@ export class App { checkForUpdates(automatic = false) { - if (automatic && !optionsController.options.updateCheckEnabled) return; - - return host.getCurrentVersion(optionsController.options.updateChannel) + if (automatic && !optionsController.getOption("updateCheckEnabled")) return; + + return host.getCurrentVersion(optionsController.getOption("updateChannel")) .then(data => { let newVersion = null; - if (data.updateChannel == optionsController.options.updateChannel && data.isNewerVersion) { + if (data.updateChannel == optionsController.getOption("updateChannel") && data.isNewerVersion) { newVersion = new AppVersion({ version: data.version, downloadUrl: data.downloadUrl, diff --git a/src/Scripts/controllers/options.ts b/src/Scripts/controllers/options.ts index acf24b0a..23fe7069 100644 --- a/src/Scripts/controllers/options.ts +++ b/src/Scripts/controllers/options.ts @@ -25,9 +25,14 @@ export interface Options { customOptions?: ClientOptions } -export enum PolicyStatus { - NotConfigured = 0, - Forced = 1, +export interface Policies { + telemetryEnabled?: boolean | null + updateChannel?: UpdateChannelType | null + updateCheckEnabled?: boolean | null + useSystemBrowserForAuthentication?: boolean | null + builtInTemplatesEnabled?: boolean | null + customTemplatesEnabled?: boolean | null + customTemplatesOrganizationRepositoryPath?: string | null } export interface ProxyOptions { @@ -231,7 +236,7 @@ export class OptionsController extends OptionsStore { } }; - constructor(options?: Options, public policies?: Dic) { + constructor(options?: Options, public policies: Policies = {}) { super(options); if (options) { @@ -267,35 +272,44 @@ export class OptionsController extends OptionsStore { } /** - * Check if there is a policy for passed option path - * Note that this check policy also at group level. Child policy has priority over group policy. - * E.g.: - * - updateChannelEnabledPolicy -> policy for `updateChannel` option at root level - * - customOptions.localePolicy -> policy for `localeEnabled` option in the `customOptions` group - * - proxyPolicy -> policy for every option inside the `proxy` group - children inhereit this policy + * Get the effective policy value for the passed option path, or `undefined`/`null` if not configured. + * A policy applies at group level only when the value found along the path is itself a scalar + * (not a plain object) - in that case every option under that group inherits it. If the value found + * is a plain object, it is a dictionary of per-child overrides: an exact-leaf match is required, + * siblings that aren't explicitly configured are NOT locked by the rest of the group. + * E.g. (with policies = { updateChannel: 2, customOptions: { formatting: { preview: true } }, proxy: true }): + * - updateChannel -> 2 (scalar leaf match) + * - customOptions.formatting.preview -> true (scalar leaf match) + * - customOptions.formatting.region -> undefined (sibling not configured - NOT locked) + * - proxy.address -> true (proxy itself is a scalar - cascades to every child) */ - optionPolicy(optionPath: string) { - let obj = this.policies; - let path = optionPath.split("."); - let status = PolicyStatus.NotConfigured; - - for (let i = 0; i < path.length; i++) { - const prop = path[i]; + optionPolicy(optionPath: string): any { + let obj: any = this.policies; - // Check also if it is a group - const propPolicy = (obj)[`${prop}Policy`]; - if (propPolicy !== undefined) - status = propPolicy; + // Invariant: obj is a plain object at the start of every iteration (the loop returns + // as soon as it isn't), so there is no need to guard against a non-object obj here. + for (const prop of optionPath.split(".")) { + obj = obj[prop]; - if (i <= path.length - 1) { - if ((prop in obj)) - obj = (obj)[prop]; - } + if (obj === null || obj === undefined || typeof obj !== "object") + return obj; // leaf value, or a scalar found mid-path (group-level cascade) } - return status; + + return undefined; // path resolved to a plain object, not a usable scalar policy value } optionIsPolicyLocked(optionPath: string) { - return (this.optionPolicy(optionPath) != PolicyStatus.NotConfigured); + return (this.optionPolicy(optionPath) != null); + } + + /** + * Get the effective value for the passed option path: the policy value when one is + * configured (matching `optionIsPolicyLocked`), otherwise the user-configured option value. + * Overridden so every consumer of `getOption` - including the generic dialog renderer - + * automatically reflects policy overrides instead of the raw (possibly locked-but-stale) setting. + */ + getOption(optionPath: string): any { + const policyValue = this.optionPolicy(optionPath); + return (policyValue != null ? policyValue : super.getOption(optionPath)); } } \ No newline at end of file diff --git a/src/Scripts/controllers/telemetry.ts b/src/Scripts/controllers/telemetry.ts index 62d2f83c..b8fb0038 100644 --- a/src/Scripts/controllers/telemetry.ts +++ b/src/Scripts/controllers/telemetry.ts @@ -28,7 +28,7 @@ export class Telemetry { constructor(config: TelemetryConfig) { - this.enabled = optionsController.options.telemetryEnabled; + this.enabled = optionsController.getOption("telemetryEnabled"); // Configuration options at https://docs.microsoft.com/en-us/azure/azure-monitor/app/javascript this.appInsights = new ApplicationInsights({ config: { @@ -68,7 +68,7 @@ export class Telemetry { // Detect telemetry option change optionsController.on("telemetryEnabled.change", (changedOptions: any) => { - this.enabled = optionsController.options.telemetryEnabled; + this.enabled = optionsController.getOption("telemetryEnabled"); this.appInsights.updateSnippetDefinitions({ config: { diff --git a/src/Scripts/view/options-dialog-about.ts b/src/Scripts/view/options-dialog-about.ts index eb24ee9d..6ae5ccba 100644 --- a/src/Scripts/view/options-dialog-about.ts +++ b/src/Scripts/view/options-dialog-about.ts @@ -26,7 +26,7 @@ export class OptionsDialogAbout { } get canCheckForUpdates() { - return (this.canChangeCheckForUpdates || optionsController.options.updateCheckEnabled); + return (this.canChangeCheckForUpdates || optionsController.getOption("updateCheckEnabled")); } render(element: HTMLElement) { @@ -41,7 +41,7 @@ export class OptionsDialogAbout {
  ${i18n(strings.appVersion, { version: app.currentVersion.info.version})} @@ -49,7 +49,7 @@ export class OptionsDialogAbout {
${this.canCheckForUpdates ? `
` : ""}
- +
${!this.canChangeChannel || !this.canChangeCheckForUpdates ? `
diff --git a/src/Scripts/view/options-dialog-dev.ts b/src/Scripts/view/options-dialog-dev.ts index 0590c93a..11352d5b 100644 --- a/src/Scripts/view/options-dialog-dev.ts +++ b/src/Scripts/view/options-dialog-dev.ts @@ -35,6 +35,7 @@ export class OptionsDialogDev { let optionsStruct: OptionStruct[] = [ { option: "customTemplatesEnabled", + lockedByPolicy: optionsController.optionIsPolicyLocked("customTemplatesEnabled"), icon: "template-dev", name: i18n(strings.optionDev), description: i18n(strings.optionDevDescription), diff --git a/src/Scripts/view/scene-manage-dates-calendar.ts b/src/Scripts/view/scene-manage-dates-calendar.ts index 985b5607..82de711f 100644 --- a/src/Scripts/view/scene-manage-dates-calendar.ts +++ b/src/Scripts/view/scene-manage-dates-calendar.ts @@ -91,7 +91,7 @@ export class ManageDatesSceneCalendar extends ManageDatesScenePane { this.dateConfigurations.forEach(dateConfiguration => { values.push([dateConfiguration.templateUri, dateConfigurationName(dateConfiguration)]); }); - if (optionsController.options.customTemplatesEnabled) + if (optionsController.getOption("customTemplatesEnabled")) values.push(["{browse}", `(${i18n(strings.devTemplatesBrowse)}...)`]); selectElement.innerHTML = ` diff --git a/src/Scripts/view/scene-manage-dates.ts b/src/Scripts/view/scene-manage-dates.ts index 604f5c2a..7218226f 100644 --- a/src/Scripts/view/scene-manage-dates.ts +++ b/src/Scripts/view/scene-manage-dates.ts @@ -188,9 +188,10 @@ export class ManageDatesScene extends DocScene { // User/Org templates try { + const customTemplatesEnabled = optionsController.getOption("customTemplatesEnabled"); let customTemplates = [ - ...await host.getOrganizationTemplates(), - ...(optionsController.options.customTemplatesEnabled ? optionsController.options.customOptions.templates : []) + ...(customTemplatesEnabled ? await host.getOrganizationTemplates() : []), + ...(customTemplatesEnabled ? optionsController.options.customOptions.templates : []) ]; for (let i = 0; i < customTemplates.length; i++) { diff --git a/src/Services/ManageDatesService.cs b/src/Services/ManageDatesService.cs index 2c1fee92..9039c94c 100644 --- a/src/Services/ManageDatesService.cs +++ b/src/Services/ManageDatesService.cs @@ -7,6 +7,7 @@ using Sqlbi.Bravo.Infrastructure; using Sqlbi.Bravo.Infrastructure.Extensions; using Sqlbi.Bravo.Infrastructure.Helpers; + using Sqlbi.Bravo.Infrastructure.Policies; using Sqlbi.Bravo.Infrastructure.Services; using Sqlbi.Bravo.Infrastructure.Services.DaxTemplate; using Sqlbi.Bravo.Models; @@ -29,7 +30,12 @@ public interface IManageDatesService internal class ManageDatesService : IManageDatesService { - private readonly DaxTemplateManager _templateManager = new(); + private readonly DaxTemplateManager _templateManager; + + public ManageDatesService(IPolicies policies) + { + _templateManager = new DaxTemplateManager(policies); + } public IEnumerable GetConfigurations(PBIDesktopReport report, CancellationToken cancellationToken) { diff --git a/src/Services/TemplateDevelopmentService.cs b/src/Services/TemplateDevelopmentService.cs index 52071e60..a380ad4e 100644 --- a/src/Services/TemplateDevelopmentService.cs +++ b/src/Services/TemplateDevelopmentService.cs @@ -7,7 +7,6 @@ using Sqlbi.Bravo.Infrastructure; using Sqlbi.Bravo.Infrastructure.Extensions; using Sqlbi.Bravo.Infrastructure.Helpers; - using Sqlbi.Bravo.Infrastructure.Security.Policies; using Sqlbi.Bravo.Infrastructure.Telemetry; using Sqlbi.Bravo.Infrastructure.Services; using Sqlbi.Bravo.Infrastructure.Services.DaxTemplate; @@ -20,6 +19,7 @@ using System.Linq; using System.Text.Json; using System.Threading; + using Sqlbi.Bravo.Infrastructure.Policies; public interface ITemplateDevelopmentService { @@ -54,11 +54,13 @@ internal class TemplateDevelopmentService : ITemplateDevelopmentService private readonly JsonSerializerOptions _serializerOptions; private readonly DaxTemplateManager _templateManager; private readonly IServerAddressProvider _serverAddressProvider; + private readonly IPolicies _policies; - public TemplateDevelopmentService(IServerAddressProvider serverAddressProvider) + public TemplateDevelopmentService(IServerAddressProvider serverAddressProvider, IPolicies policies) { _serverAddressProvider = serverAddressProvider; - _templateManager = new DaxTemplateManager(); + _policies = policies; + _templateManager = new DaxTemplateManager(policies); _serializerOptions = new JsonSerializerOptions(AppEnvironment.DefaultJsonOptions) { WriteIndented = true }; } @@ -163,32 +165,28 @@ public IEnumerable GetOrganizationCustomPackages() { var customPackages = new List(); - var repositoryEnabled = BravoPolicies.Current.CustomTemplatesOrganizationRepositoryPathPolicy == PolicyStatus.Forced; - if (repositoryEnabled) + var repositoryPath = _policies.CustomTemplatesOrganizationRepositoryPath; + if (repositoryPath is not null && Directory.Exists(repositoryPath)) { - var repositoryExists = Directory.Exists(BravoPolicies.Current.CustomTemplatesOrganizationRepositoryPath); - if (repositoryExists) + var packagePaths = Directory.EnumerateFiles(repositoryPath, searchPattern: $"*{CustomPackageFileExtension}", new EnumerationOptions { - var packagePaths = Directory.EnumerateFiles(BravoPolicies.Current.CustomTemplatesOrganizationRepositoryPath!, searchPattern: $"*{CustomPackageFileExtension}", new EnumerationOptions - { - IgnoreInaccessible = true, - //RecurseSubdirectories = true, - }); + IgnoreInaccessible = true, + //RecurseSubdirectories = true, + }); - foreach (var packagePath in packagePaths) + foreach (var packagePath in packagePaths) + { + var package = _templateManager.GetPackage(packagePath); + var customPackage = new CustomPackage { - var package = _templateManager.GetPackage(packagePath); - var customPackage = new CustomPackage - { - Type = CustomPackageType.Organization, - Path = packagePath, - Name = package.Configuration.Name, - Description = package.Configuration.Description, - HasPackage = true, - }; - customPackages.Add(customPackage); - } - } + Type = CustomPackageType.Organization, + Path = packagePath, + Name = package.Configuration.Name, + Description = package.Configuration.Description, + HasPackage = true, + }; + customPackages.Add(customPackage); + } } return customPackages; diff --git a/src/Startup.cs b/src/Startup.cs index 6ba6e12f..ba95ccb7 100644 --- a/src/Startup.cs +++ b/src/Startup.cs @@ -8,6 +8,7 @@ using Microsoft.Extensions.DependencyInjection; using Sqlbi.Bravo.Infrastructure.Configuration.Settings; using Sqlbi.Bravo.Infrastructure.Extensions; + using Sqlbi.Bravo.Infrastructure.Policies; using Sqlbi.Bravo.Infrastructure.PowerBI; using Sqlbi.Bravo.Infrastructure.Services; using Sqlbi.Bravo.Infrastructure.Services.PowerBI; @@ -35,7 +36,11 @@ public void ConfigureServices(IServiceCollection services) #endif services.AddHttpClient(); services.AddOptions().Configure((settings) => settings.FromCommandLineArguments()); //.ValidateDataAnnotations(); - services.AddSingleton(); + + services.AddGroupPolicies(); + services.AddSingleton(_ => TelemetryService.Instance); + services.AddPowerBI(); + services.AddSingleton(); services.AddSingleton(); services.AddSingleton(); @@ -46,7 +51,6 @@ public void ConfigureServices(IServiceCollection services) services.AddSingleton(); services.AddSingleton(); services.AddSingleton(); - services.AddPowerBIServices(); } public void Configure(IApplicationBuilder application, IWebHostEnvironment environment) diff --git a/test/Bravo.Tests/GlobalUsings.cs b/test/Bravo.Tests/GlobalUsings.cs new file mode 100644 index 00000000..b99c99bc --- /dev/null +++ b/test/Bravo.Tests/GlobalUsings.cs @@ -0,0 +1,13 @@ +global using System; +global using System.Collections.Generic; +global using System.Diagnostics; +global using System.Diagnostics.CodeAnalysis; +global using System.IO; +global using System.Linq; +global using System.Net; +global using System.Net.Mime; +global using System.Text; +global using System.Text.Json; +global using System.Threading; +global using System.Threading.Tasks; +global using System.Globalization; diff --git a/test/Bravo.Tests/Infrastructure/Policies/PoliciesFactoryTests.cs b/test/Bravo.Tests/Infrastructure/Policies/PoliciesFactoryTests.cs new file mode 100644 index 00000000..b30201e5 --- /dev/null +++ b/test/Bravo.Tests/Infrastructure/Policies/PoliciesFactoryTests.cs @@ -0,0 +1,260 @@ +namespace Bravo.Tests.Infrastructure.Policies; + +using Sqlbi.Bravo.Infrastructure.Configuration.Settings; +using Sqlbi.Bravo.Infrastructure.Policies; +using System.Collections.Generic; +using Xunit; + +/// +/// Exercises PoliciesFactory's parsing/precedence logic using an in-memory fake instead of the +/// real registry. Registry-adapter behavior itself is covered separately by +/// . +/// +public class PoliciesFactoryTests +{ + private sealed class FakePolicySource : IPolicySource + { + private readonly Dictionary _values = new(); + + public void Set(string name, object value) => _values[name] = value; + + public int? GetInt(string name) + => _values.TryGetValue(name, out var value) && value is int intValue ? intValue : null; + + public string? GetString(string name) + => _values.TryGetValue(name, out var value) ? value as string : null; + } + + // MemberData deliberately carries only the (public) property name rather than a + // Func selector: Policies/IPolicies are internal, and a public test + // method cannot declare a parameter of a less-accessible type (CS0051), even with + // InternalsVisibleTo. The property is read via reflection inside the test body instead. + public static IEnumerable BoolPolicyNames() => new[] + { + new object[] { nameof(IPolicies.TelemetryEnabled) }, + new object[] { nameof(IPolicies.UpdateCheckEnabled) }, + new object[] { nameof(IPolicies.UseSystemBrowserForAuthentication) }, + new object[] { nameof(IPolicies.BuiltInTemplatesEnabled) }, + new object[] { nameof(IPolicies.CustomTemplatesEnabled) }, + }; + + private static bool? GetBoolProperty(Policies policies, string propertyName) + => (bool?)typeof(Policies).GetProperty(propertyName)!.GetValue(policies); + + [Fact] + public void FromSource_EmptySource_AllPropertiesAreNull() + { + var policies = PoliciesFactory.FromSource(new FakePolicySource()); + + Assert.Null(policies.TelemetryEnabled); + Assert.Null(policies.UpdateChannel); + Assert.Null(policies.UpdateCheckEnabled); + Assert.Null(policies.UseSystemBrowserForAuthentication); + Assert.Null(policies.BuiltInTemplatesEnabled); + Assert.Null(policies.CustomTemplatesEnabled); + Assert.Null(policies.CustomTemplatesOrganizationRepositoryPath); + } + + [Theory] + [MemberData(nameof(BoolPolicyNames))] + public void FromSource_BoolPolicyValueOne_ReturnsTrue(string propertyName) + { + var source = new FakePolicySource(); + source.Set(propertyName, 1); + + var policies = PoliciesFactory.FromSource(source); + + Assert.True(GetBoolProperty(policies, propertyName)); + } + + [Theory] + [MemberData(nameof(BoolPolicyNames))] + public void FromSource_BoolPolicyValueZero_ReturnsFalse(string propertyName) + { + var source = new FakePolicySource(); + source.Set(propertyName, 0); + + var policies = PoliciesFactory.FromSource(source); + + Assert.False(GetBoolProperty(policies, propertyName)); + } + + [Theory] + [MemberData(nameof(BoolPolicyNames))] + public void FromSource_BoolPolicyOutOfRangeIntValue_ReturnsNull(string propertyName) + { + var source = new FakePolicySource(); + source.Set(propertyName, 42); + + var policies = PoliciesFactory.FromSource(source); + + Assert.Null(GetBoolProperty(policies, propertyName)); + } + + [Theory] + [MemberData(nameof(BoolPolicyNames))] + public void FromSource_BoolPolicyNonIntValue_ReturnsNull(string propertyName) + { + var source = new FakePolicySource(); + source.Set(propertyName, "not-a-number"); + + var policies = PoliciesFactory.FromSource(source); + + Assert.Null(GetBoolProperty(policies, propertyName)); + } + + [Fact] + public void FromSource_UpdateChannelDefinedEnumValue_ReturnsParsedValue() + { + var source = new FakePolicySource(); + source.Set("UpdateChannel", (int)UpdateChannelType.Dev); + + var policies = PoliciesFactory.FromSource(source); + + Assert.Equal(UpdateChannelType.Dev, policies.UpdateChannel); + } + + [Fact] + public void FromSource_UpdateChannelUndefinedEnumValue_ReturnsNull() + { + // 1 is not a defined UpdateChannelType member (Beta is reserved/commented out) - must not be misparsed + var source = new FakePolicySource(); + source.Set("UpdateChannel", 1); + + var policies = PoliciesFactory.FromSource(source); + + Assert.Null(policies.UpdateChannel); + } + + [Fact] + public void FromSource_UpdateChannelNonIntValue_ReturnsNull() + { + var source = new FakePolicySource(); + source.Set("UpdateChannel", "Dev"); + + var policies = PoliciesFactory.FromSource(source); + + Assert.Null(policies.UpdateChannel); + } + + [Fact] + public void FromSource_CustomTemplatesOrganizationRepositoryPathStringValueSet_ReturnsValue() + { + var source = new FakePolicySource(); + source.Set("CustomTemplatesOrganizationRepositoryPath", @"C:\Templates\Org"); + + var policies = PoliciesFactory.FromSource(source); + + Assert.Equal(@"C:\Templates\Org", policies.CustomTemplatesOrganizationRepositoryPath); + } + + [Fact] + public void FromSource_CustomTemplatesOrganizationRepositoryPathNonStringValue_ReturnsNull() + { + var source = new FakePolicySource(); + source.Set("CustomTemplatesOrganizationRepositoryPath", 123); + + var policies = PoliciesFactory.FromSource(source); + + Assert.Null(policies.CustomTemplatesOrganizationRepositoryPath); + } + + [Fact] + public void Create_DoesNotThrow_AndReturnsAnInstance() + { + // Create() reads from the real HKLM/HKCU policy path, so its output depends on the + // machine's actual group-policy configuration and cannot be asserted deterministically here. + // This is a smoke test for the composition wiring (LocalMachine + CurrentUser -> Merge); + // the precedence rule itself is covered deterministically by the Merge tests below. + var policies = PoliciesFactory.Create(); + + Assert.NotNull(policies); + } + + private static readonly Policies AllNull = new( + TelemetryEnabled: null, + UpdateChannel: null, + UpdateCheckEnabled: null, + UseSystemBrowserForAuthentication: null, + BuiltInTemplatesEnabled: null, + CustomTemplatesEnabled: null, + CustomTemplatesOrganizationRepositoryPath: null); + + [Fact] + public void Merge_OnlyMachineValueSet_ReturnsMachineValue() + { + var machine = AllNull with { TelemetryEnabled = true }; + var user = AllNull; + + var merged = PoliciesFactory.Merge(machine, user); + + Assert.True(merged.TelemetryEnabled); + } + + [Fact] + public void Merge_OnlyUserValueSet_ReturnsUserValue() + { + var machine = AllNull; + var user = AllNull with { TelemetryEnabled = false }; + + var merged = PoliciesFactory.Merge(machine, user); + + Assert.False(merged.TelemetryEnabled); + } + + [Fact] + public void Merge_BothSet_MachineTakesPrecedenceOverUser() + { + var machine = AllNull with { TelemetryEnabled = true, UpdateChannel = UpdateChannelType.Stable, UpdateCheckEnabled = false }; + var user = AllNull with { TelemetryEnabled = false, UpdateChannel = UpdateChannelType.Dev, UpdateCheckEnabled = true }; + + var merged = PoliciesFactory.Merge(machine, user); + + Assert.True(merged.TelemetryEnabled); + Assert.Equal(UpdateChannelType.Stable, merged.UpdateChannel); + Assert.False(merged.UpdateCheckEnabled); + } + + [Fact] + public void Merge_NeitherSet_ReturnsNull() + { + var merged = PoliciesFactory.Merge(AllNull, AllNull); + + Assert.Null(merged.TelemetryEnabled); + } + + [Fact] + public void Merge_EachPropertyResolvedIndependently() + { + // Guards against a copy-paste wiring mistake in Merge() (e.g. reading the wrong + // property from machine/user) by exercising all 7 properties in a single assertion, + // each with a distinct machine/user combination. + var machine = new Policies( + TelemetryEnabled: true, + UpdateChannel: null, + UpdateCheckEnabled: null, + UseSystemBrowserForAuthentication: true, + BuiltInTemplatesEnabled: null, + CustomTemplatesEnabled: null, + CustomTemplatesOrganizationRepositoryPath: null); + + var user = new Policies( + TelemetryEnabled: false, // machine wins + UpdateChannel: UpdateChannelType.Dev, // machine unset -> user wins + UpdateCheckEnabled: true, // machine unset -> user wins + UseSystemBrowserForAuthentication: false, // machine wins + BuiltInTemplatesEnabled: false, // machine unset -> user wins + CustomTemplatesEnabled: null, // neither set + CustomTemplatesOrganizationRepositoryPath: @"C:\User\Path"); // machine unset -> user wins + + var merged = PoliciesFactory.Merge(machine, user); + + Assert.True(merged.TelemetryEnabled); + Assert.Equal(UpdateChannelType.Dev, merged.UpdateChannel); + Assert.True(merged.UpdateCheckEnabled); + Assert.True(merged.UseSystemBrowserForAuthentication); + Assert.False(merged.BuiltInTemplatesEnabled); + Assert.Null(merged.CustomTemplatesEnabled); + Assert.Equal(@"C:\User\Path", merged.CustomTemplatesOrganizationRepositoryPath); + } +} diff --git a/test/Bravo.Tests/Infrastructure/Policies/RegistryPolicySourceTests.cs b/test/Bravo.Tests/Infrastructure/Policies/RegistryPolicySourceTests.cs new file mode 100644 index 00000000..3e9b4261 --- /dev/null +++ b/test/Bravo.Tests/Infrastructure/Policies/RegistryPolicySourceTests.cs @@ -0,0 +1,89 @@ +namespace Bravo.Tests.Infrastructure.Policies; + +using Microsoft.Win32; +using Sqlbi.Bravo.Infrastructure.Policies; +using System; +using Xunit; + +/// +/// Covers only the thin RegistryKey-to-IPolicySource bridging contract (missing key, missing +/// value, type mismatches). Uses a real, isolated key under HKEY_CURRENT_USER (writable without +/// admin rights) since RegistryKey is a sealed BCL type that cannot be faked. The +/// parsing/precedence logic that matters is covered by PoliciesTests against a fake source instead. +/// +public class RegistryPolicySourceTests : IDisposable +{ + private readonly string _testKeyPath = @"SOFTWARE\Bravo.Tests\Policies\" + Guid.NewGuid().ToString("N"); + private readonly RegistryKey _testKey; + + public RegistryPolicySourceTests() + { + _testKey = Registry.CurrentUser.CreateSubKey(_testKeyPath, writable: true)!; + } + + public void Dispose() + { + _testKey.Dispose(); + Registry.CurrentUser.DeleteSubKeyTree(_testKeyPath, throwOnMissingSubKey: false); + } + + [Fact] + public void GetInt_NullKey_ReturnsNull() + { + var source = new RegistryPolicySource(key: null); + + Assert.Null(source.GetInt("AnyValue")); + } + + [Fact] + public void GetInt_ValueNotSet_ReturnsNull() + { + var source = new RegistryPolicySource(_testKey); + + Assert.Null(source.GetInt("Missing")); + } + + [Fact] + public void GetInt_DWordValue_ReturnsInt() + { + _testKey.SetValue("TelemetryEnabled", 1, RegistryValueKind.DWord); + var source = new RegistryPolicySource(_testKey); + + Assert.Equal(1, source.GetInt("TelemetryEnabled")); + } + + [Fact] + public void GetInt_StringValue_ReturnsNull() + { + _testKey.SetValue("TelemetryEnabled", "1", RegistryValueKind.String); + var source = new RegistryPolicySource(_testKey); + + Assert.Null(source.GetInt("TelemetryEnabled")); + } + + [Fact] + public void GetString_NullKey_ReturnsNull() + { + var source = new RegistryPolicySource(key: null); + + Assert.Null(source.GetString("AnyValue")); + } + + [Fact] + public void GetString_StringValue_ReturnsString() + { + _testKey.SetValue("CustomTemplatesOrganizationRepositoryPath", @"C:\Templates\Org", RegistryValueKind.String); + var source = new RegistryPolicySource(_testKey); + + Assert.Equal(@"C:\Templates\Org", source.GetString("CustomTemplatesOrganizationRepositoryPath")); + } + + [Fact] + public void GetString_DWordValue_ReturnsNull() + { + _testKey.SetValue("CustomTemplatesOrganizationRepositoryPath", 123, RegistryValueKind.DWord); + var source = new RegistryPolicySource(_testKey); + + Assert.Null(source.GetString("CustomTemplatesOrganizationRepositoryPath")); + } +}