AllowedRoles attribute should be used with the full path instead of a…

… simple name. (#1783)
SenseNet · Aug 28, 2022 · 032f5b5 · 032f5b5
1 parent 448783d
commit 032f5b5
Show file tree

Hide file tree

Showing 5 changed files with 68 additions and 15 deletions.
diff --git a/src/ContentRepository/ApplicationModel/N.cs b/src/ContentRepository/ApplicationModel/N.cs
@@ -33,11 +33,11 @@ public static class CT
         /// </summary>
         public static class R
         {
-            public const string Administrators = "Administrators";
-            public const string Developers = "Developers";
-            public const string Everyone = "Everyone";
-            public const string IdentifiedUsers = "IdentifiedUsers";
-            public const string Visitor = "Visitor";
+            public const string Administrators = "/Root/IMS/BuiltIn/Portal/Administrators";
+            public const string Developers = "/Root/IMS/BuiltIn/Portal/Developers";
+            public const string Everyone = "/Root/IMS/BuiltIn/Portal/Everyone";
+            public const string IdentifiedUsers = "/Root/IMS/BuiltIn/Portal/IdentifiedUsers";
+            public const string Visitor = "/Root/IMS/BuiltIn/Portal/Visitor";
             public const string All = "All";
         }
 

diff --git a/src/OData/OperationInspector.cs b/src/OData/OperationInspector.cs
@@ -1,4 +1,5 @@
-using System.Collections.Generic;
+using System;
+using System.Collections.Generic;
 using System.Linq;
 using SenseNet.Configuration;
 using SenseNet.ContentRepository;
@@ -30,14 +31,14 @@ public virtual bool CheckByRoles(string[] expectedRoles, IEnumerable<string> act
             var userId = User.Current.Id;
             if (User.Current.Id == Identifiers.SystemUserId)
                 return true;
-            if (userId == Identifiers.VisitorUserId && expectedRoles.Contains("Visitor"))
+            if (userId == Identifiers.VisitorUserId && expectedRoles.Contains("/Root/IMS/BuiltIn/Portal/Visitor", StringComparer.InvariantCultureIgnoreCase))
                 return true;
             if (expectedRoles.Contains("All"))
                 return true;
 
             if (actualRoles == null)
-                actualRoles = NodeHead.Get(Providers.Instance.SecurityHandler.GetGroups()).Select(y => y.Name).ToArray();
-            return actualRoles.Intersect(expectedRoles).Any();
+                actualRoles = NodeHead.Get(Providers.Instance.SecurityHandler.GetGroups()).Select(y => y.Path).ToArray();
+            return actualRoles.Intersect(expectedRoles, StringComparer.InvariantCultureIgnoreCase).Any();
         }
 
         public virtual bool CheckByPermissions(Content content, string[] permissions)

diff --git a/src/OData/OperationMethodStorage.cs b/src/OData/OperationMethodStorage.cs
@@ -21,7 +21,7 @@ public OperationMethodStorage(OperationInspector inspector)
 
         public IEnumerable<ActionBase> GetActions(IEnumerable<ActionBase> storedActions, Content content, string scenario, object state)
         {
-            var actualRoles =  NodeHead.Get(Providers.Instance.SecurityHandler.GetGroups()).Select(y => y.Name).ToArray();
+            var actualRoles =  NodeHead.Get(Providers.Instance.SecurityHandler.GetGroups()).Select(y => y.Path).ToArray();
             var inspector = _inspector;
             var stored = storedActions.ToArray();
 

diff --git a/src/Tests/SenseNet.ODataTests/ODataOperationMethodTests.cs b/src/Tests/SenseNet.ODataTests/ODataOperationMethodTests.cs
@@ -203,7 +203,8 @@ public void OD_MBO_GetInfo_Attributes_SnAuthorize()
             ODataTest(() =>
             {
                 var info = AddMethod(typeof(TestOperations).GetMethod("Op2"));
-                Assert.AreEqual("Administrators,Editors,Visitor", ArrayToString(info.Roles, true));
+                Assert.AreEqual("/Root/IMS/BuiltIn/Portal/Administrators,path/Editors,path/Visitor",
+                    ArrayToString(info.Roles, true));
                 Assert.AreEqual("P1,P2,P3", ArrayToString(info.Permissions, true));
                 Assert.AreEqual("Policy1,Policy2,Policy3", ArrayToString(info.Policies, true));
             });
@@ -1379,7 +1380,7 @@ public void OD_MBO_Call_Inspection()
                 var lines = inspector.Log.Split(new[] {'\r', '\n'}, StringSplitOptions.RemoveEmptyEntries);
                 Assert.AreEqual(4, lines.Length);
                 Assert.AreEqual("CheckContentType: User ==>, User,Group,OrgUnit", lines[0]);
-                Assert.AreEqual("CheckByRoles: 1, Administrators,Editors", lines[1]);
+                Assert.AreEqual("CheckByRoles: 1, /Root/IMS/BuiltIn/Portal/Administrators,path/Editors", lines[1]);
                 Assert.AreEqual("CheckByPermissions: 0, 1, See,Run", lines[2]);
                 Assert.AreEqual("CheckPolicies: 1, Policy1", lines[3]);
             });
@@ -2637,7 +2638,7 @@ public void OD_MBO_Actions_Authorization_Membership()
                     var m1 = AddMethod(new TestMethodInfo("fv1", "Content content, string a", null),
                         new Attribute[] {new ODataAction(), new AllowedRolesAttribute(N.R.Developers) });
                     var m3 = AddMethod(new TestMethodInfo("fv2", "Content content, string a", null),
-                        new Attribute[] {new ODataAction(), new AllowedRolesAttribute("Developers,Administrators")});
+                        new Attribute[] { new ODataAction(), new AllowedRolesAttribute($"{N.R.Administrators},{N.R.Developers}") });
                     var m4 = AddMethod(new TestMethodInfo("fv3", "Content content, string a", null),
                         new Attribute[] {new ODataAction(), new AllowedRolesAttribute(N.R.Developers, "UnknownGroup42")});
 
@@ -2664,6 +2665,57 @@ public void OD_MBO_Actions_Authorization_Membership()
             });
         }
 
+        [TestMethod]
+        public void OD_MBO_Actions_Authorization_Membership_Admin()
+        {
+            ODataTest(() =>
+            {
+                using (new CleanOperationCenterBlock())
+                {
+                    var publicDomain = Node.LoadNode("/Root/IMS/Public");
+                    var user1 = new User(publicDomain) {Name = "User1", Email = "user1@example.com", Enabled = true};
+                    user1.Save();
+                    var publicAdmins = Node.Load<Group>("/Root/IMS/Public/Administrators");
+                    publicAdmins.AddMember(user1);
+                    new SecurityHandler().CreateAclEditor()
+                        .Allow(Repository.ImsFolder.Id, publicAdmins.Id, false, PermissionType.BuiltInPermissionTypes)
+                        .Apply();
+
+                    var m0 = AddMethod(new TestMethodInfo("fv0", "Content content, string a", null),
+                        new Attribute[] { new ODataAction(), new AllowedRolesAttribute(N.R.Administrators) });
+                    var m1 = AddMethod(new TestMethodInfo("fv1", "Content content, string a", null),
+                        new Attribute[] { new ODataAction(), new AllowedRolesAttribute(publicAdmins.Path) });
+
+                    using (new CurrentUserBlock(User.Administrator))
+                    {
+                        // ACTION-1
+                        var response = ODataGetAsync("/OData.svc/Root('IMS')/Actions", "")
+                            .ConfigureAwait(false).GetAwaiter().GetResult();
+
+                        // ASSERT-1
+                        AssertNoError(response);
+                        var entities = GetEntities(response);
+                        var operationNames = entities.Select(x => x.Name).OrderBy(x => x).ToArray();
+                        Assert.IsTrue(operationNames.Contains("fv0"));
+                        Assert.IsFalse(operationNames.Contains("fv1"));
+                    }
+                    using (new CurrentUserBlock(user1))
+                    {
+                        // ACTION-2
+                        var response = ODataGetAsync("/OData.svc/Root('IMS')/Actions", "")
+                            .ConfigureAwait(false).GetAwaiter().GetResult();
+
+                        // ASSERT-2
+                        AssertNoError(response);
+                        var entities = GetEntities(response);
+                        var operationNames = entities.Select(x => x.Name).OrderBy(x => x).ToArray();
+                        Assert.IsFalse(operationNames.Contains("fv0"));
+                        Assert.IsTrue(operationNames.Contains("fv1"));
+                    }
+                }
+            });
+        }
+
         [TestMethod]
         public void OD_MBO_Actions_Authorization_Permission()
         {

diff --git a/src/Tests/SenseNet.ODataTests/TestTypes.cs b/src/Tests/SenseNet.ODataTests/TestTypes.cs
@@ -29,7 +29,7 @@ public class TestOperations
 
         [ODataFunction]
         [ContentTypes(N.CT.User, N.CT.Group, "OrgUnit")]
-        [AllowedRoles(N.R.Administrators, "Editors")]
+        [AllowedRoles(N.R.Administrators, "path/Editors")]
         [RequiredPolicies("Policy1")]
         [RequiredPermissions("See, Run")]
         [Scenario("Scenario1, Scenario2")]
@@ -42,7 +42,7 @@ public static object[] Op1(Content content,
 
         [ODataAction]
         [ContentTypes(N.CT.GenericContent, N.CT.ContentType, N.CT.File)] // Causes no content type check ("File" is redundant).
-        [AllowedRoles(N.R.Administrators, "Editors", "Editors,Visitor")]
+        [AllowedRoles(N.R.Administrators, "path/Editors", "path/Editors,path/Visitor")]
         [RequiredPolicies("Policy1,Policy2", "Policy2,Policy3")]
         [RequiredPermissions("P1, P2", "P3")]
         public static object[] Op2(Content content,