fix: used to panic when checking login attempts via underprivileged user

src/access_control.rs

@@ -31,6 +31,7 @@ use std::{
     fmt::Display,
 };
 
+use tarantool::index::Index;
 use tarantool::{
     access_control::{
         box_access_check_ddl, box_access_check_space, PrivType,
@@ -117,6 +118,23 @@ pub fn user_by_id(id: UserId) -> tarantool::Result<UserMetadata> {
     }
 }
 
+pub fn user_by_name(name: &str) -> tarantool::Result<UserMetadata> {
+    const USER_INDEX_NAME: u32 = 2;
+    // SAFETY: check `box.space._user.index.name.id == 2`
+    let index_user_by_name =
+        unsafe { Index::from_ids_unchecked(SystemSpace::User as _, USER_INDEX_NAME) };
+
+    let Some(tuple) = index_user_by_name.get(&(name,))? else {
+        tarantool::set_error!(
+            tarantool::error::TarantoolErrorCode::NoSuchUser,
+            "no such user '{name}'",
+        );
+        return Err(tarantool::error::TarantoolError::last().into());
+    };
+
+    tuple.decode()
+}
+
 /// There are no cases when box_access_check_ddl is called several times
 /// in a row so it is ok that we need to switch once to user who initiated the request
 /// This wrapper is needed because usually before checking permissions we need to

src/lib.rs

@@ -20,6 +20,7 @@ use storage::Clusterwide;
 use traft::RaftSpaceAccess;
 
 use crate::access_control::user_by_id;
+use crate::access_control::user_by_name;
 use crate::cli::args;
 use crate::cli::args::Address;
 use crate::cli::init_cfg::InitCfg;
@@ -299,26 +300,27 @@ fn set_login_attempts_check(storage: Clusterwide) {
     }
 
     // Determines the outcome of an authentication attempt.
-    let compute_auth_verdict = move |user: String, status: bool| {
+    let compute_auth_verdict = move |user: String, successful_authentication: bool| {
         use std::collections::hash_map::Entry;
 
         let max_login_attempts = || {
-            storage
-                .properties
-                .max_login_attempts()
-                .expect("accessing storage should not fail")
+            session::with_su(ADMIN_ID, || {
+                storage
+                    .properties
+                    .max_login_attempts()
+                    .expect("accessing storage should not fail")
+            })
+            .expect("switching user to admin shouldn't fail")
         };
 
-        let user_exists = storage
-            .users
-            .by_name(&user)
-            .expect("accessing storage should not fail")
-            .is_some();
+        let user_exists = session::with_su(ADMIN_ID, || user_by_name(&user).is_ok())
+            .expect("switching user to admin shouldn't fail");
 
         // Prevent DOS attacks by first checking whether the user exists.
         // If it doesn't, we shouldn't even bother tracking its attempts.
         // Too many hashmap records will cause a global OOM event.
         if !user_exists {
+            debug_assert!(!successful_authentication);
             return Verdict::UnknownUser;
         }
 
@@ -333,7 +335,7 @@ fn set_login_attempts_check(storage: Clusterwide) {
                 Verdict::UserBlocked
             }
             Entry::Occupied(mut e) => {
-                if status {
+                if successful_authentication {
                     // Forget about previous failures
                     e.remove();
                     Verdict::AuthOk
@@ -343,7 +345,7 @@ fn set_login_attempts_check(storage: Clusterwide) {
                 }
             }
             Entry::Vacant(e) => {
-                if status {
+                if successful_authentication {
                     Verdict::AuthOk
                 } else {
                     // Remember the failure, but don't raise an error yet