----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/10658/#review19596 -----------------------------------------------------------
What is the motivation for this change? Many applications (not least most of our tests) live on trusted networks and do not require security or need/want to be encumbered with security related config. We should not require an ACL file simply to use links or any other feature. I suggest we add the new requirements only if auth=yes. That makes them required by default (auth=yes) but easy to opt-out by setting auth=no. trunk/qpid/cpp/src/qpid/acl/AclConnectionCounter.cpp <https://reviews.apache.org/r/10658/#comment40492> This assert doesn't look right. It would be hit for any string without a '-'. If thats really illegal it sould be a throw not an assert. trunk/qpid/cpp/src/qpid/broker/ConnectionHandler.cpp <https://reviews.apache.org/r/10658/#comment40730> I don't like this at all. There has to be an easy way for users who don't care about security to opt out. How about if auth=no? This would remove the need for you to tweak all our tests just for this, and more importantly it won't break every customer running on a trusted network without security. - Alan Conway On April 19, 2013, 5:33 p.m., Chug Rolke wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/10658/ > ----------------------------------------------------------- > > (Updated April 19, 2013, 5:33 p.m.) > > > Review request for qpid, Alan Conway and Gordon Sim. > > > Description > ------- > > Lock down federation link creation to be allowed only by ACL approval. When > no ACL file is specified then no federation links are allowed. > This version is more forgiving than the description in QPID-4631 as no > explicit CREATE LINK rules are required. Simple 'allow all all' ACL rules are > sufficient. > > ACL files are added to all diagnostics broker instances so that cmake 'make > check' and autotools 'make test' work. > > > This addresses bug QPID-4631. > https://issues.apache.org/jira/browse/QPID-4631 > > > Diffs > ----- > > trunk/qpid/cpp/src/qpid/acl/AclConnectionCounter.cpp 1469944 > trunk/qpid/cpp/src/qpid/broker/ConnectionHandler.cpp 1469944 > trunk/qpid/cpp/src/tests/ha_test.py 1469944 > trunk/qpid/cpp/src/tests/run_acl_tests 1469944 > trunk/qpid/cpp/src/tests/run_federation_sys_tests 1469944 > trunk/qpid/cpp/src/tests/run_federation_tests 1469944 > trunk/qpid/cpp/src/tests/sasl_fed 1469944 > > Diff: https://reviews.apache.org/r/10658/diff/ > > > Testing > ------- > > The ACL self test is enhanced to show that brokers running without the ACL > module/file loaded cannot create federation links. > > > Thanks, > > Chug Rolke > >
