From 313a2e6f7cb5beda126c083315c00956aef0d3e6 Mon Sep 17 00:00:00 2001 From: Shannon Date: Fri, 27 Nov 2020 13:32:41 +1100 Subject: [PATCH] Ensures that all back office controllers are authenticated under the back office scheme --- .../Controllers/AuthenticationController.cs | 2 +- .../Controllers/BackOfficeController.cs | 1 + .../PublishedSnapshotCacheStatusController.cs | 3 +-- .../Extensions/UmbracoBuilderExtensions.cs | 10 ++------ .../IBackOfficeExternalLoginProviders.cs | 11 +++++--- ...uthenticateAsBackOfficeSchemeConvention.cs | 16 ++++++++++++ .../BackOfficeApplicationModelProvider.cs | 14 +++++------ .../BackOfficeIdentityCultureConvention.cs | 3 +++ ...racoApiBehaviorApplicationModelProvider.cs | 3 ++- .../UmbracoJsonModelBinderConvention.cs | 5 ++++ .../EnsureUmbracoBackOfficeAuthentication.cs | 25 +++++++++++++++++++ .../UmbracoBackOfficeAuthorizeAttribute.cs | 3 ++- .../UmbracoBackOfficeAuthorizeFilter.cs | 2 ++ 13 files changed, 74 insertions(+), 24 deletions(-) create mode 100644 src/Umbraco.Web.Common/ApplicationModels/AuthenticateAsBackOfficeSchemeConvention.cs create mode 100644 src/Umbraco.Web.Common/Filters/EnsureUmbracoBackOfficeAuthentication.cs diff --git a/src/Umbraco.Web.BackOffice/Controllers/AuthenticationController.cs b/src/Umbraco.Web.BackOffice/Controllers/AuthenticationController.cs index 9d76e58982..bf1271c910 100644 --- a/src/Umbraco.Web.BackOffice/Controllers/AuthenticationController.cs +++ b/src/Umbraco.Web.BackOffice/Controllers/AuthenticationController.cs @@ -45,7 +45,7 @@ namespace Umbraco.Web.BackOffice.Controllers [PluginController(Constants.Web.Mvc.BackOfficeApiArea)] // TODO: Maybe this could be applied with our Application Model conventions //[ValidationFilter] // TODO: I don't actually think this is required with our custom Application Model conventions applied [AngularJsonOnlyConfiguration] // TODO: This could be applied with our Application Model conventions - [IsBackOffice] // TODO: This could be applied with our Application Model conventions + [IsBackOffice] public class AuthenticationController : UmbracoApiControllerBase { private readonly IBackOfficeSecurityAccessor _backofficeSecurityAccessor; diff --git a/src/Umbraco.Web.BackOffice/Controllers/BackOfficeController.cs b/src/Umbraco.Web.BackOffice/Controllers/BackOfficeController.cs index 83e94c1e30..3998a65cd6 100644 --- a/src/Umbraco.Web.BackOffice/Controllers/BackOfficeController.cs +++ b/src/Umbraco.Web.BackOffice/Controllers/BackOfficeController.cs @@ -41,6 +41,7 @@ namespace Umbraco.Web.BackOffice.Controllers [DisableBrowserCache] //[UmbracoRequireHttps] //TODO Reintroduce [PluginController(Constants.Web.Mvc.BackOfficeArea)] + [IsBackOffice] public class BackOfficeController : UmbracoController { private readonly IBackOfficeUserManager _userManager; diff --git a/src/Umbraco.Web.BackOffice/Controllers/PublishedSnapshotCacheStatusController.cs b/src/Umbraco.Web.BackOffice/Controllers/PublishedSnapshotCacheStatusController.cs index 1a0e3457ca..c9c420f254 100644 --- a/src/Umbraco.Web.BackOffice/Controllers/PublishedSnapshotCacheStatusController.cs +++ b/src/Umbraco.Web.BackOffice/Controllers/PublishedSnapshotCacheStatusController.cs @@ -8,8 +8,7 @@ using Umbraco.Web.PublishedCache; namespace Umbraco.Web.BackOffice.Controllers { - [PluginController(Constants.Web.Mvc.BackOfficeApiArea)] - [IsBackOffice] + [PluginController(Constants.Web.Mvc.BackOfficeApiArea)] public class PublishedSnapshotCacheStatusController : UmbracoAuthorizedApiController { private readonly IPublishedSnapshotService _publishedSnapshotService; diff --git a/src/Umbraco.Web.BackOffice/Extensions/UmbracoBuilderExtensions.cs b/src/Umbraco.Web.BackOffice/Extensions/UmbracoBuilderExtensions.cs index 75d53c91de..fe6eacbb35 100644 --- a/src/Umbraco.Web.BackOffice/Extensions/UmbracoBuilderExtensions.cs +++ b/src/Umbraco.Web.BackOffice/Extensions/UmbracoBuilderExtensions.cs @@ -32,15 +32,9 @@ namespace Umbraco.Extensions builder.Services.AddAntiforgery(); builder.Services.AddSingleton(); - // TODO: We need to see if we are 'allowed' to do this, the docs say: - // "The call to AddIdentity configures the default scheme settings. The AddAuthentication(String) overload sets the DefaultScheme property. The AddAuthentication(Action) overload allows configuring authentication options, which can be used to set up default authentication schemes for different purposes. Subsequent calls to AddAuthentication override previously configured AuthenticationOptions properties." - // So if someone calls services.AddAuthentication() ... in Startup does that overwrite all of this? - // It also says "When the app requires multiple providers, chain the provider extension methods behind AddAuthentication" - // Which leads me to believe it all gets overwritten? :/ - // UPDATE: I have tested this breifly in Startup doing Services.AddAuthentication().AddGoogle() ... and the back office auth - // still seems to work. We'll see how it goes i guess. builder.Services - .AddAuthentication(Core.Constants.Security.BackOfficeAuthenticationType) + .AddAuthentication() // This just creates a builder, nothing more + // Add our custom schemes which are cookie handlers .AddCookie(Core.Constants.Security.BackOfficeAuthenticationType) .AddCookie(Core.Constants.Security.BackOfficeExternalAuthenticationType, o => { diff --git a/src/Umbraco.Web.BackOffice/Security/IBackOfficeExternalLoginProviders.cs b/src/Umbraco.Web.BackOffice/Security/IBackOfficeExternalLoginProviders.cs index 6d0b64e84f..6b78e58ead 100644 --- a/src/Umbraco.Web.BackOffice/Security/IBackOfficeExternalLoginProviders.cs +++ b/src/Umbraco.Web.BackOffice/Security/IBackOfficeExternalLoginProviders.cs @@ -26,6 +26,11 @@ namespace Umbraco.Web.BackOffice.Security _loginProviderOptions = loginProviderOptions; } + public string SchemeForBackOffice(string scheme) + { + return Constants.Security.BackOfficeExternalAuthenticationTypePrefix + scheme; + } + /// /// Overridden to track the final authenticationScheme being registered for the external login /// @@ -36,11 +41,11 @@ namespace Umbraco.Web.BackOffice.Security /// /// public override AuthenticationBuilder AddRemoteScheme(string authenticationScheme, string displayName, Action configureOptions) - { - //Ensure the prefix is set + { + // Validate that the prefix is set if (!authenticationScheme.StartsWith(Constants.Security.BackOfficeExternalAuthenticationTypePrefix)) { - authenticationScheme = Constants.Security.BackOfficeExternalAuthenticationTypePrefix + authenticationScheme; + throw new InvalidOperationException($"The {nameof(authenticationScheme)} is not prefixed with {Constants.Security.BackOfficeExternalAuthenticationTypePrefix}. The scheme must be created with a call to the method {nameof(SchemeForBackOffice)}"); } // add our login provider to the container along with a custom options configuration diff --git a/src/Umbraco.Web.Common/ApplicationModels/AuthenticateAsBackOfficeSchemeConvention.cs b/src/Umbraco.Web.Common/ApplicationModels/AuthenticateAsBackOfficeSchemeConvention.cs new file mode 100644 index 0000000000..838cc59ac4 --- /dev/null +++ b/src/Umbraco.Web.Common/ApplicationModels/AuthenticateAsBackOfficeSchemeConvention.cs @@ -0,0 +1,16 @@ +using Microsoft.AspNetCore.Mvc.ApplicationModels; +using Umbraco.Web.Common.Filters; + +namespace Umbraco.Web.Common.ApplicationModels +{ + /// + /// Ensures all requests with this convention are authenticated with the back office scheme + /// + public class AuthenticateAsBackOfficeSchemeConvention : IActionModelConvention + { + public void Apply(ActionModel action) + { + action.Filters.Add(new EnsureUmbracoBackOfficeAuthentication()); + } + } +} diff --git a/src/Umbraco.Web.Common/ApplicationModels/BackOfficeApplicationModelProvider.cs b/src/Umbraco.Web.Common/ApplicationModels/BackOfficeApplicationModelProvider.cs index 0ad6c4ec1a..dc0816e1e2 100644 --- a/src/Umbraco.Web.Common/ApplicationModels/BackOfficeApplicationModelProvider.cs +++ b/src/Umbraco.Web.Common/ApplicationModels/BackOfficeApplicationModelProvider.cs @@ -6,6 +6,8 @@ using Umbraco.Web.Common.Attributes; namespace Umbraco.Web.Common.ApplicationModels { + // TODO: This should just exist in the back office project + /// /// An application model provider for all Umbraco Back Office controllers /// @@ -15,7 +17,8 @@ namespace Umbraco.Web.Common.ApplicationModels { ActionModelConventions = new List() { - new BackOfficeIdentityCultureConvention() + new BackOfficeIdentityCultureConvention(), + new AuthenticateAsBackOfficeSchemeConvention() }; } @@ -49,12 +52,7 @@ namespace Umbraco.Web.Common.ApplicationModels } private bool IsBackOfficeController(ControllerModel controller) - { - var pluginControllerAttribute = controller.Attributes.OfType().FirstOrDefault(); - return pluginControllerAttribute != null - && (pluginControllerAttribute.AreaName == Core.Constants.Web.Mvc.BackOfficeArea - || pluginControllerAttribute.AreaName == Core.Constants.Web.Mvc.BackOfficeApiArea - || pluginControllerAttribute.AreaName == Core.Constants.Web.Mvc.BackOfficeTreeArea); - } + => controller.Attributes.OfType().Any(); + } } diff --git a/src/Umbraco.Web.Common/ApplicationModels/BackOfficeIdentityCultureConvention.cs b/src/Umbraco.Web.Common/ApplicationModels/BackOfficeIdentityCultureConvention.cs index d3e2096dd3..0a5a1f9945 100644 --- a/src/Umbraco.Web.Common/ApplicationModels/BackOfficeIdentityCultureConvention.cs +++ b/src/Umbraco.Web.Common/ApplicationModels/BackOfficeIdentityCultureConvention.cs @@ -3,6 +3,9 @@ using Umbraco.Web.Common.Filters; namespace Umbraco.Web.Common.ApplicationModels { + + // TODO: This should just exist in the back office project + public class BackOfficeIdentityCultureConvention : IActionModelConvention { public void Apply(ActionModel action) diff --git a/src/Umbraco.Web.Common/ApplicationModels/UmbracoApiBehaviorApplicationModelProvider.cs b/src/Umbraco.Web.Common/ApplicationModels/UmbracoApiBehaviorApplicationModelProvider.cs index 918bc3776f..be296969e7 100644 --- a/src/Umbraco.Web.Common/ApplicationModels/UmbracoApiBehaviorApplicationModelProvider.cs +++ b/src/Umbraco.Web.Common/ApplicationModels/UmbracoApiBehaviorApplicationModelProvider.cs @@ -81,6 +81,7 @@ namespace Umbraco.Web.Common.ApplicationModels } } - private bool IsUmbracoApiController(ControllerModel controller) => controller.Attributes.OfType().Any(); + private bool IsUmbracoApiController(ControllerModel controller) + => controller.Attributes.OfType().Any(); } } diff --git a/src/Umbraco.Web.Common/ApplicationModels/UmbracoJsonModelBinderConvention.cs b/src/Umbraco.Web.Common/ApplicationModels/UmbracoJsonModelBinderConvention.cs index 96c60398f0..42d23b33b3 100644 --- a/src/Umbraco.Web.Common/ApplicationModels/UmbracoJsonModelBinderConvention.cs +++ b/src/Umbraco.Web.Common/ApplicationModels/UmbracoJsonModelBinderConvention.cs @@ -2,6 +2,9 @@ using Microsoft.AspNetCore.Mvc.ModelBinding; using Umbraco.Web.Common.ModelBinding; using System.Linq; +using Umbraco.Web.Common.Attributes; +using Umbraco.Web.Actions; +using Umbraco.Web.Common.Filters; namespace Umbraco.Web.Common.ApplicationModels { @@ -21,4 +24,6 @@ namespace Umbraco.Web.Common.ApplicationModels } } } + + } diff --git a/src/Umbraco.Web.Common/Filters/EnsureUmbracoBackOfficeAuthentication.cs b/src/Umbraco.Web.Common/Filters/EnsureUmbracoBackOfficeAuthentication.cs new file mode 100644 index 0000000000..5ad43dc922 --- /dev/null +++ b/src/Umbraco.Web.Common/Filters/EnsureUmbracoBackOfficeAuthentication.cs @@ -0,0 +1,25 @@ +using Microsoft.AspNetCore.Authorization; +using Microsoft.AspNetCore.Mvc.Filters; +using Umbraco.Web.Common.ApplicationModels; +using Umbraco.Web.Common.Attributes; + +namespace Umbraco.Web.Common.Filters +{ + + /// + /// Assigned as part of the umbraco back office application model + /// to always ensure that back office authentication occurs for all controller/actions with + /// applied. + /// + public class EnsureUmbracoBackOfficeAuthentication : IAuthorizationFilter, IAuthorizeData + { + // Implements IAuthorizeData only to return the back office scheme + public string AuthenticationSchemes { get; set; } = Umbraco.Core.Constants.Security.BackOfficeAuthenticationType; + public string Policy { get; set; } + public string Roles { get; set; } + + public void OnAuthorization(AuthorizationFilterContext context) + { + } + } +} diff --git a/src/Umbraco.Web.Common/Filters/UmbracoBackOfficeAuthorizeAttribute.cs b/src/Umbraco.Web.Common/Filters/UmbracoBackOfficeAuthorizeAttribute.cs index 1f4abbaa25..40f534f289 100644 --- a/src/Umbraco.Web.Common/Filters/UmbracoBackOfficeAuthorizeAttribute.cs +++ b/src/Umbraco.Web.Common/Filters/UmbracoBackOfficeAuthorizeAttribute.cs @@ -1,4 +1,5 @@ -using Microsoft.AspNetCore.Mvc; +using Microsoft.AspNetCore.Authorization; +using Microsoft.AspNetCore.Mvc; namespace Umbraco.Web.Common.Filters { diff --git a/src/Umbraco.Web.Common/Filters/UmbracoBackOfficeAuthorizeFilter.cs b/src/Umbraco.Web.Common/Filters/UmbracoBackOfficeAuthorizeFilter.cs index 8fad886f27..c8ae0aacd8 100644 --- a/src/Umbraco.Web.Common/Filters/UmbracoBackOfficeAuthorizeFilter.cs +++ b/src/Umbraco.Web.Common/Filters/UmbracoBackOfficeAuthorizeFilter.cs @@ -12,6 +12,8 @@ using IHostingEnvironment = Umbraco.Core.Hosting.IHostingEnvironment; namespace Umbraco.Web.Common.Filters { + + /// /// Ensures authorization is successful for a back office user. ///