Hmm, not particularly keen on this suggestion... having role names embedded in annotations isn't a good idea, I think.
For myself, I'd rather that this requirement was handled in a cross-cutting fashoin, through some sort of extension to the Isis' authorizor API. However, to solve this issue properly, I think we might need to extend Isis' security model somewhat. Right now in the authorizor we check if a given user (via their roles) can view/use each of an object's members (properties/collections/actions) but we don't have the concept of checking if the user can view the object at all. If we did introduce this idea (of object-level checks), then we could also implement some sort of plug-in point for the authorizor such that instance-based authorizers (eg in a chain of responsibility pattern) could be added to supplement Isis' usual class-based authorizors. I know this design would work because the Naked Objects "sister project" (on .NET) works this way. Hopefully that makes some sort of sense.... thoughts, anyone? Dan On 29 July 2014 22:56, matias nahuel heredia <mat...@informaticos.com> wrote: > El 29/07/14 08:16, Dan Haywood escribió: > > On 29 July 2014 02:28, matias nahuel heredia <mat...@informaticos.com> >> wrote: >> >> hi Dan!! >>> how can i fix this vulnerability in Apache isis? >>> https://www.youtube.com/watch?v=AghmbcVE710 >>> >>> >> thanks for recording this, helps explain the issue. >> >> What we're talking about here is per-instance security... the idea that >> some users shouldn't be able to access some object instances (even though >> they do in general have access to other instances of a given class type, >> such as ToDoItem). >> >> As of Isis 1.6.0, the best I can suggest is to use the loaded() lifecycle >> callback (on ToDoItem itself) and have it thrown an exception if the >> object >> should not be accessible. Then, the ExceptionRecognizer can be used to >> translate this exception into a user-friendly error message. If the >> loaded() lifecycle callback throws the javax.jdo.ObjectNotFoundException >> then this is already detected by the default implementation of >> ExceptionRecognizer (namely ExceptionRecognizerCompositeFo >> rJdoObjectStore): >> >> >> final String ownedBy = getOwnedBy(); >> final String currentUser = container.getUser().getName(); >> if(!ownedBy.equals(currentUser)) { >> throw new JDOObjectNotFoundException("No such object"); >> } >> >> >> This code results in a redirect to the error page with a message: "Unable >> to load object. Has it been deleted by someone else? No such object". >> The "Unable to load object. Has it been deleted by someone else?" bit >> comes >> from the ExceptionRecognizer, the "No such object" comes from the loaded() >> method. >> >> I can think of two improvements. >> >> First, it would be to move this cross-cutting concern out of the entity >> (ToDoItem) and into some single subscriber that could do this sort of >> verification for all classes. We do have a ticket [1] to do this, I'll >> bring it forward to tackle in the next release 1.7.0. >> >> Second, the stack trace shown by Isis is rather ugly and also leaks the >> information that there is an object with the given identifier (even if the >> object itself isn't shown). So perhaps the ExceptionRecognizer needs to >> become more sophisticated such that the stack trace will be suppressed in >> some cases. >> >> I've raised a ticket for this requirement [2], citing this thread [3] >> >> Let me know your thoughts on the above. >> >> Thanks >> Dan >> >> >> [1] https://issues.apache.org/jira/browse/ISIS-803 >> [2] https://issues.apache.org/jira/browse/ISIS-846 >> >> i think the better way to validate this is the creation of metanotation > in the Framework > like that > @forUser(name="getOwnedBy",allfor="role1,role2") > public class NameClass > { > > > } >
