I don't see any response from @otter606 you seem to be responding to but the reasons you stated are very close to motives I had when I came up with http://www.tynamo.org/tapestry-security-jpa+guide/ that tapestry folks have been using for years. Tapestry-security-jpa library supports security checks on data instances (i.e. row-based access) and in it, you could express your proposed annotation @RequiresAttributes("$this.memberIds.contains($user.id)") simply with @RequiresAssociation("member") (or even just @RequiresAssocation without parameters if user is accessing his own user profile). All entity relations would be implicitly associated with the currently executing subject, thereby making both "$this" and "contains($user.id)" unnecessary. Obviously, my implementation is tied to the data level and specifically JPA, with its implementation of SecureEntityManager but conceivably, it could be extended to other data centric libraries. At the time, there was some talk on including basic support for it into Shiro but it never lead to concrete actions.
The danger with supporting complex syntax within annotation parameters, as other have pointed out, is that you are simply switching the target of where you are implementing the security checks. I'm with you on the benefits of concentrating cross cutting concerns to a single point from a high level perspective, but at least RequiresAttributesOnReturn seem far too specific to be supported in a wide-spread, generic framework. The trick is to have enough support in the core framework that allows you to easily implement custom, often annotation-driven, security checks. Kalle On Fri, Jul 8, 2016 at 4:17 PM, jbuhacoff <[email protected]> wrote: > Thanks everyone for all the comments! > > I think the bottom line here is that SHIRO-417 is a step along the same > lines I am proposing, but I’ll need some features beyond that, so I’ll take > the advice and post my code on Github for people to try it out, and if > there’s interest in the additional capabilities I’m proposing maybe later > there could be continued discussion about including it in Shiro. > > I haven’t started writing any code yet and have no idea when I’ll post it, > but to answer Brian’s last note I think that even my proposal and SHIRO-417 > could co-exist, since SHIRO-417 is just about extending the existing > wildcard syntax to replace part of it with the value of a method parameter, > and my work would add a separate capability to actually compare values > across parameters & other context available. I could probably also support > the @PermissionParam annotation so that if someone wants to use both > capabilities together, they only need to annotate a method parameter one > time. > I also wanted to respond to some of the comments: > > @scSynergy, there could be a default implementation of $user that works > with > Shiro’s built-in realms, but in principle it would be customizable so you > could add attributes to it and also add entirely new variables to the > context. But Shiro already has support for roles, including a > RequiresRoles > annotation, so I suggest to use that if possible. Anyway to do the same > thing via attributes, I suggest an expression like $user.roles.contains(…). > Also, the clearance level was just an example so that would be something > that an application would add as a customization, not something that would > be built in. > @Rich, I agree it's possible to check permissions from inside the method, > like calling isPermitted instead of using the RequiresPermissions > annotation. > > The nice thing about using the annotations is that I can scan the code to > see what permissions are required. Consider an extensible application that > just installed a new plugin that comes with its own permissions - the > application can scan all the jars from the plugin to find > RequiresPermissions annotations, parses the domain & action from the > annotation value, and enters them in the database. Now an administrator can > see these permissions in the management panel and assign them to new or > existing roles. It would also be possible to scan for uses of isPermitted > but a lot more variation possible with what gets passed in, whereas in > annotations the value is static. > > When using RequiresAttributes, it’s also easy to scan for this annotation > and inform the administrator about features that use attributed-based > access > control and actually display the expression. Ideally an administrator would > be able to actually create those attribute-based access controls in the > management panel. That would require an alternative implementation of the > aspect that, instead of reading the expression from the annotation, would > retrieve the expression from the database and then apply it with variables > provided by the application – the same parameters annotated with @Map, the > special variables $this and $return, and whatever else was set in the > security context. > I could make a call from each method to the engine to do this, or use > annotations to eliminate the boilerplate. > > @otter606, I’ve had a similar issue trying to fit row-based access into the > wildcard permission syntax. It looks like it is designed to handle it but > in > practice, updating a static array of what records someone might have access > to is not so convenient. Even if you have the data already in the database, > it’s in some table and not in the wildcard permission format, and even if > it > were, that doesn’t work with the annotations so you have to call > isPermitted > everywhere. Which is one of the pain points that led me to explore the idea > being proposed, because I can set up the relationship as an attribute > expression during development, and then write an “oracle” object (DAO, or > whatever) and put it in the security context to query which objects are > associated with that user at runtime. No more comma separated lists, and no > more deciding whether they should be on the user or on the object, because > the database designer already decided that, and all we have to do is now > plug in. > > @lprimak, completely agree about the shiny new objects. In this case, at > least, it’s a need that arose after using Shiro for a couple of years, and > in fact I did write some custom code here and there for “can only edit my > own records” and other custom rules. But looking around I’m not aware of > any > library that would be able to add the same functionality that would > integrate with same code that is already using Shiro, to allow me to do it > in more places with the same convenience as using the existing Shiro > annotations. > > But… it looks like there are two other efforts from 2011 & 2013 that are > along the same lines, and actually the 2013 ticket #417 looks very similar > to what I’m proposing, with expression syntax and annotation of parameters > to provide the mapping of those values into the context of the evaluation > engine. Looks like mine is a little more general with allowing access to > any bean-style property and some methods, not limited to one level, and > also > applying access controls on return values which is useful for data access > settings where you don’t know the object attributes until after you fetch > the record from the database. > > > > > -- > View this message in context: > http://shiro-user.582556.n2.nabble.com/Attribute-based-access-control-tp7581093p7581112.html > Sent from the Shiro User mailing list archive at Nabble.com. >
