enforce allowed hosts when an auth module accepts the login

The Allow list was only checked in CZNC::AuthUser() after the core
password check, so logins accepted by imapauth, cyrusauth or certauth
(including SASL EXTERNAL) ignored it. Check it in CAuthBase::AcceptLogin()
and CClient::AcceptSASLLogin() instead.

src/Client.cpp

@@ -420,6 +420,13 @@ void CAuthBase::Invalidate() { m_pSock = nullptr; }
 
 void CAuthBase::AcceptLogin(CUser& User) {
     if (m_pSock) {
+        // Auth modules call this directly, so check allowed hosts here rather
+        // than only in CZNC::AuthUser()
+        CString sHost = GetRemoteIP();
+        if (!User.IsHostAllowed(sHost)) {
+            RefuseLogin("Your host [" + sHost + "] is not allowed");
+            return;
+        }
         AcceptedLogin(User);
     }
     Invalidate();
@@ -1340,6 +1347,11 @@ void CClient::RefuseSASLLogin(const CString& sReason) {
 }
 
 void CClient::AcceptSASLLogin(CUser& User) {
+    if (!User.IsHostAllowed(GetRemoteIP())) {
+        RefuseSASLLogin("Your host [" + GetRemoteIP() + "] is not allowed");
+        return;
+    }
+
     PutClient(":irc.znc.in 900 " + GetNick() + " " + GetNick() + "!" +
               User.GetIdent() + "@" + GetRemoteIP() + " " + User.GetUsername() +
               " :You are now logged in as " + User.GetUsername());

src/znc.cpp

@@ -1978,8 +1978,6 @@ CZNC::TrafficStatsMap CZNC::GetNetworkTrafficStats(const CString& sUsername,
 }
 
 void CZNC::AuthUser(std::shared_ptr<CAuthBase> AuthClass) {
-    // TODO unless the auth module calls it, CUser::IsHostAllowed() is not
-    // honoured
     bool bReturn = false;
     GLOBALMODULECALL(OnLoginAttempt(AuthClass), &bReturn);
     if (bReturn) return;
@@ -1991,13 +1989,7 @@ void CZNC::AuthUser(std::shared_ptr<CAuthBase> AuthClass) {
         return;
     }
 
-    CString sHost = AuthClass->GetRemoteIP();
+    // CAuthBase::AcceptLogin() checks allowed hosts
-
-    if (!pUser->IsHostAllowed(sHost)) {
-        AuthClass->RefuseLogin("Your host [" + sHost + "] is not allowed");
-        return;
-    }
-
     AuthClass->AcceptLogin(*pUser);
 }
 

test/ClientTest.cpp

@@ -390,3 +390,11 @@ TEST_F(ClientTest, OnSendToClientMessage) {
     EXPECT_THAT(m_pTestSock->vsLines, ElementsAre(msg.ToString()));
     m_pTestModule->bSendHooks = false;
 }
+
+TEST_F(ClientTest, AcceptSASLLoginAllowedHosts) {
+    TestClient client;
+    m_pTestUser->AddAllowedHost("203.0.113.0/24");
+    client.AcceptSASLLogin(*m_pTestUser);
+    EXPECT_THAT(client.vsLines,
+                ElementsAre(":irc.znc.in 904 me :Your host [] is not allowed"));
+}

test/integration/tests/modules.cpp

@@ -589,6 +589,41 @@ TEST_F(ZNCTest, SaslAuthPlainImapAuthLineBreak) {
     EXPECT_FALSE(imap.hasPendingConnections());
 }
 
+TEST_F(ZNCTest, ImapAuthAllowedHosts) {
+    QTcpServer imap;
+    ASSERT_TRUE(imap.listen(QHostAddress::LocalHost)) << imap.errorString().toStdString();
+    QFile conf(m_dir.path() + "/configs/znc.conf");
+    ASSERT_TRUE(conf.open(QIODevice::Append | QIODevice::Text));
+    QTextStream(&conf) << R"(
+        <User foo>
+            Pass = plain#hunter2
+            Allow = 203.0.113.0/24
+        </User>
+    )";
+    conf.close();
+    auto znc = Run();
+    auto ircd = ConnectIRCd();
+    auto client = LoginClient();
+    client.Write(QStringLiteral("znc loadmod imapauth 127.0.0.1 %1")
+                     .arg(imap.serverPort())
+                     .toUtf8());
+    client.ReadUntil("Loaded");
+
+    auto client2 = ConnectClient();
+    client2.Write("PASS foo:hunter2");
+    client2.Write("NICK foo");
+    client2.Write("USER foo x x :x");
+
+    ASSERT_TRUE(imap.waitForNewConnection(30000 /* msec */));
+    auto imapsock = WrapIO(imap.nextPendingConnection());
+    imapsock.Write("* OK IMAP4rev1 Service Ready");
+    imapsock.ReadUntil("AUTH LOGIN \"foo\" \"hunter2\"");
+    imapsock.Write("AUTH OK");
+
+    client2.ReadUntil(":irc.znc.in 464 foo :Your host [");
+    client2.ReadUntil("] is not allowed");
+}
+
 TEST_F(ZNCTest, SaslAuthExternal) {
 #ifndef HAVE_LIBSSL
     GTEST_SKIP() << "SSL is disabled";