Merge pull request #790 from matrix-org/hs/quit-banned-users

Banned users should be quit from rooms

lib/bridge/MatrixHandler.js

@@ -960,7 +960,7 @@ MatrixHandler.prototype._onJoin = Promise.coroutine(function*(req, event, user)
                 try {
                     yield kickIntent.kick(
                         event.room_id, user.getId(),
-                    `Connection limit reached for ${room.server.domain}. Please try again later.`
+                    `IRC connection failure.`
                     );
                     self._incrementMetric(room.server.domain, "connection_failure_kicks");
                     break;

lib/irc/BridgedClient.js

@@ -47,6 +47,7 @@ function BridgedClient(server, ircClientConfig, matrixUser, isBot, eventBroker,
     this.inst = null;
     this.instCreationFailed = false;
     this.explicitDisconnect = false;
+    this.disconnectReason = null;
     this.chanList = [];
     this._connectDefer = promiseutil.defer();
     this._id = (Math.random() * 1e20).toString(36);
@@ -582,8 +583,6 @@ BridgedClient.prototype.getLastActionTs = function() {
     return this.lastActionTs;
 };
 BridgedClient.prototype._onConnectionCreated = function(connInst, nameInfo) {
-    var self = this;
-
     // listen for a connect event which is done when the TCP connection is
     // established and set ident info (this is different to the connect() callback
     // in node-irc which actually fires on a registered event..)
@@ -597,13 +596,18 @@ BridgedClient.prototype._onConnectionCreated = function(connInst, nameInfo) {
         }
     });
 
-    connInst.onDisconnect = function() {
-        self.emit("client-disconnected", self);
-        self._eventBroker.sendMetadata(self,
-            "Your connection to the IRC network '" + self.server.domain +
+    connInst.onDisconnect = (reason) => {
+        this.disconnectReason = reason;
+        if (reason === "banned") {
+            // If we've been banned, this is intentional.
+            this.explicitDisconnect = true;
+        }
+        this.emit("client-disconnected", this);
+        this._eventBroker.sendMetadata(this,
+            "Your connection to the IRC network '" + this.server.domain +
             "' has been lost. "
         );
-        clearTimeout(self._idleTimeout);
+        clearTimeout(this._idleTimeout);
     };
 
     this._eventBroker.addHooks(this, connInst);

lib/irc/ClientPool.js

@@ -8,6 +8,7 @@ const stats = require("../config/stats");
 const log = require("../logging").get("ClientPool");
 const Promise = require("bluebird");
 const QueuePool = require("../util/QueuePool");
+const BridgeRequest = require("../models/BridgeRequest");
 
 function ClientPool(ircBridge) {
     this._ircBridge = ircBridge;
@@ -330,6 +331,14 @@ ClientPool.prototype._onClientDisconnected = function(bridgedClient) {
     this._removeBridgedClient(bridgedClient);
     this._sendConnectionMetric(bridgedClient.server);
 
+    if (bridgedClient.disconnectReason === "banned") {
+        const req = new BridgeRequest(this._ircBridge._bridge.getRequestFactory().newRequest());
+        this._ircBridge.matrixHandler.quitUser(
+            req, bridgedClient.userId, [bridgedClient],
+            null, "User was banned from the network"
+        );
+    }
+
     if (bridgedClient.explicitDisconnect) {
         // don't reconnect users which explicitly disconnected e.g. client
         // cycling, idle timeouts, leaving rooms, etc.

lib/irc/ConnectionInstance.js

@@ -18,8 +18,6 @@ const PING_TIMEOUT_MS = 1000 * 60 * 10;
 // due to throttling.
 const THROTTLE_WAIT_MS = 20 * 1000;
 
-const BANNED_TIME_MS = 6 * 60 * 60 * 1000; // once every 6 hours
-
 // The rate at which to send pings to the IRCd if the client is being quiet for a while.
 // Whilst the IRCd *should* be sending pings to us to keep the connection alive, it appears
 // that sometimes they don't get around to it and end up ping timing us out.
@@ -132,7 +130,7 @@ ConnectionInstance.prototype.disconnect = function(reason) {
             // call this now it would potentially invoke this 3 times (once per
             // connection instance!). Each time would have dead=false as they are
             // separate objects.
-            this.onDisconnect();
+            this.onDisconnect(reason);
         }
         resolve();
     });
@@ -216,7 +214,7 @@ ConnectionInstance.prototype._listenForErrors = function() {
                     self.disconnect("throttled").catch(logError);
                     return;
                 }
-                var wasBanned = errText.indexOf("banned") !== -1;
+                const wasBanned = errText.includes("banned") || errText.includes("k-lined");
                 if (wasBanned) {
                     self.disconnect("banned").catch(logError);
                     return;
@@ -378,9 +376,10 @@ ConnectionInstance.create = Promise.coroutine(function*(server, opts, onCreatedC
             if (err.message === "banned") {
                 log.error(
                     `${opts.nick} is banned from ${server.domain}, ` +
-                    `retrying in ${BANNED_TIME_MS}ms`
+                    `throwing`
                 );
-                yield Promise.delay(BANNED_TIME_MS);
+                throw new Error("User is banned from the network.");
+                // If the user is banned, we should part them from any rooms.
             }
 
             if (err.message === "toomanyconns") {