feat: add --idp-configuration-id flag for multi-IDP Tableau Cloud sites - #444
feat: add --idp-configuration-id flag for multi-IDP Tableau Cloud sites#444jacalata wants to merge 2 commits into
Conversation
Tableau Cloud 2024.3+ supports up to 20 identity providers per site, and the REST API takes an idpConfigurationId attribute when creating users. Neither `createsiteusers` nor `createUsers` previously exposed the field, so operators on multi-IDP sites had to bypass tabcmd and script the REST API directly. Adds `--idp-configuration-id UUID` to both commands. The flag applies to every user in the batch. Argparse's add_mutually_exclusive_group enforces that --auth-type and --idp-configuration-id cannot both be passed (TSC's create_req rejects that combination on the wire). The CSV format is unchanged (still 8 columns) so it stays consistent with Tableau Server's web-UI user-import and tabcmd 1. Per-user IDP assignment via a 9th CSV column was considered and deferred; it would require a coordinated cross-tool format update. While here, extracts the per-user `--role`/`--auth-type`/`--idp-configuration-id` application into UserCommand.apply_cli_overrides, removing the block duplicated between the two commands, and drops an unused tableauserverclient import from create_users_command.py. Adds 6 new tests: 4 for the parser (both commands, valid parse + mutual exclusion), 2 for apply_cli_overrides (IDP clears pre-existing auth_setting, role flag applied). All 347 tests pass. Closes #393 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Adds support for multi-IDP Tableau Cloud sites by exposing the REST API’s idpConfigurationId field via a new CLI flag, and centralizes per-user CLI override logic to avoid duplication across user-import commands.
Changes:
- Adds
--idp-configuration-idto bothcreatesiteusersandcreateUsers, enforced as mutually exclusive with--auth-type. - Refactors role/auth/idp application into
UserCommand.apply_cli_overridesand updates both commands to use it. - Extends parser and command-level unit tests, plus adds an i18n help string for the new flag.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/parsers/test_parser_create_user.py | Adds parser coverage for --idp-configuration-id and mutual exclusion. |
| tests/parsers/test_parser_create_site_users.py | Adds parser coverage for --idp-configuration-id and mutual exclusion. |
| tests/commands/test_user_utils.py | Adds unit tests for UserCommand.apply_cli_overrides behavior (role/auth/idp precedence). |
| tests/commands/test_run_commands.py | Updates mocked args to include the new idp_configuration_id attribute. |
| tabcmd/locales/en/tabcmd_messages_en.properties | Adds help text for --idp-configuration-id. |
| tabcmd/commands/user/user_data.py | Introduces set_auth_and_idp_args and apply_cli_overrides in shared user command base. |
| tabcmd/commands/user/create_users_command.py | Wires new args + shared override logic into createUsers. |
| tabcmd/commands/user/create_site_users.py | Wires new args + shared override logic into createsiteusers. |
Suppressed comments (2)
tests/parsers/test_parser_create_user.py:49
- This mutual-exclusion test also uses a non-UUID placeholder value for
--idp-configuration-id. Consider switching to a valid UUID so the test reflects expected real-world input.
"--idp-configuration-id",
"abc-123-idp",
]
tests/parsers/test_parser_create_site_users.py:69
- This mutual-exclusion test also uses a non-UUID placeholder value for
--idp-configuration-id. Consider switching to a valid UUID so the test reflects expected real-world input.
"SAML",
"--idp-configuration-id",
"abc-123-idp",
]
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Address Copilot review comments on #444 requesting UUID validation on the flag value. Malformed input now exits with an actionable error at argument-parse time instead of failing later with a server-side validation error. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
🟡 Changes recommended
New localization keys were added without updating the compiled gettext .mo catalogs, which can cause raw message IDs to appear in user-facing CLI help/error output.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 8/8 changed files
- Comments generated: 1
- Review effort level: Lite
| 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. |
Closes #393.
Motivation
Tableau Cloud 2024.3+ supports up to 20 identity providers per site.
The REST API takes an
idpConfigurationIdattribute when creatingusers, but neither
createsiteusersnorcreateUsersexposed it, sooperators on multi-IDP sites had to bypass tabcmd and script the REST
API directly.
Behavior change
For users:
--idp-configuration-id UUIDadded to bothcreatesiteusersandcreateUsers. Applies to every user in the batch.argparse.add_mutually_exclusive_groupenforces that--auth-typeand
--idp-configuration-idcan't both be passed, matching the RESTAPI's own constraint (TSC's
create_reqraises if both are set on aUserItem).--idp-configuration-idclears any pre-existingauth_settingon theUserItem(mostly to handle the case where theCSV column 8 supplied an auth type and the CLI flag overrides it).
Scope not covered:
format divergence with tabcmd 1 and the Server web-UI user import;
per-user IDP assignment would need a coordinated cross-tool change.
Refactor bundled: the per-user
--role/--auth-type/--idp-configuration-idapplication was duplicated between the twocommands. Lifted into
UserCommand.apply_cli_overrides.Test plan
pytest tests/— 347 passed, 2 skippedcreatesiteusersand
createUsers(4 tests);apply_cli_overridesIDP-clears-auth androle-application (2 tests)
check-stringspasses;black --checkclean; mypy cleantabcmd createsiteusers --auth-type SAML --idp-configuration-id x users.csv— argparse rejects with "not allowed with argument--auth-type"
Follow-ups
Per-user IDP assignment via a 9th CSV column would need to land
coordinated with tabcmd 1 and the Server web-UI import.
🤖 Generated with Claude Code