fix: security issues with member profile update and request forgery tokens

Author: glitchedmob

SgfDevs/Controllers/AccountController.cs

@@ -14,11 +14,13 @@
 using Umbraco.Cms.Core.Web;
 using Umbraco.Cms.Infrastructure.Persistence;
 using Umbraco.Cms.Web.Common.Models;
+using Umbraco.Cms.Web.Common.Filters;
 using Umbraco.Cms.Web.Common.Security;
 using Umbraco.Cms.Web.Website.Controllers;
 
 namespace SGFDevs.Controllers;
 
+[AutoValidateAntiforgeryToken]
 public class AccountController : SurfaceController
 {
     private IMemberSignInManager _memberSignInManager;
@@ -84,12 +86,15 @@ public async Task<IActionResult> Login(LoginModel model)
         return CurrentUmbracoPage();
     }
 
+    [HttpPost]
+    [UmbracoMemberAuthorize]
     public async Task<IActionResult> Logout()
     {
         await _memberSignInManager.SignOutAsync();
         return Redirect("/");
     }
 
+    [HttpPost]
     public async Task<IActionResult> Register(RegisterModel model)
     {
         if (!ModelState.IsValid)
@@ -132,9 +137,24 @@ public async Task<IActionResult> Register(RegisterModel model)
     }
 
     [HttpPost]
-    public IActionResult ProfileUpdate(MemberProfile profile)
+    [UmbracoMemberAuthorize]
+    public async Task<IActionResult> ProfileUpdate(MemberProfile profile)
     {
-        var member = _memberService.GetByKey(profile.MemberKey);
+        var currentMember = await _memberManager.GetCurrentMemberAsync();
+        if (currentMember == null)
+        {
+            return Forbid();
+        }
+
+        var member = _memberService.GetByKey(currentMember.Key);
+        var fullName = string.Join(" ", new[] { profile.FirstName, profile.LastName }
+            .Where(value => !string.IsNullOrWhiteSpace(value)));
+
+        if (!string.IsNullOrWhiteSpace(fullName))
+        {
+            member.Name = fullName;
+        }
+
         member.Email = profile.Email;
         member.SetValue("FirstName", profile.FirstName);
         member.SetValue("LastName", profile.LastName);

SgfDevs/ViewModels/MemberProfile.cs

@@ -1,11 +1,7 @@
-using System;
-
 namespace SGFDevs.ViewModels;
 
 public class MemberProfile
 {
-    public int MemberId { get; set; }
-    public Guid MemberKey { get; set; }
     public string Email { get; set; }
     public string FirstName { get; set; }
     public string LastName { get; set; }

SgfDevs/Views/Components/Login/Default.cshtml

@@ -9,8 +9,6 @@
     <p>@ViewData["loginWorked"]</p>
     @using (Html.BeginUmbracoForm("Login", "Account", FormMethod.Post))
     {
-        @* @Html.AntiForgeryToken() *@
-
         <div class="form">
             @Html.ValidationSummary(true)
 
@@ -39,4 +37,4 @@
     }
 </div>
 
-<div class="mt_75"></div>
+<div class="mt_75"></div>

SgfDevs/Views/Components/Register/Default.cshtml

@@ -61,4 +61,4 @@
     }
 </div>
 
-<div class="mt_75"></div>
+<div class="mt_75"></div>

SgfDevs/Views/Profile.cshtml

@@ -47,9 +47,7 @@
            <p>Profile updated</p>
        }
 
-        <form asp-controller="Account" asp-action="ProfileUpdate" method="post">
-            <input type="hidden" id="MemberId" name="MemberId" value="@member.Id"/>
-            <input type="hidden" id="MemberKey" name="MemberKey" value="@member.Key"/>
+        <form asp-controller="Account" asp-action="ProfileUpdate" method="post" asp-antiforgery="true">
 
             <div class="field">
                 <label for="member.Email">Email</label>

SgfDevs/Views/Shared/LoginStatus.cshtml

@@ -1,14 +1,6 @@
-@using ContentModels = Umbraco.Cms.Web.Common.PublishedModels;
 @using Umbraco.Cms.Core.Security
-@using Umbraco.Cms.Web.Common.Models
-@using Umbraco.Cms.Web.Common.Controllers
 @inject IMemberManager _memberManager;
 
-@{
-    //var loginStatusModel = Members.GetCurrentLoginStatus();
-    //var logoutModel = new PostRedirectModel();
-}
-
 @if (_memberManager.IsLoggedIn())
 {
     var currentMember = await _memberManager.GetCurrentMemberAsync();
@@ -17,16 +9,14 @@
         <a href="/account">@currentMember.Name</a>
     </li>
     <li>
-        <a href="/umbraco/surface/account/logout">Logout</a>
-        @* @using (Html.BeginUmbracoForm<UmbLoginStatusController>("HandleLogout")) *@
-        @* { *@
-        @*     <button>Logout</button> *@
-        @*     @Html.HiddenFor(m => logoutModel.RedirectUrl) *@
-        @* } *@
+        @using (Html.BeginUmbracoForm("Logout", "Account", FormMethod.Post))
+        {
+            <button type="submit">Logout</button>
+        }
     </li>
 }
 else
 {
     <li><a href="/login">Login</a></li>
     <li><a href="/register">Sign Up</a></li>
-}
+}