You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@qpid.apache.org by Pavel Moravec <pm...@redhat.com> on 2014/06/15 09:15:34 UTC

Review Request 22606: [C++ broker] Improve ACL authorisation of QMF methods and queries

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

Review request for qpid, Chug Rolke and Ted Ross.


Bugs: QPID-5817
    https://issues.apache.org/jira/browse/QPID-5817


Repository: qpid


Description
-------

Examples of new ACL lines:

acl deny bob@QPID purge queue name=q7
# "purge" action previously existed but did not work

acl deny bob@QPID move queue name=q8 queuename=q7

acl deny bob@QPID redirect queue name=q8 queuename=q7
# in both cases, "name" is the source queue name, "queuename" is the destination queue

acl deny bob@QPID reroute queue name=q8 exchangename=amq.fanout

acl allow bob@QPID access query name=org.apache.qpid.broker:queue:q1
# to allow query of one particular object

acl allow bob@QPID access query schemaclass=exchange
# to allow whole object class to be queried - this allows _both_ querying some particular exchange and also whole class / all exchanges in one query


I have not tested QMFv1 authorisation of query, and also automated tests don't cover it. But the implementation there is straightforward.

Automated tests require QPID-5797 fixed (already committed).


Diffs
-----

  /trunk/qpid/cpp/src/qpid/broker/AclModule.h 1601656 
  /trunk/qpid/cpp/src/qpid/broker/Broker.h 1601656 
  /trunk/qpid/cpp/src/qpid/broker/Broker.cpp 1601656 
  /trunk/qpid/cpp/src/qpid/broker/Queue.cpp 1601656 
  /trunk/qpid/cpp/src/qpid/management/ManagementAgent.h 1601656 
  /trunk/qpid/cpp/src/qpid/management/ManagementAgent.cpp 1601656 
  /trunk/qpid/cpp/src/tests/acl.py 1601656 
  /trunk/qpid/cpp/src/tests/ha_tests.py 1601656 
  /trunk/qpid/doc/book/src/cpp-broker/Security.xml 1582719 

Diff: https://reviews.apache.org/r/22606/diff/


Testing
-------

All automated tests passed.

Various manual tests passed as well.


Thanks,

Pavel Moravec


Re: Review Request 22606: [C++ broker] Improve ACL authorisation of QMF methods and queries

Posted by Chug Rolke <cr...@redhat.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22606/#review45752
-----------------------------------------------------------



/trunk/qpid/cpp/src/qpid/broker/AclModule.h
<https://reviews.apache.org/r/22606/#comment80714>

    Thanks for maintaining this structure even though it's not used anywhere. It would be needed if the set of actions were to be limited to things that the broker would actually call.



/trunk/qpid/cpp/src/qpid/broker/Broker.h
<https://reviews.apache.org/r/22606/#comment80713>

    Don't change the EXTERN published function return signature, keep the int32_t. External users would need to understand the Manageable::STATUS_xxx to decode properly.
    
    You could throw if acl reject; return (-1) for param error; return >= 0 for normal move.



/trunk/qpid/cpp/src/qpid/broker/Broker.cpp
<https://reviews.apache.org/r/22606/#comment80715>

    I think this was ok as it was.



/trunk/qpid/cpp/src/tests/acl.py
<https://reviews.apache.org/r/22606/#comment80717>

    Nice.


- Chug Rolke


On June 15, 2014, 7:15 a.m., Pavel Moravec wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/22606/
> -----------------------------------------------------------
> 
> (Updated June 15, 2014, 7:15 a.m.)
> 
> 
> Review request for qpid, Chug Rolke and Ted Ross.
> 
> 
> Bugs: QPID-5817
>     https://issues.apache.org/jira/browse/QPID-5817
> 
> 
> Repository: qpid
> 
> 
> Description
> -------
> 
> Examples of new ACL lines:
> 
> acl deny bob@QPID purge queue name=q7
> # "purge" action previously existed but did not work
> 
> acl deny bob@QPID move queue name=q8 queuename=q7
> 
> acl deny bob@QPID redirect queue name=q8 queuename=q7
> # in both cases, "name" is the source queue name, "queuename" is the destination queue
> 
> acl deny bob@QPID reroute queue name=q8 exchangename=amq.fanout
> 
> acl allow bob@QPID access query name=org.apache.qpid.broker:queue:q1
> # to allow query of one particular object
> 
> acl allow bob@QPID access query schemaclass=exchange
> # to allow whole object class to be queried - this allows _both_ querying some particular exchange and also whole class / all exchanges in one query
> 
> 
> I have not tested QMFv1 authorisation of query, and also automated tests don't cover it. But the implementation there is straightforward.
> 
> Automated tests require QPID-5797 fixed (already committed).
> 
> 
> Diffs
> -----
> 
>   /trunk/qpid/cpp/src/qpid/broker/AclModule.h 1601656 
>   /trunk/qpid/cpp/src/qpid/broker/Broker.h 1601656 
>   /trunk/qpid/cpp/src/qpid/broker/Broker.cpp 1601656 
>   /trunk/qpid/cpp/src/qpid/broker/Queue.cpp 1601656 
>   /trunk/qpid/cpp/src/qpid/management/ManagementAgent.h 1601656 
>   /trunk/qpid/cpp/src/qpid/management/ManagementAgent.cpp 1601656 
>   /trunk/qpid/cpp/src/tests/acl.py 1601656 
>   /trunk/qpid/cpp/src/tests/ha_tests.py 1601656 
>   /trunk/qpid/doc/book/src/cpp-broker/Security.xml 1582719 
> 
> Diff: https://reviews.apache.org/r/22606/diff/
> 
> 
> Testing
> -------
> 
> All automated tests passed.
> 
> Various manual tests passed as well.
> 
> 
> Thanks,
> 
> Pavel Moravec
> 
>


Re: Review Request 22606: [C++ broker] Improve ACL authorisation of QMF methods and queries

Posted by Chug Rolke <cr...@redhat.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22606/#review46026
-----------------------------------------------------------

Ship it!


Ship It!

- Chug Rolke


On June 17, 2014, 9:16 a.m., Pavel Moravec wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/22606/
> -----------------------------------------------------------
> 
> (Updated June 17, 2014, 9:16 a.m.)
> 
> 
> Review request for qpid, Chug Rolke and Ted Ross.
> 
> 
> Bugs: QPID-5817
>     https://issues.apache.org/jira/browse/QPID-5817
> 
> 
> Repository: qpid
> 
> 
> Description
> -------
> 
> Examples of new ACL lines:
> 
> acl deny bob@QPID purge queue name=q7
> # "purge" action previously existed but did not work
> 
> acl deny bob@QPID move queue name=q8 queuename=q7
> 
> acl deny bob@QPID redirect queue name=q8 queuename=q7
> # in both cases, "name" is the source queue name, "queuename" is the destination queue
> 
> acl deny bob@QPID reroute queue name=q8 exchangename=amq.fanout
> 
> acl allow bob@QPID access query name=org.apache.qpid.broker:queue:q1
> # to allow query of one particular object
> 
> acl allow bob@QPID access query schemaclass=exchange
> # to allow whole object class to be queried - this allows _both_ querying some particular exchange and also whole class / all exchanges in one query
> 
> 
> I have not tested QMFv1 authorisation of query, and also automated tests don't cover it. But the implementation there is straightforward.
> 
> Automated tests require QPID-5797 fixed (already committed).
> 
> 
> Diffs
> -----
> 
>   /trunk/qpid/cpp/src/qpid/broker/AclModule.h 1601656 
>   /trunk/qpid/cpp/src/qpid/broker/Broker.h 1601656 
>   /trunk/qpid/cpp/src/qpid/broker/Broker.cpp 1601656 
>   /trunk/qpid/cpp/src/qpid/broker/Queue.cpp 1601656 
>   /trunk/qpid/cpp/src/qpid/management/ManagementAgent.h 1601656 
>   /trunk/qpid/cpp/src/qpid/management/ManagementAgent.cpp 1601656 
>   /trunk/qpid/cpp/src/tests/acl.py 1601656 
>   /trunk/qpid/cpp/src/tests/ha_tests.py 1601656 
>   /trunk/qpid/doc/book/src/cpp-broker/Security.xml 1582719 
> 
> Diff: https://reviews.apache.org/r/22606/diff/
> 
> 
> Testing
> -------
> 
> All automated tests passed.
> 
> Various manual tests passed as well.
> 
> 
> Thanks,
> 
> Pavel Moravec
> 
>


Re: Review Request 22606: [C++ broker] Improve ACL authorisation of QMF methods and queries

Posted by Pavel Moravec <pm...@redhat.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22606/
-----------------------------------------------------------

(Updated June 17, 2014, 9:16 a.m.)


Review request for qpid, Chug Rolke and Ted Ross.


Changes
-------

Both issues raised by Chuck are incorporated.


Bugs: QPID-5817
    https://issues.apache.org/jira/browse/QPID-5817


Repository: qpid


Description
-------

Examples of new ACL lines:

acl deny bob@QPID purge queue name=q7
# "purge" action previously existed but did not work

acl deny bob@QPID move queue name=q8 queuename=q7

acl deny bob@QPID redirect queue name=q8 queuename=q7
# in both cases, "name" is the source queue name, "queuename" is the destination queue

acl deny bob@QPID reroute queue name=q8 exchangename=amq.fanout

acl allow bob@QPID access query name=org.apache.qpid.broker:queue:q1
# to allow query of one particular object

acl allow bob@QPID access query schemaclass=exchange
# to allow whole object class to be queried - this allows _both_ querying some particular exchange and also whole class / all exchanges in one query


I have not tested QMFv1 authorisation of query, and also automated tests don't cover it. But the implementation there is straightforward.

Automated tests require QPID-5797 fixed (already committed).


Diffs (updated)
-----

  /trunk/qpid/cpp/src/qpid/broker/AclModule.h 1601656 
  /trunk/qpid/cpp/src/qpid/broker/Broker.h 1601656 
  /trunk/qpid/cpp/src/qpid/broker/Broker.cpp 1601656 
  /trunk/qpid/cpp/src/qpid/broker/Queue.cpp 1601656 
  /trunk/qpid/cpp/src/qpid/management/ManagementAgent.h 1601656 
  /trunk/qpid/cpp/src/qpid/management/ManagementAgent.cpp 1601656 
  /trunk/qpid/cpp/src/tests/acl.py 1601656 
  /trunk/qpid/cpp/src/tests/ha_tests.py 1601656 
  /trunk/qpid/doc/book/src/cpp-broker/Security.xml 1582719 

Diff: https://reviews.apache.org/r/22606/diff/


Testing
-------

All automated tests passed.

Various manual tests passed as well.


Thanks,

Pavel Moravec