From 555b520a0ce85b8bc3282a3b4dd076d135378b1e Mon Sep 17 00:00:00 2001 From: Shannon Date: Thu, 19 Nov 2015 18:12:21 +0100 Subject: [PATCH] Cleans up the usages of auth cookies. OWIN is in charge of auth cookies but because we have Webforms, WebApi, MVC and OWIN, they all like to deal with cookies differently. OWIN should still be solely in charge of the auth cookies, so the auth extensions are cleaned up, the renewal now works by queuing the renewal and we have custom middleware detect if a force renewal has been queued and we renew the auth cookie there. Have obsoleted a few methods that should not be used that write auth tickets directly (this is purely for backwards compat with webforms). All of these changes now ensure that the auth cookie is renewed consistently between Webforms, WebApi, MVC and OWIN. Some changes also include ensuring that OWIN is used to sign out. --- src/Umbraco.Core/Constants-Web.cs | 7 +- .../Security/AuthenticationExtensions.cs | 81 +++---------- .../Security/BackOfficeSignInManager.cs | 5 + .../TestHelpers/FakeHttpContextFactory.cs | 3 +- .../Editors/AuthenticationController.cs | 18 ++- src/Umbraco.Web/HttpCookieExtensions.cs | 11 ++ .../InstallSteps/SetUmbracoVersionStep.cs | 8 +- .../Security/Identity/AppBuilderExtensions.cs | 20 +++- .../Identity/BackOfficeCookieManager.cs | 44 ++++++- ...ForceRenewalCookieAuthenticationHandler.cs | 112 ++++++++++++++++++ ...ceRenewalCookieAuthenticationMiddleware.cs | 22 ++++ .../Identity/GetUserSecondsMiddleWare.cs | 4 +- .../UmbracoBackOfficeCookieAuthOptions.cs | 2 +- .../Security/{Identity => }/OwinExtensions.cs | 10 +- src/Umbraco.Web/Security/WebAuthExtensions.cs | 74 ++++++++++++ src/Umbraco.Web/Security/WebSecurity.cs | 28 ++--- src/Umbraco.Web/Umbraco.Web.csproj | 4 + src/Umbraco.Web/UmbracoModule.cs | 67 +---------- .../UmbracoBackOfficeLogoutAttribute.cs | 16 +-- src/Umbraco.Web/umbraco.presentation/macro.cs | 4 + src/umbraco.businesslogic/StateHelper.cs | 3 +- 21 files changed, 356 insertions(+), 187 deletions(-) create mode 100644 src/Umbraco.Web/Security/Identity/ForceRenewalCookieAuthenticationHandler.cs create mode 100644 src/Umbraco.Web/Security/Identity/ForceRenewalCookieAuthenticationMiddleware.cs rename src/Umbraco.Web/Security/{Identity => }/OwinExtensions.cs (51%) create mode 100644 src/Umbraco.Web/Security/WebAuthExtensions.cs diff --git a/src/Umbraco.Core/Constants-Web.cs b/src/Umbraco.Core/Constants-Web.cs index 13dee96b97..60fba0ae40 100644 --- a/src/Umbraco.Core/Constants-Web.cs +++ b/src/Umbraco.Core/Constants-Web.cs @@ -1,4 +1,7 @@ -namespace Umbraco.Core +using System; +using System.ComponentModel; + +namespace Umbraco.Core { public static partial class Constants { @@ -15,6 +18,8 @@ /// /// The auth cookie name /// + [Obsolete("DO NOT USE THIS, USE ISecuritySection.AuthCookieName, this will be removed in future versions")] + [EditorBrowsable(EditorBrowsableState.Never)] public const string AuthCookieName = "UMB_UCONTEXT"; } diff --git a/src/Umbraco.Core/Security/AuthenticationExtensions.cs b/src/Umbraco.Core/Security/AuthenticationExtensions.cs index e332cb6dca..1c7c544ed8 100644 --- a/src/Umbraco.Core/Security/AuthenticationExtensions.cs +++ b/src/Umbraco.Core/Security/AuthenticationExtensions.cs @@ -2,6 +2,7 @@ using System.Collections; using System.Collections.Generic; using System.Collections.Specialized; +using System.ComponentModel; using System.Linq; using System.Net.Http; using System.Net.Http.Headers; @@ -15,7 +16,6 @@ using Microsoft.Owin; using Newtonsoft.Json; using Umbraco.Core.Configuration; using Umbraco.Core.Models.Membership; -using Microsoft.Owin; using Umbraco.Core.Logging; namespace Umbraco.Core.Security @@ -157,9 +157,6 @@ namespace Umbraco.Core.Security return new HttpContextWrapper(http).GetCurrentIdentity(authenticateRequestIfNotFound); } - /// - /// This clears the forms authentication cookie - /// public static void UmbracoLogout(this HttpContextBase http) { if (http == null) throw new ArgumentNullException("http"); @@ -170,6 +167,8 @@ namespace Umbraco.Core.Security /// This clears the forms authentication cookie for webapi since cookies are handled differently /// /// + [Obsolete("Use OWIN IAuthenticationManager.SignOut instead")] + [EditorBrowsable(EditorBrowsableState.Never)] public static void UmbracoLogoutWebApi(this HttpResponseMessage response) { if (response == null) throw new ArgumentNullException("response"); @@ -195,11 +194,8 @@ namespace Umbraco.Core.Security response.Headers.AddCookies(new[] { authCookie, prevCookie, extLoginCookie }); } - /// - /// This adds the forms authentication cookie for webapi since cookies are handled differently - /// - /// - /// + [Obsolete("Use WebSecurity.SetPrincipalForRequest")] + [EditorBrowsable(EditorBrowsableState.Never)] public static FormsAuthenticationTicket UmbracoLoginWebApi(this HttpResponseMessage response, IUser user) { if (response == null) throw new ArgumentNullException("response"); @@ -250,26 +246,29 @@ namespace Umbraco.Core.Security if (http == null) throw new ArgumentNullException("http"); new HttpContextWrapper(http).UmbracoLogout(); } - + /// - /// Renews the Umbraco authentication ticket + /// This will force ticket renewal in the OWIN pipeline /// /// /// public static bool RenewUmbracoAuthTicket(this HttpContextBase http) { if (http == null) throw new ArgumentNullException("http"); - return RenewAuthTicket(http, - UmbracoConfig.For.UmbracoSettings().Security.AuthCookieName, - UmbracoConfig.For.UmbracoSettings().Security.AuthCookieDomain, - //Umbraco has always persisted it's original cookie for 1 day so we'll keep it that way - 1440); + http.Items["umbraco-force-auth"] = true; + return true; } + /// + /// This will force ticket renewal in the OWIN pipeline + /// + /// + /// internal static bool RenewUmbracoAuthTicket(this HttpContext http) { if (http == null) throw new ArgumentNullException("http"); - return new HttpContextWrapper(http).RenewUmbracoAuthTicket(); + http.Items["umbraco-force-auth"] = true; + return true; } /// @@ -390,8 +389,7 @@ namespace Umbraco.Core.Security //ensure there's def an expired cookie http.Response.Cookies.Add(new HttpCookie(c) { Expires = DateTime.Now.AddYears(-1) }); } - } - + } } private static FormsAuthenticationTicket GetAuthTicket(this HttpContextBase http, string cookieName) @@ -432,51 +430,6 @@ namespace Umbraco.Core.Security return FormsAuthentication.Decrypt(formsCookie); } - /// - /// Renews the forms authentication ticket & cookie - /// - /// - /// - /// - /// - /// true if there was a ticket to renew otherwise false if there was no ticket - private static bool RenewAuthTicket(this HttpContextBase http, string cookieName, string cookieDomain, int minutesPersisted) - { - if (http == null) throw new ArgumentNullException("http"); - //get the ticket - var ticket = GetAuthTicket(http, cookieName); - //renew the ticket - var renewed = FormsAuthentication.RenewTicketIfOld(ticket); - if (renewed == null) - { - return false; - } - - //get the request cookie to get it's expiry date, - //NOTE: this will never be null becaues we already do this - // check in teh GetAuthTicket. - var formsCookie = http.Request.Cookies[cookieName]; - - //encrypt it - var hash = FormsAuthentication.Encrypt(renewed); - //write it to the response - var cookie = new HttpCookie(cookieName, hash) - { - Expires = DateTime.Now.AddMinutes(minutesPersisted), - Domain = cookieDomain - }; - - if (GlobalSettings.UseSSL) - cookie.Secure = true; - - //ensure http only, this should only be able to be accessed via the server - cookie.HttpOnly = true; - - //rewrite the cooke - http.Response.Cookies.Set(cookie); - return true; - } - /// /// Creates a custom FormsAuthentication ticket with the data specified /// diff --git a/src/Umbraco.Core/Security/BackOfficeSignInManager.cs b/src/Umbraco.Core/Security/BackOfficeSignInManager.cs index 85d6a0c715..f1d18b9d0f 100644 --- a/src/Umbraco.Core/Security/BackOfficeSignInManager.cs +++ b/src/Umbraco.Core/Security/BackOfficeSignInManager.cs @@ -53,6 +53,11 @@ namespace Umbraco.Core.Security switch (result) { case SignInStatus.Success: + _logger.WriteCore(TraceEventType.Information, 0, + string.Format( + "User: {0} logged in from IP address {1}", + userName, + _request.RemoteIpAddress), null, null); break; case SignInStatus.LockedOut: _logger.WriteCore(TraceEventType.Information, 0, diff --git a/src/Umbraco.Tests/TestHelpers/FakeHttpContextFactory.cs b/src/Umbraco.Tests/TestHelpers/FakeHttpContextFactory.cs index 31bdb616b0..7f38bf61a1 100644 --- a/src/Umbraco.Tests/TestHelpers/FakeHttpContextFactory.cs +++ b/src/Umbraco.Tests/TestHelpers/FakeHttpContextFactory.cs @@ -8,6 +8,7 @@ using System.Web; using System.Web.Routing; using Moq; using Umbraco.Core; +using Umbraco.Core.Configuration; namespace Umbraco.Tests.TestHelpers { @@ -60,7 +61,7 @@ namespace Umbraco.Tests.TestHelpers //Cookie collection var cookieCollection = new HttpCookieCollection(); - cookieCollection.Add(new HttpCookie(Constants.Web.AuthCookieName, "FBA996E7-D6BE-489B-B199-2B0F3D2DD826")); + cookieCollection.Add(new HttpCookie("UMB_UCONTEXT", "FBA996E7-D6BE-489B-B199-2B0F3D2DD826")); //Request var requestMock = new Mock(); diff --git a/src/Umbraco.Web/Editors/AuthenticationController.cs b/src/Umbraco.Web/Editors/AuthenticationController.cs index 7f7b59199a..25b60c5cb7 100644 --- a/src/Umbraco.Web/Editors/AuthenticationController.cs +++ b/src/Umbraco.Web/Editors/AuthenticationController.cs @@ -177,19 +177,14 @@ namespace Umbraco.Web.Editors //get the user var user = Security.GetBackOfficeUser(loginModel.Username); var userDetail = Mapper.Map(user); - + //update the userDetail and set their remaining seconds + userDetail.SecondsUntilTimeout = TimeSpan.FromMinutes(GlobalSettings.TimeOutInMinutes).TotalSeconds; + //create a response with the userDetail object var response = Request.CreateResponse(HttpStatusCode.OK, userDetail); - //set the response cookies with the ticket (NOTE: This needs to be done with the custom webapi extension because - // we cannot mix HttpContext.Response.Cookies and the way WebApi/Owin work) - var ticket = response.UmbracoLoginWebApi(user); - - //This ensure the current principal is set, otherwise any logic executing after this wouldn't actually be authenticated - http.Result.AuthenticateCurrentRequest(ticket, false); - - //update the userDetail and set their remaining seconds - userDetail.SecondsUntilTimeout = ticket.GetRemainingAuthSeconds(); + //ensure the user is set for the current request + Request.SetPrincipalForRequest(user); return response; @@ -241,11 +236,12 @@ namespace Umbraco.Web.Editors /// Logs the current user out /// /// - [UmbracoBackOfficeLogout] [ClearAngularAntiForgeryToken] [ValidateAngularAntiForgeryToken] public HttpResponseMessage PostLogout() { + Request.TryGetOwinContext().Result.Authentication.SignOut(); + Logger.Info("User {0} from IP address {1} has logged out", () => User.Identity == null ? "UNKNOWN" : User.Identity.Name, () => TryGetOwinContext().Result.Request.RemoteIpAddress); diff --git a/src/Umbraco.Web/HttpCookieExtensions.cs b/src/Umbraco.Web/HttpCookieExtensions.cs index dc6d087fb3..7aec1466da 100644 --- a/src/Umbraco.Web/HttpCookieExtensions.cs +++ b/src/Umbraco.Web/HttpCookieExtensions.cs @@ -3,6 +3,7 @@ using System.Linq; using System.Net.Http; using System.Net.Http.Headers; using System.Web; +using Microsoft.Owin; using Umbraco.Core; namespace Umbraco.Web @@ -62,6 +63,16 @@ namespace Umbraco.Web return request.Cookies[Constants.Web.PreviewCookieName] != null; } + /// + /// Does a preview cookie exist ? + /// + /// + /// + public static bool HasPreviewCookie(this IOwinRequest request) + { + return request.Cookies[Constants.Web.PreviewCookieName] != null; + } + /// /// Does a cookie exist with the specified key ? /// diff --git a/src/Umbraco.Web/Install/InstallSteps/SetUmbracoVersionStep.cs b/src/Umbraco.Web/Install/InstallSteps/SetUmbracoVersionStep.cs index b8be5b7a22..6523925505 100644 --- a/src/Umbraco.Web/Install/InstallSteps/SetUmbracoVersionStep.cs +++ b/src/Umbraco.Web/Install/InstallSteps/SetUmbracoVersionStep.cs @@ -43,14 +43,8 @@ namespace Umbraco.Web.Install.InstallSteps var security = new WebSecurity(_httpContext, _applicationContext); security.PerformLogin(0); - ////Clear the auth cookie - this is required so that the login screen is displayed after upgrade and so the - //// csrf anti-forgery tokens are created, otherwise there will just be JS errors if the user has an old - //// login token from a previous version when we didn't have csrf tokens in place - //var security = new WebSecurity(new HttpContextWrapper(Context), ApplicationContext.Current); - //security.ClearCurrentLogin(); - //reports the ended install - InstallHelper ih = new InstallHelper(UmbracoContext.Current); + var ih = new InstallHelper(UmbracoContext.Current); ih.InstallStatus(true, ""); return null; diff --git a/src/Umbraco.Web/Security/Identity/AppBuilderExtensions.cs b/src/Umbraco.Web/Security/Identity/AppBuilderExtensions.cs index d535bc4f8a..1668b6776d 100644 --- a/src/Umbraco.Web/Security/Identity/AppBuilderExtensions.cs +++ b/src/Umbraco.Web/Security/Identity/AppBuilderExtensions.cs @@ -155,7 +155,25 @@ namespace Umbraco.Web.Security.Identity UmbracoConfig.For.UmbracoSettings().Security, app.CreateLogger()); - app.UseCookieAuthentication(authOptions); + app.UseUmbracoBackOfficeCookieAuthentication(authOptions); + + return app; + } + + internal static IAppBuilder UseUmbracoBackOfficeCookieAuthentication(this IAppBuilder app, CookieAuthenticationOptions options) + { + if (app == null) + { + throw new ArgumentNullException("app"); + } + + //First the normal cookie middleware + app.Use(typeof(CookieAuthenticationMiddleware), app, options); + app.UseStageMarker(PipelineStage.Authenticate); + + //Then our custom middleware + app.Use(typeof(ForceRenewalCookieAuthenticationMiddleware), app, options); + app.UseStageMarker(PipelineStage.Authenticate); return app; } diff --git a/src/Umbraco.Web/Security/Identity/BackOfficeCookieManager.cs b/src/Umbraco.Web/Security/Identity/BackOfficeCookieManager.cs index 841c94c80c..06ce201ef9 100644 --- a/src/Umbraco.Web/Security/Identity/BackOfficeCookieManager.cs +++ b/src/Umbraco.Web/Security/Identity/BackOfficeCookieManager.cs @@ -1,6 +1,9 @@ -using Microsoft.Owin; +using System; +using System.Web; +using Microsoft.Owin; using Microsoft.Owin.Infrastructure; using Umbraco.Core; +using Umbraco.Core.IO; namespace Umbraco.Web.Security.Identity { @@ -33,8 +36,8 @@ namespace Umbraco.Web.Security.Identity return null; } - return UmbracoModule.ShouldAuthenticateRequest( - context.HttpContextFromOwinContext().Request, + return ShouldAuthenticateRequest( + context, _umbracoContextAccessor.Value.OriginalRequestUrl) == false //Don't auth request, don't return a cookie ? null @@ -42,5 +45,40 @@ namespace Umbraco.Web.Security.Identity : base.GetRequestCookie(context, key); } + /// + /// Determines if we should authenticate the request + /// + /// + /// + /// + /// + /// We auth the request when: + /// * it is a back office request + /// * it is an installer request + /// * it is a /base request + /// * it is a preview request + /// + internal static bool ShouldAuthenticateRequest(IOwinContext ctx, Uri originalRequestUrl) + { + 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) + //check back office + || request.Uri.IsBackOfficeRequest(HttpRuntime.AppDomainAppVirtualPath) + //check installer + || request.Uri.IsInstallerRequest() + //detect in preview + || (request.HasPreviewCookie() && request.Uri != null && request.Uri.AbsolutePath.StartsWith(IOHelper.ResolveUrl(SystemDirectories.Umbraco)) == false) + //check for base + || BaseRest.BaseRestHandler.IsBaseRestRequest(originalRequestUrl)) + { + return true; + } + return false; + } + } } \ No newline at end of file diff --git a/src/Umbraco.Web/Security/Identity/ForceRenewalCookieAuthenticationHandler.cs b/src/Umbraco.Web/Security/Identity/ForceRenewalCookieAuthenticationHandler.cs new file mode 100644 index 0000000000..24beb708f7 --- /dev/null +++ b/src/Umbraco.Web/Security/Identity/ForceRenewalCookieAuthenticationHandler.cs @@ -0,0 +1,112 @@ +using System.Threading.Tasks; +using Microsoft.Owin; +using Microsoft.Owin.Security; +using Microsoft.Owin.Security.Cookies; +using Microsoft.Owin.Security.Infrastructure; + +namespace Umbraco.Web.Security.Identity +{ + /// + /// If a flag is set on the context to force renew the ticket, this will do it + /// + internal class ForceRenewalCookieAuthenticationHandler : AuthenticationHandler + { + /// + /// This handler doesn't actually do any auth so we return null; + /// + /// + protected override Task AuthenticateCoreAsync() + { + return Task.FromResult((AuthenticationTicket)null); + } + + /// + /// Gets the ticket from the request + /// + /// + private AuthenticationTicket GetTicket() + { + var cookie = Options.CookieManager.GetRequestCookie(Context, Options.CookieName); + if (string.IsNullOrWhiteSpace(cookie)) + { + return null; + } + var ticket = Options.TicketDataFormat.Unprotect(cookie); + if (ticket == null) + { + return null; + } + return ticket; + } + + /// + /// This will check if the token exists in the request to force renewal + /// + /// + protected override Task ApplyResponseGrantAsync() + { + //Now we need to check if we should force renew this based on a flag in the context + + var httpCtx = Context.TryGetHttpContext(); + //check for the special flag in either the owin or http context + var shouldRenew = Context.Get("umbraco-force-auth") != null || (httpCtx.Success && httpCtx.Result.Items["umbraco-force-auth"] != null); + + if (shouldRenew) + { + var signin = Helper.LookupSignIn(Options.AuthenticationType); + var shouldSignin = signin != null; + var signout = Helper.LookupSignOut(Options.AuthenticationType, Options.AuthenticationMode); + var shouldSignout = signout != null; + + //we don't care about the sign in/sign out scenario, only renewal + if (shouldSignin == false && shouldSignout == false) + { + //get the ticket + var model = GetTicket(); + if (model != null) + { + var currentUtc = Options.SystemClock.UtcNow; + var issuedUtc = model.Properties.IssuedUtc; + var expiresUtc = model.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); + + //now save back all the required cookie details + var cookieValue = Options.TicketDataFormat.Protect(model); + var cookieOptions = new CookieOptions + { + 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); + } + } + } + } + + return Task.FromResult(0); + } + } +} \ No newline at end of file diff --git a/src/Umbraco.Web/Security/Identity/ForceRenewalCookieAuthenticationMiddleware.cs b/src/Umbraco.Web/Security/Identity/ForceRenewalCookieAuthenticationMiddleware.cs new file mode 100644 index 0000000000..e159a1c847 --- /dev/null +++ b/src/Umbraco.Web/Security/Identity/ForceRenewalCookieAuthenticationMiddleware.cs @@ -0,0 +1,22 @@ +using Microsoft.Owin; +using Microsoft.Owin.Security.Cookies; +using Microsoft.Owin.Security.Infrastructure; +using Owin; + +namespace Umbraco.Web.Security.Identity +{ + /// + /// This middleware is used simply to force renew the auth ticket if a flag to do so is found in the request + /// + internal class ForceRenewalCookieAuthenticationMiddleware : CookieAuthenticationMiddleware + { + public ForceRenewalCookieAuthenticationMiddleware(OwinMiddleware next, IAppBuilder app, CookieAuthenticationOptions options) : base(next, app, options) + { + } + + protected override AuthenticationHandler CreateHandler() + { + return new ForceRenewalCookieAuthenticationHandler(); + } + } +} \ 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 85cab39120..c3ce720fa5 100644 --- a/src/Umbraco.Web/Security/Identity/GetUserSecondsMiddleWare.cs +++ b/src/Umbraco.Web/Security/Identity/GetUserSecondsMiddleWare.cs @@ -98,13 +98,13 @@ namespace Umbraco.Web.Security.Identity remainingSeconds = (ticket.Properties.ExpiresUtc.Value - DateTime.Now.ToUniversalTime()).TotalSeconds; } - else if (remainingSeconds <=30) + else if (remainingSeconds <= 30) { //NOTE: We are using 30 seconds because that is what is coded into angular to force logout to give some headway in // the timeout process. _logger.WriteCore(TraceEventType.Information, 0, - string.Format("User logged will be logged out due to timeout: {0}, IP Address: {1}", ticket.Identity.Name, request.RemoteIpAddress), + string.Format("User logged will be logged out due to timeout: {0}, IP Address: {1}", ticket.Identity.Name, request.RemoteIpAddress), null, null); } diff --git a/src/Umbraco.Web/Security/Identity/UmbracoBackOfficeCookieAuthOptions.cs b/src/Umbraco.Web/Security/Identity/UmbracoBackOfficeCookieAuthOptions.cs index 9a55e42b51..957cb66b0a 100644 --- a/src/Umbraco.Web/Security/Identity/UmbracoBackOfficeCookieAuthOptions.cs +++ b/src/Umbraco.Web/Security/Identity/UmbracoBackOfficeCookieAuthOptions.cs @@ -36,7 +36,7 @@ namespace Umbraco.Web.Security.Identity } SlidingExpiration = true; - ExpireTimeSpan = TimeSpan.FromMinutes(GlobalSettings.TimeOutInMinutes); + ExpireTimeSpan = TimeSpan.FromMinutes(LoginTimeoutMinutes); CookieDomain = securitySection.AuthCookieDomain; CookieName = securitySection.AuthCookieName; CookieHttpOnly = true; diff --git a/src/Umbraco.Web/Security/Identity/OwinExtensions.cs b/src/Umbraco.Web/Security/OwinExtensions.cs similarity index 51% rename from src/Umbraco.Web/Security/Identity/OwinExtensions.cs rename to src/Umbraco.Web/Security/OwinExtensions.cs index ceb0bdafb2..d1f1fbc1ed 100644 --- a/src/Umbraco.Web/Security/Identity/OwinExtensions.cs +++ b/src/Umbraco.Web/Security/OwinExtensions.cs @@ -1,7 +1,8 @@ -using System.Web; +using System.Web; using Microsoft.Owin; +using Umbraco.Core; -namespace Umbraco.Web.Security.Identity +namespace Umbraco.Web.Security { internal static class OwinExtensions { @@ -11,9 +12,10 @@ namespace Umbraco.Web.Security.Identity /// /// /// - internal static HttpContextBase HttpContextFromOwinContext(this IOwinContext owinContext) + internal static Attempt TryGetHttpContext(this IOwinContext owinContext) { - return owinContext.Get(typeof(HttpContextBase).FullName); + var ctx = owinContext.Get(typeof(HttpContextBase).FullName); + return ctx == null ? Attempt.Fail() : Attempt.Succeed(ctx); } } diff --git a/src/Umbraco.Web/Security/WebAuthExtensions.cs b/src/Umbraco.Web/Security/WebAuthExtensions.cs new file mode 100644 index 0000000000..ec43244b6e --- /dev/null +++ b/src/Umbraco.Web/Security/WebAuthExtensions.cs @@ -0,0 +1,74 @@ +using System.Net.Http; +using System.Security.Claims; +using System.Security.Principal; +using System.ServiceModel.Channels; +using System.Threading; +using System.Web; +using AutoMapper; +using Umbraco.Core.Models.Membership; +using Umbraco.Core.Security; +using Umbraco.Web.WebApi; + +namespace Umbraco.Web.Security +{ + internal static class WebAuthExtensions + { + /// + /// This will set a an authenticated IPrincipal to the current request given the IUser object + /// + /// + /// + /// + internal static IPrincipal SetPrincipalForRequest(this HttpRequestMessage request, IUser user) + { + var principal = new ClaimsPrincipal( + new UmbracoBackOfficeIdentity( + new ClaimsIdentity(), + Mapper.Map(user))); + + //It is actually not good enough to set this on the current app Context and the thread, it also needs + // to be set explicitly on the HttpContext.Current !! This is a strange web api thing that is actually + // an underlying fault of asp.net not propogating the User correctly. + if (HttpContext.Current != null) + { + HttpContext.Current.User = principal; + } + var http = request.TryGetHttpContext(); + if (http) + { + http.Result.User = principal; + } + Thread.CurrentPrincipal = principal; + + //For WebAPI + request.SetUserPrincipal(principal); + + return principal; + } + + /// + /// This will set a an authenticated IPrincipal to the current request given the IUser object + /// + /// + /// + /// + internal static IPrincipal SetPrincipalForRequest(this HttpContextBase httpContext, IUser user) + { + var principal = new ClaimsPrincipal( + new UmbracoBackOfficeIdentity( + new ClaimsIdentity(), + Mapper.Map(user))); + + //It is actually not good enough to set this on the current app Context and the thread, it also needs + // to be set explicitly on the HttpContext.Current !! This is a strange web api thing that is actually + // an underlying fault of asp.net not propogating the User correctly. + if (HttpContext.Current != null) + { + HttpContext.Current.User = principal; + } + httpContext.User = principal; + Thread.CurrentPrincipal = principal; + return principal; + } + } +} \ No newline at end of file diff --git a/src/Umbraco.Web/Security/WebSecurity.cs b/src/Umbraco.Web/Security/WebSecurity.cs index d3d6f76999..1e986dccfc 100644 --- a/src/Umbraco.Web/Security/WebSecurity.cs +++ b/src/Umbraco.Web/Security/WebSecurity.cs @@ -1,5 +1,6 @@ using System; using System.Collections.Generic; +using System.ComponentModel; using System.Linq; using System.Web; using System.Web.Security; @@ -8,8 +9,9 @@ using Umbraco.Core; using Umbraco.Core.Logging; using Umbraco.Core.Models.Membership; using Umbraco.Core.Security; -using umbraco; +using Microsoft.AspNet.Identity.Owin; using umbraco.businesslogic.Exceptions; +using Umbraco.Web.Models.ContentEditing; using GlobalSettings = Umbraco.Core.Configuration.GlobalSettings; using User = umbraco.BusinessLogic.User; @@ -84,32 +86,27 @@ namespace Umbraco.Web.Security /// returns the number of seconds until their session times out public virtual double PerformLogin(int userId) { + var owinCtx = _httpContext.GetOwinContext(); + var user = _applicationContext.Services.UserService.GetUserById(userId); - return PerformLogin(user).GetRemainingAuthSeconds(); + var userDetail = Mapper.Map(user); + //update the userDetail and set their remaining seconds + userDetail.SecondsUntilTimeout = TimeSpan.FromMinutes(GlobalSettings.TimeOutInMinutes).TotalSeconds; + var principal = _httpContext.SetPrincipalForRequest(user); + owinCtx.Authentication.SignIn((UmbracoBackOfficeIdentity)principal.Identity); + return TimeSpan.FromMinutes(GlobalSettings.TimeOutInMinutes).TotalSeconds; } - /// - /// Logs the user in - /// - /// - /// returns the Forms Auth ticket created which is used to log them in + [Obsolete("This method should not be used, login is performed by the OWIN pipeline, use the overload that returns double and accepts a UserId instead")] public virtual FormsAuthenticationTicket PerformLogin(IUser user) { - //clear the external cookie - we do this without owin context because we're writing cookies directly to httpcontext - // and cookie handling is different with httpcontext vs webapi and owin, normally we'd do: - //_httpContext.GetOwinContext().Authentication.SignOut(Constants.Security.BackOfficeExternalAuthenticationType); - var externalLoginCookie = _httpContext.Request.Cookies.Get(Constants.Security.BackOfficeExternalCookieName); if (externalLoginCookie != null) { externalLoginCookie.Expires = DateTime.Now.AddYears(-1); _httpContext.Response.Cookies.Set(externalLoginCookie); } - var ticket = _httpContext.CreateUmbracoAuthTicket(Mapper.Map(user)); - - LogHelper.Info("User Id: {0} logged in", () => user.Id); - return ticket; } @@ -119,6 +116,7 @@ namespace Umbraco.Web.Security public virtual void ClearCurrentLogin() { _httpContext.UmbracoLogout(); + _httpContext.GetOwinContext().Authentication.SignOut(); } /// diff --git a/src/Umbraco.Web/Umbraco.Web.csproj b/src/Umbraco.Web/Umbraco.Web.csproj index 7a4c6a4cd0..b67b410801 100644 --- a/src/Umbraco.Web/Umbraco.Web.csproj +++ b/src/Umbraco.Web/Umbraco.Web.csproj @@ -308,11 +308,15 @@ + + + + diff --git a/src/Umbraco.Web/UmbracoModule.cs b/src/Umbraco.Web/UmbracoModule.cs index 02bb83bd32..a7c41ba90f 100644 --- a/src/Umbraco.Web/UmbracoModule.cs +++ b/src/Umbraco.Web/UmbracoModule.cs @@ -149,72 +149,7 @@ namespace Umbraco.Web #endregion #region Methods - - /// - /// Determines if we should authenticate the request - /// - /// - /// - /// - /// - /// We auth the request when: - /// * it is a back office request - /// * it is an installer request - /// * it is a /base request - /// * it is a preview request - /// - internal static bool ShouldAuthenticateRequest(HttpRequestBase request, Uri originalRequestUrl) - { - if (//check back office - request.Url.IsBackOfficeRequest(HttpRuntime.AppDomainAppVirtualPath) - //check installer - || request.Url.IsInstallerRequest() - //detect in preview - || (request.HasPreviewCookie() && request.Url != null && request.Url.AbsolutePath.StartsWith(IOHelper.ResolveUrl(SystemDirectories.Umbraco)) == false) - //check for base - || BaseRest.BaseRestHandler.IsBaseRestRequest(originalRequestUrl)) - { - return true; - } - return false; - } - - //private static readonly ConcurrentHashSet IgnoreTicketRenewUrls = new ConcurrentHashSet(); - ///// - ///// Determines if the authentication ticket should be renewed with a new timeout - ///// - ///// - ///// - ///// - ///// - ///// We do not want to renew the ticket when we are checking for the user's remaining timeout unless - - ///// UmbracoConfig.For.UmbracoSettings().Security.KeepUserLoggedIn == true - ///// - //internal static bool ShouldIgnoreTicketRenew(Uri url, HttpContextBase httpContext) - //{ - // //this setting will renew the ticket for all requests. - // if (UmbracoConfig.For.UmbracoSettings().Security.KeepUserLoggedIn) - // { - // return false; - // } - - // //initialize the ignore ticket urls - we don't need to lock this, it's concurrent and a hashset - // // we don't want to have to gen the url each request so this will speed things up a teeny bit. - // if (IgnoreTicketRenewUrls.Any() == false) - // { - // var urlHelper = new UrlHelper(new RequestContext(httpContext, new RouteData())); - // var checkSessionUrl = urlHelper.GetUmbracoApiService(controller => controller.GetRemainingTimeoutSeconds()); - // IgnoreTicketRenewUrls.Add(checkSessionUrl); - // } - - // if (IgnoreTicketRenewUrls.Any(x => url.AbsolutePath.StartsWith(x))) - // { - // return true; - // } - - // return false; - //} - + /// /// Checks the current request and ensures that it is routable based on the structure of the request and URI /// diff --git a/src/Umbraco.Web/WebApi/Filters/UmbracoBackOfficeLogoutAttribute.cs b/src/Umbraco.Web/WebApi/Filters/UmbracoBackOfficeLogoutAttribute.cs index 336d8bb403..29ed4da73e 100644 --- a/src/Umbraco.Web/WebApi/Filters/UmbracoBackOfficeLogoutAttribute.cs +++ b/src/Umbraco.Web/WebApi/Filters/UmbracoBackOfficeLogoutAttribute.cs @@ -1,16 +1,12 @@ -using System.Web.Http.Filters; +using System; +using System.ComponentModel; +using System.Web.Http.Filters; using Umbraco.Core.Security; namespace Umbraco.Web.WebApi.Filters { - /// - /// A filter that is used to remove the authorization cookie for the current user when the request is successful - /// - /// - /// This is used so that we can log a user OUT in conjunction with using other filters that modify the cookies collection. - /// SD: I beleive this is a bug with web api since if you modify the cookies collection on the HttpContext.Current and then - /// use a filter to write the cookie headers, the filter seems to have no affect at all. - /// + [Obsolete("This is no longer used and will be removed from the codebase in the future, use OWIN IAuthenticationManager.SignOut instead")] + [EditorBrowsable(EditorBrowsableState.Never)] public sealed class UmbracoBackOfficeLogoutAttribute : ActionFilterAttribute { public override void OnActionExecuted(HttpActionExecutedContext context) @@ -20,7 +16,7 @@ namespace Umbraco.Web.WebApi.Filters //this clears out all of our cookies context.Response.UmbracoLogoutWebApi(); - + //this calls the underlying owin sign out logic - which should call the // auth providers middleware callbacks if using custom auth middleware context.Request.TryGetOwinContext().Result.Authentication.SignOut(); diff --git a/src/Umbraco.Web/umbraco.presentation/macro.cs b/src/Umbraco.Web/umbraco.presentation/macro.cs index 53bdaaa9de..19768d15b5 100644 --- a/src/Umbraco.Web/umbraco.presentation/macro.cs +++ b/src/Umbraco.Web/umbraco.presentation/macro.cs @@ -1705,6 +1705,10 @@ namespace umbraco // propagate the user's context // zb-00004 #29956 : refactor cookies names & handling + + //TODO: This is the worst thing ever. This will also not work if people decide to put their own + // custom auth system in place. + HttpCookie inCookie = StateHelper.Cookies.UserContext.RequestCookie; var cookie = new Cookie(inCookie.Name, inCookie.Value, inCookie.Path, HttpContext.Current.Request.ServerVariables["SERVER_NAME"]); diff --git a/src/umbraco.businesslogic/StateHelper.cs b/src/umbraco.businesslogic/StateHelper.cs index e84d3aa129..61df4318ab 100644 --- a/src/umbraco.businesslogic/StateHelper.cs +++ b/src/umbraco.businesslogic/StateHelper.cs @@ -3,6 +3,7 @@ using System.Reflection; using System.Web; using System.Web.UI; using Umbraco.Core; +using Umbraco.Core.Configuration; namespace umbraco.BusinessLogic { @@ -350,7 +351,7 @@ namespace umbraco.BusinessLogic * that actually make sense? shouldn't some cookie have _no_ expires? */ static readonly Cookie _preview = new Cookie(Constants.Web.PreviewCookieName, TimeSpan.Zero); // was "PreviewSet" - static readonly Cookie _userContext = new Cookie(Constants.Web.AuthCookieName, 30d); // was "UserContext" + static readonly Cookie _userContext = new Cookie(UmbracoConfig.For.UmbracoSettings().Security.AuthCookieName, 30d); // was "UserContext" static readonly Cookie _member = new Cookie("UMB_MEMBER", 30d); // was "umbracoMember" public static Cookie Preview { get { return _preview; } }