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