> On July 5, 2018, 11:55 p.m., Chun-Hung Hsiao wrote: > > src/master/http.cpp > > Lines 3821 (patched) > > <https://reviews.apache.org/r/67693/diff/3/?file=2044470#file2044470line3821> > > > > Do we want to: > > 1. Simply skip such operations, as suggested in the code here, or > > 2. Return an `InternalServerError` eventually, or > > 3. Just `CHECK` that this shouldn't happen? > > Ditto below. > > > > Please feel free to drop this if you think skipping is good enough.
Thanks for bringing this up. Since we here only are concerned with authorizing operations, I still think that skipping is a fair solution (we cannot show the operation without authorizing, but also do not need to render the response not useful with `InternalServerError`; we do not need to `CHECK` either I believe as that would introduce very hard coupling). I added a `WARNING` log with some details. Does that work? > On July 5, 2018, 11:55 p.m., Chun-Hung Hsiao wrote: > > src/tests/api_tests.cpp > > Lines 131-134 (original), 131-134 (patched) > > <https://reviews.apache.org/r/67693/diff/3/?file=2044472#file2044472line131> > > > > How about making the same change as you did in `AgentAPITest::post`? > > ``` > > Future<v1::master::Response> post( > > const process::PID<master::Master>& pid, > > const v1::master::Call& call, > > const ContentType& contentType, > > const Credential& credential = DEFAULT_CREDENTIAL) > > ``` Done, also adjusted usage in `*/MasterAPITest.GetOperations/*` below. - Benjamin ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67693/#review205755 ----------------------------------------------------------- On July 6, 2018, 9:37 p.m., Benjamin Bannier wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/67693/ > ----------------------------------------------------------- > > (Updated July 6, 2018, 9:37 p.m.) > > > Review request for mesos, Chun-Hung Hsiao and Jan Schlicht. > > > Bugs: MESOS-8473 > https://issues.apache.org/jira/browse/MESOS-8473 > > > Repository: mesos > > > Description > ------- > > This patch adds filtering of operations reported for `GET_OPERATIONS` > calls. A principal is allowed to view see all operations for which it > is allowed to see the role of the underlying resources. > > > Diffs > ----- > > src/master/http.cpp 0492b979e4657a489ca3428e6f8022ef20cb05f5 > src/slave/http.cpp aa9cdc5f58f73323958a6872e2721c83317f354c > src/tests/api_tests.cpp f343991a5d23ac665429456471ac06a5315fc692 > > > Diff: https://reviews.apache.org/r/67693/diff/4/ > > > Testing > ------- > > `make check` > > The added tests fails without the corresponding changes to the master and > agent code. > > > Thanks, > > Benjamin Bannier > >
