httpwg / http-core

Core HTTP Specifications
https://httpwg.org/http-core/
467 stars 43 forks source link

Disambiguate 403 Forbidden #218

Closed asbjornu closed 5 years ago

asbjornu commented 5 years ago

In its current wording, I interpret RFC 7231 §6.5.3 to allow 403 Forbidden to be used in the following two situations:

  1. The requested operation is forbidden in the current state of the resource.
  2. The requested operation is forbidden (for the user identified) by the provided authentication credentials.

This ambiguity is established in the following paragraph:

If authentication credentials were provided in the request, the server considers them insufficient to grant access. The client SHOULD NOT automatically repeat the request with the same credentials. The client MAY repeat the request with new or different credentials.

The ambiguity is somewhat lifted (but not entirely) by the last sentence:

However, a request might be forbidden for reasons unrelated to the credentials.

However, I believe the ambiguity exists and causes a lot of developers to believe that 403 Forbidden is only applicable to authorization errors. I think the wording in the previous paragraph:

The 403 (Forbidden) status code indicates that the server understood the request but refuses to authorize it. A server that wishes to make public why the request has been forbidden can describe that reason in the response payload (if any).

…the example given in RFC 7807:

HTTP/1.1 403 Forbidden
Content-Type: application/problem+json
Content-Language: en

{
    "type": "https://example.com/probs/out-of-credit",
    "title": "You do not have enough credit.",
    "detail": "Your current balance is 30, but that costs 50.",
    "instance": "/account/12345/msgs/abc",
    "balance": 30,
    "accounts": ["/account/12345",
                 "/account/67890"]
}

…and the wording in RFC 7807 §4:

For example, a "write access disallowed" problem is probably unnecessary, since a 403 Forbidden status code in response to a PUT request is self-explanatory.

…as well as existing practice establishes that 403 Forbidden is the correct status code to respond with when the requested operation is not allowed in the current state of the application (use-case 1 above), regardless of the Authorization header provided.

Introducing the ambiguity of 403 only being applicable to the provided authorization when authorization is available makes the status code harder to reason about and therefore harder to use in practice. Would it be possible to remove this ambiguity somehow? Here's some suggested wording:

If authentication credentials were provided in the request, the server might consider them insufficient to grant access to the requested resource. In that case, the client SHOULD NOT automatically repeat the request with the same credentials. The client MAY repeat the request with new or different credentials. However, a request might be forbidden for reasons unrelated to the credentials. For request methods other than HEAD, the server SHOULD generate a payload indicating why the request was forbidden.

mnot commented 5 years ago

ISTM that the phrase "authorize it" in the first sentence of 6.5.3 is the root of the issue. I note that 2616 used "fulfill it" instead; we should dig around and see if that was an intentional change.

asbjornu commented 5 years ago

I agree "authorize it" is the root issue, but think it's exacerbated by the following paragraph. It would be good to clarify the entire section to make it obvious that the presence of an Authorization header doesn't require the semantics to change.

mnot commented 5 years ago

Discussed in Montreal; ok to change.

asbjornu commented 5 years ago

The change from “authorize” to “fulfill” helps to resolve the ambiguity, although I would like to see a MAY in the following paragraph:

If authentication credentials were provided in the request, the server MAY consider them insufficient to grant access. The client SHOULD NOT automatically repeat the request with the same credentials. The client MAY repeat the request with new or different credentials.

reschke commented 5 years ago

I don't think a "MAY" helps here.

The current text is:

If authentication credentials were provided in the request, the server considers them insufficient to grant access.

That's a statement of fact. If the credentials would have been sufficient, there wouldn't be an error. (It does not necessarily imply that things would have worked with different credentials).

asbjornu commented 5 years ago

I think that’s exactly what the current text implies: That sufficient credentials would have lead to a successful request, regardless of the state of the resource.

If we take the example from RFC 7807, there is nothing that a change of credentials can do to make the request succeed — the credit is out regardless of the authorisation.

ioggstream commented 5 years ago

Question: this change makes 429 a specialization of 403, right?

MikeBishop commented 5 years ago

But that proposed "MAY" isn't permissive. This is a statement, not necessarily of fact, but of one likely possibility. Perhaps "the server considers" => "the server might consider"