> 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.
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? - Ken ----------------------------------------------------------- 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 > >