V8: Fix missing site identifier (#12035)

* Add SiteIdentifierService

* Use SiteIdentifier service to create site identifier if it doesn't exist

* Use default timeout for telemetry request

* Use SiteIdentifierService in TelemetryIdentifierStep

* Send SiteId when requesting dashboard

* Make empty GUID an invalid site identifier

Co-authored-by: nikolajlauridsen <[email protected]>

Author: nikolajlauridsen

src/Umbraco.Core/Telemetry/ISiteIdentifierService.cs

@@ -0,0 +1,29 @@
+using System;
+
+namespace Umbraco.Core.Telemetry
+{
+    /// <summary>
+    /// Used to get and create the site identifier
+    /// </summary>
+    public interface ISiteIdentifierService
+    {
+
+        /// <summary>
+        /// Tries to get the site identifier
+        /// </summary>
+        /// <returns></returns>
+        bool TryGetSiteIdentifier(out Guid siteIdentifier);
+
+        /// <summary>
+        /// Tries to get the site identifier or otherwise create it if it doesn't exist.
+        /// </summary>
+        /// <param name="siteIdentifier"></param>
+        /// <returns></returns>
+        bool TryGetOrCreateSiteIdentifier(out Guid siteIdentifier);
+
+        /// <summary>
+        /// Creates the site identifier and writes it to config.
+        /// </summary>
+        bool TryCreateSiteIdentifier(out Guid createdGuid);
+    }
+}

src/Umbraco.Core/Telemetry/TelemetryService.cs

@@ -1,7 +1,6 @@
 using System;
 using System.Collections.Generic;
 using Umbraco.Core.Configuration;
-using Umbraco.Core.Configuration.UmbracoSettings;
 using Umbraco.Core.Manifest;
 using Umbraco.Core.Telemetry.Models;
 
@@ -10,24 +9,24 @@ namespace Umbraco.Core.Telemetry
     /// <inheritdoc/>
     internal class TelemetryService : ITelemetryService
     {
-        private readonly IUmbracoSettingsSection _settings;
+        private readonly ISiteIdentifierService _siteIdentifierService;
         private readonly ManifestParser _manifestParser;
 
         /// <summary>
         /// Initializes a new instance of the <see cref="TelemetryService"/> class.
         /// </summary>
         public TelemetryService(
             ManifestParser manifestParser,
-            IUmbracoSettingsSection settings)
+            ISiteIdentifierService siteIdentifierService)
         {
             _manifestParser = manifestParser;
-            _settings = settings;
+            _siteIdentifierService = siteIdentifierService;
         }
 
         /// <inheritdoc/>
         public bool TryGetTelemetryReportData(out TelemetryReportData telemetryReportData)
         {
-            if (TryGetTelemetryId(out Guid telemetryId) is false)
+            if (_siteIdentifierService.TryGetOrCreateSiteIdentifier(out Guid telemetryId) is false)
             {
                 telemetryReportData = null;
                 return false;
@@ -42,20 +41,6 @@ public bool TryGetTelemetryReportData(out TelemetryReportData telemetryReportDat
             return true;
         }
 
-        private bool TryGetTelemetryId(out Guid telemetryId)
-        {
-            // Parse telemetry string as a GUID & verify its a GUID and not some random string
-            // since users may have messed with or decided to empty the app setting or put in something random
-            if (Guid.TryParse(_settings.BackOffice.Id, out var parsedTelemetryId) is false)
-            {
-                telemetryId = Guid.Empty;
-                return false;
-            }
-
-            telemetryId = parsedTelemetryId;
-            return true;
-        }
-
         private IEnumerable<PackageTelemetry> GetPackageTelemetry()
         {
             List<PackageTelemetry> packages = new ();

src/Umbraco.Core/Umbraco.Core.csproj

@@ -400,6 +400,7 @@
     <Compile Include="Services\DateTypeServiceExtensions.cs" />
     <Compile Include="Services\PropertyValidationService.cs" />
     <Compile Include="Composing\TypeCollectionBuilderBase.cs" />
+    <Compile Include="Telemetry\ISiteIdentifierService.cs" />
     <Compile Include="Telemetry\ITelemetryService.cs" />
     <Compile Include="Telemetry\Models\PackageTelemetry.cs" />
     <Compile Include="Telemetry\Models\TelemetryReportData.cs" />

src/Umbraco.Web/Editors/DashboardController.cs

@@ -20,6 +20,7 @@
 using Umbraco.Core.Models;
 using Umbraco.Web.Services;
 using System.Web.Http;
+using Umbraco.Core.Telemetry;
 
 namespace Umbraco.Web.Editors
 {
@@ -34,18 +35,28 @@ public class DashboardController : UmbracoApiController
     {
         private readonly IDashboardService _dashboardService;
         private readonly IContentDashboardSettings _dashboardSettings;
+        private readonly ISiteIdentifierService _siteIdentifierService;
 
         /// <summary>
         /// Initializes a new instance of the <see cref="DashboardController"/> with all its dependencies.
         /// </summary>
-        public DashboardController(IGlobalSettings globalSettings, IUmbracoContextAccessor umbracoContextAccessor,
-            ISqlContext sqlContext, ServiceContext services, AppCaches appCaches, IProfilingLogger logger,
-            IRuntimeState runtimeState, IDashboardService dashboardService, UmbracoHelper umbracoHelper,
-            IContentDashboardSettings dashboardSettings)
+        public DashboardController(
+            IGlobalSettings globalSettings,
+            IUmbracoContextAccessor umbracoContextAccessor,
+            ISqlContext sqlContext,
+            ServiceContext services,
+            AppCaches appCaches,
+            IProfilingLogger logger,
+            IRuntimeState runtimeState,
+            IDashboardService dashboardService,
+            UmbracoHelper umbracoHelper,
+            IContentDashboardSettings dashboardSettings,
+            ISiteIdentifierService siteIdentifierService)
             : base(globalSettings, umbracoContextAccessor, sqlContext, services, appCaches, logger, runtimeState, umbracoHelper)
         {
             _dashboardService = dashboardService;
             _dashboardSettings = dashboardSettings;
+            _siteIdentifierService = siteIdentifierService;
         }
 
         //we have just one instance of HttpClient shared for the entire application
@@ -60,17 +71,19 @@ public async Task<JObject> GetRemoteDashboardContent(string section, string base
             var language = user.Language;
             var version = UmbracoVersion.SemanticVersion.ToSemanticString();
             var isAdmin = user.IsAdmin();
+            _siteIdentifierService.TryGetOrCreateSiteIdentifier(out var siteIdentifier);
 
             VerifyDashboardSource(baseUrl);
 
-            var url = string.Format("{0}{1}?section={2}&allowed={3}&lang={4}&version={5}&admin={6}",
+            var url = string.Format("{0}{1}?section={2}&allowed={3}&lang={4}&version={5}&admin={6}&siteid={7}",
                 baseUrl,
                 _dashboardSettings.ContentDashboardPath,
                 section,
                 allowedSections,
                 language,
                 version,
-                isAdmin);
+                isAdmin,
+                siteIdentifier.ToString());
             var key = "umbraco-dynamic-dashboard-" + language + allowedSections.Replace(",", "-") + section;
 
             var content = AppCaches.RuntimeCache.GetCacheItem<JObject>(key);

src/Umbraco.Web/Install/InstallSteps/TelemetryIdentifierStep.cs

@@ -1,10 +1,6 @@
-using System;
-using System.IO;
-using System.Threading.Tasks;
-using System.Xml.Linq;
+using System.Threading.Tasks;
 using Umbraco.Core.Configuration.UmbracoSettings;
-using Umbraco.Core.IO;
-using Umbraco.Core.Logging;
+using Umbraco.Core.Telemetry;
 using Umbraco.Web.Install.Models;
 
 namespace Umbraco.Web.Install.InstallSteps
@@ -14,54 +10,20 @@ namespace Umbraco.Web.Install.InstallSteps
         PerformsAppRestart = false)]
     internal class TelemetryIdentifierStep : InstallSetupStep<object>
     {
-        private readonly IProfilingLogger _logger;
         private readonly IUmbracoSettingsSection _settings;
+        private readonly ISiteIdentifierService _siteIdentifierService;
 
-        public TelemetryIdentifierStep(IProfilingLogger logger, IUmbracoSettingsSection settings)
+        public TelemetryIdentifierStep(
+            IUmbracoSettingsSection settings,
+            ISiteIdentifierService siteIdentifierService)
         {
-            _logger = logger;
             _settings = settings;
+            _siteIdentifierService = siteIdentifierService;
         }
 
         public override Task<InstallSetupResult> ExecuteAsync(object model)
         {
-            // Generate GUID
-            var telemetrySiteIdentifier = Guid.NewGuid();
-
-            // Modify the XML to add a new GUID site identifier
-            // hack: ensure this does not trigger a restart
-            using (ChangesMonitor.Suspended())
-            {
-                var umbracoSettingsPath = IOHelper.MapPath(SystemFiles.UmbracoSettings);
-                if(File.Exists(umbracoSettingsPath) == false)
-                {
-                    // Log an error
-                    _logger.Error<TelemetryIdentifierStep>("Unable to find umbracoSettings.config file to add telemetry site identifier");
-                    return Task.FromResult<InstallSetupResult>(null);
-                }
-
-                try
-                {
-                    var umbracoConfigXml = XDocument.Load(umbracoSettingsPath, LoadOptions.PreserveWhitespace);
-                    if (umbracoConfigXml.Root != null)
-                    {
-                        var backofficeElement = umbracoConfigXml.Root.Element("backOffice");
-                        if (backofficeElement == null)
-                            return Task.FromResult<InstallSetupResult>(null);
-
-                        // Will add ID attribute if it does not exist
-                        backofficeElement.SetAttributeValue("id", telemetrySiteIdentifier.ToString());
-
-                        // Save file back down
-                        umbracoConfigXml.Save(umbracoSettingsPath, SaveOptions.DisableFormatting);
-                    }
-                }
-                catch (Exception ex)
-                {
-                    _logger.Error<TelemetryIdentifierStep>(ex, "Couldn't update umbracoSettings.config with a backoffice with a telemetry site identifier");
-                }
-            }
-
+            _siteIdentifierService.TryCreateSiteIdentifier(out _);
             return Task.FromResult<InstallSetupResult>(null);
         }
 

src/Umbraco.Web/Runtime/WebInitialComposer.cs

@@ -39,7 +39,9 @@
 using Current = Umbraco.Web.Composing.Current;
 using Umbraco.Web.PropertyEditors;
 using Umbraco.Core.Models;
+using Umbraco.Core.Telemetry;
 using Umbraco.Web.Models;
+using Umbraco.Web.Telemetry;
 
 namespace Umbraco.Web.Runtime
 {
@@ -58,6 +60,8 @@ public override void Compose(Composition composition)
 
             composition.ComposeWebMappingProfiles();
 
+            composition.RegisterUnique<ISiteIdentifierService, SiteIdentifierService>();
+
             //register the install components
             //NOTE: i tried to not have these registered if we weren't installing or upgrading but post install when the site restarts
             //it still needs to use the install controller so we can't do that

src/Umbraco.Web/Telemetry/ReportSiteTask.cs

@@ -62,9 +62,6 @@ public override async Task<bool> PerformRunAsync(CancellationToken token)
                 {
                     request.Content = new StringContent(JsonConvert.SerializeObject(telemetryReportData), Encoding.UTF8, "application/json"); //CONTENT-TYPE header
 
-                    // Set a low timeout - no need to use a larger default timeout for this POST request
-                    _httpClient.Timeout = new TimeSpan(0, 0, 1);
-
                     // Make a HTTP Post to telemetry service
                     // https://telemetry.umbraco.com/installs/
                     // Fire & Forget, do not need to know if its a 200, 500 etc

src/Umbraco.Web/Telemetry/SiteIdentifierService.cs

@@ -0,0 +1,100 @@
+using System;
+using System.IO;
+using System.Xml.Linq;
+using Umbraco.Core.Configuration.UmbracoSettings;
+using Umbraco.Core.IO;
+using Umbraco.Core.Logging;
+using Umbraco.Core.Telemetry;
+using Umbraco.Web.Install;
+
+namespace Umbraco.Web.Telemetry
+{
+    internal class SiteIdentifierService : ISiteIdentifierService
+    {
+        private readonly IUmbracoSettingsSection _settings;
+        private readonly ILogger _logger;
+
+        public SiteIdentifierService(IUmbracoSettingsSection settings, ILogger logger)
+        {
+            _settings = settings;
+            _logger = logger;
+        }
+
+        public bool TryGetSiteIdentifier(out Guid siteIdentifier)
+        {
+            // Parse telemetry string as a GUID & verify its a GUID and not some random string
+            // since users may have messed with or decided to empty the app setting or put in something random
+            if (Guid.TryParse(_settings.BackOffice.Id, out var parsedTelemetryId) is false
+                || parsedTelemetryId == Guid.Empty)
+            {
+                siteIdentifier = Guid.Empty;
+                return false;
+            }
+
+            siteIdentifier = parsedTelemetryId;
+            return true;
+        }
+
+        public bool TryGetOrCreateSiteIdentifier(out Guid siteIdentifier)
+        {
+            if (TryGetSiteIdentifier(out var existingId))
+            {
+                siteIdentifier = existingId;
+                return true;
+            }
+
+            if (TryCreateSiteIdentifier(out var createdId))
+            {
+                siteIdentifier = createdId;
+                return true;
+            }
+
+            siteIdentifier = Guid.Empty;
+            return false;
+        }
+
+        public bool TryCreateSiteIdentifier(out Guid createdGuid)
+        {
+            createdGuid = Guid.NewGuid();
+
+            // Modify the XML to add a new GUID site identifier
+            // hack: ensure this does not trigger a restart
+            using (ChangesMonitor.Suspended())
+            {
+                var umbracoSettingsPath = IOHelper.MapPath(SystemFiles.UmbracoSettings);
+
+                if (File.Exists(umbracoSettingsPath) is false)
+                {
+                    _logger.Error<SiteIdentifierService>("Unable to find umbracoSettings.config file to add telemetry site identifier");
+                    return false;
+                }
+
+                try
+                {
+                    var umbracoConfigXml = XDocument.Load(umbracoSettingsPath, LoadOptions.PreserveWhitespace);
+                    if (umbracoConfigXml.Root != null)
+                    {
+                        var backofficeElement = umbracoConfigXml.Root.Element("backOffice");
+                        if (backofficeElement is null)
+                        {
+                            return false;
+                        }
+
+                        // Will add ID attribute if it does not exist
+                        backofficeElement.SetAttributeValue("id", createdGuid.ToString());
+
+                        // Save file back down
+                        umbracoConfigXml.Save(umbracoSettingsPath, SaveOptions.DisableFormatting);
+                    }
+                }
+                catch (Exception ex)
+                {
+                    _logger.Error<SiteIdentifierService>(ex, "Couldn't update umbracoSettings.config with a backoffice with a telemetry site identifier");
+                    return false;
+                }
+            }
+
+            return true;
+        }
+    }
+}

src/Umbraco.Web/Telemetry/TelemetryComponent.cs

@@ -12,7 +12,7 @@ public class TelemetryComponent : IComponent
         private readonly ITelemetryService _telemetryService;
         private BackgroundTaskRunner<IBackgroundTask> _telemetryReporterRunner;
 
-        public TelemetryComponent(IProfilingLogger logger, IUmbracoSettingsSection settings, ITelemetryService telemetryService)
+        public TelemetryComponent(IProfilingLogger logger, ITelemetryService telemetryService)
         {
             _logger = logger;
             _telemetryService = telemetryService;

src/Umbraco.Web/Umbraco.Web.csproj

@@ -347,6 +347,7 @@
     <Compile Include="Models\LinkType.cs" />
     <Compile Include="Models\TemplateQuery\OperatorFactory.cs" />
     <Compile Include="Telemetry\ReportSiteTask.cs" />
+    <Compile Include="Telemetry\SiteIdentifierService.cs" />
     <Compile Include="Telemetry\TelemetryComponent.cs" />
     <Compile Include="Telemetry\TelemetryComposer.cs" />
     <Compile Include="Templates\HtmlLocalLinkParser.cs" />