Page MenuHomePhabricator

Remove "viewmywatchlist" user right from anonymous users
Closed, DeclinedPublic

Description

I don't see the point of setting
https://github.com/wikimedia/mediawiki-core/blob/c31fbf073e112526236d3afe6ef4dab3d5cd8e6f/includes/DefaultSettings.php#L4459

$wgGroupPermissions['*']['viewmywatchlist'] = true;

if we also have this:
https://github.com/wikimedia/mediawiki-core/blob/1ea16c08d70152062ba7feef62de8a69b3d08820/includes/specials/SpecialWatchlist.php#L41-L42

// Anons don't get a watchlist
$this->requireLogin( 'watchlistanontext' );

Shouldn't

$wgGroupPermissions['*']['viewmywatchlist'] = true;

be replaced by

$wgGroupPermissions['user']['viewmywatchlist'] = true;

?

Event Timeline

bzimport raised the priority of this task from to Low.Nov 22 2014, 3:31 AM
bzimport set Reference to bz69301.
bzimport added a subscriber: Unknown Object (MLST).

There seems to be other permissions in the same situation: editmywatchlist, editmyusercss, editmyuserjs, editmyoptions (T22151?).

True, these have no effect for anonymous users. But it does allow the possibility of using User::isEveryoneAllowed() in situations where that's advantageous, since it's very likely that outside of OAuth requests all relevant users really will have these permissions.

For context, here is where I noticed removing the permission from would be relevant:
https://en.wikipedia.org/w/index.php?title=MediaWiki_talk:Gadgets-definition#GeoNotice
The gadget in question modifies [[Special:Watchlist]] but it is intended only for users with access to that page (currently, logged in users), and the Gadgets extension is based on "rights" not "groups" (per T14211#159041).

The ['*'] implies these are catch-all rights.

But instead of dabbling with user rights, I'd like to be able to specify user groups when defining a gadget module, so we can target user classes.

He7d3r set Security to None.

Great discussion made so far @He7d3r. Is this still open for a fix? This is so i can do that ASAP. Thanks

Change 365509 had a related patch set uploaded (by Eugene233; owner: Eugene233):
[mediawiki/core@master] Remove "viewmywatchlist" user right from anonymous users

https://gerrit.wikimedia.org/r/365509

Change 365524 had a related patch set uploaded (by TerraCodes; owner: Eugene233):
[mediawiki/core@master] Remove "editmywatchlist" user right from anonymous users

https://gerrit.wikimedia.org/r/365524

@Eugene233 could you please take a look at the comments left on your patch?

Change 365509 abandoned by Eugene233:
Remove "viewmywatchlist" user right from anonymous users

Reason:
Abandoned in favor of https://gerrit.wikimedia.org/r/#/c/365524/3/includes/DefaultSettings.php

https://gerrit.wikimedia.org/r/365509

Change 581552 had a related patch set uploaded (by QEDK; owner: QEDK):
[mediawiki/core@master] Remove 'viewmywatchlist' and 'editmywatchlist' from anons

https://gerrit.wikimedia.org/r/581552

Krinkle added a subscriber: Krinkle.

MediaWiki grants most (or all) rights that are not restricted to *. Some features require a login, or maybe limited to certain namespaces, or may be unavailable if blocked. That is business logic specific to the feature.

If we want to change this, I would recommend that we not try to mimic the business logic by moving these around ad-hoc for individual cases, but rather decide more generally what we want to do here. Otherwise, we will have some features do it one way, and other features do it another way. Right now, they do it the same way. This has some benefits and some downsides. But at least it has benefits. If we change some of them but not others, we will loose those benefits as generic code will no longer be able to make any assumptions about what the rights mean.

Tagging CPT for input. Feel free to tag TechCom if you'd like more input or help in making a decision.

Change 365524 abandoned by Matěj Suchánek:
Remove "viewmywatchlist" user right from anonymous users

Reason:
See Ibe7ad1000c90a8b5dd40377a8bce196ef545ae8d

https://gerrit.wikimedia.org/r/365524

MediaWiki grants most (or all) rights that are not restricted to *. Some features require a login, or maybe limited to certain namespaces, or may be unavailable if blocked. That is business logic specific to the feature.

If we want to change this, I would recommend that we not try to mimic the business logic by moving these around ad-hoc for individual cases, but rather decide more generally what we want to do here. Otherwise, we will have some features do it one way, and other features do it another way. Right now, they do it the same way. This has some benefits and some downsides. But at least it has benefits. If we change some of them but not others, we will loose those benefits as generic code will no longer be able to make any assumptions about what the rights mean.

Tagging CPT for input. Feel free to tag TechCom if you'd like more input or help in making a decision.

In this specific case, and my input is only about this case since well, that's what my commit was for. This particular change is not business logic but a technical improbability, if something like this is to be implemented, then every watchlist would be accompanied with its key (i.e. anyone can create watchlists) and each user should be able to have multiple watchlists accessible via different keys, instead of the one-per-user facility we have now; because IPs cannot have an account-attached watchlist since not all IPs are static and ownership changes occur between IP addresses. That leaves the solution of ephemeral watchlists that IPs (non-persistent watchlists for non-logged-in users) can generate for a time period but I doubt its efficacy and need, hence making any alternatives to this change (essentially housekeeping) very unlikely.

If we're doing this, should we also change editmyprivateinfo and editmyoptions and viewmyprivateinfo too?

We're going to untag Platform Engineering in favor of the questions here being decided at TechCom. In general, we agree with what @Krinkle wrote in T71301#5988602.

tstarling added a subscriber: tstarling.

I agree with @Krinkle, I don't think rights should be used in this way. There were no objections to closing this in the TechCom meeting, but I note that we didn't have a quorum, so I'm closing the task on my own authority.

Change 581552 abandoned by QEDK:
Remove 'viewmywatchlist' and 'editmywatchlist' from anons

https://gerrit.wikimedia.org/r/581552