diff --git a/src/Umbraco.Core/Security/BackOfficeCookieAuthenticationProvider.cs b/src/Umbraco.Core/Security/BackOfficeCookieAuthenticationProvider.cs index bcc4ef5fff..0a51359bde 100644 --- a/src/Umbraco.Core/Security/BackOfficeCookieAuthenticationProvider.cs +++ b/src/Umbraco.Core/Security/BackOfficeCookieAuthenticationProvider.cs @@ -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) diff --git a/src/Umbraco.Core/Security/SessionIdValidator.cs b/src/Umbraco.Core/Security/SessionIdValidator.cs index d6c39d235f..1737baa778 100644 --- a/src/Umbraco.Core/Security/SessionIdValidator.cs +++ b/src/Umbraco.Core/Security/SessionIdValidator.cs @@ -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 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(); - var userId = context.Identity.GetUserId(); - if (manager != null) + + if (validate == false) + return true; + + var manager = owinCtx.GetUserManager(); + if (manager == null) + return false; + + var userId = currentIdentity.GetUserId(); + 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); - } - } - } - } - - /// - /// stores a last checked intance per user - /// - private static readonly ConcurrentDictionary SessionChecks = new ConcurrentDictionary(); - - /// - /// Used to store a locking object and a last checked date/time per user - /// - private struct SessionCheck - { - public SemaphoreSlim Locker { get; set; } - public DateTimeOffset LastChecked { get; set; } + return true; } + } } \ No newline at end of file diff --git a/src/Umbraco.Tests/packages.config b/src/Umbraco.Tests/packages.config index 494df99e12..6972dbc12e 100644 --- a/src/Umbraco.Tests/packages.config +++ b/src/Umbraco.Tests/packages.config @@ -33,6 +33,7 @@ + diff --git a/src/Umbraco.Web.UI/config/umbracoSettings.Release.config b/src/Umbraco.Web.UI/config/umbracoSettings.Release.config index 4e0c24434c..0e431e8202 100644 --- a/src/Umbraco.Web.UI/config/umbracoSettings.Release.config +++ b/src/Umbraco.Web.UI/config/umbracoSettings.Release.config @@ -38,10 +38,6 @@ In Preview Mode - click to end]]> - - - 1800 - - assets/img/installer.jpg + assets/img/installer.jpg + diff --git a/src/Umbraco.Web/Security/Identity/GetUserSecondsMiddleWare.cs b/src/Umbraco.Web/Security/Identity/GetUserSecondsMiddleWare.cs index baf0ef581b..04dea56c89 100644 --- a/src/Umbraco.Web/Security/Identity/GetUserSecondsMiddleWare.cs +++ b/src/Umbraco.Web/Security/Identity/GetUserSecondsMiddleWare.cs @@ -10,6 +10,7 @@ using Microsoft.Owin.Security.Cookies; using Umbraco.Core; using Umbraco.Core.Configuration; using Umbraco.Core.Configuration.UmbracoSettings; +using Umbraco.Core.Security; namespace Umbraco.Web.Security.Identity { @@ -103,7 +104,10 @@ namespace Umbraco.Web.Security.Identity remainingSeconds = (ticket.Properties.ExpiresUtc.Value - currentUtc).TotalSeconds; } - } + } + + //We also need to re-validate the user's session if we are relying on this ping to keep their session alive + await SessionIdValidator.ValidateSessionAsync(TimeSpan.FromMinutes(1), context, _authOptions.CookieManager, _authOptions.SystemClock, issuedUtc, ticket.Identity); } else if (remainingSeconds <= 30) { diff --git a/src/UmbracoExamine/UmbracoExamine.csproj b/src/UmbracoExamine/UmbracoExamine.csproj index 8fdd5b2d04..eb83b5c813 100644 --- a/src/UmbracoExamine/UmbracoExamine.csproj +++ b/src/UmbracoExamine/UmbracoExamine.csproj @@ -87,7 +87,6 @@ ..\packages\SharpZipLib.0.86.0\lib\20\ICSharpCode.SharpZipLib.dll - True ..\packages\Lucene.Net.2.9.4.1\lib\net40\Lucene.Net.dll diff --git a/src/UmbracoExamine/packages.config b/src/UmbracoExamine/packages.config index 94318fda38..36a8eb3060 100644 --- a/src/UmbracoExamine/packages.config +++ b/src/UmbracoExamine/packages.config @@ -2,4 +2,5 @@ + \ No newline at end of file diff --git a/src/umbraco.MacroEngines/packages.config b/src/umbraco.MacroEngines/packages.config index 3e343b299e..a46f0572ef 100644 --- a/src/umbraco.MacroEngines/packages.config +++ b/src/umbraco.MacroEngines/packages.config @@ -12,4 +12,5 @@ + \ No newline at end of file diff --git a/src/umbraco.MacroEngines/umbraco.MacroEngines.csproj b/src/umbraco.MacroEngines/umbraco.MacroEngines.csproj index 929ab4ae73..63d8314084 100644 --- a/src/umbraco.MacroEngines/umbraco.MacroEngines.csproj +++ b/src/umbraco.MacroEngines/umbraco.MacroEngines.csproj @@ -54,7 +54,6 @@ ..\packages\SharpZipLib.0.86.0\lib\20\ICSharpCode.SharpZipLib.dll - True ..\packages\Lucene.Net.2.9.4.1\lib\net40\Lucene.Net.dll