Skip to content

Commit

Permalink
Fixing FormatException when the login screen is posted with values ot…
Browse files Browse the repository at this point in the history
…her than true/false for RememberMe (Lombiq Technologies: OCORE-132) (OrchardCMS#14845)

* Fixing FormatException when the login screen is posted with values other than true/false for RememberMe

* Changing anti-cracking attempt model binding to use a custom model binder instead

* Moving SafeBoolModelBinder to OrchardCore.Mvc.Core

* Sealing internal classes

* Formatting

Co-authored-by: Hisham Bin Ateya <[email protected]>

---------

Co-authored-by: Hisham Bin Ateya <[email protected]>
  • Loading branch information
2 people authored and urbanit committed Mar 18, 2024
1 parent e2c995c commit 6c1b370
Show file tree
Hide file tree
Showing 5 changed files with 62 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
using Microsoft.Extensions.Localization;
using Microsoft.Extensions.Logging;
using OrchardCore.DisplayManagement.Notify;
using OrchardCore.Entities;
using OrchardCore.Modules;
using OrchardCore.Mvc.Core.Utilities;
using OrchardCore.Settings;
Expand Down
2 changes: 1 addition & 1 deletion src/OrchardCore.Modules/OrchardCore.Users/Startup.cs
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@
using OrchardCore.Admin.Models;
using OrchardCore.Data;
using OrchardCore.Data.Migration;
using OrchardCore.DisplayManagement.Descriptors;
using OrchardCore.Deployment;
using OrchardCore.DisplayManagement.Descriptors;
using OrchardCore.DisplayManagement.Handlers;
using OrchardCore.DisplayManagement.Theming;
using OrchardCore.Environment.Commands;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
using System;
using System.Threading.Tasks;
using Microsoft.AspNetCore.Mvc.ModelBinding;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Localization;

namespace OrchardCore.Mvc.ModelBinding;

/// <summary>
/// Model binder to produce a validation error when a Boolean field contains a value that's not a valid bool. The
/// default model binder would throw a <see cref="FormatException"/>. That's an issue for e.g. the Users module, see
/// <see href="https://github.com/OrchardCMS/OrchardCore/issues/14792"/>.
/// </summary>
internal sealed class SafeBoolModelBinder : IModelBinder
{
public Task BindModelAsync(ModelBindingContext bindingContext)
{
ArgumentNullException.ThrowIfNull(nameof(bindingContext));

var valueProviderResult = bindingContext.ValueProvider.GetValue(bindingContext.ModelName);

if (valueProviderResult == ValueProviderResult.None)
{
return Task.CompletedTask;
}

if (bool.TryParse(valueProviderResult.FirstValue, out bool result))
{
bindingContext.Result = ModelBindingResult.Success(result);

return Task.CompletedTask;
}

var localizer = bindingContext.HttpContext.RequestServices.GetService<IStringLocalizer<SafeBoolModelBinder>>();

bindingContext.ModelState.TryAddModelError(bindingContext.ModelName, localizer["Invalid Boolean value."]);

return Task.CompletedTask;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
using System;
using Microsoft.AspNetCore.Mvc.ModelBinding;

namespace OrchardCore.Mvc.ModelBinding;

internal sealed class SafeBoolModelBinderProvider : IModelBinderProvider
{
public IModelBinder GetBinder(ModelBinderProviderContext context)
{
ArgumentNullException.ThrowIfNull(context, nameof(context));

if (context.Metadata.ModelType == typeof(bool))
{
return new SafeBoolModelBinder();
}

return null;
}
}
2 changes: 2 additions & 0 deletions src/OrchardCore/OrchardCore.Mvc.Core/Startup.cs
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,8 @@ public override void ConfigureServices(IServiceCollection services)
// Custom model binder to testing purpose
options.ModelBinderProviders.Insert(0, new CheckMarkModelBinderProvider());
options.ModelBinderProviders.Insert(0, new SafeBoolModelBinderProvider());
});

// Add a route endpoint selector policy.
Expand Down

0 comments on commit 6c1b370

Please sign in to comment.