Skip to content

fix: Allow system administrators to suppress telemetry by policy #1204

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

Merged
merged 10 commits into from
Sep 29, 2021
Original file line number Diff line number Diff line change
Expand Up @@ -263,11 +263,11 @@
<Grid.ColumnDefinitions>
<ColumnDefinition Width="320"/>
</Grid.ColumnDefinitions>
<TextBlock Grid.Row="0" Style="{StaticResource TxtTelemetrySettingInfo}" TextWrapping="Wrap">
<TextBlock Grid.Row="0" Name ="telemetryDescription" Style="{StaticResource TxtTelemetrySettingInfo}" TextWrapping="Wrap">
<Run Text="{x:Static Properties:Resources.TextBlockTelemetrySettingInfo}"/>
<LineBreak/>
</TextBlock>
<sharedControls:PrivacyLearnMore Style="{StaticResource TxtTelemetrySettingInfo}" Grid.Row="1"/>
<sharedControls:PrivacyLearnMore x:Name="privacyLearnMore" Style="{StaticResource TxtTelemetrySettingInfo}" Grid.Row="1"/>
</Grid>
<Grid Grid.Row="2" Margin="1,5,0,0">
<Grid.ColumnDefinitions>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
using AccessibilityInsights.SharedUx.Dialogs;
using AccessibilityInsights.SharedUx.Enums;
using AccessibilityInsights.SharedUx.Settings;
using AccessibilityInsights.SharedUx.Telemetry;
using AccessibilityInsights.SharedUx.ViewModels;
using System;
using System.Collections.Generic;
Expand Down Expand Up @@ -71,6 +72,13 @@ public ApplicationSettingsControl()
this.btnHotkeyToggle,
this.btnHotkeyToParent
};
if (!TelemetryController.DoesGroupPolicyAllowTelemetry)
{
this.telemetryDescription.Text = Properties.Resources.ApplicationSettingsControl_TelemetryDisabledByAdministrator;
this.privacyLearnMore.Visibility = Visibility.Collapsed;
this.lblEnableTelemetryLabel.Visibility = Visibility.Collapsed;
this.tgbtnEnableTelemetry.Visibility = Visibility.Collapsed;
}
}

/// <summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,9 +55,9 @@ private void btnExit_Click(object sender, RoutedEventArgs e)
DialogResult = ckbxAgreeToHelp.IsChecked ?? false;

if (DialogResult)
TelemetryController.EnableTelemetry();
TelemetryController.OptIntoTelemetry();
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is just a rename to better distinguish which level of enabling we are setting

else
TelemetryController.DisableTelemetry();
TelemetryController.OptOutOfTelemetry();
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is just a rename to better distinguish which level of enabling we are setting


WaitHandle.Set();
}
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -1548,4 +1548,7 @@ Do you want to change the channel? </value>
<data name="closeDialogText" xml:space="preserve">
<value>Close</value>
</data>
<data name="ApplicationSettingsControl_TelemetryDisabledByAdministrator" xml:space="preserve">
<value>Telemetry has been disabled by an administrator of this computer.</value>
</data>
</root>
Original file line number Diff line number Diff line change
Expand Up @@ -631,7 +631,7 @@ public static ConfigurationModel GetDefaultConfigurationModel(FixedConfigSetting
FontSize = FontSize.Standard,
HighlighterMode = HighlighterMode.HighlighterBeakerTooltip,
ShowAncestry = true,
EnableTelemetry = true,
EnableTelemetry = false,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't strictly necessary but better reflects what the default value should be

ShowTelemetryDialog = true,

IsUnderElementScope = true,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,15 @@ namespace AccessibilityInsights.SharedUx.Telemetry
{
internal interface ITelemetrySink
{
/// <summary>
/// We allow group policy to disable telemetry. This takes precedence over user opt-in
/// </summary>
bool DoesGroupPolicyAllowTelemetry { get; }

/// <summary>
/// Whether or not telemetry toggle button is enabled in the settings.
/// </summary>
bool IsTelemetryAllowed { get; set; }
bool HasUserOptedIntoTelemetry { get; set; }
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is just a rename to better distinguish which level of enabling we are setting


/// <summary>
/// Whether or not telemetry is enabled. Exposed to allow callers who do lots of
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,13 @@ public static class TelemetryController
{
private static readonly ITelemetrySink Sink = TelemetrySink.DefaultTelemetrySink;

public static void EnableTelemetry()
public static void OptIntoTelemetry()
{
if (!DoesGroupPolicyAllowTelemetry)
return;

// Open the telemetry sink
Sink.IsTelemetryAllowed = true;
Sink.HasUserOptedIntoTelemetry = true;

// Begin listening for telemetry events
// This must be done after the low-level sink is opened above
Expand All @@ -21,10 +24,15 @@ public static void EnableTelemetry()
AxeWindowsTelemetrySink.Enable();
}

public static void DisableTelemetry()
public static void OptOutOfTelemetry()
{
if (!DoesGroupPolicyAllowTelemetry)
return;

// Close the telemetry sink
Sink.IsTelemetryAllowed = false;
Sink.HasUserOptedIntoTelemetry = false;
}

public static bool DoesGroupPolicyAllowTelemetry => Sink.DoesGroupPolicyAllowTelemetry;
} // class
} // namespace
27 changes: 22 additions & 5 deletions src/AccessibilityInsights.SharedUx/Telemetry/TelemetrySink.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// Licensed under the MIT license. See LICENSE file in the project root for full license information.
using AccessibilityInsights.Extensions;
using AccessibilityInsights.Extensions.Interfaces.Telemetry;
using Microsoft.Win32;
using System;
using System.Collections.Generic;

Expand All @@ -19,24 +20,40 @@ internal class TelemetrySink : ITelemetrySink
/// <summary>
/// Holds the production TelemetrySink object
/// </summary>
internal static ITelemetrySink DefaultTelemetrySink { get; } = new TelemetrySink(Container.GetDefaultInstance()?.Telemetry);
internal static ITelemetrySink DefaultTelemetrySink { get; } = new TelemetrySink(Container.GetDefaultInstance()?.Telemetry, ReadGroupPolicyFromRegistry());

private readonly ITelemetry _telemetry;

internal TelemetrySink(ITelemetry telemetry)
internal TelemetrySink(ITelemetry telemetry, bool doesGroupPolicyAllowTelemetry)
{
_telemetry = telemetry;
DoesGroupPolicyAllowTelemetry = doesGroupPolicyAllowTelemetry;
}

private static bool ReadGroupPolicyFromRegistry()
{
// Return true unless the policy exists to disable the telemetry
int? policyValue = (int?)Registry.GetValue(
@"HKEY_LOCAL_MACHINE\Software\Policies\Accessibility Insights for Windows",
"DisableTelemetry", 0);

return !(policyValue.HasValue && policyValue.Value == 1);
}

/// <summary>
/// Implements <see cref="ITelemetrySink.DoesGroupPolicyAllowTelemetry"/>
/// </summary>
public bool DoesGroupPolicyAllowTelemetry { get; }

/// <summary>
/// Implements <see cref="ITelemetrySink.IsTelemetryAllowed"/>
/// Implements <see cref="ITelemetrySink.HasUserOptedIntoTelemetry"/>
/// </summary>
public bool IsTelemetryAllowed { get; set; }
public bool HasUserOptedIntoTelemetry { get; set; }

/// <summary>
/// Implements <see cref="ITelemetrySink.IsEnabled"/>
/// </summary>
public bool IsEnabled => IsTelemetryAllowed && _telemetry != null;
public bool IsEnabled => DoesGroupPolicyAllowTelemetry && HasUserOptedIntoTelemetry && _telemetry != null;

/// <summary>
/// Implements <see cref="ITelemetrySink.PublishTelemetryEvent(string, string, string)"/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ public void LoadFromJSON_FileDoesNotExist_DataIsCorrect()
ConfirmEnumerablesMatchExpectations(new int[] { }, config.CoreTPAttributes.ToArray());
Assert.IsFalse(config.DisableTestsInSnapMode);
Assert.IsFalse(config.DisableDarkMode);
Assert.IsTrue(config.EnableTelemetry);
Assert.IsFalse(config.EnableTelemetry);
Assert.IsTrue(config.EventRecordPath.Equals(testProvider.UserDataFolderPath));
Assert.AreEqual(FontSize.Standard, config.FontSize);
Assert.AreEqual(HighlighterMode.HighlighterBeakerTooltip, config.HighlighterMode);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,22 +28,30 @@ public class TelemetrySinkUnitTests
public void BeforeEachTest()
{
_telemetryMock = new Mock<ITelemetry>(MockBehavior.Strict);
_telemetrySink = new TelemetrySink(_telemetryMock.Object);
_telemetrySink = new TelemetrySink(_telemetryMock.Object, true);
}

[TestMethod]
[Timeout(1000)]
public void IsEnabled_TelemetryIsNull_ReturnsFalse()
{
TelemetrySink sink = new TelemetrySink(null);
TelemetrySink sink = new TelemetrySink(null, true);
Assert.IsFalse(sink.IsEnabled);
}

[TestMethod]
[Timeout(1000)]
public void IsEnabled_TelemetryIsDisabledByGroupPolicy_ReturnsFalse()
{
TelemetrySink sink = new TelemetrySink(_telemetryMock.Object, false);
Assert.IsFalse(sink.IsEnabled);
}

[TestMethod]
[Timeout(1000)]
public void IsTelemetryAllowed_DefaultToFalse()
{
Assert.IsFalse(_telemetrySink.IsTelemetryAllowed);
Assert.IsFalse(_telemetrySink.HasUserOptedIntoTelemetry);
}

[TestMethod]
Expand All @@ -57,7 +65,7 @@ public void IsEnabled_IsTelemetryAllowedIsFalse_ReturnsFalse()
[Timeout(1000)]
public void IsEnabled_IsTelemetryAllowedIsTrue_ReturnsTrue()
{
_telemetrySink.IsTelemetryAllowed = true;
_telemetrySink.HasUserOptedIntoTelemetry = true;
Assert.IsTrue(_telemetrySink.IsEnabled);
}

Expand All @@ -74,7 +82,7 @@ public void PublishTelemetryEvent_SingleProperty_TelemetryAllowed_PublishesCorre
{
PropertyBag actualPropertyBag = null;

_telemetrySink.IsTelemetryAllowed = true;
_telemetrySink.HasUserOptedIntoTelemetry = true;
_telemetryMock.Setup(x => x.PublishEvent(EventName1, It.IsAny<PropertyBag>()))
.Callback<string, PropertyBag>((_, p) => actualPropertyBag = p);

Expand All @@ -90,7 +98,7 @@ public void PublishTelemetryEvent_SingleProperty_TelemetryAllowed_PublishesCorre
public void PublishTelemetryEvent_SingleProperty_TelemetryAllowed_ThrowsOnPublish_ReportsException()
{
Exception expectedExpection = new OutOfMemoryException();
_telemetrySink.IsTelemetryAllowed = true;
_telemetrySink.HasUserOptedIntoTelemetry = true;
_telemetryMock.Setup(x => x.PublishEvent(EventName1, It.IsAny<PropertyBag>()))
.Throws(expectedExpection);
_telemetryMock.Setup(x => x.ReportException(expectedExpection));
Expand Down Expand Up @@ -123,7 +131,7 @@ public void PublishTelemetryEvent_PropertyBag_TelemetryAllowed_ChainsSameData()
{ PropertyName2, Value2 },
};

_telemetrySink.IsTelemetryAllowed = true;
_telemetrySink.HasUserOptedIntoTelemetry = true;
_telemetryMock.Setup(x => x.PublishEvent(EventName2, expectedPropertyBag));
_telemetrySink.PublishTelemetryEvent(EventName2, expectedPropertyBag);

Expand All @@ -141,7 +149,7 @@ public void PublishTelemetryEvent_PropertyBag_TelemetryAllowed_ThrowsOnPublish_R
{ PropertyName2, Value1 },
};

_telemetrySink.IsTelemetryAllowed = true;
_telemetrySink.HasUserOptedIntoTelemetry = true;
_telemetryMock.Setup(x => x.PublishEvent(EventName2, expectedPropertyBag))
.Throws(expectedExpection);
_telemetryMock.Setup(x => x.ReportException(expectedExpection));
Expand All @@ -162,7 +170,7 @@ public void AddOrUpdateContextProperty_TelemetryNotAllowed_DoesNotChain()
[Timeout(1000)]
public void AddOrUpdateContextProperty_TelemetryIsAllowed_ChainsSameData()
{
_telemetrySink.IsTelemetryAllowed = true;
_telemetrySink.HasUserOptedIntoTelemetry = true;
_telemetryMock.Setup(x => x.AddOrUpdateContextProperty(PropertyName2, Value2));

_telemetrySink.AddOrUpdateContextProperty(PropertyName2, Value2);
Expand All @@ -175,7 +183,7 @@ public void AddOrUpdateContextProperty_TelemetryIsAllowed_ChainsSameData()
public void AddOrUpdateContextProperty_TelemetryIsAllowed_TelemetryThrowsException_ReportsException()
{
Exception expectedExpection = new OutOfMemoryException();
_telemetrySink.IsTelemetryAllowed = true;
_telemetrySink.HasUserOptedIntoTelemetry = true;
_telemetryMock.Setup(x => x.AddOrUpdateContextProperty(PropertyName2, Value2))
.Throws(expectedExpection);
_telemetryMock.Setup(x => x.ReportException(expectedExpection));
Expand All @@ -196,7 +204,7 @@ public void ReportException_TelemetryNotAllowed_DoesNotChain()
[Timeout(1000)]
public void ReportException_TelemetryIsAllowed_ExceptionIsNull_DoesNotChain()
{
_telemetrySink.IsTelemetryAllowed = true;
_telemetrySink.HasUserOptedIntoTelemetry = true;
_telemetrySink.ReportException(null);
}

Expand All @@ -206,7 +214,7 @@ public void ReportException_TelemetryIsAllowed_ChainsSameData()
{
Exception expectedException = new OutOfMemoryException();

_telemetrySink.IsTelemetryAllowed = true;
_telemetrySink.HasUserOptedIntoTelemetry = true;
_telemetryMock.Setup(x => x.ReportException(expectedException));

_telemetrySink.ReportException(expectedException);
Expand All @@ -221,7 +229,7 @@ public void ReportException_TelemetryIsAllowed_TelemetryThrowsException_DoesNotR
Exception expectedException = new OutOfMemoryException();
Exception unexpectedException = new InvalidOperationException();

_telemetrySink.IsTelemetryAllowed = true;
_telemetrySink.HasUserOptedIntoTelemetry = true;
_telemetryMock.Setup(x => x.ReportException(expectedException))
.Throws(unexpectedException);

Expand Down
9 changes: 6 additions & 3 deletions src/AccessibilityInsights/MainWindowHelpers/Initialize.cs
Original file line number Diff line number Diff line change
Expand Up @@ -141,9 +141,12 @@ private static void PopulateConfigurations(TelemetryBuffer telemetryBuffer)
// Configure the correct ReleaseChannel for autoupdate
Container.SetAutoUpdateReleaseChannel(ConfigurationManager.GetDefaultInstance().AppConfig.ReleaseChannel.ToString());

// enable/disable telemetry
if (ConfigurationManager.GetDefaultInstance().AppConfig.EnableTelemetry)
TelemetryController.EnableTelemetry();
// Opt into telemetry if allowed and user has chosen to do so
if (TelemetryController.DoesGroupPolicyAllowTelemetry &&
ConfigurationManager.GetDefaultInstance().AppConfig.EnableTelemetry)
{
TelemetryController.OptIntoTelemetry();
}

// Update theming since it depends on config options
SetColorTheme();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// Licensed under the MIT license. See LICENSE file in the project root for full license information.
using AccessibilityInsights.SharedUx.Dialogs;
using AccessibilityInsights.SharedUx.Settings;
using AccessibilityInsights.SharedUx.Telemetry;
using System.Windows;

namespace AccessibilityInsights
Expand All @@ -18,9 +19,12 @@ private void ShowTelemetryDialog()
{
if (ConfigurationManager.GetDefaultInstance().AppConfig.ShowTelemetryDialog)
{
if (TelemetryController.DoesGroupPolicyAllowTelemetry)
{
#pragma warning disable CS4014 // Because this call is not awaited, execution of the current method continues before the call is completed
ctrlDialogContainer.ShowDialog(new TelemetryApproveContainedDialog());
ctrlDialogContainer.ShowDialog(new TelemetryApproveContainedDialog());
#pragma warning restore CS4014 // Because this call is not awaited, execution of the current method continues before the call is completed
}
}
}
}
Expand Down