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
7 changes: 2 additions & 5 deletions tabcmd/commands/user/create_site_users.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ def define_args(create_site_users_parser):
UserCommand.set_role_arg(args_group)
set_users_file_positional(args_group)
set_completeness_options(args_group)
UserCommand.set_auth_arg(args_group)
UserCommand.set_auth_and_idp_args(args_group)

@classmethod
def run_command(cls, args):
Expand All @@ -46,10 +46,7 @@ def run_command(cls, args):
error_list = []
for user_obj in user_obj_list:
try:
if args.role:
user_obj.site_role = args.role # tsc is case sensitive
if args.auth_type:
user_obj.auth_setting = args.auth_type
UserCommand.apply_cli_overrides(user_obj, args)
number_of_users_listed += 1
result = server.users.add(user_obj)
logger.info(_("common.output.succeeded").format(user_obj.name))
Expand Down
9 changes: 2 additions & 7 deletions tabcmd/commands/user/create_users_command.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
import tableauserverclient as TSC

from tabcmd.commands.auth.session import Session
from tabcmd.commands.constants import Errors
from tabcmd.execution.global_options import *
Expand All @@ -24,7 +22,7 @@ def define_args(create_users_parser):
UserCommand.set_role_arg(args_group)
set_users_file_positional(args_group)
set_completeness_options(args_group)
UserCommand.set_auth_arg(args_group)
UserCommand.set_auth_and_idp_args(args_group)

@classmethod
def run_command(cls, args):
Expand Down Expand Up @@ -54,10 +52,7 @@ def run_command(cls, args):
for user_obj in user_obj_list:
try:
number_of_users_listed += 1
if args.role:
user_obj.site_role = args.role
if args.auth_type:
user_obj.auth_setting = args.auth_type
UserCommand.apply_cli_overrides(user_obj, args)
server.users.add(user_obj)
logger.info(_("common.output.succeeded").format(user_obj.name))
number_of_users_added += 1
Expand Down
45 changes: 42 additions & 3 deletions tabcmd/commands/user/user_data.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import argparse
import io
import logging
import uuid
from enum import IntEnum
from typing import List, Callable, Optional

Expand All @@ -12,6 +13,20 @@
from tabcmd.execution.global_options import case_insensitive_string_type


def _idp_configuration_id_type(value: str) -> str:
"""argparse ``type=`` callable that requires ``--idp-configuration-id`` to be a UUID.

The REST API expects a UUID for this field; validating at argument-parse time
gives an immediate, actionable error rather than deferring to a server-side
400 later in the run.
"""
try:
uuid.UUID(value)
except (ValueError, AttributeError, TypeError):
raise argparse.ArgumentTypeError(_("tabcmd.user.error.idp_configuration_id_invalid_uuid").format(value))
return value


class Userdata:
def __init__(self):
self.name = None
Expand Down Expand Up @@ -117,17 +132,41 @@ def set_role_arg(parser):
return parser

@staticmethod
def set_auth_arg(parser):
parser.add_argument(
def set_auth_and_idp_args(parser):
# --auth-type and --idp-configuration-id both drive the user's authentication;
# the REST API rejects requests that set both, so argparse enforces exclusion here.
auth_group = parser.add_mutually_exclusive_group()
auth_group.add_argument(
"--auth-type",
metavar="TYPE",
choices=auth_types,
type=case_insensitive_string_type(auth_types),
# default="TableauID", # default is Local for on-prem, TableauID for Online. Does the server apply the default?
help=_("tabcmd.user.help.auth_type") + " " + ", ".join(auth_types),
)
auth_group.add_argument(
"--idp-configuration-id",
metavar="UUID",
type=_idp_configuration_id_type,
help=_("tabcmd.user.help.idp_configuration_id"),
)
Comment thread
Copilot marked this conversation as resolved.
return parser

@staticmethod
def apply_cli_overrides(user_obj, args) -> None:
"""Apply --role / --auth-type / --idp-configuration-id CLI overrides onto a UserItem.

--idp-configuration-id and --auth-type are mutually exclusive per the REST API.
When an IDP is provided (via the CLI flag), any auth_setting on the user is
cleared so the outgoing request has only one of the two attributes set.
"""
if args.role:
user_obj.site_role = args.role # TSC is case sensitive
if args.idp_configuration_id:
user_obj.idp_configuration_id = args.idp_configuration_id
user_obj.auth_setting = None
elif args.auth_type:
user_obj.auth_setting = args.auth_type

# read the file containing usernames or user details and validate each line
# log out any errors encountered
# returns the number of valid lines in the file
Expand Down
2 changes: 2 additions & 0 deletions tabcmd/locales/en/tabcmd_messages_en.properties
Original file line number Diff line number Diff line change
Expand Up @@ -212,8 +212,10 @@ tabcmd.server.error.site_not_found=Could not get site from server
tabcmd.status.fetched_user_details=Fetched user details from server
tabcmd.status.job_completed=Job completed
tabcmd.status.waiting_for_refresh_job=Waiting for refresh job to begin
tabcmd.user.error.idp_configuration_id_invalid_uuid=--idp-configuration-id must be a valid UUID (got: ''{0}'')
tabcmd.user.error.site_role_required=Site role is required
tabcmd.user.help.auth_type=Assigns the authentication type for all users in the CSV file. Possible values:
tabcmd.user.help.idp_configuration_id=Assigns an identity provider (IDP) configuration ID to all users in the CSV file. Required when your Tableau Cloud site has multiple IDPs enabled. Mutually exclusive with --auth-type.
Comment on lines +215 to +218
tabcmd.user.help.site_role=Specifies a site role for all users in the .csv file. Possible roles:
tabcmd.warning.calculations_not_supported=Adding or removing Calculations tasks are not supported
tabcmdparser.global.behaviors=Global behaviors:
Expand Down
4 changes: 4 additions & 0 deletions tests/commands/test_run_commands.py
Original file line number Diff line number Diff line change
Expand Up @@ -428,6 +428,7 @@ def test_create_site_users(self, mock_session, mock_server):
mock_args.site_name = None
mock_args.role = "Viewer"
mock_args.auth_type = "SAML"
mock_args.idp_configuration_id = None
create_site_users.CreateSiteUsersCommand.run_command(mock_args)
mock_session.assert_called()

Expand All @@ -446,6 +447,9 @@ def test_create_user(self, mock_session, mock_server):
mock_args.filename = RunCommandsTest._set_up_file()
mock_args.site_name = None
mock_args.require_all_valid = False
mock_args.role = None
mock_args.auth_type = None
mock_args.idp_configuration_id = None
create_users_command.CreateUsersCommand.run_command(mock_args)
mock_session.assert_called()

Expand Down
55 changes: 55 additions & 0 deletions tests/commands/test_user_utils.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import argparse
import unittest
from unittest.mock import *
from tabcmd.commands.user.user_data import UserCommand, Userdata
Expand Down Expand Up @@ -150,3 +151,57 @@ def test_parse_line_preserves_role(self):
user = UserCommand._parse_line("username, pword, fname, creator, none, yes, email")
assert user is not None
assert user.site_role == "Creator", f"Expected Creator, got {user.site_role}"


class ApplyCliOverridesTest(unittest.TestCase):
"""
Covers UserCommand.apply_cli_overrides, which both createsiteusers and
createUsers invoke before calling server.users.add.
"""

def _mk_args(self, role=None, auth_type=None, idp_configuration_id=None):
return argparse.Namespace(role=role, auth_type=auth_type, idp_configuration_id=idp_configuration_id)

def test_idp_flag_applied_when_present(self):
user = TSC.UserItem("alice", "Viewer")
UserCommand.apply_cli_overrides(user, self._mk_args(idp_configuration_id="idp-uuid-1"))
assert user.idp_configuration_id == "idp-uuid-1"
assert user.auth_setting is None

def test_idp_flag_overrides_auth_type_flag(self):
# Argparse's mutually_exclusive_group prevents both flags from being passed
# via the CLI in practice, but the runtime clear is still the last-line-of-defense
# so the wire request never has both attributes.
user = TSC.UserItem("alice", "Viewer")
UserCommand.apply_cli_overrides(user, self._mk_args(auth_type="SAML", idp_configuration_id="idp-uuid-2"))
assert user.idp_configuration_id == "idp-uuid-2"
assert user.auth_setting is None

def test_idp_flag_clears_existing_auth_setting(self):
# A user_obj may already have an auth_setting picked up from the CSV row's
# 8th column before apply_cli_overrides runs. If the operator passes
# --idp-configuration-id, that IDP wins and the pre-existing auth is cleared.
user = TSC.UserItem("alice", "Viewer")
user.auth_setting = "OpenID" # e.g. set from CSV col 8
UserCommand.apply_cli_overrides(user, self._mk_args(idp_configuration_id="idp-uuid-3"))
assert user.idp_configuration_id == "idp-uuid-3"
assert user.auth_setting is None

def test_auth_type_still_works_when_no_idp(self):
user = TSC.UserItem("alice", "Viewer")
UserCommand.apply_cli_overrides(user, self._mk_args(auth_type="SAML"))
assert user.auth_setting == "SAML"
assert user.idp_configuration_id is None

def test_neither_flag_leaves_user_unchanged(self):
# Backward-compat: users invoking without either flag see no change.
user = TSC.UserItem("alice", "Viewer")
user.auth_setting = "OpenID"
UserCommand.apply_cli_overrides(user, self._mk_args())
assert user.auth_setting == "OpenID"
assert user.idp_configuration_id is None

def test_role_flag_applied(self):
user = TSC.UserItem("alice", "Viewer")
UserCommand.apply_cli_overrides(user, self._mk_args(role="Creator"))
assert user.site_role == "Creator"
30 changes: 30 additions & 0 deletions tests/parsers/test_parser_create_site_users.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,3 +48,33 @@ def test_create_site_user_parser_auth_TabId_NotAvailable(self):
mock_args = [commandname, "users.csv", "--site", "site-name", "--auth-type", "TableauId"]
with self.assertRaises(SystemExit):
args = self.parser_under_test.parse_args(mock_args)

def test_create_site_user_parser_idp_configuration_id(self):
with mock.patch("builtins.open", mock.mock_open(read_data="test")):
mock_args = [commandname, "users.csv", "--idp-configuration-id", "12345678-1234-5678-1234-567812345678"]
args = self.parser_under_test.parse_args(mock_args)
assert args.idp_configuration_id == "12345678-1234-5678-1234-567812345678", args
assert args.auth_type is None, args

def test_create_site_user_parser_auth_and_idp_mutually_exclusive(self):
# argparse should reject requests that pass both flags at once.
with mock.patch("builtins.open", mock.mock_open(read_data="test")):
mock_args = [
commandname,
"users.csv",
"--auth-type",
"SAML",
"--idp-configuration-id",
"12345678-1234-5678-1234-567812345678",
]
with self.assertRaises(SystemExit):
self.parser_under_test.parse_args(mock_args)

def test_create_site_user_parser_idp_configuration_id_rejects_non_uuid(self):
# argparse should reject non-UUID values (typos, free text) at parse time
# rather than sending them to the REST API and failing with a server-side
# validation error.
with mock.patch("builtins.open", mock.mock_open(read_data="test")):
for bad_value in ("not-a-uuid", "abc-123-idp", ""):
with self.assertRaises(SystemExit):
self.parser_under_test.parse_args([commandname, "users.csv", "--idp-configuration-id", bad_value])
30 changes: 30 additions & 0 deletions tests/parsers/test_parser_create_user.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,3 +28,33 @@ def test_create_user_parser_role(self):
mock_args = [commandname, "users.csv", "-r", "SiteAdministrator"]
args = self.parser_under_test.parse_args(mock_args)
assert args.role.lower() == "SiteAdministrator".lower(), args

def test_create_user_parser_idp_configuration_id(self):
with mock.patch("builtins.open", mock.mock_open(read_data="test")):
mock_args = [commandname, "users.csv", "--idp-configuration-id", "12345678-1234-5678-1234-567812345678"]
args = self.parser_under_test.parse_args(mock_args)
assert args.idp_configuration_id == "12345678-1234-5678-1234-567812345678", args
assert args.auth_type is None, args

def test_create_user_parser_auth_and_idp_mutually_exclusive(self):
# argparse should reject requests that pass both flags at once.
with mock.patch("builtins.open", mock.mock_open(read_data="test")):
mock_args = [
commandname,
"users.csv",
"--auth-type",
"SAML",
"--idp-configuration-id",
"12345678-1234-5678-1234-567812345678",
]
with self.assertRaises(SystemExit):
self.parser_under_test.parse_args(mock_args)

def test_create_user_parser_idp_configuration_id_rejects_non_uuid(self):
# argparse should reject non-UUID values (typos, free text) at parse time
# rather than sending them to the REST API and failing with a server-side
# validation error.
with mock.patch("builtins.open", mock.mock_open(read_data="test")):
for bad_value in ("not-a-uuid", "abc-123-idp", ""):
with self.assertRaises(SystemExit):
self.parser_under_test.parse_args([commandname, "users.csv", "--idp-configuration-id", bad_value])