[tabcmd] fix: CSVImport bugs (AUTH column, case-insensitive validation, username case)

CHANGELOG.md

@@ -5,6 +5,16 @@
   hierarchy path (e.g. `"Marketing/Q1 Reports"`). The walk is performed level by
   level using the REST API name filter, so a path with *n* components issues *n*
   requests. Returns the matching `ProjectItem` or `None` if no project is found.
+* **Behavior change**: `UserItem.CSVImport.create_user_from_line` no longer
+  lowercases the entire CSV line before parsing. Previously the whole line,
+  including the username, display name, fullname, and email fields, was
+  lowercased destructively (e.g. `JSmith` became `jsmith`). Case is now
+  preserved for those fields; only the comparison-relevant fields (license,
+  admin_level, publisher, auth_setting) are normalized internally for
+  validation. Callers relying on the previous lowercased output — e.g. dict
+  lookups keyed on `user.name`, or assertions against lowercased values —
+  need to update. This unblocks CSV imports for LDAP and other case-sensitive
+  auth backends where mixed-case usernames must be preserved.
 
 ## 0.18.0 (6 April 2022)    
 * Switched to using defused_xml for xml attack protection

tableauserverclient/models/user_item.py

@@ -308,7 +308,10 @@ def _set_values(
         if email:
             self.email = email
         if auth_setting:
-            self._auth_setting = auth_setting
+            # Go through the @property_is_enum(Auth) setter rather than writing
+            # to _auth_setting directly, so CSV-parsed users can't carry an
+            # invalid auth_setting that only fails later at the API call.
+            self.auth_setting = auth_setting
         if domain_name:
             self._domain_name = domain_name
         if locale:
@@ -432,36 +435,59 @@ class ColumnType(IntEnum):
             EMAIL = 6
             AUTH = 7
 
-            MAX = 7
+        # Total number of columns supported by the import format. Held outside
+        # the ColumnType enum so it can't be mistaken for a real column index.
+        COLUMN_COUNT = 8
+
+        # Lowercase -> canonical form mapping for the AUTH column. Class-level
+        # so the dict isn't rebuilt on every call to create_user_from_line /
+        # _validate_import_line_or_throw. The set of accepted values is derived
+        # from this map (see _valid_attributes[AUTH]) so there's a single
+        # source of truth.
+        _AUTH_CANONICAL: dict[str, str] = {
+            "saml": "SAML",
+            "openid": "OpenID",
+            "serverdefault": "ServerDefault",
+            "tableauidwithmfa": "TableauIDWithMFA",
+        }
 
         # Read a csv line and create a user item populated by the given attributes
         @staticmethod
         def create_user_from_line(line: str):
             if line is None or line is False or line == "\n" or line == "":
                 return None
-            line = line.strip().lower()
-            values: list[str] = list(map(str.strip, line.split(",")))
-            user = UserItem(values[UserItem.CSVImport.ColumnType.USERNAME])
+            values: list[str] = list(map(str.strip, line.strip().split(",")))
+            if len(values) > UserItem.CSVImport.COLUMN_COUNT:
+                raise ValueError("Too many attributes for user import")
+            username = values[UserItem.CSVImport.ColumnType.USERNAME]
+            user = UserItem(username)
             if len(values) > 1:
-                if len(values) > UserItem.CSVImport.ColumnType.MAX:
-                    raise ValueError("Too many attributes for user import")
-                while len(values) <= UserItem.CSVImport.ColumnType.MAX:
+                while len(values) < UserItem.CSVImport.COLUMN_COUNT:
                     values.append("")
                 site_role = UserItem.CSVImport._evaluate_site_role(
                     values[UserItem.CSVImport.ColumnType.LICENSE],
                     values[UserItem.CSVImport.ColumnType.ADMIN],
                     values[UserItem.CSVImport.ColumnType.PUBLISHER],
                 )
-
+                raw_auth = values[UserItem.CSVImport.ColumnType.AUTH]
+                if raw_auth:
+                    auth = UserItem.CSVImport._AUTH_CANONICAL.get(raw_auth.lower())
+                    if auth is None:
+                        raise ValueError(
+                            f"Unknown auth setting: {raw_auth!r}. "
+                            f"Valid values: {sorted(UserItem.CSVImport._AUTH_CANONICAL.values())}"
+                        )
+                else:
+                    auth = None
                 user._set_values(
                     None,
-                    values[UserItem.CSVImport.ColumnType.USERNAME],
+                    username,
                     site_role,
                     None,
                     None,
                     values[UserItem.CSVImport.ColumnType.DISPLAY_NAME],
                     values[UserItem.CSVImport.ColumnType.EMAIL],
-                    values[UserItem.CSVImport.ColumnType.AUTH],
+                    auth,
                     None,
                     None,
                     None,
@@ -493,6 +519,8 @@ def validate_file_for_import(csv_file: io.TextIOWrapper, logger) -> tuple[int, l
         # Iterate through each field and validate the given value against hardcoded constraints
         @staticmethod
         def _validate_import_line_or_throw(incoming, logger) -> None:
+            # AUTH column's valid set is derived from _AUTH_CANONICAL so there's
+            # one source of truth for the accepted values.
             _valid_attributes: list[list[str]] = [
                 [],
                 [],
@@ -501,20 +529,26 @@ def _validate_import_line_or_throw(incoming, logger) -> None:
                 ["system", "site", "none", "no"],  # admin
                 ["yes", "true", "1", "no", "false", "0"],  # publisher
                 [],
-                [UserItem.Auth.SAML, UserItem.Auth.OpenID, UserItem.Auth.ServerDefault],  # auth
+                list(UserItem.CSVImport._AUTH_CANONICAL.values()),  # auth — normalized before comparison
             ]
 
             line = list(map(str.strip, incoming.split(",")))
-            if len(line) > UserItem.CSVImport.ColumnType.MAX:
+            if len(line) > UserItem.CSVImport.COLUMN_COUNT:
                 raise AttributeError("Too many attributes in line")
             username = line[UserItem.CSVImport.ColumnType.USERNAME.value]
             logger.debug(f"> details - {username}")
             UserItem.validate_username_or_throw(username)
             for i in range(1, len(line)):
-                logger.debug(f"column {UserItem.CSVImport.ColumnType(i).name}: {line[i]}")
-                UserItem.CSVImport._validate_attribute_value(
-                    line[i], _valid_attributes[i], UserItem.CSVImport.ColumnType(i)
-                )
+                value = line[i]
+                valid = _valid_attributes[i]
+                # normalize case for fields with a restricted value set
+                if valid:
+                    if i == UserItem.CSVImport.ColumnType.AUTH:
+                        value = UserItem.CSVImport._AUTH_CANONICAL.get(value.lower(), value)
+                    else:
+                        value = value.lower()
+                logger.debug(f"column {UserItem.CSVImport.ColumnType(i).name}: {value}")
+                UserItem.CSVImport._validate_attribute_value(value, valid, UserItem.CSVImport.ColumnType(i))
 
         # Given a restricted set of possible values, confirm the item is in that set
         @staticmethod

test/test_user_model.py

@@ -136,3 +136,58 @@ def test_validate_usernames_file() -> None:
     test_data = _mock_file_content(usernames)
     valid, invalid = TSC.UserItem.CSVImport.validate_file_for_import(test_data, logger)
     assert valid == 5, f"Exactly 5 of the lines were valid, counted {valid + len(invalid)}"
+
+
+def test_validate_mixed_case_license() -> None:
+    # Regression: issue #1809 — 'Viewer' (capital V) was rejected by case-sensitive check
+    TSC.UserItem.CSVImport._validate_import_line_or_throw("username, pword, fname, Viewer, None, no, email", logger)
+    TSC.UserItem.CSVImport._validate_import_line_or_throw("username, pword, fname, Creator, Site, yes, email", logger)
+    TSC.UserItem.CSVImport._validate_import_line_or_throw("username, pword, fname, EXPLORER, NONE, YES, email", logger)
+
+
+def test_validate_tableauid_with_mfa_auth() -> None:
+    # TableauIDWithMFA is a valid auth value and must not be rejected
+    TSC.UserItem.CSVImport._validate_import_line_or_throw(
+        "username, pword, fname, creator, none, yes, email, TableauIDWithMFA", logger
+    )
+
+
+def test_create_user_preserves_username_case() -> None:
+    # Username must not be lowercased — case matters for LDAP and email-format usernames
+    user = TSC.UserItem.CSVImport.create_user_from_line("JSmith, pword, John Smith, creator, none, yes, j@example.com")
+    assert user is not None
+    assert user.name == "JSmith", f"Username was lowercased: {user.name}"
+
+
+def test_create_user_with_auth_column() -> None:
+    # AUTH column (position 7) must be parsed — was broken by MAX=7 off-by-one
+    user = TSC.UserItem.CSVImport.create_user_from_line("username, pword, fname, creator, none, yes, email, SAML")
+    assert user is not None
+    assert user.auth_setting == "SAML", f"Expected SAML, got {user.auth_setting}"
+
+
+def test_too_many_columns_raises() -> None:
+    with pytest.raises(ValueError):
+        TSC.UserItem.CSVImport.create_user_from_line("u, p, n, creator, none, yes, email, SAML, extra")
+
+
+def test_create_user_with_unknown_auth_raises() -> None:
+    # Unknown AUTH values must raise, not silently produce a UserItem with auth_setting=None.
+    # A caller can catch this if lenient behavior is wanted.
+    with pytest.raises(ValueError, match="Unknown auth setting"):
+        TSC.UserItem.CSVImport.create_user_from_line("username, pword, fname, creator, none, yes, email, NotAnAuthType")
+
+
+def test_create_user_with_lowercase_auth_accepted() -> None:
+    # AUTH values are canonicalized case-insensitively — 'saml' should produce 'SAML'.
+    user = TSC.UserItem.CSVImport.create_user_from_line("username, pword, fname, creator, none, yes, email, saml")
+    assert user is not None
+    assert user.auth_setting == "SAML"
+
+
+def test_set_values_rejects_invalid_auth_setting() -> None:
+    # _set_values must route auth_setting through the @property_is_enum guard.
+    # Attempting to set an invalid auth string must raise, not silently succeed.
+    user = TSC.UserItem("someone")
+    with pytest.raises(ValueError):
+        user._set_values(None, None, None, None, None, None, None, "NotAnAuthType", None, None, None, None)