> On Feb. 24, 2017, 2:07 a.m., Vinod Kone wrote: > > src/slave/http.cpp, line 831 > > <https://reviews.apache.org/r/56812/diff/2/?file=1641768#file1641768line831> > > > > so previously we were sending an empty subject (`Subject()`) but now we > > are sending `Option::None()` to `getObjectApprover`? what's the difference? > > i wish this change was done in a separate review instead of mixing it here > > with the use of auth context. > > Greg Mann wrote: > Yes, the existing code is buggy. If authorization is enabled, but > authentication is disabled (so that `principal` is `NONE`), then we end up > default-constructing the authorization `Subject` instead of using `None()`. > This creates a `Subject` with an empty string for its value. > > This means that if authorization is enabled, authentication is disabled, > and an ACL has been set allowing a user called "" to do things, then requests > which do not specify any Authorization header will be granted access > incorrectly. Clearly, this is an extreme corner case, but a bug nonetheless. > > I didn't notice the bug until the reviews were already in flight, so I > applied the change in these patches. If you like, I can create a patch before > this one which applies the changes to make use of `None()` correctly, and > then rebase onto that.
Yea, lets separate the patch out into dependent review. Will enable us to cherry pick it into older releases if needed. - Vinod ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56812/#review166636 ----------------------------------------------------------- On Feb. 22, 2017, 1:18 a.m., Greg Mann wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/56812/ > ----------------------------------------------------------- > > (Updated Feb. 22, 2017, 1:18 a.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 the > agent process to accept an `AuthenticationContext` > instead of an `Option<string>& principal`. > > > Diffs > ----- > > src/slave/http.cpp 8a9fabf861369d3ae659dce21fa3932f6f7b9161 > src/slave/slave.hpp 3b0aea4e3e9a17501077beccbccaab4abbe11af2 > src/slave/slave.cpp 45905297836017e9031359894fc71e614c13cfcc > > Diff: https://reviews.apache.org/r/56812/diff/ > > > Testing > ------- > > Testing details can be found at the end of this review chain. > > > Thanks, > > Greg Mann > >
