扩展IPrincipal.IsInRole()

Question

在我的例子中，我需要为用户提供角色，但是需要给特定的部门提供角色，我不想实现一个新的授权机制，每当我想检查用户是否被授权到某个特定的部门时，它都要求我访问数据库。

因此，我想我只需要创建一个带有两个参数IsInRole的扩展方法(role, departmentId)。以下是确切的代码：

public static bool IsInRole(this IPrincipal principal, string role, int departmentId)
{
    var ident = principal?.Identity;
    if (ident == null)
        return false;

    if (departmentId <= 0)
        return false;

    var roleDeparments = ((ClaimsIdentity)ident).FindFirst($"{role} departments")?.Value;
    if (string.IsNullOrWhiteSpace(roleDeparments))
        return false;

    var inInRole = roleDeparments
        .Split(',')
        .Any(d => d == departmentId.ToString() || d == "all");

    return inInRole;
}

用户角色保存在db中，并在登录时获取一次，如果departmentId设置为NULL，则意味着该角色对于所有部门都是通用的。标识对象是在登录时使用以下方法创建的：

private ClaimsIdentity CreateIdentity(User user)
{
    var identity = new ClaimsIdentity(MyAuthentication.ApplicationCookie, ClaimsIdentity.DefaultNameClaimType, ClaimsIdentity.DefaultRoleClaimType);
    identity.AddClaim(new Claim("http://schemas.microsoft.com/accesscontrolservice/2010/07/claims/identityprovider", "Active Directory"));
    identity.AddClaim(new Claim(ClaimTypes.Name, user.Id));
    identity.AddClaim(new Claim(ClaimTypes.GivenName, user.Name));
    identity.AddClaim(new Claim("DepartmentId", user.DepartmentId.ToString()));

    var roles = user.UserRoles
        .GroupBy(r => r.Role);

    foreach (var role in roles)
    {
        var departments = role
            .Select(r => r.DepartmentId)
            .Distinct()
            .ToList();

        bool isAll = departments.Any(d => d == null);

        string departmentIds = isAll
            ? "all"
            : string.Join(",", departments);

        identity.AddClaim(new Claim($"{role.Key.Name} departments", departmentIds));
    }

    return identity;
}

现在，我可以简单地做这样的事情：

if (User.IsInRole("schedule viewer", 2))
{
   .....
}

这样做对吗？

Answer 1

IsInRole()

我不喜欢方法参数被验证的方式。由于零传播运算符降低了可读性，所以它不应该用于任何事情。如果对象是赋值的第一个，比如在检查var ident = principal?.Identity;时，有一个用于“安全地”访问一个属性的空传播运算符是可以的，但是我个人还是会坚持这样的组合空检查。

if (principal == null || principal.Identity == null) { return false; }  

但是为什么你要在方法开始的时候检查这个呢？我会首先检查简单类型，比如departmentId <= 0。

这里的零传播算子

var roleDeparments = ((ClaimsIdentity)ident).FindFirst($"{role} departments")?.Value;  

乍一看，如果代码的维护者不知道这是关于什么的，至少如果他/她没有像我这样好的眼睛的话，他/她可能会错过。乍一看，他/她无法理解您在这里“安全”地访问Value属性。

你能在这里发现错误吗？

var inInRole = roleDeparments
    .Split(',')
    .Any(d => d == departmentId.ToString() || d == "all");  

除了inInRole中的拼写错误之外，您还可以将departmentId.ToString()的结果存储在变量中，以签入Any()，但是编译器可能足够聪明地为您做到这一点。

省略单行{}语句的大括号是您的责任。我不会这么做的。如果我这么做的话，那就像if (departmentId <= 0) return false;一样

使用缩写会导致代码的可读性降低。你为什么不把ident改名为identity呢？

实施上述各点将导致

public static bool IsInRole(this IPrincipal principal, string role, int departmentId)
{
    if (departmentId <= 0) { return false; }
    if (principal == null || principal.Identity == null) { return false; }  
    
    var identity = principal.Identity;

    var claim = ((ClaimsIdentity)identity).FindFirst($"{role} departments");
    if (claim == null) { return false; }
    
    var roleDeparments = claim.Value;
    if (string.IsNullOrWhiteSpace(roleDeparments)) { return false; }

    var isInRole = roleDeparments
        .Split(',')
        .Any(d => d == departmentId.ToString() || d == "all");

    return isInRole;
}

CreateIdentity()

这让我想知道

bool isAll = departments.Any(d => d == null);  

阅读isAll和Any。重读你的问题后，我无意中发现

如果departmentId设置为NULL，则意味着该角色对所有部门都是通用的。

这是对评论的尖叫，因为代码的维护人员也应该知道这一点。再加上一个更好的名字，对于一个维护人员来说，这里发生的事情是清楚的。

// If one of the user.UserRoles has a DepartmentId which is null,
// the role is general for all departments
bool isGeneralForAllDepartments = departments.Any(d => d == null);  

string departmentIds = isGeneralForAllDepartments 
    ? "all"
    : string.Join(",", departments);