Merge ec68807 into 8de259d

Author: kungasc

ydb/core/tx/schemeshard/schemeshard__operation_modify_acl.cpp

@@ -51,6 +51,24 @@ class TModifyACL: public TSubOperationBase {
             return result;
         }
 
+        if (acl) {
+            NACLib::TDiffACL diffACL(acl);
+            for (const NACLibProto::TDiffACE& diffACE : diffACL.GetDiffACE()) {
+                if (static_cast<NACLib::EDiffType>(diffACE.GetDiffType()) == NACLib::EDiffType::Add) {
+                    if (!context.SS->LoginProvider.CheckSidExistsOrIsNonYdb(diffACE.GetACE().GetSID())) {
+                        result->SetError(NKikimrScheme::StatusPreconditionFailed, "SID not found");
+                        return result;
+                    }
+                } // remove diff type is allowed in any case
+            }
+        }
+        if (owner) {
+            if (!context.SS->LoginProvider.CheckSidExistsOrIsNonYdb(owner)) {
+                result->SetError(NKikimrScheme::StatusPreconditionFailed, "Owner SID not found");
+                return result;
+            }
+        }
+
         THashSet<TPathId> subTree;
         if (acl || (owner && path.Base()->IsTable())) {
             subTree = context.SS->ListSubTree(path.Base()->PathId, context.Ctx);

ydb/core/tx/schemeshard/ut_login/ut_login.cpp

@@ -91,6 +91,7 @@ Y_UNIT_TEST_SUITE(TSchemeShardLoginTest) {
         TTestEnv env(runtime);
         ui64 txId = 100;
         CreateAlterLoginCreateUser(runtime, ++txId, "/MyRoot", "user1", "password1");
+        CreateAlterLoginCreateUser(runtime, ++txId, "/MyRoot", "user2", "password2");
         auto resultLogin = Login(runtime, "user1", "password1");
         UNIT_ASSERT_VALUES_EQUAL(resultLogin.error(), "");
 
@@ -150,6 +151,7 @@ Y_UNIT_TEST_SUITE(TSchemeShardLoginTest) {
         TTestEnv env(runtime);
         ui64 txId = 100;
         CreateAlterLoginCreateUser(runtime, ++txId, "/MyRoot", "user1", "password1");
+        CreateAlterLoginCreateUser(runtime, ++txId, "/MyRoot", "user2", "password2");
         auto resultLogin = Login(runtime, "user1", "password1");
         UNIT_ASSERT_VALUES_EQUAL(resultLogin.error(), "");
 
@@ -228,6 +230,7 @@ Y_UNIT_TEST_SUITE(TSchemeShardLoginTest) {
         TTestEnv env(runtime);
         ui64 txId = 100;
         CreateAlterLoginCreateUser(runtime, ++txId, "/MyRoot", "user1", "password1");
+        CreateAlterLoginCreateUser(runtime, ++txId, "/MyRoot", "user2", "password2");
         auto resultLogin = Login(runtime, "user1", "password1");
         UNIT_ASSERT_VALUES_EQUAL(resultLogin.error(), "");
 
@@ -268,6 +271,53 @@ Y_UNIT_TEST_SUITE(TSchemeShardLoginTest) {
         }
     }
 
+    Y_UNIT_TEST(AddAccess_NonExisting) {
+        TTestBasicRuntime runtime;
+        TTestEnv env(runtime);
+        ui64 txId = 100;
+
+        AsyncMkDir(runtime, ++txId, "/MyRoot", "Dir1");
+        TestModificationResult(runtime, txId, NKikimrScheme::StatusAccepted);
+
+        {
+            NACLib::TDiffACL diffACL;
+            diffACL.AddAccess(NACLib::EAccessType::Allow, NACLib::GenericUse, "user1");
+            AsyncModifyACL(runtime, ++txId, "/MyRoot", "Dir1", diffACL.SerializeAsString(), "");
+            TestModificationResults(runtime, txId, {{NKikimrScheme::StatusPreconditionFailed, "SID not found"}});
+        }
+
+        {
+            AsyncModifyACL(runtime, ++txId, "/MyRoot", "Dir1", NACLib::TDiffACL{}.SerializeAsString(), "user1");
+            TestModificationResults(runtime, txId, {{NKikimrScheme::StatusPreconditionFailed, "Owner SID not found"}});
+        }
+        
+        CreateAlterLoginCreateUser(runtime, ++txId, "/MyRoot", "user1", "password1");
+
+        TestDescribeResult(DescribePath(runtime, "/MyRoot/Dir1"),
+            {NLs::HasNoRight("+U:user1"), NLs::HasNoEffectiveRight("+U:user1"), NLs::HasOwner("root@builtin")});
+    }
+
+    Y_UNIT_TEST(AddAccess_NonYdb) {
+        TTestBasicRuntime runtime;
+        TTestEnv env(runtime);
+        ui64 txId = 100;
+
+        AsyncMkDir(runtime, ++txId, "/MyRoot", "Dir1");
+        TestModificationResult(runtime, txId, NKikimrScheme::StatusAccepted);
+
+        {
+            NACLib::TDiffACL diffACL;
+            diffACL.AddAccess(NACLib::EAccessType::Allow, NACLib::GenericUse, "user1@staff");
+            AsyncModifyACL(runtime, ++txId, "/MyRoot", "Dir1", diffACL.SerializeAsString(), "");
+            TestModificationResult(runtime, txId, NKikimrScheme::StatusSuccess);
+        }
+
+        {
+            AsyncModifyACL(runtime, ++txId, "/MyRoot", "Dir1", NACLib::TDiffACL{}.SerializeAsString(), "user1@staff");
+            TestModificationResult(runtime, txId, NKikimrScheme::StatusSuccess);
+        }
+    }
+
     Y_UNIT_TEST(DisableBuiltinAuthMechanism) {
         TTestBasicRuntime runtime;
         TTestEnv env(runtime);

ydb/library/login/login.cpp

@@ -92,6 +92,11 @@ bool TLoginProvider::CheckUserExists(const TString& user) {
     return CheckSubjectExists(user, ESidType::USER);
 }
 
+bool TLoginProvider::CheckSidExistsOrIsNonYdb(const TString& sid) {
+    // non-YDB user's sid format is <login>@<subsystem>
+    return sid.Contains('@') || Sids.contains(sid);
+}
+
 TLoginProvider::TBasicResponse TLoginProvider::ModifyUser(const TModifyUserRequest& request) {
     TBasicResponse response;
 

ydb/library/login/login.h

@@ -162,6 +162,7 @@ class TLoginProvider {
     TBasicResponse ModifyUser(const TModifyUserRequest& request);
     TRemoveUserResponse RemoveUser(const TString& user);
     bool CheckUserExists(const TString& user);
+    bool CheckSidExistsOrIsNonYdb(const TString& sid);
 
     TBasicResponse CreateGroup(const TCreateGroupRequest& request);
     TBasicResponse AddGroupMembership(const TAddGroupMembershipRequest& request);