diff --git a/src/Umbraco.Configuration/Models/SecuritySettings.cs b/src/Umbraco.Configuration/Models/SecuritySettings.cs index 9244eace96..297c95b1af 100644 --- a/src/Umbraco.Configuration/Models/SecuritySettings.cs +++ b/src/Umbraco.Configuration/Models/SecuritySettings.cs @@ -14,7 +14,7 @@ namespace Umbraco.Configuration.Models _configuration = configuration; } - public bool KeepUserLoggedIn => _configuration.GetValue(Prefix + "KeepUserLoggedIn", true); + public bool KeepUserLoggedIn => _configuration.GetValue(Prefix + "KeepUserLoggedIn", false); public bool HideDisabledUsersInBackoffice => _configuration.GetValue(Prefix + "HideDisabledUsersInBackoffice", false); diff --git a/src/Umbraco.Tests.Integration/Umbraco.Web.BackOffice.Security/BackOfficeCookieManagerTests.cs b/src/Umbraco.Tests.Integration/Umbraco.Web.BackOffice.Security/BackOfficeCookieManagerTests.cs index ac2647dd8e..3464259052 100644 --- a/src/Umbraco.Tests.Integration/Umbraco.Web.BackOffice.Security/BackOfficeCookieManagerTests.cs +++ b/src/Umbraco.Tests.Integration/Umbraco.Web.BackOffice.Security/BackOfficeCookieManagerTests.cs @@ -67,7 +67,7 @@ namespace Umbraco.Tests.Security } [Test] - public void ShouldAuthenticateRequest_No_User_Seconds() + public void ShouldAuthenticateRequest_Is_Back_Office() { var testHelper = new TestHelper(); @@ -85,28 +85,9 @@ namespace Umbraco.Tests.Security GetMockLinkGenerator(out var remainingTimeoutSecondsPath, out var isAuthPath)); var result = mgr.ShouldAuthenticateRequest(new Uri($"http://localhost{remainingTimeoutSecondsPath}")); - Assert.IsFalse(result); - } + Assert.IsTrue(result); - [Test] - public void ShouldAuthenticateRequest_Is_Auth() - { - var testHelper = new TestHelper(); - - var httpContextAccessor = testHelper.GetHttpContextAccessor(); - var globalSettings = testHelper.SettingsForTests.GenerateMockGlobalSettings(); - - var runtime = Mock.Of(x => x.Level == RuntimeLevel.Run); - - var mgr = new BackOfficeCookieManager( - Mock.Of(), - runtime, - Mock.Of(x => x.ApplicationVirtualPath == "/" && x.ToAbsolute(globalSettings.UmbracoPath) == "/umbraco" && x.ToAbsolute(Constants.SystemDirectories.Install) == "/install"), - globalSettings, - Mock.Of(), - GetMockLinkGenerator(out var remainingTimeoutSecondsPath, out var isAuthPath)); - - var result = mgr.ShouldAuthenticateRequest(new Uri($"http://localhost{isAuthPath}")); + result = mgr.ShouldAuthenticateRequest(new Uri($"http://localhost{isAuthPath}")); Assert.IsTrue(result); } diff --git a/src/Umbraco.Web.BackOffice/Controllers/AuthenticationController.cs b/src/Umbraco.Web.BackOffice/Controllers/AuthenticationController.cs index 584c8b0b3b..08aa255b89 100644 --- a/src/Umbraco.Web.BackOffice/Controllers/AuthenticationController.cs +++ b/src/Umbraco.Web.BackOffice/Controllers/AuthenticationController.cs @@ -1,7 +1,9 @@ -using Microsoft.AspNetCore.Mvc; +using Microsoft.AspNetCore.Authentication; +using Microsoft.AspNetCore.Mvc; using System; using System.Net; using System.Threading.Tasks; +using Umbraco.Core; using Umbraco.Core.BackOffice; using Umbraco.Core.Configuration; using Umbraco.Core.Logging; @@ -182,6 +184,22 @@ namespace Umbraco.Web.BackOffice.Controllers throw new HttpResponseException(HttpStatusCode.BadRequest); } + /// + /// Logs the current user out + /// + /// + [TypeFilter(typeof(ValidateAngularAntiForgeryTokenAttribute))] + public IActionResult PostLogout() + { + HttpContext.SignOutAsync(Core.Constants.Security.BackOfficeAuthenticationType); + + _logger.Info("User {UserName} from IP address {RemoteIpAddress} has logged out", User.Identity == null ? "UNKNOWN" : User.Identity.Name, HttpContext.Connection.RemoteIpAddress); + + _userManager.RaiseLogoutSuccessEvent(User, int.Parse(User.Identity.GetUserId())); + + return Ok(); + } + /// /// Return the for the given /// diff --git a/src/Umbraco.Web.BackOffice/Filters/ValidateAngularAntiForgeryTokenAttribute.cs b/src/Umbraco.Web.BackOffice/Filters/ValidateAngularAntiForgeryTokenAttribute.cs index 2fd37e2875..6431911a1f 100644 --- a/src/Umbraco.Web.BackOffice/Filters/ValidateAngularAntiForgeryTokenAttribute.cs +++ b/src/Umbraco.Web.BackOffice/Filters/ValidateAngularAntiForgeryTokenAttribute.cs @@ -24,6 +24,8 @@ namespace Umbraco.Web.BackOffice.Filters /// public sealed class ValidateAngularAntiForgeryTokenAttribute : ActionFilterAttribute { + // TODO: Either make this inherit from TypeFilter or make this just a normal IActionFilter + private readonly ILogger _logger; private readonly IBackOfficeAntiforgery _antiforgery; private readonly ICookieManager _cookieManager; diff --git a/src/Umbraco.Web.BackOffice/Security/BackOfficeCookieManager.cs b/src/Umbraco.Web.BackOffice/Security/BackOfficeCookieManager.cs index 70a57db0e6..19f9316cfb 100644 --- a/src/Umbraco.Web.BackOffice/Security/BackOfficeCookieManager.cs +++ b/src/Umbraco.Web.BackOffice/Security/BackOfficeCookieManager.cs @@ -89,9 +89,6 @@ namespace Umbraco.Web.BackOffice.Security if (_explicitPaths != null) return _explicitPaths.Any(x => x.InvariantEquals(requestUri.AbsolutePath)); - //check user seconds path - if (requestUri.AbsolutePath.InvariantEquals(_getRemainingSecondsPath)) return false; - if (//check the explicit flag checkForceAuthTokens && _requestCache.IsAvailable && _requestCache.Get(Constants.Security.ForceReAuthFlag) != null //check back office diff --git a/src/Umbraco.Web.BackOffice/Security/ConfigureBackOfficeCookieOptions.cs b/src/Umbraco.Web.BackOffice/Security/ConfigureBackOfficeCookieOptions.cs index c81d8bd07c..fd4d530591 100644 --- a/src/Umbraco.Web.BackOffice/Security/ConfigureBackOfficeCookieOptions.cs +++ b/src/Umbraco.Web.BackOffice/Security/ConfigureBackOfficeCookieOptions.cs @@ -36,6 +36,7 @@ namespace Umbraco.Web.BackOffice.Security private readonly IRequestCache _requestCache; private readonly IUserService _userService; private readonly IIpResolver _ipResolver; + private readonly ISystemClock _systemClock; private readonly BackOfficeSessionIdValidator _sessionIdValidator; private readonly LinkGenerator _linkGenerator; @@ -49,6 +50,7 @@ namespace Umbraco.Web.BackOffice.Security IRequestCache requestCache, IUserService userService, IIpResolver ipResolver, + ISystemClock systemClock, BackOfficeSessionIdValidator sessionIdValidator, LinkGenerator linkGenerator) { @@ -61,6 +63,7 @@ namespace Umbraco.Web.BackOffice.Security _requestCache = requestCache; _userService = userService; _ipResolver = ipResolver; + _systemClock = systemClock; _sessionIdValidator = sessionIdValidator; _linkGenerator = linkGenerator; } @@ -116,7 +119,7 @@ namespace Umbraco.Web.BackOffice.Security // It would be possible to re-use the default behavior if any of these need to be set but that must be taken into account else // our back office requests will not function correctly. For now we don't need to set/configure any of these callbacks because // the defaults work fine with our setup. - + OnValidatePrincipal = async ctx => { // We need to resolve the BackOfficeSecurityStampValidator per request as a requirement (even in aspnetcore they do this) @@ -136,6 +139,7 @@ namespace Umbraco.Web.BackOffice.Security await EnsureValidSessionId(ctx); await securityStampValidator.ValidateAsync(ctx); + EnsureTicketRenewalIfKeepUserLoggedIn(ctx); // add a claim to track when the cookie expires, we use this to track time remaining backOfficeIdentity.AddClaim(new Claim( @@ -145,7 +149,11 @@ namespace Umbraco.Web.BackOffice.Security UmbracoBackOfficeIdentity.Issuer, UmbracoBackOfficeIdentity.Issuer, backOfficeIdentity)); - + + if (_securitySettings.KeepUserLoggedIn) + { + + } }, OnSigningIn = ctx => { @@ -180,7 +188,6 @@ namespace Umbraco.Web.BackOffice.Security OnSigningOut = ctx => { //Clear the user's session on sign out - // TODO: We need to test this once we have signout functionality, not sure if the httpcontext.user.identity will still be set here if (ctx.HttpContext?.User?.Identity != null) { var claimsIdentity = ctx.HttpContext.User.Identity as ClaimsIdentity; @@ -197,7 +204,9 @@ namespace Umbraco.Web.BackOffice.Security BackOfficeSessionIdValidator.CookieName, _securitySettings.AuthCookieName, Constants.Web.PreviewCookieName, - Constants.Security.BackOfficeExternalCookieName + Constants.Security.BackOfficeExternalCookieName, + Constants.Web.AngularCookieName, + Constants.Web.CsrfValidationCookieName, }; foreach (var cookie in cookies) { @@ -223,5 +232,31 @@ namespace Umbraco.Web.BackOffice.Security if (_runtimeState.Level == RuntimeLevel.Run) await _sessionIdValidator.ValidateSessionAsync(TimeSpan.FromMinutes(1), context); } + + /// + /// Ensures the ticket is renewed if the is set to true + /// and the current request is for the get user seconds endpoint + /// + /// + private void EnsureTicketRenewalIfKeepUserLoggedIn(CookieValidatePrincipalContext context) + { + if (!_securitySettings.KeepUserLoggedIn) return; + + var currentUtc = _systemClock.UtcNow; + var issuedUtc = context.Properties.IssuedUtc; + var expiresUtc = context.Properties.ExpiresUtc; + + if (expiresUtc.HasValue && issuedUtc.HasValue) + { + var timeElapsed = currentUtc.Subtract(issuedUtc.Value); + var timeRemaining = expiresUtc.Value.Subtract(currentUtc); + + //if it's time to renew, then do it + if (timeRemaining < timeElapsed) + { + context.ShouldRenew = true; + } + } + } } } diff --git a/src/Umbraco.Web/Editors/AuthenticationController.cs b/src/Umbraco.Web/Editors/AuthenticationController.cs index 8ae88b62ef..fe8a93f67a 100644 --- a/src/Umbraco.Web/Editors/AuthenticationController.cs +++ b/src/Umbraco.Web/Editors/AuthenticationController.cs @@ -386,30 +386,7 @@ namespace Umbraco.Web.Editors } - /// - /// Logs the current user out - /// - /// - [ClearAngularAntiForgeryToken] - [ValidateAngularAntiForgeryToken] - public HttpResponseMessage PostLogout() - { - var owinContext = Request.TryGetOwinContext().Result; - - owinContext.Authentication.SignOut( - Core.Constants.Security.BackOfficeAuthenticationType, - Core.Constants.Security.BackOfficeExternalAuthenticationType); - - Logger.Info("User {UserName} from IP address {RemoteIpAddress} has logged out", User.Identity == null ? "UNKNOWN" : User.Identity.Name, owinContext.Request.RemoteIpAddress); - - if (UserManager != null) - { - int.TryParse(User.Identity.GetUserId(), out var userId); - UserManager.RaiseLogoutSuccessEvent(User, userId); - } - - return Request.CreateResponse(HttpStatusCode.OK); - } + // NOTE: This has been migrated to netcore, but in netcore we don't explicitly set the principal in this method, that's done in ConfigureUmbracoBackOfficeCookieOptions so don't worry about that private HttpResponseMessage SetPrincipalAndReturnUserDetail(IUser user, IPrincipal principal) diff --git a/src/Umbraco.Web/Umbraco.Web.csproj b/src/Umbraco.Web/Umbraco.Web.csproj index 49cf3a75a4..dc04863c30 100755 --- a/src/Umbraco.Web/Umbraco.Web.csproj +++ b/src/Umbraco.Web/Umbraco.Web.csproj @@ -350,7 +350,6 @@ - diff --git a/src/Umbraco.Web/WebApi/Filters/ClearAngularAntiForgeryTokenAttribute.cs b/src/Umbraco.Web/WebApi/Filters/ClearAngularAntiForgeryTokenAttribute.cs deleted file mode 100644 index 2aa2f1c52f..0000000000 --- a/src/Umbraco.Web/WebApi/Filters/ClearAngularAntiForgeryTokenAttribute.cs +++ /dev/null @@ -1,35 +0,0 @@ -using System; -using System.Net.Http; -using System.Net.Http.Headers; -using System.Web.Http.Filters; - -namespace Umbraco.Web.WebApi.Filters -{ - /// - /// Clears the angular csrf cookie if the request was successful - /// - public sealed class ClearAngularAntiForgeryTokenAttribute : ActionFilterAttribute - { - public override void OnActionExecuted(HttpActionExecutedContext context) - { - if (context.Response == null) return; - if (context.Response.IsSuccessStatusCode == false) return; - - //remove the cookies - var angularCookie = new CookieHeaderValue(Core.Constants.Web.AngularCookieName, "null") - { - Expires = DateTime.Now.AddYears(-1), - //must be js readable - HttpOnly = false, - Path = "/" - }; - var validationCookie = new CookieHeaderValue(Core.Constants.Web.CsrfValidationCookieName, "null") - { - Expires = DateTime.Now.AddYears(-1), - HttpOnly = true, - Path = "/" - }; - context.Response.Headers.AddCookies(new[] { angularCookie, validationCookie }); - } - } -}