----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/14310/#review26379 -----------------------------------------------------------
Ship it! configure.ac <https://reviews.apache.org/r/14310/#comment51523> I thought we needed couple other packages too. Can we check for them here? include/mesos/mesos.proto <https://reviews.apache.org/r/14310/#comment51524> s/AuthenticationInfo/Credential/ Also add a required principal field. Also, if we are anticipating the framework to authenticate more than one principal, then we could perhaps do? message Credentials { repeated Credential; } src/messages/messages.proto <https://reviews.apache.org/r/14310/#comment51525> Should the mechanisms be enums? src/sasl/authenticatee.hpp <https://reviews.apache.org/r/14310/#comment51526> Since want users to not re-use Authenticatee for multiple attempts, how about taking the 'pid' as part of constructor argument? That probably drives the point home better? src/sasl/authenticatee.hpp <https://reviews.apache.org/r/14310/#comment51527> Why virtual? src/sasl/authenticatee.hpp <https://reviews.apache.org/r/14310/#comment51528> s/client/server/ src/sasl/authenticatee.hpp <https://reviews.apache.org/r/14310/#comment51529> Can you add a comment here on why you set it to false instead of an error? src/sasl/authenticator.hpp <https://reviews.apache.org/r/14310/#comment51530> Same blocking issue applies here as it did in Authenticatee? src/sasl/authenticator.hpp <https://reviews.apache.org/r/14310/#comment51531> Is there a benefit for 'link'ing the Authenticator? I would propose we either do it for both Authenticatee and Authenticator or none. I think the semantics would be easier to reason about then. - Vinod Kone On Sept. 24, 2013, 8:26 p.m., Benjamin Hindman wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/14310/ > ----------------------------------------------------------- > > (Updated Sept. 24, 2013, 8:26 p.m.) > > > Review request for mesos, Ben Mahler and Vinod Kone. > > > Repository: mesos-git > > > Description > ------- > > Initial SASL based authentication support. (I'll likely also be cleaning some > stuff up as I deal with compilation issues with different libsasl2* versions.) > > > Diffs > ----- > > configure.ac 96ac7a81311e9f3765bd54e078368566a3cc33ca > include/mesos/mesos.proto 8f845cc9b7cd491622cb29a69f6909170510a664 > src/Makefile.am 3eae9642b639e78f101e7fe580aa803beadbebda > src/messages/messages.proto 4d400c2d8f6ecb9f7cf8635b2a98341206bbb6c5 > src/sasl/authenticatee.hpp PRE-CREATION > src/sasl/authenticator.hpp PRE-CREATION > src/sasl/auxprop.hpp PRE-CREATION > src/sasl/auxprop.cpp PRE-CREATION > src/tests/sasl_tests.cpp PRE-CREATION > > Diff: https://reviews.apache.org/r/14310/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Benjamin Hindman > >
