hi shane, we could keep both methods for now and prototype & discuss the type-safe approach. if it works without major disadvantages, we could drop the 2nd method.
regards, gerhard 2012/4/24 Shane Bryzak <[email protected]> > On 23/04/12 20:51, Gerhard Petracek wrote: > >> hi shane, >> >> imo we just need a solution for your @CheckPermission example. >> e.g. a custom annotation which can use the custom Operation (e.g. the >> custom enum) directly. the custom annotation would be meta-annotated with >> a >> marker annotation. >> > > The issue was not with the Operation, it was in regards to how the correct > ResourceIdentifier type could be selected by the authorizer for the > @CheckPermissions security binding type. IMO this is a showstopper for the > case where we only provide the Identity.hasPermission(**ResourceIdentifier, > ...) method and not Identity.hasPermission(Object, ...) as well. > > >> the rest shouldn't be a problem. >> >> regards, >> gerhard >> >> >> >> 2012/4/23 Shane Bryzak<[email protected]> >> >> On 23/04/12 18:03, Marek Posolda wrote: >>> >>> Hi all, >>>> >>>> >>>> On 22.4.2012 19:27, Christian Kaltepoth wrote: >>>> >>>> I prefer the method signature Shane suggested in his initial mail. >>>>> Providing a single method with an object parameter makes most sense to >>>>> me: >>>>> >>>>> Identity.hasPermission(Object resource, String operation) >>>>> >>>>> This way the developer is completely free to choose how to identify >>>>> the resource. I think in most cases the object itself will be used. >>>>> And the JSF example Shane showed is a very good example for this. >>>>> >>>>> Of cause people may need other ways to refer to resources like in the >>>>> ResourceIdentifier concept Marek showed. But actually I see something >>>>> like this more as an usage pattern and I don't think it belongs >>>>> directly into the API. Perhaps we could provide something like >>>>> ResourceIdentifier as an utility class that people can use if they >>>>> want to refer to resources this way. But the API of Identity should be >>>>> as simple as possible. And the object parameter will allow people to >>>>> use any object they want. Even something like ResourceIdentifier. >>>>> >>>>> That's the question. I think that API should be clear and using single >>>> method: >>>> >>>> Identity.hasPermission(Object resource, String operation) >>>> >>>> >>>> is not clear enough. As in some cases, your argument is directly secured >>>> object and in other cases it's only ResourceIdentifier for this object. >>>> Also it would mean that you will need to ask for the type of object in >>>> your >>>> PermissionResolver implementation: >>>> >>>> if (resource instanceof ResourceIdentifier) >>>> { >>>> // compute permissions for ResourceIdentifier >>>> } >>>> else >>>> { >>>> // Argument is "resource" so compute permissions for this resource >>>> } >>>> >>>> >>>> >>>> >>>> On the other hand, using single method: >>>> Identity.hasPermission(****ResourceIdentifier resource, Operation >>>> >>>> operation) >>>> >>>> may not be appropriate enough for some use-cases. Let's imagine that you >>>> have Customer and CustomerResourceIdntifier objects: >>>> >>>> public class Customer >>>> { >>>> private String ID; >>>> private String customerName; >>>> >>>> // getters and setters here >>>> } >>>> >>>> public class CustomerResourceIdentifier implements ResourceIdentifier >>>> { >>>> private String customerId; >>>> >>>> // getters and setters >>>> } >>>> >>>> >>>> And you want to create PermissionResolver, which will compute permission >>>> based on customer name. With the API method: >>>> >>>> Identity.hasPermission(****ResourceIdentifier resourceIdentifier, >>>> >>>> Operation operation) >>>> >>>> >>>> you can't manage to do it (unless you change implementation of >>>> CustomerResourceIdentifier to encapsulate customerName) >>>> >>>> >>>> Operation >>>> I agree that big advantage of this approach is type-safety. On the other >>>> hand, using String directly is more flexible as you have more freedom. >>>> Let's imagine that you have some permission rules stored in database and >>>> you want to add new type of operation like "ADMINISTER". With using >>>> String >>>> as argument, you can directly create records in DB and use this new >>>> "ADMINISTER" argument without doing any changes in Java code. With the >>>> Operation approach, you would need to add this new ADMINISTER operation >>>> into your Operation enum: >>>> >>>> This is exactly why the operation cannot be an enum - permissions will >>> be >>> stored in the database as string values, and if we use an enum value >>> there >>> will be no way of knowing which enum class the operation(s) string needs >>> to >>> be converted back into. I should have actually expanded on the details >>> of >>> this a little bit earlier which would have made this more obvious. >>> >>> Another use case which I didn't mention is the ability to use bit flags >>> to >>> set permissions. This is extremely important for a database with >>> millions >>> of records - instead of using a String in the database to store the >>> operations it is possible to use a numerical value instead, with each bit >>> representing a different operation. It also allows us to define >>> available >>> permissions for a particular domain object, which is important when >>> building management interfaces. >>> >>> Let's say we have the following entity: >>> >>> @Entity >>> @Permissions({ >>> @Permission(operation="create"****), >>> @Permission(operation="read"), >>> @Permission(operation="update"****), >>> @Permission(operation="delete"****) >>> >>> }) >>> public class Customer >>> { >>> // snip >>> } >>> >>> The @Permissions annotation tells the PermissionManager that we're >>> allowed >>> to assign four specific permissions; create, read, update and delete for >>> the Customer entity. We can take this a step further and define a bit >>> mask >>> value for each of these: >>> >>> @Entity >>> @Permissions({ >>> @Permission(operation="create"****, mask = 1), >>> >>> @Permission(operation="read", mask = 2), >>> @Permission(operation="update"****, mask = 4), >>> @Permission(operation="delete"****, mask = 8) >>> >>> }) >>> public class Customer >>> { >>> // snip >>> } >>> >>> By providing mask values, the PermissionManager knows that the cumulative >>> permissions for this entity will be stored in the database as a numerical >>> value. So a user with create, read and update permissions for a >>> particular >>> customer will have a stored operation value of 1 (create) + 2 (read) + 4 >>> (update) = 7. >>> >>> >>> >>> >>> public enum CrudOperation implements Operation >>>> { >>>> CREATE, READ, UPDATE, DELETE, ADMINISTER >>>> } >>>> >>>> I wanted to point this out, but I agree that for most cases is type-safe >>>> approach with Operation interface probably better:-) >>>> >>>> So after all, my opinion is to have two methods: >>>> >>>> Identity.hasPermission(Object object, Operation operation) throws >>>> AuthorizationException; >>>> Identity.hasPermission(****ResourceIdentifier resourceIdentifier, >>>> Operation operation) throws AuthorizationException; >>>> >>>> +1, I think that having two methods is the best solution, with the >>> caveat >>> that overloading like this doesn't cause issues with EL. The Object >>> parameter version of this method will most likely delegate to the >>> ResourceIdentifier method anyway, as a ResourceIdentifier will be >>> required >>> for all lookups. By the way, we should make this as transparent as >>> possible - out of the box, DeltaSpike should provide a ResourceIdentifier >>> implementation for entity beans that will build a unique identifier based >>> on the class name and the primary key value. Another alternative that we >>> should support is allowing the user to specify which ResourceIdentifier >>> to >>> use on the domain class itself: >>> >>> @Entity >>> @Identifier(****CustomerResourceIdentifier.****class) >>> >>> public class Customer >>> { >>> >>> } >>> >>> Yet another alternative that we should support is the packaging of >>> additional ResourceIdentifier implementations with an application, that >>> are >>> automatically discovered and iterated through when the permission check >>> can't determine which ResourceIdentifier to use. With that in mind, the >>> ResourceIdentifier interface should look like this: >>> >>> public interface ResourceIdentifier >>> { >>> boolean canIdentify(Class targetClass); >>> String getIdentifier(Object resource); >>> } >>> >>> The canIdentify() method will return true if the specified class is one >>> that this ResourceIdentifier is capable of generating identifier strings >>> for. >>> >>> >>> Thanks, >>>> Marek >>>> >>>> >>> >
