> 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.
Would you mind pointing me to where the existing check happens for my own edification? Thanks! - Jared ----------------------------------------------------------- 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 > >