Fix #434: map "Local" auth_setting to ServerDefault

tabcmd/commands/user/create_site_users.py

@@ -41,15 +41,20 @@ def run_command(cls, args):
         UserCommand.validate_file_for_import(args.filename, logger, detailed=True, strict=args.require_all_valid)
 
         logger.info(_("addusers.status").format(args.filename.name, creation_site))
-        user_obj_list = UserCommand.get_users_from_file(args.filename, logger)
+        # A --auth-type override replaces any CSV-supplied auth value, so suppress
+        # the CSV-time "Local" remap warning when one is given -- it would describe
+        # a value that's about to be discarded.
+        user_obj_list = UserCommand.get_users_from_file(
+            args.filename, logger, warn_on_local_remap=not getattr(args, "auth_type", None)
+        )
         logger.info(_("session.monitorjob.percent_complete").format(0))
         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
+                    user_obj.auth_setting = UserCommand.normalize_auth_setting(args.auth_type, user_obj.name, logger)
                 number_of_users_listed += 1
                 result = server.users.add(user_obj)
                 logger.info(_("common.output.succeeded").format(user_obj.name))

tabcmd/commands/user/create_users_command.py

@@ -48,7 +48,12 @@ def run_command(cls, args):
         UserCommand.validate_file_for_import(args.filename, logger, detailed=True, strict=args.require_all_valid)
 
         logger.info(_("createusers.status").format(args.filename.name))
-        user_obj_list = UserCommand.get_users_from_file(args.filename, logger)
+        # A --auth-type override replaces any CSV-supplied auth value, so suppress
+        # the CSV-time "Local" remap warning when one is given -- it would describe
+        # a value that's about to be discarded.
+        user_obj_list = UserCommand.get_users_from_file(
+            args.filename, logger, warn_on_local_remap=not getattr(args, "auth_type", None)
+        )
         logger.info(_("session.monitorjob.percent_complete").format(0))
         error_list = []
         for user_obj in user_obj_list:
@@ -57,7 +62,7 @@ def run_command(cls, args):
                 if args.role:
                     user_obj.site_role = args.role
                 if args.auth_type:
-                    user_obj.auth_setting = args.auth_type
+                    user_obj.auth_setting = UserCommand.normalize_auth_setting(args.auth_type, user_obj.name, logger)
                 server.users.add(user_obj)
                 logger.info(_("common.output.succeeded").format(user_obj.name))
                 number_of_users_added += 1

tabcmd/commands/user/user_data.py

@@ -41,11 +41,12 @@ def populate(self, values: List[str]) -> None:
         if n_values >= 8:
             self.auth = values[7]
 
-    def to_tsc_user(self) -> TSC.UserItem:
+    def to_tsc_user(self, logger: Optional[logging.Logger] = None, warn_on_local_remap: bool = True) -> TSC.UserItem:
         site_role = UserCommand.evaluate_site_role(self.license_level, self.admin_level, self.publisher)
         if not site_role:
             raise AttributeError(_("tabcmd.user.error.site_role_required"))
-        user = TSC.UserItem(self.name, site_role, self.auth)
+        auth = UserCommand.normalize_auth_setting(self.auth, self.name, logger, warn=warn_on_local_remap)
+        user = TSC.UserItem(self.name, site_role, auth)
         user.email = self.email
         user.fullname = self.fullname
         return user
@@ -104,6 +105,28 @@ class UserCommand(Server):
     This class acts as a base class for user related group of commands
     """
 
+    # tabcmd Classic accepts "Local" as an auth type; TSC's UserItem.Auth enum
+    # has no Local value and rejects it. Map Classic's "Local" to ServerDefault
+    # (both for CSV values and for the CLI --auth-type override) so inputs
+    # authored for Classic import without crashing, and log a warning so the
+    # operator sees the remap happened.
+    # warn=False suppresses the warning for a remap whose result won't be the
+    # effective value -- e.g. a CSV row's "Local" that a CLI --auth-type
+    # override is about to replace.
+    @staticmethod
+    def normalize_auth_setting(
+        auth, username: Optional[str] = None, logger: Optional[logging.Logger] = None, warn: bool = True
+    ):
+        if isinstance(auth, str) and auth.lower() == "local":
+            if warn:
+                # Use the command's logger when given: it carries the console handlers,
+                # so the operator sees the warning on stderr, not only in tabcmd.log.
+                (logger or logging.getLogger(__name__)).warning(
+                    _("tabcmd.user.warning.local_auth_remapped").format(username)
+                )
+            return TSC.UserItem.Auth.ServerDefault
+        return auth
+
     @staticmethod
     def set_role_arg(parser):
         parser.add_argument(
@@ -193,7 +216,9 @@ def _validate_item(item: str, possible_values: List[str], column_type) -> None:
         raise AttributeError(_("commandlineutils.errors.bad_value").format(column_type, item, possible_values))
 
     @staticmethod
-    def get_users_from_file(csv_file: io.TextIOWrapper, logger=None) -> List[TSC.UserItem]:
+    def get_users_from_file(
+        csv_file: io.TextIOWrapper, logger=None, warn_on_local_remap: bool = True
+    ) -> List[TSC.UserItem]:
         csv_file.seek(0)  # set to start of file in case it has been read earlier
         if logger:
             logger.debug("Reading from file {}".format(csv_file.name))
@@ -202,22 +227,24 @@ def get_users_from_file(csv_file: io.TextIOWrapper, logger=None) -> List[TSC.Use
         if logger:
             logger.debug("> {}".format(line))
         while line:
-            user: Optional[TSC.UserItem] = UserCommand._parse_line(line)
+            user: Optional[TSC.UserItem] = UserCommand._parse_line(line, logger, warn_on_local_remap)
             if user:
                 user_list.append(user)
             line = csv_file.readline()
         return user_list
 
     @staticmethod
-    def _parse_line(line: str) -> Optional[TSC.UserItem]:
+    def _parse_line(
+        line: str, logger: Optional[logging.Logger] = None, warn_on_local_remap: bool = True
+    ) -> Optional[TSC.UserItem]:
         if line is None or line is False or line == "\n" or line == "":
             return None
         line = line.strip()
         line_parts: List[str] = line.split(",")
         data = Userdata()
         values: List[str] = list(map(str.strip, line_parts))
         data.populate(values)
-        return data.to_tsc_user()
+        return data.to_tsc_user(logger, warn_on_local_remap)
 
     # https://help.tableau.com/current/server/en-us/csvguidelines.htm#settings_and_site_roles
     @staticmethod

tabcmd/locales/en/tabcmd_messages_en.properties

@@ -230,6 +230,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.warning.local_auth_remapped=User '{0}': auth_setting 'Local' is not a Tableau Server auth type; mapped to ServerDefault. The site's default auth method will be used.
 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:

tests/commands/test_user_utils.py

@@ -1,3 +1,4 @@
+import logging
 import unittest
 from unittest.mock import *
 from tabcmd.commands.user.user_data import UserCommand, Userdata
@@ -150,3 +151,66 @@ 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}"
+
+    def test_local_auth_maps_to_server_default(self):
+        # tabcmd Classic accepts "Local" as an auth type; TSC's Auth enum has no
+        # Local value, so passing it through raises ValueError on server.users.add.
+        # to_tsc_user should map Classic's "Local" -> ServerDefault for parity
+        # and log a warning so the operator sees the remap happened.
+        data = Userdata()
+        data.populate(["username", "pword", "fname", "creator", "none", "yes", "email", "Local"])
+        with self.assertLogs("tabcmd.commands.user.user_data", level="WARNING") as logs:
+            user = data.to_tsc_user()
+        assert user.auth_setting == TSC.UserItem.Auth.ServerDefault, user.auth_setting
+        # Assert the warning names the affected user so the operator can act on
+        # it. Avoid coupling to the localize key -- once the .mo catalog carries
+        # this msgid, `_()` returns the translated text instead of the key.
+        assert any("username" in msg for msg in logs.output), logs.output
+
+    def test_local_auth_case_insensitive(self):
+        data = Userdata()
+        data.populate(["username", "pword", "fname", "creator", "none", "yes", "email", "local"])
+        with self.assertLogs("tabcmd.commands.user.user_data", level="WARNING"):
+            user = data.to_tsc_user()
+        assert user.auth_setting == TSC.UserItem.Auth.ServerDefault, user.auth_setting
+
+    def test_non_local_auth_passes_through(self):
+        # SAML/OpenID/etc. are not remapped.
+        data = Userdata()
+        data.populate(["username", "pword", "fname", "creator", "none", "yes", "email", TSC.UserItem.Auth.SAML])
+        user = data.to_tsc_user()
+        assert user.auth_setting == TSC.UserItem.Auth.SAML, user.auth_setting
+
+    def test_local_auth_warning_uses_supplied_logger(self):
+        # The command logger carries the console handlers; the warning must go there.
+        data = Userdata()
+        data.populate(["username", "pword", "fname", "creator", "none", "yes", "email", "Local"])
+        with self.assertLogs("tabcmd.test.command_logger", level="WARNING") as logs:
+            data.to_tsc_user(logging.getLogger("tabcmd.test.command_logger"))
+        assert any("username" in msg for msg in logs.output), logs.output
+
+    def test_normalize_auth_setting_cli_value(self):
+        with self.assertLogs("tabcmd.test.command_logger", level="WARNING"):
+            result = UserCommand.normalize_auth_setting("Local", "bob", logging.getLogger("tabcmd.test.command_logger"))
+        assert result == TSC.UserItem.Auth.ServerDefault
+
+    def test_local_auth_csv_warning_suppressed_when_cli_overrides(self):
+        # Regression: a CSV row of "Local" logged the remap warning during parsing
+        # even when a --auth-type CLI override was about to replace that value,
+        # describing a remap that never took effect. When a CLI override is
+        # coming, the CSV-time warning must be suppressed.
+        data = Userdata()
+        data.populate(["username", "pword", "fname", "creator", "none", "yes", "email", "Local"])
+        with self.assertNoLogs("tabcmd.commands.user.user_data", level="WARNING"):
+            user = data.to_tsc_user(warn_on_local_remap=False)
+        assert user.auth_setting == TSC.UserItem.Auth.ServerDefault, user.auth_setting
+
+    def test_local_auth_csv_warning_still_fires_without_override(self):
+        # Without a CLI override, the CSV's own "Local" value is effective, so the
+        # warning should still fire.
+        data = Userdata()
+        data.populate(["username", "pword", "fname", "creator", "none", "yes", "email", "Local"])
+        with self.assertLogs("tabcmd.commands.user.user_data", level="WARNING") as logs:
+            user = data.to_tsc_user(warn_on_local_remap=True)
+        assert user.auth_setting == TSC.UserItem.Auth.ServerDefault, user.auth_setting
+        assert any("username" in msg for msg in logs.output), logs.output