diff --git a/tabcmd/commands/user/create_site_users.py b/tabcmd/commands/user/create_site_users.py index c63436b0..f51257ae 100644 --- a/tabcmd/commands/user/create_site_users.py +++ b/tabcmd/commands/user/create_site_users.py @@ -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): @@ -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)) diff --git a/tabcmd/commands/user/create_users_command.py b/tabcmd/commands/user/create_users_command.py index 9dafcb3f..fa47def0 100644 --- a/tabcmd/commands/user/create_users_command.py +++ b/tabcmd/commands/user/create_users_command.py @@ -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 * @@ -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): @@ -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 diff --git a/tabcmd/commands/user/user_data.py b/tabcmd/commands/user/user_data.py index 272a5375..40a4f23d 100644 --- a/tabcmd/commands/user/user_data.py +++ b/tabcmd/commands/user/user_data.py @@ -117,17 +117,40 @@ 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", + help=_("tabcmd.user.help.idp_configuration_id"), + ) 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 diff --git a/tabcmd/locales/en/tabcmd_messages_en.properties b/tabcmd/locales/en/tabcmd_messages_en.properties index 7eda4bff..ed783ebd 100644 --- a/tabcmd/locales/en/tabcmd_messages_en.properties +++ b/tabcmd/locales/en/tabcmd_messages_en.properties @@ -214,6 +214,7 @@ tabcmd.status.job_completed=Job completed tabcmd.status.waiting_for_refresh_job=Waiting for refresh job to begin 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. 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: diff --git a/tests/commands/test_run_commands.py b/tests/commands/test_run_commands.py index f1777d13..c412aa75 100644 --- a/tests/commands/test_run_commands.py +++ b/tests/commands/test_run_commands.py @@ -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() @@ -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() diff --git a/tests/commands/test_user_utils.py b/tests/commands/test_user_utils.py index 67de8d56..44df96ae 100644 --- a/tests/commands/test_user_utils.py +++ b/tests/commands/test_user_utils.py @@ -1,3 +1,4 @@ +import argparse import unittest from unittest.mock import * from tabcmd.commands.user.user_data import UserCommand, Userdata @@ -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" diff --git a/tests/parsers/test_parser_create_site_users.py b/tests/parsers/test_parser_create_site_users.py index 93a51db2..7e31bf74 100644 --- a/tests/parsers/test_parser_create_site_users.py +++ b/tests/parsers/test_parser_create_site_users.py @@ -48,3 +48,24 @@ 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", "abc-123-idp"] + args = self.parser_under_test.parse_args(mock_args) + assert args.idp_configuration_id == "abc-123-idp", 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", + "abc-123-idp", + ] + with self.assertRaises(SystemExit): + self.parser_under_test.parse_args(mock_args) diff --git a/tests/parsers/test_parser_create_user.py b/tests/parsers/test_parser_create_user.py index d4a533a1..1f29e7a9 100644 --- a/tests/parsers/test_parser_create_user.py +++ b/tests/parsers/test_parser_create_user.py @@ -28,3 +28,24 @@ 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", "abc-123-idp"] + args = self.parser_under_test.parse_args(mock_args) + assert args.idp_configuration_id == "abc-123-idp", 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", + "abc-123-idp", + ] + with self.assertRaises(SystemExit): + self.parser_under_test.parse_args(mock_args)