rave-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Chris Geer <ch...@cxtsoftware.com>
Subject Re: [Proposal] Spring Permissions Change
Date Thu, 04 Oct 2012 15:32:14 GMT
On Thu, Oct 4, 2012 at 8:22 AM, Carlucci, Tony <acarlucci@mitre.org> wrote:

> >-----Original Message-----
> >From: Chris Geer [mailto:chris@cxtsoftware.com]
> >Sent: Thursday, October 04, 2012 11:08 AM
> >To: dev@rave.apache.org
> >Subject: Re: [Proposal] Spring Permissions Change
> >
> >Tony,
> >
> >On Thu, Oct 4, 2012 at 6:58 AM, Carlucci, Tony <acarlucci@mitre.org>
> wrote:
> >
> >> Hi Chris, could you go a little more into your use case (I think what
> >> you've hinted at it with your Widget->add_comment block)?  I believe the
> >> spirit of that Permission enum was to define the context of the security
> >> check to keep in line with CRUD actions.  The detailed business logic of
> >> the Model/Permission Context combination can then be customized as
> needed
> >> in the various Default<Model>PagePermissionEvaluator.hasPermission
> >> functions.  So if there is some specific security logic related to
> adding a
> >> comment to a widget, I believe you can put it in the appropriate
> >> PermissionEvaluator class.
> >>
> >
> >I understand the current model and I think it works great for top level
> >objects but it doesn't work all so well for subordinate objects, or for
> >business logic checks that are beyond CRUD. Right now everything is a top
> >level object (everything has it's own repository for example) but as part
> >of the object model restructure we have proposed to change that slightly.
> >If you view WidgetComment as a subordinate object to a Widget, the
> security
> >checks are different. Instead of checking WidgetComment => "Create" as a
> >standalone check, you really want to check Widget => "can_add_comment"
> >which is at the Widget level since the Comment doesn't exist yet. This
> >check would check to make sure the Widget is published, that the user has
> >access to the Widget, etc. Once the WidgetComment exists, the current
> >checks in place make sense (mostly).
> >
> >I know currently we could just check the widget in the
> >WidgetCommentPermissionEvaluator because the WidgetComment has an
> >attribute
> >of "widget_id" but that is another thing we are proposing to change in the
> >object model restructure. As we try and restructure things so that we can
> >support backends other than JPA we need to tweak the object model at the
> >interface level. For example, WidgetComment would no longer have an
> >attribute of widget_id, it is just associated with whatever widget it's
> >part of. This cleans up a few things like being able to create a
> >WidgetComment with a widget_id of 3 but adding it to the WidgetComment
> >collection of Widget id 2.
> >
> >Does that make sense?
> >
> >Beyond the WidgetComment example I still think there is a need for more
> >fine grained permission checks. For example:
> > - can_publish_widget
> > - can_reset_other_users_password (low level admin who can't do some other
> >functions)
> > - can_delete_other_users_comment (like a moderator)
> > - ...
> >
> >I know those functions can be covered by "admin" but I know our product
> >needs a finer level of control than just "admin". This will become much
> >more important as we start talking multi-tenancy which I'll bring up again
> >soon where you need multiple levels of admins.
> >
> >Chris
>
> Ok yes, this definitely makes sense to me given the context of the model
> refactoring changes.  I think changing the enum to strings should be fine,
> so long as we don't see hard-coded "can_add_comment" strings in lots of
> spots, which could make maintenance a little difficult.
>

The only place the string would show up would be in the hasPermission
annotation (which is already a string) and in the PermissionEvaluator. In
the permission  evaluators we could still define/use enums to define the
valid strings, the interface just wouldn't be bound to a single enum. Sound
reasonable?

>
> Also we should keep in mind the current ability to override the default
> security behaviors[1] as part of this change, and make sure only the most
> common of changes should go into the Rave code base.
>
> [1] http://rave.apache.org/documentation/model-permission-override.html
>
> Tony
>
> >
> >>
> >> Am I understanding your use case or completely off the mark? :)
> >>
> >> Tony
> >>
> >> >-----Original Message-----
> >> >From: Chris Geer [mailto:chris@cxtsoftware.com]
> >> >Sent: Tuesday, October 02, 2012 7:50 PM
> >> >To: dev
> >> >Subject: [Proposal] Spring Permissions Change
> >> >
> >> >I would like to propose we change how the spring permission checks work
> >> >slightly. Right now the "Permission" value (i.e. Create, Update...) is
> >> >defined as part of a enum named Permission defined in
> >> >the org.apache.rave.portal.security.ModelPermissionEvaluator interface.
> >> The
> >> >various hasPermissions methods take an instance of that Permission enum
> >> >(created from a string on the check permission annotation). Having the
> >> >permissions defined in an enumeration limits what we are able to check
> >> >permissions for in my opinion. Right now we have two choices, 1) limit
> our
> >> >permission checks to the list there is now, 2) add new permissions to
> the
> >> >generic Permissions enum which could lead to a bunch of permissions
> stored
> >> >on a generic enum that aren't really reusable (i.e. Widget ->
> add_comment
> >> >permission). I would like to propose we change the way we define
> >> >permissions to remove the enum and just pass along the string defined
> in
> >> >the annotations. The only real downside of that is that we can't use a
> >> >switch/case statement during permissions checks unless we use Java7.
> >> >
> >> >Thoughts/concerns?
> >> >
> >> >Thanks,
> >> >Chris
> >>
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message