> On Aug. 7, 2017, 8:21 p.m., Jared Stewart wrote: > > geode-cq/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/ExecuteCQ61.java > > Lines 140 (patched) > > <https://reviews.apache.org/r/61480/diff/1/?file=1791025#file1791025line141> > > > > Can you clarify something for me? The change looks like it's addding a > > check for CLUSTER:MANAGE:QUERY. But from reading GEODE-3330, I thought we > > already checked for that and instead needed to add a check for > > DATA:READ:REGION. But it looks like that might have already been happening > > on line 126 (122 of the original code). > > Jinmei Liao wrote: > line 126 is the authorization for old AccessControl interface, it does > nothing for the new integrated security. it was there in case user is still > using the old security model. > > I was orignally confused as well. Turns out we are checking > DATA:READ:regionName already for both execute and executeWithInitialResult, > we are just adding the CLUSTER:MANAGE.QUERY check. > > Jared Stewart wrote: > Would you mind pointing me to where the existing check happens for my own > edification? Thanks! > > Jinmei Liao wrote: > It's in line 184 processQuery, which eventually leads to BaseCommandQuery > line 90. > > Jared Stewart wrote: > Good to know. Thanks again. > > Ken Howe wrote: > My understanding was that DATA:READ:regionName was to be required only > for executeWithInitialResult, not for execute. As it is now this change > requires both permissions for both execute and executeWithInitialResult. Is > this what we want?
Yes, according to Dan. He said the user who calls execute() will get the result back in CQEvents for updates of the data, so whether or not he/she will get the inital set of result back or not, he will get continuous update of the data afterwards. - Jinmei ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/61480/#review182317 ----------------------------------------------------------- On Aug. 8, 2017, 9:10 p.m., Jinmei Liao wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/61480/ > ----------------------------------------------------------- > > (Updated Aug. 8, 2017, 9:10 p.m.) > > > Review request for geode, Emily Yeh, Jared Stewart, Ken Howe, Kirk Lund, and > Patrick Rhomberg. > > > Repository: geode > > > Description > ------- > > * cq.execute() and cq.executeWithInitialResult() all would still require > DATA:READ because it will send the result back to the client either initially > or later. > > > Diffs > ----- > > geode-core/src/test/java/org/apache/geode/security/SecurityTestUtil.java > 5d5c2148e2eba8054df305942e04f43ea69c0a79 > > geode-core/src/test/java/org/apache/geode/test/dunit/rules/LocatorServerStartupRule.java > a455aff315cd26abd398630ff63d8b54b0d1d12b > > geode-cq/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/CloseCQ.java > 6748f7d3193babfa668a7ff2846f974f0cdc1cbd > > geode-cq/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/ExecuteCQ61.java > 77a608c7e65a2030c0037e9f327cf8c17e9313db > > geode-cq/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/StopCQ.java > a3d51edcc141391e9d818fc0ed7e514d3cb5d6d0 > > geode-cq/src/test/java/org/apache/geode/internal/cache/tier/sockets/command/CloseCQTest.java > PRE-CREATION > > geode-cq/src/test/java/org/apache/geode/internal/cache/tier/sockets/command/ExecuteCQ61Test.java > PRE-CREATION > > geode-cq/src/test/java/org/apache/geode/internal/cache/tier/sockets/command/StopCQTest.java > PRE-CREATION > geode-cq/src/test/java/org/apache/geode/security/CQClientAuthDUnitTest.java > 89167258ddbc06315068c849255a8530faefe060 > > geode-cq/src/test/java/org/apache/geode/security/CQPostProcessorDunitTest.java > 45f45abd17cf4f90f96163ebe4f01e67b3b53633 > geode-cq/src/test/java/org/apache/geode/security/ClientCQAuthDUnitTest.java > PRE-CREATION > > geode-cq/src/test/java/org/apache/geode/security/ClientQueryAuthDUnitTest.java > cc5dde409c561522ae3739eeaee892079c509ae8 > > geode-cq/src/test/java/org/apache/geode/test/dunit/rules/CQUnitTestRule.java > PRE-CREATION > > > Diff: https://reviews.apache.org/r/61480/diff/2/ > > > Testing > ------- > > precheckin running > > > Thanks, > > Jinmei Liao > >
