Page MenuHomePhabricator

Adapting Gadget-BugStatusUpdate.js to Phabricator
Closed, ResolvedPublic

Description

Gadget-BugStatusUpdate.js needs to be adapted to work with Phabricator. Currently, the gadget queries Bugzilla's API dynamically to show the wiki user the updated status. The goal is to do the same with Phabricator's API, Conduit.

There is already some discussion at http://en.wikipedia.org/wiki/MediaWiki_talk:Gadget-BugStatusUpdate.js#Adapting_this_gadget_to_Phabricator

It looks like maniphest.info should provide the needed information.

However, from talking to people on the phabricator IRC channel, it does not seem that Conduit supports JSONP or CORS. However, if not, we can look into alternatives, like a single-purpose proxy (e.g. a JSONP proxy on Wikimedia Labs) or configuring the server to support CORS.

Event Timeline

Qgil created this task.Oct 3 2014, 11:51 AM
Qgil raised the priority of this task from to Medium.
Qgil updated the task description. (Show Details)
Qgil added a project: Bugzilla-Migration.
Qgil changed Security from none to None.
Qgil added subscribers: Aklapper, Qgil, Quiddity, mmodell.
Qgil added a comment.Oct 3 2014, 11:54 AM

Let me paste this:

I've cast a glance at how to do the same thing with phabricator as with bugzilla. First, the API (conduit) “auto”-documentation is very light; second, we'll need an additional step to get a CSRF token, even for read-only requests (I still wonder why); third, I haven't found yet how to query information for several tasks at the same time (I certainly wouldn't use the gadget if it was doing one request per task) — but again, the documentation is subpar.
Example, request for bugzilla was this one, now it'll be like this one (except that it only gets information for ''one'' task and that some other query — which one and why, I've no idea — is required first to get the CSRF token).
I dunno if we had to patch bugzilla in the past, but I've the feeling that we'll have to contribute to phabricator in the future…
Best regards — Arkanosis...

chasemp added a subscriber: chasemp.Oct 3 2014, 1:04 PM

i'm not sure what api docs you guys are looking at, but maniphest.query has the ability to query multiple id's:

"maniphest.query": {
    "params": {
        "authorPHIDs": "optional list<phid>",
        "ccPHIDs": "optional list<phid>",
        "fullText": "optional string",
        "ids": "optional list<uint>",
        "limit": "optional int",
        "offset": "optional int",
        "order": "optional string-constant<\"order-priority\", \"order-created\", \"order-modified\">",
        "ownerPHIDs": "optional list<phid>",
        "phids": "optional list<phid>",
        "projectPHIDs": "optional list<phid>",
        "status": "optional string-constant<\"status-any\", \"status-open\", \"status-closed\", \"status-resolved\", \"status-wontfix\", \"status-invalid\", \"status-spite\", \"status-duplicate\">"
    }
},
In T539#8038, @chasemp wrote:

maniphest.query has the ability to query multiple id's:

Yes, thank you! I wonder why there's maniphest.info then… It does the same thing with less flexibility.

So the query is something like “https://phabricator.wikimedia.org/api/maniphest.query?__csrf__=TOKEN&params[ids]=[%222%22%2C+%223%22]&output=json”.

Any idea on whether it'd be possible to perform this query without having to get a CSRF token first or not? Unless I'm mistaken, that's of no use for read-only requests on public issues.

Thanks again,

AFAIK conduit cannot be used w/o a token, I would be opposed to allowing it for security / practical reasons. My understanding is bugzilla is moving more towards the "get a token to do things programmatic" view of he world as well. There are just too many abuse scenario's where it's a nightmare not to be able to tie odd / bot like behavior back to something that can be blocked or at least the user can be contacted.

That being said! You can request a bot account w/ its own API token independent of yours and I think that's totally reasonable.

two things:

  • there are actual non-priv bot accounts in phab, I don't just mean a person who isn't a person
  • echo {} | arc call-conduit conduit.query

^should output the api specs

I understand the need of a token for state-changing requests, not for read-only requests. Here, we want the end-user (who certainly wont always have an account, let alone a bot account) to get publicly available information about tasks (summary, state…).

MediaWiki's API requires CSRF tokens for action=move, block, delete… not for action=query.

Thanks for your help!

we want the end-user […] to get publicly available information about tasks (summary, state…)

Err… it comes to my mind that not everybody is familiar with what Gadget-BugStatusUpdate.js does: it displays the up-to-date summary of a task and its current state (fixed, wontfix…) on mediawiki sites where bugzilla (for now) is linked to.

thanks for noting that!

For the tokens on read/edit, abusing read is just as easy if not easier than inadvertantly abusing programmatic edit, so while it may seem unnecessary to some it seems like not so high of a barrier as to be controversial? Since tokens are free, it's not an access issue but an accountability one. If you want to hit phab w/ a bot it comes w/ some responsibilities as well as power.

In T539#8120, @chasemp wrote:

If you want to hit phab w/ a bot it comes w/ some responsibilities as well as power.

There's no bot involved here. Only individuals, most of whom are non-technical, know nothing about phabricator and only want to know if a bug has been solved or not when someone talks about it on some mediawiki instance.

In T539#8120, @chasemp wrote:

abusing read is just as easy

What kind of “abuse” are you refering to? I'm sorry for asking, but that's a genuine question. The only kinds of reading abuses that I can think of are reading of private information (which doesn't apply there since it's all about public information) and flood / abuse of bandwith / server resources / denial of service (which CSRF tokens do not address). Do I miss something?

abuse in this case is just a generic term for undesirable behavior. The type of thing I mean is just that, using more resources than is reasonable or expected or even possible for things to function for all. tokens are not meant to solve these problems, only tie them to a specific user. for responsibility I mean for the person maintaining the automated thing itself that grabs the information, not really the people who consume whatever the output of that bot's behavior is.

I wonder if we're really talking about the same tokens (CSRF vs auth)… But maybe phabricator uses auth token as CSRF tokens. Anyway, the result is the same:

Does it mean that anonymous users definitely can't read phabricator in JSON ? (I know that technically, for now, the answer is yes; but is this what we really want?) Why would it be ok for HTML and not for JSON?

For me that's a missing feature.

Edokter added a subscriber: Edokter.Oct 4 2014, 1:02 PM
Qgil added a comment.Oct 10 2014, 9:34 PM

For me that's a missing feature.

This feature should probably be discussed upstream, though.

Back to the gadget, @Arkanosis, can we assign this task to you? It would be great if you could drive it forward.

I will remove it from the Bugzilla-Migration project in order to remove some pressure. The {{tracked}} template works without the gadget, even is manually only. All the better if the gadget is enabled before the migration, of course.

In T539#10638, @Qgil wrote:

For me that's a missing feature.

This feature should probably be discussed upstream, though.

I agree.

Back to the gadget, @Arkanosis, can we assign this task to you? It would be great if you could drive it forward.

Sure! This will take more time than juste patching the script, though.

There seems to be an additional problem which hasn't been mentioned yet: While Bugzilla provides JSONP, there seems to be no way in Phabricator to request JSONP instead of raw JSON, at least appending &callback=foo doesn't work. This is somewhat consistent with the fact that a token is required, but makes querying from a different domain even more complicated. Either Phabricator must be configured to set correct CORS headers for queries from WMF wikis, or (perhaps simpler) somebody must set up a proxy, that takes care about tokens etc. and outputs JSONP.

Uh, I wasn't even aware that Bugzilla offers JSONP. Not sure either what's the best way forward.
If too complicated we might end up with not being able to show an updated task status on-wiki. :-/

Some information was removed when this was split from T177. I've added it back to the description.

Uh, I wasn't even aware that Bugzilla offers JSONP. Not sure either what's the best way forward.
If too complicated we might end up with not being able to show an updated task status on-wiki. :-/

It's possible (and not complicated compared to the rest of the migration). If nothing else, we can set up a JSONP proxy on Wikimedia Labs.

@Arkanosis are you still working on this? If not, I may take it.

In T539#22469, @Mattflaschen wrote:

@Arkanosis are you still working on this? If not, I may take it.

It's not at the top of my TODO list, right now, so feel free to take it if you want to.

Thank you!

I'm going to try the approach of a JSONP proxy on Wikimedia Labs.

Qgil moved this task from To Triage to Ready to Go on the Phabricator board.Jan 15 2015, 7:36 AM
Aklapper lowered the priority of this task from Medium to Low.Jan 20 2015, 11:37 PM

I'm going to try the approach of a JSONP proxy on Wikimedia Labs.

@Mattflaschen: If you found some time or made any progress, feel free to share it here. :)

I haven't done this yet, but I would like to keep it assigned for a little longer.

He7d3r awarded a token.Feb 6 2015, 5:08 PM
He7d3r added a project: JavaScript.

This is mostly done. I'll be able to finish and test the server-side part, then port the gadget, once the bot is created.

Se4598 added a subscriber: Se4598.Feb 10 2015, 1:33 AM
gerritbot added a subscriber: gerritbot.

Change 189670 had a related patch set uploaded (by Mattflaschen):
Initial commit

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

Patch-For-Review

Change 189670 merged by Mattflaschen:
Initial commit

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

Would you mind publishing it on Meta-Wiki or mediawiki.org to keep it central?

@Ricordisamoa My staff account doesn't have admin rights on either, so I don't really have the ability to control where it's deployed first.

Hopefully, the new version will be accepted at all wikis that use this gadget. I would post a note on the talk page on MediaWiki.org or Meta, but surprisingly it doesn't seem to be a gadget in either wiki (MW.org, Meta) yet.

Given that, I'd like to keep the initial discussion (any changes required before it becomes a gadget) on English Wikipedia. After that, of course it can be copied anywhere, including any central gadget galleries such as here.

BTW, you may want to look at my list of desired improvements for the template. If those are addressed, they should also be ported, and then the gadget can be cleaned up a little.

In T539#1027130, @Mattflaschen wrote:

Given that, I'd like to keep the initial discussion (any changes required before it becomes a gadget) on English Wikipedia. After that, of course it can be copied anywhere, including any central gadget galleries such as here.

The point is avoid having to maintain multiple copies on multiple wikis.

Qgil added a comment.Feb 10 2015, 10:42 AM

fwiw I have advocating on defaulting to mediawiki.org as central place for tech.

Mattflaschen-WMF closed this task as Resolved.Feb 10 2015, 8:28 PM

The point is avoid having to maintain multiple copies on multiple wikis.

Yeah, that's reasonable. I've made a canonical version at https://www.mediawiki.org/wiki/Extension:Gadgets/Scripts/BugStatusUpdate.js , linked from https://www.mediawiki.org/wiki/Extension:Gadgets/Scripts#List_of_gadget_scripts .

The original version is now deployed on English Wikipedia (the only difference in the canonical version is the header).

Qgil awarded a token.Feb 10 2015, 8:50 PM
Ricordisamoa added a comment.EditedFeb 11 2015, 5:51 AM
In T539#1028871, @Mattflaschen wrote:

It should be in the MediaWiki namespace, to avoid edits by untrusted people, and enwiki should load it dynamically.

...and enwiki should load it dynamically.

Please define 'dynamically'. If you mean load it raw from MediaWiki, I think that is a bad idea. How long before we have global gadgets?

...and enwiki should load it dynamically.

Please define 'dynamically'. If you mean load it raw from MediaWiki, I think that is a bad idea. How long before we have global gadgets?

Why is that a bad idea? Long enough.

He7d3r added a subscriber: He7d3r.Feb 11 2015, 11:06 AM

It should be in the MediaWiki namespace, to avoid edits by untrusted people,

+1, for many other reasons too (see e.g. T86904: Use the appropriate content models for CentralNotice HTML, scripts and CSS).

and enwiki should load it dynamically.

+1

Please define 'dynamically'. If you mean load it raw from MediaWiki, I think that is a bad idea. How long before we have global gadgets?

No one wants to keep syncing forks manually forever, and I'm afraid the epic T22153: Implement global gadgets (WMF-wide) will probably complete 6 years before it gets implemented, since it is still low priority. So, the only option not to waste human resources on these copies is to mw.loader.load it from a central place.

...and enwiki should load it dynamically.

Please define 'dynamically'. If you mean load it raw from MediaWiki, I think that is a bad idea. How long before we have global gadgets?

Why is that a bad idea? Long enough.

An additional HTTP request to a non-minified script that doesn't hit the Varnish cache but a different domain, I guess… Inlining gadgets locally allows to benefit from ResourceLoader. And requires updates, that's right (but I think this is worth the hassle).

There might be better ways than a plain `action=raw``` that I'm not aware of, though.

...and enwiki should load it dynamically.

Please define 'dynamically'. If you mean load it raw from MediaWiki, I think that is a bad idea. How long before we have global gadgets?

Loading scripts from other wikis is the standard practice for many of them, including HotCat. It's perhaps the closest alternative to global gadgets we have today.

...and enwiki should load it dynamically.

Please define 'dynamically'. If you mean load it raw from MediaWiki, I think that is a bad idea. How long before we have global gadgets?

Why is that a bad idea? Long enough.

An additional HTTP request to a non-minified script that doesn't hit the Varnish cache but a different domain, I guess… Inlining gadgets locally allows to benefit from ResourceLoader. And requires updates, that's right (but I think this is worth the hassle).

There might be better ways than a plain `action=raw``` that I'm not aware of, though.

If it were a ResourceLoader-enabled gadget, it should be possible to include it via load.php instead of index.php.

In T539#1028871, @Mattflaschen wrote:

It should be in the MediaWiki namespace, to avoid edits by untrusted people, and enwiki should load it dynamically.

I don't support actually loading it cross-wiki, for the same reasons @Edokter and @Arkanosis said.

It's not how standard gadgets work, and introduces an additional separate cross-domain load for every gadget (thereby defeating the bundling benefits of ResourceLoader). RL is normally very good about bundling gadgets. In fact, it even bundles them with other modules, avoiding even a "gadgets-only" request.

I have no objections to moving the MediaWiki.org canonical version to the MediaWiki namespace. I don't currently have the rights to do so, so someone else will have to, and then I'll have to ping people to update it at least until I get admin rights for my staff account on MediaWiki.org.