From f4ea58b6385454788cd4b4cff685f3d80e6924d1 Mon Sep 17 00:00:00 2001 From: Ondřej Nosek Date: Feb 07 2024 00:46:48 +0000 Subject: Methods for setting API tokens use getpass The methods `set-distgit-token`, `set-pagure-token` originally accepted a token as a command-line argument and this wasn't a sane approach. Now the token is safely put via getpass method. Also the code was moved to a shared place in rpkg because other tool uses it too. Fixes: rhbz#2089694 JIRA: RHELCMP-13435 Signed-off-by: Ondřej Nosek --- diff --git a/fedpkg/cli.py b/fedpkg/cli.py index bf167f5..90cc4c4 100644 --- a/fedpkg/cli.py +++ b/fedpkg/cli.py @@ -27,7 +27,7 @@ import six from pyrpkg import rpkgError from pyrpkg.cli import cliClient from six.moves import configparser -from six.moves.configparser import ConfigParser, NoOptionError, NoSectionError +from six.moves.configparser import NoOptionError, NoSectionError from six.moves.urllib_parse import urlparse from fedpkg.bugzilla import BugzillaClient @@ -307,7 +307,7 @@ class fedpkgClient(cliClient): "Create a new ticket" Update your token with the following command: - fedpkg set-pagure-token + fedpkg set-pagure-token Command saves token to fedpkg config file: ~/.config/rpkg/{0}.conf @@ -501,7 +501,7 @@ class fedpkgClient(cliClient): "Fork a project" Update your token with the following command: - fedpkg set-distgit-token + fedpkg set-distgit-token Command saves token to fedpkg config file: ~/.config/rpkg/{0}.conf @@ -553,40 +553,36 @@ class fedpkgClient(cliClient): parser.set_defaults(command=self.show_releases_info) def register_set_distgit_token(self): - help_msg = \ - 'Updates the fedpkg.distgit API token in ~/.config/rpkg/{0}.conf file.\n\n\ - Tokens are of length 64 and contain only uppercase and numerical values.'\ - .format(self.name) distgit_section = '{0}.distgit'.format(self.name) distgit_api_base_url = config_get_safely(self.config, distgit_section, "apibaseurl") + help_msg = \ + 'Updates the fedpkg.distgit API token in ~/.config/rpkg/{0}.conf file.\n\n\ + Tokens are of length 64 and contain only uppercase and numerical values.\n\ + The new API token can be generated at: \n\ + https://{1}/settings/token/new'\ + .format(self.name, urlparse(distgit_api_base_url).netloc) parser = self.subparsers.add_parser( 'set-distgit-token', help=help_msg, description=help_msg) - parser.add_argument( - 'token', - help='The new API token. Can be generated at: ' - 'https://{0}/settings/token/new'.format(urlparse(distgit_api_base_url).netloc)) parser.set_defaults(command=self.set_distgit_token) def register_set_pagure_token(self): - help_msg = \ - 'Updates the fedpkg.pagure API token in ~/.config/rpkg/{0}.conf file.\n\n\ - Tokens are of length 64 and contain only uppercase and numerical values.'\ - .format(self.name) pagure_section = '{0}.pagure'.format(self.name) pagure_url = config_get_safely(self.config, pagure_section, 'url') + help_msg = \ + 'Updates the fedpkg.pagure API token in ~/.config/rpkg/{0}.conf file.\n\n\ + Tokens are of length 64 and contain only uppercase and numerical values.\n\ + The new API token. Can be generated at: \n\ + https://{1}/settings/token/new'\ + .format(self.name, urlparse(pagure_url).netloc) parser = self.subparsers.add_parser( 'set-pagure-token', help=help_msg, description=help_msg) - parser.add_argument( - 'token', - help='The new API token. Can be generated at: ' - 'https://{0}/settings/token/new'.format(urlparse(pagure_url).netloc)) parser.set_defaults(command=self.set_pagure_token) @@ -747,7 +743,7 @@ class fedpkgClient(cliClient): "Modify an existing project" Update your token with the following command: - fedpkg set-distgit-token + fedpkg set-distgit-token Command saves token to fedpkg config file: ~/.config/rpkg/{0}.conf @@ -1474,7 +1470,7 @@ class fedpkgClient(cliClient): def _check_token(self, token, token_type): - if token is None: + if token is None or token == "": self.log.error("ERROR: No input.") return False @@ -1485,69 +1481,11 @@ class fedpkgClient(cliClient): else: return True - def _set_token(self, token_type): - - # Pop token off of the parse stack - TOKEN = self.args.token - - # Get the path to the fedpkg config file. - PATH = os.path.join(os.path.expanduser('~'), - '.config', - 'rpkg', - '{0}.conf'.format(self.name)) - - # load new config parser - local_config = ConfigParser() - local_config.read(PATH) - - # Ensure that user config file exists. - if not os.path.isfile(PATH): - self.log.warning("WARNING: User config file not found at: {0}".format(PATH)) - self.log.info("The config file will be created.") - # Make sure there is subdirectory already prepared. - try: - # Use parameter "exist_ok=True" in Python 3 only - os.makedirs(os.path.dirname(PATH)) - except OSError: - # Directory already exists or could not be created. - pass - if not os.path.isdir(os.path.dirname(PATH)): - self.log.error("ERROR: Could not create a directory for the user config file.") - return - - # Check that the user passed a valid token - if self._check_token(TOKEN, token_type): - - print("updating config '{}'".format(PATH)) - - # Update the token in the config object. - section = "{0}.{1}".format(self.name, token_type) - - # add the section if it doesn't already exist - if not local_config.has_section(section): - local_config.add_section(section) - - # set the distgit / pagure urls as needed (might already be set) - if token_type == "pagure": - local_config.set(section, "url", "https://pagure.io/") - else: - local_config.set(section, "apibaseurl", "https://src.fedoraproject.org") - - # update the token - local_config.set(section, "token", TOKEN) - - # Write the config to the user's config file. - with open(PATH, "w") as fp: - try: - local_config.write(fp) - except configparser.Error: - self.log.error("ERROR: Could not write to user config file.") - def set_pagure_token(self): - self._set_token("pagure") + super(fedpkgClient, self)._set_token("pagure") def set_distgit_token(self): - self._set_token("distgit") + super(fedpkgClient, self)._set_token("distgit") def retire(self): """ diff --git a/test/test_cli.py b/test/test_cli.py index 34c7ed7..be16d9d 100644 --- a/test/test_cli.py +++ b/test/test_cli.py @@ -2542,7 +2542,8 @@ class TestSetToken(CliTestCase): return self.new_cli(name=name, cfg=cfg) # Test: using lowercase characters rather than upper-case. - def test_token_input_mixed_lowercase_numerical(self): + @patch('getpass.getpass') + def test_token_input_mixed_lowercase_numerical(self, mock_getpass): LOWERCASE_TOKEN = "".join(["x" for _ in range(64)]) NUMERICAL_TOKEN = "".join([str(i % 10) for i in range(64)]) @@ -2551,8 +2552,9 @@ class TestSetToken(CliTestCase): for i in range(32): MIXED_LOWERCASE_NUMERICAL_TOKEN += LOWERCASE_TOKEN[i] MIXED_LOWERCASE_NUMERICAL_TOKEN += NUMERICAL_TOKEN[i] + mock_getpass.return_value = MIXED_LOWERCASE_NUMERICAL_TOKEN - cli_cmd = ['fedpkg', 'set-pagure-token', MIXED_LOWERCASE_NUMERICAL_TOKEN] + cli_cmd = ['fedpkg', 'set-pagure-token'] cli = self.get_cli(cli_cmd) try: @@ -2562,10 +2564,12 @@ class TestSetToken(CliTestCase): self.assertEqual(error, expected_error) # Test: no input, none input. - def test_token_input_none(self): + @patch('getpass.getpass') + def test_token_input_none(self, mock_getpass): - cli_cmd = ["fedpkg", "set-pagure-token", None] + cli_cmd = ["fedpkg", "set-pagure-token"] cli = self.get_cli(cli_cmd) + mock_getpass.return_value = None try: cli.set_pagure_token() @@ -2589,12 +2593,14 @@ class TestSetToken(CliTestCase): """ # Test: input is only lowercase. - def test_token_input_lowercase(self): + @patch('getpass.getpass') + def test_token_input_lowercase(self, mock_getpass): LOWERCASE_TOKEN = "".join(["x" for _ in range(64)]) - cli_cmd = ['fedpkg', 'set-pagure-token', LOWERCASE_TOKEN] + cli_cmd = ['fedpkg', 'set-pagure-token'] cli = self.get_cli(cli_cmd) + mock_getpass.return_value = LOWERCASE_TOKEN try: cli.set_pagure_token() @@ -2602,12 +2608,14 @@ class TestSetToken(CliTestCase): expected_error = "ERROR: Token is not properly formatted." self.assertEqual(error, expected_error) - def test_token_input_too_short(self): + @patch('getpass.getpass') + def test_token_input_too_short(self, mock_getpass): SHORT_TOKEN = "".join(['x' for _ in range(63)]) - cli_cmd = ['fedpkg', 'set-pagure-token', SHORT_TOKEN] + cli_cmd = ['fedpkg', 'set-pagure-token'] cli = self.get_cli(cli_cmd) + mock_getpass.return_value = SHORT_TOKEN try: cli.set_pagure_token() @@ -2615,12 +2623,14 @@ class TestSetToken(CliTestCase): expected_error = "ERROR: Token is not properly formatted." self.assertEqual(error, expected_error) - def test_token_input_too_long(self): + @patch('getpass.getpass') + def test_token_input_too_long(self, mock_getpass): LONG_TOKEN = "".join(['x' for _ in range(65)]) - cli_cmd = ['fedpkg', 'set-pagure-token', LONG_TOKEN] + cli_cmd = ['fedpkg', 'set-pagure-token'] cli = self.get_cli(cli_cmd) + mock_getpass.return_value = LONG_TOKEN try: cli.set_pagure_token()