Merge remote-tracking branch 'ph1/Roles-PR' into dev

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)

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<ClaimsAuthorizationRequirement>().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<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")));
+        }
+
+        [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<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")));
+        }
     }
 }