From 58ad244af538d6ed7019e18e9678042f2d1886d4 Mon Sep 17 00:00:00 2001
From: Dmitry Rodionov <d.rodionov@picodata.io>
Date: Tue, 19 Dec 2023 02:19:41 +0300
Subject: [PATCH] fix: include verdict in all auth_fail events

Cases when verdict was missing:
- unknown user
- user was blocked

Close: https://git.picodata.io/picodata/picodata/picodata/-/issues/438
---
 src/lib.rs             |  3 +++
 test/conftest.py       | 11 ++++++++---
 test/int/test_acl.py   |  3 +--
 test/int/test_audit.py | 34 ++++++++++++++++++++++++++++------
 4 files changed, 40 insertions(+), 11 deletions(-)

diff --git a/src/lib.rs b/src/lib.rs
index 6c1706dc23..6ac1677070 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -365,6 +365,7 @@ fn set_login_attempts_check(storage: Clusterwide) {
                         severity: High,
                         user: &user,
                         initiator: &user,
+                        verdict: "user is not blocked",
                     );
                 }
                 Verdict::AuthFail => {
@@ -374,6 +375,7 @@ fn set_login_attempts_check(storage: Clusterwide) {
                         severity: High,
                         user: &user,
                         initiator: &user,
+                        verdict: "user is not blocked",
                     );
                 }
                 Verdict::UnknownUser => {
@@ -383,6 +385,7 @@ fn set_login_attempts_check(storage: Clusterwide) {
                         severity: High,
                         user: &user,
                         initiator: &user,
+                        verdict: "user is not blocked",
                     );
                 }
                 Verdict::UserBlocked => {
diff --git a/test/conftest.py b/test/conftest.py
index 2b9132ae75..4892cdb18d 100644
--- a/test/conftest.py
+++ b/test/conftest.py
@@ -46,6 +46,8 @@ BASE_HOST = "127.0.0.1"
 BASE_PORT = 3300
 PORT_RANGE = 200
 
+MAX_LOGIN_ATTEMPTS = 4
+
 
 def eprint(*args, **kwargs):
     print(*args, file=sys.stderr, **kwargs)
@@ -730,6 +732,12 @@ class Instance:
             start_new_session=True,
         )
 
+        # Assert a new process group is created
+        assert os.getpgid(self.process.pid) == self.process.pid
+
+        if out == subprocess.DEVNULL:
+            return
+
         for src, out in [
             (self.process.stdout, sys.stdout),
             (self.process.stderr, sys.stderr),
@@ -740,9 +748,6 @@ class Instance:
                 daemon=True,
             ).start()
 
-        # Assert a new process group is created
-        assert os.getpgid(self.process.pid) == self.process.pid
-
     def fail_to_start(self, timeout: int = 5):
         assert self.process is None
         self.start()
diff --git a/test/int/test_acl.py b/test/int/test_acl.py
index 366d0d511b..2182fe67a3 100644
--- a/test/int/test_acl.py
+++ b/test/int/test_acl.py
@@ -1,11 +1,10 @@
 import pytest
-from conftest import Cluster, Instance, TarantoolError, ReturnError
+from conftest import MAX_LOGIN_ATTEMPTS, Cluster, Instance, TarantoolError, ReturnError
 from tarantool.error import NetworkError  # type: ignore
 from tarantool.connection import Connection  # type: ignore
 
 VALID_PASSWORD = "long enough"
 PASSWORD_MIN_LENGTH_KEY = "password_min_length"
-MAX_LOGIN_ATTEMPTS = 4
 
 
 def expected_min_password_violation_error(min_length: int):
diff --git a/test/int/test_audit.py b/test/int/test_audit.py
index 716d580243..c21d45d261 100644
--- a/test/int/test_audit.py
+++ b/test/int/test_audit.py
@@ -10,6 +10,7 @@ from tarantool.error import (  # type: ignore
 )
 
 from conftest import (
+    MAX_LOGIN_ATTEMPTS,
     Instance,
     Cluster,
     retrying,
@@ -101,6 +102,7 @@ class EventAuthOk(Event):
     TITLE: ClassVar[str] = "auth_ok"
     user: str
     initiator: str
+    verdict: str
 
 
 @dataclass
@@ -108,7 +110,7 @@ class EventAuthFail(Event):
     TITLE: ClassVar[str] = "auth_fail"
     user: str
     initiator: str
-    verdict: Optional[str] = None
+    verdict: str
 
 
 @dataclass
@@ -544,7 +546,7 @@ def test_auth(instance: Instance):
         pass
     events = audit.events()
 
-    with instance.connect(4, user="ymir", password="0123456789") as _:
+    with instance.connect(4, user="ymir", password="0123456789"):
         pass
 
     auth_ok = take_until_type(events, EventAuthOk)
@@ -552,15 +554,35 @@ def test_auth(instance: Instance):
     assert auth_ok.user == "ymir"
     assert auth_ok.severity == Severity.High
     assert auth_ok.initiator == "ymir"
-
-    with pytest.raises(NetworkError):
-        with instance.connect(4, user="ymir", password="wrong_pwd") as _:
+    assert auth_ok.verdict == "user is not blocked"
+
+    for _ in range(MAX_LOGIN_ATTEMPTS):
+        with pytest.raises(
+            NetworkError, match="User not found or supplied credentials are invalid"
+        ):
+            with instance.connect(4, user="ymir", password="wrong_pwd"):
+                pass
+
+        auth_fail = take_until_type(events, EventAuthFail)
+        assert auth_fail is not None
+        assert auth_fail.message == "failed to authenticate user `ymir`"
+        assert auth_fail.severity == Severity.High
+        assert auth_fail.verdict == "user is not blocked"
+        assert auth_fail.user == "ymir"
+        assert auth_fail.initiator == "ymir"
+
+    with pytest.raises(NetworkError, match="Maximum number of login attempts exceeded"):
+        with instance.connect(4, user="ymir", password="wrong_pwd"):
             pass
 
     auth_fail = take_until_type(events, EventAuthFail)
     assert auth_fail is not None
-    assert auth_fail.user == "ymir"
+    assert auth_fail.message == "failed to authenticate user `ymir`"
     assert auth_fail.severity == Severity.High
+    assert auth_fail.verdict == (
+        "Maximum number of login attempts exceeded; user will be blocked indefinitely"
+    )
+    assert auth_fail.user == "ymir"
     assert auth_fail.initiator == "ymir"
 
 
-- 
GitLab