Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Updated manager security. Fixes #1741 #1742

Merged
merged 2 commits into from
Oct 25, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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 }");
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Custom authentication providers should also redirect to this new route which is responsible for setting up the CSRF request token for the application.

}

return new RedirectToPageResult("Index");
return LocalRedirect("~/manager/login/auth");
}
}
}
2 changes: 1 addition & 1 deletion core/Piranha.Manager/Areas/Manager/Pages/Index.cshtml.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
Original file line number Diff line number Diff line change
@@ -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">
Expand Down
Original file line number Diff line number Diff line change
@@ -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">
Expand Down
Original file line number Diff line number Diff line change
@@ -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">
Expand Down
Original file line number Diff line number Diff line change
@@ -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">
Expand Down
Original file line number Diff line number Diff line change
@@ -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">
Expand Down
7 changes: 6 additions & 1 deletion core/Piranha.Manager/Areas/Manager/Shared/_Layout.cshtml
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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" />
Expand Down
1 change: 1 addition & 0 deletions core/Piranha.Manager/Controllers/AliasApiController.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
55 changes: 55 additions & 0 deletions core/Piranha.Manager/Controllers/AuthController.cs
Original file line number Diff line number Diff line change
@@ -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");
}
}
}
35 changes: 21 additions & 14 deletions core/Piranha.Manager/Controllers/CommentApiController.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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
{
Expand All @@ -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
{
Expand All @@ -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
{
Expand Down
1 change: 1 addition & 0 deletions core/Piranha.Manager/Controllers/ConfigApiController.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
7 changes: 4 additions & 3 deletions core/Piranha.Manager/Controllers/ContentApiController.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
{
Expand Down
56 changes: 51 additions & 5 deletions core/Piranha.Manager/Controllers/LanguageApiController.cs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
using Microsoft.AspNetCore.Mvc;
using Piranha.Manager.Models;
using Piranha.Manager.Services;
using Piranha.Models;

namespace Piranha.Manager.Controllers
{
Expand All @@ -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;
Expand Down Expand Up @@ -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);
}
}
}
}
9 changes: 5 additions & 4 deletions core/Piranha.Manager/Controllers/MediaApiController.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
{
Expand Down Expand Up @@ -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)
Expand Down
1 change: 1 addition & 0 deletions core/Piranha.Manager/Controllers/ModuleApiController.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Loading