Fix coding style and handle case where empty roles & schemes are empty
This commit is contained in:
parent
ee6a57e9a2
commit
9a5da5861b
|
|
@ -85,23 +85,27 @@ namespace Microsoft.AspNet.Authorization
|
|||
policyBuilder.Combine(policy);
|
||||
useDefaultPolicy = false;
|
||||
}
|
||||
var rolesSplit = authorizeAttribute.Roles?.Split(',');
|
||||
var rolesSplit = authorizeAttribute.Roles?.Split(new char[] { ',' }, StringSplitOptions.RemoveEmptyEntries);
|
||||
if (rolesSplit != null && rolesSplit.Any())
|
||||
{
|
||||
for (int i = 0; i < rolesSplit.Length; ++i)
|
||||
rolesSplit[i] = rolesSplit[i]?.Trim();
|
||||
{
|
||||
rolesSplit[i] = rolesSplit[i].Trim();
|
||||
}
|
||||
|
||||
policyBuilder.RequireRole(rolesSplit);
|
||||
policyBuilder.RequireRole(rolesSplit.Where(r => !string.IsNullOrWhiteSpace(r)));
|
||||
useDefaultPolicy = false;
|
||||
}
|
||||
var authTypesSplit = authorizeAttribute.ActiveAuthenticationSchemes?.Split(',');
|
||||
var authTypesSplit = authorizeAttribute.ActiveAuthenticationSchemes?.Split(new char[] { ',' }, StringSplitOptions.RemoveEmptyEntries);
|
||||
if (authTypesSplit != null && authTypesSplit.Any())
|
||||
{
|
||||
foreach (var authType in authTypesSplit)
|
||||
{
|
||||
if (string.IsNullOrEmpty(authType))
|
||||
continue;
|
||||
policyBuilder.AuthenticationSchemes.Add(authType.Trim());
|
||||
var trimmedAuthType = authType.Trim();
|
||||
if(!string.IsNullOrWhiteSpace(trimmedAuthType))
|
||||
{
|
||||
policyBuilder.AuthenticationSchemes.Add(trimmedAuthType);
|
||||
}
|
||||
}
|
||||
}
|
||||
if (useDefaultPolicy)
|
||||
|
|
|
|||
|
|
@ -77,8 +77,10 @@ namespace Microsoft.AspNet.Authroization.Test
|
|||
};
|
||||
var options = new AuthorizationOptions();
|
||||
|
||||
// Act
|
||||
var combined = AuthorizationPolicy.Combine(options, attributes);
|
||||
|
||||
// Assert
|
||||
Assert.True(combined.Requirements.Any(r => r is RolesAuthorizationRequirement));
|
||||
var rolesAuthorizationRequirement = combined.Requirements.OfType<RolesAuthorizationRequirement>().First();
|
||||
Assert.Equal(2, rolesAuthorizationRequirement.AllowedRoles.Count());
|
||||
|
|
@ -95,11 +97,51 @@ namespace Microsoft.AspNet.Authroization.Test
|
|||
};
|
||||
var options = new AuthorizationOptions();
|
||||
|
||||
// Act
|
||||
var combined = AuthorizationPolicy.Combine(options, attributes);
|
||||
|
||||
// Assert
|
||||
Assert.Equal(2, combined.AuthenticationSchemes.Count());
|
||||
Assert.True(combined.AuthenticationSchemes.Any(a => a.Equals("a1")));
|
||||
Assert.True(combined.AuthenticationSchemes.Any(a => a.Equals("a2")));
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void CombineMustIgnoreEmptyAuthenticationScheme()
|
||||
{
|
||||
// Arrange
|
||||
var attributes = new AuthorizeAttribute[] {
|
||||
new AuthorizeAttribute() { ActiveAuthenticationSchemes = "a1 , , ,,, a2" }
|
||||
};
|
||||
var options = new AuthorizationOptions();
|
||||
|
||||
// Act
|
||||
var combined = AuthorizationPolicy.Combine(options, attributes);
|
||||
|
||||
// Assert
|
||||
Assert.Equal(2, combined.AuthenticationSchemes.Count());
|
||||
Assert.True(combined.AuthenticationSchemes.Any(a => a.Equals("a1")));
|
||||
Assert.True(combined.AuthenticationSchemes.Any(a => a.Equals("a2")));
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void CombineMustIgnoreEmptyRoles()
|
||||
{
|
||||
// Arrange
|
||||
var attributes = new AuthorizeAttribute[] {
|
||||
new AuthorizeAttribute() { Roles = "r1 , ,, , r2" }
|
||||
};
|
||||
var options = new AuthorizationOptions();
|
||||
|
||||
// Act
|
||||
var combined = AuthorizationPolicy.Combine(options, attributes);
|
||||
|
||||
// Assert
|
||||
Assert.True(combined.Requirements.Any(r => r is RolesAuthorizationRequirement));
|
||||
var rolesAuthorizationRequirement = combined.Requirements.OfType<RolesAuthorizationRequirement>().First();
|
||||
Assert.Equal(2, rolesAuthorizationRequirement.AllowedRoles.Count());
|
||||
Assert.True(rolesAuthorizationRequirement.AllowedRoles.Any(r => r.Equals("r1")));
|
||||
Assert.True(rolesAuthorizationRequirement.AllowedRoles.Any(r => r.Equals("r2")));
|
||||
}
|
||||
}
|
||||
}
|
||||
Loading…
Reference in New Issue