When an identity is created it may belong to one or more roles.
Currently the OidcHandler has
public override Task TokenValidated(TokenValidatedContext context)
{
var identity = (ClaimsIdentity)context.Principal!.Identity!;
if (!string.IsNullOrWhiteSpace(options.OidcRoleClaimType) && options.OidcRoleMapping?.Count >= 0)
{
var role = identity.FindFirst(x => x.Type == options.OidcRoleClaimType)?.Value;
if (!string.IsNullOrWhiteSpace(role) && options.OidcRoleMapping.TryGetValue(role, out var permissions) && permissions != null)
{
foreach (var permission in permissions)
{
identity.AddClaim(new Claim(SquidexClaimTypes.Permissions, permission));
}
}
}
return base.TokenValidated(context);
}
which is grabbing the first role it sees and then trying to use that to match on the role mapping collection. I’m curious if you would be open to expanding this to either
- Matching a single role within the collection.
- Matching all roles in the collection.
Take the below example:
var options = new Options {
OidcRoleClaimType = "role",
OidcRoleMapping = new Dictionary<string, List<string>> {
{ "role1", new List<string> { "perm1", "perm2" } },
{ "role2", new List<string> { "perm3", "perm1" } }
}
};
var identity = new Identity(
new List<Claim> {
new Claim("role", "user"),
new Claim("role", "role3"),
new Claim("role", "role2"),
new Claim("role", "role1")
}
);
In the current handler, the first role user does not exist in the role mapping and thus no permissions are returned.
In option 1, we would match on the first role that exists in the mapping. In this case, role1 and the resulting permissions added would be ["perm1", "perm2"]
In option2, we would match on all roles that exist in the mapping. So, in this case both role1 and role2 exist in the mapping and the result would be ["perm1", "perm2", "perm3"]
While I think it would make sense to go with option #2, it does potentially result in a breaking change for people using the existing role mapping; this may result in more permissions being applied than anticipated. While it does fall more inline with rbac in general, it may be worthwhile to consider.
An example showing the current implementation and the options: https://gist.github.com/jrlost/adaf6e32502ee55ad7c401cfbde9fec5
