Fixing IsInRole Always False in a .NET MVC Project without Microsoft Identity

The GenericPrincipal constructor requires an array of roles for `IsInRole` to work. If null is passed in for that, then it will always return false, even if role claims are present on its identities.

Too long; didn’t read

To use roles with a custom instantiated GenericPrincipal, the second parameter of the constructor can’t be null:

- var principal = new GenericPrincipal(identity, null);
+ var principal = new GenericPrincipal(identity, new string[] {});

The IsInRole method on GenericPrincipal first checks a collection of roles before checking role claims. That’s the second parameter of the constructor. If that collection is null the method returns early with false, never even checking role claims.

Explanation

We’ve had a perplexing issue on a .net MVC application at work for a while where the user IsInRole() method always returns false. We finally figured out the problem yesterday.

Our project doesn’t use the standard Microsoft Identity methods for authentication. Our work uses Shibboleth for single sign-on, so we integrate with that for our authentication. With how it works, our application receives a bunch of server variables related to the authentication.

We register an AuthenticationHandler that gets the user id from the Shibboleth server variables and creates a Principal, adds it to an AuthenticationTicket and returns success. The key method is HandleAuthenticateAsync:

// ShibbolethAuthenticationHandler.cs

/// 
/// Method HandleAuthenticateAsync contains the logic to
/// authenticate the current user.
/// 
/// An AuthenticateResult with the result of the
/// authentication attempt.
protected override async Task HandleAuthenticateAsync()
{
    return await Task.Run(() => {
        // Try to get the user Id from the ShibbolethService.
        string? authenticateUID = _shibboleth.GetAuthenticateUID(Context, Options.SpoofAuthVariables);

        // If no user Id, then the user is not authenticated.
        if (authenticateUID is null)
        {
            return AuthenticateResult.NoResult();
        }

        // If the user is authenticated, add the user Id as the Name
        // claim and as our custom EmoryIdClaim.
        var claims = new List
        {
            new Claim(ClaimTypes.Name, authenticateUID),
            new Claim(EmoryIdClaimType, authenticateUID)
        };

        // Create a new Principal object and add it to the successful authentication ticket.
        ClaimsIdentity identity = new(claims, Scheme.Name);
        var principal = new System.Security.Principal.GenericPrincipal(identity, null);
        var ticket = new AuthenticationTicket(principal, Scheme.Name);
        return AuthenticateResult.Success(ticket);
    });
}

That _shibboleth variable is an instance of our ShibbolethService class that handles the Shibboleth-related server variable work.

Then we have a ClaimsTransformation class that looks up info about the authenticated user in the database and adds additional claims, including role claims. The key method is TransformAsync (“LakeTown” is the codename for the application):

// LakeTownClaimsTransformation.cs

/// 
/// The TransformAsync method adds a SchemeName Identity
/// to the Principal with application-related claims.
/// 
/// The currently-authenticated user.
/// A ClaimsPrincipal Task.
public async Task TransformAsync(ClaimsPrincipal principal)
{
    // …
    // Code getting the User from the database and creating a new ClaimsIdentity to add to the
    // the principal.
    // …

    // Add role claims for each Role on the db user.
    foreach(Role role in user.Roles)
    {
        identity.AddClaim(new Claim(ClaimTypes.Role, role.Name));
    }

    // Add StoreManager claims for each Store on the db user.
    if (identity.HasClaim(c => c.Type == ClaimTypes.Role && c.Value == Roles.StoreManagerRole))
    {
        foreach(Store store in user.Stores)
        {
            identity.AddClaim(new Claim(StoreManagerClaimType, store.Id.ToString()));
        }
    }

    // Add the new identity to the principal.
    principal.AddIdentity(identity);

    // Return the updated principal.
    return principal;
}

At this point, we expected the IsInRole() method to work, but it didn’t. In LakeTownClaimsTransformation, we’re working with a ClaimsPrincipal, whose IsInRole method looks like this:

// ClaimsPrincipal.cs

/// 
/// IsInRole answers the question: does an identity this principal possesses
/// contain a claim of type RoleClaimType where the value is '==' to the role.
/// 
/// The role to check for.
/// 'True' if a claim is found. Otherwise 'False'.
/// Each Identity has its own definition of the ClaimType that represents a role.
public virtual bool IsInRole(string role)
{
    for (int i = 0; i < _identities.Count; i++)
    {
        if (_identities[i] != null)
        {
            if (_identities[i].HasClaim(_identities[i].RoleClaimType, role))
            {
                return true;
            }
        }
    }

    return false;
}

(source: runtime/src/libraries/System.Security.Claims/src/System/Security/Claims/ClaimsPrincipal.cs at 765d8845dbd6fff2f91105fec6e4d2315ff32815 · dotnet/runtime)

But what we actually created in ShibbolethAuthenticationHandler was a GenericPrincipal, whose IsInRole method looks like this:

// GenericPrincipal.cs

public override bool IsInRole([NotNullWhen(true)] string? role)
{
    if (role == null || m_roles == null)
        return false;

    for (int i = 0; i < m_roles.Length; ++i)
    {
        if (string.Equals(m_roles[i], role, StringComparison.OrdinalIgnoreCase))
            return true;
    }

    // it may be the case a ClaimsIdentity was passed in as the IIdentity which may have contained claims, they need to be checked.
    return base.IsInRole(role);
}

(source: runtime/src/libraries/System.Security.Claims/src/System/Security/Claims/GenericPrincipal.cs at 765d8845dbd6fff2f91105fec6e4d2315ff32815 · dotnet/runtime)

It’s checking a collection of roles first and then calling the base ClaimsPrincipal.IsInRole method, but returns false if that collection is null. Which leads us to our problem, this line in: ShibbolethAuthenticationHandler:

// ShibbolethAuthenticationHandler.cs

var principal = new System.Security.Principal.GenericPrincipal(identity, null);

The second argument in that constructor is an array of roles. We can fix that by either passing in an array of roles, an empty array, or by creating a ClaimsPrincipal instead of a GenericPrincipal. But passing in null for that value effectively prevents any IsInRole checking.

We fixed by using a ClaimsPrincipal since the GenericPrincipal didn’t seem to add anything for us anyways:

- var principal = new System.Security.Principal.GenericPrincipal(identity, null);
+ var principal = new ClaimsPrincipal(identity);