Allow Phabricator IRC bot to connect to channels that require password
Closed, WontfixPublic
Actions

Assigned To

None

Authored By

	• ltt
	Apr 10 2015, 1:54 AM

Description

So here's the deal:

I've been working on getting Phabricator to run on our server.
We got it started pretty easily (thanks guys!) and we wanted to setup an IRC bot,
since it is a communication channel we use a lot in our project.

A colleague of mine set out to get the bot working, but he stumbled upon a problem:
our IRC channel requires a password to be joined to. Since we could not find anything on the documentation that talked about
joining channels that required password, we thought the default behavior would be similar to weechat's config: the name of the
channel followed by a space and then the channel password. You can see what I'm are talking about below:

{
  "server" : "irc.freenode.net",
  "port" : 6667,
  "nick" : "phabot",
  "join" : [
   "<channel> <password>"
  ],
  "handlers" : [
    "PhabricatorBotWhatsNewHandler"
  ],

  "conduit.uri" : null,
  "conduit.user" : null,
  "conduit.cert" : null,

  "macro.size" : 48,
  "macro.aspect" : 0.66,

  "notification.channels" : ["<channel>"]
}

This worked for simply joining our channel.

The problem is the way messages are sent to a channel: it uses the channel name it extracts from that 'join' list.
So in our case it considers "<channel> <password>" the actual name of the channel.
This caused us to get Phabot spitting out the channel's password at the beginning of every message it sent.

So we identified the code used by the bot to send messages in src/infrastructure/daemon/bot/handler/PhabricatorBotFeedNotificationHandler.php. We altered the way the $channel_name variable receives its value
so we trim off the password portion and we got it working for our use case.

We believe that joining channels with password is a missing feature, and we'd like to help you guys implement it (if you agree, of course).
What we have right now is probably more of a hack than a proper solution )-:

What do you guys think?

PS: I'm not sure if I selected the correct project when creating this Task. Please feel free to correct me.

Revisions and Commits

rP Phabricator
	D17756	rP5c1e4488deda Remove all "Phabricator Bot" code

Related Objects

Mentioned In: T7829: PhabricatorBotFeedNotificationHandler is completely broken and unusable

Event Timeline

• ltt created this task.Apr 10 2015, 1:54 AM

• ltt renamed this task from Allow Phabricator IRC bot to connect to channels that use password to Allow Phabricator IRC bot to connect to channels that requirepassword.

• ltt raised the priority of this task from to Needs Triage.

• ltt updated the task description. (Show Details)

• ltt added a project: Phabot.

• ltt added a subscriber: • ltt.

• ltt renamed this task from Allow Phabricator IRC bot to connect to channels that requirepassword to Allow Phabricator IRC bot to connect to channels that require password.Apr 10 2015, 12:02 PM

cburroughs added a subscriber: cburroughs.Apr 10 2015, 12:19 PM

eadler added a subscriber: eadler.Apr 27 2015, 10:56 PM

FWIW this allows key channels to work, although it is obviously not ideal:

diff --git a/src/infrastructure/daemon/bot/handler/PhabricatorBotFeedNotificationHandler.php b/src/infrastructure/daemon/bot/handler/PhabricatorBotFeedNotificationHandler.php
index 21eadf1..d4631d5 100644
--- a/src/infrastructure/daemon/bot/handler/PhabricatorBotFeedNotificationHandler.php
+++ b/src/infrastructure/daemon/bot/handler/PhabricatorBotFeedNotificationHandler.php
@@ -163,6 +163,7 @@ final class PhabricatorBotFeedNotificationHandler

         $channels = $this->getConfig('join');
         foreach ($channels as $channel_name) {
+          $trueName = explode(" ", $trueName);

           $channel = id(new PhabricatorBotChannel())
             ->setName($channel_name);
@@ -170,7 +171,7 @@ final class PhabricatorBotFeedNotificationHandler
           $this->writeMessage(
             id(new PhabricatorBotMessage())
             ->setCommand('MESSAGE')
-            ->setTarget($channel)
+            ->setTarget($trueName[0])
             ->setBody($message));
         }
       }