Fixes issue with growing cookie/identity (we don't want to use Actor), fixes GlobalSettings issue, ensures temp claims are not cloned

This commit is contained in:
Shannon
2020-06-09 17:40:35 +10:00
parent 246e28d147
commit 2b412f337c
7 changed files with 26 additions and 13 deletions

View File

@@ -39,7 +39,7 @@ namespace Umbraco.Configuration.Models
}
public int TimeOutInMinutes => _configuration.GetValue(Prefix + "TimeOutInMinutes", 20);
public string DefaultUILanguage => _configuration.GetValue(Prefix + "TimeOutInMinutes", "en-US");
public string DefaultUILanguage => _configuration.GetValue(Prefix + "DefaultUILanguage", "en-US");
public bool HideTopLevelNodeFromPath =>
_configuration.GetValue(Prefix + "HideTopLevelNodeFromPath", false);

View File

@@ -36,7 +36,6 @@ namespace Umbraco.Core.BackOffice
private UmbracoBackOfficeIdentity(ClaimsIdentity identity)
: base(identity.Claims, Constants.Security.BackOfficeAuthenticationType)
{
Actor = identity;
}
/// <summary>
@@ -89,7 +88,7 @@ namespace Umbraco.Core.BackOffice
if (string.IsNullOrWhiteSpace(realName)) throw new ArgumentException("Value cannot be null or whitespace.", nameof(realName));
if (string.IsNullOrWhiteSpace(culture)) throw new ArgumentException("Value cannot be null or whitespace.", nameof(culture));
if (string.IsNullOrWhiteSpace(securityStamp)) throw new ArgumentException("Value cannot be null or whitespace.", nameof(securityStamp));
Actor = childIdentity;
AddRequiredClaims(userId, username, realName, startContentNodes, startMediaNodes, culture, securityStamp, allowedApps, roles);
}
@@ -204,5 +203,18 @@ namespace Umbraco.Core.BackOffice
public string[] Roles => this.FindAll(x => x.Type == DefaultRoleClaimType).Select(role => role.Value).ToArray();
/// <summary>
/// Overridden to remove any temporary claims that shouldn't be copied
/// </summary>
/// <returns></returns>
public override ClaimsIdentity Clone()
{
var clone = base.Clone();
foreach (var claim in clone.FindAll(x => x.Type == Constants.Security.TicketExpiresClaimType).ToList())
clone.RemoveClaim(claim);
return clone;
}
}
}

View File

@@ -9,6 +9,8 @@ namespace Umbraco.Core
/// </summary>
public static class AppSettings
{
// TODO: Are these all obsolete in netcore now?
public const string MainDomLock = "Umbraco.Core.MainDom.Lock";
// TODO: Kill me - still used in Umbraco.Core.IO.SystemFiles:27

View File

@@ -37,6 +37,7 @@ namespace Umbraco.Tests.UnitTests.Umbraco.Core.BackOffice
if (!UmbracoBackOfficeIdentity.FromClaimsIdentity(claimsIdentity, out var backofficeIdentity))
Assert.Fail();
Assert.IsNull(backofficeIdentity.Actor);
Assert.AreEqual(1234, backofficeIdentity.Id);
//Assert.AreEqual(sessionId, backofficeIdentity.SessionId);
Assert.AreEqual(securityStamp, backofficeIdentity.SecurityStamp);
@@ -60,7 +61,7 @@ namespace Umbraco.Tests.UnitTests.Umbraco.Core.BackOffice
new Claim(ClaimTypes.Name, "testing", ClaimValueTypes.String, TestIssuer, TestIssuer),
});
if (UmbracoBackOfficeIdentity.FromClaimsIdentity(claimsIdentity, out var backofficeIdentity))
if (UmbracoBackOfficeIdentity.FromClaimsIdentity(claimsIdentity, out _))
Assert.Fail();
Assert.Pass();
@@ -83,7 +84,7 @@ namespace Umbraco.Tests.UnitTests.Umbraco.Core.BackOffice
new Claim(ClaimsIdentity.DefaultRoleClaimType, "admin", ClaimValueTypes.String, TestIssuer, TestIssuer),
});
if (UmbracoBackOfficeIdentity.FromClaimsIdentity(claimsIdentity, out var backofficeIdentity))
if (UmbracoBackOfficeIdentity.FromClaimsIdentity(claimsIdentity, out _))
Assert.Fail();
Assert.Pass();
@@ -105,6 +106,7 @@ namespace Umbraco.Tests.UnitTests.Umbraco.Core.BackOffice
1234, "testing", "hello world", new[] { 654 }, new[] { 654 }, "en-us", securityStamp, new[] { "content", "media" }, new[] { "admin" });
Assert.AreEqual(12, identity.Claims.Count());
Assert.IsNull(identity.Actor);
}
@@ -116,7 +118,11 @@ namespace Umbraco.Tests.UnitTests.Umbraco.Core.BackOffice
var identity = new UmbracoBackOfficeIdentity(
1234, "testing", "hello world", new[] { 654 }, new[] { 654 }, "en-us", securityStamp, new[] { "content", "media" }, new[] { "admin" });
// this will be filtered out during cloning
identity.AddClaim(new Claim(Constants.Security.TicketExpiresClaimType, "test"));
var cloned = identity.Clone();
Assert.IsNull(cloned.Actor);
Assert.AreEqual(10, cloned.Claims.Count());
}

View File

@@ -31,7 +31,6 @@ namespace Umbraco.Web.BackOffice.Security
private readonly IGlobalSettings _globalSettings;
private readonly IRequestCache _requestCache;
private readonly string[] _explicitPaths;
private readonly string _getRemainingSecondsPath;
public BackOfficeCookieManager(
IUmbracoContextAccessor umbracoContextAccessor,
@@ -58,7 +57,6 @@ namespace Umbraco.Web.BackOffice.Security
_globalSettings = globalSettings;
_requestCache = requestCache;
_explicitPaths = explicitPaths?.ToArray();
_getRemainingSecondsPath = linkGenerator.GetUmbracoApiService<AuthenticationController>(x => x.GetRemainingTimeoutSeconds());
}
/// <summary>

View File

@@ -9,7 +9,6 @@ namespace Umbraco.Web.BackOffice.Security
/// <summary>
/// Custom secure format that ensures the Identity in the ticket is <see cref="UmbracoBackOfficeIdentity"/> and not just a ClaimsIdentity
/// </summary>
// TODO: Unsure if we really need this, there's no real reason why we have a custom Identity instead of just a ClaimsIdentity
internal class BackOfficeSecureDataFormat : ISecureDataFormat<AuthenticationTicket>
{
private readonly int _loginTimeoutMinutes;
@@ -23,7 +22,7 @@ namespace Umbraco.Web.BackOffice.Security
public string Protect(AuthenticationTicket data, string purpose)
{
//create a new ticket based on the passed in tickets details, however, we'll adjust the expires utc based on the specified timeout mins
// create a new ticket based on the passed in tickets details, however, we'll adjust the expires utc based on the specified timeout mins
var ticket = new AuthenticationTicket(data.Principal,
new AuthenticationProperties(data.Properties.Items)
{

View File

@@ -150,10 +150,6 @@ namespace Umbraco.Web.BackOffice.Security
UmbracoBackOfficeIdentity.Issuer,
backOfficeIdentity));
if (_securitySettings.KeepUserLoggedIn)
{
}
},
OnSigningIn = ctx =>
{