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

Create system roles by default #17341

Merged
merged 33 commits into from
Jan 21, 2025
Merged
Show file tree
Hide file tree
Changes from 32 commits
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
bd65eed
Create system roles by default
hishamco Jan 12, 2025
151ad09
Add missing roles
hishamco Jan 12, 2025
6117d19
Remove roles from *.recipe.json
hishamco Jan 12, 2025
da19704
Address feedback
hishamco Jan 13, 2025
718d098
Merge branch 'main' into hishamco/system-roles
hishamco Jan 13, 2025
a79ade2
Create a separate data migration
hishamco Jan 13, 2025
5e21655
Remove system roles from migrations.recipe.json
hishamco Jan 13, 2025
e5e0c0d
Address feedback
hishamco Jan 13, 2025
915a554
Merge branch 'main' into hishamco/system-roles
hishamco Jan 13, 2025
5b70ef2
Remove roles from pages.recipe.json
hishamco Jan 14, 2025
784e912
Introduce ISystemRoleProvider
hishamco Jan 15, 2025
7ee0c37
Remove warnings
hishamco Jan 15, 2025
9d38a6c
Merge branch 'main' into hishamco/system-roles
hishamco Jan 15, 2025
56ffeb3
Apply suggestions from code review
hishamco Jan 15, 2025
cb0fd77
localize the description
hishamco Jan 18, 2025
0a49bae
Merge branch 'main' into hishamco/system-roles
hishamco Jan 18, 2025
7a2da40
Remove obsolete from DefaultSystemRoleNameProvider
hishamco Jan 18, 2025
b97848f
Don't depend on role position
hishamco Jan 18, 2025
9026f46
Add GetAdminRoleAsync() to ISystemRoleProvider
hishamco Jan 18, 2025
fa7775a
Merge branch 'main' into hishamco/system-roles
hishamco Jan 18, 2025
48a3719
Remove ISystemRoleNameProvider injection
hishamco Jan 18, 2025
3f104ab
Add more unit tests
hishamco Jan 18, 2025
674c862
Fix the build
hishamco Jan 18, 2025
748e6d8
localizer -> stringLocalizer
hishamco Jan 18, 2025
313651e
Obsolete SystemRoleNameProviderExtensions
hishamco Jan 18, 2025
d2245e3
Address feedback
hishamco Jan 19, 2025
2470fb7
Fix warnings
hishamco Jan 19, 2025
7b7ba19
Apply suggestions from code review
hishamco Jan 20, 2025
b3c15a6
Fix the build
hishamco Jan 20, 2025
26177f3
Remove unnecessary local field
hishamco Jan 20, 2025
fd7e657
Remove extra cast
hishamco Jan 20, 2025
ea580d0
cleanup for approval
MikeAlhayek Jan 21, 2025
1791300
Fix tests
MikeAlhayek Jan 21, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
using Microsoft.AspNetCore.Identity;
using Microsoft.Extensions.Logging;
using OrchardCore.Data.Migration;
using OrchardCore.Security;

namespace OrchardCore.Roles.Migrations;

public sealed class SystemRolesMigrations : DataMigration
{
private readonly ISystemRoleProvider _systemRoleProvider;
private readonly RoleManager<IRole> _roleManager;
private readonly ILogger _logger;

public SystemRolesMigrations(
ISystemRoleProvider systemRoleProvider,
RoleManager<IRole> roleManager,
ILogger<RolesMigrations> logger)
{
_systemRoleProvider = systemRoleProvider;
_roleManager = roleManager;
_logger = logger;
}

public async Task<int> CreateAsync()
{
var systemRoles = _systemRoleProvider.GetSystemRoles();

foreach (var systemRole in systemRoles)
{
if (!await _roleManager.RoleExistsAsync(systemRole.RoleName))
{
await _roleManager.CreateAsync(systemRole);
}
}

_logger.LogInformation("The system roles have been created successfully.");

return 1;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -13,15 +13,15 @@ namespace OrchardCore.Roles.Recipes;
public sealed class RolesStep : NamedRecipeStepHandler
{
private readonly RoleManager<IRole> _roleManager;
private readonly ISystemRoleNameProvider _systemRoleNameProvider;
private readonly ISystemRoleProvider _systemRoleProvider;

public RolesStep(
RoleManager<IRole> roleManager,
ISystemRoleNameProvider systemRoleNameProvider)
ISystemRoleProvider systemRoleProvider)
: base("Roles")
{
_roleManager = roleManager;
_systemRoleNameProvider = systemRoleNameProvider;
_systemRoleProvider = systemRoleProvider;
}

protected override async Task HandleAsync(RecipeExecutionContext context)
Expand Down Expand Up @@ -59,7 +59,7 @@ protected override async Task HandleAsync(RecipeExecutionContext context)
r.RoleClaims.RemoveAll(c => c.ClaimType == Permission.ClaimType);
}

if (!await _systemRoleNameProvider.IsAdminRoleAsync(roleName))
if (!_systemRoleProvider.IsAdminRole(roleName))
{
if (roleEntry.PermissionBehavior == PermissionBehavior.Remove)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,18 +11,18 @@ public class RoleClaimsProvider : IUserClaimsProvider
{
private readonly UserManager<IUser> _userManager;
private readonly RoleManager<IRole> _roleManager;
private readonly ISystemRoleNameProvider _systemRoleNameProvider;
private readonly ISystemRoleProvider _systemRoleProvider;
private readonly IdentityOptions _identityOptions;

public RoleClaimsProvider(
UserManager<IUser> userManager,
RoleManager<IRole> roleManager,
ISystemRoleNameProvider systemRoleNameProvider,
ISystemRoleProvider systemRoleProvider,
IOptions<IdentityOptions> identityOptions)
{
_userManager = userManager;
_roleManager = roleManager;
_systemRoleNameProvider = systemRoleNameProvider;
_systemRoleProvider = systemRoleProvider;
_identityOptions = identityOptions.Value;
}

Expand All @@ -39,7 +39,7 @@ public async Task GenerateAsync(IUser user, ClaimsIdentity claims)

foreach (var roleName in roleNames)
{
if (await _systemRoleNameProvider.IsAdminRoleAsync(roleName))
if (_systemRoleProvider.IsAdminRole(roleName))
{
isAdministrator = true;
break;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,16 +13,17 @@ namespace OrchardCore.Roles.Services;
public class RoleStore : IRoleClaimStore<IRole>, IQueryableRoleStore<IRole>
{
private readonly IServiceProvider _serviceProvider;
private readonly ISystemRoleNameProvider _systemRoleProvider;
private readonly ISystemRoleProvider _systemRoleProvider;
private readonly IDocumentManager<RolesDocument> _documentManager;
protected readonly IStringLocalizer S;
private readonly ILogger _logger;

protected readonly IStringLocalizer S;

private bool _updating;

public RoleStore(
IServiceProvider serviceProvider,
ISystemRoleNameProvider systemRoleProvider,
ISystemRoleProvider systemRoleProvider,
IDocumentManager<RolesDocument> documentManager,
IStringLocalizer<RoleStore> stringLocalizer,
ILogger<RoleStore> logger)
Expand Down Expand Up @@ -91,7 +92,7 @@ public async Task<IdentityResult> DeleteAsync(IRole role, CancellationToken canc
});
}

if (await _systemRoleProvider.IsSystemRoleAsync(roleToRemove.RoleName))
if (_systemRoleProvider.IsSystemRole(roleToRemove.RoleName))
{
return IdentityResult.Failed(new IdentityError
{
Expand Down
24 changes: 12 additions & 12 deletions src/OrchardCore.Modules/OrchardCore.Roles/Services/RoleUpdater.cs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ public class RoleUpdater : FeatureEventHandler, IRoleCreatedEventHandler, IRoleR
private readonly ShellDescriptor _shellDescriptor;
private readonly IExtensionManager _extensionManager;
private readonly IDocumentManager<RolesDocument> _documentManager;
private readonly ISystemRoleNameProvider _systemRoleNameProvider;
private readonly ISystemRoleProvider _systemRoleProvider;
private readonly IEnumerable<IPermissionProvider> _permissionProviders;
private readonly ITypeFeatureProvider _typeFeatureProvider;
private readonly ILogger _logger;
Expand All @@ -26,15 +26,15 @@ public RoleUpdater(
ShellDescriptor shellDescriptor,
IExtensionManager extensionManager,
IDocumentManager<RolesDocument> documentManager,
ISystemRoleNameProvider systemRoleNameProvider,
ISystemRoleProvider systemRoleProvider,
IEnumerable<IPermissionProvider> permissionProviders,
ITypeFeatureProvider typeFeatureProvider,
ILogger<RoleUpdater> logger)
{
_shellDescriptor = shellDescriptor;
_extensionManager = extensionManager;
_documentManager = documentManager;
_systemRoleNameProvider = systemRoleNameProvider;
_systemRoleProvider = systemRoleProvider;
_permissionProviders = permissionProviders;
_typeFeatureProvider = typeFeatureProvider;
_logger = logger;
Expand Down Expand Up @@ -80,7 +80,7 @@ private async Task UpdateRolesForInstalledFeatureAsync(IFeatureInfo feature)
var permissions = (stereotype.Permissions ?? [])
.Select(stereotype => stereotype.Name);

if (await UpdatePermissionsAsync(role, permissions))
if (UpdatePermissions(role, permissions))
{
updated = true;
}
Expand Down Expand Up @@ -121,7 +121,7 @@ private async Task UpdateRolesForEnabledFeatureAsync(IFeatureInfo feature)
updated = true;

missingFeatures.Remove(feature.Id);
await UpdateRolesForEnabledFeatureAsync(role, providers);
UpdateRolesForEnabledFeature(role, providers);
}

if (updated)
Expand Down Expand Up @@ -166,7 +166,7 @@ private async Task UpdateRoleForInstalledFeaturesAsync(string roleName)
.SelectMany(stereotype => stereotype.Permissions ?? [])
.Select(stereotype => stereotype.Name);

await UpdatePermissionsAsync(role, permissions);
UpdatePermissions(role, permissions);
}

private async Task RemoveRoleForMissingFeaturesAsync(string roleName)
Expand All @@ -179,15 +179,15 @@ private async Task RemoveRoleForMissingFeaturesAsync(string roleName)
}
}

private Task<bool> UpdateRolesForEnabledFeatureAsync(Role role, IEnumerable<IPermissionProvider> providers)
private bool UpdateRolesForEnabledFeature(Role role, IEnumerable<IPermissionProvider> providers)
{
var stereotypes = providers
.SelectMany(provider => provider.GetDefaultStereotypes())
.Where(stereotype => string.Equals(stereotype.Name, role.RoleName, StringComparison.OrdinalIgnoreCase));

if (!stereotypes.Any())
{
return Task.FromResult(false);
return false;
}

var permissions = stereotypes
Expand All @@ -196,15 +196,15 @@ private Task<bool> UpdateRolesForEnabledFeatureAsync(Role role, IEnumerable<IPer

if (!permissions.Any())
{
return Task.FromResult(false);
return false;
}

return UpdatePermissionsAsync(role, permissions);
return UpdatePermissions(role, permissions);
}

private async Task<bool> UpdatePermissionsAsync(Role role, IEnumerable<string> permissions)
private bool UpdatePermissions(Role role, IEnumerable<string> permissions)
{
if (await _systemRoleNameProvider.IsAdminRoleAsync(role.RoleName))
if (_systemRoleProvider.IsAdminRole(role.RoleName))
{
// Don't update claims for admin role.
return true;
Expand Down
3 changes: 3 additions & 0 deletions src/OrchardCore.Modules/OrchardCore.Roles/Startup.cs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,10 @@ public Startup(IShellConfiguration shellConfiguration)
public override void ConfigureServices(IServiceCollection services)
{
services.AddScoped<IUserClaimsProvider, RoleClaimsProvider>();

services.AddDataMigration<SystemRolesMigrations>();
services.AddDataMigration<RolesMigrations>();

services.AddScoped<RoleStore>();
services.Replace(ServiceDescriptor.Scoped<IRoleClaimStore<IRole>>(sp => sp.GetRequiredService<RoleStore>()));
services.Replace(ServiceDescriptor.Scoped<IRoleStore<IRole>>(sp => sp.GetRequiredService<RoleStore>()));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,14 @@ namespace OrchardCore.Settings.Services;
public class SuperUserHandler : IAuthorizationHandler
{
private readonly ISiteService _siteService;
private readonly ISystemRoleNameProvider _systemRoleNameProvider;
private readonly ISystemRoleProvider _systemRoleProvider;

public SuperUserHandler(
ISiteService siteService,
ISystemRoleNameProvider systemRoleNameProvider)
ISystemRoleProvider systemRoleProvider)
{
_siteService = siteService;
_systemRoleNameProvider = systemRoleNameProvider;
_systemRoleProvider = systemRoleProvider;
}

public async Task HandleAsync(AuthorizationHandlerContext context)
Expand All @@ -30,7 +30,9 @@ public async Task HandleAsync(AuthorizationHandlerContext context)
return;
}

if (user.IsInRole(await _systemRoleNameProvider.GetAdminRoleAsync()))
var adminRole = _systemRoleProvider.GetAdminRole();

if (user.IsInRole(adminRole.RoleName))
{
SucceedAllRequirements(context);

Expand Down
15 changes: 0 additions & 15 deletions src/OrchardCore.Themes/TheAdmin/Recipes/blank.recipe.json
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The SaaS recipe still has these. pages.recipe.json too but I'm not sure how that's used.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Me too :) but seems used the module for test project to test Assembly attributes if I recall

Original file line number Diff line number Diff line change
Expand Up @@ -69,11 +69,6 @@
{
"name": "Roles",
"Roles": [
{
"Name": "Administrator",
"Description": "A system role that grants all permissions to the assigned users.",
"Permissions": []
},
{
"Name": "Moderator",
"Description": "Grants users the ability to moderate content.",
Expand All @@ -93,16 +88,6 @@
"Name": "Contributor",
"Description": "Grants users the ability to contribute content.",
"Permissions": []
},
{
"Name": "Authenticated",
"Description": "A system role representing all authenticated users.",
"Permissions": []
},
{
"Name": "Anonymous",
"Description": "A system role representing all non-authenticated users.",
"Permissions": []
}
]
}
Expand Down
10 changes: 0 additions & 10 deletions src/OrchardCore.Themes/TheAdmin/Recipes/headless.recipe.json
Original file line number Diff line number Diff line change
Expand Up @@ -64,11 +64,6 @@
{
"name": "Roles",
"Roles": [
{
"Name": "Administrator",
"Description": "A system role that grants all permissions to the assigned users.",
"Permissions": []
},
{
"Name": "Moderator",
"Description": "Grants users the ability to moderate content.",
Expand Down Expand Up @@ -97,11 +92,6 @@
"ExecuteGraphQL",
"ExecuteApiAll"
]
},
{
"Name": "Anonymous",
"Description": "A system role representing all non-authenticated users.",
"Permissions": []
}
]
},
Expand Down
15 changes: 0 additions & 15 deletions src/OrchardCore.Themes/TheAgencyTheme/Recipes/agency.recipe.json
Original file line number Diff line number Diff line change
Expand Up @@ -73,11 +73,6 @@
{
"name": "Roles",
"Roles": [
{
"Name": "Administrator",
"Description": "A system role that grants all permissions to the assigned users.",
"Permissions": []
},
{
"Name": "Moderator",
"Description": "Grants users the ability to moderate content.",
Expand All @@ -97,16 +92,6 @@
"Name": "Contributor",
"Description": "Grants users the ability to contribute content.",
"Permissions": []
},
{
"Name": "Authenticated",
"Description": "A system role representing all authenticated users.",
"Permissions": []
},
{
"Name": "Anonymous",
"Description": "A system role representing all non-authenticated users.",
"Permissions": []
}
]
},
Expand Down
15 changes: 0 additions & 15 deletions src/OrchardCore.Themes/TheBlogTheme/Recipes/blog.recipe.json
Original file line number Diff line number Diff line change
Expand Up @@ -87,11 +87,6 @@
{
"name": "Roles",
"Roles": [
{
"Name": "Administrator",
"Description": "A system role that grants all permissions to the assigned users.",
"Permissions": []
},
{
"Name": "Moderator",
"Description": "Grants users the ability to moderate content.",
Expand All @@ -111,16 +106,6 @@
"Name": "Contributor",
"Description": "Grants users the ability to contribute content.",
"Permissions": []
},
{
"Name": "Authenticated",
"Description": "A system role representing all authenticated users.",
"Permissions": []
},
{
"Name": "Anonymous",
"Description": "A system role representing all non-authenticated users.",
"Permissions": []
}
]
},
Expand Down
Loading
Loading