Page MenuHomePhabricator

Update Phabricator BlockIpComplete hook to use "user.edit" Conduit API
Closed, ResolvedPublicBUG REPORT

Description

Original task title: "Port PhabBanBot code from calling "user.disable"/"user.enable" to "user.edit" Conduit API"

Per noise on https://phabricator.wikimedia.org/conduit/log/query/deprecated/ , https://phabricator.wikimedia.org/p/PhabBanBot/ should one day get ported from using the deprecated Phabricator Conduit API calls user.disable and user.enable to user.edit.

See also https://phabricator.wikimedia.org/conduit/method/user.edit/

Event Timeline

Aklapper triaged this task as Low priority.

The code has already been changed based on this commit from more than one year ago. Maybe it just needs to be deployed.

Heh, thanks. Yes, that is what I also just realized. :D

The code actually already uses user.edit in https://gitlab.wikimedia.org/toolforge-repos/phab-ban/-/blob/main/phabricator.py?ref_type=heads#L82-84 ...

Aklapper renamed this task from Port PhabBanBot code from calling "user.disable"/"user.enable" to "user.edit" Conduit API to Deploy more recent PhabBanBot code from repository into production.Jun 4 2024, 12:25 PM
Aklapper updated the task description. (Show Details)
Aklapper added a subscriber: bd808.

@bd808: Would you have an idea how/who to achieve that, maybe? :)

@bd808: Would you have an idea how/who to achieve that, maybe? :)

$ ssh login.toolforge.org
$ become phab-ban
$ cd www/python/src
$ git show --pretty 7956e6c4
commit 7956e6c47d276e45e57368f780d2e3d9e2d13989
Author: Stang <stang@toolforge.org>
Date:   Thu Sep 8 21:01:29 2022 +0000

    Replace deprecated "user.disable" with "user.edit"

diff --git a/phabricator.py b/phabricator.py
index 0f9d0f7..903fe9f 100644
--- a/phabricator.py
+++ b/phabricator.py
@@ -78,7 +78,15 @@ class Client(object):

     def user_disable(self, phid):
         """Disable the given user."""
-        r = self.post("user.disable", {"phids": [phid]})
+        r = self.post("user.edit",
+            {
+                "transactions": [{
+                    "type": "disabled",
+                    "value": True
+                }],
+                "objectIdentifier": phid
+            }
+        )
         return r

     def project_members(self, name):
$ git grep user.disable
app.py:        r = phab.user_disable(phabuser["phid"])
app.py:    """Log a user disable action on-wiki with code loaned from Stashbot."""
phabricator.py:    def user_disable(self, phid):

The tool is using the user.edit endpoint as far as I can tell. I will give it a hard stop/start cycle just to make sure that the code on disk is the code used by the tool.

Mentioned in SAL (#wikimedia-cloud) [2024-06-04T16:47:08Z] <wmbot~bd808@tools-bastion-12> Hard stop/start cycle for webservice to make sure that the code in $HOME/www/python/src is the code running for the tool. (T366587)

Mentioned in SAL (#wikimedia-cloud) [2024-06-04T17:01:22Z] <wmbot~bd808@tools-bastion-12> Restart to switch to a new API token (T366587)

I just realized that the Deprecated Calls Log shows also user.enable, which I don't see even in old versions of the code of the tool. Is it possible for the user to be shared by a different tool and that other tool being the one responsible of the deprecated calls?

I just realized that the Deprecated Calls Log shows also user.enable, which I don't see even in old versions of the code of the tool. Is it possible for the user to be shared by a different tool and that other tool being the one responsible of the deprecated calls?

I wondered about a second deployment somewhere using the bot's credentials, so I rotated the API token yesterday. On the Phabricator side this is a bot account so there is not any password to rotate as well. I guess we'll see if the pattern stops or not.

The Wikitech Phabricator ban integration uses user.disable. Which account does that use?

That would completely explain things. That also means I broke phab disable/enables when I rotated the API tokens. Ok, that gives me a couple of things to fix now.

bd808 renamed this task from Deploy more recent PhabBanBot code from repository into production to Update Phabricator BlockIpComplete hook to use "user.edit" Conduit API.Jun 5 2024, 9:54 PM
bd808 raised the priority of this task from Low to Medium.
bd808 edited projects, added wikitech.wikimedia.org; removed Tool-phab-ban.

Change #1039307 had a related patch set uploaded (by BryanDavis; author: Bryan Davis):

[operations/mediawiki-config@master] wikitech: Update Phabricator Conduit calls to disable/enable users

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

@PhabBanBot's profile even has (emphasis added) "Bot account for performing account bans via https://phab-ban.toolforge.org/ and Wikitech user blocks." on it. :facepalm:

bd808 changed the task status from Open to In Progress.Jun 5 2024, 10:54 PM
bd808 claimed this task.
bd808 moved this task from Backlog to Config on the wikitech.wikimedia.org board.
bd808 changed the subtype of this task from "Task" to "Bug Report".
bd808 moved this task from Unsorted to Migrate / Replace on the Technical-Debt board.

That also means I broke phab disable/enables when I rotated the API tokens.

I provisioned new Conduit API token for @PhabBanBot and @Eevans did the needful to update Puppet secrets with the new value.

Change #1039307 merged by jenkins-bot:

[operations/mediawiki-config@master] wikitech: Update Phabricator Conduit calls to disable/enable users

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

Mentioned in SAL (#wikimedia-operations) [2024-06-06T23:04:09Z] <bd808@deploy1002> Started scap: Backport for [[gerrit:1039307|wikitech: Update Phabricator Conduit calls to disable/enable users (T366587)]]

Mentioned in SAL (#wikimedia-operations) [2024-06-06T23:06:31Z] <bd808@deploy1002> bd808: Backport for [[gerrit:1039307|wikitech: Update Phabricator Conduit calls to disable/enable users (T366587)]] synced to the testservers (https://wikitech.wikimedia.org/wiki/Mwdebug)

Mentioned in SAL (#wikimedia-operations) [2024-06-06T23:16:10Z] <bd808@deploy1002> Finished scap: Backport for [[gerrit:1039307|wikitech: Update Phabricator Conduit calls to disable/enable users (T366587)]] (duration: 12m 01s)