> On April 7, 2017, 11:40 a.m., Alexander Rojas wrote: > > src/authorizer/local/authorizer.cpp > > Lines 654-657 (original), 683-690 (patched) > > <https://reviews.apache.org/r/58254/diff/1/?file=1686361#file1686361line683> > > > > I'm not so sure returning a `RejectingObjectApprover()` is the right > > thing to do. It looks to me that the request is in an invalid state and at > > the very least should log that, but this if sounds like a precondition to > > me: If it has claims, it needs to be one of the three actions. Probably > > chang it for a `CHECK`. > > > > If I'm wrong, at least a comment mentioning when is it a valid case > > having a request that fails the check would help.
There's nothing stopping a client from providing a token with claims but no value in an operator API request with some other action, so we need to fail authorization for those cases somewhere. However, it's probably better to do it in the body of `getObjectApprover`, rather than here. Will update and add a comment. > On April 7, 2017, 11:40 a.m., Alexander Rojas wrote: > > src/authorizer/local/authorizer.cpp > > Lines 692-699 (patched) > > <https://reviews.apache.org/r/58254/diff/1/?file=1686361#file1686361line692> > > > > Refer to my review of previous patch. I'm not sure precisely what you mean to say here, could you elaborate? - Greg ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58254/#review171320 ----------------------------------------------------------- On April 7, 2017, 11:25 p.m., Greg Mann wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/58254/ > ----------------------------------------------------------- > > (Updated April 7, 2017, 11:25 p.m.) > > > Review request for mesos, Adam B, Alexander Rojas, Till Toenshoff, and Vinod > Kone. > > > Bugs: MESOS-7014 > https://issues.apache.org/jira/browse/MESOS-7014 > > > Repository: mesos > > > Description > ------- > > This patch updates the agent handlers for the LAUNCH_, WAIT_, > and KILL_NESTED_CONTAINER calls of the operator API to set the > `container_id` field within the authorization object, > facilitating implicit executor authorization. > > > Diffs > ----- > > include/mesos/authorizer/authorizer.proto > 736f76d552956f2351ffd40fc51d088dff83f8c8 > src/authorizer/local/authorizer.cpp > e241edf4afa48d35dbbbb94d72e8e8690f5bedfc > src/slave/http.cpp b07ce7c73a90ef297d980806ebba9530d86f25ae > > > Diff: https://reviews.apache.org/r/58254/diff/2/ > > > Testing > ------- > > Testing details can be found at the end of this chain. > > > Thanks, > > Greg Mann > >
