From 26dc9219c4674a05e302ebea65b1bc305efde5a9 Mon Sep 17 00:00:00 2001 From: Bjarke Berg Date: Fri, 27 Nov 2020 13:35:43 +0100 Subject: [PATCH] Revert "Revert "Cleans up BackofficeSecurity, fixes up AuthenticationController for endpoints that aren't authorized (and simplifies)"" Signed-off-by: Bjarke Berg --- .../BackOffice/ClaimsPrincipalExtensions.cs | 8 +++-- .../Security/IBackofficeSecurity.cs | 7 ----- .../Controllers/AuthenticationController.cs | 31 +++++++++++-------- ...uthenticateAsBackOfficeSchemeConvention.cs | 16 ---------- .../BackOfficeApplicationModelProvider.cs | 3 +- .../EnsureUmbracoBackOfficeAuthentication.cs | 25 --------------- .../UmbracoBackOfficeAuthorizeAttribute.cs | 9 +++++- .../Security/BackofficeSecurity.cs | 13 +------- 8 files changed, 33 insertions(+), 79 deletions(-) delete mode 100644 src/Umbraco.Web.Common/ApplicationModels/AuthenticateAsBackOfficeSchemeConvention.cs delete mode 100644 src/Umbraco.Web.Common/Filters/EnsureUmbracoBackOfficeAuthentication.cs diff --git a/src/Umbraco.Core/BackOffice/ClaimsPrincipalExtensions.cs b/src/Umbraco.Core/BackOffice/ClaimsPrincipalExtensions.cs index 6a792ce69b..7cbca0428a 100644 --- a/src/Umbraco.Core/BackOffice/ClaimsPrincipalExtensions.cs +++ b/src/Umbraco.Core/BackOffice/ClaimsPrincipalExtensions.cs @@ -17,6 +17,8 @@ namespace Umbraco.Extensions /// public static UmbracoBackOfficeIdentity GetUmbracoIdentity(this IPrincipal user) { + // TODO: It would be nice to get rid of this and only rely on Claims, not a strongly typed identity instance + //If it's already a UmbracoBackOfficeIdentity if (user.Identity is UmbracoBackOfficeIdentity backOfficeIdentity) return backOfficeIdentity; @@ -53,10 +55,10 @@ namespace Umbraco.Extensions /// public static double GetRemainingAuthSeconds(this IPrincipal user, DateTimeOffset now) { - var umbIdentity = user.GetUmbracoIdentity(); - if (umbIdentity == null) return 0; + var claimsPrincipal = user as ClaimsPrincipal; + if (claimsPrincipal == null) return 0; - var ticketExpires = umbIdentity.FindFirstValue(Constants.Security.TicketExpiresClaimType); + var ticketExpires = claimsPrincipal.FindFirst(Constants.Security.TicketExpiresClaimType)?.Value; if (ticketExpires.IsNullOrWhiteSpace()) return 0; var utcExpired = DateTimeOffset.Parse(ticketExpires, null, DateTimeStyles.RoundtripKind); diff --git a/src/Umbraco.Core/Security/IBackofficeSecurity.cs b/src/Umbraco.Core/Security/IBackofficeSecurity.cs index 4ba20f7bfa..12d6dc1688 100644 --- a/src/Umbraco.Core/Security/IBackofficeSecurity.cs +++ b/src/Umbraco.Core/Security/IBackofficeSecurity.cs @@ -32,13 +32,6 @@ namespace Umbraco.Core.Security /// ValidateRequestAttempt ValidateCurrentUser(bool throwExceptions, bool requiresApproval = true); - /// - /// Authorizes the full request, checks for SSL and validates the current user - /// - /// set to true if you want exceptions to be thrown if failed - /// - ValidateRequestAttempt AuthorizeRequest(bool throwExceptions = false); - /// /// Checks if the specified user as access to the app /// diff --git a/src/Umbraco.Web.BackOffice/Controllers/AuthenticationController.cs b/src/Umbraco.Web.BackOffice/Controllers/AuthenticationController.cs index bf1271c910..e9f9c9fa69 100644 --- a/src/Umbraco.Web.BackOffice/Controllers/AuthenticationController.cs +++ b/src/Umbraco.Web.BackOffice/Controllers/AuthenticationController.cs @@ -2,6 +2,7 @@ using System.Collections.Generic; using System.Linq; using System.Net; +using System.Security.Claims; using System.Threading.Tasks; using Microsoft.AspNetCore.Authentication; using Microsoft.AspNetCore.Identity; @@ -206,18 +207,26 @@ namespace Umbraco.Web.BackOffice.Controllers } [HttpGet] - public double GetRemainingTimeoutSeconds() + public async Task GetRemainingTimeoutSeconds() { - var backOfficeIdentity = HttpContext.User.GetUmbracoIdentity(); - var remainingSeconds = HttpContext.User.GetRemainingAuthSeconds(); - if (remainingSeconds <= 30 && backOfficeIdentity != null) + // force authentication to occur since this is not an authorized endpoint + var result = await HttpContext.AuthenticateAsync(Constants.Security.BackOfficeAuthenticationType); + if (!result.Succeeded) { + return 0; + } + + var remainingSeconds = HttpContext.User.GetRemainingAuthSeconds(); + if (remainingSeconds <= 30) + { + var username = result.Principal.FindFirst(ClaimTypes.Name)?.Value; + //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.LogInformation( "User logged will be logged out due to timeout: {Username}, IP Address: {IPAddress}", - backOfficeIdentity.Name, + username ?? "unknown", _ipResolver.GetCurrentRequestIpAddress()); } @@ -229,14 +238,11 @@ namespace Umbraco.Web.BackOffice.Controllers /// /// [HttpGet] - public bool IsAuthenticated() + public async Task IsAuthenticated() { - var attempt = _backofficeSecurityAccessor.BackOfficeSecurity.AuthorizeRequest(); - if (attempt == ValidateRequestAttempt.Success) - { - return true; - } - return false; + // force authentication to occur since this is not an authorized endpoint + var result = await HttpContext.AuthenticateAsync(Constants.Security.BackOfficeAuthenticationType); + return result.Succeeded; } /// @@ -586,7 +592,6 @@ namespace Umbraco.Web.BackOffice.Controllers } - /// /// Return the for the given /// diff --git a/src/Umbraco.Web.Common/ApplicationModels/AuthenticateAsBackOfficeSchemeConvention.cs b/src/Umbraco.Web.Common/ApplicationModels/AuthenticateAsBackOfficeSchemeConvention.cs deleted file mode 100644 index 838cc59ac4..0000000000 --- a/src/Umbraco.Web.Common/ApplicationModels/AuthenticateAsBackOfficeSchemeConvention.cs +++ /dev/null @@ -1,16 +0,0 @@ -using Microsoft.AspNetCore.Mvc.ApplicationModels; -using Umbraco.Web.Common.Filters; - -namespace Umbraco.Web.Common.ApplicationModels -{ - /// - /// Ensures all requests with this convention are authenticated with the back office scheme - /// - public class AuthenticateAsBackOfficeSchemeConvention : IActionModelConvention - { - public void Apply(ActionModel action) - { - action.Filters.Add(new EnsureUmbracoBackOfficeAuthentication()); - } - } -} diff --git a/src/Umbraco.Web.Common/ApplicationModels/BackOfficeApplicationModelProvider.cs b/src/Umbraco.Web.Common/ApplicationModels/BackOfficeApplicationModelProvider.cs index dc0816e1e2..11d82d4db5 100644 --- a/src/Umbraco.Web.Common/ApplicationModels/BackOfficeApplicationModelProvider.cs +++ b/src/Umbraco.Web.Common/ApplicationModels/BackOfficeApplicationModelProvider.cs @@ -17,8 +17,7 @@ namespace Umbraco.Web.Common.ApplicationModels { ActionModelConventions = new List() { - new BackOfficeIdentityCultureConvention(), - new AuthenticateAsBackOfficeSchemeConvention() + new BackOfficeIdentityCultureConvention() }; } diff --git a/src/Umbraco.Web.Common/Filters/EnsureUmbracoBackOfficeAuthentication.cs b/src/Umbraco.Web.Common/Filters/EnsureUmbracoBackOfficeAuthentication.cs deleted file mode 100644 index 5ad43dc922..0000000000 --- a/src/Umbraco.Web.Common/Filters/EnsureUmbracoBackOfficeAuthentication.cs +++ /dev/null @@ -1,25 +0,0 @@ -using Microsoft.AspNetCore.Authorization; -using Microsoft.AspNetCore.Mvc.Filters; -using Umbraco.Web.Common.ApplicationModels; -using Umbraco.Web.Common.Attributes; - -namespace Umbraco.Web.Common.Filters -{ - - /// - /// Assigned as part of the umbraco back office application model - /// to always ensure that back office authentication occurs for all controller/actions with - /// applied. - /// - public class EnsureUmbracoBackOfficeAuthentication : IAuthorizationFilter, IAuthorizeData - { - // Implements IAuthorizeData only to return the back office scheme - public string AuthenticationSchemes { get; set; } = Umbraco.Core.Constants.Security.BackOfficeAuthenticationType; - public string Policy { get; set; } - public string Roles { get; set; } - - public void OnAuthorization(AuthorizationFilterContext context) - { - } - } -} diff --git a/src/Umbraco.Web.Common/Filters/UmbracoBackOfficeAuthorizeAttribute.cs b/src/Umbraco.Web.Common/Filters/UmbracoBackOfficeAuthorizeAttribute.cs index 40f534f289..5ddf285601 100644 --- a/src/Umbraco.Web.Common/Filters/UmbracoBackOfficeAuthorizeAttribute.cs +++ b/src/Umbraco.Web.Common/Filters/UmbracoBackOfficeAuthorizeAttribute.cs @@ -6,8 +6,15 @@ namespace Umbraco.Web.Common.Filters /// /// Ensures authorization is successful for a back office user. /// - public class UmbracoBackOfficeAuthorizeAttribute : TypeFilterAttribute + public class UmbracoBackOfficeAuthorizeAttribute : TypeFilterAttribute, IAuthorizeData { + // Implements IAuthorizeData to return the back office scheme so that all requests with this attributes + // get authenticated with this scheme. + // TODO: We'll have to refactor this as part of the authz policy changes. + public string AuthenticationSchemes { get; set; } = Umbraco.Core.Constants.Security.BackOfficeAuthenticationType; + public string Policy { get; set; } + public string Roles { get; set; } + /// /// Default constructor /// diff --git a/src/Umbraco.Web.Common/Security/BackofficeSecurity.cs b/src/Umbraco.Web.Common/Security/BackofficeSecurity.cs index 23ff63a328..3d9d1e8622 100644 --- a/src/Umbraco.Web.Common/Security/BackofficeSecurity.cs +++ b/src/Umbraco.Web.Common/Security/BackofficeSecurity.cs @@ -52,18 +52,6 @@ namespace Umbraco.Web.Common.Security } } - /// - public ValidateRequestAttempt AuthorizeRequest(bool throwExceptions = false) - { - // check for secure connection - if (_globalSettings.UseHttps && !_httpContextAccessor.GetRequiredHttpContext().Request.IsHttps) - { - if (throwExceptions) throw new SecurityException("This installation requires a secure connection (via SSL). Please update the URL to include https://"); - return ValidateRequestAttempt.FailedNoSsl; - } - return ValidateCurrentUser(throwExceptions); - } - /// public Attempt GetUserId() { @@ -106,6 +94,7 @@ namespace Umbraco.Web.Common.Security var user = CurrentUser; + // TODO: All of this is done as part of identity/backofficesigninmanager // Check for console access if (user == null || (requiresApproval && user.IsApproved == false) || (user.IsLockedOut && RequestIsInUmbracoApplication(_httpContextAccessor, _globalSettings, _hostingEnvironment))) {