> 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. > > Vinod Kone wrote: > Yea, lets separate the patch out into dependent review. Will enable us to > cherry pick it into older releases if needed.
New patch fixing the handlers can now be found earlier in this chain: https://reviews.apache.org/r/57054/ - Greg ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56812/#review166636 ----------------------------------------------------------- On Feb. 24, 2017, 11:06 p.m., Greg Mann wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/56812/ > ----------------------------------------------------------- > > (Updated Feb. 24, 2017, 11:06 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 the > agent process to accept an `AuthenticationContext` > instead of an `Option<string>& principal`. > > > Diffs > ----- > > src/slave/http.cpp 94731ec883c309cefb811694dc4e39de12d1ac59 > src/slave/slave.hpp 3b0aea4e3e9a17501077beccbccaab4abbe11af2 > src/slave/slave.cpp fc480ae23ffa5cdeeb79b3621a08e1f8703bc01a > > Diff: https://reviews.apache.org/r/56812/diff/ > > > Testing > ------- > > Testing details can be found at the end of this review chain. > > > Thanks, > > Greg Mann > >
