From 89cd1f84f23d78d5051fb98e3bac202875e3b983 Mon Sep 17 00:00:00 2001 From: Jacob Overgaard <752371+iOvergaard@users.noreply.github.com> Date: Thu, 30 Nov 2023 15:04:41 +0100 Subject: [PATCH 1/5] add check for the presence of the ?logout=true parameter on the login screen to avoid autoredirect to external login --- .../Controllers/BackOfficeController.cs | 21 ++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/src/Umbraco.Web.BackOffice/Controllers/BackOfficeController.cs b/src/Umbraco.Web.BackOffice/Controllers/BackOfficeController.cs index 134a89372fd9..1f5707fa4cf7 100644 --- a/src/Umbraco.Web.BackOffice/Controllers/BackOfficeController.cs +++ b/src/Umbraco.Web.BackOffice/Controllers/BackOfficeController.cs @@ -486,11 +486,22 @@ private async Task RenderDefaultOrProcessExternalLoginAsync( // if the user is not logged in, check if there's any auto login redirects specified if (!authenticateResult.Succeeded) { - var oauthRedirectAuthProvider = _externalLogins.GetAutoLoginProvider(); - if (!oauthRedirectAuthProvider.IsNullOrWhiteSpace()) - { - return ExternalLogin(oauthRedirectAuthProvider!); - } + return defaultResponse(); + } + + var oauthRedirectAuthProvider = _externalLogins.GetAutoLoginProvider(); + + // If there's no auto login provider specified, then we'll render the default view + if (oauthRedirectAuthProvider.IsNullOrWhiteSpace()) + { + return defaultResponse(); + } + + // If the ?logout=true query string is not specified, then we'll redirect to the external login provider + // which will then redirect back to the ExternalLoginCallback action + if (Request.Query.TryGetValue("logout", out StringValues logout) == false || logout != "true") + { + return ExternalLogin(oauthRedirectAuthProvider); } return defaultResponse(); From 85fa51e091c654d68355457ef3c3e5431a81c74a Mon Sep 17 00:00:00 2001 From: Jacob Overgaard <752371+iOvergaard@users.noreply.github.com> Date: Thu, 30 Nov 2023 15:04:48 +0100 Subject: [PATCH 2/5] add using --- src/Umbraco.Web.BackOffice/Controllers/BackOfficeController.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Umbraco.Web.BackOffice/Controllers/BackOfficeController.cs b/src/Umbraco.Web.BackOffice/Controllers/BackOfficeController.cs index 1f5707fa4cf7..7bfb309872f3 100644 --- a/src/Umbraco.Web.BackOffice/Controllers/BackOfficeController.cs +++ b/src/Umbraco.Web.BackOffice/Controllers/BackOfficeController.cs @@ -11,6 +11,7 @@ using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; +using Microsoft.Extensions.Primitives; using Umbraco.Cms.Core; using Umbraco.Cms.Core.Cache; using Umbraco.Cms.Core.Configuration.Grid; From 811c28e16655f56bfa3c4868a31c6a70385e3578 Mon Sep 17 00:00:00 2001 From: Jacob Overgaard <752371+iOvergaard@users.noreply.github.com> Date: Thu, 30 Nov 2023 15:04:54 +0100 Subject: [PATCH 3/5] cleanup usings --- src/Umbraco.Web.BackOffice/Controllers/BackOfficeController.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Umbraco.Web.BackOffice/Controllers/BackOfficeController.cs b/src/Umbraco.Web.BackOffice/Controllers/BackOfficeController.cs index 7bfb309872f3..4fff954e7cbb 100644 --- a/src/Umbraco.Web.BackOffice/Controllers/BackOfficeController.cs +++ b/src/Umbraco.Web.BackOffice/Controllers/BackOfficeController.cs @@ -18,7 +18,6 @@ using Umbraco.Cms.Core.Configuration.Models; using Umbraco.Cms.Core.Hosting; using Umbraco.Cms.Core.Manifest; -using Umbraco.Cms.Core.Models; using Umbraco.Cms.Core.Security; using Umbraco.Cms.Core.Serialization; using Umbraco.Cms.Core.Services; From f6352148e4608163160533486bb06fd9fb6acbbb Mon Sep 17 00:00:00 2001 From: Jacob Overgaard <752371+iOvergaard@users.noreply.github.com> Date: Thu, 30 Nov 2023 15:05:58 +0100 Subject: [PATCH 4/5] refactor controller to eliminate need for duplicate defaultResponse and externalLoginResponse (they are always the same) --- .../Controllers/BackOfficeController.cs | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/src/Umbraco.Web.BackOffice/Controllers/BackOfficeController.cs b/src/Umbraco.Web.BackOffice/Controllers/BackOfficeController.cs index 4fff954e7cbb..5d0c7488c4de 100644 --- a/src/Umbraco.Web.BackOffice/Controllers/BackOfficeController.cs +++ b/src/Umbraco.Web.BackOffice/Controllers/BackOfficeController.cs @@ -145,7 +145,6 @@ public async Task Default() return await RenderDefaultOrProcessExternalLoginAsync( result, - () => defaultView, () => defaultView); } @@ -172,7 +171,6 @@ public async Task Login() return await RenderDefaultOrProcessExternalLoginAsync( result, - () => View(viewPath), () => View(viewPath)); } @@ -460,11 +458,9 @@ public async Task ExternalLinkLoginCallback() /// private async Task RenderDefaultOrProcessExternalLoginAsync( AuthenticateResult authenticateResult, - Func defaultResponse, - Func externalSignInResponse) + Func defaultResponse) { ArgumentNullException.ThrowIfNull(defaultResponse); - ArgumentNullException.ThrowIfNull(externalSignInResponse); ViewData.SetUmbracoPath(_globalSettings.GetUmbracoMvcArea(_hostingEnvironment)); @@ -483,8 +479,8 @@ private async Task RenderDefaultOrProcessExternalLoginAsync( if (loginInfo == null || loginInfo.Principal == null) { - // if the user is not logged in, check if there's any auto login redirects specified - if (!authenticateResult.Succeeded) + // If the user is not logged in, check if there's any auto login redirects specified + if (authenticateResult.Succeeded) { return defaultResponse(); } @@ -508,7 +504,7 @@ private async Task RenderDefaultOrProcessExternalLoginAsync( } // we're just logging in with an external source, not linking accounts - return await ExternalSignInAsync(loginInfo, externalSignInResponse); + return await ExternalSignInAsync(loginInfo, defaultResponse); } private async Task ExternalSignInAsync(ExternalLoginInfo loginInfo, Func response) From 79797027952c7eb32196c07285628df2374d2cd2 Mon Sep 17 00:00:00 2001 From: Jacob Overgaard <752371+iOvergaard@users.noreply.github.com> Date: Thu, 30 Nov 2023 15:13:51 +0100 Subject: [PATCH 5/5] refactor to reduce nesting --- .../Controllers/BackOfficeController.cs | 43 ++++++++++--------- 1 file changed, 22 insertions(+), 21 deletions(-) diff --git a/src/Umbraco.Web.BackOffice/Controllers/BackOfficeController.cs b/src/Umbraco.Web.BackOffice/Controllers/BackOfficeController.cs index 5d0c7488c4de..65d7c442f2c7 100644 --- a/src/Umbraco.Web.BackOffice/Controllers/BackOfficeController.cs +++ b/src/Umbraco.Web.BackOffice/Controllers/BackOfficeController.cs @@ -477,34 +477,35 @@ private async Task RenderDefaultOrProcessExternalLoginAsync( // First check if there's external login info, if there's not proceed as normal ExternalLoginInfo? loginInfo = await _signInManager.GetExternalLoginInfoAsync(); - if (loginInfo == null || loginInfo.Principal == null) + if (loginInfo != null) { - // If the user is not logged in, check if there's any auto login redirects specified - if (authenticateResult.Succeeded) - { - return defaultResponse(); - } - - var oauthRedirectAuthProvider = _externalLogins.GetAutoLoginProvider(); + // we're just logging in with an external source, not linking accounts + return await ExternalSignInAsync(loginInfo, defaultResponse); + } - // If there's no auto login provider specified, then we'll render the default view - if (oauthRedirectAuthProvider.IsNullOrWhiteSpace()) - { - return defaultResponse(); - } + // If we are authenticated then we can just render the default view + if (authenticateResult.Succeeded) + { + return defaultResponse(); + } - // If the ?logout=true query string is not specified, then we'll redirect to the external login provider - // which will then redirect back to the ExternalLoginCallback action - if (Request.Query.TryGetValue("logout", out StringValues logout) == false || logout != "true") - { - return ExternalLogin(oauthRedirectAuthProvider); - } + // If the user is not logged in, check if there's any auto login redirects specified + var oauthRedirectAuthProvider = _externalLogins.GetAutoLoginProvider(); + // If there's no auto login provider specified, then we'll render the default view + if (oauthRedirectAuthProvider.IsNullOrWhiteSpace()) + { return defaultResponse(); } - // we're just logging in with an external source, not linking accounts - return await ExternalSignInAsync(loginInfo, defaultResponse); + // If the ?logout=true query string is not specified, then we'll redirect to the external login provider + // which will then redirect back to the ExternalLoginCallback action + if (Request.Query.TryGetValue("logout", out StringValues logout) == false || logout != "true") + { + return ExternalLogin(oauthRedirectAuthProvider); + } + + return defaultResponse(); } private async Task ExternalSignInAsync(ExternalLoginInfo loginInfo, Func response)