Move CheckKick event to Channel and make os_forbid use it instead of …

…kicking users in the join event, which does bad things

include/channels.h

@@ -278,6 +278,12 @@ class CoreExport Channel : public Base, public Extensible
 	 */
 	bool Unban(User *u, bool full = false);
 
+	/** Check whether a user is permitted to be on this channel
+	 * @param u The user
+	 * @return true if they are allowed, false if they aren't and were kicked
+	 */
+	bool CheckKick(User *user);
+
 	/** Finds a channel
 	 * @param name The channel to find
 	 * @return The channel, if found

include/modules.h

@@ -679,13 +679,13 @@ class CoreExport Module : public Extensible
 
 	/** Called after a user join a channel when we decide whether to kick them or not
 	 * @param u The user
-	 * @param ci The channel
+	 * @param c The channel
 	 * @param kick Set to true to kick
 	 * @param mask The mask to ban, if any
 	 * @param reason The reason for the kick
 	 * @return EVENT_STOP to prevent the user from joining by kicking/banning the user
 	 */
-	virtual EventReturn OnCheckKick(User *u, ChannelInfo *ci, Anope::string &mask, Anope::string &reason) { throw NotImplementedException(); }
+	virtual EventReturn OnCheckKick(User *u, Channel *c, Anope::string &mask, Anope::string &reason) { throw NotImplementedException(); }
 
 	/** Called when a user requests info for a channel
 	 * @param source The user requesting info

include/regchannel.h

@@ -393,12 +393,6 @@ class CoreExport ChannelInfo : public Serializable, public Extensible
 	 */
 	Anope::string GetMLockAsString(bool complete) const;
 
-	/** Check whether a user is permitted to be on this channel
-	 * @param u The user
-	 * @return true if they are allowed, false if they aren't and were kicked
-	 */
-	bool CheckKick(User *user);
-
 	/** Get the level for a privilege
 	 * @param priv The privilege name
 	 * @return the level

modules/commands/cs_akick.cpp

@@ -394,7 +394,7 @@ class CommandCSAKick : public Command
 			ChanUserContainer *uc = it->second;
 			++it;
 
-			if (ci->CheckKick(uc->user))
+			if (c->CheckKick(uc->user))
 				++count;
 		}
 
@@ -514,14 +514,14 @@ class CSAKick : public Module
 	{
 	}
 
-	EventReturn OnCheckKick(User *u, ChannelInfo *ci, Anope::string &mask, Anope::string &reason) anope_override
+	EventReturn OnCheckKick(User *u, Channel *c, Anope::string &mask, Anope::string &reason) anope_override
 	{
-		if (ci->c->MatchesList(u, "EXCEPT"))
+		if (!c->ci || c->MatchesList(u, "EXCEPT"))
 			return EVENT_CONTINUE;
 
-		for (unsigned j = 0, end = ci->GetAkickCount(); j < end; ++j)
+		for (unsigned j = 0, end = c->ci->GetAkickCount(); j < end; ++j)
 		{
-			AutoKick *autokick = ci->GetAkick(j);
+			AutoKick *autokick = c->ci->GetAkick(j);
 			bool kick = false;
 
 			if (autokick->nc)

modules/commands/cs_set.cpp

@@ -1130,12 +1130,12 @@ class CSSet : public Module
 	{
 	}
 
-	EventReturn OnCheckKick(User *u, ChannelInfo *ci, Anope::string &mask, Anope::string &reason) anope_override
+	EventReturn OnCheckKick(User *u, Channel *c, Anope::string &mask, Anope::string &reason) anope_override
 	{
-		if (!ci->HasExt("RESTRICTED") || ci->c->MatchesList(u, "EXCEPT"))
+		if (!c->ci || !c->ci->HasExt("RESTRICTED") || c->MatchesList(u, "EXCEPT"))
 			return EVENT_CONTINUE;
 
-		if (ci->AccessFor(u).empty() && (!ci->GetFounder() || u->Account() != ci->GetFounder()))
+		if (c->ci->AccessFor(u).empty() && (!c->ci->GetFounder() || u->Account() != c->ci->GetFounder()))
 			return EVENT_STOP;
 
 		return EVENT_CONTINUE;

modules/commands/cs_suspend.cpp

@@ -207,9 +207,9 @@ class CSSuspend : public Module
 		catch (const ConvertException &) { }
 	}
 
-	EventReturn OnCheckKick(User *u, ChannelInfo *ci, Anope::string &mask, Anope::string &reason) anope_override
+	EventReturn OnCheckKick(User *u, Channel *c, Anope::string &mask, Anope::string &reason) anope_override
 	{
-		if (u->HasMode("OPER") || !ci->HasExt("SUSPENDED"))
+		if (u->HasMode("OPER") || !c->ci || !c->ci->HasExt("SUSPENDED"))
 			return EVENT_CONTINUE;
 
 		reason = Language::Translate(u, _("This channel may not be used."));

modules/commands/os_forbid.cpp

@@ -282,35 +282,36 @@ class OSForbid : public Module
 		}
 	}
 
-	void OnJoinChannel(User *u, Channel *c) anope_override
+	EventReturn OnCheckKick(User *u, Channel *c, Anope::string &mask, Anope::string &reason) anope_override
 	{
 		BotInfo *OperServ = Config->GetClient("OperServ");
 		if (u->HasMode("OPER") || !OperServ)
-			return;
+			return EVENT_CONTINUE;
 
 		ForbidData *d = this->forbidService.FindForbid(c->name, FT_CHAN);
 		if (d != NULL)
 		{
 			ServiceReference<ChanServService> chanserv("ChanServService", "ChanServ");
-			if (!chanserv)
-				;
-			else if (IRCD->CanSQLineChannel)
+			if (IRCD->CanSQLineChannel)
 			{
 				time_t inhabit = Config->GetModule("chanserv")->Get<time_t>("inhabit", "15s");
 				XLine x(c->name, OperServ->nick, Anope::CurTime + inhabit, d->reason);
 				IRCD->SendSQLine(NULL, &x);
 			}
-			else
+			else if (chanserv)
 			{
-				if (chanserv)
-					chanserv->Hold(c);
+				chanserv->Hold(c);
 			}
 
 			if (d->reason.empty())
-				c->Kick(OperServ, u, _("This channel has been forbidden."));
+				reason = Language::Translate(u, _("This channel has been forbidden."));
 			else
-				c->Kick(OperServ, u, _("This channel has been forbidden: %s"), d->reason.c_str());
+				reason = Anope::printf(Language::Translate(u, _("This channel has been forbidden: %s")), d->reason.c_str());
+
+			return EVENT_STOP;
 		}
+
+		return EVENT_CONTINUE;
 	}
 
 	EventReturn OnPreCommand(CommandSource &source, Command *command, std::vector<Anope::string> &params) anope_override

modules/pseudoclients/operserv.cpp

@@ -233,9 +233,9 @@ class OperServCore : public Module
 			this->sqlines.CheckAllXLines(u);
 	}
 
-	EventReturn OnCheckKick(User *u, ChannelInfo *ci, Anope::string &mask, Anope::string &reason) anope_override
+	EventReturn OnCheckKick(User *u, Channel *c, Anope::string &mask, Anope::string &reason) anope_override
 	{
-		XLine *x = this->sqlines.CheckChannel(ci->c);
+		XLine *x = this->sqlines.CheckChannel(c);
 		if (x)
 		{
 			this->sqlines.OnMatch(u, x);

src/channels.cpp

@@ -23,6 +23,7 @@
 #include "config.h"
 #include "access.h"
 #include "sockets.h"
+#include "language.h"
 
 channel_map ChannelList;
 
@@ -174,12 +175,12 @@ ChanUserContainer* Channel::JoinUser(User *user)
 	if (user->server && user->server->IsSynced())
 		Log(user, this, "join");
 
-	FOREACH_MOD(OnJoinChannel, (user, this));
-
 	ChanUserContainer *cuc = new ChanUserContainer(user, this);
 	user->chans[this] = cuc;
 	this->users[user] = cuc;
 
+	FOREACH_MOD(OnJoinChannel, (user, this));
+
 	return cuc;
 }
 
@@ -908,6 +909,39 @@ bool Channel::Unban(User *u, bool full)
 	return ret;
 }
 
+bool Channel::CheckKick(User *user)
+{
+	if (user->super_admin)
+		return false;
+
+	/* We don't enforce services restrictions on clients on ulined services
+	 * as this will likely lead to kick/rejoin floods. ~ Viper */
+	if (user->server->IsULined())
+		return false;
+
+	if (user->IsProtected())
+		return false;
+
+	Anope::string mask, reason;
+
+	EventReturn MOD_RESULT;
+	FOREACH_RESULT(OnCheckKick, MOD_RESULT, (user, this, mask, reason));
+	if (MOD_RESULT != EVENT_STOP)
+		return false;
+
+	if (mask.empty())
+		mask = this->ci->GetIdealBan(user);
+	if (reason.empty())
+		reason = Language::Translate(user->Account(), CHAN_NOT_ALLOWED_TO_JOIN);
+
+	Log(LOG_DEBUG) << "Autokicking " << user->nick << " (" << mask << ") from " << this->name;
+
+	this->SetMode(NULL, "BAN", mask);
+	this->Kick(NULL, user, "%s", reason.c_str());
+
+	return true;
+}
+
 Channel* Channel::Find(const Anope::string &name)
 {
 	channel_map::const_iterator it = ChannelList.find(name);

src/messages.cpp

@@ -139,8 +139,7 @@ void Join::SJoin(MessageSource &source, const Anope::string &chan, time_t ts, co
 		 */
 		c->SetCorrectModes(u, true);
 
-		if (c->ci)
-			c->ci->CheckKick(u);
+		c->CheckKick(u);
 	}
 
 	/* Channel is done syncing */

src/regchannel.cpp

@@ -18,7 +18,6 @@
 #include "channels.h"
 #include "config.h"
 #include "bots.h"
-#include "language.h"
 #include "servers.h"
 
 Serialize::Checker<registered_channel_map> RegisteredChannelList("ChannelInfo");
@@ -1007,42 +1006,6 @@ Anope::string ChannelInfo::GetMLockAsString(bool complete) const
 	return pos + neg + params;
 }
 
-bool ChannelInfo::CheckKick(User *user)
-{
-	if (!user || !this->c)
-		return false;
-
-	if (user->super_admin)
-		return false;
-
-	/* We don't enforce services restrictions on clients on ulined services
-	 * as this will likely lead to kick/rejoin floods. ~ Viper */
-	if (user->server->IsULined())
-		return false;
-
-	if (user->IsProtected())
-		return false;
-
-	Anope::string mask, reason;
-
-	EventReturn MOD_RESULT;
-	FOREACH_RESULT(OnCheckKick, MOD_RESULT, (user, this, mask, reason));
-	if (MOD_RESULT != EVENT_STOP)
-		return false;
-
-	if (mask.empty())
-		mask = this->GetIdealBan(user);
-	if (reason.empty())
-		reason = Language::Translate(user->Account(), CHAN_NOT_ALLOWED_TO_JOIN);
-
-	Log(LOG_DEBUG) << "Autokicking " << user->nick << " (" << mask << ") from " << this->name;
-
-	this->c->SetMode(NULL, "BAN", mask);
-	this->c->Kick(NULL, user, "%s", reason.c_str());
-
-	return true;
-}
-
 int16_t ChannelInfo::GetLevel(const Anope::string &priv) const
 {
 	if (PrivilegeManager::FindPrivilege(priv) == NULL)
@@ -1074,7 +1037,8 @@ void ChannelInfo::ClearLevels()
 
 Anope::string ChannelInfo::GetIdealBan(User *u) const
 {
-	switch (this->bantype)
+	int bt = this ? this->bantype : -1;
+	switch (bt)
 	{
 		case 0:
 			return "*!" + u->GetVIdent() + "@" + u->GetDisplayedHost();