Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Disambiguate 403 Forbidden #218

Closed
asbjornu opened this issue Apr 8, 2019 · 8 comments
Closed

Disambiguate 403 Forbidden #218

asbjornu opened this issue Apr 8, 2019 · 8 comments
Assignees

Comments

@asbjornu
Copy link

asbjornu commented Apr 8, 2019

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
Copy link
Member

mnot commented Apr 10, 2019

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
Copy link
Author

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 mnot added the semantics label Apr 12, 2019
@mnot mnot added the discuss label Jul 11, 2019
@mnot
Copy link
Member

mnot commented Jul 25, 2019

Discussed in Montreal; ok to change.

@reschke reschke self-assigned this Jul 25, 2019
reschke added a commit that referenced this issue Jul 26, 2019
@asbjornu
Copy link
Author

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
Copy link
Contributor

reschke commented Jul 26, 2019

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
Copy link
Author

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
Copy link
Contributor

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

royfielding added a commit that referenced this issue Aug 7, 2019
@MikeBishop
Copy link
Contributor

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"

@reschke reschke closed this as completed Aug 31, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

5 participants