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

CHANGELOG.md

@@ -5,6 +5,23 @@
   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.
+* `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.
+* `UserItem.CSVImport._validate_attribute_value` and the too-many-columns
+  branch of `_validate_import_line_or_throw` now raise `ValueError` instead
+  of `AttributeError` for invalid CSV input. `AttributeError` was the wrong
+  exception type for input validation and inconsistent with
+  `create_user_from_line`. `validate_file_for_import` catches `Exception` so
+  it is unaffected; direct callers who caught `AttributeError` specifically
+  need to widen their handler.
 * Added `JobItem.status_notes` for the structured `<statusNotes><statusNote
   type=".." value=".." text=".."/></statusNotes>` block documented on the Query
   Job REST endpoint. Populated for UserImport and other multi-row jobs where

tableauserverclient/models/user_item.py

@@ -308,6 +308,14 @@ def _set_values(
         if email:
             self.email = email
         if auth_setting:
+            # Write directly to _auth_setting rather than going through the
+            # @property_is_enum(Auth) setter. This method is called from both
+            # CSV import and server response parsing (from_xml, populate, etc.);
+            # if the server ever returns an auth type we don't yet know about
+            # (a new Auth value in a future Tableau release), the enum guard
+            # would raise ValueError during response parsing. CSV callers
+            # already validate the auth string against CSVImport._AUTH_CANONICAL
+            # before calling here, so this write is safe.
             self._auth_setting = auth_setting
         if domain_name:
             self._domain_name = domain_name
@@ -432,36 +440,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 +524,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 +534,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:
-                raise AttributeError("Too many attributes in line")
+            if len(line) > UserItem.CSVImport.COLUMN_COUNT:
+                raise ValueError("Too many attributes for user import")
             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
@@ -524,7 +563,7 @@ def _validate_attribute_value(item: str, possible_values: list[str], column_type
                 return
             if item in possible_values or possible_values == []:
                 return
-            raise AttributeError(f"Invalid value {item} for {column_type}")
+            raise ValueError(f"Invalid value {item} for {column_type}")
 
         # https://help.tableau.com/current/server/en-us/csvguidelines.htm#settings_and_site_roles
         # This logic is hardcoded to match the existing rules for import csv files

test/test_user.py

@@ -598,3 +598,24 @@ def test_remove_users_csv(server: TSC.Server) -> None:
             assert (
                 name == f"{user.domain_name}\\{user.name}"
             ), f"Name in csv does not match expected name: {user.domain_name}\\{user.name}"
+
+
+def test_from_xml_accepts_unknown_auth_setting() -> None:
+    # Regression: response parsing must not enforce the UserItem.Auth enum. If a
+    # future Tableau release adds a new auth type, TSC's `_set_values` used to
+    # route through the @property_is_enum(Auth) setter, which raised ValueError
+    # on any value outside {SAML, OpenID, TableauIDWithMFA, ServerDefault},
+    # breaking response parsing entirely. The CSV import path validates upstream
+    # against CSVImport._AUTH_CANONICAL, so the enum guard on _set_values was
+    # redundant there and harmful on server-parse paths.
+    ns = {"t": "http://tableau.com/api"}
+    xml = ET.fromstring(
+        b"<user xmlns='http://tableau.com/api' "
+        b"id='u1' name='alice' siteRole='Viewer' authSetting='FutureAuthType42' />"
+    )
+    item = TSC.UserItem.from_xml(xml, ns)
+    assert (
+        item.auth_setting == "FutureAuthType42"
+    ), "from_xml must preserve unknown auth values verbatim, not raise or drop them"
+    assert item.name == "alice"
+    assert item.site_role == "Viewer"

test/test_user_model.py

@@ -136,3 +136,62 @@ 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_validate_import_line_rejects_unknown_auth() -> None:
+    # _validate_import_line_or_throw shares the AUTH canonicalization with
+    # create_user_from_line -- confirm both paths reject unknown auth values so
+    # that validate_file_for_import (which uses this path) doesn't silently
+    # accept rows create_user_from_line would refuse.
+    with pytest.raises(ValueError, match="Invalid value"):
+        TSC.UserItem.CSVImport._validate_import_line_or_throw(
+            "username, pword, fname, creator, none, yes, email, NotAnAuthType",
+            logger,
+        )