----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/47530/#review133719 -----------------------------------------------------------
src/slave/http.cpp (line 756) <https://reviews.apache.org/r/47530/#comment198316> Please make sure that `request.method` is "GET" when this function is called with enabled authorization. See `Slave::Http::flags` for an example. Currently, for many existing endpoints, the request method isn't checked which can lead to problems with authorization. We plan to change that later, see [MESOS-5346](https://issues.apache.org/jira/browse/MESOS-5346). src/slave/http.cpp (line 786) <https://reviews.apache.org/r/47530/#comment198322> Please call `authorizeEndpoint` as soon as possible, i.e. after the endpoint has been extracted from the URL. While I like the idea of doing work in parallel, by requesting the containerizer statuses prior to the authorization, this work should only be done after the authorization was successful. Hence this part should be in the `_containers` continuation. src/slave/slave.hpp (lines 96 - 97) <https://reviews.apache.org/r/47530/#comment198315> Please don't do this in a header. The `using namespace process;` above is a bad example and probably shouldn't even be there. - Jan Schlicht On May 18, 2016, 12:06 p.m., Abhishek Dasgupta wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/47530/ > ----------------------------------------------------------- > > (Updated May 18, 2016, 12:06 p.m.) > > > Review request for mesos, Adam B, Alexander Rukletsov, Greg Mann, Jan > Schlicht, and Till Toenshoff. > > > Bugs: MESOS-5317 > https://issues.apache.org/jira/browse/MESOS-5317 > > > Repository: mesos > > > Description > ------- > > Used GET_ENDPOINT_WITH_PATH coarse-grained authz on agent's > '/containers' endpoint to enable authorization on this endpoint. > Updated docs and testcases as well. > > > Diffs > ----- > > docs/endpoints/slave/containers.md 959f40b9db4de4b6cea456ecf7bcb402f7a94f05 > src/slave/http.cpp fb48ec61e2fe0c83f80d3b8aa4c2ef5a96b748ae > src/slave/slave.hpp 209f071448e3c52d16d3366d564003ee36b1d2e0 > src/tests/slave_authorization_tests.cpp > 843cf1c631e0a25125ca1c0c0028ad1a920c2c2f > > Diff: https://reviews.apache.org/r/47530/diff/ > > > Testing > ------- > > sudo GTEST_FILTER="*SlaveEndpointTest*.*" make -j2 check > > On ubuntu 16.04. > > Ran manual testing as well. > > > Thanks, > > Abhishek Dasgupta > >
