Page MenuHomePhabricator

bad wiki Username (WMF) URL in "Confirm MediaWiki Account Link" and "Linked Accounts and Authentication"
Closed, InvalidPublic

Description

From https://www.mediawiki.org/wiki/Phabricator/Help#Creating_your_account I followed the link "Connect your Wikimedia SUL and LDAP accounts to a single Phabricator username!" It worked great and called up OAuth for confirmation.

But the Phabricator "Confirm MediaWiki Account Link" form for the second step has a bogus link. It shows

Confirm the link with this MediaWiki account. This account will be able to log in to your Phabricator account.
"mediawiki" User
MediaWiki Account · 14576418
https://www.mediawiki.org:/w/index.php?title=User%3ASPage%2B%2528WMF%2529

That URL is garbled and results in 'Bad title'

  1. The space between "SPage" and "(WMF)" is represented as a + (OK), but then is getting URL-encoded as %2B, which is bad.
  2. The parentheses are double-URL-encoded -- '(' represented as %28 is OK, but then translating the '%' to produce %2528 is garbage.
  3. index.php?title isn't necessary for a straight title with no query parameters.

https://www.mediawiki.org/wiki/User:SPage_(WMF) is what you want and all you need here.

I confirmed anway and now I see the same bad URL on the https://phabricator.wikimedia.org/settings/panel/external/ page under "MediaWiki OAuth1 Account"

Event Timeline

Spage created this task.Oct 8 2014, 1:51 AM
Spage raised the priority of this task from to 0.
Spage updated the task description. (Show Details)
Spage added a project: Phabricator.
Spage changed Security from none to None.
Spage added a subscriber: Spage.

Qgil's MediaWiki URL in the M2 mock, https://www.mediawiki.org/w/index.php?title=User%3AQgil-WMF, works: it doesn't have a space or parentheses.

luser assigned this task to mmodell.Oct 8 2014, 4:31 AM
Qgil added a subscriber: Qgil.Oct 8 2014, 9:07 AM
He7d3r added a subscriber: He7d3r.Oct 8 2014, 10:35 AM
Qgil triaged this task as Normal priority.Oct 8 2014, 7:29 PM
Qgil edited projects, added Bugzilla-Migration; removed Phabricator.
Qgil moved this task from Backlog to Doing on the Bugzilla-Migration board.

This is a lot more complex than it looks. Because the wikimedia oauth plugin needs to work with other mediawiki installs, and those can have different url schemes, we can't hard-code it to assume the conventions used on mediawiki.org, specifically, clean urls can't be assumed to be on so we can't leave out the index.php?title= part of the url or assume that /wiki will work as a path prefix. Additionally, upstream rejected the code when I had it customized specifically for our use case. As upstream acceptance has been considered extremely important, I made changes in response to D9202 (upstream).

This was done with the advice of other wikimedia developers:

In D9202#83860, @csteipp wrote:
In D9202#48, @WikiChad wrote:
In D9202#47, @20after4 wrote:

The /w/ and /wiki/ urls are the default and recommended mediawiki configuration:

https://www.mediawiki.org/wiki/Manual:Short_URL

One issue is that the plugin only works with mediawiki.org when it uses /w/ and /wiki/ in appropriate places. I'm not sure how to make that part configurable without making it sorta confusing...
@csteipp can you offer any suggestions here?

I think it's pretty easy actually. Don't use /wiki/ and assume the wiki never has short URLs (which they very well might not). We don't have to use them. Then include /w/ in the base part of the URI you configure and you're set.

We can just use whatever url the user inputs (maybe suggest they use the script path, /w/, etc), but then mobile redirects don't work.
We could add an optional parameter in the setup, which asks for the optional url to redirect the user's browser to, and suggest a /wiki/Special:OAuth url. If the user fills it out, we use that for the /authorize url. Otherwise we just use the one url they gave us?

The double url-encoding might be fixable, however, the short url is not a simple thing to implement.

mmodell lowered the priority of this task from Normal to Low.Oct 10 2014, 3:55 AM
Qgil added a comment.Oct 10 2014, 6:33 AM

I think the fixes needed sorted by priority are:

  1. The URL to the user page must be functional in all cases, regardless of the characters involved.
  2. The visible string should be the username correctly formatted, hiding the URL itself i.e. "S Page (WMF)"
  3. This needs to be implemented in all the places where we are showing this link
    • Registration dialog (pending)
    • User profile (fixed)
    • User preferences (pending)
    • Anywhere else?
  4. The "long URL" shouldn't show unnecessary url-encoding.

How much of this can we reasonably include before the Bugzilla migration?

Personally I don't think it is relevant whether these encodings exist in the background as long as the URLs work and the ugly encodings are not exposed in the UI.

Providing "short URL" format is not possible today and it would require significant changes in the extension if we want to upstream them, something that we are not going to pursue now, if ever. If you are interested in showing short URLs, then please create a new task, but as said we will not look at it anytime soon.

Se4598 added a subscriber: Se4598.Oct 15 2014, 10:39 PM
Ireas added a subscriber: Ireas.Oct 15 2014, 11:19 PM
Qgil moved this task from To Triage to Need discussion on the Phabricator board.Dec 22 2014, 9:57 AM
mmodell closed this task as Invalid.Jun 25 2015, 1:01 AM

I think the glaring issues with broken encodings are resolved. The rest of this task is wontfix as there is no hook to override the way the link is displayed in the UI, it's a phabricator core ui and only the url it's self is provided by our extension.

Restricted Application added a subscriber: scfc. · View Herald TranscriptJun 25 2015, 1:01 AM