Merge pull request #93 from umbraco/bugfix/zapier-auth

acoumb · web-flow · commit 2f92b4b6635d · 2023-11-03T09:50:29.000+02:00
Update Zapier API key authentication flow
diff --git a/src/Umbraco.Forms.Integrations.Automation.Zapier/Configuration/AppSettings.cs b/src/Umbraco.Forms.Integrations.Automation.Zapier/Configuration/AppSettings.cs
@@ -1,6 +1,6 @@
 ﻿namespace Umbraco.Forms.Integrations.Automation.Zapier.Configuration
 {
-    public class ZapierSettings
+    public class AppSettings
     {
         public string UserGroupAlias { get; set; }
 
diff --git a/src/Umbraco.Forms.Integrations.Automation.Zapier/Configuration/ZapierCmsSettings.cs b/src/Umbraco.Forms.Integrations.Automation.Zapier/Configuration/ZapierCmsSettings.cs
@@ -0,0 +1,6 @@
+﻿namespace Umbraco.Forms.Integrations.Automation.Zapier.Configuration
+{
+    public class ZapierCmsSettings : AppSettings
+    {
+    }
+}
diff --git a/src/Umbraco.Forms.Integrations.Automation.Zapier/Configuration/ZapierFormsSettings.cs b/src/Umbraco.Forms.Integrations.Automation.Zapier/Configuration/ZapierFormsSettings.cs
@@ -0,0 +1,6 @@
+﻿namespace Umbraco.Forms.Integrations.Automation.Zapier.Configuration
+{
+    public class ZapierFormsSettings : AppSettings
+    {
+    }
+}
diff --git a/src/Umbraco.Forms.Integrations.Automation.Zapier/Constants.cs b/src/Umbraco.Forms.Integrations.Automation.Zapier/Constants.cs
@@ -14,6 +14,8 @@ public static class ZapierAppConfiguration
 
         public static class Configuration
         {
+            public const string CmsSettings = "Umbraco:CMS:Integrations:Automation:Zapier:Settings";
+
             public const string Settings = "Umbraco:Forms:Integrations:Automation:Zapier:Settings";
         }
 
diff --git a/src/Umbraco.Forms.Integrations.Automation.Zapier/Controllers/FormController.cs b/src/Umbraco.Forms.Integrations.Automation.Zapier/Controllers/FormController.cs
@@ -3,9 +3,6 @@
 
 using Umbraco.Forms.Integrations.Automation.Zapier.Models.Dtos;
 using Umbraco.Forms.Integrations.Automation.Zapier.Services;
-using Umbraco.Forms.Integrations.Automation.Zapier.Configuration;
-
-using Microsoft.Extensions.Options;
 
 namespace Umbraco.Forms.Integrations.Automation.Zapier.Controllers
 {
@@ -16,11 +13,9 @@ public class FormController : ZapierFormAuthorizedApiController
     {
         private readonly ZapierFormService _zapierFormService;
 
-        public FormController(IOptions<ZapierSettings> options, IUserValidationService userValidationService, ZapierFormService zapierFormService)
-            : base(options, userValidationService)
-        {
-            _zapierFormService = zapierFormService;
-        }
+        public FormController(
+            IUserValidationService userValidationService, ZapierFormService zapierFormService)
+            : base(userValidationService) => _zapierFormService = zapierFormService;
 
         public IEnumerable<FormDto> GetForms()
         {
diff --git a/src/Umbraco.Forms.Integrations.Automation.Zapier/Controllers/FormPollingController.cs b/src/Umbraco.Forms.Integrations.Automation.Zapier/Controllers/FormPollingController.cs
@@ -1,13 +1,6 @@
 ﻿using System.Collections.Generic;
-using System.Linq;
-
-using Microsoft.Extensions.Options;
-
-using Umbraco.Forms.Core.Data.Storage;
 using Umbraco.Forms.Integrations.Automation.Zapier.Extensions;
-using Umbraco.Forms.Integrations.Automation.Zapier.Helpers;
 using Umbraco.Forms.Integrations.Automation.Zapier.Services;
-using Umbraco.Forms.Integrations.Automation.Zapier.Configuration;
 
 namespace Umbraco.Forms.Integrations.Automation.Zapier.Controllers
 {
@@ -19,21 +12,10 @@ public class FormPollingController : ZapierFormAuthorizedApiController
     {
         private readonly ZapierFormService _zapierFormService;
 
-        private readonly IRecordStorage _recordStorage;
-
-        private readonly UmbUrlHelper _umbUrlHelper;
-
-        public FormPollingController(IOptions<ZapierSettings> options, ZapierFormService zapierFormService, IRecordStorage recordStorage, 
-            UmbUrlHelper umbUrlHelper,
+        public FormPollingController(
+            ZapierFormService zapierFormService,
             IUserValidationService userValidationService)
-            : base(options, userValidationService)
-        {
-            _zapierFormService = zapierFormService;
-
-            _recordStorage = recordStorage;
-
-            _umbUrlHelper = umbUrlHelper;
-        }
+            : base(userValidationService) => _zapierFormService = zapierFormService;
 
         public List<Dictionary<string, string>> GetFormPropertiesById(string id)
         {
diff --git a/src/Umbraco.Forms.Integrations.Automation.Zapier/Controllers/ZapierFormAuthorizedApiController.cs b/src/Umbraco.Forms.Integrations.Automation.Zapier/Controllers/ZapierFormAuthorizedApiController.cs
@@ -1,25 +1,16 @@
 ﻿using System.Linq;
-
-using Microsoft.Extensions.Options;
-
-using Umbraco.Forms.Integrations.Automation.Zapier.Configuration;
-using Umbraco.Forms.Integrations.Automation.Zapier.Services;
 using Umbraco.Cms.Web.Common.Controllers;
+using Umbraco.Forms.Integrations.Automation.Zapier.Services;
 
 namespace Umbraco.Forms.Integrations.Automation.Zapier.Controllers
 {
     public class ZapierFormAuthorizedApiController : UmbracoApiController
     {
-        private readonly ZapierSettings Options;
-
         private readonly IUserValidationService _userValidationService;
 
-        public ZapierFormAuthorizedApiController(IOptions<ZapierSettings> options, IUserValidationService userValidationService)
-        {
-            Options = options.Value;
-
+        public ZapierFormAuthorizedApiController(
+            IUserValidationService userValidationService) =>
             _userValidationService = userValidationService;
-        }
 
         public bool IsAccessValid()
         {
@@ -39,10 +30,7 @@ public bool IsAccessValid()
 
             if (string.IsNullOrEmpty(apiKey) && (string.IsNullOrEmpty(username) || string.IsNullOrEmpty(password))) return false;
 
-            if (!string.IsNullOrEmpty(apiKey))
-                return apiKey == Options.ApiKey;
-
-            var isAuthorized = _userValidationService.Validate(username, password, Options.ApiKey).GetAwaiter()
+            var isAuthorized = _userValidationService.Validate(username, password, apiKey).GetAwaiter()
                 .GetResult();
             if (!isAuthorized) return false;
 
diff --git a/src/Umbraco.Forms.Integrations.Automation.Zapier/Services/UserValidationService.cs b/src/Umbraco.Forms.Integrations.Automation.Zapier/Services/UserValidationService.cs
@@ -11,36 +11,73 @@ namespace Umbraco.Forms.Integrations.Automation.Zapier.Services
 {
     public class UserValidationService : IUserValidationService
     {
-        private readonly ZapierSettings _zapierSettings;
+        private readonly ZapierCmsSettings _zapierCmsSettings;
+        private readonly ZapierFormsSettings _zapierFormsSettings;
 
         private readonly IUserService _userService;
 
         private readonly IBackOfficeUserManager _backOfficeUserManager;
 
-        public UserValidationService(IOptions<ZapierSettings> options, IUserService userService, IBackOfficeUserManager backOfficeUserManager)
+        public UserValidationService(
+            IOptions<ZapierCmsSettings> cmsOptions, 
+            IOptions<ZapierFormsSettings> formsOptions,
+            IUserService userService, 
+            IBackOfficeUserManager backOfficeUserManager)
         {
-            _zapierSettings = options.Value;
+            _zapierCmsSettings = cmsOptions.Value;
+            _zapierFormsSettings = formsOptions.Value;
 
             _backOfficeUserManager = backOfficeUserManager;
 
             _userService = userService;
         }
 
+        /// <summary>
+        /// Allow access by validating API Key. If API key is missing, validate user credentials.
+        /// </summary>
+        /// <param name="username"></param>
+        /// <param name="password"></param>
+        /// <param name="apiKey"></param>
+        /// <returns></returns>
         public async Task<bool> Validate(string username, string password, string apiKey)
         {
             if (!string.IsNullOrEmpty(apiKey))
-                return apiKey == _zapierSettings.ApiKey;
+            {
+                return ValidateByApiKey(apiKey);
+            }
+
+            return await ValidateByCredentials(username, password);
+        }
+
+        /// <summary>
+        /// Validates user based on provided API key. 
+        /// When both CMS and Forms packages are installed, both settings (CMS/Forms) will be compared.
+        /// </summary>
+        /// <param name="apiKey">Provided API key in the Zap authentication.</param>
+        /// <returns></returns>
+        private bool ValidateByApiKey(string apiKey) =>
+            // Check API key from CMS and Forms settings. 
+            (!string.IsNullOrEmpty(_zapierCmsSettings.ApiKey) && _zapierCmsSettings.ApiKey == apiKey)
+                || (!string.IsNullOrEmpty(_zapierFormsSettings.ApiKey) && _zapierFormsSettings.ApiKey == apiKey);
 
+        /// <summary>
+        /// Validates user based on provided credentials.
+        /// </summary>
+        /// <param name="username"></param>
+        /// <param name="password"></param>
+        /// <returns></returns>
+        private async Task<bool> ValidateByCredentials(string username, string password)
+        {
             var isUserValid =
                 await _backOfficeUserManager.ValidateCredentialsAsync(username, password);
 
             if (!isUserValid) return false;
 
-            if (!string.IsNullOrEmpty(_zapierSettings.UserGroupAlias))
+            if (!string.IsNullOrEmpty(_zapierFormsSettings.UserGroupAlias))
             {
                 var user = _userService.GetByUsername(username);
 
-                return user != null && user.Groups.Any(p => p.Alias == _zapierSettings.UserGroupAlias);
+                return user != null && user.Groups.Any(p => p.Alias == _zapierFormsSettings.UserGroupAlias);
             }
 
             return true;
diff --git a/src/Umbraco.Forms.Integrations.Automation.Zapier/Umbraco.Forms.Integrations.Automation.Zapier.csproj b/src/Umbraco.Forms.Integrations.Automation.Zapier/Umbraco.Forms.Integrations.Automation.Zapier.csproj
@@ -11,7 +11,7 @@
 		<PackageIconUrl></PackageIconUrl>
 		<PackageProjectUrl>https://github.com/umbraco/Umbraco.Forms.Integrations/tree/main-v10/src/Umbraco.Forms.Integrations.Automation.Zapier</PackageProjectUrl>
 		<RepositoryUrl>https://github.com/umbraco/Umbraco.Forms.Integrations</RepositoryUrl>
-		<Version>2.0.5</Version>
+		<Version>2.0.6</Version>
 		<Authors>Umbraco HQ</Authors>
 		<Company>Umbraco</Company>
 		<PackageTags>Umbraco;Umbraco-Marketplace</PackageTags>
diff --git a/src/Umbraco.Forms.Integrations.Automation.Zapier/ZapierFormsComposer.cs b/src/Umbraco.Forms.Integrations.Automation.Zapier/ZapierFormsComposer.cs
@@ -14,8 +14,11 @@ public class ZapierFormsComposer : IComposer
     {
         public void Compose(IUmbracoBuilder builder)
         {
-            var options = builder.Services
-                .AddOptions<ZapierSettings>()
+            builder.Services
+                .AddOptions<ZapierCmsSettings>()
+                .Bind(builder.Config.GetSection(Constants.Configuration.CmsSettings));
+            builder.Services
+                .AddOptions<ZapierFormsSettings>()
                 .Bind(builder.Config.GetSection(Constants.Configuration.Settings));
 
             builder

Original file line number	Diff line number	Diff line change
`@@ -1,6 +1,6 @@`
`1`	`1`	`namespace Umbraco.Forms.Integrations.Automation.Zapier.Configuration`
`2`	`2`	`{`
`3`		`- public class ZapierSettings`
	`3`	`+ public class AppSettings`
`4`	`4`	`{`
`5`	`5`	`public string UserGroupAlias { get; set; }`
`6`	`6`
-Original file line number
+Diff line change
@@ @@ -0,0 +1,6 @@ @@
 +namespace Umbraco.Forms.Integrations.Automation.Zapier.Configuration
 +{
 +    public class ZapierCmsSettings : AppSettings
 +    {
 +    }
 +}
Original file line number	Diff line number	Diff line change
`@@ -14,6 +14,8 @@ public static class ZapierAppConfiguration`
`14`	`14`
`15`	`15`	`public static class Configuration`
`16`	`16`	`{`
	`17`	`+ public const string CmsSettings = "Umbraco:CMS:Integrations:Automation:Zapier:Settings";`
	`18`	`+`
`17`	`19`	`public const string Settings = "Umbraco:Forms:Integrations:Automation:Zapier:Settings";`
`18`	`20`	`}`
`19`	`21`
Original file line number	Diff line number	Diff line change
`@@ -14,8 +14,11 @@ public class ZapierFormsComposer : IComposer`
`14`	`14`	`{`
`15`	`15`	`public void Compose(IUmbracoBuilder builder)`
`16`	`16`	`{`
`17`		`- var options = builder.Services`
`18`		`- .AddOptions<ZapierSettings>()`
	`17`	`+ builder.Services`
	`18`	`+ .AddOptions<ZapierCmsSettings>()`
	`19`	`+ .Bind(builder.Config.GetSection(Constants.Configuration.CmsSettings));`
	`20`	`+ builder.Services`
	`21`	`+ .AddOptions<ZapierFormsSettings>()`
`19`	`22`	`.Bind(builder.Config.GetSection(Constants.Configuration.Settings));`
`20`	`23`
`21`	`24`	`builder`