Fix various inconsistencies/crashes related to having multiple opers configured for the same user through combinations of os_oper/config/m_sql_oper

modules/commands/os_oper.cpp

@@ -135,6 +135,8 @@ class CommandOSOper : public Command
 				source.Reply(_("Nick \002%s\002 is not a Services Operator."), oper.c_str());
 			else if (!HasPrivs(source, na->nc->o->ot))
 				source.Reply(ACCESS_DENIED);
+			else if (std::find(Config->Opers.begin(), Config->Opers.end(), na->nc->o) != Config->Opers.end())
+				source.Reply(_("Oper \002%s\002 is configured in the configuration file(s) and can not be removed by this command."), na->nc->display.c_str());
 			else
 			{
 				delete na->nc->o;

modules/extra/m_sql_oper.cpp

@@ -17,6 +17,18 @@ class SQLOperResult : public SQL::Interface
 		~SQLOperResultDeleter() { delete res; }
 	};
 
+	void Deoper()
+	{
+		if (user->Account() && user->Account()->o && dynamic_cast<SQLOper *>(user->Account()->o))
+		{
+			delete user->Account()->o;
+			user->Account()->o = NULL;
+
+			Log(this->owner) << "m_sql_oper: Removed services operator from " << user->nick << " (" << user->Account()->display << ")";
+			user->RemoveMode(OperServ, "OPER"); // Probably not set, just incase
+		}
+	}
+
  public:
 	SQLOperResult(Module *m, User *u) : SQL::Interface(m), user(u) { }
 
@@ -24,9 +36,16 @@ class SQLOperResult : public SQL::Interface
 	{
 		SQLOperResultDeleter d(this);
 
-		if (!user || !user->Account() || r.Rows() == 0)
+		if (!user || !user->Account())
 			return;
 
+		if (r.Rows() == 0)
+		{
+			Log(LOG_DEBUG) << "m_sql_oper: Got 0 rows for " << user->nick;
+			Deoper();
+			return;
+		}
+
 		Anope::string opertype;
 		try
 		{
@@ -34,6 +53,7 @@ class SQLOperResult : public SQL::Interface
 		}
 		catch (const SQL::Exception &)
 		{
+			Log(this) << "Expected column named \"opertype\" but one was not found";
 			return;
 		}
 
@@ -44,19 +64,15 @@ class SQLOperResult : public SQL::Interface
 		{
 			modes = r.Get(0, "modes");
 		}
-		catch (const SQL::Exception &) { }
+		catch (const SQL::Exception &)
+		{
+			// Common case here is an exception, but this probably doesn't get this far often
+		}
 
 		BotInfo *OperServ = Config->GetClient("OperServ");
 		if (opertype.empty())
 		{
-			if (user->Account() && user->Account()->o && dynamic_cast<SQLOper *>(user->Account()->o))
-			{
-				delete user->Account()->o;
-				user->Account()->o = NULL;
-
-				Log(this->owner) << "m_sql_oper: Removed services operator from " << user->nick << " (" << user->Account()->display << ")";
-				user->RemoveMode(OperServ, "OPER"); // Probably not set, just incase
-			}
+			Deoper();
 			return;
 		}
 
@@ -67,9 +83,17 @@ class SQLOperResult : public SQL::Interface
 			return;
 		}
 
+		if (user->Account()->o && !dynamic_cast<SQLOper *>(user->Account()->o))
+		{
+			Log(this) << "Oper " << user->Account()->display << " has type " << opertype << ", but is already configured as an oper of type " << user->Account()->o->ot->GetName();
+			return;
+		}
+
 		if (!user->Account()->o || user->Account()->o->ot != ot)
 		{
 			Log(this->owner) << "m_sql_oper: Tieing oper " << user->nick << " to type " << opertype;
+
+			delete user->Account()->o;
 			user->Account()->o = new SQLOper(user->Account()->display, ot);
 		}
 

src/config.cpp

@@ -502,6 +502,13 @@ Conf::Conf() : Block("")
 		if (!na)
 			continue;
 
+		if (!na->nc || na->nc->o)
+		{
+			// If the account is already an oper it might mean two oper blocks for the same nick, or
+			// something else has configured them as an oper (like a module)
+			continue;
+		}
+
 		na->nc->o = o;
 		Log() << "Tied oper " << na->nc->display << " to type " << o->ot->GetName();
 	}