Last modified: 2014-01-03 16:17:49 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 T23912, the corresponding Phabricator task for complete and up-to-date bug report information.
Bug 21912 - Watchlist token auto-generator too zealous
Watchlist token auto-generator too zealous
Status: RESOLVED FIXED
Product: MediaWiki
Classification: Unclassified
Watchlist (Other open bugs)
unspecified
All All
: Low enhancement (vote)
: ---
Assigned To: Bartosz Dziewoński
http://en.wikipedia.org/w/index.php?o...
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2009-12-20 20:47 UTC by MZMcBride
Modified: 2014-01-03 16:17 UTC (History)
6 users (show)

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


Attachments

Description MZMcBride 2009-12-20 20:47:33 UTC
The watchlist token now has an auto-generator, but it's become impossible to clear the field (presumably to disable the feature entirely). This is confusing behavior for the end user, as seen in this thread: http://en.wikipedia.org/w/index.php?oldid=332906857#Watchlist_token

It should be changed so that a blank watchlist token field stays blank.
Comment 1 Umherirrender 2009-12-22 20:06:02 UTC
INVALID per bug 471 comment 62
Comment 2 Alex Z. 2009-12-22 20:39:23 UTC
Re-opening, just because it was designed that way doesn't mean its the best design. The comments there don't address the concerns here and on the linked discussion, that is, with the current design its impossible to disable it. I don't really have an opinion myself, but if its going to be closed without any change, the reason for closing should at least address the reason for opening.
Comment 3 Alexandre Emsenhuber [IAlex] 2009-12-26 21:40:51 UTC
I'm not able to reproduce the bug both on my local wiki and on Wikipedia.
Comment 4 Priyanka Dhanda 2009-12-29 01:14:01 UTC
werdna, Simmertical,

I think yall discussed this with Brion extensively. Any comments on MZMcBride's inability to clear the token entirely? Seems like it gets auto generated everytime a user visits the Special:Watchlist page. 

-p
Comment 5 Aryeh Gregor (not reading bugmail, please e-mail directly) 2009-12-29 01:24:31 UTC
Maybe we should autogenerate only on user creation, and have a maintenance script add the tokens to existing users?  I don't see a better way.  If not that, we'd have to distinguish between "default, so autogenerate when convenient" and "user has explicitly disabled the feature".  I guess we could use a magic token that means the latter, so if the user saves with the preference blank we secretly substitute it or something like that.  But autogenerating on creation sounds much saner to me.
Comment 6 Andrew Garrett 2009-12-29 10:32:38 UTC
I think we should use a proper new form field type instead of a random textbox for this. We could then have a checkbox ("enable watchlist token"), which when disabled adds 'disabled' to that field (the magic-value solution). When the checkbox is enabled, it should have a token and then allow users to regenerate the token.

There shouldn't be any need to allow users to enter their own custom values for watchlist tokens, it's just a token.
Comment 7 Aryeh Gregor (not reading bugmail, please e-mail directly) 2009-12-29 20:28:25 UTC
On reflection, agreed.  I ended up doing the token thing as a quick hack to get the feature working with as little work as possible.  We should just have a checkbox to enable/disable, that's it.

The question is, what happens if the user wants to change the token because someone has stolen it?  If we ignore this case, it all becomes very easy.  Just hardcode the token to some hash of user_id + secret fixed global salt.  But then you can't change the token for an individual user short of changing their user_id, so if anyone ever finds out what the token is, they can never safely enable watchlist feeds again.  This is what prompted me to go with a manually-alterable token approach.

Alternatively, what if the hash depending on user_password instead of user_id?  Then all they'd have to do is change their password.  This means your RSS feed breaks if you change your password, but maybe that's expected?  I imagine most users don't change their password very often either.

So here are a few possibilities:

1) Have a checkbox in preferences to disable watchlist feeds, and a button to generate a new token.  Tokens are autogenerated and stored per-user, but not directly exposed in prefs.

2) Have a checkbox in preferences to disable watchlist feeds, and just use a hash of user_id + secret global salt.  If a user's hash gets revealed, they can decide to either stop using watchlist feeds or let everyone see what's on their watchlist.  (If the global salt becomes public somehow, it would have to be regenerated, so maybe random per-user hashes would still make more sense here, but that's an implementation detail.)

3) Have a checkbox in preferences to disable watchlist feeds, and use a hash of user_password + secret global salt.  If a user changes his password, he has to update the link in his feed reader too.  (If the global salt becomes public somehow, users' passwords would be vulnerable to much easier brute-forcing, so a per-user hash might be a good idea here too.  In that case this is just (1), with changing your password instead of a separate button.)

I guess I'm inclined to say (1) is best here, although it will require a bit more work than (2) or (3).
Comment 8 Andrew Garrett 2009-12-29 23:43:12 UTC
I guess I was unclear, my suggestion was (1) :)
Comment 9 Aryeh Gregor (not reading bugmail, please e-mail directly) 2009-12-30 00:11:36 UTC
No, you were clear, I was just pondering other possibilities.  It would probably be easier for you to make a new preference type than me, so if you want to do it, go ahead.
Comment 10 Andrew Garrett 2009-12-30 00:16:55 UTC
Reassigning to me.
Comment 11 Mark A. Hershberger 2011-01-27 00:16:40 UTC
While you're looking at this, I can't help but cry a little as I think of all the entropy that must be wasted because a new value is generated on each page load.

For example, the text for 1.17 says "Filling in this field with a secret key will generate an RSS feed for your watchlist. Anyone who knows the key in this field will be able to read your watchlist, so choose a secure value. Here's a randomly-generated value you can use:" (and a new MD5 each time).

Meanwhile the textbox already contains a token from (I assume) the time the user was created.
Comment 12 MZMcBride 2013-06-12 23:39:43 UTC
Assigning this to MatmaRex. This bug may no longer be valid.
Comment 13 Bartosz Dziewoński 2013-06-14 18:54:01 UTC
I read and pondered. Here's what I came up with: change I0bdd2469.

(In reply to comment #7)
> 1) Have a checkbox in preferences to disable watchlist feeds, and a button to
> generate a new token.  Tokens are autogenerated and stored per-user, but not
> directly exposed in prefs.

My change redoes the interface:
* prevents the user from changing the token directly
* autogenerates it whenever used
* adds a form where they can reset the token to a randomly generated value
* keeps using the prefs mechanism and allows for changing the token via API

There is no way to disable the functionality, but it would be trivial to implement if deemed useful (literally maybe 10 lines of code changed).
Comment 14 Gerrit Notification Bot 2013-07-24 20:31:05 UTC
Change 64565 merged by jenkins-bot:
Refactor watchlist token handling

https://gerrit.wikimedia.org/r/64565
Comment 15 Bartosz Dziewoński 2013-07-24 20:39:08 UTC
Merged. Rejoice!

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


Navigation
Links