feat: add --idp-configuration-id flag for multi-IDP Tableau Cloud sites - #444
Open
jacalata wants to merge 1 commit into
Open
feat: add --idp-configuration-id flag for multi-IDP Tableau Cloud sites#444jacalata wants to merge 1 commit into
jacalata wants to merge 1 commit 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>
Contributor
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.
Comment on lines
+131
to
+135
| auth_group.add_argument( | ||
| "--idp-configuration-id", | ||
| metavar="UUID", | ||
| help=_("tabcmd.user.help.idp_configuration_id"), | ||
| ) |
Comment on lines
+33
to
+37
| 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 |
Comment on lines
+53
to
+57
| 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 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #393.
Summary
Tableau Cloud 2024.3+ supports up to 20 identity providers per site, and the REST API takes an
idpConfigurationIdattribute when creating users. NeithercreatesiteusersnorcreateUserspreviously exposed the field, so operators on multi-IDP sites had to bypass tabcmd and script the REST API directly.Adds
--idp-configuration-id UUIDto bothcreatesiteusersandcreateUsers. The flag applies to every user in the batch. Argparse'sadd_mutually_exclusive_groupenforces that--auth-typeand--idp-configuration-idcannot both be passed, matching the REST API's own constraint (TSC'screate_reqraises if both attributes are set).Scope decisions
--role/--auth-type/--idp-configuration-idapplication was duplicated between the two commands. Lifted intoUserCommand.apply_cli_overridesso the branch is defined and tested in one place.Test plan
pytest tests/— 347 passed, 2 skipped, 0 failedcreatesiteusersandcreateUsers(4 tests)apply_cli_overrides: IDP flag clears a pre-existingauth_setting(covers the CSV col 8 +--idp-configuration-idinteraction); role flag applied (2 tests)check-strings— SUCCESS, all i18n keys definedblack --check— clean (using pinnedblack>=22,<23)mypy tabcmd/commands/user/— 0 new errorstabcmd createsiteusers --auth-type SAML --idp-configuration-id x users.csv→ argparse rejects with "not allowed with argument --auth-type" (confirmed at CLI)Follow-ups (not in this PR)
--auth-typedoc forcreatesiteusers/createUsers.🤖 Generated with Claude Code