From c0f991575e08400a0accbccc4dc4018dfde70e9b Mon Sep 17 00:00:00 2001
From: Andrew Walker <awalker@ixsystems.com>
Date: Wed, 3 Jul 2024 08:08:37 -0700
Subject: [PATCH 1/2] Fix schema for directory services users

We impose more restrictive validation on our local usernames than
NSS on Linux imposes. nss_winbind for instance uses \ as a separator
for domain name and username to prevent collisions with local users.

This commit fixes our validator for UserEntry but maintains for
UserCreate and fixes various minor issues with schema returned by
directory services cache.
---
 .../middlewared/api/base/types/user.py        | 41 ++++++++++++++-----
 .../middlewared/api/v25_04_0/user.py          | 10 ++---
 .../plugins/directoryservices_/util_cache.py  |  7 ++--
 src/middlewared/middlewared/plugins/idmap.py  |  6 +--
 4 files changed, 42 insertions(+), 22 deletions(-)

diff --git a/src/middlewared/middlewared/api/base/types/user.py b/src/middlewared/middlewared/api/base/types/user.py
index afa9d5cd3f453..1592778786d21 100644
--- a/src/middlewared/middlewared/api/base/types/user.py
+++ b/src/middlewared/middlewared/api/base/types/user.py
@@ -4,26 +4,47 @@
 from pydantic.functional_validators import AfterValidator
 from typing_extensions import Annotated
 
-__all__ = ["LocalUsername", "LocalUID"]
+__all__ = ["LocalUsername", "RemoteUsername", "LocalUID"]
 
 TRUENAS_IDMAP_DEFAULT_LOW = 90000001
 
+DEFAULT_VALID_CHARS = string.ascii_letters + string.digits + '_' + '-' + '$' + '.'
+DEFAULT_VALID_START = string.ascii_letters + '_'
+DEFAULT_MAX_LENGTH = 32
 
-def validate_local_username(val):
-    # see man 8 useradd, specifically the CAVEATS section
-    # NOTE: we are ignoring the man page's recommendation for insistence
-    # upon the starting character of a username be a lower-case letter.
-    # We aren't enforcing this for maximum backwards compatibility
+
+def validate_username(
+    val: str,
+    valid_chars: str = DEFAULT_VALID_CHARS,
+    valid_start_chars : str | None = DEFAULT_VALID_START,
+    max_length: int | None = DEFAULT_MAX_LENGTH
+):
     val_len = len(val)
-    valid_chars = string.ascii_letters + string.digits + '_' + '-' + '$' + '.'
-    valid_start = string.ascii_letters + '_'
     assert val_len > 0, 'Username must be at least 1 character in length'
-    assert val_len <= 32, 'Username cannot exceed 32 characters in length'
-    assert val[0] in valid_start, 'Username must start with a letter or an underscore'
+    if max_length is not None:
+        assert val_len <= max_length, f'Username cannot exceed {max_length} charaters in length'
+    if valid_start_chars is not None:
+        assert val[0] in valid_start_chars, 'Username must start with a letter or an underscore'
+
     assert '$' not in val or val[-1] == '$', 'Username must end with a dollar sign character'
     assert all(char in valid_chars for char in val), f'Valid characters for a username are: {", ".join(valid_chars)!r}'
     return val
 
 
+def validate_local_username(val):
+    # see man 8 useradd, specifically the CAVEATS section
+    # NOTE: we are ignoring the man page's recommendation for insistence
+    # upon the starting character of a username be a lower-case letter.
+    # We aren't enforcing this for maximum backwards compatibility
+    return validate_username(val)
+
+
+def validate_remote_username(val):
+    # Restrictions on names returned by nss_winbind are more lax than we place
+    # on our local usernames. \\ is used as a separator for domain and username
+    return validate_username(val, DEFAULT_VALID_CHARS + '\\', None, None)
+
+
 LocalUsername = Annotated[str, AfterValidator(validate_local_username)]
+RemoteUsername = Annotated[str, AfterValidator(validate_remote_username)]
 LocalUID = Annotated[int, Ge(0), Le(TRUENAS_IDMAP_DEFAULT_LOW - 1)]
diff --git a/src/middlewared/middlewared/api/v25_04_0/user.py b/src/middlewared/middlewared/api/v25_04_0/user.py
index b45168c617d6d..2da4fb7aff5bb 100644
--- a/src/middlewared/middlewared/api/v25_04_0/user.py
+++ b/src/middlewared/middlewared/api/v25_04_0/user.py
@@ -2,8 +2,8 @@
 from pydantic import EmailStr
 from typing_extensions import Annotated
 
-from middlewared.api.base import (BaseModel, Excluded, excluded_field, ForUpdateMetaclass, LocalUsername, LocalUID,
-                                  LongString, NonEmptyString, Private, single_argument_result)
+from middlewared.api.base import (BaseModel, Excluded, excluded_field, ForUpdateMetaclass, LocalUsername, RemoteUsername,
+                                  LocalUID, LongString, NonEmptyString, Private, single_argument_result)
 
 __all__ = ["UserEntry", "UserCreateArgs", "UserCreateResult", "UserUpdateArgs", "UserUpdateResult",
            "UserRenew2faSecretArgs", "UserRenew2faSecretResult"]
@@ -15,9 +15,9 @@
 class UserEntry(BaseModel):
     id: int
     uid: int
-    username: LocalUsername
-    unixhash: Private[str]
-    smbhash: Private[str]
+    username: LocalUsername | RemoteUsername
+    unixhash: Private[str | None]
+    smbhash: Private[str | None]
     home: NonEmptyString = DEFAULT_HOME_PATH
     shell: NonEmptyString = "/usr/bin/zsh"
     "Available choices can be retrieved with `user.shell_choices`."
diff --git a/src/middlewared/middlewared/plugins/directoryservices_/util_cache.py b/src/middlewared/middlewared/plugins/directoryservices_/util_cache.py
index 6bcbeea5050f4..b5f7d34f62910 100644
--- a/src/middlewared/middlewared/plugins/directoryservices_/util_cache.py
+++ b/src/middlewared/middlewared/plugins/directoryservices_/util_cache.py
@@ -253,16 +253,15 @@ def fill_cache(
                     'shell': user_data.pw_shell,
                     'full_name': user_data.pw_gecos,
                     'builtin': False,
-                    'email': '',
+                    'email': None,
                     'password_disabled': False,
                     'locked': False,
                     'sudo_commands': [],
-                    'sudo_commands_nopasswd': False,
-                    'attributes': {},
+                    'sudo_commands_nopasswd': [],
                     'groups': [],
                     'sshpubkey': None,
                     'immutable': True,
-                    'two_factor_auth_configured': False,
+                    'twofactor_auth_configured': False,
                     'local': False,
                     'id_type_both': id_type_both,
                     'nt_name': user_data.pw_name,
diff --git a/src/middlewared/middlewared/plugins/idmap.py b/src/middlewared/middlewared/plugins/idmap.py
index 1c78ccfa3c3af..03f5e985132d3 100644
--- a/src/middlewared/middlewared/plugins/idmap.py
+++ b/src/middlewared/middlewared/plugins/idmap.py
@@ -1174,11 +1174,11 @@ async def synthetic_user(self, passwd, sid):
             'unixhash': None,
             'smbhash': None,
             'group': {},
-            'home': '',
-            'shell': '',
+            'home': passwd['pw_dir'],
+            'shell': passwd['pw_shell'],
             'full_name': passwd['pw_gecos'],
             'builtin': False,
-            'email': '',
+            'email': None,
             'password_disabled': False,
             'locked': False,
             'sudo_commands': [],

From f27cfd3f4b998d6b7927b95579f053a7697fe6bd Mon Sep 17 00:00:00 2001
From: Andrew Walker <awalker@ixsystems.com>
Date: Wed, 3 Jul 2024 09:11:46 -0700
Subject: [PATCH 2/2] Add annotations

---
 src/middlewared/middlewared/api/base/types/user.py | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/middlewared/middlewared/api/base/types/user.py b/src/middlewared/middlewared/api/base/types/user.py
index 1592778786d21..eda72977f1a9e 100644
--- a/src/middlewared/middlewared/api/base/types/user.py
+++ b/src/middlewared/middlewared/api/base/types/user.py
@@ -18,7 +18,7 @@ def validate_username(
     valid_chars: str = DEFAULT_VALID_CHARS,
     valid_start_chars : str | None = DEFAULT_VALID_START,
     max_length: int | None = DEFAULT_MAX_LENGTH
-):
+) -> str:
     val_len = len(val)
     assert val_len > 0, 'Username must be at least 1 character in length'
     if max_length is not None:
@@ -31,7 +31,7 @@ def validate_username(
     return val
 
 
-def validate_local_username(val):
+def validate_local_username(val: str) -> str:
     # see man 8 useradd, specifically the CAVEATS section
     # NOTE: we are ignoring the man page's recommendation for insistence
     # upon the starting character of a username be a lower-case letter.
@@ -39,7 +39,7 @@ def validate_local_username(val):
     return validate_username(val)
 
 
-def validate_remote_username(val):
+def validate_remote_username(val: str) -> str:
     # Restrictions on names returned by nss_winbind are more lax than we place
     # on our local usernames. \\ is used as a separator for domain and username
     return validate_username(val, DEFAULT_VALID_CHARS + '\\', None, None)