From e6712751dd9df5f4a7f2531ee0069d3958cc3b3b Mon Sep 17 00:00:00 2001
From: Joseph Sutton <josephsutton@catalyst.net.nz>
Date: Tue, 10 May 2022 13:02:30 +1200
Subject: [PATCH] samdb: Avoid half-created accounts

If newuser() or newcomputer() create an account over LDAP, and an
attempt to modify it (e.g. to change the password) fails, ensure that we
properly clean up the account. If we are connected over LDAP, we won't
have transactions to clean things up for us.

Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
---
 python/samba/samdb.py                    | 45 ++++++++++++++++++------
 selftest/knownfail.d/samba-tool-add-user |  5 ++-
 2 files changed, 36 insertions(+), 14 deletions(-)

diff --git a/python/samba/samdb.py b/python/samba/samdb.py
index fe5e1c407e1..1cba3845fd3 100644
--- a/python/samba/samdb.py
+++ b/python/samba/samdb.py
@@ -55,6 +55,26 @@ class SamDB(samba.Ldb):
     hash_oid_name = {}
     hash_well_known = {}
 
+    class _CleanUpOnError:
+        def __init__(self, samdb, dn):
+            self.samdb = samdb
+            self.dn = dn
+
+        def __enter__(self):
+            pass
+
+        def __exit__(self, exc_type, exc_val, exc_tb):
+            if exc_type is not None:
+                # We failed to modify the account. If we connected to the
+                # database over LDAP, we don't have transactions, and so when
+                # we call transaction_cancel(), the account will still exist in
+                # a half-created state. We'll delete the account to ensure that
+                # doesn't happen.
+                self.samdb.delete(self.dn)
+
+            # Don't suppress any exceptions
+            return False
+
     def __init__(self, url=None, lp=None, modules_dir=None, session_info=None,
                  credentials=None, flags=ldb.FLG_DONT_CREATE_DB,
                  options=None, global_schema=True,
@@ -638,15 +658,17 @@ member: %s
         self.transaction_start()
         try:
             self.add(ldbmessage)
-            if ldbmessage2:
-                self.modify(ldbmessage2)
 
-            # Sets the password for it
-            if setpassword:
-                self.setpassword(("(distinguishedName=%s)" %
-                                  ldb.binary_encode(user_dn)),
-                                 password,
-                                 force_password_change_at_next_login_req)
+            with self._CleanUpOnError(self, user_dn):
+                if ldbmessage2:
+                    self.modify(ldbmessage2)
+
+                # Sets the password for it
+                if setpassword:
+                    self.setpassword(("(distinguishedName=%s)" %
+                                      ldb.binary_encode(user_dn)),
+                                     password,
+                                     force_password_change_at_next_login_req)
         except:
             self.transaction_cancel()
             raise
@@ -807,9 +829,10 @@ member: %s
 
             if prepare_oldjoin:
                 password = cn.lower()
-                self.setpassword(("(distinguishedName=%s)" %
-                                  ldb.binary_encode(computer_dn)),
-                                 password, False)
+                with self._CleanUpOnError(self, computer_dn):
+                    self.setpassword(("(distinguishedName=%s)" %
+                                      ldb.binary_encode(computer_dn)),
+                                     password, False)
         except:
             self.transaction_cancel()
             raise
diff --git a/selftest/knownfail.d/samba-tool-add-user b/selftest/knownfail.d/samba-tool-add-user
index 91d5d380cf8..e487281ed7d 100644
--- a/selftest/knownfail.d/samba-tool-add-user
+++ b/selftest/knownfail.d/samba-tool-add-user
@@ -1,3 +1,2 @@
-^samba.tests.samba_tool.user.samba.tests.samba_tool.user.UserCmdTestCase.test_newuser_weak_password.ad_dc_ntvfs:local
-^samba.tests.samba_tool.user.samba.tests.samba_tool.user.UserCmdTestCase.test_newuser_weak_password.ad_dc:local
-^samba.tests.samba_tool.user.samba.tests.samba_tool.user.UserCmdTestCase.test_newuser_weak_password.ad_dc_no_ntlm:local
+^samba.tests.samba_tool.user_check_password_script.samba.tests.samba_tool.user_check_password_script.UserCheckPwdTestCase.test_checkpassword_unacceptable.chgdcpass:local
+^samba.tests.samba_tool.user_check_password_script.samba.tests.samba_tool.user_check_password_script.UserCheckPwdTestCase.test_checkpassword_username.chgdcpass:local