Patch attached. I also added two new tests and changed the expectation of the one discussed earlier in this thread.
Bart On Fri, Dec 12, 2008 at 11:59 PM, Ayende Rahien <[email protected]> wrote: > I think that this is a test that should be expected to fail after this > change. > > > On Fri, Dec 12, 2008 at 11:30 PM, Bart Reyserhove < > [email protected]> wrote: > >> So I changed this method: >> /// <summary> >> /// Gets the permissions for the specified etntity >> /// </summary> >> /// <param name="user">The user.</param> >> /// <param name="operationName">Name of the operation.</param> >> /// <returns></returns> >> public Permission[] GetPermissionsFor(IUser user, string operationName) >> { >> string[] operationNames = Strings.GetHierarchicalOperationNames( >> operationName); >> DetachedCriteria criteria = DetachedCriteria.For<Permission>() >> .Add(Expression.Eq("User", user) >> || Subqueries.PropertyIn("UsersGroup.Id", >> SecurityCriterions.AllGroups( >> user).SetProjection(Projections.Id()))) >> .CreateAlias("Operation", "op") >> .Add(Expression.In("op.Name", operationNames)); >> >> return FindResults(criteria); >> } >> >> into: >> >> /// <summary> >> /// Gets the global permissions without taking groups into account >> /// </summary> >> /// <param name="user">The user.</param> >> /// <param name="operationName">Name of the operation.</param> >> /// <returns></returns> >> public Permission[] GetGlobalPermissionsFor(IUser user, string >> operationName) >> { >> string[] operationNames = >> Strings.GetHierarchicalOperationNames(operationName); >> DetachedCriteria criteria = DetachedCriteria.For<Permission>() >> .Add(Expression.Eq("User", user) >> || Subqueries.PropertyIn("UsersGroup.Id", >> >> SecurityCriterions.AllGroups(user).SetProjection(Projections.Id()))) >> .Add(Expression.IsNull("EntitiesGroup")) >> .CreateAlias("Operation", "op") >> .Add(Expression.In("op.Name", operationNames)); >> >> return FindResults(criteria); >> } >> >> I added a test for it and I ran all tests. Two tests are failing now, but >> it is actually only one case since it is tested for NHibernate and >> ActiveRecord. >> >> The failing test: >> >> [Test] >> public void >> ExplainWhyAllowedIfPermissionWasGrantedToUsersGroupAssociatedWithUser() >> { >> permissionsBuilderService >> .Allow("/Account/Edit") >> .For("Administrators") >> .On("Important Accounts") >> .DefaultLevel() >> .Save(); >> UnitOfWork.Current.TransactionalFlush(); >> AuthorizationInformation information = >> authorizationService.GetAuthorizationInformation(user, >> "/Account/Edit"); >> string expected = >> @"Permission (level 1) for operation '/Account/Edit' was >> granted to group 'Administrators' on 'Important Accounts' ('Ayende' is a >> member of 'Administrators') >> "; >> Assert.AreEqual(expected, information.ToString()); >> } >> >> Knowing the change this is quite logic, because in this test the >> permission is only granted to a group and checked globally. Functionally >> seen you could discuss about this. Permission was only granted to the >> "Important accounts" and the question is "OnEverything". So one could argue >> that it is correct that the permission is not granted, but of course this >> could lead to breaking changes... >> >> What's your opinion? >> >> Bart >> >> >> On Fri, Dec 12, 2008 at 8:27 AM, Bart Reyserhove < >> [email protected]> wrote: >> >>> All right. I'll make a patch this weekend. >>> >>> >>> On Thu, Dec 11, 2008 at 10:21 PM, Ayende Rahien <[email protected]>wrote: >>> >>>> Yes, so in this case, you are right.What we need to do is to change the >>>> name of the method to GetGlobalPermissionsFor >>>> >>>> >>>> On Thu, Dec 11, 2008 at 3:33 PM, Bart Reyserhove < >>>> [email protected]> wrote: >>>> >>>>> I ask the question: IsAllowed(IUser user, string operation) >>>>> I checked where GetPermissionsFor(IUser user, string operationName) is >>>>> used and it is used in the following two methods: >>>>> >>>>> /// <summary> >>>>> /// Determines whether the specified user is allowed to perform the >>>>> /// specified operation on the entity. >>>>> /// </summary> >>>>> /// <param name="user">The user.</param> >>>>> /// <param name="operation">The operation.</param> >>>>> /// <returns> >>>>> /// <c>true</c> if the specified user is allowed; otherwise, >>>>> <c>false</c>. >>>>> /// </returns> >>>>> public bool IsAllowed(IUser user, string operation) >>>>> { >>>>> Permission[] permissions = permissionsService.GetPermissionsFor(user, >>>>> operation); >>>>> if (permissions.Length == 0) >>>>> return false; >>>>> return permissions[0].Allow; >>>>> } >>>>> >>>>> /// <summary> >>>>> /// Gets the authorization information for the specified user and >>>>> operation, >>>>> /// allows to easily understand why a given operation was granted / >>>>> denied. >>>>> /// </summary> >>>>> /// <param name="user">The user.</param> >>>>> /// <param name="operation">The operation.</param> >>>>> /// <returns></returns> >>>>> public AuthorizationInformation GetAuthorizationInformation(IUser user, >>>>> string operation) >>>>> { >>>>> AuthorizationInformation info; >>>>> if (InitializeAuthorizationInfo(operation, out info)) >>>>> return info; >>>>> Permission[] permissions = permissionsService.GetPermissionsFor(user, >>>>> operation); >>>>> AddPermissionDescriptionToAuthorizationInformation<object>(operation, >>>>> info, user, permissions, null); >>>>> return info; >>>>> } >>>>> >>>>> Both methods above do not take the entity into accoutn since the entity >>>>> is not passed. >>>>> >>>>> I understand that you have to take the situation into account where the >>>>> permission is directly given on the entity but shouldn't I use >>>>> IsAllowed<TEntity>(IUser >>>>> user, TEntity entity, string operation) where TEntity : class then? >>>>> >>>>> Bart >>>>> >>>>> >>>>> On Thu, Dec 11, 2008 at 4:56 PM, Ayende Rahien <[email protected]>wrote: >>>>> >>>>>> What question are you asking? >>>>>> Get permissions isn't actually that useful for the user, it is the >>>>>> question that you ask when you want to know if it has a permission to do >>>>>> X >>>>>> on anything. Anything include entities group. >>>>>> The question that you are likely asking is IsAllowed(), and in this >>>>>> case, I think that you are correct, if it is allowed to do this in >>>>>> general, >>>>>> it is allowed to do so. >>>>>> The problem is that I am not sure that your solution is the >>>>>> appropriate one. >>>>>> GetPermissionsFor is used by the Rhino Security infrastructure, and we >>>>>> need to review this to make sure it doesn't break things. >>>>>> For that matter, we also need to handle the case where a permission >>>>>> was given directly on an entity. >>>>>> >>>>>> >>>>>> On Thu, Dec 11, 2008 at 3:18 AM, Bart Reyserhove < >>>>>> [email protected]> wrote: >>>>>> >>>>>>> The "PermissionsService" contains the following method: >>>>>>> /// <summary> >>>>>>> /// Gets the permissions for the specified etntity >>>>>>> /// </summary> >>>>>>> /// <param name="user">The user.</param> >>>>>>> /// <param name="operationName">Name of the operation.</param> >>>>>>> /// <returns></returns> >>>>>>> public Permission[] GetPermissionsFor(IUser user, string >>>>>>> operationName) >>>>>>> { >>>>>>> string[] operationNames = >>>>>>> Strings.GetHierarchicalOperationNames(operationName); >>>>>>> DetachedCriteria criteria = DetachedCriteria.For<Permission>() >>>>>>> .Add(Expression.Eq("User", user) >>>>>>> || Subqueries.PropertyIn("UsersGroup.Id", >>>>>>> >>>>>>> SecurityCriterions.AllGroups(user).SetProjection(Projections.Id()))) >>>>>>> >>>>>>> .CreateAlias("Operation", "op") >>>>>>> .Add(Expression.In("op.Name", operationNames)); >>>>>>> >>>>>>> return FindResults(criteria); >>>>>>> } >>>>>>> >>>>>>> This method returns all permissions for a user for a certain >>>>>>> operation. With "All" I mean also the ones that are defined on >>>>>>> entitiesgroups. I have for example an operation "/Department/List" with >>>>>>> the >>>>>>> permission "on everything" set to "allow" and with the permission on >>>>>>> group >>>>>>> "departments of company x" set on "deny". Now if the user wants to >>>>>>> access >>>>>>> the "http://localhost/Department/List"; it is not allowed because >>>>>>> Rhino.Security finds a "deny" permission for a certain entitiesgroup. >>>>>>> In my >>>>>>> opinion it should not take the permissions on entititiesgroups into >>>>>>> account >>>>>>> in this case. It could also be that I have configured it in the wrong >>>>>>> way of >>>>>>> course. >>>>>>> >>>>>>> This fixed it for me: >>>>>>> >>>>>>> /// <summary> >>>>>>> /// Gets the permissions for the specified etntity >>>>>>> /// </summary> >>>>>>> /// <param name="user">The user.</param> >>>>>>> /// <param name="operationName">Name of the operation.</param> >>>>>>> /// <returns></returns> >>>>>>> public Permission[] GetPermissionsFor(IUser user, string >>>>>>> operationName) >>>>>>> { >>>>>>> string[] operationNames = >>>>>>> Strings.GetHierarchicalOperationNames(operationName); >>>>>>> DetachedCriteria criteria = DetachedCriteria.For<Permission>() >>>>>>> .Add(Expression.Eq("User", user) >>>>>>> || Subqueries.PropertyIn("UsersGroup.Id", >>>>>>> >>>>>>> SecurityCriterions.AllGroups(user).SetProjection(Projections.Id()))) >>>>>>> .Add(Expression.IsNull("EntitiesGroup")) >>>>>>> .CreateAlias("Operation", "op") >>>>>>> .Add(Expression.In("op.Name", operationNames)); >>>>>>> >>>>>>> return FindResults(criteria); >>>>>>> } >>>>>>> >>>>>>> Bart >>>>>>> >>>>>>> >>>>>>> >>>>>>> >>>>>> >>>>>> >>>>>> >>>>> >>>>> >>>>> >>>> >>>> >>>> >>> >> >> >> > > > > --~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "Rhino Tools Dev" group. To post to this group, send email to [email protected] To unsubscribe from this group, send email to [email protected] For more options, visit this group at http://groups.google.com/group/rhino-tools-dev?hl=en -~----------~----~----~----~------~----~------~--~---
RhinoSecurityGlobalPermissionsPatch.patch
Description: Binary data
