Fixes up more the the session id validation, makes sure that the GetUserSecondsMiddleWare also validates the session since this is what keeps the user logged in if that option is being used.
This commit is contained in:
@@ -67,6 +67,11 @@ namespace Umbraco.Core.Security
|
||||
base.ResponseSignOut(context);
|
||||
|
||||
//Make sure the definitely all of these cookies are cleared when signing out with cookies
|
||||
context.Response.Cookies.Append(SessionIdValidator.CookieName, "", new CookieOptions
|
||||
{
|
||||
Expires = DateTime.Now.AddYears(-1),
|
||||
Path = "/"
|
||||
});
|
||||
context.Response.Cookies.Append(UmbracoConfig.For.UmbracoSettings().Security.AuthCookieName, "", new CookieOptions
|
||||
{
|
||||
Expires = DateTime.Now.AddYears(-1),
|
||||
@@ -107,7 +112,7 @@ namespace Umbraco.Core.Security
|
||||
protected virtual async Task EnsureValidSessionId(CookieValidateIdentityContext context)
|
||||
{
|
||||
if (_appCtx.IsConfigured && _appCtx.IsUpgrading == false)
|
||||
await SessionIdValidator.ValidateSession(TimeSpan.FromMinutes(1), context);
|
||||
await SessionIdValidator.ValidateSessionAsync(TimeSpan.FromMinutes(1), context);
|
||||
}
|
||||
|
||||
private void EnsureCulture(CookieValidateIdentityContext context)
|
||||
|
||||
@@ -1,11 +1,13 @@
|
||||
using System;
|
||||
using System.Collections.Concurrent;
|
||||
using System.Security.Claims;
|
||||
using System.Threading;
|
||||
using System.Threading.Tasks;
|
||||
using System.Web;
|
||||
using Microsoft.AspNet.Identity;
|
||||
using Microsoft.AspNet.Identity.Owin;
|
||||
using Microsoft.Owin;
|
||||
using Microsoft.Owin.Infrastructure;
|
||||
using Microsoft.Owin.Security.Cookies;
|
||||
using Umbraco.Core.Configuration;
|
||||
using Umbraco.Core.Models.Identity;
|
||||
@@ -25,142 +27,92 @@ namespace Umbraco.Core.Security
|
||||
internal static class SessionIdValidator
|
||||
{
|
||||
public const string CookieName = "UMB_UCONTEXT_C";
|
||||
|
||||
public static readonly object CookieLocker = new object();
|
||||
|
||||
public static async Task ValidateSession(TimeSpan validateInterval, CookieValidateIdentityContext context)
|
||||
|
||||
public static async Task ValidateSessionAsync(TimeSpan validateInterval, CookieValidateIdentityContext context)
|
||||
{
|
||||
if (context.Request.Uri.IsBackOfficeRequest(HttpRuntime.AppDomainAppVirtualPath) == false)
|
||||
return;
|
||||
|
||||
var valid = await ValidateSessionAsync(validateInterval, context.OwinContext, context.Options.CookieManager, context.Options.SystemClock, context.Properties.IssuedUtc, context.Identity);
|
||||
|
||||
if (valid == false)
|
||||
{
|
||||
context.RejectIdentity();
|
||||
context.OwinContext.Authentication.SignOut(context.Options.AuthenticationType);
|
||||
}
|
||||
}
|
||||
|
||||
public static async Task<bool> ValidateSessionAsync(
|
||||
TimeSpan validateInterval,
|
||||
IOwinContext owinCtx,
|
||||
ICookieManager cookieManager,
|
||||
ISystemClock systemClock,
|
||||
DateTimeOffset? authTicketIssueDate,
|
||||
ClaimsIdentity currentIdentity)
|
||||
{
|
||||
if (owinCtx == null) throw new ArgumentNullException("owinCtx");
|
||||
if (cookieManager == null) throw new ArgumentNullException("cookieManager");
|
||||
if (systemClock == null) throw new ArgumentNullException("systemClock");
|
||||
|
||||
DateTimeOffset? issuedUtc = null;
|
||||
var currentUtc = DateTimeOffset.UtcNow;
|
||||
var currentUtc = systemClock.UtcNow;
|
||||
|
||||
//read the last checked time from a custom cookie
|
||||
var lastCheckedCookie = context.Options.CookieManager.GetRequestCookie(context.OwinContext, CookieName);
|
||||
|
||||
var lastCheckedCookie = cookieManager.GetRequestCookie(owinCtx, CookieName);
|
||||
|
||||
if (lastCheckedCookie.IsNullOrWhiteSpace() == false)
|
||||
{
|
||||
DateTimeOffset parsed;
|
||||
if (DateTimeOffset.TryParse(lastCheckedCookie, out parsed))
|
||||
{
|
||||
issuedUtc = parsed;
|
||||
}
|
||||
}
|
||||
|
||||
//no cookie, use the issue time of the auth ticket
|
||||
if (issuedUtc.HasValue == false)
|
||||
{
|
||||
if (context.Options != null && context.Options.SystemClock != null)
|
||||
{
|
||||
currentUtc = context.Options.SystemClock.UtcNow;
|
||||
}
|
||||
issuedUtc = context.Properties.IssuedUtc;
|
||||
issuedUtc = authTicketIssueDate;
|
||||
}
|
||||
|
||||
// Only validate if enough time has elapsed
|
||||
var validate = (issuedUtc == null);
|
||||
if (issuedUtc != null)
|
||||
var validate = issuedUtc.HasValue == false;
|
||||
if (issuedUtc.HasValue)
|
||||
{
|
||||
var timeElapsed = currentUtc.Subtract(issuedUtc.Value);
|
||||
validate = timeElapsed > validateInterval;
|
||||
}
|
||||
if (validate)
|
||||
{
|
||||
var manager = context.OwinContext.GetUserManager<BackOfficeUserManager>();
|
||||
var userId = context.Identity.GetUserId<int>();
|
||||
if (manager != null)
|
||||
|
||||
if (validate == false)
|
||||
return true;
|
||||
|
||||
var manager = owinCtx.GetUserManager<BackOfficeUserManager>();
|
||||
if (manager == null)
|
||||
return false;
|
||||
|
||||
var userId = currentIdentity.GetUserId<int>();
|
||||
var user = await manager.FindByIdAsync(userId);
|
||||
if (user == null)
|
||||
return false;
|
||||
|
||||
var sessionId = currentIdentity.FindFirstValue(Constants.Security.SessionIdClaimType);
|
||||
if (await manager.ValidateSessionIdAsync(userId, sessionId) == false)
|
||||
return false;
|
||||
|
||||
//we will re-issue the cookie last checked cookie
|
||||
cookieManager.AppendResponseCookie(
|
||||
owinCtx,
|
||||
CookieName,
|
||||
DateTimeOffset.UtcNow.ToString("yyyy-MM-ddTHH:mm:ss.fffffffzzz"),
|
||||
new CookieOptions
|
||||
{
|
||||
var user = await manager.FindByIdAsync(userId);
|
||||
var reject = true;
|
||||
if (user != null)
|
||||
{
|
||||
//get or create the session checker for the user, this allows us to perform some locking and to check the server side
|
||||
//last checked date/time which allows us to avoid a bunch of requests being checked at the same time and thus a bunch of
|
||||
//db calls to check the session id at the same time and also writing back cookies for each request. Essentially this just
|
||||
//makes things perform better to reduce the amount of db calls and cookie writing.
|
||||
var sessionChecker = SessionChecks.GetOrAdd(user.Id, i => new SessionCheck
|
||||
{
|
||||
LastChecked = issuedUtc ?? DateTimeOffset.UtcNow,
|
||||
//1,1 means only one thread at a time
|
||||
Locker = new SemaphoreSlim(1, 1)
|
||||
});
|
||||
HttpOnly = true,
|
||||
Secure = GlobalSettings.UseSSL || owinCtx.Request.IsSecure,
|
||||
Path = "/"
|
||||
});
|
||||
|
||||
//re-evaluate
|
||||
if (issuedUtc != null)
|
||||
{
|
||||
var timeElapsed = currentUtc.Subtract(sessionChecker.LastChecked);
|
||||
validate = timeElapsed > validateInterval;
|
||||
}
|
||||
|
||||
//immediately update the last checked date which will update it in our SessionChecks memory storage
|
||||
//so that subsequent requests occuring at the same time don't perform the check
|
||||
sessionChecker.LastChecked = DateTimeOffset.UtcNow;
|
||||
|
||||
//we use a lock per user to prevent having the same cookie being written multiple times if multiple requests come in at the very same
|
||||
//time. This has occured for me when clicking on a node that has multiple pickers and multiple REST calls are made at the same time this
|
||||
//validation occurs. This would mean that we would make more db lookups than required and also write the cookie out more times than required
|
||||
//so this prevents this scenario.
|
||||
if (validate)
|
||||
{
|
||||
try
|
||||
{
|
||||
await sessionChecker.Locker.WaitAsync().ConfigureAwait(false);
|
||||
|
||||
//re-evaluate inside the lock
|
||||
var timeElapsed = currentUtc.Subtract(sessionChecker.LastChecked);
|
||||
validate = timeElapsed > validateInterval;
|
||||
if (validate == false)
|
||||
{
|
||||
//exit, another thread must have beat us to it
|
||||
reject = false;
|
||||
}
|
||||
else
|
||||
{
|
||||
var sessionId = context.Identity.FindFirstValue(Constants.Security.SessionIdClaimType);
|
||||
if (await manager.ValidateSessionIdAsync(userId, sessionId))
|
||||
{
|
||||
reject = false;
|
||||
|
||||
//we will re-issue the cookie last checked cookie
|
||||
context.Options.CookieManager.AppendResponseCookie(
|
||||
context.OwinContext,
|
||||
CookieName,
|
||||
DateTimeOffset.UtcNow.ToString("yyyy-MM-ddTHH:mm:ss.fffffffzzz"),
|
||||
new CookieOptions
|
||||
{
|
||||
HttpOnly = true,
|
||||
Secure = GlobalSettings.UseSSL || context.Request.IsSecure,
|
||||
Path = "/"
|
||||
});
|
||||
}
|
||||
}
|
||||
}
|
||||
finally
|
||||
{
|
||||
sessionChecker.Locker.Release();
|
||||
}
|
||||
}
|
||||
}
|
||||
if (reject)
|
||||
{
|
||||
context.RejectIdentity();
|
||||
context.OwinContext.Authentication.SignOut(context.Options.AuthenticationType);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// stores a last checked intance per user
|
||||
/// </summary>
|
||||
private static readonly ConcurrentDictionary<int, SessionCheck> SessionChecks = new ConcurrentDictionary<int, SessionCheck>();
|
||||
|
||||
/// <summary>
|
||||
/// Used to store a locking object and a last checked date/time per user
|
||||
/// </summary>
|
||||
private struct SessionCheck
|
||||
{
|
||||
public SemaphoreSlim Locker { get; set; }
|
||||
public DateTimeOffset LastChecked { get; set; }
|
||||
return true;
|
||||
}
|
||||
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user