I like the simplicity of the annotation, but that simplicity has some problems other than just playing nicely with lambdas. If permission required is dependent on function parameters or cluster state, it can't (currently) be resolved as part of the annotation. We already work-around this in a few places where we check if a cache is persistent before requiring CLUSTER:WRITE:DISK.
On Thu, Aug 17, 2017 at 1:56 PM, Jared Stewart <jstew...@pivotal.io> wrote: > We already have a similar annotation that we use internally to secure GFSH > commands: > @ResourceOperation(resource = Resource.DATA, operation = Operation.MANAGE) > public Result createRegion( > One option might be to move this annotation into a public package and to > enforce it if present on Functions. The only caveat is that we would need > to extend the annotation a bit to support multiple permissions (right now > it only supports a single permission like "DATA:READ", but not e.g. > "DATA:READ and CLUSTER:READ" ). In any case, the permissions required to > execute a function should not change from what they are today unless a > function author explicitly specifies the permissions that should be > required. (Whether in the form of an annotation, a method, or whatever > form we decide on.) > > - Jared > > On Aug 17, 2017, at 1:35 PM, Jacob Barrett <jbarr...@pivotal.io> wrote: > > > > Y'all should checkout PicketLink (now KeyCloak) and how they handle > this. I > > used this heavily in the past with JBoss AS. It supports annotation and > > inline restrictions. Also supports EL but that's for another day. While I > > am not advocating today (maybe tomorrow) that we witch to PicketLink but > > looking at other projects that have solved with application server domain > > issue may help us make better choices. > > > > > > http://docs.jboss.org/picketlink/2/latest/reference/ > html/Checking_for_Permissions.html > > > > > > On Thu, Aug 17, 2017 at 1:27 PM Udo Kohlmeyer <ukohlme...@pivotal.io> > wrote: > > > >> I don't think we should give a user explicit permissions to change the > >> authorization levels for their function. > >> > >> I think that the SecurityService should have some knowledge about > >> authorization levels on a per-user or per-role. That would check the > >> authorization level on a per-user basis for every function. This might > >> require a more fine grained authorization mechanism to be role-based or > >> more specific user authorization settings should be set up that certain > >> users can only execute specific functions, or not execute functions at > >> all... > >> > >> In addition to that, there are many functions that could feasibly be > >> read-only. Thus "DATA:READ" permissions would be sufficient... but the > >> current system does not protect itself to access a region and execute a > >> "WRITE" operation, so we are left with the lowest common denominator. > >> > >> --Udo > >> > >> > >> On 8/17/17 12:10, Swapnil Bawaskar wrote: > >>> So, it sounds like if we did #1 above: i.e: > >>> 1) externalize SecurityService so that function author can use it in > the > >>> function.execute code to check authorization. > >>> > >>> we could get this to work with lambdas: > >>> ResultCollector rc = getExecution().execute(context -> > >>> context.getCache().getSecurityService().authorizeRead(); > >>> // perform read > >>> context.getResultSender().lastResult(result) > >>> ); > >>> > >>> Udo, > >>> I think there are plenty of use cases where a user would only have > >>> privileges to read data, if this reading of data involves a function, > >> that > >>> user will need write privileges too. > >>> > >>> On Thu, Aug 17, 2017 at 11:26 AM Udo Kohlmeyer <ukohlme...@pivotal.io> > >>> wrote: > >>> > >>>> In the case of lambda executions... we have no way to annotate the > >>>> lambda... > >>>> > >>>> So maybe the "outer" service call needs to help us... Maybe the > security > >>>> framework should automatically prevent the execution of any function > is > >>>> the user does not have "DATA:READ"/"DATA:WRITE" or "FUNCTION:EXEC" > >>>> privileges. > >>>> > >>>> In addition to this, do we need to distinguish between "read" and > >>>> "write" functions? > >>>> > >>>> > >>>> On 8/17/17 10:43, Dan Smith wrote: > >>>>>> if we get to Lambda-based functions > >>>>> No if about it, this works right now and we do this in our tests :) > >>>>> > >>>>> ResultCollector rc = getExecution().execute(context -> > >>>>> context.getResultSender().lastResult("done") > >>>>> ); > >>>>> > >>>>> -Dan > >>>>> > >>>>> > >>>>> On Thu, Aug 17, 2017 at 9:59 AM, Udo Kohlmeyer < > ukohlme...@pivotal.io> > >>>>> wrote: > >>>>> > >>>>>> I think there might be more to this than just > >> "Data:READ"/"Data:WRITE". > >>>>>> Why would we not support something like > >>>> /*@Authorize(hasRole("functionExecutor"))*/ > >>>>>> or /*@Authorize(requiresPermission("DATA:READ"))*/ > >>>>>> > >>>>>> The next question in my mind would be, if we get to Lambda-based > >>>>>> functions, how do we specify authorization credentials then? > >> Annotations > >>>>>> are great to "static" definition on services, not so great for more > >>>>>> "dynamic" execution of stuff... > >>>>>> > >>>>>> > >>>>>> > >>>>>> On 8/17/17 09:29, Dan Smith wrote: > >>>>>> > >>>>>>> I like the annotation idea, especially considering that Jared was > >>>> talking > >>>>>>> about adding a @RegisterableFunction annotation as well. maybe > >>>> something > >>>>>>> like @ResourcePermission("Data:READ") or something like that? > >>>>>>> > >>>>>>> -Dan > >>>>>>> > >>>>>>> On Thu, Aug 17, 2017 at 8:18 AM, Michael William Dodge < > >>>> mdo...@pivotal.io > >>>>>>> wrote: > >>>>>>> > >>>>>>> What about an annotation for read-only functions or a subinterface > >> off > >>>>>>>> org.apache.geode.cache.execute.Function? > >>>>>>>> > >>>>>>>> Sarge > >>>>>>>> > >>>>>>>> On 17 Aug, 2017, at 01:42, Swapnil Bawaskar <sbawas...@pivotal.io > > > >>>>>>>> wrote: > >>>>>>>> > >>>>>>>>> Discuss fix for GEODE-2817 > >>>>>>>>> <https://issues.apache.org/jira/browse/GEODE-2817> > >>>>>>>>> > >>>>>>>>> Currently to execute a function, you will need "data:write" > >>>> permission, > >>>>>>>> but > >>>>>>>> > >>>>>>>>> it really depends on what the function is doing. For example, if > a > >>>>>>>>> > >>>>>>>> function > >>>>>>>> > >>>>>>>>> is just reading data, the function author might want users with > >>>>>>>>> DATA:READ > >>>>>>>>> permissions to execute the function. The two options mentioned in > >> the > >>>>>>>>> ticket are: > >>>>>>>>> > >>>>>>>>> 1) externalize SecurityService so that function author can use it > >> in > >>>> the > >>>>>>>>> function.execute code to check authorization. > >>>>>>>>> 2) add a method to function interface to tell the framework what > >>>>>>>>> > >>>>>>>> permission > >>>>>>>> > >>>>>>>>> this function needs to execute, so that the framework will check > >> the > >>>>>>>>> permission before executing the function. > >>>>>>>>> > >>>>>>>>> I vote for #2 because, I think, a function author will be able to > >>>> easily > >>>>>>>>> discover a method on the Function interface, rather than trying > to > >>>> look > >>>>>>>> for > >>>>>>>> > >>>>>>>>> SecurityService. > >>>>>>>>> > >>>>>>>>> I propose that we add the following new method to Function: > >>>>>>>>> > >>>>>>>>> default public List<ResourcePermission> requiredPermissions() { > >>>>>>>>> // default DATA:WRITE > >>>>>>>>> } > >>>>>>>>> > >>>>>>>>> In order to preserve existing behavior, the default required > >>>> permission > >>>>>>>>> would be DATA:WRITE. > >>>>>>>>> > >>>> > >> > >> > >
