Skip to content

Commit

Permalink
Cleanup UserFilters (#17081)
Browse files Browse the repository at this point in the history
  • Loading branch information
MikeAlhayek authored Nov 27, 2024
1 parent b6f5d98 commit 2355c10
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 27 deletions.
27 changes: 0 additions & 27 deletions src/OrchardCore.Modules/OrchardCore.Users/Liquid/UserFilters.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,7 @@
using Fluid.Values;
using Microsoft.AspNetCore.Http;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Logging;
using OrchardCore.Liquid;
using OrchardCore.Security;
using OrchardCore.Security.Permissions;

namespace OrchardCore.Users.Liquid;

Expand All @@ -29,30 +26,6 @@ public static ValueTask<FluidValue> HasClaim(FluidValue input, FilterArguments a
{
return BooleanValue.True;
}

// The following if condition was added in 2.1 for backward compatibility. It should be removed in v3 and documented as a breaking change.
// The change log should state the following:
// The `Administrator` role no longer registers permission-based claims by default during login. This means that directly checking for specific claims in Liquid, such as:
//
// ```liquid
// {% assign isAuthorized = User | has_claim: "Permission", "AccessAdminPanel" %}
// ```
//
// will return `false` for administrators, even though they still have full access. Non-admin users, however, may return `true` if they have the claim.
// it's important to use the `has_permission` filter for permission checks going forward:
//
// ```liquid
// {% assign isAuthorized = User | has_permission: "AccessAdminPanel" %}
// ```
if (string.Equals(claimType, Permission.ClaimType, StringComparison.OrdinalIgnoreCase) &&
user.HasClaim(StandardClaims.SiteOwner.Type, StandardClaims.SiteOwner.Value))
{
var logger = context.Services.GetRequiredService<ILogger<Startup>>();

logger.LogWarning("The tenant is using the 'has_claim' Liquid filter for Permission claims '{ClaimName}', which will break in the next major release of OrchardCore; please use 'has_permission: \"{ClaimName}\"' instead.", claimName, claimName);

return BooleanValue.True;
}
}
}

Expand Down
14 changes: 14 additions & 0 deletions src/docs/releases/3.0.0.md
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,20 @@ The `ExternalLogin` action has been removed from the `Account` controller. If yo

The `AssignRoleToUsers` permission is no longer implicitly granted by `EditUsers`. To maintain the same behavior, make sure to explicitly assign the `AssignRoleToUsers` permission to any role that already has the `EditUsers` permission.

#### The Behavior of 'has_claim' Liquid Filter Changed.

The `Administrator` role no longer registers permission-based claims by default during login. This means that directly checking for specific claims in Liquid, such as:

```liquid
{% assign isAuthorized = User | has_claim: "Permission", "AccessAdminPanel" %}
```

will return `false` for administrators, even though they still have full access. Non-admin users, however, may return `true` if they have the claim. It's important to use the `has_permission` filter for permission checks going forward:

```liquid
{% assign isAuthorized = User | has_permission: "AccessAdminPanel" %}
```

### Sealing Types

Many type commonly used by classes can be `sealed`, which improves runtime performance. While it's not mandatory, we recommend that you consider applying this improvement to your own extensions as well. We've implemented this enhancement in pull request [#16897](https://github.com/OrchardCMS/OrchardCore/pull/16897).
Expand Down

0 comments on commit 2355c10

Please sign in to comment.