> On Feb. 16, 2017, 3:56 p.m., Alexander Rojas wrote: > > src/master/http.cpp, lines 796-798 > > <https://reviews.apache.org/r/56619/diff/1/?file=1632590#file1632590line796> > > > > This appears here, there and everywhere, that I'm considering it may be > > a good idea to abstract it to something like: > > > > ``` > > static Option<string> AuthenticationContext::extractPrincipal(const > > Option<AuthenticationContext>& context); > > ``` > > > > That will make it easier if one day we need to modify how the > > extraction works. > > Greg Mann wrote: > Good call, will do.
I think I would actually prefer to update the validation code to accept the `AuthenticationContext` instead of adding this helper method. > On Feb. 16, 2017, 3:56 p.m., Alexander Rojas wrote: > > src/master/http.cpp, lines 1479-1481 > > <https://reviews.apache.org/r/56619/diff/1/?file=1632590#file1632590line1479> > > > > likewise, this looks like a prime candidate to be abstracted away. As I suggested in https://reviews.apache.org/r/56618/, let's avoid passing default-initialized `Subject`s at all, and instead use a helper which returns `Option<authorization::Subject>`. - Greg ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56619/#review165838 ----------------------------------------------------------- On Feb. 17, 2017, 10:36 p.m., Greg Mann wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/56619/ > ----------------------------------------------------------- > > (Updated Feb. 17, 2017, 10:36 p.m.) > > > Review request for mesos, Adam B, Alexander Rojas, Jan Schlicht, Till > Toenshoff, and Vinod Kone. > > > Bugs: MESOS-7003 > https://issues.apache.org/jira/browse/MESOS-7003 > > > Repository: mesos > > > Description > ------- > > This patch updates the HTTP endpoint handlers in Mesos > to accept an `AuthenticationContext` instead of an > `Option<string>& principal`. > > > Diffs > ----- > > src/files/files.hpp 8cffc26fc7d674187e55663f23f1e10bed40229e > src/files/files.cpp 8327f8002fbfa3be77a4bbe4aa83a73d0f170f7a > src/master/http.cpp e2fd71c5ae4178564b9a08756df5175aec5d6ca1 > src/master/master.hpp 764adb18ddf09b62529c5c96f8e4dfaf7803483e > src/master/master.cpp 5534674fa2740b0a6e37fc1df34b582f1e691133 > src/master/quota_handler.cpp 3ad28e4a9363a877d0610b529a6c17fb30ece37a > src/master/registrar.cpp d7134eea34102ab7b24d2f0131363bdd9005cfd3 > src/master/weights_handler.cpp da0b995e1866e7eb57b8ef885864c6c6d66405d6 > src/slave/http.cpp af70b6f294a04f23b04cd1d8c36c1c3e86d7d5e6 > src/slave/slave.hpp 3b0aea4e3e9a17501077beccbccaab4abbe11af2 > src/slave/slave.cpp ebba8e16bc9ec45781183e78cb5a3c351a5f65f5 > > Diff: https://reviews.apache.org/r/56619/diff/ > > > Testing > ------- > > Testing information can be found at the end of this review chain. > > > Thanks, > > Greg Mann > >
