From e24b22f6bd1a06ba79b9420c51a38a18afef6f7c Mon Sep 17 00:00:00 2001 From: Shannon Date: Fri, 15 Mar 2019 12:20:24 +1100 Subject: [PATCH 1/2] Fixes the authorization for certain endpoints by non admins so that data cannot be seen and avatars cannot be changed --- src/Umbraco.Web/Editors/UsersController.cs | 3 ++ src/Umbraco.Web/Umbraco.Web.csproj | 1 + .../Filters/AdminUsersAuthorizeAttribute.cs | 42 +++++++++++++++++++ .../UmbracoApplicationAuthorizeAttribute.cs | 3 +- 4 files changed, 47 insertions(+), 2 deletions(-) create mode 100644 src/Umbraco.Web/WebApi/Filters/AdminUsersAuthorizeAttribute.cs diff --git a/src/Umbraco.Web/Editors/UsersController.cs b/src/Umbraco.Web/Editors/UsersController.cs index dbfcf64ce2..932f10f343 100644 --- a/src/Umbraco.Web/Editors/UsersController.cs +++ b/src/Umbraco.Web/Editors/UsersController.cs @@ -82,6 +82,7 @@ namespace Umbraco.Web.Editors [AppendUserModifiedHeader("id")] [FileUploadCleanupFilter(false)] + [AdminUsersAuthorize] public async Task PostSetAvatar(int id) { return await PostSetAvatarInternal(Request, Services.UserService, ApplicationContext.ApplicationCache.StaticCache, id); @@ -145,6 +146,7 @@ namespace Umbraco.Web.Editors } [AppendUserModifiedHeader("id")] + [AdminUsersAuthorize] public HttpResponseMessage PostClearAvatar(int id) { var found = Services.UserService.GetUserById(id); @@ -183,6 +185,7 @@ namespace Umbraco.Web.Editors /// /// [OutgoingEditorModelEvent] + [AdminUsersAuthorize] public UserDisplay GetById(int id) { var user = Services.UserService.GetUserById(id); diff --git a/src/Umbraco.Web/Umbraco.Web.csproj b/src/Umbraco.Web/Umbraco.Web.csproj index 036cb96b50..759e6b47f3 100644 --- a/src/Umbraco.Web/Umbraco.Web.csproj +++ b/src/Umbraco.Web/Umbraco.Web.csproj @@ -832,6 +832,7 @@ + diff --git a/src/Umbraco.Web/WebApi/Filters/AdminUsersAuthorizeAttribute.cs b/src/Umbraco.Web/WebApi/Filters/AdminUsersAuthorizeAttribute.cs new file mode 100644 index 0000000000..8701eaf226 --- /dev/null +++ b/src/Umbraco.Web/WebApi/Filters/AdminUsersAuthorizeAttribute.cs @@ -0,0 +1,42 @@ +using System.Linq; +using System.Net; +using System.Net.Http; +using System.Web.Http; +using System.Web.Http.Controllers; +using Umbraco.Core; +using Umbraco.Web.Editors; + +namespace Umbraco.Web.WebApi.Filters +{ + /// + /// if the user being edited is an admin then we must ensure that the current user is also an admin + /// + public sealed class AdminUsersAuthorizeAttribute : AuthorizeAttribute + { + protected override bool IsAuthorized(HttpActionContext actionContext) + { + if (actionContext.ActionArguments.TryGetValue("id", out var userId) == false) + { + var queryString = actionContext.Request.GetQueryNameValuePairs(); + var ids = queryString.Where(x => x.Key == "id").ToArray(); + if (ids.Length == 0) + return base.IsAuthorized(actionContext); + userId = ids[0].Value; + } + + if (userId == null) return base.IsAuthorized(actionContext); + var intUserId = userId.TryConvertTo(); + if (intUserId.Success == false) + return base.IsAuthorized(actionContext); + + var user = ApplicationContext.Current.Services.UserService.GetUserById(intUserId.Result); + if (user == null) + return base.IsAuthorized(actionContext); + + //Perform authorization here to see if the current user can actually save this user with the info being requested + var authHelper = new UserEditorAuthorizationHelper(ApplicationContext.Current.Services.ContentService, ApplicationContext.Current.Services.MediaService, ApplicationContext.Current.Services.UserService, ApplicationContext.Current.Services.EntityService); + var canSaveUser = authHelper.IsAuthorized(UmbracoContext.Current.Security.CurrentUser, user, null, null, null); + return canSaveUser; + } + } +} diff --git a/src/Umbraco.Web/WebApi/Filters/UmbracoApplicationAuthorizeAttribute.cs b/src/Umbraco.Web/WebApi/Filters/UmbracoApplicationAuthorizeAttribute.cs index 1899584d6f..bc8370c0d5 100644 --- a/src/Umbraco.Web/WebApi/Filters/UmbracoApplicationAuthorizeAttribute.cs +++ b/src/Umbraco.Web/WebApi/Filters/UmbracoApplicationAuthorizeAttribute.cs @@ -1,5 +1,4 @@ using System.Linq; -using System.Web.Http; using System.Web.Http.Controllers; namespace Umbraco.Web.WebApi.Filters @@ -41,4 +40,4 @@ namespace Umbraco.Web.WebApi.Filters return authorized; } } -} \ No newline at end of file +} From d74ec8f8cb531fa631caa2050cfcac91b9654e64 Mon Sep 17 00:00:00 2001 From: Shannon Date: Fri, 15 Mar 2019 12:39:46 +1100 Subject: [PATCH 2/2] Authorizes more endpoints for admin/non-admins --- src/Umbraco.Web/Editors/UsersController.cs | 9 +++- .../Filters/AdminUsersAuthorizeAttribute.cs | 44 ++++++++++++------- 2 files changed, 36 insertions(+), 17 deletions(-) diff --git a/src/Umbraco.Web/Editors/UsersController.cs b/src/Umbraco.Web/Editors/UsersController.cs index 932f10f343..0588eef2c0 100644 --- a/src/Umbraco.Web/Editors/UsersController.cs +++ b/src/Umbraco.Web/Editors/UsersController.cs @@ -605,12 +605,13 @@ namespace Umbraco.Web.Editors display.AddSuccessNotification(Services.TextService.Localize("speechBubbles/operationSavedHeader"), Services.TextService.Localize("speechBubbles/editUserSaved")); return display; - } + } /// /// Disables the users with the given user ids /// /// + [AdminUsersAuthorize("userIds")] public HttpResponseMessage PostDisableUsers([FromUri]int[] userIds) { if (userIds.Contains(Security.GetUserId())) @@ -641,6 +642,7 @@ namespace Umbraco.Web.Editors /// Enables the users with the given user ids /// /// + [AdminUsersAuthorize("userIds")] public HttpResponseMessage PostEnableUsers([FromUri]int[] userIds) { var users = Services.UserService.GetUsersById(userIds).ToArray(); @@ -664,6 +666,7 @@ namespace Umbraco.Web.Editors /// Unlocks the users with the given user ids /// /// + [AdminUsersAuthorize("userIds")] public async Task PostUnlockUsers([FromUri]int[] userIds) { if (userIds.Length <= 0) @@ -696,6 +699,7 @@ namespace Umbraco.Web.Editors Services.TextService.Localize("speechBubbles/unlockUsersSuccess", new[] { userIds.Length.ToString() })); } + [AdminUsersAuthorize("userIds")] public HttpResponseMessage PostSetUserGroupsOnUsers([FromUri]string[] userGroupAliases, [FromUri]int[] userIds) { var users = Services.UserService.GetUsersById(userIds).ToArray(); @@ -721,7 +725,8 @@ namespace Umbraco.Web.Editors /// Limited to users that haven't logged in to avoid issues with related records constrained /// with a foreign key on the user Id /// - public async Task PostDeleteNonLoggedInUser(int id) + [AdminUsersAuthorize] + public HttpResponseMessage PostDeleteNonLoggedInUser(int id) { var user = Services.UserService.GetUserById(id); if (user == null) diff --git a/src/Umbraco.Web/WebApi/Filters/AdminUsersAuthorizeAttribute.cs b/src/Umbraco.Web/WebApi/Filters/AdminUsersAuthorizeAttribute.cs index 8701eaf226..e9cb1d8c6e 100644 --- a/src/Umbraco.Web/WebApi/Filters/AdminUsersAuthorizeAttribute.cs +++ b/src/Umbraco.Web/WebApi/Filters/AdminUsersAuthorizeAttribute.cs @@ -9,34 +9,48 @@ using Umbraco.Web.Editors; namespace Umbraco.Web.WebApi.Filters { /// - /// if the user being edited is an admin then we must ensure that the current user is also an admin + /// if the users being edited is an admin then we must ensure that the current user is also an admin /// + /// + /// This will authorize against one or multiple ids + /// public sealed class AdminUsersAuthorizeAttribute : AuthorizeAttribute { + private readonly string _parameterName; + + public AdminUsersAuthorizeAttribute(string parameterName) + { + _parameterName = parameterName; + } + + public AdminUsersAuthorizeAttribute() : this("id") + { + } + protected override bool IsAuthorized(HttpActionContext actionContext) { - if (actionContext.ActionArguments.TryGetValue("id", out var userId) == false) + int[] userIds; + if (actionContext.ActionArguments.TryGetValue(_parameterName, out var userId)) + { + var intUserId = userId.TryConvertTo(); + if (intUserId) + userIds = new[] {intUserId.Result}; + else return base.IsAuthorized(actionContext); + } + else { var queryString = actionContext.Request.GetQueryNameValuePairs(); - var ids = queryString.Where(x => x.Key == "id").ToArray(); + var ids = queryString.Where(x => x.Key == _parameterName).ToArray(); if (ids.Length == 0) return base.IsAuthorized(actionContext); - userId = ids[0].Value; + userIds = ids.Select(x => x.Value.TryConvertTo()).Where(x => x.Success).Select(x => x.Result).ToArray(); } - if (userId == null) return base.IsAuthorized(actionContext); - var intUserId = userId.TryConvertTo(); - if (intUserId.Success == false) - return base.IsAuthorized(actionContext); + if (userIds.Length == 0) return base.IsAuthorized(actionContext); - var user = ApplicationContext.Current.Services.UserService.GetUserById(intUserId.Result); - if (user == null) - return base.IsAuthorized(actionContext); - - //Perform authorization here to see if the current user can actually save this user with the info being requested + var users = ApplicationContext.Current.Services.UserService.GetUsersById(userIds); var authHelper = new UserEditorAuthorizationHelper(ApplicationContext.Current.Services.ContentService, ApplicationContext.Current.Services.MediaService, ApplicationContext.Current.Services.UserService, ApplicationContext.Current.Services.EntityService); - var canSaveUser = authHelper.IsAuthorized(UmbracoContext.Current.Security.CurrentUser, user, null, null, null); - return canSaveUser; + return users.All(user => authHelper.IsAuthorized(UmbracoContext.Current.Security.CurrentUser, user, null, null, null) != false); } } }