diff --git a/src/Umbraco.Web.BackOffice/Controllers/AuthenticationController.cs b/src/Umbraco.Web.BackOffice/Controllers/AuthenticationController.cs index 08aa255b89..47772449ec 100644 --- a/src/Umbraco.Web.BackOffice/Controllers/AuthenticationController.cs +++ b/src/Umbraco.Web.BackOffice/Controllers/AuthenticationController.cs @@ -106,7 +106,7 @@ namespace Umbraco.Web.BackOffice.Controllers /// cookies which means that the auth cookie could be valid but the csrf cookies are no longer there, in that case we need to re-set the csrf cookies. /// [UmbracoAuthorize] - [TypeFilter(typeof(SetAngularAntiForgeryTokens))] + [SetAngularAntiForgeryTokens] //[CheckIfUserTicketDataIsStale] // TODO: Migrate this, though it will need to be done differently at the cookie auth level public UserDetail GetCurrentUser() { @@ -123,7 +123,7 @@ namespace Umbraco.Web.BackOffice.Controllers /// Logs a user in /// /// - [TypeFilter(typeof(SetAngularAntiForgeryTokens))] + [SetAngularAntiForgeryTokens] public async Task PostLogin(LoginModel loginModel) { // Sign the user in with username/password, this also gives a chance for developers to @@ -188,7 +188,7 @@ namespace Umbraco.Web.BackOffice.Controllers /// Logs the current user out /// /// - [TypeFilter(typeof(ValidateAngularAntiForgeryTokenAttribute))] + [ValidateAngularAntiForgeryToken] public IActionResult PostLogout() { HttpContext.SignOutAsync(Core.Constants.Security.BackOfficeAuthenticationType); diff --git a/src/Umbraco.Web.BackOffice/Controllers/BackOfficeNotificationsController.cs b/src/Umbraco.Web.BackOffice/Controllers/BackOfficeNotificationsController.cs index 0bea7a257e..82fe9ef928 100644 --- a/src/Umbraco.Web.BackOffice/Controllers/BackOfficeNotificationsController.cs +++ b/src/Umbraco.Web.BackOffice/Controllers/BackOfficeNotificationsController.cs @@ -1,5 +1,4 @@ -using Microsoft.AspNetCore.Mvc; -using Umbraco.Web.WebApi.Filters; +using Umbraco.Web.WebApi.Filters; namespace Umbraco.Web.BackOffice.Controllers { @@ -8,10 +7,9 @@ namespace Umbraco.Web.BackOffice.Controllers /// resulting message is INotificationModel in which case it will append any Event Messages /// currently in the request. /// - [TypeFilter(typeof(AppendCurrentEventMessagesAttribute))] //[PrefixlessBodyModelValidator] // TODO implement this!! + [AppendCurrentEventMessagesAttribute] public abstract class BackOfficeNotificationsController : UmbracoAuthorizedJsonController { - } } diff --git a/src/Umbraco.Web.BackOffice/Controllers/DashboardController.cs b/src/Umbraco.Web.BackOffice/Controllers/DashboardController.cs index 68026d9c89..03bbe132f3 100644 --- a/src/Umbraco.Web.BackOffice/Controllers/DashboardController.cs +++ b/src/Umbraco.Web.BackOffice/Controllers/DashboardController.cs @@ -67,7 +67,7 @@ namespace Umbraco.Web.BackOffice.Controllers private static readonly HttpClient HttpClient = new HttpClient(); //we have baseurl as a param to make previewing easier, so we can test with a dev domain from client side - [TypeFilter(typeof(ValidateAngularAntiForgeryTokenAttribute))] + [ValidateAngularAntiForgeryToken] public async Task GetRemoteDashboardContent(string section, string baseUrl = "https://dashboard.umbraco.org/") { var user = _umbracoContextAccessor.GetRequiredUmbracoContext().Security.CurrentUser; @@ -211,7 +211,7 @@ namespace Umbraco.Web.BackOffice.Controllers } // return IDashboardSlim - we don't need sections nor access rules - [TypeFilter(typeof(ValidateAngularAntiForgeryTokenAttribute))] + [ValidateAngularAntiForgeryToken] [TypeFilter(typeof(OutgoingEditorModelEventAttribute))] public IEnumerable> GetDashboard(string section) { diff --git a/src/Umbraco.Web.BackOffice/Controllers/DataTypeController.cs b/src/Umbraco.Web.BackOffice/Controllers/DataTypeController.cs index 9db0cc2c75..bbbea978ba 100644 --- a/src/Umbraco.Web.BackOffice/Controllers/DataTypeController.cs +++ b/src/Umbraco.Web.BackOffice/Controllers/DataTypeController.cs @@ -265,7 +265,7 @@ namespace Umbraco.Web.BackOffice.Controllers /// /// /// - [TypeFilter(typeof(DataTypeValidateAttribute))] + [DataTypeValidate] public ActionResult PostSave(DataTypeSave dataType) { //If we've made it here, then everything has been wired up and validated by the attribute diff --git a/src/Umbraco.Web.BackOffice/Controllers/UmbracoAuthorizedJsonController.cs b/src/Umbraco.Web.BackOffice/Controllers/UmbracoAuthorizedJsonController.cs index 2c1b106aa3..917aeaef23 100644 --- a/src/Umbraco.Web.BackOffice/Controllers/UmbracoAuthorizedJsonController.cs +++ b/src/Umbraco.Web.BackOffice/Controllers/UmbracoAuthorizedJsonController.cs @@ -1,6 +1,4 @@ -using Microsoft.AspNetCore.Mvc; -using Umbraco.Web.BackOffice.Controllers; -using Umbraco.Web.BackOffice.Filters; +using Umbraco.Web.BackOffice.Filters; using Umbraco.Web.Common.Filters; namespace Umbraco.Web.BackOffice.Controllers @@ -12,10 +10,9 @@ namespace Umbraco.Web.BackOffice.Controllers /// Inheriting from this controller means that ALL of your methods are JSON methods that are called by Angular, /// methods that are not called by Angular or don't contain a valid csrf header will NOT work. /// - [TypeFilter(typeof(ValidateAngularAntiForgeryTokenAttribute))] + [ValidateAngularAntiForgeryToken] [AngularJsonOnlyConfiguration] // TODO: This could be applied with our Application Model conventions public abstract class UmbracoAuthorizedJsonController : UmbracoAuthorizedApiController { - } } diff --git a/src/Umbraco.Web.BackOffice/Filters/AppendCurrentEventMessagesAttribute.cs b/src/Umbraco.Web.BackOffice/Filters/AppendCurrentEventMessagesAttribute.cs index 60a4ae375f..23377c8651 100644 --- a/src/Umbraco.Web.BackOffice/Filters/AppendCurrentEventMessagesAttribute.cs +++ b/src/Umbraco.Web.BackOffice/Filters/AppendCurrentEventMessagesAttribute.cs @@ -12,62 +12,73 @@ namespace Umbraco.Web.WebApi.Filters /// resulting message is INotificationModel in which case it will append any Event Messages /// currently in the request. /// - internal sealed class AppendCurrentEventMessagesAttribute : ActionFilterAttribute + internal sealed class AppendCurrentEventMessagesAttribute : TypeFilterAttribute { - private readonly IUmbracoContextAccessor _umbracoContextAccessor; - private readonly IEventMessagesFactory _eventMessagesFactory; - - public AppendCurrentEventMessagesAttribute(IUmbracoContextAccessor umbracoContextAccessor, IEventMessagesFactory eventMessagesFactory) + public AppendCurrentEventMessagesAttribute() : base(typeof(AppendCurrentEventMessagesFilter)) { - _umbracoContextAccessor = umbracoContextAccessor; - _eventMessagesFactory = eventMessagesFactory; } - public override void OnActionExecuted(ActionExecutedContext context) + private class AppendCurrentEventMessagesFilter : IActionFilter { - if (context.HttpContext.Response == null) return; - if (context.HttpContext.Request.Method.Equals(HttpMethod.Get.ToString(), StringComparison.InvariantCultureIgnoreCase)) return; - var umbracoContext = _umbracoContextAccessor.UmbracoContext; - if (umbracoContext == null) return; + private readonly IUmbracoContextAccessor _umbracoContextAccessor; + private readonly IEventMessagesFactory _eventMessagesFactory; - if(!(context.Result is ObjectResult obj)) return; - - var notifications = obj.Value as INotificationModel; - if (notifications == null) return; - - var msgs = _eventMessagesFactory.GetOrDefault(); - if (msgs == null) return; - - foreach (var eventMessage in msgs.GetAll()) + public AppendCurrentEventMessagesFilter(IUmbracoContextAccessor umbracoContextAccessor, IEventMessagesFactory eventMessagesFactory) { - NotificationStyle msgType; - switch (eventMessage.MessageType) - { - case EventMessageType.Default: - msgType = NotificationStyle.Save; - break; - case EventMessageType.Info: - msgType = NotificationStyle.Info; - break; - case EventMessageType.Error: - msgType = NotificationStyle.Error; - break; - case EventMessageType.Success: - msgType = NotificationStyle.Success; - break; - case EventMessageType.Warning: - msgType = NotificationStyle.Warning; - break; - default: - throw new ArgumentOutOfRangeException(); - } + _umbracoContextAccessor = umbracoContextAccessor; + _eventMessagesFactory = eventMessagesFactory; + } - notifications.Notifications.Add(new BackOfficeNotification + public void OnActionExecuted(ActionExecutedContext context) + { + if (context.HttpContext.Response == null) return; + if (context.HttpContext.Request.Method.Equals(HttpMethod.Get.ToString(), StringComparison.InvariantCultureIgnoreCase)) return; + var umbracoContext = _umbracoContextAccessor.UmbracoContext; + if (umbracoContext == null) return; + + if (!(context.Result is ObjectResult obj)) return; + + var notifications = obj.Value as INotificationModel; + if (notifications == null) return; + + var msgs = _eventMessagesFactory.GetOrDefault(); + if (msgs == null) return; + + foreach (var eventMessage in msgs.GetAll()) { - Message = eventMessage.Message, - Header = eventMessage.Category, - NotificationType = msgType - }); + NotificationStyle msgType; + switch (eventMessage.MessageType) + { + case EventMessageType.Default: + msgType = NotificationStyle.Save; + break; + case EventMessageType.Info: + msgType = NotificationStyle.Info; + break; + case EventMessageType.Error: + msgType = NotificationStyle.Error; + break; + case EventMessageType.Success: + msgType = NotificationStyle.Success; + break; + case EventMessageType.Warning: + msgType = NotificationStyle.Warning; + break; + default: + throw new ArgumentOutOfRangeException(); + } + + notifications.Notifications.Add(new BackOfficeNotification + { + Message = eventMessage.Message, + Header = eventMessage.Category, + NotificationType = msgType + }); + } + } + + public void OnActionExecuting(ActionExecutingContext context) + { } } } diff --git a/src/Umbraco.Web.BackOffice/Filters/DataTypeValidateAttribute.cs b/src/Umbraco.Web.BackOffice/Filters/DataTypeValidateAttribute.cs index 41e928053d..83b2bde0e5 100644 --- a/src/Umbraco.Web.BackOffice/Filters/DataTypeValidateAttribute.cs +++ b/src/Umbraco.Web.BackOffice/Filters/DataTypeValidateAttribute.cs @@ -1,6 +1,7 @@ using System; using System.Linq; using System.Net; +using Microsoft.AspNetCore.Mvc; using Microsoft.AspNetCore.Mvc.Filters; using Umbraco.Core; using Umbraco.Core.Mapping; @@ -15,94 +16,98 @@ using Umbraco.Web.Models.ContentEditing; namespace Umbraco.Web.Editors { /// - /// An action filter that wires up the persisted entity of the DataTypeSave model and validates the whole request + /// An attribute/filter that wires up the persisted entity of the DataTypeSave model and validates the whole request /// - internal sealed class DataTypeValidateAttribute : ActionFilterAttribute + internal sealed class DataTypeValidateAttribute : TypeFilterAttribute { - private readonly IDataTypeService _dataTypeService; - private readonly PropertyEditorCollection _propertyEditorCollection; - private readonly UmbracoMapper _umbracoMapper; - - - /// - /// For use in unit tests. Not possible to use as attribute ctor. - /// - /// - /// - /// - public DataTypeValidateAttribute(IDataTypeService dataTypeService, PropertyEditorCollection propertyEditorCollection, UmbracoMapper umbracoMapper) + public DataTypeValidateAttribute() : base(typeof(DataTypeValidateFilter)) { - _dataTypeService = dataTypeService ?? throw new ArgumentNullException(nameof(dataTypeService)); - _propertyEditorCollection = propertyEditorCollection ?? throw new ArgumentNullException(nameof(propertyEditorCollection)); - _umbracoMapper = umbracoMapper ?? throw new ArgumentNullException(nameof(umbracoMapper)); } - public override void OnActionExecuting(ActionExecutingContext context) + private class DataTypeValidateFilter : IActionFilter { - var dataType = (DataTypeSave) context.ActionArguments["dataType"]; + private readonly IDataTypeService _dataTypeService; + private readonly PropertyEditorCollection _propertyEditorCollection; + private readonly UmbracoMapper _umbracoMapper; - dataType.Name = dataType.Name.CleanForXss('[', ']', '(', ')', ':'); - dataType.Alias = dataType.Alias == null ? dataType.Name : dataType.Alias.CleanForXss('[', ']', '(', ')', ':'); - - // get the property editor, ensuring that it exits - if (!_propertyEditorCollection.TryGet(dataType.EditorAlias, out var propertyEditor)) + public DataTypeValidateFilter(IDataTypeService dataTypeService, PropertyEditorCollection propertyEditorCollection, UmbracoMapper umbracoMapper) { - var message = $"Property editor \"{dataType.EditorAlias}\" was not found."; - context.Result = new UmbracoProblemResult(message, HttpStatusCode.NotFound); - return; + _dataTypeService = dataTypeService ?? throw new ArgumentNullException(nameof(dataTypeService)); + _propertyEditorCollection = propertyEditorCollection ?? throw new ArgumentNullException(nameof(propertyEditorCollection)); + _umbracoMapper = umbracoMapper ?? throw new ArgumentNullException(nameof(umbracoMapper)); } - // assign - dataType.PropertyEditor = propertyEditor; - - // validate that the data type exists, or create one if required - IDataType persisted; - switch (dataType.Action) + public void OnActionExecuted(ActionExecutedContext context) { - case ContentSaveAction.Save: - persisted = _dataTypeService.GetDataType(Convert.ToInt32(dataType.Id)); - if (persisted == null) - { - var message = $"Data type with id {dataType.Id} was not found."; - context.Result = new UmbracoProblemResult(message, HttpStatusCode.NotFound); - return; - } - // map the model to the persisted instance - _umbracoMapper.Map(dataType, persisted); - break; + } - case ContentSaveAction.SaveNew: - // create the persisted model from mapping the saved model - persisted = _umbracoMapper.Map(dataType); - ((DataType) persisted).ResetIdentity(); - break; + public void OnActionExecuting(ActionExecutingContext context) + { + var dataType = (DataTypeSave)context.ActionArguments["dataType"]; - default: - context.Result = new UmbracoProblemResult($"Data type action {dataType.Action} was not found.", HttpStatusCode.NotFound); + dataType.Name = dataType.Name.CleanForXss('[', ']', '(', ')', ':'); + dataType.Alias = dataType.Alias == null ? dataType.Name : dataType.Alias.CleanForXss('[', ']', '(', ')', ':'); + + // get the property editor, ensuring that it exits + if (!_propertyEditorCollection.TryGet(dataType.EditorAlias, out var propertyEditor)) + { + var message = $"Property editor \"{dataType.EditorAlias}\" was not found."; + context.Result = new UmbracoProblemResult(message, HttpStatusCode.NotFound); return; - } + } - // assign (so it's available in the action) - dataType.PersistedDataType = persisted; + // assign + dataType.PropertyEditor = propertyEditor; - // validate the configuration - // which is posted as a set of fields with key (string) and value (object) - var configurationEditor = propertyEditor.GetConfigurationEditor(); - foreach (var field in dataType.ConfigurationFields) - { - var editorField = configurationEditor.Fields.SingleOrDefault(x => x.Key == field.Key); - if (editorField == null) continue; + // validate that the data type exists, or create one if required + IDataType persisted; + switch (dataType.Action) + { + case ContentSaveAction.Save: + persisted = _dataTypeService.GetDataType(Convert.ToInt32(dataType.Id)); + if (persisted == null) + { + var message = $"Data type with id {dataType.Id} was not found."; + context.Result = new UmbracoProblemResult(message, HttpStatusCode.NotFound); + return; + } + // map the model to the persisted instance + _umbracoMapper.Map(dataType, persisted); + break; - // run each IValueValidator (with null valueType and dataTypeConfiguration: not relevant here) - foreach (var validator in editorField.Validators) - foreach (var result in validator.Validate(field.Value, null, null)) - context.ModelState.AddValidationError(result, "Properties", field.Key); - } + case ContentSaveAction.SaveNew: + // create the persisted model from mapping the saved model + persisted = _umbracoMapper.Map(dataType); + ((DataType)persisted).ResetIdentity(); + break; - if (context.ModelState.IsValid == false) - { - // if it is not valid, do not continue and return the model state - throw HttpResponseException.CreateValidationErrorResponse(context.ModelState); + default: + context.Result = new UmbracoProblemResult($"Data type action {dataType.Action} was not found.", HttpStatusCode.NotFound); + return; + } + + // assign (so it's available in the action) + dataType.PersistedDataType = persisted; + + // validate the configuration + // which is posted as a set of fields with key (string) and value (object) + var configurationEditor = propertyEditor.GetConfigurationEditor(); + foreach (var field in dataType.ConfigurationFields) + { + var editorField = configurationEditor.Fields.SingleOrDefault(x => x.Key == field.Key); + if (editorField == null) continue; + + // run each IValueValidator (with null valueType and dataTypeConfiguration: not relevant here) + foreach (var validator in editorField.Validators) + foreach (var result in validator.Validate(field.Value, null, null)) + context.ModelState.AddValidationError(result, "Properties", field.Key); + } + + if (context.ModelState.IsValid == false) + { + // if it is not valid, do not continue and return the model state + throw HttpResponseException.CreateValidationErrorResponse(context.ModelState); + } } } } diff --git a/src/Umbraco.Web.BackOffice/Filters/SetAngularAntiForgeryTokens.cs b/src/Umbraco.Web.BackOffice/Filters/SetAngularAntiForgeryTokens.cs deleted file mode 100644 index 77804e3801..0000000000 --- a/src/Umbraco.Web.BackOffice/Filters/SetAngularAntiForgeryTokens.cs +++ /dev/null @@ -1,78 +0,0 @@ -using Microsoft.AspNetCore.Antiforgery; -using Microsoft.AspNetCore.Mvc.Filters; -using System.Net; -using System.Text; -using System.Threading.Tasks; -using Umbraco.Core; -using Umbraco.Core.Configuration; -using Umbraco.Web.BackOffice.Security; - -namespace Umbraco.Extensions -{ - - /// - /// A filter to set the csrf cookie token based on angular conventions - /// - public sealed class SetAngularAntiForgeryTokens : IAsyncActionFilter - { - private readonly IBackOfficeAntiforgery _antiforgery; - private readonly IGlobalSettings _globalSettings; - - public SetAngularAntiForgeryTokens(IBackOfficeAntiforgery antiforgery, IGlobalSettings globalSettings) - { - _antiforgery = antiforgery; - _globalSettings = globalSettings; - } - - public async Task OnActionExecutionAsync(ActionExecutingContext context, ActionExecutionDelegate next) - { - if (context.HttpContext.Response != null) - { - //DO not set the token cookies if the request has failed!! - if (context.HttpContext.Response.StatusCode == (int)HttpStatusCode.OK) - { - //don't need to set the cookie if they already exist and they are valid - if (context.HttpContext.Request.Cookies.TryGetValue(Constants.Web.AngularCookieName, out var angularCookieVal) - && context.HttpContext.Request.Cookies.TryGetValue(Constants.Web.CsrfValidationCookieName, out var csrfCookieVal)) - { - //if they are not valid for some strange reason - we need to continue setting valid ones - var valResult = await _antiforgery.ValidateRequestAsync(context.HttpContext); - if (valResult.Success) - { - await next(); - return; - } - } - - string cookieToken, headerToken; - _antiforgery.GetTokens(context.HttpContext, out cookieToken, out headerToken); - - //We need to set 2 cookies: one is the cookie value that angular will use to set a header value on each request, - // the 2nd is the validation value generated by the anti-forgery helper that we use to validate the header token against. - - context.HttpContext.Response.Cookies.Append( - Constants.Web.AngularCookieName, headerToken, - new Microsoft.AspNetCore.Http.CookieOptions - { - Path = "/", - //must be js readable - HttpOnly = false, - Secure = _globalSettings.UseHttps - }); - - context.HttpContext.Response.Cookies.Append( - Constants.Web.CsrfValidationCookieName, cookieToken, - new Microsoft.AspNetCore.Http.CookieOptions - { - Path = "/", - HttpOnly = true, - Secure = _globalSettings.UseHttps - }); - } - } - - await next(); - } - - } -} diff --git a/src/Umbraco.Web.BackOffice/Filters/SetAngularAntiForgeryTokensAttribute.cs b/src/Umbraco.Web.BackOffice/Filters/SetAngularAntiForgeryTokensAttribute.cs new file mode 100644 index 0000000000..e52287b57e --- /dev/null +++ b/src/Umbraco.Web.BackOffice/Filters/SetAngularAntiForgeryTokensAttribute.cs @@ -0,0 +1,83 @@ +using System.Net; +using System.Threading.Tasks; +using Microsoft.AspNetCore.Mvc; +using Microsoft.AspNetCore.Mvc.Filters; +using Umbraco.Core; +using Umbraco.Core.Configuration; +using Umbraco.Web.BackOffice.Security; + +namespace Umbraco.Extensions +{ + /// + /// An attribute/filter to set the csrf cookie token based on angular conventions + /// + public class SetAngularAntiForgeryTokensAttribute : TypeFilterAttribute + { + public SetAngularAntiForgeryTokensAttribute() : base(typeof(SetAngularAntiForgeryTokensFilter)) + { + } + + private class SetAngularAntiForgeryTokensFilter : IAsyncActionFilter + { + private readonly IBackOfficeAntiforgery _antiforgery; + private readonly IGlobalSettings _globalSettings; + + public SetAngularAntiForgeryTokensFilter(IBackOfficeAntiforgery antiforgery, IGlobalSettings globalSettings) + { + _antiforgery = antiforgery; + _globalSettings = globalSettings; + } + + public async Task OnActionExecutionAsync(ActionExecutingContext context, ActionExecutionDelegate next) + { + if (context.HttpContext.Response != null) + { + //DO not set the token cookies if the request has failed!! + if (context.HttpContext.Response.StatusCode == (int)HttpStatusCode.OK) + { + //don't need to set the cookie if they already exist and they are valid + if (context.HttpContext.Request.Cookies.TryGetValue(Constants.Web.AngularCookieName, out var angularCookieVal) + && context.HttpContext.Request.Cookies.TryGetValue(Constants.Web.CsrfValidationCookieName, out var csrfCookieVal)) + { + //if they are not valid for some strange reason - we need to continue setting valid ones + var valResult = await _antiforgery.ValidateRequestAsync(context.HttpContext); + if (valResult.Success) + { + await next(); + return; + } + } + + string cookieToken, headerToken; + _antiforgery.GetTokens(context.HttpContext, out cookieToken, out headerToken); + + //We need to set 2 cookies: one is the cookie value that angular will use to set a header value on each request, + // the 2nd is the validation value generated by the anti-forgery helper that we use to validate the header token against. + + context.HttpContext.Response.Cookies.Append( + Constants.Web.AngularCookieName, headerToken, + new Microsoft.AspNetCore.Http.CookieOptions + { + Path = "/", + //must be js readable + HttpOnly = false, + Secure = _globalSettings.UseHttps + }); + + context.HttpContext.Response.Cookies.Append( + Constants.Web.CsrfValidationCookieName, cookieToken, + new Microsoft.AspNetCore.Http.CookieOptions + { + Path = "/", + HttpOnly = true, + Secure = _globalSettings.UseHttps + }); + } + } + + await next(); + } + + } + } +} diff --git a/src/Umbraco.Web.BackOffice/Filters/ValidateAngularAntiForgeryTokenAttribute.cs b/src/Umbraco.Web.BackOffice/Filters/ValidateAngularAntiForgeryTokenAttribute.cs index 4558cf348c..acafacff47 100644 --- a/src/Umbraco.Web.BackOffice/Filters/ValidateAngularAntiForgeryTokenAttribute.cs +++ b/src/Umbraco.Web.BackOffice/Filters/ValidateAngularAntiForgeryTokenAttribute.cs @@ -1,11 +1,9 @@ -using System; -using System.Linq; +using System.Linq; using System.Net; using System.Security.Claims; using System.Threading.Tasks; using Microsoft.AspNetCore.Antiforgery; using Microsoft.AspNetCore.Http; -using Microsoft.AspNetCore.Http.Features; using Microsoft.AspNetCore.Mvc; using Microsoft.AspNetCore.Mvc.Filters; using Umbraco.Core; @@ -16,41 +14,44 @@ using Umbraco.Web.BackOffice.Security; namespace Umbraco.Web.BackOffice.Filters { /// - /// A filter to check for the csrf token based on Angular's standard approach + /// An attribute/filter to check for the csrf token based on Angular's standard approach /// /// /// Code derived from http://ericpanorel.net/2013/07/28/spa-authentication-and-csrf-mvc4-antiforgery-implementation/ /// /// If the authentication type is cookie based, then this filter will execute, otherwise it will be disabled /// - public sealed class ValidateAngularAntiForgeryTokenAttribute : ActionFilterAttribute + public sealed class ValidateAngularAntiForgeryTokenAttribute : TypeFilterAttribute { - // TODO: Either make this inherit from TypeFilter or make this just a normal IActionFilter - - private readonly ILogger _logger; - private readonly IBackOfficeAntiforgery _antiforgery; - private readonly ICookieManager _cookieManager; - - public ValidateAngularAntiForgeryTokenAttribute(ILogger logger, IBackOfficeAntiforgery antiforgery, ICookieManager cookieManager) + public ValidateAngularAntiForgeryTokenAttribute() : base(typeof(ValidateAngularAntiForgeryTokenFilter)) { - _logger = logger; - _antiforgery = antiforgery; - _cookieManager = cookieManager; } - public override async Task OnActionExecutionAsync(ActionExecutingContext context, ActionExecutionDelegate next) + private class ValidateAngularAntiForgeryTokenFilter : IAsyncActionFilter { - if (context.Controller is ControllerBase controller && controller.User.Identity is ClaimsIdentity userIdentity) + private readonly ILogger _logger; + private readonly IBackOfficeAntiforgery _antiforgery; + private readonly ICookieManager _cookieManager; + + public ValidateAngularAntiForgeryTokenFilter(ILogger logger, IBackOfficeAntiforgery antiforgery, ICookieManager cookieManager) { - //if there is not CookiePath claim, then exit - if (userIdentity.HasClaim(x => x.Type == ClaimTypes.CookiePath) == false) - { - await base.OnActionExecutionAsync(context, next); - return; - } + _logger = logger; + _antiforgery = antiforgery; + _cookieManager = cookieManager; } - var cookieToken = _cookieManager.GetCookieValue(Constants.Web.CsrfValidationCookieName); - var httpContext = context.HttpContext; + + public async Task OnActionExecutionAsync(ActionExecutingContext context, ActionExecutionDelegate next) + { + if (context.Controller is ControllerBase controller && controller.User.Identity is ClaimsIdentity userIdentity) + { + //if there is not CookiePath claim, then exit + if (userIdentity.HasClaim(x => x.Type == ClaimTypes.CookiePath) == false) + { + await next(); + } + } + var cookieToken = _cookieManager.GetCookieValue(Constants.Web.CsrfValidationCookieName); + var httpContext = context.HttpContext; var validateResult = await ValidateHeaders(httpContext, cookieToken); if (validateResult.Item1 == false) @@ -60,51 +61,52 @@ namespace Umbraco.Web.BackOffice.Filters return; } - await next(); - } - - private async Task<(bool,string)> ValidateHeaders( - HttpContext httpContext, - string cookieToken) - { - var requestHeaders = httpContext.Request.Headers; - if (requestHeaders.Any(z => z.Key.InvariantEquals(Constants.Web.AngularHeadername)) == false) - { - return (false, "Missing token"); + await next(); } - var headerToken = requestHeaders - .Where(z => z.Key.InvariantEquals(Constants.Web.AngularHeadername)) - .Select(z => z.Value) - .SelectMany(z => z) - .FirstOrDefault(); - - // both header and cookie must be there - if (cookieToken == null || headerToken == null) + private async Task<(bool, string)> ValidateHeaders( + HttpContext httpContext, + string cookieToken) { - return (false, "Missing token null"); + var requestHeaders = httpContext.Request.Headers; + if (requestHeaders.Any(z => z.Key.InvariantEquals(Constants.Web.AngularHeadername)) == false) + { + return (false, "Missing token"); + } + + var headerToken = requestHeaders + .Where(z => z.Key.InvariantEquals(Constants.Web.AngularHeadername)) + .Select(z => z.Value) + .SelectMany(z => z) + .FirstOrDefault(); + + // both header and cookie must be there + if (cookieToken == null || headerToken == null) + { + return (false, "Missing token null"); + } + + if (await ValidateTokens(httpContext) == false) + { + return (false, "Invalid token"); + } + + return (true, "Success"); } - if (await ValidateTokens(httpContext) == false) + private async Task ValidateTokens(HttpContext httpContext) { - return (false, "Invalid token"); - } - - return (true, "Success"); - } - - private async Task ValidateTokens(HttpContext httpContext) - { - // ensure that the cookie matches the header and then ensure it matches the correct value! - try - { - await _antiforgery.ValidateRequestAsync(httpContext); - return true; - } - catch (AntiforgeryValidationException ex) - { - _logger.Error(ex, "Could not validate XSRF token"); - return false; + // ensure that the cookie matches the header and then ensure it matches the correct value! + try + { + await _antiforgery.ValidateRequestAsync(httpContext); + return true; + } + catch (AntiforgeryValidationException ex) + { + _logger.Error(ex, "Could not validate XSRF token"); + return false; + } } } } diff --git a/src/Umbraco.Web.Common/Controllers/UmbracoApiControllerBase.cs b/src/Umbraco.Web.Common/Controllers/UmbracoApiControllerBase.cs index 5a38d6c0ab..787da05ca4 100644 --- a/src/Umbraco.Web.Common/Controllers/UmbracoApiControllerBase.cs +++ b/src/Umbraco.Web.Common/Controllers/UmbracoApiControllerBase.cs @@ -1,9 +1,8 @@ using Microsoft.AspNetCore.Mvc; -using Microsoft.AspNetCore.Mvc.ModelBinding; +using Umbraco.Web.Common.Attributes; using Umbraco.Web.Common.Filters; using Umbraco.Web.Features; using Umbraco.Web.WebApi.Filters; -using Umbraco.Web.Common.Attributes; namespace Umbraco.Web.Common.Controllers { diff --git a/src/Umbraco.Web.Common/Filters/AngularJsonOnlyConfigurationAttribute.cs b/src/Umbraco.Web.Common/Filters/AngularJsonOnlyConfigurationAttribute.cs index 0eabbf0f54..85eb55b6d9 100644 --- a/src/Umbraco.Web.Common/Filters/AngularJsonOnlyConfigurationAttribute.cs +++ b/src/Umbraco.Web.Common/Filters/AngularJsonOnlyConfigurationAttribute.cs @@ -1,5 +1,4 @@ - -using System.Buffers; +using System.Buffers; using Microsoft.AspNetCore.Mvc; using Microsoft.AspNetCore.Mvc.Filters; using Microsoft.Extensions.Options; @@ -16,7 +15,7 @@ namespace Umbraco.Web.Common.Filters { } - private class AngularJsonOnlyConfigurationFilter : IResultFilter + private class AngularJsonOnlyConfigurationFilter : IResultFilter { private readonly IOptions _mvcNewtonsoftJsonOptions; private readonly ArrayPool _arrayPool; @@ -31,7 +30,6 @@ namespace Umbraco.Web.Common.Filters public void OnResultExecuted(ResultExecutedContext context) { - } public void OnResultExecuting(ResultExecutingContext context) diff --git a/src/Umbraco.Web.Common/Install/InstallApiController.cs b/src/Umbraco.Web.Common/Install/InstallApiController.cs index 1f41d34123..edc73ef293 100644 --- a/src/Umbraco.Web.Common/Install/InstallApiController.cs +++ b/src/Umbraco.Web.Common/Install/InstallApiController.cs @@ -4,7 +4,6 @@ using System.Linq; using System.Reflection; using System.Threading.Tasks; using Microsoft.AspNetCore.Mvc; -using Microsoft.AspNetCore.Mvc.ModelBinding; using Newtonsoft.Json.Linq; using Umbraco.Core; using Umbraco.Core.Logging; @@ -13,7 +12,6 @@ using Umbraco.Net; using Umbraco.Web.Common.Attributes; using Umbraco.Web.Common.Exceptions; using Umbraco.Web.Common.Filters; -using Umbraco.Web.Common.ModelBinding; using Umbraco.Web.Common.Security; using Umbraco.Web.Install; using Umbraco.Web.Install.Models;