Last modified: 2011-03-22 22:37:05 UTC

Wikimedia Bugzilla is closed!

Wikimedia migrated from Bugzilla to Phabricator. Bug reports are handled in Wikimedia Phabricator.
This static website is read-only and for historical purposes. It is not possible to log in and except for displaying bug reports and their history, links might be broken. See T30166, the corresponding Phabricator task for complete and up-to-date bug report information.
Bug 28166 - UploadBase assumes that 'edit' and 'upload' rights are not per page restrictions
UploadBase assumes that 'edit' and 'upload' rights are not per page restrictions
Status: RESOLVED FIXED
Product: MediaWiki
Classification: Unclassified
Uploading (Other open bugs)
unspecified
All All
: Normal major with 1 vote (vote)
: ---
Assigned To: Nobody - You can work on this!
:
: 28169 (view as bug list)
Depends on:
Blocks: 27339 27470
  Show dependency treegraph
 
Reported: 2011-03-21 20:58 UTC by Sam Reed (reedy)
Modified: 2011-03-22 22:37 UTC (History)
3 users (show)

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


Attachments

Description Sam Reed (reedy) 2011-03-21 20:58:41 UTC
<vvv> Reedy: probably someone removed check for page creation permission from
upload module
<vvv> Was it rewritten in 1.17?
<Reedy> I think so
<snip>
<vvv> Reedy: it looks like UploadBase assumes that 'edit', 'upload', etc are
not per-page restrictions
<vvv> Reedy: oh, and it also allows to upload images even when they are
create-protected


Seems to have caused bug 27470
Comment 1 Victor Vasiliev 2011-03-21 21:14:55 UTC
Just to clarify: the UploadBase should use proper Title::getUserPermissionsErrors call for checking create, edit and upload rights instead of checking them itself.
Comment 2 Sam Reed (reedy) 2011-03-21 21:27:42 UTC
I suppose it's Title::newFromWhatever( $idontcare )->userCan( 'action' )
Comment 3 Victor Vasiliev 2011-03-21 21:30:58 UTC
(In reply to comment #2)
> I suppose it's Title::newFromWhatever( $idontcare )->userCan( 'action' )

No, do NOT use userCan. If you use it, you cannot output the meaningful error message, and you certainly should.
Comment 4 Sam Reed (reedy) 2011-03-21 21:31:12 UTC
*** Bug 28169 has been marked as a duplicate of this bug. ***
Comment 5 Sam Reed (reedy) 2011-03-21 21:31:49 UTC
Well, getUserPermissionsErrors is private for a start ;)
Comment 6 Victor Vasiliev 2011-03-21 21:48:00 UTC
(In reply to comment #5)
> Well, getUserPermissionsErrors is private for a start ;)


Is it?

public function getUserPermissionsErrors( $action, $user, $doExpensiveQueries = true, $ignoreErrors = array() )
Comment 7 Sam Reed (reedy) 2011-03-21 21:50:56 UTC
Looks like I'm being blind. Never mind. :)
Comment 8 Bryan Tong Minh 2011-03-22 08:40:46 UTC
	public function verifyPermissions( $user ) { 
[...]
403 	$permErrors = $nt->getUserPermissionsErrors( 'edit', $user );
404 	$permErrorsUpload = $nt->getUserPermissionsErrors( 'upload', $user );
405 	if ( $nt->exists() ) {
406 	$permErrorsCreate = $nt->getUserPermissionsErrors( 'createpage', $user );
407 	} else {
408 	$permErrorsCreate = array();
409 	}
Comment 9 Victor Vasiliev 2011-03-22 08:58:45 UTC
(In reply to comment #8)
> 405     if ( $nt->exists() ) {
> 406     $permErrorsCreate = $nt->getUserPermissionsErrors( 'createpage', $user

I think it should be !$nt->exists().
Comment 10 Sam Reed (reedy) 2011-03-22 22:23:59 UTC
I'd concur, also based on CR on r65898

I've committed it in r84573, but it hasn't fix the issue as of yet
Comment 11 Sam Reed (reedy) 2011-03-22 22:37:05 UTC
Fixed with r84573 and r84575

However, seems I broke some error handling in r83979, will investigate

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


Navigation
Links