Page MenuHomePhabricator

Do API permission checks before validation
Open, LowPublic


When working on T211119, I noticed that the API checks the validity of requests before it checks whether the user has the right to execute the request.
That order should be inverted. The new order should probably look something like this:

  1. Check whether the user is allowed to execute e.g. wbcreateclaim. (He might be blocked)
  2. Look at the other request parameters and check whether they are valid. (A parameter might be missing or malformed)
  3. Check whether the user has the rights to take that action on that item. (The item might be protected)

Acceptance criteria:

  • API requests check that a user is allowed to make that request before checking the validity of the request parameters
  • A test for this behavior

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald Transcript
Addshore added a project: Technical-Debt.
Addshore moved this task from incoming to ready to go on the Wikidata board.

There are a couple of situations that I remember where some validation needs to take place first in order to know what permissions to check.
This mainly applies to api modules that act on multiple entity types where entity type specific permissions also exist.
As for the points #1,2,3 in the description I agree with the order there.

This is probably pretty low prio and probably doesn't it in as a trailbalzer, but rather a campsite task.
Thoughts @alaa_wmde ?

Dinoguy1000 renamed this task from Do API persmission checks before validation to Do API permission checks before validation.Aug 13 2019, 4:05 AM

I'm going to put this in the freezer as it would be nice to have but we should be moving toward transitioning away from these older API modules towards newer REST modules (to be created).
We should take this into account when creating those though!