Merge pull request #1742 from PiranhaCMS/features/manager-security-up…

…date Updated manager security. Fixes #1741
PiranhaCMS · Oct 25, 2021 · e42abac · e42abac
2 parents c504166 + 704d7f3
commit e42abac
Show file tree

Hide file tree

Showing 73 changed files with 1,051 additions and 630 deletions.
diff --git a/core/Piranha.Manager.LocalAuth/Areas/Manager/Pages/Login.cshtml.cs b/core/Piranha.Manager.LocalAuth/Areas/Manager/Pages/Login.cshtml.cs
@@ -104,10 +104,9 @@ public async Task<IActionResult> OnPostAsync(string returnUrl = null)
 
             if (!string.IsNullOrEmpty(returnUrl))
             {
-                return LocalRedirect(returnUrl);
+                return LocalRedirect($"~/manager/login/auth?returnUrl={ returnUrl }");
             }
-
-            return new RedirectToPageResult("Index");
+            return LocalRedirect("~/manager/login/auth");
         }
     }
 }
diff --git a/core/Piranha.Manager/Areas/Manager/Pages/Index.cshtml.cs b/core/Piranha.Manager/Areas/Manager/Pages/Index.cshtml.cs
@@ -24,7 +24,7 @@ public IndexModel(IAuthorizationService service)
         {
             _service = service;
         }
-        public async Task<IActionResult> OnGet()
+        public async Task<IActionResult> OnGet(string returnUrl = null)
         {
             var items = await Menu.Items.GetForUser(HttpContext.User, _service);
 

diff --git a/core/Piranha.Manager/Areas/Manager/Shared/Partial/_ContentPickerModal.cshtml b/core/Piranha.Manager/Areas/Manager/Shared/Partial/_ContentPickerModal.cshtml
@@ -1,6 +1,5 @@
 @inject ManagerLocalizer Localizer
 
-<!-- The Modal -->
 <div class="modal modal-panel fade" id="contentpicker">
     <div class="modal-dialog modal-lg">
         <div class="modal-content">

diff --git a/core/Piranha.Manager/Areas/Manager/Shared/Partial/_MediaPickerModal.cshtml b/core/Piranha.Manager/Areas/Manager/Shared/Partial/_MediaPickerModal.cshtml
@@ -1,7 +1,6 @@
 @inject IAuthorizationService Auth
 @inject ManagerLocalizer Localizer
 
-<!-- The Modal -->
 <div class="modal modal-panel fade" id="mediapicker">
     <div class="modal-dialog modal-lg">
         <div class="modal-content">

diff --git a/core/Piranha.Manager/Areas/Manager/Shared/Partial/_PagePickerModal.cshtml b/core/Piranha.Manager/Areas/Manager/Shared/Partial/_PagePickerModal.cshtml
@@ -1,6 +1,5 @@
 @inject ManagerLocalizer Localizer
 
-<!-- The Modal -->
 <div class="modal modal-panel fade" id="pagepicker">
     <div class="modal-dialog modal-lg">
         <div class="modal-content">

diff --git a/core/Piranha.Manager/Areas/Manager/Shared/Partial/_PostPickerModal.cshtml b/core/Piranha.Manager/Areas/Manager/Shared/Partial/_PostPickerModal.cshtml
@@ -1,6 +1,5 @@
 @inject ManagerLocalizer Localizer
 
-<!-- The Modal -->
 <div class="modal modal-panel fade" id="postpicker">
     <div class="modal-dialog modal-lg">
         <div class="modal-content">

diff --git a/core/Piranha.Manager/Areas/Manager/Shared/Partial/_PreviewModal.cshtml b/core/Piranha.Manager/Areas/Manager/Shared/Partial/_PreviewModal.cshtml
@@ -1,7 +1,6 @@
 @inject IAuthorizationService Auth
 @inject ManagerLocalizer Localizer
 
-<!-- The Modal -->
 <div class="modal fade" id="previewModal">
     <div class="modal-dialog modal-lg">
         <div class="modal-content">

diff --git a/core/Piranha.Manager/Areas/Manager/Shared/_Layout.cshtml b/core/Piranha.Manager/Areas/Manager/Shared/_Layout.cshtml
@@ -1,4 +1,5 @@
-@{
+@inject Microsoft.Extensions.Options.IOptions<ManagerOptions> Options
+@{
     var module = Piranha.App.Modules.Get<Piranha.Manager.Module>();
     var prerelease = Piranha.Utils.IsPreRelease(typeof(Piranha.Manager.Module).Assembly) ? "pre-release" : "";
     var isRightToLeft =  @System.Globalization.CultureInfo.CurrentCulture.TextInfo.IsRightToLeft;
@@ -53,6 +54,10 @@
         var piranha = {};
         window.piranha = piranha;
         piranha.baseUrl = "@Url.Content("~/")";
+        piranha.antiForgery = {
+            cookieName: "@Options.Value.XsrfCookieName",
+            headerName: "@Options.Value.XsrfHeaderName"
+        };
     </script>
 
     <partial name="~/Areas/Manager/Shared/Partial/_EditorConfig.cshtml" />

diff --git a/core/Piranha.Manager/Controllers/AliasApiController.cs b/core/Piranha.Manager/Controllers/AliasApiController.cs
@@ -25,6 +25,7 @@ namespace Piranha.Manager.Controllers
     [Route("manager/api/alias")]
     [Authorize(Policy = Permission.Admin)]
     [ApiController]
+    [AutoValidateAntiforgeryToken]
     public class AliasApiController : Controller
     {
         private readonly IApi _api;

diff --git a/core/Piranha.Manager/Controllers/AuthController.cs b/core/Piranha.Manager/Controllers/AuthController.cs
@@ -0,0 +1,55 @@
+/*
+ * Copyright (c) .NET Foundation and Contributors
+ *
+ * This software may be modified and distributed under the terms
+ * of the MIT license.  See the LICENSE file for details.
+ *
+ * https://github.com/piranhacms/piranha.core
+ *
+ */
+
+using Microsoft.AspNetCore.Antiforgery;
+using Microsoft.AspNetCore.Authorization;
+using Microsoft.AspNetCore.Http;
+using Microsoft.AspNetCore.Mvc;
+using Microsoft.Extensions.Options;
+
+namespace Piranha.Manager.Controllers
+{
+    [Area("Manager")]
+    [Route("manager/login/auth")]
+    [Authorize(Policy = Permission.Admin)]
+    [ApiController]
+    public sealed class AuthController : Controller
+    {
+        private readonly IAntiforgery _antiForgery;
+        private readonly ManagerOptions _options;
+
+        /// <summary>
+        /// Default constructor.
+        /// </summary>
+        /// <param name="antiforgery">The antiforgery service</param>
+        /// <param name="options">The manager options</param>
+        public AuthController(IAntiforgery antiforgery, IOptions<ManagerOptions> options)
+        {
+            _antiForgery = antiforgery;
+            _options = options.Value;
+        }
+
+        [Route("{returnUrl?}")]
+        [HttpGet]
+        public IActionResult SetAuthCookie(string returnUrl = null)
+        {
+            var tokens = _antiForgery.GetAndStoreTokens(HttpContext);
+            Response.Cookies.Append(_options.XsrfCookieName, tokens.RequestToken, new CookieOptions
+            {
+                HttpOnly = false
+            });    
+            if (!string.IsNullOrEmpty(returnUrl))
+            {
+                return LocalRedirect(returnUrl);
+            }        
+            return LocalRedirect("~/manager");
+        }
+    }
+}
diff --git a/core/Piranha.Manager/Controllers/CommentApiController.cs b/core/Piranha.Manager/Controllers/CommentApiController.cs
@@ -24,8 +24,15 @@ namespace Piranha.Manager.Controllers
     [Route("manager/api/comment")]
     [Authorize(Policy = Permission.Admin)]
     [ApiController]
+    [AutoValidateAntiforgeryToken]
     public class CommentApiController : Controller
     {
+        public class ApprovalModel
+        {
+            public Guid Id { get; set; }
+            public Guid? ParentId { get; set; }
+        }
+
         private readonly CommentService _service;
         private readonly ManagerLocalizer _localizer;
 
@@ -50,14 +57,14 @@ public Task<CommentListModel> List(Guid? id = null)
             return _service.Get(id);
         }
 
-        [Route("approve/{id}/{parentId?}")]
-        [HttpGet]
+        [Route("approve")]
+        [HttpPost]
         [Authorize(Policy = Permission.CommentsApprove)]
-        public async Task<CommentListModel> Approve(Guid id, Guid? parentId = null)
+        public async Task<CommentListModel> Approve(ApprovalModel model)
         {
-            await _service.ApproveAsync(id);
+            await _service.ApproveAsync(model.Id).ConfigureAwait(false);
 
-            var result = await List(parentId);
+            var result = await List(model.ParentId).ConfigureAwait(false);
 
             result.Status = new StatusMessage
             {
@@ -67,14 +74,14 @@ public async Task<CommentListModel> Approve(Guid id, Guid? parentId = null)
             return result;
         }
 
-        [Route("unapprove/{id}/{parentId?}")]
-        [HttpGet]
+        [Route("unapprove")]
+        [HttpPost]
         [Authorize(Policy = Permission.CommentsApprove)]
-        public async Task<CommentListModel> UnApprove(Guid id, Guid? parentId = null)
+        public async Task<CommentListModel> UnApprove(ApprovalModel model)
         {
-            await _service.UnApproveAsync(id);
+            await _service.UnApproveAsync(model.Id).ConfigureAwait(false);
 
-            var result = await List(parentId);
+            var result = await List(model.ParentId).ConfigureAwait(false);
 
             result.Status = new StatusMessage
             {
@@ -84,12 +91,12 @@ public async Task<CommentListModel> UnApprove(Guid id, Guid? parentId = null)
             return result;
         }
 
-        [Route("delete/{id}")]
-        [HttpGet]
+        [Route("delete")]
+        [HttpDelete]
         [Authorize(Policy = Permission.CommentsDelete)]
-        public async Task<StatusMessage> Delete(Guid id)
+        public async Task<StatusMessage> Delete([FromBody]Guid id)
         {
-            await _service.DeleteAsync(id);
+            await _service.DeleteAsync(id).ConfigureAwait(false);
 
             var result = new StatusMessage
             {

diff --git a/core/Piranha.Manager/Controllers/ConfigApiController.cs b/core/Piranha.Manager/Controllers/ConfigApiController.cs
@@ -22,6 +22,7 @@ namespace Piranha.Manager.Controllers
     [Route("manager/api/config")]
     [Authorize(Policy = Permission.Admin)]
     [ApiController]
+    [AutoValidateAntiforgeryToken]
     public class ConfigApiController : Controller
     {
         private readonly ConfigService _service;

diff --git a/core/Piranha.Manager/Controllers/ContentApiController.cs b/core/Piranha.Manager/Controllers/ContentApiController.cs
@@ -25,6 +25,7 @@ namespace Piranha.Manager.Controllers
     [Route("manager/api/content")]
     [Authorize(Policy = Permission.Admin)]
     [ApiController]
+    [AutoValidateAntiforgeryToken]
     public class ContentApiController : Controller
     {
         private readonly IApi _api;
@@ -228,10 +229,10 @@ public async Task<ContentEditModel> Save(ContentEditModel model)
         /// </summary>
         /// <param name="id">The unique id</param>
         /// <returns>The result of the operation</returns>
-        [Route("delete/{id}")]
-        [HttpGet]
+        [Route("delete")]
+        [HttpDelete]
         [Authorize(Policy = Permission.ContentDelete)]
-        public async Task<StatusMessage> Delete(Guid id)
+        public async Task<StatusMessage> Delete([FromBody]Guid id)
         {
             try
             {

diff --git a/core/Piranha.Manager/Controllers/LanguageApiController.cs b/core/Piranha.Manager/Controllers/LanguageApiController.cs
@@ -14,7 +14,6 @@
 using Microsoft.AspNetCore.Mvc;
 using Piranha.Manager.Models;
 using Piranha.Manager.Services;
-using Piranha.Models;
 
 namespace Piranha.Manager.Controllers
 {
@@ -25,6 +24,7 @@ namespace Piranha.Manager.Controllers
     [Route("manager/api/language")]
     [Authorize(Policy = Permission.Admin)]
     [ApiController]
+    [AutoValidateAntiforgeryToken]
     public class LanguageApiController : Controller
     {
         private readonly LanguageService _service;
@@ -56,16 +56,62 @@ public async Task<LanguageEditModel> Get()
         /// <param name="model">The model</param>
         [Route("")]
         [HttpPost]
-        public async Task<LanguageEditModel> Save(LanguageEditModel model)
+        public async Task<IActionResult> Save(LanguageEditModel model)
         {
-            return await _service.Save(model);
+            try 
+            {
+                var result = await _service.Save(model);
+
+                result.Status = new StatusMessage
+                {
+                    Type = StatusMessage.Success,
+                    Body = _localizer.Language["The language was successfully saved"]
+                };
+
+                return Ok(result);                
+            }
+            catch (Exception e)
+            {
+                var result = new LanguageEditModel();
+                result.Status = new StatusMessage
+                {
+                    Type = StatusMessage.Error,
+                    Body = e.Message
+                };   
+                return BadRequest(result);             
+            }
         }
 
+        /// <summary>
+        /// Deletes the language with the given id.
+        /// </summary>
+        /// <param name="id">The unique id</param>
         [Route("{id}")]
         [HttpDelete]
-        public async Task<LanguageEditModel> Delete(Guid id)
+        public async Task<IActionResult> Delete(Guid id)
         {
-            return await _service.Delete(id);
+            try 
+            {
+                var result = await _service.Delete(id);
+
+                result.Status = new StatusMessage
+                {
+                    Type = StatusMessage.Success,
+                    Body = _localizer.Language["The language was successfully deleted"]
+                };
+
+                return Ok(result);              
+            }
+            catch (Exception e)
+            {
+                var result = new LanguageEditModel();
+                result.Status = new StatusMessage
+                {
+                    Type = StatusMessage.Error,
+                    Body = e.Message
+                };   
+                return BadRequest(result);             
+            }
         }
     }
 }
diff --git a/core/Piranha.Manager/Controllers/MediaApiController.cs b/core/Piranha.Manager/Controllers/MediaApiController.cs
@@ -28,6 +28,7 @@ namespace Piranha.Manager.Controllers
     [Route("manager/api/media")]
     [Authorize(Policy = Permission.Admin)]
     [ApiController]
+    [AutoValidateAntiforgeryToken]
     public class MediaApiController : Controller
     {
         private readonly MediaService _service;
@@ -153,10 +154,10 @@ public async Task<IActionResult> SaveFolder(MediaFolderModel model, MediaType? f
             }
         }
 
-        [Route("folder/delete/{id:Guid}")]
-        [HttpGet]
+        [Route("folder/delete")]
+        [HttpDelete]
         [Authorize(Policy = Permission.MediaDeleteFolder)]
-        public async Task<IActionResult> DeleteFolder(Guid id)
+        public async Task<IActionResult> DeleteFolder([FromBody]Guid id)
         {
             try
             {
@@ -299,7 +300,7 @@ public async Task<IActionResult> Move([FromBody] IEnumerable<Guid> items, Guid?
         }
 
         [Route("delete")]
-        [HttpPost]
+        [HttpDelete]
         [Consumes("application/json")]
         [Authorize(Policy = Permission.MediaDelete)]
         public async Task<IActionResult> Delete([FromBody] IEnumerable<Guid> items)

diff --git a/core/Piranha.Manager/Controllers/ModuleApiController.cs b/core/Piranha.Manager/Controllers/ModuleApiController.cs
@@ -23,6 +23,7 @@ namespace Piranha.Manager.Controllers
     [Route("manager/api/module")]
     [Authorize(Policy = Permission.Admin)]
     [ApiController]
+    [AutoValidateAntiforgeryToken]
     public class ModuleApiController : Controller
     {
         private readonly ModuleService _service;