mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Greg Mann <g...@mesosphere.io>
Subject Re: Review Request 56813: Updated master handlers to use the 'Principal' type.
Date Sat, 04 Mar 2017 00:55:17 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.

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.


- Greg


-----------------------------------------------------------
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