diff --git a/tabcmd/commands/datasources_and_workbooks/publish_command.py b/tabcmd/commands/datasources_and_workbooks/publish_command.py index dfd26b28..0a94cacb 100644 --- a/tabcmd/commands/datasources_and_workbooks/publish_command.py +++ b/tabcmd/commands/datasources_and_workbooks/publish_command.py @@ -42,10 +42,27 @@ def define_args(publish_parser): set_append_replace_option(group) set_parent_project_arg(group) + @classmethod + def _require_db_server_for_workbook(cls, args, filename, logger): + """Fail fast if the caller is publishing a workbook with embedded + credentials but did not supply --db-server. Called both up front + (direct-file case) and inside the per-file publish loop (folder case).""" + if (args.db_username or args.oauth_username) and not args.db_server: + filename_lower = (filename or "").lower() + if filename_lower.endswith(".twb") or filename_lower.endswith(".twbx"): + Errors.exit_with_error(logger, _("publish.errors.db_server_required")) + @classmethod def run_command(cls, args): logger = log(cls.__name__, args.logging_level) logger.debug(_("tabcmd.launching")) + + # Fail fast before auth when the direct target is clearly a workbook file: + # TSC's workbook publish path requires ConnectionItem.server_address whenever + # embedded credentials are supplied. Datasource publishes never need it, so + # this check is deliberately scoped to workbook extensions. + PublishCommand._require_db_server_for_workbook(args, args.filename, logger) + session = Session() server = session.create_session(args, logger) @@ -88,6 +105,9 @@ def run_command(cls, args): source = PublishCommand.get_filename_extension_if_tableau_type(logger, str_filename) logger.info(_("publish.status").format(str_filename)) if source in ["twbx", "twb"]: + # TSC's workbook publish path requires ConnectionItem.server_address whenever + # embedded connection credentials are supplied; datasource publish does not. + PublishCommand._require_db_server_for_workbook(args, str_filename, logger) try: published_item = PublishCommand.publish_workbook_file( args=args, @@ -96,7 +116,7 @@ def run_command(cls, args): project_id=project_id, str_filename=str_filename, publish_mode=publish_mode, - credentials=workbook_connections, + connection=workbook_connections, ) except Exception as e: Errors.exit_with_error(logger, exception=e) @@ -172,7 +192,7 @@ def get_publish_mode(args, logger): return publish_mode @staticmethod - def publish_workbook_file(args, logger, server, project_id, str_filename, publish_mode, credentials): + def publish_workbook_file(args, logger, server, project_id, str_filename, publish_mode, connection): if args.thumbnail_group: raise AttributeError("Generating thumbnails for a group is not yet implemented.") if args.thumbnail_username and args.thumbnail_group: @@ -185,7 +205,7 @@ def publish_workbook_file(args, logger, server, project_id, str_filename, publis publish_mode, # args.thumbnail_username, not yet implemented in tsc # args.thumbnail_group, - connections=[credentials] if credentials else None, + connections=[connection] if connection else None, as_job=False, skip_connection_check=args.skip_connection_check, ) diff --git a/tabcmd/locales/en/LC_MESSAGES/tabcmd.mo b/tabcmd/locales/en/LC_MESSAGES/tabcmd.mo index 7238ce24..85c42d08 100644 Binary files a/tabcmd/locales/en/LC_MESSAGES/tabcmd.mo and b/tabcmd/locales/en/LC_MESSAGES/tabcmd.mo differ diff --git a/tabcmd/locales/en/tabcmd_messages_en.properties b/tabcmd/locales/en/tabcmd_messages_en.properties index e5274230..88c40483 100644 --- a/tabcmd/locales/en/tabcmd_messages_en.properties +++ b/tabcmd/locales/en/tabcmd_messages_en.properties @@ -75,9 +75,10 @@ logout.short_description=Sign out from the server publish.description=Publish a workbook, data source, or extract to the server publish.errors.unexpected_server_response=Unexpected response from the server: {0} publish.errors.server_resource_not_found=The resource you specified does not exist, or you do not have permission to see it. Check your project name and permissions +publish.errors.db_server_required=--db-server is required when publishing a workbook with --db-username or --oauth-username. Use --db-server to specify the database server address the embedded credentials should be associated with. publish.options.append=Append extract file to existing data source publish.options.db-password=Database password for all data sources -publish.options.db-server=Server address of the database to associate with the embedded connection credentials. Required when using --db-username or --oauth-username to publish a workbook. +publish.options.db-server=Server address of the database to associate with the embedded connection credentials. Required when using --db-username or --oauth-username to publish a workbook. Must match the connection server stored in the workbook; a mismatch silently drops the embedded credentials. publish.options.db-username=Database username for all data sources publish.options.encrypt_extracts=Encrypt extracts in the workbook or datasource being published to the server. publish.options.name=Workbook or data source name on the server. If omitted, the workbook or data source will be named after the file name, without the twb(x), tds(x), or tde extension. Publishing a .tde file will create a data source diff --git a/tabcmd/locales/pt/LC_MESSAGES/tabcmd.mo b/tabcmd/locales/pt/LC_MESSAGES/tabcmd.mo index 8929f104..c50a8721 100644 Binary files a/tabcmd/locales/pt/LC_MESSAGES/tabcmd.mo and b/tabcmd/locales/pt/LC_MESSAGES/tabcmd.mo differ diff --git a/tabcmd/locales/pt/tabcmd_messages_pt.properties b/tabcmd/locales/pt/tabcmd_messages_pt.properties index c9dbfddd..ffccf710 100644 --- a/tabcmd/locales/pt/tabcmd_messages_pt.properties +++ b/tabcmd/locales/pt/tabcmd_messages_pt.properties @@ -77,7 +77,7 @@ publish.errors.unexpected_server_response=Resposta inesperada do servidor: {0} publish.errors.server_resource_not_found=O recurso especificado não existe ou você não tem permissão para vê-lo. Verifique o nome e as permissões do projeto publish.options.append=Anexe o arquivo de extração à fonte de dados existente publish.options.db-password=Senha do banco de dados para todas as fontes de dados -publish.options.db-server=Endereço do servidor do banco de dado associado as credenciais. Obrigatório ao usar --db-username ou --oauth-username. +publish.options.db-server=Endereço do servidor do banco de dados associado às credenciais. Obrigatório ao usar --db-username ou --oauth-username. publish.options.db-username=Nome de usuário do banco de dados para todas as fontes de dados publish.options.encrypt_extracts=Criptografar extrações no servidor publish.options.name=O nome da pasta de trabalho/fonte de dados no servidor. Se omitido, a pasta de trabalho/fonte de dados será nomeado de acordo com nome de arquivo, sem a extensão twb(x), tds(x) ou tde. Publicar um arquivo .tde criará uma fonte de dados diff --git a/tests/commands/test_publish_command.py b/tests/commands/test_publish_command.py index df4b7115..5c54bc6e 100644 --- a/tests/commands/test_publish_command.py +++ b/tests/commands/test_publish_command.py @@ -72,7 +72,7 @@ def test_publish_with_creds(self, mock_path, mock_glob, mock_session): self.assertEqual(len(connections), 1) self.assertEqual(connections[0].server_address, "db.example.com") - def test_publish_with_creds_no_db_server(self, mock_path, mock_glob, mock_session): + def test_publish_with_db_username_missing_db_server_exits(self, mock_path, mock_glob, mock_session): set_up_mock_server(mock_session) mock_path = set_up_mock_path(mock_path) @@ -87,7 +87,7 @@ def test_publish_with_creds_no_db_server(self, mock_path, mock_glob, mock_sessio mock_args.tabbed = True mock_args.db_username = "username" - mock_args.db_password = "oauth_u" + mock_args.db_password = "password" mock_args.db_server = None mock_args.save_db_password = True mock_args.oauth_username = None @@ -97,13 +97,111 @@ def test_publish_with_creds_no_db_server(self, mock_path, mock_glob, mock_sessio mock_args.thumbnail_group = None mock_args.skip_connection_check = False + with self.assertRaises(SystemExit): + PublishCommand.run_command(mock_args) + mock_session.internal_server.workbooks.publish.assert_not_called() + + def test_publish_with_oauth_username_missing_db_server_exits(self, mock_path, mock_glob, mock_session): + set_up_mock_server(mock_session) + mock_path = set_up_mock_path(mock_path) + + mock_args.overwrite = False + mock_args.append = True + mock_args.replace = False + + mock_args.filename = "existing_file.twbx" + mock_args.project_name = "project-name" + mock_args.parent_project_path = "projects" + mock_args.name = "" + mock_args.tabbed = True + + mock_args.db_username = None + mock_args.db_password = None + mock_args.db_server = None + mock_args.save_db_password = False + mock_args.oauth_username = "oauth_user" + mock_args.save_oauth = True + mock_args.embed = False + + mock_args.thumbnail_username = None + mock_args.thumbnail_group = None + mock_args.skip_connection_check = False + + with self.assertRaises(SystemExit): + PublishCommand.run_command(mock_args) + mock_session.internal_server.workbooks.publish.assert_not_called() + + def test_publish_with_oauth_creds(self, mock_path, mock_glob, mock_session): + set_up_mock_server(mock_session) + mock_path = set_up_mock_path(mock_path) + + mock_args.overwrite = False + mock_args.append = True + mock_args.replace = False + + mock_args.filename = "existing_file.twbx" + mock_args.project_name = "project-name" + mock_args.parent_project_path = "projects" + mock_args.name = "" + mock_args.tabbed = True + + mock_args.db_username = None + mock_args.db_password = None + mock_args.save_db_password = False + mock_args.oauth_username = "oauth_user" + mock_args.save_oauth = True + mock_args.db_server = "db.example.com" + mock_args.embed = False + + mock_args.thumbnail_username = None + mock_args.thumbnail_group = None + mock_args.skip_connection_check = False + PublishCommand.run_command(mock_args) mock_session.internal_server.workbooks.publish.assert_called() call_kwargs = mock_session.internal_server.workbooks.publish.call_args.kwargs connections = call_kwargs["connections"] self.assertEqual(len(connections), 1) - self.assertIsNone(connections[0].server_address) + self.assertEqual(connections[0].server_address, "db.example.com") + + def test_publish_datasource_with_db_username_no_db_server(self, mock_path, mock_glob, mock_session): + # Datasource publishes send credentials via _add_credentials_element on the TSC side, + # which does not require server_address. --db-server must not be required here. + set_up_mock_server(mock_session) + # set_up_mock_server only wires .workbooks on the mocked server (TSC.Server class-spec + # doesn't expose instance attributes from __init__); wire .datasources by hand. + mock_session.internal_server.datasources = mock.MagicMock() + mock_path = set_up_mock_path(mock_path) + # set_up_mock_path hardcodes splitext -> [_, 'twbx']; override so extension routing + # actually sees a datasource here. + mock_path.splitext = lambda x: ["file", "tdsx"] + + mock_args.overwrite = False + mock_args.append = True + mock_args.replace = False + + mock_args.filename = "existing_file.tdsx" + mock_args.project_name = "project-name" + mock_args.parent_project_path = "projects" + mock_args.name = "" + mock_args.tabbed = True + + mock_args.db_username = "username" + mock_args.db_password = "password" + mock_args.db_server = None + mock_args.save_db_password = True + mock_args.oauth_username = None + mock_args.embed = False + mock_args.use_tableau_bridge = False + + mock_args.thumbnail_username = None + mock_args.thumbnail_group = None + mock_args.skip_connection_check = False + + PublishCommand.run_command(mock_args) + mock_session.internal_server.datasources.publish.assert_called() + mock_session.internal_server.workbooks.publish.assert_not_called() def test_get_files_to_publish_twbx(self, mock_path, mock_glob, mock_session): set_up_mock_server(mock_session) diff --git a/tests/e2e/online_tests.py b/tests/e2e/online_tests.py index 741af663..ed631e0a 100644 --- a/tests/e2e/online_tests.py +++ b/tests/e2e/online_tests.py @@ -75,6 +75,8 @@ def get_publishable_name(file_value: str) -> str: TDS_FILE_LIVE = "live_mysql.tds" TWB_FILE_WITH_EMBEDDED_CONNECTION = "EmbeddedCredentials.twb" + # server_address baked into the workbook's element. + TWB_FILE_EMBEDDED_CONNECTION_SERVER = "see-internal-slack" USERS_DETAILS_FILE = "detailed_users.csv" USERNAMES_FILE = "usernames.csv" @@ -142,7 +144,9 @@ def _publish_args(file, name, optional_args=None): return arguments @staticmethod - def _publish_creds_args(arguments, db_user=None, db_pass=None, db_save=None, oauth_user=None, oauth_save=None): + def _publish_creds_args( + arguments, db_user=None, db_pass=None, db_save=None, oauth_user=None, oauth_save=None, db_server=None + ): if db_user: arguments.append("--db-username") arguments.append(db_user) @@ -156,6 +160,9 @@ def _publish_creds_args(arguments, db_user=None, db_pass=None, db_save=None, oau arguments.append(oauth_user) if oauth_save: arguments.append("--save-oauth") + if db_server: + arguments.append("--db-server") + arguments.append(db_server) return arguments @staticmethod @@ -468,14 +475,50 @@ def test_view_get_png(self): @pytest.mark.order(11) def test_wb_publish_embedded(self): + # NOTE: This asserts only that publish did not crash. It does NOT verify + # that credentials actually embedded on the server side - a mismatched + # --db-server would still exit 0 while silently dropping the creds at + # TSC's request-factory boundary. A stronger assertion would populate + # connections post-publish and check embed_password=True. file = os.path.join("tests", "assets", TestAssets.TWB_FILE_WITH_EMBEDDED_CONNECTION) name_on_server = TestAssets.get_publishable_name(TestAssets.TWB_FILE_WITH_EMBEDDED_CONNECTION) arguments = TabcmdCall._publish_args(file, name_on_server) - arguments = TabcmdCall._publish_creds_args(arguments, database_user, database_password, True) + arguments = TabcmdCall._publish_creds_args( + arguments, + database_user, + database_password, + True, + db_server=TestAssets.TWB_FILE_EMBEDDED_CONNECTION_SERVER, + ) arguments.append("--tabbed") arguments.append("--skip-connection-check") _test_command(arguments) + @pytest.mark.order(11) + def test_wb_publish_embedded_missing_db_server_fails(self): + # Publish with --db-username but no --db-server must exit non-zero with our + # friendly message rather than a raw tableauserverclient traceback. The guard + # short-circuits before any real work, so throwaway credentials are safe here. + file = os.path.join("tests", "assets", TestAssets.TWB_FILE_WITH_EMBEDDED_CONNECTION) + name_on_server = TestAssets.get_publishable_name(TestAssets.TWB_FILE_WITH_EMBEDDED_CONNECTION) + "-no-server" + arguments = TabcmdCall._publish_args(file, name_on_server) + arguments = TabcmdCall._publish_creds_args(arguments, "placeholder_user", "placeholder_pass", True) + arguments.append("--tabbed") + arguments.append("--skip-connection-check") + + login_args = setup_e2e.get_login_args() + if login_args is None: + pytest.skip("No credentials available (credentials.py not found)") + calling_args = ["python", "-m", "tabcmd"] + arguments + login_args + [debug_log] + ["--no-certcheck"] + result = subprocess.run(calling_args, capture_output=True, text=True) + + assert result.returncode != 0, "expected non-zero exit for missing --db-server" + # Localized string OR the raw key (if .mo has not been regenerated yet) both signal our guard. + combined = (result.stdout or "") + (result.stderr or "") + assert "publish.errors.db_server_required" in combined or "--db-server is required" in combined, ( + "expected guard message in output; got:\n" + combined + ) + @pytest.mark.order(12) def test_publish_ds(self): file = os.path.join("tests", "assets", TestAssets.TDSX_FILE_WITH_EXTRACT)