diff --git a/src/Umbraco.Web.UI.Client/src/common/directives/components/application/umblogin.directive.js b/src/Umbraco.Web.UI.Client/src/common/directives/components/application/umblogin.directive.js index 94ad0e360d..a2833a22aa 100644 --- a/src/Umbraco.Web.UI.Client/src/common/directives/components/application/umblogin.directive.js +++ b/src/Umbraco.Web.UI.Client/src/common/directives/components/application/umblogin.directive.js @@ -40,15 +40,21 @@ uploadProgress: 0, maxFileSize: Umbraco.Sys.ServerVariables.umbracoSettings.maxFileSize + "KB", acceptedFileTypes: mediaHelper.formatFileTypes(Umbraco.Sys.ServerVariables.umbracoSettings.imageFileTypes), - uploaded: false + uploaded: false }; vm.allowPasswordReset = Umbraco.Sys.ServerVariables.umbracoSettings.canSendRequiredEmail && Umbraco.Sys.ServerVariables.umbracoSettings.allowPasswordReset; vm.errorMsg = ""; vm.externalLoginFormAction = Umbraco.Sys.ServerVariables.umbracoUrls.externalLoginsUrl; vm.externalLoginProviders = externalLoginInfoService.getLoginProviders(); + vm.externalLoginProviders.forEach(x => { + x.customView = externalLoginInfoService.getExternalLoginProviderView(x.authType); + // if there are errors set for this specific provider than assign them directly to the model + if (externalLoginInfo.errorProvider === x.authType) { + x.errors = externalLoginInfo.errors; + } + }); vm.denyLocalLogin = externalLoginInfoService.hasDenyLocalLogin(); - vm.externalLoginProviderView = externalLoginInfo.errorProvider ? externalLoginInfoService.getExternalLoginProviderViewForErrors(externalLoginInfo.errorProvider) : null; vm.externalLoginInfo = externalLoginInfo; vm.resetPasswordCodeInfo = resetPasswordCodeInfo; vm.backgroundImage = Umbraco.Sys.ServerVariables.umbracoSettings.loginBackgroundImage; diff --git a/src/Umbraco.Web.UI.Client/src/common/services/externallogininfo.service.js b/src/Umbraco.Web.UI.Client/src/common/services/externallogininfo.service.js index fe9d1f1f8a..e9e8d3cbd3 100644 --- a/src/Umbraco.Web.UI.Client/src/common/services/externallogininfo.service.js +++ b/src/Umbraco.Web.UI.Client/src/common/services/externallogininfo.service.js @@ -7,7 +7,7 @@ function externalLoginInfoService(externalLoginInfo, umbRequestHelper) { function getExternalLoginProvider(provider) { if (provider) { - var found = _.filter(externalLoginInfo.providers, x => x.authType == provider); + var found = _.find(externalLoginInfo.providers, x => x.authType == provider); return found; } return null; @@ -21,14 +21,6 @@ function externalLoginInfoService(externalLoginInfo, umbRequestHelper) { return null; } - function getExternalLoginProviderViewForErrors(provider) { - var found = getExternalLoginProvider(provider); - if (found && found.properties.UmbracoBackOfficeExternalLoginOptions.BackOfficeCustomLoginViewHandlesErrors) { - return umbRequestHelper.convertVirtualToAbsolutePath(found.properties.UmbracoBackOfficeExternalLoginOptions.BackOfficeCustomLoginView); - } - return null; - } - /** * Returns true if any provider denies local login if `provider` is null, else whether the passed * @param {any} provider @@ -74,8 +66,7 @@ function externalLoginInfoService(externalLoginInfo, umbRequestHelper) { hasDenyLocalLogin: hasDenyLocalLogin, getLoginProviders: getLoginProviders, getUserInviteLink: getUserInviteLink, - getExternalLoginProviderView: getExternalLoginProviderView, - getExternalLoginProviderViewForErrors: getExternalLoginProviderViewForErrors + getExternalLoginProviderView: getExternalLoginProviderView }; } angular.module('umbraco.services').factory('externalLoginInfoService', externalLoginInfoService); diff --git a/src/Umbraco.Web.UI.Client/src/views/components/application/umb-login.html b/src/Umbraco.Web.UI.Client/src/views/components/application/umb-login.html index 385977fdc5..4c5386eeff 100644 --- a/src/Umbraco.Web.UI.Client/src/views/components/application/umb-login.html +++ b/src/Umbraco.Web.UI.Client/src/views/components/application/umb-login.html @@ -125,15 +125,6 @@
-
-
- {{error}} -
-
-
-
-
-
@@ -146,6 +137,14 @@ Sign in with {{login.caption}} +
+
+ {{error}} +
+
+ +
+
diff --git a/src/Umbraco.Web/Editors/BackOfficeController.cs b/src/Umbraco.Web/Editors/BackOfficeController.cs index 8fcfdfea7c..4d0f560c14 100644 --- a/src/Umbraco.Web/Editors/BackOfficeController.cs +++ b/src/Umbraco.Web/Editors/BackOfficeController.cs @@ -354,8 +354,9 @@ namespace Umbraco.Web.Editors ViewData.SetUmbracoPath(GlobalSettings.GetUmbracoMvcArea()); - //check if there is the TempData with the any token name specified, if so, assign to view bag and render the view - if (ViewData.FromTempData(TempData, ViewDataExtensions.TokenExternalSignInError) || + //check if there is the TempData or cookies with the any token name specified, if so, assign to view bag and render the view + if (ViewData.FromBase64CookieData(HttpContext, ViewDataExtensions.TokenExternalSignInError) || + ViewData.FromTempData(TempData, ViewDataExtensions.TokenExternalSignInError) || ViewData.FromTempData(TempData, ViewDataExtensions.TokenPasswordResetCode)) return defaultResponse(); @@ -491,10 +492,7 @@ namespace Umbraco.Web.Editors } //call the callback if one is assigned - if (autoLinkOptions.OnAutoLinking != null) - { - autoLinkOptions.OnAutoLinking(autoLinkUser, loginInfo); - } + autoLinkOptions.OnAutoLinking?.Invoke(autoLinkUser, loginInfo); var userCreationResult = await UserManager.CreateAsync(autoLinkUser); diff --git a/src/Umbraco.Web/HttpCookieExtensions.cs b/src/Umbraco.Web/HttpCookieExtensions.cs index 5f520653f5..ebb77bd4a4 100644 --- a/src/Umbraco.Web/HttpCookieExtensions.cs +++ b/src/Umbraco.Web/HttpCookieExtensions.cs @@ -1,9 +1,11 @@ using System; +using System.Collections.Generic; using System.Linq; using System.Net.Http; using System.Net.Http.Headers; using System.Web; using Microsoft.Owin; +using Newtonsoft.Json; using Umbraco.Core; namespace Umbraco.Web @@ -61,11 +63,11 @@ namespace Umbraco.Web http.Request.Cookies.Remove(cookieName); //expire from the response - var angularCookie = http.Response.Cookies[cookieName]; - if (angularCookie != null) + var cookie = http.Response.Cookies[cookieName]; + if (cookie != null) { //this will expire immediately and be removed from the browser - angularCookie.Expires = DateTime.Now.AddYears(-1); + cookie.Expires = DateTime.Now.AddYears(-1); } else { diff --git a/src/Umbraco.Web/OwinExtensions.cs b/src/Umbraco.Web/OwinExtensions.cs index ee8b54f92b..e7d41d113c 100644 --- a/src/Umbraco.Web/OwinExtensions.cs +++ b/src/Umbraco.Web/OwinExtensions.cs @@ -5,20 +5,25 @@ using Microsoft.Owin; using Microsoft.Owin.Security; using Umbraco.Core; using Umbraco.Core.Models.Identity; -using Umbraco.Core.Security; using Umbraco.Web.Security; namespace Umbraco.Web { public static class OwinExtensions { + /// + /// Used by external login providers to set any errors that occur during the OAuth negotiation + /// + /// + /// public static void SetExternalLoginProviderErrors(this IOwinContext owinContext, BackOfficeExternalLoginProviderErrors errors) - { - // TODO: Once this is set, we could use more custom middleware to set the cookie and redirect so it's not up to the - // oauth provider to handle all of this manually - owinContext.Set(errors); - } + => owinContext.Set(errors); + /// + /// Retrieve any errors set by external login providers during OAuth negotiation + /// + /// + /// internal static BackOfficeExternalLoginProviderErrors GetExternalLoginProviderErrors(this IOwinContext owinContext) => owinContext.Get(); diff --git a/src/Umbraco.Web/Security/AppBuilderExtensions.cs b/src/Umbraco.Web/Security/AppBuilderExtensions.cs index 1ff5416dac..8f33f10eea 100644 --- a/src/Umbraco.Web/Security/AppBuilderExtensions.cs +++ b/src/Umbraco.Web/Security/AppBuilderExtensions.cs @@ -1,6 +1,8 @@ using System; using System.Threading; +using System.Web; using System.Web.Mvc; +using System.Web.SessionState; using Microsoft.AspNet.Identity; using Microsoft.AspNet.Identity.Owin; using Microsoft.Owin; @@ -386,7 +388,7 @@ namespace Umbraco.Web.Security /// /// public static IAppBuilder UseUmbracoBackOfficeExternalLoginErrors(this IAppBuilder app, PipelineStage stage = PipelineStage.Authorize) - { + { app.Use(typeof(BackOfficeExternalLoginProviderErrorMiddlware)); app.UseStageMarker(stage); return app; diff --git a/src/Umbraco.Web/Security/BackOfficeExternalLoginProviderErrorMiddlware.cs b/src/Umbraco.Web/Security/BackOfficeExternalLoginProviderErrorMiddlware.cs index 5a08fdf1cf..7d3c48c078 100644 --- a/src/Umbraco.Web/Security/BackOfficeExternalLoginProviderErrorMiddlware.cs +++ b/src/Umbraco.Web/Security/BackOfficeExternalLoginProviderErrorMiddlware.cs @@ -1,4 +1,6 @@ -using System.Collections.Generic; +using System; +using System.Collections.Generic; +using System.Text; using System.Threading.Tasks; using System.Web.Mvc; using Microsoft.Owin; @@ -7,6 +9,13 @@ using Umbraco.Core; namespace Umbraco.Web.Security { + /// + /// Used to handle errors registered by external login providers + /// + /// + /// When an external login provider registers an error with during the OAuth process, + /// this middleware will detect that, store the errors into cookie data and redirect to the back office login so we can read the errors back out. + /// internal class BackOfficeExternalLoginProviderErrorMiddlware : OwinMiddleware { public BackOfficeExternalLoginProviderErrorMiddlware(OwinMiddleware next) : base(next) @@ -15,41 +24,32 @@ namespace Umbraco.Web.Security public override async Task Invoke(IOwinContext context) { + var shortCircuit = false; if (!context.Request.Uri.IsClientSideRequest()) { // check if we have any errors registered var errors = context.GetExternalLoginProviderErrors(); if (errors != null) { - // this is pretty nasty to resolve this from the MVC service locator but that's all we can really work with since that is where it is - var tempDataProvider = DependencyResolver.Current.GetService(); + shortCircuit = true; - // create a 'fake' controller context for temp data to work. we want to use temp data because it's self managing and we won't have to - // deal with resetting anything and plus it's configurable (by default uses session). better than creating a state manager ourselves. - var controllerContext = new ControllerContext( - context.TryGetHttpContext().Result, - new System.Web.Routing.RouteData(), - new EmptyController()); + var serialized = Convert.ToBase64String(Encoding.UTF8.GetBytes(JsonConvert.SerializeObject(errors))); - tempDataProvider.SaveTempData(controllerContext, new Dictionary + context.Response.Cookies.Append("ExternalSignInError", serialized, new CookieOptions { - [ViewDataExtensions.TokenExternalSignInError] = errors + Expires = DateTime.Now.AddMinutes(5), + HttpOnly = true, + Secure = context.Request.IsSecure }); + + context.Response.Redirect(context.Request.Uri.ToString()); } } - if (Next != null) + if (Next != null && !shortCircuit) { await Next.Invoke(context); } } - - private class EmptyController : ControllerBase - { - protected override void ExecuteCore() - { - throw new System.NotImplementedException(); - } - } } } diff --git a/src/Umbraco.Web/Security/BackOfficeExternalLoginProviderErrors.cs b/src/Umbraco.Web/Security/BackOfficeExternalLoginProviderErrors.cs index 85807d511f..39b967fa96 100644 --- a/src/Umbraco.Web/Security/BackOfficeExternalLoginProviderErrors.cs +++ b/src/Umbraco.Web/Security/BackOfficeExternalLoginProviderErrors.cs @@ -1,16 +1,22 @@ using System.Collections.Generic; +using System.Linq; namespace Umbraco.Web.Security { public class BackOfficeExternalLoginProviderErrors { + // required for deserialization + public BackOfficeExternalLoginProviderErrors() + { + } + public BackOfficeExternalLoginProviderErrors(string authenticationType, IEnumerable errors) { AuthenticationType = authenticationType; - Errors = errors; + Errors = errors ?? Enumerable.Empty(); } - public string AuthenticationType { get; } - public IEnumerable Errors { get; } + public string AuthenticationType { get; set; } + public IEnumerable Errors { get; set; } } } diff --git a/src/Umbraco.Web/Security/BackOfficeExternalLoginProviderOptions.cs b/src/Umbraco.Web/Security/BackOfficeExternalLoginProviderOptions.cs index 4cd43f3b2b..c85f56f562 100644 --- a/src/Umbraco.Web/Security/BackOfficeExternalLoginProviderOptions.cs +++ b/src/Umbraco.Web/Security/BackOfficeExternalLoginProviderOptions.cs @@ -53,16 +53,9 @@ namespace Umbraco.Web.Security /// /// /// The types of customization used for this view are: - /// - Displaying external login provider errors /// - Displaying extra information, links, etc... to the user when logging out /// - Displaying extra information, links, etc... to the user when logging in (if auto-login redirect is disabled) /// public string BackOfficeCustomLoginView { get; set; } - - /// - /// If set to true then Umbraco will not automatically show any external login provider errors for this provider and instead will leave it up to the custom view - /// assigned to display any errors - /// - public bool BackOfficeCustomLoginViewHandlesErrors { get; set; } } } diff --git a/src/Umbraco.Web/UmbracoDefaultOwinStartup.cs b/src/Umbraco.Web/UmbracoDefaultOwinStartup.cs index 105d2aff4c..9a9f5e7e01 100644 --- a/src/Umbraco.Web/UmbracoDefaultOwinStartup.cs +++ b/src/Umbraco.Web/UmbracoDefaultOwinStartup.cs @@ -68,6 +68,9 @@ namespace Umbraco.Web // Configure OWIN for authentication. ConfigureUmbracoAuthentication(app); + // must come after all authentication + app.UseUmbracoBackOfficeExternalLoginErrors(); + app .UseSignalR(GlobalSettings) .FinalizeMiddlewareConfiguration(); @@ -99,8 +102,8 @@ namespace Umbraco.Web app .UseUmbracoBackOfficeCookieAuthentication(UmbracoContextAccessor, RuntimeState, Services.UserService, GlobalSettings, UmbracoSettings.Security, PipelineStage.Authenticate) .UseUmbracoBackOfficeExternalCookieAuthentication(UmbracoContextAccessor, RuntimeState, GlobalSettings, PipelineStage.Authenticate) - .UseUmbracoPreviewAuthentication(UmbracoContextAccessor, RuntimeState, GlobalSettings, UmbracoSettings.Security, PipelineStage.Authorize) - .UseUmbracoBackOfficeExternalLoginErrors(); + // TODO: this would be considered a breaking change but this must come after all authentication so should be moved within ConfigureMiddleware + .UseUmbracoPreviewAuthentication(UmbracoContextAccessor, RuntimeState, GlobalSettings, UmbracoSettings.Security, PipelineStage.Authorize); } public static event EventHandler MiddlewareConfigured; diff --git a/src/Umbraco.Web/ViewDataExtensions.cs b/src/Umbraco.Web/ViewDataExtensions.cs index 7a881bd8f9..ac4f4cdf75 100644 --- a/src/Umbraco.Web/ViewDataExtensions.cs +++ b/src/Umbraco.Web/ViewDataExtensions.cs @@ -1,7 +1,11 @@ -using System; +using Newtonsoft.Json; +using System; using System.Collections.Generic; using System.Linq; +using System.Text; +using System.Web; using System.Web.Mvc; +using Umbraco.Core; using Umbraco.Web.Security; namespace Umbraco.Web @@ -21,6 +25,49 @@ namespace Umbraco.Web return true; } + /// + /// Copies data from a request cookie to view data and then clears the cookie in the response + /// + /// + /// + /// + /// + /// + /// + /// This is similar to TempData but in some cases we cannot use TempData which relies on the temp data provider and session. + /// The cookie value can either be a simple string value + /// + /// + internal static bool FromBase64CookieData(this ViewDataDictionary viewData, HttpContextBase httpContext, string cookieName) + { + var hasCookie = httpContext.Request.HasCookie(cookieName); + if (!hasCookie) return false; + + // get the cookie value + var cookieVal = httpContext.Request.GetCookieValue(cookieName); + + if (cookieVal == null) + return false; + + // ensure the cookie is expired (must be done after reading the value) + httpContext.ExpireCookie(cookieName); + + if (cookieVal.IsNullOrWhiteSpace()) + return false; + + try + { + var decoded = Encoding.UTF8.GetString(Convert.FromBase64String(System.Net.WebUtility.UrlDecode(cookieVal))); + // deserialize to T and store in viewdata + viewData[cookieName] = JsonConvert.DeserializeObject(decoded); + return true; + } + catch (Exception) + { + return false; + } + } + public static string GetUmbracoPath(this ViewDataDictionary viewData) { return (string)viewData[TokenUmbracoPath];