Skip to content

Commit

Permalink
Roles with no permission are auto migrated to Administrator roles (#1…
Browse files Browse the repository at this point in the history
  • Loading branch information
MikeAlhayek authored Dec 12, 2024
1 parent b13f229 commit 913046f
Show file tree
Hide file tree
Showing 3 changed files with 67 additions and 7 deletions.
1 change: 1 addition & 0 deletions mkdocs.yml
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ validation:
# Add files here that are intentionally not in the navigation and thus omitted_files shouldn't warn about them.
not_in_nav: |
samples/
releases/2.1.3.md
# Extensions
markdown_extensions:
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
using Microsoft.AspNetCore.Identity;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Logging;
using Microsoft.Extensions.Options;
using OrchardCore.BackgroundJobs;
using OrchardCore.Data.Migration;
Expand All @@ -15,10 +16,14 @@ public sealed class RolesMigrations : DataMigration
private static readonly string _alternativeAdminRoleName = "SiteOwner";

private readonly SystemRoleOptions _systemRoleOptions;
private readonly ILogger _logger;

public RolesMigrations(IOptions<SystemRoleOptions> systemRoleOptions)
public RolesMigrations(
IOptions<SystemRoleOptions> systemRoleOptions,
ILogger<RolesMigrations> logger)
{
_systemRoleOptions = systemRoleOptions.Value;
_logger = logger;
}

#pragma warning disable CA1822 // Mark members as static
Expand All @@ -41,10 +46,8 @@ public int Create()
continue;
}

// When a new tenant is created, the RoleClaims will be empty for Admin roles.
var hasSiteOwner = r.RoleClaims is null ||
r.RoleClaims.Count == 0 ||
r.RoleClaims.Any(x => x.ClaimValue == "SiteOwner");
// Check to see if the role contains the obsolete 'SiteOwner' permission claim.
var hasSiteOwner = r.RoleClaims is not null && r.RoleClaims.Any(x => x.ClaimValue == "SiteOwner");

if (r.RoleName.Equals(OrchardCoreConstants.Roles.Administrator, StringComparison.OrdinalIgnoreCase))
{
Expand All @@ -54,16 +57,24 @@ public int Create()
// We'll need to create a new role name that does not exists and assign it as the system 'Administrator' role.
adminSystemRoleName = GenerateNewAdminRoleName(roles);

_logger.LogInformation("The {DefaultAdministratorRoleName} does not contain SiteOwner permission. Creating a new AdminRoleName as the system admin name. The new role name is {NewAdminRoleName}.", OrchardCoreConstants.Roles.Administrator, adminSystemRoleName);

await roleManager.CreateAsync(new Role
{
RoleName = adminSystemRoleName,
});

}
else
{
_logger.LogInformation("Removing all existing permission claims from the default {DefaultAdministratorRoleName} Administrator name.", OrchardCoreConstants.Roles.Administrator);

r.RoleClaims.Clear();

await roleManager.UpdateAsync(r);

// Don't processed to avoid adding the default 'Administrator' role to the adminRoles collection;
continue;
}
}

Expand All @@ -84,9 +95,14 @@ await HttpBackgroundJob.ExecuteAfterEndOfRequestAsync("MigrateAdminUsersToNewAdm
{
var users = await userManager.GetUsersInRoleAsync(adminRole.RoleName);

foreach (var user in users)
if (users.Count > 0)
{
await userManager.AddToRoleAsync(user, adminSystemRoleName);
_logger.LogInformation("Migrating all users {Count} users from {PreviousRoleName} to {NewRoleName}", users.Count, adminRole, adminSystemRoleName);

foreach (var user in users)
{
await userManager.AddToRoleAsync(user, adminSystemRoleName);
}
}
}
});
Expand All @@ -98,6 +114,8 @@ await HttpBackgroundJob.ExecuteAfterEndOfRequestAsync("MigrateAdminUsersToNewAdm
var shellSettings = scope.ServiceProvider.GetRequiredService<ShellSettings>();
var shellHost = scope.ServiceProvider.GetRequiredService<IShellHost>();

_logger.LogInformation("The {DefaultAdministratorRoleName} does not contain SiteOwner permission. Creating a new AdminRoleName as the system admin name and storing it in the tenant app settings provider. The new name is {NewAdminRoleName}", OrchardCoreConstants.Roles.Administrator, adminSystemRoleName);

shellSettings["AdminRoleName"] = adminSystemRoleName;

await shellHost.UpdateShellSettingsAsync(shellSettings);
Expand Down
41 changes: 41 additions & 0 deletions src/docs/releases/2.1.3.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
## Orchard Core 2.1.3

**Release Date:** Not yet released

This release includes critical security fixes that grant unintentionally full access to certain users.

## Changelog

### Important Security Notice: Role Assignment Issue After Upgrade

If you have recently upgraded from a previous version of Orchard Core to version 2.1.0, 2.0.1, or 2.0.2, please be aware of a potential security issue that may impact your system. Specifically, if a role is assigned with **no permissions**, any user assigned to that role will automatically be granted the **Administrator** role after the upgrade, potentially giving them full access to your site.

#### Example Scenario:

Let's say your app includes a role named **Director**, which has no permissions assigned. If a user like **Mike** is assigned to this role, after the upgrade, **Mike** will automatically be granted the **Administrator** role, giving them full control over your site. This could pose a significant security risk if not addressed promptly.

#### Mitigation Steps:

To ensure your site's security, we strongly recommend that you review the users who currently have the **Administrator** role. If you find users who should not have administrative privileges, you should remove the **Administrator** role from their account immediately.

#### How to Check Users Assigned to the Administrator Role:

1. **Identify the Administrator Role:**
- Go to the `/Admin/Roles/Index` page on your site. Check if the **Administrator** role has a **System** badge.

- If the **Administrator** role **has the System badge**, proceed with the following steps:

1. Go to the `/Admin/Users/Index?q=role:Administrator` page.
2. Review the list of users assigned the **Administrator** role.
3. For any users who should not have **Administrator** privileges, click **Edit** on their account and remove the role.

- If the **Administrator** role **does not have the System badge**, follow these steps:

1. Look for the role with the **System** badge. This role is typically named **SiteOwner** or something similar (e.g., **SiteOwner1**, **SiteOwner2**, etc.).
2. Once identified, replace `Site__Owner_RoleName_Goes_Here` with the correct role name and visit the `/Admin/Users/Index?q=role:{Site__Owner_RoleName_Goes_Here}` page on your site.
3. Review the list of users with the site owner role.
4. For any users who should not have site owner privileges, click **Edit** on their account and remove the role.

### Final Reminder:

We recommend that you take immediate action to verify user roles and ensure that only authorized users have administrative or site owner access. Failing to do so could expose your application to significant security risks.

0 comments on commit 913046f

Please sign in to comment.