Remove the asynchronous identifing hack and replace it with something better. Fixes m_*_authentication only being able to properly work when people identify normally using nickserv/identify

54 files changed, 680 insertions, 463 deletions

diff --git a/data/modules.example.conf b/data/modules.example.conf
index db8b555ba..4ff3c710c 100644
--- a/data/modules.example.conf
+++ b/data/modules.example.conf
@@ -359,6 +359,9 @@ m_sql_authentication
 	* @n@ is replaced with the user's nickname
 	* @i@ is replaced with the user's IP
 	*
+	* Note that @n@ and @i@ may not always exist in the case of a user identifying outside of the normal
+	* nickserv/identify command, such as through the web panel.
+	*
 	* Furthermore, if a field named email is returned from this query the user's email is
 	* set to its value.
 	*/
diff --git a/include/account.h b/include/account.h
index 8b4920fe4..7aecab43a 100644
--- a/include/account.h
+++ b/include/account.h
@@ -304,7 +304,39 @@ class CoreExport NickCore : public Extensible, public Flags<NickCoreFlag, NI_END
 	 * Deletes all the memory allocated in the certificate list vector and then clears the vector.
 	 */
 	void ClearCert();
+};
+
+class IdentifyRequest
+{
+	Anope::string account;
+	Anope::string password;
+
+	std::set<Module *> holds;
+	bool dispatched;
+	bool success;
+	
+	static std::set<IdentifyRequest *> requests;
+
+ protected:
+	IdentifyRequest(const Anope::string &acc, const Anope::string &pass);
+	virtual ~IdentifyRequest();
+
+ public:
+	virtual void OnSuccess() = 0;
+	virtual void OnFail() = 0;
+
+	const Anope::string &GetAccount() const { return account; }
+	const Anope::string &GetPassword() const { return password; }
+
+	/* Hold this request. Once held it must be Release()d later on */
+	void Hold(Module *m);
+	void Release(Module *m);
+
+	void Success(Module *m);
+
+	void Dispatch();
 
+	static void ModuleUnload(Module *m);
 };
 
 extern CoreExport void change_core_display(NickCore *nc);
diff --git a/include/defs.h b/include/defs.h
index d58f7fed6..c08d683a0 100644
--- a/include/defs.h
+++ b/include/defs.h
@@ -26,6 +26,7 @@ class ConnectionSocket;
 class DNSPacket;
 class dynamic_reference_base;
 class Entry;
+class IdentifyRequest;
 class InfoFormatter;
 class ListenSocket;
 class Log;
diff --git a/include/modules.h b/include/modules.h
index 4b992a15d..14fdff066 100644
--- a/include/modules.h
+++ b/include/modules.h
@@ -765,15 +765,11 @@ class CoreExport Module : public Extensible
 	 */
 	virtual void OnNickInfo(CommandSource &source, NickAlias *na, InfoFormatter &info, bool ShowHidden) { }
 
-	/** Check whether a users password is correct.
-	 * @param u The user
-	 * @param command The command the user is doing
-	 * @param params Command params
-	 * @param account The account the password should be checked against
-	 * @param password The password
-	 * @return EVENT_ALLOW to allow the password, EVENT_STOP to stop processing completely
+	/** Check whether a username and password is correct
+	 * @param u The user trying to identify, if applicable.
+	 * @param req The login request
 	 */
-	virtual EventReturn OnCheckAuthentication(Command *c, CommandSource *source, const std::vector<Anope::string> &params, const Anope::string &account, const Anope::string &password) { return EVENT_CONTINUE; }
+	virtual void OnCheckAuthentication(User *u, IdentifyRequest *req) { }
 
 	/** Called when a user does /ns update
 	 * @param u The user
diff --git a/modules/commands/ns_ghost.cpp b/modules/commands/ns_ghost.cpp
index 3362cf408..9b48a43d1 100644
--- a/modules/commands/ns_ghost.cpp
+++ b/modules/commands/ns_ghost.cpp
@@ -13,6 +13,43 @@
 
 #include "module.h"
 
+class NSGhostRequest : public IdentifyRequest
+{
+	CommandSource source;
+	Command *cmd;
+ 
+ public:
+	NSGhostRequest(CommandSource &src, Command *c, const Anope::string &user, const Anope::string &pass) : IdentifyRequest(user, pass), source(src), cmd(c) { }
+
+	void OnSuccess() anope_override
+	{
+		if (!source.GetUser() || !source.service)
+			return;
+
+		User *user = source.GetUser();
+		if (!user->IsIdentified())
+			source.Reply(_("You may not ghost an unidentified user, use RECOVER instead."));
+		else
+		{
+			Log(LOG_COMMAND, source, cmd) << "for " << GetAccount();
+			Anope::string buf = "GHOST command used by " + source.GetNick();
+			user->Kill(source.service->nick, buf);
+			source.Reply(_("Ghost with your nick has been killed."));
+		}
+	}
+
+	void OnFail() anope_override
+	{
+		source.Reply(ACCESS_DENIED);
+		if (!GetPassword().empty())
+		{
+			Log(LOG_COMMAND, source, cmd) << "with an invalid password for " << GetAccount();
+			if (source.GetUser())
+				bad_password(source.GetUser());
+		}
+	}
+};
+
 class CommandNSGhost : public Command
 {
  public:
@@ -50,37 +87,21 @@ class CommandNSGhost : public Command
 				ok = true;
 			else if (source.GetUser() && !source.GetUser()->fingerprint.empty() && na->nc->FindCert(source.GetUser()->fingerprint))
 				ok = true;
-			else if (!pass.empty())
-			{
-				EventReturn MOD_RESULT;
-				FOREACH_RESULT(I_OnCheckAuthentication, OnCheckAuthentication(this, &source, params, na->nc->display, pass));
-				if (MOD_RESULT == EVENT_STOP)
-					return;
-				else if (MOD_RESULT == EVENT_ALLOW)
-					ok = true;
-			}
 
-			if (ok)
+			if (ok == false && !pass.empty())
 			{
-				if (!user->IsIdentified())
-					source.Reply(_("You may not ghost an unidentified user, use RECOVER instead."));
-				else
-				{
-					Log(LOG_COMMAND, source, this) << "for " << nick;
-					Anope::string buf = "GHOST command used by " + source.GetNick();
-					user->Kill(Config->NickServ, buf);
-					source.Reply(_("Ghost with your nick has been killed."), nick.c_str());
-				}
+				NSGhostRequest *req = new NSGhostRequest(source, this, na->nc->display, pass);
+				FOREACH_MOD(I_OnCheckAuthentication, OnCheckAuthentication(source.GetUser(), req));
+				req->Dispatch();
 			}
 			else
 			{
-				source.Reply(ACCESS_DENIED);
-				if (!pass.empty())
-				{
-					Log(LOG_COMMAND, source, this) << "with an invalid password for " << nick;
-					if (source.GetUser())
-						bad_password(source.GetUser());
-				}
+				NSGhostRequest req(source, this, na->nc->display, pass);
+
+				if (ok)
+					req.OnSuccess();
+				else
+					req.OnFail();
 			}
 		}
 
diff --git a/modules/commands/ns_group.cpp b/modules/commands/ns_group.cpp
index 3df4b3460..30026c109 100644
--- a/modules/commands/ns_group.cpp
+++ b/modules/commands/ns_group.cpp
@@ -13,6 +13,61 @@
 
 #include "module.h"
 
+class NSGroupRequest : public IdentifyRequest
+{
+	CommandSource source;
+	Command *cmd;
+	Anope::string nick;
+	dynamic_reference<NickAlias> target;
+ 
+ public:
+	NSGroupRequest(CommandSource &src, Command *c, const Anope::string &n, NickAlias *targ, const Anope::string &pass) : IdentifyRequest(targ->nc->display, pass), source(src), cmd(c), nick(n), target(targ) { }
+
+	void OnSuccess() anope_override
+	{
+		if (!source.GetUser() || source.GetUser()->nick != nick || !target || !target->nc)
+			return;
+
+		User *u = source.GetUser();
+		NickAlias *na = findnick(nick);
+		/* If the nick is already registered, drop it. */
+		if (na)
+		{
+			FOREACH_MOD(I_OnChangeCoreDisplay, OnChangeCoreDisplay(na->nc, u->nick));
+			na->destroy();
+		}
+
+		na = new NickAlias(nick, target->nc);
+
+		Anope::string last_usermask = u->GetIdent() + "@" + u->GetDisplayedHost();
+		na->last_usermask = last_usermask;
+		na->last_realname = u->realname;
+		na->time_registered = na->last_seen = Anope::CurTime;
+
+		u->Login(target->nc);
+		ircdproto->SendLogin(u);
+		if (!Config->NoNicknameOwnership && na->nc == u->Account() && na->nc->HasFlag(NI_UNCONFIRMED) == false)
+			u->SetMode(findbot(Config->NickServ), UMODE_REGISTERED);
+		FOREACH_MOD(I_OnNickGroup, OnNickGroup(u, target));
+
+		Log(LOG_COMMAND, source, cmd) << "makes " << nick << " join group of " << target->nick << " (" << target->nc->display << ") (email: " << (!target->nc->email.empty() ? target->nc->email : "none") << ")";
+		source.Reply(_("You are now in the group of \002%s\002."), target->nick.c_str());
+
+		u->lastnickreg = Anope::CurTime;
+
+	}
+
+	void OnFail() anope_override
+	{
+		if (!source.GetUser())
+			return;
+
+		Log(LOG_COMMAND, source, cmd) << "failed group for " << target->nick;
+		source.Reply(PASSWORD_INCORRECT);
+		bad_password(source.GetUser());
+	}
+};
+
 class CommandNSGroup : public Command
 {
  public:
@@ -62,7 +117,7 @@ class CommandNSGroup : public Command
 			source.Reply(NICK_X_NOT_REGISTERED, nick.c_str());
 		else if (Anope::CurTime < u->lastnickreg + Config->NSRegDelay)
 			source.Reply(_("Please wait %d seconds before using the GROUP command again."), (Config->NSRegDelay + u->lastnickreg) - Anope::CurTime);
-		else if (target && target->nc->HasFlag(NI_SUSPENDED))
+		else if (target->nc->HasFlag(NI_SUSPENDED))
 		{
 			Log(LOG_COMMAND, source, this) << "tried to use GROUP for SUSPENDED nick " << target->nick;
 			source.Reply(NICK_X_SUSPENDED, target->nick.c_str());
@@ -75,6 +130,12 @@ class CommandNSGroup : public Command
 			source.Reply(_("Your nick is already registered."));
 		else if (Config->NSMaxAliases && (target->nc->aliases.size() >= Config->NSMaxAliases) && !target->nc->IsServicesOper())
 			source.Reply(_("There are too many nicks in %s's group."));
+		else if (u->nick.length() <= Config->NSGuestNickPrefix.length() + 7 &&
+			u->nick.length() >= Config->NSGuestNickPrefix.length() + 1 &&
+			!u->nick.find_ci(Config->NSGuestNickPrefix) && !u->nick.substr(Config->NSGuestNickPrefix.length()).find_first_not_of("1234567890"))
+		{
+			source.Reply(NICK_CANNOT_BE_REGISTERED, u->nick.c_str());
+		}
 		else
 		{
 			bool ok = false;
@@ -82,63 +143,23 @@ class CommandNSGroup : public Command
 				ok = true;
 			else if (!u->fingerprint.empty() && target->nc->FindCert(u->fingerprint))
 				ok = true;
-			else if (!pass.empty())
-			{
-				EventReturn MOD_RESULT;
-				FOREACH_RESULT(I_OnCheckAuthentication, OnCheckAuthentication(this, &source, params, target->nc->display, pass));
-				if (MOD_RESULT == EVENT_STOP)
-					return;
-				else if (MOD_RESULT == EVENT_ALLOW)
-					ok = true;
-			}
-			if (ok)
-			{
-				/* If the nick is already registered, drop it.
-				 * If not, check that it is valid.
-				 */
-				if (na)
-				{
-					FOREACH_MOD(I_OnChangeCoreDisplay, OnChangeCoreDisplay(na->nc, u->nick));
-					na->destroy();
-				}
-				else
-				{
-					size_t prefixlen = Config->NSGuestNickPrefix.length();
-					size_t nicklen = u->nick.length();
-
-					if (nicklen <= prefixlen + 7 && nicklen >= prefixlen + 1 && !u->nick.find_ci(Config->NSGuestNickPrefix) && !u->nick.substr(prefixlen).find_first_not_of("1234567890"))
-					{
-						source.Reply(NICK_CANNOT_BE_REGISTERED, u->nick.c_str());
-						return;
-					}
-				}
-
-				na = new NickAlias(u->nick, target->nc);
-
-				Anope::string last_usermask = u->GetIdent() + "@" + u->GetDisplayedHost();
-				na->last_usermask = last_usermask;
-				na->last_realname = u->realname;
-				na->time_registered = na->last_seen = Anope::CurTime;
-
-				u->Login(target->nc);
-				ircdproto->SendLogin(u);
-				if (!Config->NoNicknameOwnership && na->nc == u->Account() && na->nc->HasFlag(NI_UNCONFIRMED) == false)
-					u->SetMode(findbot(Config->NickServ), UMODE_REGISTERED);
-				FOREACH_MOD(I_OnNickGroup, OnNickGroup(u, target));
 
-				Log(LOG_COMMAND, source, this) << "makes " << u->nick << " join group of " << target->nick << " (" << target->nc->display << ") (email: " << (!target->nc->email.empty() ? target->nc->email : "none") << ")";
-				source.Reply(_("You are now in the group of \002%s\002."), target->nick.c_str());
-
-				u->lastnickreg = Anope::CurTime;
+			if (ok == false && !pass.empty())
+			{
+				NSGroupRequest *req = new NSGroupRequest(source, this, u->nick, target, pass);
+				FOREACH_MOD(I_OnCheckAuthentication, OnCheckAuthentication(source.GetUser(), req));
+				req->Dispatch();
 			}
 			else
 			{
-				Log(LOG_COMMAND, source, this) << "failed group for " << target->nick;
-				source.Reply(PASSWORD_INCORRECT);
-				bad_password(u);
+				NSGroupRequest req(source, this, u->nick, target, pass);
+
+				if (ok)
+					req.OnSuccess();
+				else
+					req.OnFail();
 			}
 		}
-		return;
 	}
 
 	bool OnHelp(CommandSource &source, const Anope::string &subcommand) anope_override
diff --git a/modules/commands/ns_identify.cpp b/modules/commands/ns_identify.cpp
index cce0569dd..47df4255b 100644
--- a/modules/commands/ns_identify.cpp
+++ b/modules/commands/ns_identify.cpp
@@ -13,6 +13,47 @@
 
 #include "module.h"
 
+class NSIdentifyRequest : public IdentifyRequest
+{
+	CommandSource source;
+	Command *cmd;
+
+ public:
+	NSIdentifyRequest(CommandSource &s, Command *c, const Anope::string &acc, const Anope::string &pass) : IdentifyRequest(acc, pass), source(s), cmd(c) { }
+
+	void OnSuccess() anope_override
+	{
+		if (!source.GetUser())
+			return;
+
+		User *u = source.GetUser();
+		NickAlias *na = findnick(GetAccount());
+
+		if (!na)
+			source.Reply(NICK_X_NOT_REGISTERED, GetAccount().c_str());
+		else
+		{
+			if (u->IsIdentified())
+				Log(LOG_COMMAND, source, cmd) << "to log out of account " << u->Account()->display;
+
+			Log(LOG_COMMAND, source, cmd) << "and identified for account " << na->nc->display;
+			source.Reply(_("Password accepted - you are now recognized."));
+			u->Identify(na);
+			na->Release();
+		}
+	}
+
+	void OnFail() anope_override
+	{
+		if (source.GetUser())
+		{
+			Log(LOG_COMMAND, source, cmd) << "and failed to identify";
+			source.Reply(PASSWORD_INCORRECT);
+			bad_password(source.GetUser());
+		}
+	}
+};
+
 class CommandNSIdentify : public Command
 {
  public:
@@ -40,29 +81,9 @@ class CommandNSIdentify : public Command
 			source.Reply(_("You are already identified."));
 		else
 		{
-			EventReturn MOD_RESULT;
-			FOREACH_RESULT(I_OnCheckAuthentication, OnCheckAuthentication(this, &source, params, na ? na->nc->display : nick, pass));
-			if (MOD_RESULT == EVENT_STOP)
-				return;
-
-			if (!na)
-				source.Reply(NICK_X_NOT_REGISTERED, nick.c_str());
-			else if (MOD_RESULT != EVENT_ALLOW)
-			{
-				Log(LOG_COMMAND, source, this) << "and failed to identify";
-				source.Reply(PASSWORD_INCORRECT);
-				bad_password(u);
-			}
-			else
-			{
-				if (u->IsIdentified())
-					Log(LOG_COMMAND, source, this) << "to log out of account " << u->Account()->display;
-
-				Log(LOG_COMMAND, source, this) << "and identified for account " << na->nc->display;
-				source.Reply(_("Password accepted - you are now recognized."));
-				u->Identify(na);
-				na->Release();
-			}
+			NSIdentifyRequest *req = new NSIdentifyRequest(source, this, na ? na->nc->display : nick, pass);
+			FOREACH_MOD(I_OnCheckAuthentication, OnCheckAuthentication(source.GetUser(), req));
+			req->Dispatch();
 		}
 		return;
 	}
diff --git a/modules/commands/ns_recover.cpp b/modules/commands/ns_recover.cpp
index 7c5b6cbeb..0611e6589 100644
--- a/modules/commands/ns_recover.cpp
+++ b/modules/commands/ns_recover.cpp
@@ -14,11 +14,22 @@
 
 #include "module.h"
 
-class CommandNSRecover : public Command
+class NSRecoverRequest : public IdentifyRequest
 {
- private:
-	void DoRecover(CommandSource &source, User *u, NickAlias *na, const Anope::string &nick)
+	CommandSource source;
+	Command *cmd;
+	dynamic_reference<NickAlias> na;
+ 
+ public:
+	NSRecoverRequest(CommandSource &src, Command *c, NickAlias *n, const Anope::string &pass) : IdentifyRequest(n->nc->display, pass), source(src), cmd(c), na(n) { }
+
+	void OnSuccess() anope_override
 	{
+		if (!source.GetUser() || !na)
+			return;
+
+		User *u = source.GetUser();
+
 		u->SendMessage(source.owner, FORCENICKCHANGE_NOW);
 
 		if (u->Account() == na->nc)
@@ -31,9 +42,27 @@ class CommandNSRecover : public Command
 
 		/* Convert Config->NSReleaseTimeout seconds to string format */
 		Anope::string relstr = duration(Config->NSReleaseTimeout);
-		source.Reply(NICK_RECOVERED, Config->UseStrictPrivMsgString.c_str(), Config->NickServ.c_str(), nick.c_str(), relstr.c_str());
+		source.Reply(NICK_RECOVERED, Config->UseStrictPrivMsgString.c_str(), Config->NickServ.c_str(), na->nick.c_str(), relstr.c_str());
 	}
 
+	void OnFail() anope_override
+	{
+		if (!source.GetUser())
+			return;
+
+		User *u = source.GetUser();
+
+		source.Reply(ACCESS_DENIED);
+		if (!GetPassword().empty())
+		{
+			Log(LOG_COMMAND, source, cmd) << "with invalid password for " << na->nick;
+			bad_password(u);
+		}
+	}
+};
+
+class CommandNSRecover : public Command
+{
  public:
 	CommandNSRecover(Module *creator) : Command(creator, "nickserv/recover", 1, 2)
 	{
@@ -60,25 +89,6 @@ class CommandNSRecover : public Command
 			source.Reply(NICK_X_SUSPENDED, na->nick.c_str());
 		else if (nick.equals_ci(source.GetNick()))
 			source.Reply(_("You can't recover yourself!"));
-		else if (!pass.empty())
-		{
-			EventReturn MOD_RESULT;
-			FOREACH_RESULT(I_OnCheckAuthentication, OnCheckAuthentication(this, &source, params, na->nc->display, pass));
-			if (MOD_RESULT == EVENT_STOP)
-				return;
-
-			if (MOD_RESULT == EVENT_ALLOW)
-			{
-				this->DoRecover(source, u2, na, nick);
-			}
-			else
-			{
-				source.Reply(ACCESS_DENIED);
-				Log(LOG_COMMAND, source, this) << "with invalid password for " << nick;
-				if (source.GetUser())
-					bad_password(source.GetUser());
-			}
-		}
 		else
 		{
 			bool ok = false;
@@ -88,12 +98,23 @@ class CommandNSRecover : public Command
 				ok = true;
 			else if (source.GetUser() && !source.GetUser()->fingerprint.empty() && na->nc->FindCert(source.GetUser()->fingerprint))
 				ok = true;
-			if (ok)
-				this->DoRecover(source, u2, na, nick);
+
+			if (ok == false && !pass.empty())
+			{
+				NSRecoverRequest *req = new NSRecoverRequest(source, this, na, pass);
+				FOREACH_MOD(I_OnCheckAuthentication, OnCheckAuthentication(source.GetUser(), req));
+				req->Dispatch();
+			}
 			else
-				source.Reply(ACCESS_DENIED);
+			{
+				NSRecoverRequest req(source, this, na, pass);
+
+				if (ok)
+					req.OnSuccess();
+				else
+					req.OnFail();
+			}
 		}
-		return;
 	}
 
 	bool OnHelp(CommandSource &source, const Anope::string &subcommand) anope_override
diff --git a/modules/commands/ns_release.cpp b/modules/commands/ns_release.cpp
index 9e12685aa..8afd9fade 100644
--- a/modules/commands/ns_release.cpp
+++ b/modules/commands/ns_release.cpp
@@ -13,6 +13,38 @@
 
 #include "module.h"
 
+class NSReleaseRequest : public IdentifyRequest
+{
+	CommandSource source;
+	Command *cmd;
+	dynamic_reference<NickAlias> na;
+ 
+ public:
+	NSReleaseRequest(CommandSource &src, Command *c, NickAlias *n, const Anope::string &pass) : IdentifyRequest(n->nc->display, pass), source(src), cmd(c), na(n) { }
+
+	void OnSuccess() anope_override
+	{
+		if (!source.GetUser() || !na)
+			return;
+
+		bool override = source.GetAccount() != na->nc && source.HasPriv("nickserv/release");
+		Log(override ? LOG_OVERRIDE : LOG_COMMAND, source, cmd) << "for nickname " << na->nick;
+		na->Release();
+		source.Reply(_("Services' hold on \002%s\002 has been released."), na->nick.c_str());
+	}
+
+	void OnFail() anope_override
+	{
+		source.Reply(ACCESS_DENIED);
+		if (!GetPassword().empty())
+		{
+			Log(LOG_COMMAND, source, cmd) << "with invalid password for " << n