> 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).
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. - Jinmei ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/61480/#review182317 ----------------------------------------------------------- On Aug. 7, 2017, 8:13 p.m., Jinmei Liao wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/61480/ > ----------------------------------------------------------- > > (Updated Aug. 7, 2017, 8:13 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/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 > > > Diff: https://reviews.apache.org/r/61480/diff/1/ > > > Testing > ------- > > precheckin running > > > Thanks, > > Jinmei Liao > >
