Fix missing Trim in Roles and Schemes split

muhammetfaik · Dec 28, 2015 · 6850e3b · 6850e3b
1 parent 2d21b72
commit 6850e3b
Show file tree

Hide file tree

Showing 2 changed files with 40 additions and 1 deletion.
diff --git a/src/Microsoft.AspNet.Authorization/AuthorizationPolicy.cs b/src/Microsoft.AspNet.Authorization/AuthorizationPolicy.cs
@@ -88,6 +88,9 @@ public static AuthorizationPolicy Combine(AuthorizationOptions options, IEnumera
                 var rolesSplit = authorizeAttribute.Roles?.Split(',');
                 if (rolesSplit != null && rolesSplit.Any())
                 {
+                    for (int i = 0; i < rolesSplit.Length; ++i)
+                        rolesSplit[i] = rolesSplit[i]?.Trim();
+
                     policyBuilder.RequireRole(rolesSplit);
                     useDefaultPolicy = false;
                 }
@@ -96,7 +99,9 @@ public static AuthorizationPolicy Combine(AuthorizationOptions options, IEnumera
                 {
                     foreach (var authType in authTypesSplit)
                     {
-                        policyBuilder.AuthenticationSchemes.Add(authType);
+                        if (string.IsNullOrEmpty(authType))
+                            continue;
+                        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
@@ -67,5 +67,39 @@ public void CanReplaceDefaultPolicy()
             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("2") { Roles = "r1 , r2" }
+            };
+            var options = new AuthorizationOptions();
+
+            var combined = AuthorizationPolicy.Combine(options, attributes);
+
+            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("2") { ActiveAuthenticationSchemes = "a1 , a2" }
+            };
+            var options = new AuthorizationOptions();
+
+            var combined = AuthorizationPolicy.Combine(options, attributes);
+
+            Assert.Equal(2, combined.AuthenticationSchemes.Count());
+            Assert.True(combined.AuthenticationSchemes.Any(a => a.Equals("a1")));
+            Assert.True(combined.AuthenticationSchemes.Any(a => a.Equals("a2")));
+        }
     }
 }