Replace makeExtBanmask with makeExtBanmasks

Now that we can return both account extbans and regular masks,
it makes sense to ban both.

Otherwise, adding 'account' to supybot.protocols.irc.banmask
means we banned only the account instead of the hostmask,
which arguably makes the ban weaker (/NS LOGOUT to evade)

plugins/AutoMode/plugin.py

@@ -163,9 +163,9 @@ class AutoMode(callbacks.Plugin):
                         # We're not in the channel anymore.
                         pass
                 schedule.addEvent(unban, time.time()+period)
-            banmask = conf.supybot.protocols.irc.banmask.makeExtBanmask(
+            banmasks = conf.supybot.protocols.irc.banmask.makeExtBanmasks(
                 msg.prefix, channel=channel, network=irc.network)
-            irc.queueMsg(ircmsgs.ban(channel, banmask))
+            irc.queueMsg(ircmsgs.bans(channel, banmasks))
             irc.queueMsg(ircmsgs.kick(channel, msg.nick))
 
         try:

plugins/Channel/plugin.py

@@ -368,7 +368,7 @@ class Channel(callbacks.Plugin):
             try:
                 bannedHostmask = irc.state.nickToHostmask(target)
                 banmaskstyle = conf.supybot.protocols.irc.banmask
-                banmask = banmaskstyle.makeExtBanmask(
+                banmasks = banmaskstyle.makeExtBanmasks(
                     bannedHostmask, [o[0] for o in optlist],
                     channel=channel, network=irc.network)
             except KeyError:
@@ -376,12 +376,14 @@ class Channel(callbacks.Plugin):
                         target.startswith('$'):
                     # Select the last part, or the whole target:
                     bannedNick = target.split(':')[-1]
-                    banmask = bannedHostmask = target
+                    bannedHostmask = target
+                    banmasks = [bannedHostmask]
                 else:
                     irc.error(format(_('I haven\'t seen %s.'), bannedNick), Raise=True)
         else:
             bannedNick = ircutils.nickFromHostmask(target)
-            banmask = bannedHostmask = target
+            bannedHostmask = target
+            banmasks = [bannedHostmask]
         if not irc.isNick(bannedNick):
             self.log.warning('%q tried to kban a non nick: %q',
                              msg.prefix, bannedNick)
@@ -398,6 +400,8 @@ class Channel(callbacks.Plugin):
             reason = msg.nick
         capability = ircdb.makeChannelCapability(channel, 'op')
         # Check (again) that they're not trying to make us kickban ourself.
+        for banmask in banmasks:
+            # TODO: check account ban too
             if ircutils.hostmaskPatternEqual(banmask, irc.prefix):
                 if ircutils.hostmaskPatternEqual(bannedHostmask, irc.prefix):
                     self.log.warning('%q tried to make me kban myself.',msg.prefix)
@@ -406,21 +410,28 @@ class Channel(callbacks.Plugin):
                 else:
                     self.log.warning('Using exact hostmask since banmask would '
                                      'ban myself.')
-                banmask = bannedHostmask
+                    banmasks = [bannedHostmask]
         # Now, let's actually get to it.  Check to make sure they have
         # #channel,op and the bannee doesn't have #channel,op; or that the
         # bannee and the banner are both the same person.
         def doBan():
             if irc.state.channels[channel].isOp(bannedNick):
                 irc.queueMsg(ircmsgs.deop(channel, bannedNick))
-            irc.queueMsg(ircmsgs.ban(channel, banmask))
+            irc.queueMsg(ircmsgs.bans(channel, banmasks))
             if kick:
                 irc.queueMsg(ircmsgs.kick(channel, bannedNick, reason))
             if expiry > 0:
                 def f():
-                    if channel in irc.state.channels and \
+                    if channel not in irc.state.channels:
-                       banmask in irc.state.channels[channel].bans:
+                        return
-                        irc.queueMsg(ircmsgs.unban(channel, banmask))
+                    remaining_banmasks = [
+                        banmask
+                        for banmask in banmasks
+                        if banmask in irc.state.channels[channel].bans
+                    ]
+                    if remaining_banmasks:
+                        irc.queueMsg(ircmsgs.unbans(
+                            channel, remaining_banmasks))
                 schedule.addEvent(f, expiry)
         if bannedNick == msg.nick:
             doBan()
@@ -591,7 +602,7 @@ class Channel(callbacks.Plugin):
         hostmask = wrap(hostmask, ['op', ('haveHalfop+', _('ban someone')), 'text'])
 
         @internationalizeDocstring
-        def add(self, irc, msg, args, channel, banmask, expires):
+        def add(self, irc, msg, args, channel, banmasks, expires):
             """[<channel>] <nick|hostmask> [<expires>]
 
             If you have the #channel,op capability, this will effect a
@@ -605,11 +616,14 @@ class Channel(callbacks.Plugin):
             channel itself.
             """
             c = ircdb.channels.getChannel(channel)
+            if isinstance(banmasks, str):
+                banmasks = [banmasks]
+            for banmask in banmasks:
                 c.addBan(banmask, expires)
             ircdb.channels.setChannel(channel, c)
             irc.replySuccess()
         add = wrap(add, ['op',
-                         first('hostmask', 'extbanmask'),
+                         first('hostmask', 'extbanmasks'),
                          additional('expiry', 0)])
 
         @internationalizeDocstring

plugins/Channel/test.py

@@ -29,6 +29,8 @@
 # POSSIBILITY OF SUCH DAMAGE.
 ###
 
+import itertools
+
 from supybot.test import *
 
 import supybot.conf as conf
@@ -161,9 +163,13 @@ class ChannelTestCase(ChannelPluginTestCase):
         self.assertTrue(m.command == 'MODE' and
                         m.args == (self.channel, '+v', 'bar'))
 
-    def assertKban(self, query, hostmask, **kwargs):
+    def assertKban(self, query, *hostmasks, **kwargs):
         m = self.getMsg(query, **kwargs)
-        self.assertEqual(m, ircmsgs.ban(self.channel, hostmask))
+        self.assertIn(m, [
+            ircmsgs.bans(self.channel, permutation)
+            for permutation in itertools.permutations(hostmasks)
+            ])
+
         m = self.getMsg(' ')
         self.assertEqual(m.command, 'KICK')
     def assertBan(self, query, hostmask, **kwargs):
@@ -278,25 +284,29 @@ class ChannelTestCase(ChannelPluginTestCase):
         for style in (['exact'], ['account', 'exact']):
             with conf.supybot.protocols.irc.banmask.context(style):
                 self.assertKban('kban --account --exact foobar',
-                                '~a:account1')
+                                '~a:account1', 'foobar!user@host.domain.tld')
                 join()
                 self.assertKban('kban --account foobar',
                                 '~a:account1')
                 join()
                 self.assertKban('kban --account --host foobar',
-                                '~a:account1')
+                                '~a:account1', '*!*@host.domain.tld')
                 join()
 
         with conf.supybot.protocols.irc.banmask.context(['account', 'exact']):
             self.assertKban('kban foobar',
-                            '~a:account1')
+                            '~a:account1', 'foobar!user@host.domain.tld')
+            join()
+
+        with conf.supybot.protocols.irc.banmask.context(['account', 'host']):
+            self.assertKban('kban foobar',
+                            '~a:account1', '*!*@host.domain.tld')
             join()
 
     def testBan(self):
         with conf.supybot.protocols.irc.banmask.context(['exact']):
             self.assertNotError('ban add foo!bar@baz')
             self.assertNotError('ban remove foo!bar@baz')
-            orig = conf.supybot.protocols.irc.strictRfc()
             with conf.supybot.protocols.irc.strictRfc.context(True):
                 # something wonky is going on here. irc.error (src/Channel.py|449)
                 # is being called but the assert is failing
@@ -322,7 +332,6 @@ class ChannelTestCase(ChannelPluginTestCase):
                             '"foobar!*@baz" (never expires)')
 
     def testIgnore(self):
-        orig = conf.supybot.protocols.irc.banmask()
         def ignore(given, expect=None):
             if expect is None:
                 expect = given
@@ -330,6 +339,7 @@ class ChannelTestCase(ChannelPluginTestCase):
             self.assertResponse('channel ignore list', "'%s'" % expect)
             self.assertNotError('channel ignore remove %s' % expect)
             self.assertRegexp('channel ignore list', 'not currently')
+        with conf.supybot.protocols.irc.banmask.context(['host']):
             ignore('foo!bar@baz', '*!*@baz')
             ignore('foo!*@*')
         with conf.supybot.protocols.irc.banmask.context(['exact']):

src/commands.py

@@ -437,11 +437,11 @@ def getBanmask(irc, msg, args, state):
     state.args[-1] = banmaskstyle.makeBanmask(state.args[-1],
             channel=state.channel, network=irc.network)
 
-def getExtBanmask(irc, msg, args, state):
+def getExtBanmasks(irc, msg, args, state):
     getHostmask(irc, msg, args, state)
     getChannel(irc, msg, args, state)
     banmaskstyle = conf.supybot.protocols.irc.extbanmask
-    state.args[-1] = banmaskstyle.makeExtBanmask(state.args[-1],
+    state.args[-1] = banmaskstyle.makeExtBanmasks(state.args[-1],
             channel=state.channel, network=irc.network)
 
 def getUser(irc, msg, args, state):
@@ -813,7 +813,7 @@ wrappers = ircutils.IrcDict({
     'commandName': getCommandName,
     'email': getEmail,
     'expiry': getExpiry,
-    'extbanmask': getExtBanmask,
+    'extbanmasks': getExtBanmasks,
     'filename': getSomething, # XXX Check for validity.
     'float': getFloat,
     'glob': getGlob,

src/conf.py

@@ -1215,7 +1215,7 @@ class Banmask(registry.SpaceSeparatedSetOfStrings):
         isn't specified via options, the value of
         conf.supybot.protocols.irc.banmask is used.
 
-        Unlike :meth:`makeExtBanmask`, this is guaranteed to return an
+        Unlike :meth:`makeExtBanmasks`, this is guaranteed to return an
         RFC1459-like mask, suitable for ircdb's ignore lists.
 
         options - A list specifying which parts of the hostmask should
@@ -1228,10 +1228,15 @@ class Banmask(registry.SpaceSeparatedSetOfStrings):
             options = supybot.protocols.irc.banmask.getSpecific(
                 network, channel)()
         options = [option for option in options if option != 'account']
-        return self.makeExtBanmask(hostmask, options, channel, network=network)
+        print(hostmask, options)
+        masks = self.makeExtBanmasks(
+            hostmask, options, channel, network=network)
+        assert len(masks) == 1, 'Unexpected number of banmasks: %r' % masks
+        print(masks)
+        return masks[0]
 
-    def makeExtBanmask(self, hostmask, options=None, channel=None, *, network):
+    def makeExtBanmasks(self, hostmask, options=None, channel=None, *, network):
-        """Create a banmask from the given hostmask.  If a style of banmask
+        """Create banmasks from the given hostmask.  If a style of banmask
         isn't specified via options, the value of
         conf.supybot.protocols.irc.banmask is used.
 
@@ -1256,15 +1261,22 @@ class Banmask(registry.SpaceSeparatedSetOfStrings):
         if not options:
             options = supybot.protocols.irc.banmask.getSpecific(
                 network, channel)()
+
+        add_star_mask = False
+        masks = []
+
         for option in options:
             if option == 'nick':
                 bnick = nick
+                add_star_mask = True
             elif option == 'user':
                 buser = user
+                add_star_mask = True
             elif option == 'host':
                 bhost = host
+                add_star_mask = True
             elif option == 'exact':
-                return hostmask
+                masks.append(hostmask)
             elif option == 'account':
                 import supybot.world as world
                 irc = world.getIrc(network)
@@ -1272,11 +1284,18 @@ class Banmask(registry.SpaceSeparatedSetOfStrings):
                     continue
                 extban = ircutils.accountExtban(nick, irc)
                 if extban is not None:
-                    return extban
+                    masks.append(extban)
+
+        if add_star_mask and (bnick, buser, bhost) != ('*', '*', '*'):
+            masks.append(ircutils.joinHostmask(bnick, buser, bhost))
+
         if (bnick, buser, bhost) == ('*', '*', '*') and \
-                ircutils.isUserHostmask(hostmask):
+                ircutils.isUserHostmask(hostmask) and \
-            return hostmask
+                masks == []:
-        return ircutils.joinHostmask(bnick, buser, bhost)
+            masks.append(hostmask)
+
+        return masks
+
 
 registerChannelValue(supybot.protocols.irc, 'banmask',
     Banmask(['host'], _("""Determines what will be used as the