Last modified: 2014-02-24 15:50:50 UTC

Wikimedia Bugzilla is closed!

Wikimedia has migrated from Bugzilla to Phabricator. Bug reports should be created and updated in Wikimedia Phabricator instead. Please create an account in Phabricator and add your Bugzilla email address to it.
Wikimedia Bugzilla is read-only. If you try to edit or create any bug report in Bugzilla you will be shown an intentional error message.
In order to access the Phabricator task corresponding to a Bugzilla report, just remove "static-" from its URL.
You could still run searches in Bugzilla or access your list of votes but bug reports will obviously not be up-to-date in Bugzilla.
Bug 61841 - Re-examine API module permissions (isAllowed, isBlocked) checks
Re-examine API module permissions (isAllowed, isBlocked) checks
Status: UNCONFIRMED
Product: MediaWiki
Classification: Unclassified
API (Other open bugs)
1.23.0
All All
: Low enhancement (vote)
: ---
Assigned To: Nobody - You can work on this!
:
Depends on:
Blocks: code_quality
  Show dependency treegraph
 
Reported: 2014-02-24 06:16 UTC by MZMcBride
Modified: 2014-02-24 15:50 UTC (History)
6 users (show)

See Also:
Web browser: ---
Mobile Platform: ---
Assignee Huggle Beta Tester: ---


Attachments

Description MZMcBride 2014-02-24 06:16:09 UTC
https://gerrit.wikimedia.org/r/#/c/114974/2/api/ApiGroupReview.php,unified seems to have a bad code smell to me. Do we really require every API module to define "isAllowed" and "isBlocked"? That seems like a flawed architectural design to me, if so. When being extended, it seems to me that the base MediaWiki API should be able to handle these checks implicitly, to avoid situations like this.

Or, alternately, perhaps we need to fail faster when these checks are missing.
Comment 1 Brad Jorsch 2014-02-24 15:50:50 UTC
A nitpick: We don't require API modules to "define" any such methods. But if something wants to check for user rights or for blocked status then it needs to *call* (not "define") the methods to check for user rights or blocked status.

This seems to me as little different from now the rest of MediaWiki handles permissions checks, except for Special pages. And even many special pages need to go beyond the simple checks provided for in the base class. So no, I don't think that we need to try to cram flags for block and permissions checking into the already-overgrown API base classes.


Regarding that linked change in particular, it sounds like they have code duplication with their web UI code and their API code. Instead there should be some back-end method that does the actual work and returns status to the UI layer and both the web UI and the API should be calling that.

Note You need to log in before you can comment on or make changes to this bug.


Navigation
Links