Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
68 changes: 51 additions & 17 deletions tableauserverclient/models/user_item.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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]] = [
[],
[],
Expand All @@ -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
Expand Down
55 changes: 55 additions & 0 deletions test/test_user_model.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Loading