> On Dec. 4, 2019, 8:07 p.m., Benjamin Mahler wrote: > > src/master/authorization.cpp > > Lines 145-147 (patched) > > <https://reviews.apache.org/r/71863/diff/1/?file=2181776#file2181776line145> > > > > A broader question that relates to all of these reviews with Try > > values.. are we essentially changing the way validation works? Will > > validation no longer be catching cases (and creating error message for > > them) because we now catch them during authorization, and potentially send > > worse error messages than the validation code generates? > > Andrei Sekretenko wrote: > Of the two reviews with Try, the preceding patch does not change the way > how authorization interacts with validation. Will check error messages there. > > This one does - but I will reconsider this change, as it does not address > the underlying problem fully. > > ----- > **The problem** is that currently combination of authorization + > validation handles some invalid operations in a very *interesting* way. > > If the operation is "valid enough" to build a valid authz object, we > build the object, authorize and, if authorized, validate. That produces > appropriate validation/authorization errors. > > On the other hand, if we cannot even build a valid authz object, we > request authorization for SOMETHING (usually, to perform the specified action > on ANY object), authorize, and, if authorized, validate. > This means that if permissions setup is restrictive enough, the caller > will be **declined authorization** for a wrong reason instead of **getting a > validation error**. > > I see two options to fix this systematically: > - split ACCEPT validation into stateless and stateful part, and move the > first one before authorization > - if the synchronous authz makes it (and we manage to avoid having async > fallback for non-compliant authorizers), to move the whole validation before > ACCEPT authorization > > Will file a ticket and add this as TODO.
> - if the synchronous authz makes it (and we manage to avoid having async > fallback for non-compliant authorizers), to move the whole validation before > ACCEPT authorization That would be great! - Benjamin ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/71863/#review218922 ----------------------------------------------------------- On Dec. 3, 2019, 5:54 p.m., Andrei Sekretenko wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/71863/ > ----------------------------------------------------------- > > (Updated Dec. 3, 2019, 5:54 p.m.) > > > Review request for mesos, Benjamin Bannier and Benjamin Mahler. > > > Bugs: MESOS-10023 and MESOS-10056 > https://issues.apache.org/jira/browse/MESOS-10023 > https://issues.apache.org/jira/browse/MESOS-10056 > > > Repository: mesos > > > Description > ------- > > Moved creating authorization Object out of `Master::authorize*Volume`. > > > Diffs > ----- > > src/master/authorization.hpp PRE-CREATION > src/master/authorization.cpp PRE-CREATION > src/master/http.cpp e03655863ea2d4e4464b3d14b359de3d7f059778 > src/master/master.hpp 93630421d58e6fd26566e81a23cd910957795665 > src/master/master.cpp 14b90a5e276df055bb8a570331f27cab200c9869 > > > Diff: https://reviews.apache.org/r/71863/diff/1/ > > > Testing > ------- > > > Thanks, > > Andrei Sekretenko > >
