From 8e6bbc3df9db5f52d264c1b64b2dc8c029dd6286 Mon Sep 17 00:00:00 2001 From: Shannon Date: Fri, 27 Nov 2015 16:25:39 +0100 Subject: [PATCH] Ensures that written cookies are done so consistently based on the UmbracoBackOfficeCookieAuthOptions. Ensures that when a webforms page requests token renewal that the token is not always renewed for the request, it checks if the tokens expiry correctly and only renews when necessary so the cookie is not written each time. Fixes the ForceRenewalCookieAuthenticationHandler to only write a cookie if the request is for a request that is not normally auth'd (i.e. is a webforms form that exists outside the normal /umbraco path ... legacy). --- .../Security/Identity/AppBuilderExtensions.cs | 2 +- .../Identity/BackOfficeCookieManager.cs | 7 +- ...ForceRenewalCookieAuthenticationHandler.cs | 82 +++++++++++-------- ...ceRenewalCookieAuthenticationMiddleware.cs | 13 ++- .../Identity/GetUserSecondsMiddleWare.cs | 10 +-- .../UmbracoBackOfficeCookieAuthOptions.cs | 25 ++++++ 6 files changed, 90 insertions(+), 49 deletions(-) diff --git a/src/Umbraco.Web/Security/Identity/AppBuilderExtensions.cs b/src/Umbraco.Web/Security/Identity/AppBuilderExtensions.cs index 946810ffc9..755eafedcb 100644 --- a/src/Umbraco.Web/Security/Identity/AppBuilderExtensions.cs +++ b/src/Umbraco.Web/Security/Identity/AppBuilderExtensions.cs @@ -172,7 +172,7 @@ namespace Umbraco.Web.Security.Identity app.UseStageMarker(PipelineStage.Authenticate); //Then our custom middlewares - app.Use(typeof(ForceRenewalCookieAuthenticationMiddleware), app, options); + app.Use(typeof(ForceRenewalCookieAuthenticationMiddleware), app, options, new SingletonUmbracoContextAccessor()); app.UseStageMarker(PipelineStage.Authenticate); app.Use(typeof(FixWindowsAuthMiddlware)); app.UseStageMarker(PipelineStage.Authenticate); diff --git a/src/Umbraco.Web/Security/Identity/BackOfficeCookieManager.cs b/src/Umbraco.Web/Security/Identity/BackOfficeCookieManager.cs index 06ce201ef9..c78a918fd9 100644 --- a/src/Umbraco.Web/Security/Identity/BackOfficeCookieManager.cs +++ b/src/Umbraco.Web/Security/Identity/BackOfficeCookieManager.cs @@ -50,6 +50,7 @@ namespace Umbraco.Web.Security.Identity /// /// /// + /// /// /// /// We auth the request when: @@ -58,14 +59,14 @@ namespace Umbraco.Web.Security.Identity /// * it is a /base request /// * it is a preview request /// - internal static bool ShouldAuthenticateRequest(IOwinContext ctx, Uri originalRequestUrl) + internal bool ShouldAuthenticateRequest(IOwinContext ctx, Uri originalRequestUrl, bool checkForceAuthTokens = true) { var request = ctx.Request; var httpCtx = ctx.TryGetHttpContext(); if (//check the explicit flag - ctx.Get("umbraco-force-auth") != null - || (httpCtx.Success && httpCtx.Result.Items["umbraco-force-auth"] != null) + (checkForceAuthTokens && ctx.Get("umbraco-force-auth") != null) + || (checkForceAuthTokens && httpCtx.Success && httpCtx.Result.Items["umbraco-force-auth"] != null) //check back office || request.Uri.IsBackOfficeRequest(HttpRuntime.AppDomainAppVirtualPath) //check installer diff --git a/src/Umbraco.Web/Security/Identity/ForceRenewalCookieAuthenticationHandler.cs b/src/Umbraco.Web/Security/Identity/ForceRenewalCookieAuthenticationHandler.cs index 24beb708f7..c7030b2558 100644 --- a/src/Umbraco.Web/Security/Identity/ForceRenewalCookieAuthenticationHandler.cs +++ b/src/Umbraco.Web/Security/Identity/ForceRenewalCookieAuthenticationHandler.cs @@ -1,3 +1,5 @@ +using System; +using Umbraco.Core; using System.Threading.Tasks; using Microsoft.Owin; using Microsoft.Owin.Security; @@ -11,6 +13,13 @@ namespace Umbraco.Web.Security.Identity /// internal class ForceRenewalCookieAuthenticationHandler : AuthenticationHandler { + private readonly IUmbracoContextAccessor _umbracoContextAccessor; + + public ForceRenewalCookieAuthenticationHandler(IUmbracoContextAccessor umbracoContextAccessor) + { + _umbracoContextAccessor = umbracoContextAccessor; + } + /// /// This handler doesn't actually do any auth so we return null; /// @@ -45,7 +54,20 @@ namespace Umbraco.Web.Security.Identity /// protected override Task ApplyResponseGrantAsync() { - //Now we need to check if we should force renew this based on a flag in the context + if (_umbracoContextAccessor.Value == null || Context.Request.Uri.IsClientSideRequest()) + { + return Task.FromResult(0); + } + + //Now we need to check if we should force renew this based on a flag in the context and whether this is a request that is not normally renewed by OWIN... + // which means that it is not a normal URL that is authenticated. + + var normalAuthUrl = ((BackOfficeCookieManager) Options.CookieManager) + .ShouldAuthenticateRequest(Context, _umbracoContextAccessor.Value.OriginalRequestUrl, + //Pass in false, we want to know if this is a normal auth'd page + checkForceAuthTokens: false); + //This is auth'd normally, so OWIN will naturally take care of the cookie renewal + if (normalAuthUrl) return Task.FromResult(0); var httpCtx = Context.TryGetHttpContext(); //check for the special flag in either the owin or http context @@ -62,45 +84,39 @@ namespace Umbraco.Web.Security.Identity if (shouldSignin == false && shouldSignout == false) { //get the ticket - var model = GetTicket(); - if (model != null) + var ticket = GetTicket(); + if (ticket != null) { var currentUtc = Options.SystemClock.UtcNow; - var issuedUtc = model.Properties.IssuedUtc; - var expiresUtc = model.Properties.ExpiresUtc; + var issuedUtc = ticket.Properties.IssuedUtc; + var expiresUtc = ticket.Properties.ExpiresUtc; if (expiresUtc.HasValue && issuedUtc.HasValue) { - //renew the date/times - model.Properties.IssuedUtc = currentUtc; - var timeSpan = expiresUtc.Value.Subtract(issuedUtc.Value); - model.Properties.ExpiresUtc = currentUtc.Add(timeSpan); + var timeElapsed = currentUtc.Subtract(issuedUtc.Value); + var timeRemaining = expiresUtc.Value.Subtract(currentUtc); - //now save back all the required cookie details - var cookieValue = Options.TicketDataFormat.Protect(model); - var cookieOptions = new CookieOptions + //if it's time to renew, then do it + if (timeRemaining < timeElapsed) { - Domain = Options.CookieDomain, - HttpOnly = Options.CookieHttpOnly, - Path = Options.CookiePath ?? "/", - }; - if (Options.CookieSecure == CookieSecureOption.SameAsRequest) - { - cookieOptions.Secure = Request.IsSecure; - } - else - { - cookieOptions.Secure = Options.CookieSecure == CookieSecureOption.Always; - } - if (model.Properties.IsPersistent) - { - cookieOptions.Expires = model.Properties.ExpiresUtc.Value.ToUniversalTime().DateTime; - } - Options.CookieManager.AppendResponseCookie( - Context, - Options.CookieName, - cookieValue, - cookieOptions); + //renew the date/times + ticket.Properties.IssuedUtc = currentUtc; + var timeSpan = expiresUtc.Value.Subtract(issuedUtc.Value); + ticket.Properties.ExpiresUtc = currentUtc.Add(timeSpan); + + //now save back all the required cookie details + var cookieValue = Options.TicketDataFormat.Protect(ticket); + + var cookieOptions = ((UmbracoBackOfficeCookieAuthOptions)Options).CreateRequestCookieOptions(Context, ticket); + + Options.CookieManager.AppendResponseCookie( + Context, + Options.CookieName, + cookieValue, + cookieOptions); + } + + } } } diff --git a/src/Umbraco.Web/Security/Identity/ForceRenewalCookieAuthenticationMiddleware.cs b/src/Umbraco.Web/Security/Identity/ForceRenewalCookieAuthenticationMiddleware.cs index e159a1c847..77a7a335f0 100644 --- a/src/Umbraco.Web/Security/Identity/ForceRenewalCookieAuthenticationMiddleware.cs +++ b/src/Umbraco.Web/Security/Identity/ForceRenewalCookieAuthenticationMiddleware.cs @@ -10,13 +10,20 @@ namespace Umbraco.Web.Security.Identity /// internal class ForceRenewalCookieAuthenticationMiddleware : CookieAuthenticationMiddleware { - public ForceRenewalCookieAuthenticationMiddleware(OwinMiddleware next, IAppBuilder app, CookieAuthenticationOptions options) : base(next, app, options) + private readonly IUmbracoContextAccessor _umbracoContextAccessor; + + public ForceRenewalCookieAuthenticationMiddleware( + OwinMiddleware next, + IAppBuilder app, + UmbracoBackOfficeCookieAuthOptions options, + IUmbracoContextAccessor umbracoContextAccessor) : base(next, app, options) { - } + _umbracoContextAccessor = umbracoContextAccessor; + } protected override AuthenticationHandler CreateHandler() { - return new ForceRenewalCookieAuthenticationHandler(); + return new ForceRenewalCookieAuthenticationHandler(_umbracoContextAccessor); } } } \ No newline at end of file diff --git a/src/Umbraco.Web/Security/Identity/GetUserSecondsMiddleWare.cs b/src/Umbraco.Web/Security/Identity/GetUserSecondsMiddleWare.cs index c3ce720fa5..a1f4c67681 100644 --- a/src/Umbraco.Web/Security/Identity/GetUserSecondsMiddleWare.cs +++ b/src/Umbraco.Web/Security/Identity/GetUserSecondsMiddleWare.cs @@ -80,15 +80,7 @@ namespace Umbraco.Web.Security.Identity var cookieValue = _authOptions.TicketDataFormat.Protect(ticket); - var cookieOptions = new CookieOptions - { - Path = "/", - Domain = _authOptions.CookieDomain ?? null, - Expires = DateTime.Now.AddMinutes(30), - HttpOnly = true, - Secure = _authOptions.CookieSecure == CookieSecureOption.Always - || (_authOptions.CookieSecure == CookieSecureOption.SameAsRequest && request.Uri.Scheme.InvariantEquals("https")), - }; + var cookieOptions = _authOptions.CreateRequestCookieOptions(context, ticket); _authOptions.CookieManager.AppendResponseCookie( context, diff --git a/src/Umbraco.Web/Security/Identity/UmbracoBackOfficeCookieAuthOptions.cs b/src/Umbraco.Web/Security/Identity/UmbracoBackOfficeCookieAuthOptions.cs index 957cb66b0a..397b438f35 100644 --- a/src/Umbraco.Web/Security/Identity/UmbracoBackOfficeCookieAuthOptions.cs +++ b/src/Umbraco.Web/Security/Identity/UmbracoBackOfficeCookieAuthOptions.cs @@ -1,6 +1,8 @@ using System; using System.Security.Claims; using System.Threading.Tasks; +using Microsoft.Owin; +using Microsoft.Owin.Security; using Microsoft.Owin.Security.Cookies; using Umbraco.Core; using Umbraco.Core.Configuration; @@ -20,6 +22,29 @@ namespace Umbraco.Web.Security.Identity { } + public CookieOptions CreateRequestCookieOptions(IOwinContext ctx, AuthenticationTicket ticket) + { + if (ctx == null) throw new ArgumentNullException("ctx"); + if (ticket == null) throw new ArgumentNullException("ticket"); + + var cookieOptions = new CookieOptions + { + Path = "/", + Domain = this.CookieDomain ?? null, + Expires = DateTime.Now.AddMinutes(30), + HttpOnly = true, + Secure = this.CookieSecure == CookieSecureOption.Always + || (this.CookieSecure == CookieSecureOption.SameAsRequest && ctx.Request.IsSecure), + }; + + if (ticket.Properties.IsPersistent && ticket.Properties.ExpiresUtc.HasValue) + { + cookieOptions.Expires = ticket.Properties.ExpiresUtc.Value.ToUniversalTime().DateTime; + } + + return cookieOptions; + } + public UmbracoBackOfficeCookieAuthOptions( ISecuritySection securitySection, int loginTimeoutMinutes,