WordPress.org

WordPRess GSoC Trac

Opened 9 months ago

Closed 7 months ago

#338 closed defect (fixed)

HTTP Status 405 on disabled should be 403

Reported by: MikeSchinkel Owned by: rmccue
Milestone: Priority: normal
Component: JSON REST API Keywords:
Cc:

Description

The plugin currently serves 405 Method Not Allowed as an error code when the plugin is disabled. 405 is for when the wrong HTTP method is provided where methods are GET, POST, PUT, etc.:

A request was made of a resource using a request method not supported by that resource; for example, using GET on a form which requires data to be presented via POST, or using PUT on a read-only resource.

The proper status code to return should be 403 Forbidden:

A web server may return a 403 Forbidden HTTP status code in response to a request from a client for a web page or resource to indicate that the server can be reached and understood the request, but refuses to take any further action.

Change History (12)

comment:1 rmccue9 months ago

403 is not really appropriate, given its usage context (that is, that the user cannot perform the action). A 404 or 410 might be more appropriate here instead.

comment:2 MikeSchinkel9 months ago

HTTP status codes are meant as responses to the user agent, not as responses for the user. It's the user agent's responsibility to translate to into user-friendly terms. Thus 404 is not appropriate here because the resource was indeed found, it was just not accepting responses; the latter is what 403 was intended for.

410 might have been appropriate if it were not for the fact that it includes the language "Indicates that the resource requested ... will not be available again." Given that it is likely many users will enable disabled APIs after they realize they need them thus sending a 410 could result in web intermediaries refusing to send new requests if they have already cached a 410 response.

Last edited 9 months ago by MikeSchinkel (previous) (diff)

comment:3 follow-up: bpetty9 months ago

Except that a 403 also indicates that, should the browser provide appropriate authentication credentials, it *would* be successful, but that's not true either. Reserving 403 strictly for auth credential issues (as that's what it will mostly be used for) even if it means using 404 or 410 here would be more likely to avoid poorly implemented clients from prompting users for authentication that will never work because it's disabled entirely.

So I can see arguments on both sides honestly.

comment:4 in reply to: ↑ 3 ; follow-up: MikeSchinkel9 months ago

Replying to bpetty:

Except that a 403 also indicates that, should the browser provide appropriate authentication credentials, it *would* be successful, but that's not true either. Reserving 403 strictly for auth credential issues (as that's what it will mostly be used for)...

As I read Wikipedia, it reads to me that you've got it wrong. This is what is says about 403, (emphasis mine):

The request was a valid request, but the server is refusing to respond to it.[2] Unlike a 401 Unauthorized response, authenticating will make no difference.[2] On servers where authentication is required, this commonly means that the provided credentials were successfully authenticated but that the credentials still do not grant the client permission to access the resource (e.g. a recognized user attempting to access restricted content).

So it would seem you are referring to a 401, not a 403?

comment:5 in reply to: ↑ 4 ; follow-up: rmccue9 months ago

Replying to MikeSchinkel:

So it would seem you are referring to a 401, not a 403?

Directly from RFC2616:

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.

A 403 means in practice that authentication was successful, but the user doesn't have the permission to do the task.

On the other hand, a 410 is the following (emphasis mine):

The requested resource is no longer available at the server and no forwarding address is known. This condition is expected to be considered permanent. Clients with link editing capabilities SHOULD delete references to the Request-URI after user approval. If the server does not know, or has no facility to determine, whether or not the condition is permanent, the status code 404 (Not Found) SHOULD be used instead. This response is cacheable unless indicated otherwise.

The 410 response is primarily intended to assist the task of web maintenance by notifying the recipient that the resource is intentionally unavailable and that the server owners desire that remote links to that resource be removed. Such an event is common for limited-time, promotional services and for resources belonging to individuals no longer working at the server's site. It is not necessary to mark all permanently unavailable resources as "gone" or to keep the mark for any length of time -- that is left to the discretion of the server owner.

Hence my comment that either 404 or 410 is appropriate. A 410 is probably too permanent, so 404 is most likely the best option here.

comment:6 in reply to: ↑ 5 ; follow-up: MikeSchinkel9 months ago

Replying to rmccue:

A 403 means in practice that authentication was successful, but the user doesn't have the permission to do the task.

That is not my interpretation of the section on 403 in RFC2616. But be that as it may.

comment:7 in reply to: ↑ 6 rmccue9 months ago

Replying to MikeSchinkel:

Replying to rmccue:

A 403 means in practice that authentication was successful, but the user doesn't have the permission to do the task.

That is not my interpretation of the section on 403 in RFC2616. But be that as it may.

In essence:

  • 401 - Not logged in, no permissions to perform task, try authenticating
  • 403 - Logged in, no permissions to perform task

Also, I'd like to note that both 401 and 403 are directly related to authentication, whereas this error condition is not at all. The error condition is directly related to resource availability.

So, to determine which error code, here's the process of elimination I used (in addition to what the spec actually says):

  • The error is a client error (as it's related to the request), so 4xx
  • The error is not related to the syntax, so not 400, 405, 406, 408, 411, 412, 413, 414, 416, 417
  • The error is not related to authentication, so not 401, 403, 407
  • The error is not related to payment, so not 402
  • The error is not related to a submitted entity, so not 409
  • This leaves 404, 410, 415
  • Of those, only 404 and 410 relate to the availability of the resource

In addition, the spec outlines a case similar to this one in the 410 specification. We specifically want to indicate that the resource is intentionally unavailable. We don't know whether it will be permanent or not, so 404 is the correct error here, but 410 may be more specific (since it's fairly likely that it will be disabled at least semi-permanently).

comment:8 MikeSchinkel9 months ago

I think we've all been wrong. 4xx is the wrong class, it should be 5xx, specifically 503 Service Unavailable.

Reference this thread: https://groups.google.com/forum/#!topic/api-craft/faC3OklpoCo

comment:9 follow-up: rmccue9 months ago

Hmm, good point. 503 is probably a good choice here, although it does usually indicate a temporary unavailability (and also means in future we have to be careful about maintenance mode handling and such).

I'll have a look over this in the morning, but I think a server error is probably the right track (and 503 seems the most appropriate). Thanks!

comment:10 in reply to: ↑ 9 MikeSchinkel9 months ago

Replying to rmccue:

Thanks!

It was really the collective knowledge of the API Craft group. I like to run things by them because their many heads are better than just mine, at least when it comes to web APIs.

comment:11 MikeSchinkel9 months ago

Funny, some of the more prolific posters on the list are now saying not 5xx. Probably best to follow that thread I posted in comment 8.

comment:12 rmccue7 months ago

  • Owner set to rmccue
  • Resolution set to fixed
  • Status changed from new to closed

In 2292:

Use better error codes for disabled features

Fixes #338

Note: See TracTickets for help on using tickets.