mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Vinod Kone <vinodk...@gmail.com>
Subject Re: Review Request 56813: Updated master handlers to use the 'Principal' type.
Date Sat, 04 Mar 2017 01:01:51 GMT


> On March 4, 2017, 12:31 a.m., Vinod Kone wrote:
> > src/master/http.cpp
> > Lines 482 (patched)
> > <https://reviews.apache.org/r/56813/diff/3-7/?file=1642755#file1642755line483>
> >
> >     Maybe `BadRequest` is better here than `Forbidden`. For example we send `BadRequest`
below in `scheduler()` handler when principals do not match etc. What do you think? 
> >     
> >     Here and below.
> 
> Greg Mann wrote:
>     Let's consider the definitions of the two status codes:
>     
>     ```
>     10.4.1 400 Bad Request
>     
>     The request could not be understood by the server due to malformed syntax. The client
SHOULD NOT repeat the request without modifications.
>     ```
>     
>     ```
>     10.4.4 403 Forbidden
>     
>     The server understood the request, but is refusing to fulfill it. Authorization will
not help and the request SHOULD NOT be repeated. If the request method was not HEAD and the
server wishes to make public why the request has not been fulfilled, it SHOULD describe the
reason for the refusal in the entity. If the server does not wish to make this information
available to the client, the status code 404 (Not Found) can be used instead.
>     ```
>     
>     '400 Bad Request' is used to indicate malformed syntax in the request. In the case
of mismatched principals elsewhere in `scheduler()`, for example, the request body is malformed
because its `principal` field is required to match its authenticated principal. In this case,
however, we are not indicating that the request is malformed, but rather that the authenticated
principal is not allowed to access this resource. In a sense, this is a case of implicit authorization.
I think that '403 Forbidden' makes more sense here.

makes sense.


- Vinod


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56813/#review167908
-----------------------------------------------------------


On March 3, 2017, 6:45 p.m., Greg Mann wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56813/
> -----------------------------------------------------------
> 
> (Updated March 3, 2017, 6:45 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rojas, Jan Schlicht, Till Toenshoff, and
Vinod Kone.
> 
> 
> Bugs: MESOS-7003
>     https://issues.apache.org/jira/browse/MESOS-7003
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This patch updates the HTTP endpoint handlers in the
> master process to accept the `Principal` type instead
> of an `Option<string>& principal`.
> 
> 
> Diffs
> -----
> 
>   src/master/http.cpp 37b96c07f17d3e5ba96f6eade69d86ba9e039b3d 
>   src/master/master.hpp 81320e0da3a2cd99d8dea1fbea9276296ef9e515 
>   src/master/master.cpp 2232cb7ad7c9ba7c067334d7327f592ab48e48fa 
>   src/master/quota_handler.cpp 3ad28e4a9363a877d0610b529a6c17fb30ece37a 
>   src/master/registrar.cpp d7134eea34102ab7b24d2f0131363bdd9005cfd3 
>   src/master/weights_handler.cpp d8047f2354322ec18b008afe44f507069b36a71f 
> 
> 
> Diff: https://reviews.apache.org/r/56813/diff/7/
> 
> 
> Testing
> -------
> 
> Testing details can be found at the end of this review chain.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message