Page MenuHomePhabricator

img_auth.php: Improve extendability
Open, Needs TriagePublic

Description

As maintainer of Extension:NSFileRepo, I've noticed that implementation of img_auth.php has changed between LTS branches REL1_23 and REL1_27 in a way that makes it more difficult to extend it.

Here is a short list of what I believe should be done:

  • Object oriented implementation (similar to api.php)
  • Plugin interface (to make implementation of custom behavior more easy)
  • Allow access to file repos other than local
  • Change calculation of $publicWiki (in intranet there might be the need to have image authorization even if '*' can read)

I'd like to volunteer for this work, but only if there is a real chance for my changes to make it into the software (and prefereable even into REL1_27 as a backport). Is the dev community generally interested in this? Which developers should I talk to about this (and are willing / have time to answer some of my questions)?

Event Timeline

Thank you for the hint. I probably will do so. But I wrote to mailinglists (mediawiki-l, wikitech-l) in the past and never got much reaction from there.

You are probably interested in the ongoing RfC T66214: Define an official thumb API.

Object oriented implementation (similar to api.php)

That would be awesome (even more so if it includes thumb.php and thumb_handler.php). Turning those into proper objects (and eventually adding unit tests so that they can be refactored with less risk) is long overdue.

Plugin interface (to make implementation of custom behavior more easy)

Not sure what you mean by that. Hooks? Some sort of AuthPlugin-like setup where the site admin can change what class handles image loading? In theory we are applying that to almost everything now with dependency injection.

Allow access to other file repos than local

Private wikis using non-local images should of course work. Does img_auth.php have to be involved in that, though? Usually the images point directly to their home repo.
(See also T27958: thumb.php should work with ForeignAPIRepo.)

Change calculation of $publicWiki (in intranet there might be the need to have image authorization even if '*' can read)

Can you explain the use case in more detail?

Thanks for your reply! And thanks for pointing me to those related tasks.

With "plugin interface" I just mean that there should be an easy way to execute own code along with and influence the processing in img_auth.php. I'm open to suggestions. I'd like to minimize the use of hooks (e.g. just one hook to register extensions/plugins). Using DI also sounds like a perfect solution for this. Are there good examples of DI in the core?

Concerning the non-local images on private wikis: You are right. But there are some extensions that use the foreign file repo mechanism to provide auto-generated files. So even it's called a "foreign file repo" it actually serves local files. Maybe that's a misuse. But it's comfortable, because otherwise such extensions would have to provide an own entry point to serve their files. Also commonly used framework code like e.g. wfFindFile searches all repos by default, so I just think img_auth.php should not be limited to local when trying to find a file.

About $publicWiki: In intranet wiki setups there often is a preliminary user authentication made by the webserver (e.g. in apache with mod_auth_kerb). So only users from the intranet domain may even access the wiki. But there still might be the need to differentiate between logged in users and anonymous ones (e.g. content in NS_MAIN is readable for *, but content from NS_HR is not. Therefore delivery of files with prefix "HR_" is only permitted to users that have group "HR"). When moving to OO implementation I'd just suggest to have this calculation in some function that can be overridden by an extension. No need to remove it completely or change the default behavior.

EDIT: Most of my requirements are connected with issues that I had when I was trying to make Extension:NSFileRepo compatible with MediaWiki REL1_27. See also this commit by C.holtermann: https://gerrit.wikimedia.org/r/#/c/229717/2 (I guess it can be abandoned now)

Are there good examples of DI in the core?

You can check the existing services in ServiceWiring.php / MediaWikiServices.php.

DI is not as flexible as hooks since there is no way for multiple extensions to replace the same class (unless they do some kind of wrapping), but it works as a desperation move.

But there are some extensions that use the foreign file repo mechanism to provide auto-generated files.

That makes sense. Also I guess it's technically possible to have foreign repos which are not wikis and cannot do their own permission checking.

Serving files from non-local repos does not sound problematic, although it should probably be limited to repos where the backend is locally accessible (fetching the image via HTTP from the remote and streaming it through via img_auth.php is not a setup that should be encouraged).

About $publicWiki: In intranet wiki setups there often is a preliminary user authentication made by the webserver (e.g. in apache with mod_auth_kerb). So only users from the intranet domain may even access the wiki. But there still might be the need to differentiate between logged in users and anonymous ones (e.g. content in NS_MAIN is readable for *, but content from NS_HR is not. Therefore delivery of files with prefix "HR_" is only permitted to users that have group "HR"). When moving to OO implementation I'd just suggest to have this calculation in some function that can be overridden by an extension. No need to remove it completely or change the default behavior.

Allowing view access to some content but not all is unwise and explicitly not supported by core, so you might want to have a wider discussion about that. User::isEveryoneAllowed( 'read' ) and the like is used as a check for the wiki being public in many places; separating the concept of "this user can view some content" from "this user can view all content" sounds like a massive change.

Looking at T66214, does it make sense that I start working on ìmg_auth.php`? Or do you believe this will collide somehow?

You are probably right about $publicWiki, but I'd still like to make it at least overrideable by extensions.