You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@geode.apache.org by Jinmei Liao <ji...@pivotal.io> on 2017/08/07 20:13:27 UTC

Review Request 61480: GEODE-3330: user needs CLUSTER:MANAGE:QUERY permission to create a CQ.

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61480/
-----------------------------------------------------------

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

Re: Review Request 61480: GEODE-3330: user needs CLUSTER:MANAGE:QUERY permission to create a CQ.

Posted by Jinmei Liao <ji...@pivotal.io>.

> 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
> 
>

Re: Review Request 61480: GEODE-3330: user needs CLUSTER:MANAGE:QUERY permission to create a CQ.

Posted by Jared Stewart <js...@pivotal.io>.

> 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
> 
>

Re: Review Request 61480: GEODE-3330: user needs CLUSTER:MANAGE:QUERY permission to create a CQ.

Posted by Jinmei Liao <ji...@pivotal.io>.

> 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
> 
>

Re: Review Request 61480: GEODE-3330: user needs CLUSTER:MANAGE:QUERY permission to create a CQ.

Posted by Jinmei Liao <ji...@pivotal.io>.

> 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!

It's in line 184 processQuery, which eventually leads to BaseCommandQuery line 90.


- 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
> 
>

Re: Review Request 61480: GEODE-3330: user needs CLUSTER:MANAGE:QUERY permission to create a CQ.

Posted by Ken Howe <kh...@pivotal.io>.

> 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
> 
>

Re: Review Request 61480: GEODE-3330: user needs CLUSTER:MANAGE:QUERY permission to create a CQ.

Posted by Jared Stewart <js...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61480/#review182317
-----------------------------------------------------------




geode-cq/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/ExecuteCQ61.java
Lines 140 (patched)
<https://reviews.apache.org/r/61480/#comment258198>

    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).


- Jared Stewart


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
> 
>

Re: Review Request 61480: GEODE-3330: user needs CLUSTER:MANAGE:QUERY permission to create a CQ.

Posted by Jared Stewart <js...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61480/#review182442
-----------------------------------------------------------


Ship it!




Ship It!

- Jared Stewart


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
> 
>

Re: Review Request 61480: GEODE-3330: user needs CLUSTER:MANAGE:QUERY permission to create a CQ.

Posted by Emily Yeh <ey...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61480/#review182528
-----------------------------------------------------------


Ship it!




Ship It!

- Emily Yeh


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
> 
>

Re: Review Request 61480: GEODE-3330: user needs CLUSTER:MANAGE:QUERY permission to create a CQ.

Posted by Ken Howe <kh...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61480/#review182443
-----------------------------------------------------------


Ship it!




Ship It!

- Ken Howe


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
> 
>

Re: Review Request 61480: GEODE-3330: user needs CLUSTER:MANAGE:QUERY permission to create a CQ.

Posted by Ken Howe <kh...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61480/#review182440
-----------------------------------------------------------


Ship it!




- Ken Howe


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
> 
>

Re: Review Request 61480: GEODE-3330: user needs CLUSTER:MANAGE:QUERY permission to create a CQ.

Posted by Jinmei Liao <ji...@pivotal.io>.
-----------------------------------------------------------
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.


Changes
-------

added more tests


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 (updated)
-----

  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/

Changes: https://reviews.apache.org/r/61480/diff/1-2/


Testing
-------

precheckin running


Thanks,

Jinmei Liao

Re: Review Request 61480: GEODE-3330: user needs CLUSTER:MANAGE:QUERY permission to create a CQ.

Posted by Jared Stewart <js...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61480/#review182434
-----------------------------------------------------------




geode-cq/src/test/java/org/apache/geode/security/ClientCQAuthDUnitTest.java
Lines 54 (patched)
<https://reviews.apache.org/r/61480/#comment258340>

    Oops, I was a little trigger happy with my "Ship it!".. Should there be one more test here to make sure that things work as expected when a user has both permissions?


- Jared Stewart


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
> 
>

Re: Review Request 61480: GEODE-3330: user needs CLUSTER:MANAGE:QUERY permission to create a CQ.

Posted by Jared Stewart <js...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61480/#review182433
-----------------------------------------------------------


Ship it!




Ship It!

- Jared Stewart


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
> 
>