I think we need to make a few more modifications to Rhino Security. Break it away from IRepository<T>, for a start, and maybe allow to use it using Common Service Locator?
On Sat, Dec 13, 2008 at 3:17 PM, Ayende Rahien <[email protected]> wrote: > Applied with a minor change to make this pass: > > [Test] > public void > WillReturnTrueOnGlobalIfPermissionWasAllowedOnGlobalButDeniedOnSpecificEntity() > { > permissionsBuilderService > .Allow("/Account/Edit") > .For(user) > .OnEverything() > .DefaultLevel() > .Save(); > UnitOfWork.Current.TransactionalFlush(); > > permissionsBuilderService > .Deny("/Account/Edit") > .For(user) > .On(account) > .DefaultLevel() > .Save(); > UnitOfWork.Current.TransactionalFlush(); > > bool IsAllowed = authorizationService.IsAllowed(user, > "/Account/Edit"); > Assert.IsTrue(IsAllowed); > > } > > On Sat, Dec 13, 2008 at 5:14 PM, Bart Reyserhove < > [email protected]> wrote: > >> 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 -~----------~----~----~----~------~----~------~--~---
