From 2caab18d293aa3af6b626b81fb0f41b8e1a4b70a Mon Sep 17 00:00:00 2001 From: Scott Brady Date: Thu, 30 Apr 2020 09:27:34 +0100 Subject: [PATCH] Reintroduced FindFirstValue extension method --- .../ClaimsIdentityExtensionsTests.cs | 49 +++++++++++++++++++ src/Umbraco.Tests/Umbraco.Tests.csproj | 1 + src/Umbraco.Web/ClaimsIdentityExtensions.cs | 15 ++++-- .../Security/AuthenticationExtensions.cs | 2 +- .../AuthenticationManagerExtensions.cs | 2 +- .../BackOfficeCookieAuthenticationProvider.cs | 2 +- .../Security/SessionIdValidator.cs | 2 +- .../Security/UmbracoBackOfficeIdentity.cs | 13 ++--- .../Security/UmbracoSecurityStampValidator.cs | 2 +- 9 files changed, 73 insertions(+), 15 deletions(-) create mode 100644 src/Umbraco.Tests/CoreThings/ClaimsIdentityExtensionsTests.cs diff --git a/src/Umbraco.Tests/CoreThings/ClaimsIdentityExtensionsTests.cs b/src/Umbraco.Tests/CoreThings/ClaimsIdentityExtensionsTests.cs new file mode 100644 index 0000000000..6cfd853179 --- /dev/null +++ b/src/Umbraco.Tests/CoreThings/ClaimsIdentityExtensionsTests.cs @@ -0,0 +1,49 @@ +using System; +using System.Collections.Generic; +using System.Security.Claims; +using NUnit.Framework; +using Umbraco.Web; + +namespace Umbraco.Tests.CoreThings +{ + public class ClaimsIdentityExtensionsTests + { + [Test] + public void FindFirstValue_WhenIdentityIsNull_ExpectArgumentNullException() + { + ClaimsIdentity identity = null; + Assert.Throws(() => identity.FindFirstValue("test")); + } + + [Test] + public void FindFirstValue_WhenClaimNotPresent_ExpectNull() + { + var identity = new ClaimsIdentity(new List()); + var value = identity.FindFirstValue("test"); + Assert.IsNull(value); + } + + [Test] + public void FindFirstValue_WhenMatchingClaimPresent_ExpectCorrectValue() + { + var expectedClaim = new Claim("test", "123", "string", "Umbraco"); + var identity = new ClaimsIdentity(new List {expectedClaim}); + + var value = identity.FindFirstValue("test"); + + Assert.AreEqual(expectedClaim.Value, value); + } + + [Test] + public void FindFirstValue_WhenMultipleMatchingClaimsPresent_ExpectFirstValue() + { + var expectedClaim = new Claim("test", "123", "string", "Umbraco"); + var dupeClaim = new Claim(expectedClaim.Type, Guid.NewGuid().ToString()); + var identity = new ClaimsIdentity(new List {expectedClaim, dupeClaim}); + + var value = identity.FindFirstValue("test"); + + Assert.AreEqual(expectedClaim.Value, value); + } + } +} diff --git a/src/Umbraco.Tests/Umbraco.Tests.csproj b/src/Umbraco.Tests/Umbraco.Tests.csproj index c2ad177bf1..9221b6d3d6 100644 --- a/src/Umbraco.Tests/Umbraco.Tests.csproj +++ b/src/Umbraco.Tests/Umbraco.Tests.csproj @@ -124,6 +124,7 @@ + diff --git a/src/Umbraco.Web/ClaimsIdentityExtensions.cs b/src/Umbraco.Web/ClaimsIdentityExtensions.cs index c7f777f60b..733bbeed79 100644 --- a/src/Umbraco.Web/ClaimsIdentityExtensions.cs +++ b/src/Umbraco.Web/ClaimsIdentityExtensions.cs @@ -13,8 +13,8 @@ namespace Umbraco.Web string userId = null; if (identity is ClaimsIdentity claimsIdentity) { - userId = claimsIdentity.FindFirst(ClaimTypes.NameIdentifier)?.Value - ?? claimsIdentity.FindFirst("sub")?.Value; + userId = claimsIdentity.FindFirstValue(ClaimTypes.NameIdentifier) + ?? claimsIdentity.FindFirstValue("sub"); } return userId; @@ -27,11 +27,18 @@ namespace Umbraco.Web string username = null; if (identity is ClaimsIdentity claimsIdentity) { - username = claimsIdentity.FindFirst(ClaimTypes.Name)?.Value - ?? claimsIdentity.FindFirst("preferred_username")?.Value; + username = claimsIdentity.FindFirstValue(ClaimTypes.Name) + ?? claimsIdentity.FindFirstValue("preferred_username"); } return username; } + + public static string FindFirstValue(this ClaimsIdentity identity, string claimType) + { + if (identity == null) throw new ArgumentNullException(nameof(identity)); + + return identity.FindFirst(claimType)?.Value; + } } } diff --git a/src/Umbraco.Web/Security/AuthenticationExtensions.cs b/src/Umbraco.Web/Security/AuthenticationExtensions.cs index 07b39a12fe..04baa02c33 100644 --- a/src/Umbraco.Web/Security/AuthenticationExtensions.cs +++ b/src/Umbraco.Web/Security/AuthenticationExtensions.cs @@ -230,7 +230,7 @@ namespace Umbraco.Web.Security var claimsIdentity = http.User.Identity as ClaimsIdentity; if (claimsIdentity != null) { - var sessionId = claimsIdentity.FindFirst(Constants.Security.SessionIdClaimType)?.Value; + var sessionId = claimsIdentity.FindFirstValue(Constants.Security.SessionIdClaimType); Guid guidSession; if (sessionId.IsNullOrWhiteSpace() == false && Guid.TryParse(sessionId, out guidSession)) { diff --git a/src/Umbraco.Web/Security/AuthenticationManagerExtensions.cs b/src/Umbraco.Web/Security/AuthenticationManagerExtensions.cs index f809c402ae..8fc3f83def 100644 --- a/src/Umbraco.Web/Security/AuthenticationManagerExtensions.cs +++ b/src/Umbraco.Web/Security/AuthenticationManagerExtensions.cs @@ -28,7 +28,7 @@ namespace Umbraco.Web.Security name = name.Replace(" ", ""); } - var email = result.Identity.FindFirst(ClaimTypes.Email)?.Value; + var email = result.Identity.FindFirstValue(ClaimTypes.Email); return new ExternalLoginInfo { ExternalIdentity = result.Identity, diff --git a/src/Umbraco.Web/Security/BackOfficeCookieAuthenticationProvider.cs b/src/Umbraco.Web/Security/BackOfficeCookieAuthenticationProvider.cs index 87de1fe9c8..01f8dc4e96 100644 --- a/src/Umbraco.Web/Security/BackOfficeCookieAuthenticationProvider.cs +++ b/src/Umbraco.Web/Security/BackOfficeCookieAuthenticationProvider.cs @@ -57,7 +57,7 @@ namespace Umbraco.Web.Security if (context?.OwinContext?.Authentication?.User?.Identity != null) { var claimsIdentity = context.OwinContext.Authentication.User.Identity as ClaimsIdentity; - var sessionId = claimsIdentity.FindFirst(Core.Constants.Security.SessionIdClaimType)?.Value; + var sessionId = claimsIdentity.FindFirstValue(Constants.Security.SessionIdClaimType); if (sessionId.IsNullOrWhiteSpace() == false && Guid.TryParse(sessionId, out var guidSession)) { _userService.ClearLoginSession(guidSession); diff --git a/src/Umbraco.Web/Security/SessionIdValidator.cs b/src/Umbraco.Web/Security/SessionIdValidator.cs index b58a59d358..9edae8da10 100644 --- a/src/Umbraco.Web/Security/SessionIdValidator.cs +++ b/src/Umbraco.Web/Security/SessionIdValidator.cs @@ -95,7 +95,7 @@ namespace Umbraco.Web.Security if (user == null) return false; - var sessionId = currentIdentity.FindFirst(Constants.Security.SessionIdClaimType)?.Value; + var sessionId = currentIdentity.FindFirstValue(Constants.Security.SessionIdClaimType); if (await manager.ValidateSessionIdAsync(userId, sessionId) == false) return false; diff --git a/src/Umbraco.Web/Security/UmbracoBackOfficeIdentity.cs b/src/Umbraco.Web/Security/UmbracoBackOfficeIdentity.cs index e2f78546fd..c3697d5e9e 100644 --- a/src/Umbraco.Web/Security/UmbracoBackOfficeIdentity.cs +++ b/src/Umbraco.Web/Security/UmbracoBackOfficeIdentity.cs @@ -2,6 +2,7 @@ using System.Collections.Generic; using System.Linq; using System.Security.Claims; +using Umbraco.Web; namespace Umbraco.Core.Security { @@ -203,17 +204,17 @@ namespace Umbraco.Core.Security private string[] _allowedApplications; public string[] AllowedApplications => _allowedApplications ?? (_allowedApplications = FindAll(x => x.Type == Constants.Security.AllowedApplicationsClaimType).Select(app => app.Value).ToArray()); - public int Id => int.Parse(this.FindFirst(ClaimTypes.NameIdentifier)?.Value); + public int Id => int.Parse(this.FindFirstValue(ClaimTypes.NameIdentifier)); - public string RealName => this.FindFirst(ClaimTypes.GivenName)?.Value; + public string RealName => this.FindFirstValue(ClaimTypes.GivenName); - public string Username => this.FindFirst(ClaimTypes.Name)?.Value; + public string Username => this.FindFirstValue(ClaimTypes.Name); - public string Culture => this.FindFirst(ClaimTypes.Locality)?.Value; + public string Culture => this.FindFirstValue(ClaimTypes.Locality); public string SessionId { - get => this.FindFirst(Constants.Security.SessionIdClaimType)?.Value; + get => this.FindFirstValue(Constants.Security.SessionIdClaimType); set { var existing = FindFirst(Constants.Security.SessionIdClaimType); @@ -223,7 +224,7 @@ namespace Umbraco.Core.Security } } - public string SecurityStamp => this.FindFirst(Constants.Web.SecurityStampClaimType)?.Value; + public string SecurityStamp => this.FindFirstValue(Constants.Web.SecurityStampClaimType); public string[] Roles => this.FindAll(x => x.Type == DefaultRoleClaimType).Select(role => role.Value).ToArray(); diff --git a/src/Umbraco.Web/Security/UmbracoSecurityStampValidator.cs b/src/Umbraco.Web/Security/UmbracoSecurityStampValidator.cs index 1264b5f62e..a3f78f5262 100644 --- a/src/Umbraco.Web/Security/UmbracoSecurityStampValidator.cs +++ b/src/Umbraco.Web/Security/UmbracoSecurityStampValidator.cs @@ -54,7 +54,7 @@ namespace Umbraco.Web.Security // Refresh the identity if the stamp matches, otherwise reject if (user != null && manager.SupportsUserSecurityStamp) { - var securityStamp = context.Identity.FindFirst(Constants.Web.SecurityStampClaimType)?.Value; + var securityStamp = context.Identity.FindFirstValue(Constants.Web.SecurityStampClaimType); var newSecurityStamp = await manager.GetSecurityStampAsync(user); if (securityStamp == newSecurityStamp)