diff --git a/src/Microsoft.AspNet.Authorization/AuthorizationPolicy.cs b/src/Microsoft.AspNet.Authorization/AuthorizationPolicy.cs index 096caeecd9..befcb3c277 100644 --- a/src/Microsoft.AspNet.Authorization/AuthorizationPolicy.cs +++ b/src/Microsoft.AspNet.Authorization/AuthorizationPolicy.cs @@ -88,7 +88,9 @@ namespace Microsoft.AspNet.Authorization var rolesSplit = authorizeAttribute.Roles?.Split(','); if (rolesSplit != null && rolesSplit.Any()) { - policyBuilder.RequireRole(rolesSplit); + var trimmedRolesSplit = rolesSplit.Where(r => !string.IsNullOrWhiteSpace(r)).Select(r => r.Trim()); + + policyBuilder.RequireRole(trimmedRolesSplit); useDefaultPolicy = false; } var authTypesSplit = authorizeAttribute.ActiveAuthenticationSchemes?.Split(','); @@ -96,7 +98,10 @@ namespace Microsoft.AspNet.Authorization { foreach (var authType in authTypesSplit) { - policyBuilder.AuthenticationSchemes.Add(authType); + if (!string.IsNullOrWhiteSpace(authType)) + { + policyBuilder.AuthenticationSchemes.Add(authType.Trim()); + } } } if (useDefaultPolicy) diff --git a/test/Microsoft.AspNet.Authorization.Test/AuthorizationPolicyFacts.cs b/test/Microsoft.AspNet.Authorization.Test/AuthorizationPolicyFacts.cs index 03eccd2a62..97430a1215 100644 --- a/test/Microsoft.AspNet.Authorization.Test/AuthorizationPolicyFacts.cs +++ b/test/Microsoft.AspNet.Authorization.Test/AuthorizationPolicyFacts.cs @@ -67,5 +67,81 @@ namespace Microsoft.AspNet.Authroization.Test Assert.False(combined.Requirements.Any(r => r is DenyAnonymousAuthorizationRequirement)); Assert.Equal(2, combined.Requirements.OfType().Count()); } + + [Fact] + public void CombineMustTrimRoles() + { + // 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().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"))); + } + + [Fact] + public void CombineMustTrimAuthenticationScheme() + { + // 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 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().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"))); + } } } \ No newline at end of file