Skip to content

Commit 140990b

Browse files
committed
Warn (don't raise) on unknown auth values during CSV import
Post-merge follow-up on #1811 -- the strict behavior it introduced (create_user_from_line raises ValueError on any auth value not in _AUTH_CANONICAL) would block CSV imports against newer servers as soon as Tableau ships an auth type TSC's hardcoded list doesn't yet know about. This is the same category of stale-list problem the _set_values enum-guard bypass exists to avoid on the server-parse path. Both entry points now warn and pass the value through: - create_user_from_line: unknown values raise a UserWarning naming the value and known set, then get assigned to auth_setting as-is. If the value really is a typo, the server rejects the row when the request posts -- a slightly-later error, but the import stays possible against forward-compatible servers. - _validate_import_line_or_throw: same shape. Skips the allowlist check for the AUTH column when the value isn't in _AUTH_CANONICAL, so validate_file_for_import doesn't return the row as invalid. Server-version-aware validation would be the cleaner long-term fix here (and for the enum-guard bypass in _set_values); noted for planning, not filing an issue. Tests updated: two former "raises ValueError" cases now assert pytest.warns(UserWarning) and confirm the raw value round-trips onto UserItem.auth_setting.
1 parent 275afdc commit 140990b

2 files changed

Lines changed: 58 additions & 19 deletions

File tree

tableauserverclient/models/user_item.py

Lines changed: 41 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import io
2+
import warnings
23
import xml.etree.ElementTree as ET
34
from datetime import datetime
45
from enum import IntEnum
@@ -476,12 +477,23 @@ def create_user_from_line(line: str):
476477
)
477478
raw_auth = values[UserItem.CSVImport.ColumnType.AUTH]
478479
if raw_auth:
479-
auth = UserItem.CSVImport._AUTH_CANONICAL.get(raw_auth.lower())
480-
if auth is None:
481-
raise ValueError(
482-
f"Unknown auth setting: {raw_auth!r}. "
483-
f"Valid values: {sorted(UserItem.CSVImport._AUTH_CANONICAL.values())}"
480+
canonical = UserItem.CSVImport._AUTH_CANONICAL.get(raw_auth.lower())
481+
if canonical is None:
482+
# Unknown auth value: pass it through instead of raising.
483+
# TSC's _AUTH_CANONICAL is a hardcoded list that will lag
484+
# server-side additions; refusing to build the UserItem
485+
# here would block CSV imports against newer servers as
486+
# soon as Tableau ships a new auth type. If it is a
487+
# typo, the server rejects the row when the request
488+
# posts. Warn so the caller has a shot at noticing.
489+
warnings.warn(
490+
f"Unknown auth setting {raw_auth!r}; passing through unchanged. "
491+
f"Known values: {sorted(UserItem.CSVImport._AUTH_CANONICAL.values())}",
492+
stacklevel=2,
484493
)
494+
auth = raw_auth
495+
else:
496+
auth = canonical
485497
else:
486498
auth = None
487499
user._set_values(
@@ -546,14 +558,35 @@ def _validate_import_line_or_throw(incoming, logger) -> None:
546558
for i in range(1, len(line)):
547559
value = line[i]
548560
valid = _valid_attributes[i]
561+
column = UserItem.CSVImport.ColumnType(i)
549562
# normalize case for fields with a restricted value set
563+
skip_validation = False
550564
if valid:
551565
if i == UserItem.CSVImport.ColumnType.AUTH:
552-
value = UserItem.CSVImport._AUTH_CANONICAL.get(value.lower(), value)
566+
canonical = UserItem.CSVImport._AUTH_CANONICAL.get(value.lower())
567+
if canonical is not None:
568+
value = canonical
569+
elif value:
570+
# Unknown auth value: warn and pass through instead
571+
# of raising. TSC's _AUTH_CANONICAL is a hardcoded
572+
# list that lags server-side additions; refusing
573+
# would block CSV imports against newer servers as
574+
# soon as Tableau ships a new auth type. Skip the
575+
# allowlist check so the row still validates.
576+
# Matches create_user_from_line's warn-and-pass.
577+
warnings.warn(
578+
f"Unknown auth setting {value!r}; passing through unchanged. "
579+
f"Known values: {sorted(UserItem.CSVImport._AUTH_CANONICAL.values())}",
580+
stacklevel=2,
581+
)
582+
skip_validation = True
553583
else:
554584
value = value.lower()
555-
logger.debug(f"column {UserItem.CSVImport.ColumnType(i).name}: {value}")
556-
UserItem.CSVImport._validate_attribute_value(value, valid, UserItem.CSVImport.ColumnType(i))
585+
# Mask the password column so it never reaches log handlers.
586+
safe_value = "***" if column == UserItem.CSVImport.ColumnType.PASS else value
587+
logger.debug(f"column {column.name}: {safe_value}")
588+
if not skip_validation:
589+
UserItem.CSVImport._validate_attribute_value(value, valid, column)
557590

558591
# Given a restricted set of possible values, confirm the item is in that set
559592
@staticmethod

test/test_user_model.py

Lines changed: 17 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -204,11 +204,18 @@ def test_too_many_columns_raises() -> None:
204204
TSC.UserItem.CSVImport.create_user_from_line("u, p, n, creator, none, yes, email, SAML, extra")
205205

206206

207-
def test_create_user_with_unknown_auth_raises() -> None:
208-
# Unknown AUTH values must raise, not silently produce a UserItem with auth_setting=None.
209-
# A caller can catch this if lenient behavior is wanted.
210-
with pytest.raises(ValueError, match="Unknown auth setting"):
211-
TSC.UserItem.CSVImport.create_user_from_line("username, pword, fname, creator, none, yes, email, NotAnAuthType")
207+
def test_create_user_with_unknown_auth_passes_through_with_warning() -> None:
208+
# Unknown AUTH values pass through with a UserWarning rather than raising.
209+
# TSC's _AUTH_CANONICAL is a hardcoded list that lags server-side auth-type
210+
# additions; refusing would block CSV imports against newer servers as
211+
# soon as Tableau ships a new auth type. If the value really is a typo,
212+
# the server rejects the row when the request posts.
213+
with pytest.warns(UserWarning, match="Unknown auth setting"):
214+
user = TSC.UserItem.CSVImport.create_user_from_line(
215+
"username, pword, fname, creator, none, yes, email, NotAnAuthType"
216+
)
217+
assert user is not None
218+
assert user.auth_setting == "NotAnAuthType"
212219

213220

214221
def test_create_user_with_lowercase_auth_accepted() -> None:
@@ -218,12 +225,11 @@ def test_create_user_with_lowercase_auth_accepted() -> None:
218225
assert user.auth_setting == "SAML"
219226

220227

221-
def test_validate_import_line_rejects_unknown_auth() -> None:
222-
# _validate_import_line_or_throw shares the AUTH canonicalization with
223-
# create_user_from_line -- confirm both paths reject unknown auth values so
224-
# that validate_file_for_import (which uses this path) doesn't silently
225-
# accept rows create_user_from_line would refuse.
226-
with pytest.raises(ValueError, match="Invalid value"):
228+
def test_validate_import_line_warns_on_unknown_auth() -> None:
229+
# _validate_import_line_or_throw matches create_user_from_line's warn-and-
230+
# pass behavior on unknown auth values: the same row shouldn't be accepted
231+
# by one path and rejected by the other.
232+
with pytest.warns(UserWarning, match="Unknown auth setting"):
227233
TSC.UserItem.CSVImport._validate_import_line_or_throw(
228234
"username, pword, fname, creator, none, yes, email, NotAnAuthType",
229235
logger,

0 commit comments

Comments
 (0)