From 948e382d624f0e0769f226576a5fddb8977cbaa9 Mon Sep 17 00:00:00 2001 From: Jac Fitzgerald Date: Mon, 6 Jul 2026 22:33:19 -0700 Subject: [PATCH 1/5] fix: CSVImport AUTH column, case-insensitive validation, username preservation; add find_by_name MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes for UserItem.CSVImport (issue #1809): - MAX=8 (was 7=AUTH index): 8-column lines with auth type no longer rejected as "too many columns" - create_user_from_line no longer lowercases the whole line before splitting — username case is preserved - _validate_import_line_or_throw normalizes license/admin/publisher to lowercase and auth to canonical form before comparison, so 'Viewer', 'Creator', 'SAML', 'tableauidwithmfa' etc. are all accepted - Add TableauIDWithMFA to valid auth values in validation (was missing) - 5 new tests covering each fix Add QuerysetEndpoint.find_by_name(name) (issue #1810): - Thin wrapper over .filter(name=name) returning a list - Available on all content-item endpoints (workbooks, datasources, views, users, projects, groups) Co-Authored-By: Claude Sonnet 4.6 --- tableauserverclient/models/user_item.py | 52 +++++++++++++------ .../server/endpoint/endpoint.py | 3 ++ test/test_user_model.py | 33 ++++++++++++ 3 files changed, 73 insertions(+), 15 deletions(-) diff --git a/tableauserverclient/models/user_item.py b/tableauserverclient/models/user_item.py index 0ba1e8eb2..05e0f5916 100644 --- a/tableauserverclient/models/user_item.py +++ b/tableauserverclient/models/user_item.py @@ -432,36 +432,37 @@ class ColumnType(IntEnum): EMAIL = 6 AUTH = 7 - MAX = 7 + MAX = 8 # total number of columns (not last index) # 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.ColumnType.MAX: + 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.ColumnType.MAX: 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] + auth = UserItem.CSVImport._auth_canonical().get(raw_auth.lower()) if raw_auth else 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, @@ -491,6 +492,16 @@ def validate_file_for_import(csv_file: io.TextIOWrapper, logger) -> tuple[int, l # Some fields in the import file are restricted to specific values # Iterate through each field and validate the given value against hardcoded constraints + @staticmethod + def _auth_canonical() -> dict[str, str]: + """Lowercase → canonical form mapping for Auth values.""" + return { + "saml": "SAML", + "openid": "OpenID", + "serverdefault": "ServerDefault", + "tableauidwithmfa": "TableauIDWithMFA", + } + @staticmethod def _validate_import_line_or_throw(incoming, logger) -> None: _valid_attributes: list[list[str]] = [ @@ -501,7 +512,12 @@ 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 + [ + "SAML", + "OpenID", + "ServerDefault", + "TableauIDWithMFA", + ], # auth — normalized by _auth_canonical before comparison ] line = list(map(str.strip, incoming.split(","))) @@ -511,10 +527,16 @@ def _validate_import_line_or_throw(incoming, logger) -> None: 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 diff --git a/tableauserverclient/server/endpoint/endpoint.py b/tableauserverclient/server/endpoint/endpoint.py index 31a0806dc..300311b4c 100644 --- a/tableauserverclient/server/endpoint/endpoint.py +++ b/tableauserverclient/server/endpoint/endpoint.py @@ -431,6 +431,9 @@ def fields(self: Self, *fields: str) -> QuerySet: queryset.request_options.fields |= set(fields) | set(("_default_",)) return queryset + def find_by_name(self, name: str) -> list[T]: + return list(self.filter(name=name)) + def only_fields(self: Self, *fields: str) -> QuerySet: """ Add fields to the request options. If no fields are provided, the diff --git a/test/test_user_model.py b/test/test_user_model.py index 49e8dc25c..1b9b74f4e 100644 --- a/test/test_user_model.py +++ b/test/test_user_model.py @@ -136,3 +136,36 @@ 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, AttributeError)): + TSC.UserItem.CSVImport.create_user_from_line("u, p, n, creator, none, yes, email, SAML, extra") From dfd5541ced82b86b68d008d397fcbccdb70edc39 Mon Sep 17 00:00:00 2001 From: Jac Fitzgerald Date: Tue, 28 Jul 2026 12:42:43 -0700 Subject: [PATCH 2/5] fix: raise on unknown AUTH in CSV import and route auth through property setter MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two related fixes so unmapped auth values fail loudly at CSV parse time rather than producing a UserItem with silently missing auth_setting: - create_user_from_line: raise ValueError instead of silently setting auth to None when the AUTH column value isn't in _auth_canonical(). - _set_values: route auth_setting through the @property_is_enum(Auth) setter rather than writing to _auth_setting directly, so any invalid auth string is rejected at assignment. These two together close bug #5 in #1809 (setter bypass) and the silent- None finding surfaced in an adversarial review of the earlier commits on this branch. Callers who want lenient behavior (skip invalid rows, keep going) can catch the exception in their own iteration loop — that's the model tabcmd uses today via its --complete/--no-complete flag. Once this lands, tabcmd can defer its per-line validation to TSC (see #1809 and #1836). Also tightens test_too_many_columns_raises to expect ValueError only (was accepting either ValueError or AttributeError). --- tableauserverclient/models/user_item.py | 15 +++++++++++++-- test/test_user_model.py | 24 +++++++++++++++++++++++- 2 files changed, 36 insertions(+), 3 deletions(-) diff --git a/tableauserverclient/models/user_item.py b/tableauserverclient/models/user_item.py index 05e0f5916..48e477950 100644 --- a/tableauserverclient/models/user_item.py +++ b/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: @@ -453,7 +456,15 @@ def create_user_from_line(line: str): values[UserItem.CSVImport.ColumnType.PUBLISHER], ) raw_auth = values[UserItem.CSVImport.ColumnType.AUTH] - auth = UserItem.CSVImport._auth_canonical().get(raw_auth.lower()) if raw_auth else None + 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, username, diff --git a/test/test_user_model.py b/test/test_user_model.py index 1b9b74f4e..212b82518 100644 --- a/test/test_user_model.py +++ b/test/test_user_model.py @@ -167,5 +167,27 @@ def test_create_user_with_auth_column() -> None: def test_too_many_columns_raises() -> None: - with pytest.raises((ValueError, AttributeError)): + 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) From 25156645b42ca305e5ac8b114dcbe7ba4340747d Mon Sep 17 00:00:00 2001 From: Jac Fitzgerald Date: Tue, 28 Jul 2026 12:51:21 -0700 Subject: [PATCH 3/5] revert: move find_by_name to its own PR find_by_name was bundled with the CSVImport fixes in earlier commits because it landed in the same working commit. It's orthogonal to the CSV work and closes a different issue (#1810), so it belongs in its own PR. Reverting the 3-line addition here; will land as a separate branch. Co-Authored-By: Claude Opus 4.7 (1M context) --- tableauserverclient/server/endpoint/endpoint.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/tableauserverclient/server/endpoint/endpoint.py b/tableauserverclient/server/endpoint/endpoint.py index 300311b4c..31a0806dc 100644 --- a/tableauserverclient/server/endpoint/endpoint.py +++ b/tableauserverclient/server/endpoint/endpoint.py @@ -431,9 +431,6 @@ def fields(self: Self, *fields: str) -> QuerySet: queryset.request_options.fields |= set(fields) | set(("_default_",)) return queryset - def find_by_name(self, name: str) -> list[T]: - return list(self.filter(name=name)) - def only_fields(self: Self, *fields: str) -> QuerySet: """ Add fields to the request options. If no fields are provided, the From 95c75da6a8d0553b82d7235ed51e70020d421a94 Mon Sep 17 00:00:00 2001 From: Jac Fitzgerald Date: Tue, 28 Jul 2026 14:20:41 -0700 Subject: [PATCH 4/5] docs: changelog entry for CSVImport username case-preservation Call out the behavior change explicitly. The old code lowercased the entire CSV line including usernames, display names, fullnames, and emails; the new code preserves case for those fields and only normalizes the fields used for validation comparisons. Callers relying on the previous lowercased output need to know. Co-Authored-By: Claude Opus 4.7 (1M context) --- CHANGELOG.md | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 943436b27..71d3a0a01 100644 --- a/CHANGELOG.md +++ b/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 From 117cbac7d714300393afb208d9a519f85d8d4e34 Mon Sep 17 00:00:00 2001 From: Jac Fitzgerald Date: Tue, 28 Jul 2026 14:32:57 -0700 Subject: [PATCH 5/5] refactor: CSVImport MAX/auth cleanup from adversarial review Three small cleanups on UserItem.CSVImport surfaced by an adversarial code review of the earlier bug fixes: - MAX renamed to COLUMN_COUNT and moved out of the ColumnType IntEnum. ColumnType(8) used to return ColumnType.MAX, a fake column mixed in with real column indices. Now the count is a class-level constant. - _auth_canonical() no longer rebuilds its dict on every call. Promoted to _AUTH_CANONICAL class attribute. - _valid_attributes[AUTH] no longer hardcodes the accepted auth values. Derived from _AUTH_CANONICAL.values() instead so there's a single source of truth for what AUTH strings are accepted. No behavior change. Co-Authored-By: Claude Opus 4.7 (1M context) --- tableauserverclient/models/user_item.py | 47 +++++++++++++------------ 1 file changed, 24 insertions(+), 23 deletions(-) diff --git a/tableauserverclient/models/user_item.py b/tableauserverclient/models/user_item.py index 48e477950..7309632cf 100644 --- a/tableauserverclient/models/user_item.py +++ b/tableauserverclient/models/user_item.py @@ -435,7 +435,21 @@ class ColumnType(IntEnum): EMAIL = 6 AUTH = 7 - MAX = 8 # total number of columns (not last index) + # 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 @@ -443,12 +457,12 @@ def create_user_from_line(line: str): if line is None or line is False or line == "\n" or line == "": return None values: list[str] = list(map(str.strip, line.strip().split(","))) - if len(values) > UserItem.CSVImport.ColumnType.MAX: + 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: - 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], @@ -457,11 +471,11 @@ def create_user_from_line(line: str): ) raw_auth = values[UserItem.CSVImport.ColumnType.AUTH] if raw_auth: - auth = UserItem.CSVImport._auth_canonical().get(raw_auth.lower()) + 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())}" + f"Valid values: {sorted(UserItem.CSVImport._AUTH_CANONICAL.values())}" ) else: auth = None @@ -503,18 +517,10 @@ def validate_file_for_import(csv_file: io.TextIOWrapper, logger) -> tuple[int, l # Some fields in the import file are restricted to specific values # Iterate through each field and validate the given value against hardcoded constraints - @staticmethod - def _auth_canonical() -> dict[str, str]: - """Lowercase → canonical form mapping for Auth values.""" - return { - "saml": "SAML", - "openid": "OpenID", - "serverdefault": "ServerDefault", - "tableauidwithmfa": "TableauIDWithMFA", - } - @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]] = [ [], [], @@ -523,16 +529,11 @@ def _validate_import_line_or_throw(incoming, logger) -> None: ["system", "site", "none", "no"], # admin ["yes", "true", "1", "no", "false", "0"], # publisher [], - [ - "SAML", - "OpenID", - "ServerDefault", - "TableauIDWithMFA", - ], # auth — normalized by _auth_canonical before comparison + 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}") @@ -543,7 +544,7 @@ def _validate_import_line_or_throw(incoming, logger) -> None: # 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) + value = UserItem.CSVImport._AUTH_CANONICAL.get(value.lower(), value) else: value = value.lower() logger.debug(f"column {UserItem.CSVImport.ColumnType(i).name}: {value}")