-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58255/#review171739
-----------------------------------------------------------
My concern about this patch is, that unlike all the other security components of Mesos, it
makes implicit authorization mandatory. Moreover, we are breaking here the process of authz
between the module and mesos itself.
I think it would be better `if`'s statements to check first that `authorizer.isSome()` as
we used somewhere else. Although I would prefer to delegate the whole thing to the authorizer
itself, along the lines of:
```c++
if (authorizer) {
authorizer->verifyClaims(principal, ???);
}
```
- Alexander Rojas
On April 12, 2017, 9:28 a.m., Greg Mann wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58255/
> -----------------------------------------------------------
>
> (Updated April 12, 2017, 9:28 a.m.)
>
>
> Review request for mesos, Adam B, Alexander Rojas, and Till Toenshoff.
>
>
> Bugs: MESOS-7339
> https://issues.apache.org/jira/browse/MESOS-7339
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This patch updates the agent handler for the executor API to
> verify the FrameworkID and ExecutorID contained within the
> executor's `Principal`, if present. This effectively performs
> implicit authorization of executor calls.
>
>
> Diffs
> -----
>
> src/slave/http.cpp 468cf332d79ed7315ecf51955235735dec0a2df1
>
>
> Diff: https://reviews.apache.org/r/58255/diff/2/
>
>
> Testing
> -------
>
> Testing details can be found at the end of this chain.
>
>
> Thanks,
>
> Greg Mann
>
>
|