Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

V13: The login screen does not display external login errors #15715

Merged
merged 3 commits into from
Feb 14, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
@inject IOptions<GlobalSettings> globalSettings
@inject IRuntimeMinifier runtimeMinifier
@inject IProfilerHtml profilerHtml
@inject IBackOfficeExternalLoginProviders externalLogins
@{
bool.TryParse(Context.Request.Query["umbDebug"], out bool isDebug);
var backOfficePath = globalSettings.Value.GetBackOfficePath(hostingEnvironment);
Expand Down Expand Up @@ -114,8 +113,6 @@

<script>
document.angularReady = function(app) {
@await Html.AngularValueExternalLoginInfoScriptAsync(externalLogins, ViewData.GetExternalSignInProviderErrors()!)
@Html.AngularValueResetPasswordCodeInfoScript(ViewData[ViewDataExtensions.TokenPasswordResetCode]!)
@await Html.AngularValueTinyMceAssetsAsync(runtimeMinifier)
//required for the noscript trick
document.getElementById("mainwrapper").style.display = "inherit";
Expand Down
10 changes: 10 additions & 0 deletions src/Umbraco.Cms.StaticAssets/umbraco/UmbracoLogin/Index.cshtml
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
var disableLocalLogin = ExternalLogins.HasDenyLocalLogin();
var externalLoginsUrl = LinkGenerator.GetPathByAction(nameof(BackOfficeController.ExternalLogin), ControllerExtensions.GetControllerName<BackOfficeController>(), new { area = Constants.Web.Mvc.BackOfficeArea });
var externalLoginProviders = await ExternalLogins.GetBackOfficeProvidersAsync();
var externalSignInErrors = ViewData.GetExternalSignInProviderErrors();
}
@addTagHelper *, Microsoft.AspNetCore.Mvc.TagHelpers

Expand Down Expand Up @@ -106,6 +107,15 @@
custom-view="@provider.ExternalLoginProvider.Options.CustomBackOfficeView">
</umb-external-login-provider>
}
@if (externalSignInErrors?.Errors?.Count() > 0)
{
<div slot="subheadline" style="color:var(--uui-color-danger-standalone)">
@foreach (var error in externalSignInErrors.Errors)
{
<div>@error</div>
}
</div>
}
</umb-auth>
</umb-backoffice-icon-registry>

Expand Down
13 changes: 9 additions & 4 deletions src/Umbraco.Web.BackOffice/Controllers/BackOfficeController.cs
Original file line number Diff line number Diff line change
Expand Up @@ -426,7 +426,8 @@
if (user == null)
{
// ... this should really not happen
return RedirectToLogin(new { flow = "external-login", status = "localUserNotFound" });
TempData[ViewDataExtensions.TokenExternalSignInError] = new[] { "Local user does not exist" };
return RedirectToLogin(new { flow = "external-login", status = "localUserNotFound", logout = "true"});
}

ExternalLoginInfo? info =
Expand All @@ -435,7 +436,9 @@
if (info == null)
{
// Add error and redirect for it to be displayed
return RedirectToLogin(new { flow = "external-login", status = "externalLoginInfoNotFound" });
TempData[ViewDataExtensions.TokenExternalSignInError] =
new[] { "An error occurred, could not get external login info" };
return RedirectToLogin(new { flow = "external-login", status = "externalLoginInfoNotFound", logout = "true"});
}

IdentityResult addLoginResult = await _userManager.AddLoginAsync(user, info);
Expand All @@ -448,7 +451,8 @@
}

// Add errors and redirect for it to be displayed
return RedirectToLogin(new { flow = "external-login", status = "failed" });
TempData[ViewDataExtensions.TokenExternalSignInError] = addLoginResult.Errors;
return RedirectToLogin(new { flow = "external-login", status = "failed", logout = "true" });
}

/// <summary>
Expand All @@ -469,8 +473,9 @@
_httpContextAccessor.HttpContext,
ViewDataExtensions.TokenExternalSignInError,
_jsonSerializer) ||
ViewData.FromTempData(TempData, ViewDataExtensions.TokenExternalSignInError) || ViewData.FromTempData(TempData, ViewDataExtensions.TokenPasswordResetCode))
ViewData.FromTempData(TempData, ViewDataExtensions.TokenExternalSignInError))

Check notice on line 476 in src/Umbraco.Web.BackOffice/Controllers/BackOfficeController.cs

View check run for this annotation

CodeScene Delta Analysis / CodeScene Cloud Delta Analysis (v13/dev)

✅ No longer an issue: Complex Conditional

RenderDefaultOrProcessExternalLoginAsync no longer has a complex conditional. A complex conditional is an expression inside a branch (e.g. if, for, while) which consists of multiple, logical operators such as AND/OR. The more logical operators in an expression, the more severe the code smell.
{
// Return early to let the client side handle the messaging

Check notice on line 478 in src/Umbraco.Web.BackOffice/Controllers/BackOfficeController.cs

View check run for this annotation

CodeScene Delta Analysis / CodeScene Cloud Delta Analysis (v13/dev)

✅ No longer an issue: Complex Method

RenderDefaultOrProcessExternalLoginAsync is no longer above the threshold for cyclomatic complexity. This function has many conditional statements (e.g. if, for, while), leading to lower code health. Avoid adding more conditionals and code to it without refactoring.
return defaultResponse();
}

Expand Down
2 changes: 2 additions & 0 deletions src/Umbraco.Web.Common/Extensions/ViewDataExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -122,9 +122,11 @@ public static bool FromTempData(this ViewDataDictionary viewData, ITempDataDicti
this ViewDataDictionary viewData,
BackOfficeExternalLoginProviderErrors errors) => viewData[TokenExternalSignInError] = errors;

[Obsolete("This is deprecated and will be removed in V15")]
public static string? GetPasswordResetCode(this ViewDataDictionary viewData) =>
(string?)viewData[TokenPasswordResetCode];

[Obsolete("This is deprecated and will be removed in V15")]
public static void SetPasswordResetCode(this ViewDataDictionary viewData, string value) =>
viewData[TokenPasswordResetCode] = value;

Expand Down