netcmd: models: Rename username to account_name for consistency

When creating the User model initially, "username" was the only field that was inconsistently named, it maps to "sAMAccountName".

It should really have been account "account_name".

There is also a field "account_type" and should be similarly named to "account_name".

Basically the naming of fields should always be consistent, breaking the rule for one field only was a mistake.

Signed-off-by: Rob van der Linde <rob@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>

python/samba/netcmd/domain/models/computer.py

@@ -31,19 +31,19 @@ class Computer(User):
     """A Computer is a type of User."""
 
     def __init__(self, **kwargs):
-        """Computer constructor automatically adds "$" to username.
+        """Computer constructor automatically adds "$" to account_name.
 
         Also applies to GroupManagedServiceAccount subclass.
         """
         name = kwargs.get("name", kwargs.get("cn"))
-        username = kwargs.get("username")
+        account_name = kwargs.get("account_name")
 
-        # If the username is missing, use name or cn and add a "$".
+        # If account_name is missing, use name or cn and add a "$".
-        # If the username is present but lacking "$", add it automatically.
+        # If account_name is present but lacking "$", add it automatically.
-        if name and not username:
+        if name and not account_name:
-            kwargs["username"] = name + "$"
+            kwargs["account_name"] = name + "$"
-        elif username and not username.endswith("$"):
+        elif account_name and not account_name.endswith("$"):
-            kwargs["username"] = username + "$"
+            kwargs["account_name"] = account_name + "$"
 
         super().__init__(**kwargs)
 
@@ -63,7 +63,7 @@ class Computer(User):
 
     @classmethod
     def find(cls, ldb, name):
-        """Helper function to find a computer, first by Dn then username.
+        """Helper function to find a computer, first by Dn then sAMAccountName.
 
         If the Dn can't be parsed use sAMAccountName, automatically add the $.
         """
@@ -71,8 +71,8 @@ class Computer(User):
             query = {"dn": Dn(ldb, name)}
         except ValueError:
             if name.endswith("$"):
-                query = {"username": name}
+                query = {"account_name": name}
             else:
-                query = {"username": name + "$"}
+                query = {"account_name": name + "$"}
 
         return cls.get(ldb, **query)

python/samba/netcmd/domain/models/user.py

@@ -30,7 +30,7 @@ from .types import AccountType, UserAccountControl
 
 
 class User(OrganizationalPerson):
-    username = StringField("sAMAccountName")
+    account_name = StringField("sAMAccountName")
     account_type = EnumField("sAMAccountType", AccountType)
     assigned_policy = DnField("msDS-AssignedAuthNPolicy")
     assigned_silo = DnField("msDS-AssignedAuthNPolicySilo")
@@ -47,8 +47,8 @@ class User(OrganizationalPerson):
     user_principal_name = StringField("userPrincipalName")
 
     def __str__(self):
-        """Return username rather than cn for User model."""
+        """Return sAMAccountName rather than cn for User model."""
-        return self.username
+        return self.account_name
 
     @staticmethod
     def get_base_dn(ldb):
@@ -75,13 +75,13 @@ class User(OrganizationalPerson):
 
     @classmethod
     def find(cls, ldb, name):
-        """Helper function to find a user first by Dn then username.
+        """Helper function to find a user first by Dn then sAMAccountName.
 
         If the Dn can't be parsed, use sAMAccountName instead.
         """
         try:
             query = {"dn": Dn(ldb, name)}
         except ValueError:
-            query = {"username": name}
+            query = {"account_name": name}
 
         return cls.get(ldb, **query)

python/samba/netcmd/service_account/service_account.py

@@ -55,10 +55,10 @@ class cmd_service_account_list(Command):
             raise CommandError(e)
 
         if output_format == "json":
-            self.print_json({account.username: account for account in accounts})
+            self.print_json({account.account_name: account for account in accounts})
         else:
             for account in accounts:
-                print(account.username, file=self.outf)
+                print(account.account_name, file=self.outf)
 
 
 class cmd_service_account_view(Command):

python/samba/tests/samba_tool/domain_models.py

@@ -81,17 +81,17 @@ class ComputerModelTests(SambaToolCmdTest):
 
     def test_computer_constructor(self):
         comp1 = Computer(name="comp1")
-        self.assertEqual(comp1.username, "comp1$")
+        self.assertEqual(comp1.account_name, "comp1$")
 
         comp2 = Computer(cn="comp2")
-        self.assertEqual(comp2.username, "comp2$")
+        self.assertEqual(comp2.account_name, "comp2$")
 
         # User accidentally left out '$' in username.
         comp3 = Computer(name="comp3", username="comp3")
-        self.assertEqual(comp3.username, "comp3$")
+        self.assertEqual(comp3.account_name, "comp3$")
 
         comp4 = Computer(cn="comp4", username="comp4$")
-        self.assertEqual(comp4.username, "comp4$")
+        self.assertEqual(comp4.account_name, "comp4$")
 
 
 class FieldTestMixin:
@@ -261,8 +261,8 @@ class RelatedFieldTest(FieldTestMixin, SambaToolCmdTest):
 
     @property
     def to_db_value(self):
-        alice = User.get(self.samdb, username="alice")
+        alice = User.get(self.samdb, account_name="alice")
-        joe = User.get(self.samdb, username="joe")
+        joe = User.get(self.samdb, account_name="joe")
         return [
             (alice, MessageElement(str(alice.dn))),
             ([joe, alice], MessageElement([str(joe.dn), str(alice.dn)])),
@@ -271,8 +271,8 @@ class RelatedFieldTest(FieldTestMixin, SambaToolCmdTest):
 
     @property
     def from_db_value(self):
-        alice = User.get(self.samdb, username="alice")
+        alice = User.get(self.samdb, account_name="alice")
-        joe = User.get(self.samdb, username="joe")
+        joe = User.get(self.samdb, account_name="joe")
         return [
             (MessageElement(str(alice.dn)), alice),
             (MessageElement([str(joe.dn), str(alice.dn)]), [joe, alice]),
@@ -285,8 +285,8 @@ class DnFieldTest(FieldTestMixin, SambaToolCmdTest):
 
     @property
     def to_db_value(self):
-        alice = User.get(self.samdb, username="alice")
+        alice = User.get(self.samdb, account_name="alice")
-        joe = User.get(self.samdb, username="joe")
+        joe = User.get(self.samdb, account_name="joe")
         return [
             (alice.dn, MessageElement(str(alice.dn))),
             ([joe.dn, alice.dn], MessageElement([str(joe.dn), str(alice.dn)])),
@@ -295,8 +295,8 @@ class DnFieldTest(FieldTestMixin, SambaToolCmdTest):
 
     @property
     def from_db_value(self):
-        alice = User.get(self.samdb, username="alice")
+        alice = User.get(self.samdb, account_name="alice")
-        joe = User.get(self.samdb, username="joe")
+        joe = User.get(self.samdb, account_name="joe")
         return [
             (MessageElement(str(alice.dn)), alice.dn),
             (MessageElement([str(joe.dn), str(alice.dn)]), [joe.dn, alice.dn]),

python/samba/tests/samba_tool/service_account.py

@@ -124,9 +124,9 @@ class ServiceAccountTests(SambaToolCmdTest):
 
         # Group Managed Service count exists.
         # Since GroupManagedServiceAccount is also a Computer it ends in '$'
-        gmsa = GroupManagedServiceAccount.get(self.samdb, username=name + "$")
+        gmsa = GroupManagedServiceAccount.get(self.samdb, account_name=name + "$")
         self.assertIsNotNone(gmsa)
-        self.assertEqual(gmsa.username, name + "$")
+        self.assertEqual(gmsa.account_name, name + "$")
         self.assertEqual(gmsa.dns_host_name, "test.com")
         self.assertEqual(gmsa.managed_password_interval, 60)
 
@@ -150,7 +150,7 @@ class ServiceAccountTests(SambaToolCmdTest):
                                           dns_host_name="example.com"),
 
         # The group managed service account exists.
-        gmsa = GroupManagedServiceAccount.get(self.samdb, username=name + "$")
+        gmsa = GroupManagedServiceAccount.get(self.samdb, account_name=name + "$")
         self.assertIsNotNone(gmsa)
 
         # Now delete the gmsa.
@@ -159,7 +159,7 @@ class ServiceAccountTests(SambaToolCmdTest):
         self.assertIsNone(result, msg=err)
 
         # Service account is gone.
-        gmsa = GroupManagedServiceAccount.get(self.samdb, username=name + "$")
+        gmsa = GroupManagedServiceAccount.get(self.samdb, account_name=name + "$")
         self.assertIsNone(gmsa, msg="Group Managed Service Account not deleted.")
 
     def test_modify(self):
@@ -170,7 +170,7 @@ class ServiceAccountTests(SambaToolCmdTest):
         self.addCleanup(gmsa.delete, self.samdb)
 
         # Build some SDDL for adding a user manually.
-        bob = User.get(self.samdb, username="bob")
+        bob = User.get(self.samdb, account_name="bob")
         sddl = gmsa.group_msa_membership.as_sddl()
         sddl += f"(A;;CCDCLCSWRPWPDTLOCRSDRCWDWO;;;{bob.object_sid})"
 
@@ -181,7 +181,7 @@ class ServiceAccountTests(SambaToolCmdTest):
         self.assertIsNone(result, msg=err)
 
         # Check field changes and see if the new user is in there.
-        gmsa = GroupManagedServiceAccount.get(self.samdb, username=name + "$")
+        gmsa = GroupManagedServiceAccount.get(self.samdb, account_name=name + "$")
         self.assertEqual(gmsa.dns_host_name, "new.example.com")
         self.assertIn(bob.object_sid, gmsa.trustees)
 
@@ -197,7 +197,7 @@ class ServiceAccountGroupMSAMembershipTests(SambaToolCmdTest):
     def setUpTestData(cls):
         """Setup initial data without the samba-tool command."""
         # Add a user other than the Administrator to the default SDDL.
-        jane = User.get(cls.samdb, username="jane")
+        jane = User.get(cls.samdb, account_name="jane")
         sddl = f"{GROUP_MSA_MEMBERSHIP_DEFAULT}(A;;CCDCLCSWRPWPDTLOCRSDRCWDWO;;;{jane.object_sid})"
         cls.gmsa = GroupManagedServiceAccount.create(cls.samdb, name="gmsa",
                                                      dns_host_name="example.com",
@@ -219,7 +219,7 @@ class ServiceAccountGroupMSAMembershipTests(SambaToolCmdTest):
         """Show password viewers on a Group Managed Service Account."""
         result, out, err = self.runcmd("service-account",
                                        "group-msa-membership", "show",
-                                       "--name", self.gmsa.username)
+                                       "--name", self.gmsa.account_name)
         self.assertIsNone(result, msg=err)
 
         # Plain text output.
@@ -234,7 +234,7 @@ class ServiceAccountGroupMSAMembershipTests(SambaToolCmdTest):
         """Show password viewers on a Group Managed Service Account as JSON."""
         result, out, err = self.runcmd("service-account",
                                        "group-msa-membership", "show",
-                                       "--name", self.gmsa.username,
+                                       "--name", self.gmsa.account_name,
                                        "--json")
         self.assertIsNone(result, msg=err)
 
@@ -248,7 +248,7 @@ class ServiceAccountGroupMSAMembershipTests(SambaToolCmdTest):
 
     def test_add__username(self):
         """Add principal to a Group Managed Service Account by username."""
-        alice = User.get(self.samdb, username="alice")
+        alice = User.get(self.samdb, account_name="alice")
         name = self.unique_name()
         gmsa = GroupManagedServiceAccount.create(self.samdb, name=name,
                                                  dns_host_name="example.com")
@@ -257,8 +257,8 @@ class ServiceAccountGroupMSAMembershipTests(SambaToolCmdTest):
         # Add user 'alice' by username.
         result, out, err = self.runcmd("service-account",
                                        "group-msa-membership", "add",
-                                       "--name", gmsa.username,
+                                       "--name", gmsa.account_name,
-                                       "--principal", alice.username)
+                                       "--principal", alice.account_name)
         self.assertIsNone(result, msg=err)
 
         # See if user was added.
@@ -276,7 +276,7 @@ class ServiceAccountGroupMSAMembershipTests(SambaToolCmdTest):
         # Add group 'DnsAdmins' by dn.
         result, out, err = self.runcmd("service-account",
                                        "group-msa-membership", "add",
-                                       "--name", gmsa.username,
+                                       "--name", gmsa.account_name,
                                        "--principal", str(admins.dn))
         self.assertIsNone(result, msg=err)
 
@@ -287,7 +287,7 @@ class ServiceAccountGroupMSAMembershipTests(SambaToolCmdTest):
     def test_remove__username(self):
         """Remove principal from a Group Managed Service Account by username."""
         # Create a GMSA with custom SDDL and add extra user.
-        bob = User.get(self.samdb, username="bob")
+        bob = User.get(self.samdb, account_name="bob")
         sddl = f"{GROUP_MSA_MEMBERSHIP_DEFAULT}(A;;CCDCLCSWRPWPDTLOCRSDRCWDWO;;;{bob.object_sid})"
         name = self.unique_name()
         gmsa = GroupManagedServiceAccount.create(self.samdb, name=name,
@@ -300,8 +300,8 @@ class ServiceAccountGroupMSAMembershipTests(SambaToolCmdTest):
         # Remove user 'bob' by username.
         result, out, err = self.runcmd("service-account",
                                        "group-msa-membership", "remove",
-                                       "--name", gmsa.username,
+                                       "--name", gmsa.account_name,
-                                       "--principal", bob.username)
+                                       "--principal", bob.account_name)
         self.assertIsNone(result, msg=err)
 
         # See if user was removed.
@@ -324,7 +324,7 @@ class ServiceAccountGroupMSAMembershipTests(SambaToolCmdTest):
         # Remove group 'DnsAdmins' by dn.
         result, out, err = self.runcmd("service-account",
                                        "group-msa-membership", "remove",
-                                       "--name", gmsa.username,
+                                       "--name", gmsa.account_name,
                                        "--principal", str(admins.dn))
         self.assertIsNone(result, msg=err)
 

python/samba/tests/samba_tool/user_auth_policy.py

@@ -34,7 +34,7 @@ class AuthPolicyCmdTestCase(SiloTest):
         self.assertIsNone(result, msg=err)
 
         # Assigned policy should be 'Developers'
-        user = User.get(self.samdb, username="alice")
+        user = User.get(self.samdb, account_name="alice")
         policy = AuthenticationPolicy.get(self.samdb, dn=user.assigned_policy)
         self.assertEqual(policy.name, "User Policy")
 
@@ -52,7 +52,7 @@ class AuthPolicyCmdTestCase(SiloTest):
                     "User Policy")
 
         # Assigned policy should be set
-        user = User.get(self.samdb, username="bob")
+        user = User.get(self.samdb, account_name="bob")
         self.assertIsNotNone(user.assigned_policy)
 
         # Now try removing it
@@ -61,7 +61,7 @@ class AuthPolicyCmdTestCase(SiloTest):
         self.assertIsNone(result, msg=err)
 
         # Assigned policy should be None
-        user = User.get(self.samdb, username="bob")
+        user = User.get(self.samdb, account_name="bob")
         self.assertIsNone(user.assigned_policy)
 
     def test_view(self):

python/samba/tests/samba_tool/user_auth_silo.py

@@ -34,7 +34,7 @@ class AuthPolicyCmdTestCase(SiloTest):
         self.assertIsNone(result, msg=err)
 
         # Assigned silo should be 'Developers'
-        user = User.get(self.samdb, username="alice")
+        user = User.get(self.samdb, account_name="alice")
         silo = AuthenticationSilo.get(self.samdb, dn=user.assigned_silo)
         self.assertEqual(silo.name, "Developers")
 
@@ -51,7 +51,7 @@ class AuthPolicyCmdTestCase(SiloTest):
         self.runcmd("user", "auth", "silo", "assign", "bob", "--silo", "QA")
 
         # Assigned silo should be set
-        user = User.get(self.samdb, username="bob")
+        user = User.get(self.samdb, account_name="bob")
         self.assertIsNotNone(user.assigned_silo)
 
         # Now try removing it
@@ -60,7 +60,7 @@ class AuthPolicyCmdTestCase(SiloTest):
         self.assertIsNone(result, msg=err)
 
         # Assigned silo should be None
-        user = User.get(self.samdb, username="bob")
+        user = User.get(self.samdb, account_name="bob")
         self.assertIsNone(user.assigned_silo)
 
     def test_view(self):

python/samba/tests/samba_tool/user_get_kerberos_ticket.py

@@ -114,8 +114,8 @@ class GetKerberosTicketTest(BlackboxTestCase):
         cls.samdb.add(user_details)
         cls.addClassCleanup(delete_force, cls.samdb, cls.user_dn)
 
-        cls.gmsa_user = User.get(cls.samdb, username=cls.gmsa_username)
+        cls.gmsa_user = User.get(cls.samdb, account_name=cls.gmsa_username)
-        cls.user = User.get(cls.samdb, username=cls.username)
+        cls.user = User.get(cls.samdb, account_name=cls.username)
 
     def get_ticket(self, username, options=None):
         if options is None:

python/samba/tests/samba_tool/user_getpassword_gmsa.py

@@ -88,7 +88,7 @@ class GMSAPasswordTest(BlackboxTestCase):
         cls.samdb.add(details)
         cls.addClassCleanup(delete_force, cls.samdb, cls.user_dn)
 
-        cls.user = User.get(cls.samdb, username=cls.username)
+        cls.user = User.get(cls.samdb, account_name=cls.username)
 
     def getpassword(self, attrs):
         shattrs = shlex.quote(attrs)