You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Alexander Rojas <al...@mesosphere.io> on 2016/01/11 16:42:49 UTC

Re: Review Request 42027: Changed 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, 4:42 p.m.)


Review request for mesos, Alexander Rukletsov, Greg Mann, Joerg Schad, Jan Schlicht, and Till Toenshoff.


Summary (updated)
-----------------

Changed HTTP responses from Unauthorized (401) to Forbidden (403).


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: Changed HTTP responses from Unauthorized (401) to Forbidden (403).

Posted by Alexander Rojas <al...@mesosphere.io>.

> On Jan. 13, 2016, 4:05 p.m., Alexander Rukletsov wrote:
> > I'm reading section 1.2 of RFC2617 and see the following paragraph:
> > 
> > >   If the origin server does not wish to accept the credentials sent
> > >   with a request, it SHOULD return a 401 (Unauthorized) response. The
> > >   response MUST include a WWW-Authenticate header field containing at
> > >   least one (possibly new) challenge applicable to the requested
> > >   resource...
> > 
> > I would like to confirm my understanding of the proposed change is correct.
> > 
> > Authentication is general to HTTP, we leverage HTTP headers and adhere RFCs. Authorization is entirely internal to Mesos and is not regulated by any standards. Currently we use `Unauthorized` for both authn and authz failures. Section 1.2 of RFC2617 hints that using `Unauthorized` for authentication is correct. However, because Mesos authorization is actually an implementation detail, we should use a more general return code, e.g. `Forbidden`. So far so good? (I can't help but think about putting an eastern egg in Mesos and sporadically return `402 Payment Required` for, say, dynamic reservation requests : ) ).
> > 
> > If I read sections 10.4.4 and 10.4.4 of RFC2616 correctly, `Forbidden` should include the reason for the refusal. Do you think it makes sense to include reasons everywhere? Maybe even remove parameterless c-tor for `Forbidden` (then comparing status will be tricky, but maybe we should make `status` static anyway)?

Completely agreed, It would be better if we removed the default constructor there and force to add the reasons. I'll open a JIRA for that.


> On Jan. 13, 2016, 4:05 p.m., Alexander Rukletsov wrote:
> > src/master/http.cpp, lines 356-360
> > <https://reviews.apache.org/r/42027/diff/4/?file=1192104#file1192104line356>
> >
> >     Could you please help me understand why this shoud be `Forbidden`? If my understanding is correct, it should stay `Unauthorized` no HTTP auth headers are provided and it has nothing to do with the Mesos authz.
> >     
> >     Here is what I see in the RFC:
> >     "   If the origin server does not wish to accept the credentials sent with a request, it SHOULD return a 401 (Unauthorized) response."

Because this is not doing HTTP Authentication, and it wasn't returning the header `WWW-Authenticate`. So we decided `Forbidden` was a more aptly return code (Remember, this is the old cram_md5 authenticator).


- Alexander


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


On Jan. 11, 2016, 4:42 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, 4:42 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: Changed HTTP responses from Unauthorized (401) to Forbidden (403).

Posted by Alexander Rojas <al...@mesosphere.io>.

> On Jan. 13, 2016, 4:05 p.m., Alexander Rukletsov wrote:
> > I'm reading section 1.2 of RFC2617 and see the following paragraph:
> > 
> > >   If the origin server does not wish to accept the credentials sent
> > >   with a request, it SHOULD return a 401 (Unauthorized) response. The
> > >   response MUST include a WWW-Authenticate header field containing at
> > >   least one (possibly new) challenge applicable to the requested
> > >   resource...
> > 
> > I would like to confirm my understanding of the proposed change is correct.
> > 
> > Authentication is general to HTTP, we leverage HTTP headers and adhere RFCs. Authorization is entirely internal to Mesos and is not regulated by any standards. Currently we use `Unauthorized` for both authn and authz failures. Section 1.2 of RFC2617 hints that using `Unauthorized` for authentication is correct. However, because Mesos authorization is actually an implementation detail, we should use a more general return code, e.g. `Forbidden`. So far so good? (I can't help but think about putting an eastern egg in Mesos and sporadically return `402 Payment Required` for, say, dynamic reservation requests : ) ).
> > 
> > If I read sections 10.4.4 and 10.4.4 of RFC2616 correctly, `Forbidden` should include the reason for the refusal. Do you think it makes sense to include reasons everywhere? Maybe even remove parameterless c-tor for `Forbidden` (then comparing status will be tricky, but maybe we should make `status` static anyway)?
> 
> Alexander Rojas wrote:
>     Completely agreed, It would be better if we removed the default constructor there and force to add the reasons. I'll open a JIRA for that.
> 
> Alexander Rukletsov wrote:
>     Great, thanks a bunch!

On the other hand, from the [rfc 2119](https://www.ietf.org/rfc/rfc2119.txt):
> SHOULD This word, or the adjective "RECOMMENDED", mean that there may exist valid reasons in particular circumstances to ignore a particular item, but the full implications must be understood and carefully weighed before choosing a different course.

I will still open the issue but I feel is more debatable now.


- Alexander


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


On Jan. 11, 2016, 4:42 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, 4:42 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: Changed HTTP responses from Unauthorized (401) to Forbidden (403).

Posted by Alexander Rukletsov <ru...@gmail.com>.

> On Jan. 13, 2016, 3:05 p.m., Alexander Rukletsov wrote:
> > I'm reading section 1.2 of RFC2617 and see the following paragraph:
> > 
> > >   If the origin server does not wish to accept the credentials sent
> > >   with a request, it SHOULD return a 401 (Unauthorized) response. The
> > >   response MUST include a WWW-Authenticate header field containing at
> > >   least one (possibly new) challenge applicable to the requested
> > >   resource...
> > 
> > I would like to confirm my understanding of the proposed change is correct.
> > 
> > Authentication is general to HTTP, we leverage HTTP headers and adhere RFCs. Authorization is entirely internal to Mesos and is not regulated by any standards. Currently we use `Unauthorized` for both authn and authz failures. Section 1.2 of RFC2617 hints that using `Unauthorized` for authentication is correct. However, because Mesos authorization is actually an implementation detail, we should use a more general return code, e.g. `Forbidden`. So far so good? (I can't help but think about putting an eastern egg in Mesos and sporadically return `402 Payment Required` for, say, dynamic reservation requests : ) ).
> > 
> > If I read sections 10.4.4 and 10.4.4 of RFC2616 correctly, `Forbidden` should include the reason for the refusal. Do you think it makes sense to include reasons everywhere? Maybe even remove parameterless c-tor for `Forbidden` (then comparing status will be tricky, but maybe we should make `status` static anyway)?
> 
> Alexander Rojas wrote:
>     Completely agreed, It would be better if we removed the default constructor there and force to add the reasons. I'll open a JIRA for that.

Great, thanks a bunch!


> On Jan. 13, 2016, 3:05 p.m., Alexander Rukletsov wrote:
> > src/master/http.cpp, lines 356-360
> > <https://reviews.apache.org/r/42027/diff/4/?file=1192104#file1192104line356>
> >
> >     Could you please help me understand why this shoud be `Forbidden`? If my understanding is correct, it should stay `Unauthorized` no HTTP auth headers are provided and it has nothing to do with the Mesos authz.
> >     
> >     Here is what I see in the RFC:
> >     "   If the origin server does not wish to accept the credentials sent with a request, it SHOULD return a 401 (Unauthorized) response."
> 
> Alexander Rojas wrote:
>     Because this is not doing HTTP Authentication, and it wasn't returning the header `WWW-Authenticate`. So we decided `Forbidden` was a more aptly return code (Remember, this is the old cram_md5 authenticator).

Ok, makes sense.


- Alexander


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


On Jan. 11, 2016, 3:42 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, 3:42 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: Changed HTTP responses from Unauthorized (401) to Forbidden (403).

Posted by Alexander Rukletsov <ru...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42027/#review114201
-----------------------------------------------------------


I'm reading section 1.2 of RFC2617 and see the following paragraph:

>   If the origin server does not wish to accept the credentials sent
>   with a request, it SHOULD return a 401 (Unauthorized) response. The
>   response MUST include a WWW-Authenticate header field containing at
>   least one (possibly new) challenge applicable to the requested
>   resource...

I would like to confirm my understanding of the proposed change is correct.

Authentication is general to HTTP, we leverage HTTP headers and adhere RFCs. Authorization is entirely internal to Mesos and is not regulated by any standards. Currently we use `Unauthorized` for both authn and authz failures. Section 1.2 of RFC2617 hints that using `Unauthorized` for authentication is correct. However, because Mesos authorization is actually an implementation detail, we should use a more general return code, e.g. `Forbidden`. So far so good? (I can't help but think about putting an eastern egg in Mesos and sporadically return `402 Payment Required` for, say, dynamic reservation requests : ) ).

If I read sections 10.4.4 and 10.4.4 of RFC2616 correctly, `Forbidden` should include the reason for the refusal. Do you think it makes sense to include reasons everywhere? Maybe even remove parameterless c-tor for `Forbidden` (then comparing status will be tricky, but maybe we should make `status` static anyway)?


src/master/http.cpp (lines 355 - 358)
<https://reviews.apache.org/r/42027/#comment175023>

    Could you please help me understand why this shoud be `Forbidden`? If my understanding is correct, it should stay `Unauthorized` no HTTP auth headers are provided and it has nothing to do with the Mesos authz.
    
    Here is what I see in the RFC:
    "   If the origin server does not wish to accept the credentials sent with a request, it SHOULD return a 401 (Unauthorized) response."


- Alexander Rukletsov


On Jan. 11, 2016, 3:42 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, 3:42 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: Changed HTTP responses from Unauthorized (401) to Forbidden (403).

Posted by Till Toenshoff <to...@me.com>.

> On Jan. 20, 2016, 7:37 p.m., Till Toenshoff wrote:
> > src/master/http.cpp, lines 549-553
> > <https://reviews.apache.org/r/42027/diff/5/?file=1200104#file1200104line549>
> >
> >     To prosterity; this means that we currently do not support mixed usage of old API and HTTP API for frameworks as soon as the master has enabled mandatory framework authentication.

Sorry, did not mean to make this an issue - dropping it to make it a comment.


- Till


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


On Jan. 20, 2016, 2:03 p.m., Alexander Rojas wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42027/
> -----------------------------------------------------------
> 
> (Updated Jan. 20, 2016, 2:03 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 pattern 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 34a70ee50553492fc8c3947497ab5922f4379d72 
>   src/master/quota_handler.cpp f44736cd5849d4fb22a75c1238d433a1c0c9708d 
>   src/tests/master_quota_tests.cpp e8cb074c2913cafdc6b1792896f29e53f1210c9d 
>   src/tests/persistent_volume_endpoints_tests.cpp f0cce190abc90f0fae84d6c3db20e8215c2d8132 
>   src/tests/reservation_endpoints_tests.cpp b8edd6fafedd4c2221a8d19c1ebc71254071a8c7 
>   src/tests/scheduler_http_api_tests.cpp 143bd414c6d9ad0b7b7c23c390b7d497e4be3e6d 
>   src/tests/teardown_tests.cpp 97cc89ba168aefff8512f6d1a25c4f7ddf180bae 
> 
> Diff: https://reviews.apache.org/r/42027/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>


Re: Review Request 42027: Changed HTTP responses from Unauthorized (401) to Forbidden (403).

Posted by Till Toenshoff <to...@me.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42027/#review115423
-----------------------------------------------------------

Ship it!



src/master/http.cpp (lines 548 - 551)
<https://reviews.apache.org/r/42027/#comment176419>

    To prosterity; this means that we currently do not support mixed usage of old API and HTTP API for frameworks as soon as the master has enabled mandatory framework authentication.


- Till Toenshoff


On Jan. 20, 2016, 2:03 p.m., Alexander Rojas wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42027/
> -----------------------------------------------------------
> 
> (Updated Jan. 20, 2016, 2:03 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 pattern 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 34a70ee50553492fc8c3947497ab5922f4379d72 
>   src/master/quota_handler.cpp f44736cd5849d4fb22a75c1238d433a1c0c9708d 
>   src/tests/master_quota_tests.cpp e8cb074c2913cafdc6b1792896f29e53f1210c9d 
>   src/tests/persistent_volume_endpoints_tests.cpp f0cce190abc90f0fae84d6c3db20e8215c2d8132 
>   src/tests/reservation_endpoints_tests.cpp b8edd6fafedd4c2221a8d19c1ebc71254071a8c7 
>   src/tests/scheduler_http_api_tests.cpp 143bd414c6d9ad0b7b7c23c390b7d497e4be3e6d 
>   src/tests/teardown_tests.cpp 97cc89ba168aefff8512f6d1a25c4f7ddf180bae 
> 
> Diff: https://reviews.apache.org/r/42027/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>


Re: Review Request 42027: Changed HTTP responses from Unauthorized (401) to Forbidden (403).

Posted by Alexander Rojas <al...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42027/
-----------------------------------------------------------

(Updated Jan. 20, 2016, 3:03 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 (updated)
-------

It is a common pattern 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 34a70ee50553492fc8c3947497ab5922f4379d72 
  src/master/quota_handler.cpp f44736cd5849d4fb22a75c1238d433a1c0c9708d 
  src/tests/master_quota_tests.cpp e8cb074c2913cafdc6b1792896f29e53f1210c9d 
  src/tests/persistent_volume_endpoints_tests.cpp f0cce190abc90f0fae84d6c3db20e8215c2d8132 
  src/tests/reservation_endpoints_tests.cpp b8edd6fafedd4c2221a8d19c1ebc71254071a8c7 
  src/tests/scheduler_http_api_tests.cpp 143bd414c6d9ad0b7b7c23c390b7d497e4be3e6d 
  src/tests/teardown_tests.cpp 97cc89ba168aefff8512f6d1a25c4f7ddf180bae 

Diff: https://reviews.apache.org/r/42027/diff/


Testing
-------

make check


Thanks,

Alexander Rojas


Re: Review Request 42027: Changed HTTP responses from Unauthorized (401) to Forbidden (403).

Posted by Alexander Rojas <al...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42027/
-----------------------------------------------------------

(Updated Jan. 18, 2016, 11:52 a.m.)


Review request for mesos, Alexander Rukletsov, Greg Mann, Joerg Schad, Jan Schlicht, and Till Toenshoff.


Changes
-------

Rebase.


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 (updated)
-----

  docs/authorization.md a928f1722dc67cd791d78ebbe4591f2e8f2e8f2a 
  src/master/http.cpp 34a70ee50553492fc8c3947497ab5922f4379d72 
  src/master/quota_handler.cpp f44736cd5849d4fb22a75c1238d433a1c0c9708d 
  src/tests/master_quota_tests.cpp e8cb074c2913cafdc6b1792896f29e53f1210c9d 
  src/tests/persistent_volume_endpoints_tests.cpp f0cce190abc90f0fae84d6c3db20e8215c2d8132 
  src/tests/reservation_endpoints_tests.cpp b8edd6fafedd4c2221a8d19c1ebc71254071a8c7 
  src/tests/scheduler_http_api_tests.cpp 143bd414c6d9ad0b7b7c23c390b7d497e4be3e6d 
  src/tests/teardown_tests.cpp 97cc89ba168aefff8512f6d1a25c4f7ddf180bae 

Diff: https://reviews.apache.org/r/42027/diff/


Testing
-------

make check


Thanks,

Alexander Rojas