Re: Review Request 42027: Changes HTTP responses from Unauthorized (401) to Forbidden (403).
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/42027/#review113769 --- Ship it! Sorry for the nit-pick, but the summary still needs `s/Changes/Changed/` before a merge :| - Benjamin Bannier On Jan. 11, 2016, 2:25 p.m., Alexander Rojas wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/42027/ > --- > > (Updated Jan. 11, 2016, 2:25 p.m.) > > > Review request for mesos, Alexander Rukletsov, Greg Mann, Joerg Schad, Jan > Schlicht, and Till Toenshoff. > > > Bugs: MESOS-4305 > https://issues.apache.org/jira/browse/MESOS-4305 > > > Repository: mesos > > > Description > --- > > It is a common patter within Mesos to return an HTTP 401 (Unauthorized) > response whenever the request is invalid for whatever reason. However, > according to the [RFC-2617 Section > 1.2](https://tools.ietf.org/html/rfc2617#section-1.2): > > The 401 (Unauthorized) response message is used by an origin server to > > challenge the authorization of a user agent. This response MUST include a > > WWW-Authenticate header field containing at least one challenge applicable > > to the requested resource. > > Meaning that despite the confusing name, the status code _401 Unauthorized_ > should be used only for authentication purposes. On the other hand, the > [RFC-2616 Section > 10.4.4](http://www.w3.org/Protocols/rfc2616/rfc2616-sec10.html#sec10.4.4) > states: > > _(403 Forbidden is returned when)_ 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. > > As such, _403 (Forbidden)_ seems to be a better return code when replying > inside endpoint handlers, while _401 (Unauthorized)_ should be left to the > HTTP Authenticators only. > > > Diffs > - > > docs/authorization.md a928f1722dc67cd791d78ebbe4591f2e8f2e8f2a > src/master/http.cpp bcafc7aff89659a68352f3876ce6042f8b34bd5d > src/master/quota_handler.cpp 134a93b1d1b6e050aa8a5037ffbec2cc305b0694 > src/tests/master_quota_tests.cpp 776a168254af6fa8a5d87d4580b35d83f2d5909a > src/tests/persistent_volume_endpoints_tests.cpp > f0cce190abc90f0fae84d6c3db20e8215c2d8132 > src/tests/reservation_endpoints_tests.cpp > b8edd6fafedd4c2221a8d19c1ebc71254071a8c7 > src/tests/scheduler_http_api_tests.cpp > 4d23a5a8368e0ed126469fa4a90a889b339ad004 > src/tests/teardown_tests.cpp 97cc89ba168aefff8512f6d1a25c4f7ddf180bae > > Diff: https://reviews.apache.org/r/42027/diff/ > > > Testing > --- > > make check > > > Thanks, > > Alexander Rojas > >
Re: Review Request 42027: Changes HTTP responses from Unauthorized (401) to Forbidden (403).
> On Jan. 11, 2016, 3:02 p.m., Benjamin Bannier wrote: > > Looks mostly gtm. `HTTPTest.Auth` still uses `Unauthorized`; given that > > that part of that test seems to just check the behavior of its own mocks it > > probably makes sense to remove it completely. Adressed your issues in follow up patches. - Alexander --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/42027/#review113755 --- On Jan. 11, 2016, 2:25 p.m., Alexander Rojas wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/42027/ > --- > > (Updated Jan. 11, 2016, 2:25 p.m.) > > > Review request for mesos, Alexander Rukletsov, Greg Mann, Joerg Schad, Jan > Schlicht, and Till Toenshoff. > > > Bugs: MESOS-4305 > https://issues.apache.org/jira/browse/MESOS-4305 > > > Repository: mesos > > > Description > --- > > It is a common patter within Mesos to return an HTTP 401 (Unauthorized) > response whenever the request is invalid for whatever reason. However, > according to the [RFC-2617 Section > 1.2](https://tools.ietf.org/html/rfc2617#section-1.2): > > The 401 (Unauthorized) response message is used by an origin server to > > challenge the authorization of a user agent. This response MUST include a > > WWW-Authenticate header field containing at least one challenge applicable > > to the requested resource. > > Meaning that despite the confusing name, the status code _401 Unauthorized_ > should be used only for authentication purposes. On the other hand, the > [RFC-2616 Section > 10.4.4](http://www.w3.org/Protocols/rfc2616/rfc2616-sec10.html#sec10.4.4) > states: > > _(403 Forbidden is returned when)_ 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. > > As such, _403 (Forbidden)_ seems to be a better return code when replying > inside endpoint handlers, while _401 (Unauthorized)_ should be left to the > HTTP Authenticators only. > > > Diffs > - > > docs/authorization.md a928f1722dc67cd791d78ebbe4591f2e8f2e8f2a > src/master/http.cpp bcafc7aff89659a68352f3876ce6042f8b34bd5d > src/master/quota_handler.cpp 134a93b1d1b6e050aa8a5037ffbec2cc305b0694 > src/tests/master_quota_tests.cpp 776a168254af6fa8a5d87d4580b35d83f2d5909a > src/tests/persistent_volume_endpoints_tests.cpp > f0cce190abc90f0fae84d6c3db20e8215c2d8132 > src/tests/reservation_endpoints_tests.cpp > b8edd6fafedd4c2221a8d19c1ebc71254071a8c7 > src/tests/scheduler_http_api_tests.cpp > 4d23a5a8368e0ed126469fa4a90a889b339ad004 > src/tests/teardown_tests.cpp 97cc89ba168aefff8512f6d1a25c4f7ddf180bae > > Diff: https://reviews.apache.org/r/42027/diff/ > > > Testing > --- > > make check > > > Thanks, > > Alexander Rojas > >
Re: Review Request 42027: Changes HTTP responses from Unauthorized (401) to Forbidden (403).
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/42027/#review113755 --- Looks mostly gtm. `HTTPTest.Auth` still uses `Unauthorized`; given that that part of that test seems to just check the behavior of its own mocks it probably makes sense to remove it completely. - Benjamin Bannier On Jan. 11, 2016, 2:25 p.m., Alexander Rojas wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/42027/ > --- > > (Updated Jan. 11, 2016, 2:25 p.m.) > > > Review request for mesos, Alexander Rukletsov, Greg Mann, Joerg Schad, Jan > Schlicht, and Till Toenshoff. > > > Bugs: MESOS-4305 > https://issues.apache.org/jira/browse/MESOS-4305 > > > Repository: mesos > > > Description > --- > > It is a common patter within Mesos to return an HTTP 401 (Unauthorized) > response whenever the request is invalid for whatever reason. However, > according to the [RFC-2617 Section > 1.2](https://tools.ietf.org/html/rfc2617#section-1.2): > > The 401 (Unauthorized) response message is used by an origin server to > > challenge the authorization of a user agent. This response MUST include a > > WWW-Authenticate header field containing at least one challenge applicable > > to the requested resource. > > Meaning that despite the confusing name, the status code _401 Unauthorized_ > should be used only for authentication purposes. On the other hand, the > [RFC-2616 Section > 10.4.4](http://www.w3.org/Protocols/rfc2616/rfc2616-sec10.html#sec10.4.4) > states: > > _(403 Forbidden is returned when)_ 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. > > As such, _403 (Forbidden)_ seems to be a better return code when replying > inside endpoint handlers, while _401 (Unauthorized)_ should be left to the > HTTP Authenticators only. > > > Diffs > - > > docs/authorization.md a928f1722dc67cd791d78ebbe4591f2e8f2e8f2a > src/master/http.cpp bcafc7aff89659a68352f3876ce6042f8b34bd5d > src/master/quota_handler.cpp 134a93b1d1b6e050aa8a5037ffbec2cc305b0694 > src/tests/master_quota_tests.cpp 776a168254af6fa8a5d87d4580b35d83f2d5909a > src/tests/persistent_volume_endpoints_tests.cpp > f0cce190abc90f0fae84d6c3db20e8215c2d8132 > src/tests/reservation_endpoints_tests.cpp > b8edd6fafedd4c2221a8d19c1ebc71254071a8c7 > src/tests/scheduler_http_api_tests.cpp > 4d23a5a8368e0ed126469fa4a90a889b339ad004 > src/tests/teardown_tests.cpp 97cc89ba168aefff8512f6d1a25c4f7ddf180bae > > Diff: https://reviews.apache.org/r/42027/diff/ > > > Testing > --- > > make check > > > Thanks, > > Alexander Rojas > >
Re: Review Request 42027: Changes HTTP responses from Unauthorized (401) to Forbidden (403).
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/42027/ --- (Updated Jan. 11, 2016, 2:25 p.m.) Review request for mesos, Alexander Rukletsov, Greg Mann, Joerg Schad, Jan Schlicht, and Till Toenshoff. Changes --- Benjamin's review requested changes. Summary (updated) - Changes HTTP responses from Unauthorized (401) to Forbidden (403). Bugs: MESOS-4305 https://issues.apache.org/jira/browse/MESOS-4305 Repository: mesos Description (updated) --- It is a common patter within Mesos to return an HTTP 401 (Unauthorized) response whenever the request is invalid for whatever reason. However, according to the [RFC-2617 Section 1.2](https://tools.ietf.org/html/rfc2617#section-1.2): > The 401 (Unauthorized) response message is used by an origin server to > challenge the authorization of a user agent. This response MUST include a > WWW-Authenticate header field containing at least one challenge applicable to > the requested resource. Meaning that despite the confusing name, the status code _401 Unauthorized_ should be used only for authentication purposes. On the other hand, the [RFC-2616 Section 10.4.4](http://www.w3.org/Protocols/rfc2616/rfc2616-sec10.html#sec10.4.4) states: > _(403 Forbidden is returned when)_ 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. As such, _403 (Forbidden)_ seems to be a better return code when replying inside endpoint handlers, while _401 (Unauthorized)_ should be left to the HTTP Authenticators only. Diffs (updated) - docs/authorization.md a928f1722dc67cd791d78ebbe4591f2e8f2e8f2a src/master/http.cpp bcafc7aff89659a68352f3876ce6042f8b34bd5d src/master/quota_handler.cpp 134a93b1d1b6e050aa8a5037ffbec2cc305b0694 src/tests/master_quota_tests.cpp 776a168254af6fa8a5d87d4580b35d83f2d5909a src/tests/persistent_volume_endpoints_tests.cpp f0cce190abc90f0fae84d6c3db20e8215c2d8132 src/tests/reservation_endpoints_tests.cpp b8edd6fafedd4c2221a8d19c1ebc71254071a8c7 src/tests/scheduler_http_api_tests.cpp 4d23a5a8368e0ed126469fa4a90a889b339ad004 src/tests/teardown_tests.cpp 97cc89ba168aefff8512f6d1a25c4f7ddf180bae Diff: https://reviews.apache.org/r/42027/diff/ Testing --- make check Thanks, Alexander Rojas