PR#2151: GSSAPI follow-up - copr/copr

		`@@ -4,7 +4,7 @@`
		`needed granularity.`
		`"""`

		`- from astroid import MANAGER, scoped_nodes, extract_node`
		`+ from astroid import MANAGER, scoped_nodes, nodes, extract_node`

		`def register(_linter):`
		`""" required pylint entrypoint """`
		`@@ -17,6 +17,16 @@`
		`return True`
		`return False`

		`+`
		`+ def add_fake_docs(the_object):`
		`+ """`
		`+ Add fake docs to the specified object so PyLint later doesn't complain about`
		`+ missing docs.`
		`+ """`
		`+ the_object.doc_node = nodes.Const("fake docs")`
		`+ the_object.doc = "fake docs"`
		`+`
		`+`
		`def transform_functions(function):`
		`"""`
		`Transformate some function definitions so pylint doesn't object.`
		`@@ -27,14 +37,14 @@`

		`if function.name in ["upgrade", "downgrade"]:`
		`# ignore missing-function-docstring in migrations`
		`- function.doc = "fake docs"`
		`+ add_fake_docs(function)`

		`if function.name == "step_impl":`
		`# behave step definition`
		`- function.doc = "fake docs"`
		`+ add_fake_docs(function)`

		`if is_test_method(function):`
		`- function.doc = "fake docs"`
		`+ add_fake_docs(function)`

		`def transform_classes(classdef):`
		`"""`
		`@@ -43,6 +53,7 @@`
		`if classdef.name.startswith("Test"):`
		`# ignore missing-function-docstring in migrations`
		`classdef.doc = "fake docs"`
		`+ classdef.doc_node = nodes.Const("fake docs")`


		`MANAGER.register_transform(scoped_nodes.FunctionDef, transform_functions)`

cli/copr-cli.spec

file modified

+1 -1

		`@@ -6,7 +6,7 @@`
		`%global with_python2 1`
		`%endif`

		`- %global min_python_copr_version 1.116.1.dev`
		`+ %global min_python_copr_version 1.116.2.dev`

		`Name: copr-cli`
		`Version: 1.99`

cli/copr_cli/main.py

file modified

+92 -73

		`@@ -29,8 +29,8 @@`

		`import copr.exceptions as copr_exceptions`
		`from copr.v3 import (`
		`- Client, config_from_file, CoprException, CoprRequestException, CoprConfigException, CoprNoResultException,`
		`- CoprGssapiException`
		`+ Client, config_from_file, CoprException, CoprRequestException,`
		`+ CoprConfigException, CoprNoResultException, CoprAuthException,`
		`)`
		`from copr.v3.pagination import next_page`
		`from copr_cli.helpers import cli_use_output_format`
		`@@ -45,15 +45,7 @@`
		`else:`
		`from urllib.parse import urljoin, urlparse`

		`- if sys.version_info < (2, 7):`
		`- class NullHandler(logging.Handler):`
		`- def emit(self, record):`
		`- pass`
		`- else:`
		`- from logging import NullHandler`
		`-`
		`log = logging.getLogger(__name__)`
		`- log.addHandler(NullHandler())`


		`ON_OFF_MAP = {`
		`@@ -132,6 +124,26 @@`
		`return buildopts`


		`+ def requires_api_auth(func):`
		`+ """`
		`+ For operations that need authentication with server, check early that we`
		`+ have either the user/token pair in hand, or that we can use the GSSAPI`
		`+ session for authentication. IOW, this decorator contacts the server`
		`+ only if the GSSAPI is ON and preferred -- and the session cookie is not`
		`+ initialized.`
		`+ """`
		`+ def wrapper(self, args):`
		`+ # This call implies that either login+token is specified, GSSAPI`
		`+ # session file exists, or (as a fallback) that we are able to do`
		`+ # GSSAPI handshake and initialize the GSSAPI session.`
		`+ self.client.base_proxy.auth_username()`
		`+ return func(self, args)`
		`+`
		`+ wrapper.__doc__ = func.__doc__`
		`+ wrapper.__name__ = func.__name__`
		`+ return wrapper`
		`+`
		`+`
		`class Commands(object):`
		`def __init__(self, config_path):`
		`self.config_path = config_path or '~/.config/copr'`
		`@@ -140,55 +152,47 @@`
		`self.config = config_from_file(self.config_path)`
		`except CoprConfigException as ex:`
		`sys.stderr.write(no_config_warning.format(self.config_path, ex))`
		`- self.config = {"copr_url": "http://copr.fedoraproject.org", "no_config": True}`
		`+ self.config = {"copr_url": "http://copr.fedoraproject.org"}`

		`- if not self.config.get("token"):`
		`+ if self.config.get("gssapi") is None:`
		`+ # Contrary to what is set in python-copr, we set GSSAPI on by`
		`+ # default (unless user explicitly says otherwise).`
		`self.config["gssapi"] = True`
		`self.config["connection_attempts"] = 3`
		`self.client = Client(self.config)`

		`- def requires_api_auth(func):`
		`- """ Decorator that checks config presence`
		`+ @property`
		`+ def username(self):`
		`"""`
		`-`
		`- def wrapper(self, args):`
		`- if "no_config" in self.config:`
		`- sys.stderr.write("Error: Operation requires api authentication\n")`
		`- sys.exit(6)`
		`-`
		`- return func(self, args)`
		`-`
		`- wrapper.__doc__ = func.__doc__`
		`- wrapper.__name__ = func.__name__`
		`- return wrapper`
		`-`
		`- def check_username_presence(func):`
		`- """ Decorator that checks if username was provided`
		`+ Get the username from config, or obtain it via auth_check (transitively`
		`+ via GSSAPI).`
		`"""`
		`+ if self.config.get("username"):`
		`+ return self.config["username"]`

		`- def wrapper(self, args):`
		`- if self.config["username"] is None and args.username is None:`
		`- if self.config.get("gssapi"):`
		`- log.debug("Username is not set in the config file")`
		`- log.debug("Trying to detect via gssapi")`
		`- log.debug("Disable the use of gssapi by setting gssapi = False in the config file")`
		`- response = self.client.base_proxy.auth_check()`
		`- try:`
		`- self.config["username"] = response.name`
		`- except AttributeError:`
		`- pass`
		`-`
		`- if self.config["username"] is None and args.username is None:`
		`- sys.stderr.write(`
		`- "Error: Operation requires username\n"`
		- "Pass username to command or add it to `{0}`\n".format(self.config_path))
		`- sys.exit(6)`
		`-`
		`- return func(self, args)`
		`+ if self.config.get("gssapi"):`
		`+ try:`
		`+ return self.client.base_proxy.auth_username()`
		`+ except CoprAuthException:`
		`+ log.error("Failed to determine Copr username from authentication.")`
		`+ raise`
		`+ raise CoprConfigException(`
		`+ "This operation tries to detect your username, but it is not "`
		`+ "possible to find it in configuration, and GSSAPI is disabled "`
		`+ )`

		`- wrapper.__doc__ = func.__doc__`
		`- wrapper.__name__ = func.__name__`
		`- return wrapper`
		`+ @property`
		`+ def ownername(self):`
		`+ """`
		`+ Determine the project ownername (== username) when not specified on`
		`+ commandline.`
		`+ """`
		`+ try:`
		`+ return self.username`
		`+ except:`
		`+ log.error("This operation needs a project ownername specified, "`
		`+ "fallback to your username failed.")`
		`+ raise`

		`def parse_name(self, name):`
		`m = re.match(r"([^/]+)/(.*)", name)`
		`@@ -196,7 +200,7 @@`
		`owner = m.group(1)`
		`name = m.group(2)`
		`else:`
		`- owner = self.config["username"]`
		`+ owner = self.ownername`
		`return owner, name`

		`def build_url(self, build_id):`
		`@@ -214,7 +218,7 @@`
		`"""`
		`m = re.match(r"(([^/]+)/)?([^/]+)/(.*)", path)`
		`if m:`
		`- owner = m.group(2) or self.config["username"]`
		`+ owner = m.group(2) or self.username`
		`return owner, m.group(3), m.group(4)`
		`raise CoprException("Unexpected chroot path format")`

		`@@ -278,11 +282,11 @@`
		`except KeyboardInterrupt:`
		`pass`

		`- def action_whoami(self, args):`
		`+ def action_whoami(self, _args):`
		`"""`
		`Simply print out the current user as defined in copr config.`
		`"""`
		`- print(self.config["username"])`
		`+ print(self.username)`

		`def action_new_webhook_secret(self, args):`
		`"""`
		`@@ -310,8 +314,10 @@`

		`:param args: argparse arguments provided by the user`
		`"""`
		`- response = self.client.base_proxy.auth_check()`
		`- self.config["username"] = response.name`
		`+`
		`+ # Before we start uploading potentially large source RPM file, make sure`
		`+ # that the user actually has a valid credentials.`
		`+ self.client.base_proxy.auth_check()`

		`builds = []`
		`for pkg in args.pkgs:`
		`@@ -585,7 +591,6 @@`
		`print(MockProfile(build_config))`


		`- @check_username_presence`
		`def action_list(self, args):`
		`""" Method called when the 'list' action has been selected by the`
		`user.`
		`@@ -593,7 +598,7 @@`
		`:param args: argparse arguments provided by the user`

		`"""`
		`- username = args.username or self.config["username"]`
		`+ username = args.username or self.ownername`
		`projects = self.client.project_proxy.get_list(username)`
		`if not projects:`
		`sys.stderr.write("No copr retrieved for user: '{0}'\n".format(username))`
		`@@ -1747,13 +1752,26 @@`
		`return parser`


		`- def enable_debug():`
		`- logging.basicConfig(`
		`- level=logging.DEBUG,`
		`- format='[%(asctime)s] {%(pathname)s:%(lineno)d} %(levelname)s - %(message)s',`
		`- datefmt='%H:%M:%S'`
		`- )`
		`- sys.stderr.write("# Debug log enabled #\n")`
		`+ def setup_logging(debug):`
		`+ """`
		`+ Configure the global 'log' object so it prints to standard error output`
		`+ (INFO+ messages). When --debug is used, all logging (even external`
		`+ libraries) is turned on for DEBUG+ level.`
		`+ """`
		`+ if debug:`
		`+ logging.basicConfig(`
		`+ level=logging.DEBUG,`
		`+ format='[%(asctime)s] {%(pathname)s:%(lineno)d} %(levelname)s - %(message)s',`
		`+ datefmt='%H:%M:%S'`
		`+ )`
		`+ sys.stderr.write("# Debug log enabled #\n")`
		`+ return`
		`+`
		`+ stderr = logging.StreamHandler()`
		`+ stderr.setLevel(logging.INFO)`
		`+ formatter = logging.Formatter("%(levelname)s: %(message)s")`
		`+ stderr.setFormatter(formatter)`
		`+ log.addHandler(stderr)`


		`def str2bool(v):`
		`@@ -1787,13 +1805,13 @@`


		`def main(argv=sys.argv[1:]):`
		`+ # pylint: disable=too-many-branches`
		`try:`
		`# Set up parser for global args`
		`parser = setup_parser()`
		`# Parse the commandline`
		`arg = parser.parse_args(argv)`
		`- if arg.debug:`
		`- enable_debug()`
		`+ setup_logging(arg.debug)`

		`if not "func" in arg:`
		`parser.print_help()`
		`@@ -1824,8 +1842,13 @@`
		`except argparse.ArgumentTypeError as e:`
		`sys.stderr.write("\nError: {0}\n".format(e))`
		`sys.exit(2)`
		`- except CoprGssapiException as e:`
		`- sys.stderr.write("\nError: {0}\n".format(e))`
		`+ except CoprConfigException as err:`
		`+ sys.stderr.write("\nError: Copr configuration error. {0}\n".format(err))`
		`+ sys.exit(6)`
		`+ except CoprAuthException as err:`
		`+ sys.stderr.write("\nError: Operation requires API authentication. "`
		`+ "See the 'AUTHENTICATION' section in man copr-cli(1).\n"`
		`+ "\nError: {0}\n".format(err))`
		`sys.exit(7)`
		`except CoprException as e:`
		`sys.stderr.write("\nError: {0}\n".format(e))`
		`@@ -1834,10 +1857,6 @@`
		`sys.stderr.write("\nError: {0}\n".format(e))`
		`sys.exit(5)`

		`- # except Exception as e:`
		`- # print "Error: {0}".format(e)`
		`- # sys.exit(100)`
		`-`

		`if __name__ == "__main__":`
		`main()`

cli/man/copr-cli.1.asciidoc

file modified

+39 -10

		`@@ -17,7 +17,7 @@`

		`This command allows you to interact with the Copr service from the command line.`

		`- You need to generate an API key first (see the API KEY section below).`
		`+ You need to to configure authentication first (see the AUTHENTICATION section below).`

		`OPTIONS`
		`-------`
		`@@ -785,32 +785,61 @@`
		`EXIT STATUS`
		`-----------`
		`Normally, the exit code is 0 when everything goes well. But if not, we could get:`
		`+`
		`1 - Bad request like wrong project name, insufficient rights etc.`
		`Also might happen when user interrupts the operation when they shouldn't.`
		`+`
		`2 - Wrong arguments given.`
		`+`
		`3 - Bad or no configuration.`
		`+`
		`4 - Build failed or was canceled.`
		`+`
		`5 - Communication error between Cli and server.`
		`This issue probably means bug and should be reported.`
		`- 6 - Configuration file is missing.`
		`- 7 - Kerberos authentication failed.`

		`+ 6 - Configuration error.`

		`- API KEY`
		`- -------`
		`+ 7 - Authentication with Copr server failed.`
		`+`
		`+`
		`+ AUTHENTICATION`
		`+ --------------`
		`+`
		`+ Copr client supports token-based and GSSAPI authentication.`

		`Visit the page https://copr.fedorainfracloud.org/api/ to obtain an API token.`
		- This token must be saved in the file `~/.config/copr` in the following
		`- format:`
		+ This token must be saved in the configuration file `~/.config/copr` in the
		`+ following format:`

		`[copr-cli]`
		`username = msuchy`
		`login = Y57wcg==##fkfaxbkjhuoiebfafadl`
		`token = vbfseelqdebzedukgombekmuvbkqwo`
		`- copr_url = https://copr.fedoraproject.org`
		`+ copr_url = https://copr.fedorainfracloud.org`
		`+ # expiration date: 2023-01-01`
		`+`
		`+ Be aware that API tokens have an expiration date!`
		`+`
		`+ To enable GSSAPI you need to obtain a Kerberos ticket. If you want to work with`
		`+ Fedora Copr, you can just do:`
		`+`
		`+ $ fkinit`
		`+ Enter your password and OTP concatenated. (Ignore that the prompt is for only the token)`
		`+ Enter OTP Token Value: <your password + OTP token>`
		`+`
		`+ To work with a different (non-Fedora Copr) instance, you will obtain the`
		`+ ticket differently and you still need to have a configuration file referring`
		+ appropriate `copr_url`:
		`+`
		`+ $ kinit username@EXAMPLE.COM`
		`+ $ cat ~/.config/copr`
		`+ [copr-cli]`
		`+ copr_url = https://copr.example.com/`
		`+`
		`+ Copr client uses the python API internally, for more info take a look at the`
		`+ page https://python-copr.readthedocs.io/en/latest/ClientV3.html#example-usage`

		`- Be aware that API tokens have an expiration date. The expiration date for`
		`- your token is listed on the /api page.`

		`USING DIFFERENT COPR INSTANCE`
		`-----------------------------`

cli/tests/cli_tests_lib.py

file modified

+3 -3

		`@@ -10,10 +10,10 @@`
		`import pytest`

		`config = {`
		`- "username": None,`
		`+ "username": "jdoe",`
		`"copr_url": "http://copr/",`
		`- "login": "",`
		`- "token": "",`
		`+ "login": "xyz",`
		`+ "token": "abc",`
		`}`

		`@pytest.yield_fixture`

cli/tests/test_cli.py

file modified

+52 -24

		`@@ -1,13 +1,20 @@`
		`import os`
		`import argparse`
		`import json`
		`+ import shutil`
		`+ import tempfile`
		`import pytest`
		`import responses`
		`from munch import Munch`

		`import copr`
		`- from copr.exceptions import CoprUnknownResponseException, CoprBuildException`
		`- from cli_tests_lib import mock, MagicMock, config as mock_config`
		`+ from copr.exceptions import (`
		`+ CoprBuildException,`
		`+ CoprConfigException,`
		`+ CoprUnknownResponseException,`
		`+ )`
		`+ from copr.v3.exceptions import CoprAuthException`
		`+ from cli_tests_lib import config as mock_config, mock, MagicMock`
		`from copr_cli import main`
		`from copr_cli.main import FrontendOutdatedCliException`

		`@@ -30,12 +37,29 @@`
		`# log = logging.getLogger()`
		`# log.info("Logger initiated")`

		`- @mock.patch('copr_cli.main.config_from_file', return_value=mock_config)`
		`- def test_parse_name(config_from_file):`
		`- cmd = main.Commands(config_path=None)`
		`- assert cmd.parse_name("foo") == (None, "foo")`
		`- assert cmd.parse_name("frostyx/foo") == ("frostyx", "foo")`
		`- assert cmd.parse_name("@copr/foo") == ("@copr", "foo")`
		`+ class TestCliWrapper:`
		`+ tmpdir = None`
		`+ configfile = None`
		`+`
		`+ def setup_method(self, _method):`
		`+ self.tmpdir = tempfile.mkdtemp(prefix="test-cli-name-parseing")`
		`+ self.configfile = os.path.join(self.tmpdir, "config")`
		`+`
		`+ with open(self.configfile, 'w') as fd:`
		`+ fd.write("[copr-cli]\n")`
		`+ fd.write("copr_url = https://xyz/\n")`
		`+ fd.write("token = xyz\n")`
		`+ fd.write("username = jdoe\n")`
		`+ fd.write("login = login\n")`
		`+`
		`+ def teardown_method(self, _method):`
		`+ shutil.rmtree(self.tmpdir)`
		`+`
		`+ def test_parse_name(self):`
		`+ cmd = main.Commands(config_path=self.configfile)`
		`+ assert cmd.parse_name("foo") == ("jdoe", "foo")`
		`+ assert cmd.parse_name("frostyx/foo") == ("frostyx", "foo")`
		`+ assert cmd.parse_name("@copr/foo") == ("@copr", "foo")`


		`@mock.patch('copr.v3.proxies.build.BuildProxy.get')`
		`@@ -146,9 +170,8 @@`

		`@responses.activate`
		`@mock.patch('copr.v3.proxies.build.BuildProxy.cancel')`
		`- @mock.patch('configparser.ConfigParser.read')`
		`- def test_cancel_build_no_config(read, build_proxy_cancel, capsys):`
		`- read.return_value = []`
		`+ @mock.patch('copr_cli.main.config_from_file', return_value=mock_config)`
		`+ def test_cancel_build_no_config(_cff, build_proxy_cancel, capsys):`
		`response_status = "foobar"`
		`build_proxy_cancel.return_value = MagicMock(state=response_status)`
		`main.main(argv=["cancel", "123"])`
		`@@ -251,17 +274,20 @@`
		`assert json.loads(out) == json.loads(expected_output)`


		`- @mock.patch('configparser.ConfigParser.read')`
		`- @mock.patch('copr.v3.proxies.BaseProxy.auth_check', return_value=Munch(name=None))`
		`- def test_list_project_no_username(_auth_check, read, capsys):`
		`- read.return_value = []`
		`-`
		`+ @mock.patch('copr.v3.proxies.BaseProxy.auth_username')`
		`+ @mock.patch('copr_cli.main.config_from_file')`
		`+ def test_list_project_no_username(ac, cff, capsys):`
		`+ """`
		`+ Config unset (and gssapi ON by default in cli)`
		`+ """`
		`+ ac.side_effect = CoprAuthException("gssapi fail")`
		`+ cff.side_effect = CoprConfigException("foo")`
		`with pytest.raises(SystemExit) as err:`
		`main.main(argv=["list"])`
		`-`
		`- assert exit_wrap(err.value) == 6`
		`- out, err = capsys.readouterr()`
		- assert "Pass username to command or add it to `~/.config/copr`" in err
		`+ assert exit_wrap(err.value) == 7`
		`+ _, err = capsys.readouterr()`
		`+ msg = "Operation requires API authentication. See the 'AUTHENTICATION'"`
		`+ assert msg in err`


		`@mock.patch('copr_cli.main.config_from_file', return_value=mock_config)`
		`@@ -271,13 +297,16 @@`
		`"copr_url": "http://copr/",`
		`"login": "",`
		`"token": "test_token_XXX",`
		`+ "gssapi": False,`
		`}`
		`with pytest.raises(SystemExit) as err:`
		`main.main(argv=["list"])`

		`assert exit_wrap(err.value) == 6`
		`out, err = capsys.readouterr()`
		- assert "Pass username to command or add it to `~/.config/copr`" in err
		`+ exp = "This operation tries to detect your username, but it is not " + \`
		`+ "possible to find it in configuration, and GSSAPI is disabled"`
		`+ assert exp in err`


		`@mock.patch('copr.v3.proxies.project.ProjectProxy.get_list')`
		`@@ -427,7 +456,7 @@`
		`args, kwargs = project_proxy_add.call_args`
		`assert kwargs == {`
		`"auto_prune": True,`
		`- "ownername": None, "persistent": False, "projectname": "foo", "description": "desc string",`
		`+ "ownername": "jdoe", "persistent": False, "projectname": "foo", "description": "desc string",`
		`"instructions": "instruction string", "chroots": ["f20", "f21"],`
		`"additional_repos": ["repo1", "repo2"],`
		`"unlisted_on_hp": None, "devel_mode": None, "enable_net": False,`
		`@@ -518,7 +547,7 @@`
		`args, kwargs = project_proxy_add.call_args`
		`assert kwargs == {`
		`"auto_prune": True,`
		`- "ownername": None, "persistent": False, "projectname": "foo",`
		`+ "ownername": "jdoe", "persistent": False, "projectname": "foo",`
		`"description": None,`
		`"instructions": "instruction string",`
		`"chroots": ["fedora-rawhide-x86_64", "fedora-rawhide-i386"],`
		`@@ -737,7 +766,6 @@`
		`args = action.call_args[0][0]`
		`assert args.permissions == expected_output`

		`-`
		`with pytest.raises(SystemExit) as err:`
		`main.main(['edit-permissions'])`
		`assert err.value.code == 2`

cli/tests/test_distgit.py

file modified

+3 -4

		`@@ -33,7 +33,7 @@`
		`"Created builds: 1\n"`
		`)`
		`default_build_call = {`
		`- 'ownername': None,`
		`+ 'ownername': "jdoe",`
		`'projectname': 'project',`
		`'project_dirname': 'project',`
		`'buildopts': {`
		`@@ -66,7 +66,6 @@`
		`capsys)`
		`assert len(f_patch_create_from_distgit.call_args_list) == 1`
		`call = f_patch_create_from_distgit.call_args_list[0]`
		`- self.default_build_call['ownername'] = "test"`
		`assert call[1] == self.default_build_call`

		`@pytest.mark.parametrize('enable_net', ["on", "off"])`
		`@@ -109,7 +108,7 @@`
		`with mock.patch("copr.v3.proxies.package.PackageProxy.edit") as p2:`
		`yield p1, p2`

		`- def test_add_package_normal(self, f_patch_package_distgit, capsys, ):`
		`+ def test_add_package_normal(self, f_patch_package_distgit, capsys):`
		`_assert_output(['add-package-distgit', '--name', 'package',`
		`'project'], self.success_stdout, "", capsys)`
		`assert len(f_patch_package_distgit[0].call_args_list) == 1`
		`@@ -117,7 +116,7 @@`

		`call = f_patch_package_distgit[0].call_args_list[0]`
		`assert call == mock.call(`
		`- "test", "project", "package", "distgit",`
		`+ "jdoe", "project", "package", "distgit",`
		`{'distgit': None,`
		`'namespace': None,`
		`'committish': None,`

docker/frontend/files/etc/copr/copr.conf

file modified

-21

		`@@ -56,27 +56,6 @@`
		`#FEDMENU_URL = "https://apps.fedoraproject.org/fedmenu/"`
		`#FEDMENU_DATA_URL = "https://apps.fedoraproject.org/js/data.js"`

		`- # Kerberos authentication configuration. Example configuration abbreviated`
		`- # like 'RH' (should not changed once used, reflects what is stored in database).`
		`- # This requires mod_auth_kerb package (Fedora/RHEL) installed on your frontend`
		`- # machine.`
		`- #`
		`- # log_text - What text will be shown in top-menu link pointing to URI`
		`- # with particular Kerberos login`
		`- # URI - Users can do authentication on /krb5_login/<URI>/`
		`- # (accessible from top-menu link). Note that apache must be`
		`- # configured to perforem KRB authentication on the`
		`- # /krb5_login/<URI>/ URI. See mod`
		`- # email_domain - For simplicity, we expect that all users coming from the`
		`- # same krb realm have the same (implicit) email domain. E.g.`
		`- # kerberos user user@REDHAT.COM is expected to have`
		`- # user@redhat.com email address.`
		`-`
		`- #KRB5_LOGIN = {`
		`- # 'log_text': "gssapi login",`
		`- # 'email_domain': "redhat.com",`
		`- #}`
		`-`
		`PUBLIC_COPR_HOSTNAME = 'localhost:5000'`
		`PUBLIC_COPR_BASE_URL = 'http://frontend:5000'`

frontend/copr-frontend.spec

file modified

		`@@ -125,6 +125,7 @@`
		`Requires: curl`
		`Requires: httpd`
		`Recommends: logrotate`
		`+ Recommends: mod_auth_gssapi`
		`Requires: redis`

		`Requires: %flavor_guard`

frontend/coprs_frontend/commands/add_user.py

file modified

+2 -2

		`@@ -1,7 +1,7 @@`
		`import click`
		`from coprs import db, app`
		`from coprs import models`
		`- from coprs.views.misc import create_user_wrapper`
		`+ from coprs.logic.users_logic import UsersLogic`

		`@click.command()`
		`@click.argument("name")`
		`@@ -28,7 +28,7 @@`
		`print("User named {0} already exists.".format(name))`
		`return`

		`- user = create_user_wrapper(name, mail)`
		`+ user = UsersLogic.create_user_wrapper(name, mail)`
		`if api_token:`
		`user.api_token = api_token`
		`if api_login:`

frontend/coprs_frontend/config/copr.conf

file modified

+24 -19

		`@@ -54,28 +54,25 @@`
		`#FEDMENU_URL = "https://apps.fedoraproject.org/fedmenu/"`
		`#FEDMENU_DATA_URL = "https://apps.fedoraproject.org/js/data.js"`

		`- # Kerberos authentication configuration. Example configuration abbreviated`
		`- # like 'RH' (should not changed once used, reflects what is stored in database).`
		`- # This requires mod_auth_kerb package (Fedora/RHEL) installed on your frontend`
		`- # machine.`
		`+ # Kerberos authentication configuration. This requires mod_auth_gssapi package`
		`+ # (Fedora/RHEL) installed on your frontend machine, and properly configured`
		`+ # HTTP server there with a Kerberos auth accepted (keytab required) on the`
		`+ # /api_v3/gssapi_login/ route.`
		`+ # When KRB5_LOGIN is enabled together with FAS_LOGIN, users are forced to first`
		`+ # log-in into Copr build system via OpenID -- so we get their info about FAS`
		`+ # groups and the real e-mail (nb the fedoraproject.org alias is only created for`
		`+ # the users who have at least one group in FAS).`
		`#`
		`# log_text - What text will be shown in top-menu link pointing to URI`
		`- # with particular Kerberos login`
		`- # URI - Users can do authentication on /krb5_login/<URI>/`
		`- # (accessible from top-menu link). Note that apache must be`
		`- # configured to perforem KRB authentication on the`
		`- # /krb5_login/<URI>/ URI. See mod`
		`- # email_domain - For simplicity, we expect that all users coming from the`
		`- # same krb realm have the same (implicit) email domain. E.g.`
		`- # kerberos user user@REDHAT.COM is expected to have`
		`- # user@redhat.com email address.`
		`+ # with particular Kerberos/GSSAPI login`
		`+ # email_domain - For now and for simplicity, we expect that all users coming`
		`+ # from the realm have the same email domain. E.g. users we`
		`+ # configure that all users in Fedora FAS have the`
		`+ # @fedoraproject.org alias working.`

		`#KRB5_LOGIN = {`
		`- # 'RH': {`
		`- # 'URI': "redhat",`
		`- # 'log_text': "Krb5 login",`
		`- # 'email_domain': "redhat.com",`
		`- # }`
		`+ # 'log_text': "GSSAPI login",`
		`+ # 'email_domain': "fedoraproject.org",`
		`#}`

		`PUBLIC_COPR_HOSTNAME = "copr.fedoraproject.org"`
		`@@ -113,7 +110,15 @@`
		`# Hide page parts not relevant to this Copr instance:`
		`# LAYOUT_OVERVIEW_HIDE_QUICK_ENABLE = False`

		`- # FAS_LOGIN = True`
		`+ # Allow logging into the Copr Web-UI using the Fedora Accounts System OpenID.`
		`+ #OPENID_PROVIDER_URL = "https://id.stg.fedoraproject.org"`
		`+`
		`+ # The OPENID_* configuration is a Fedora Accounts OpenID. This will show the`
		`+ # "FAS login" link at the top of the front page, user e-mails are downloaded`
		`+ # from OpenID, and also @groups are downloaded from OpenID.`
		`+ # OPENID_PROVIDER_URL.`
		`+ #FAS_LOGIN = True`
		`+`
		`# LOGIN_INFO = {`
		`# # Link to user's profile, {username} expaneded.`
		`# 'user_link': 'https://accounts.fedoraproject.org/user/{username}/',`

frontend/coprs_frontend/coprs/config.py

file modified

-1

		`@@ -13,7 +13,6 @@`
		`BACKEND_BASE_URL = "http://copr-be-dev.cloud.fedoraproject.org"`
		`BACKEND_STATS_URI = None`

		`- KRB5_LOGIN_BASEURI = "/api_v3/gssapi_login/"`
		`KRB5_LOGIN = {}`

		`OPENID_PROVIDER_URL = "https://id.fedoraproject.org"`

frontend/coprs_frontend/coprs/context_processors.py

file modified

+1 -1

		`@@ -59,7 +59,7 @@`

		`if config['KRB5_LOGIN']:`
		`menu.append({`
		`- 'link': config['KRB5_LOGIN_BASEURI'],`
		`+ 'link': flask.url_for("apiv3_ns.gssapi_login"),`
		`'desc': config['KRB5_LOGIN']['log_text'],`
		`})`

frontend/coprs_frontend/coprs/logic/users_logic.py

file modified

+24 -3

		`@@ -1,11 +1,12 @@`
		`+ import base64`
		`import json`
		`- from datetime import date`
		`+ import datetime`
		`from coprs import exceptions`
		`from flask import url_for`

		`from coprs import app, db`
		`from coprs.models import User, Group`
		`- from coprs.helpers import copr_url`
		`+ from coprs.helpers import copr_url, generate_api_token`
		`from sqlalchemy import update`


		`@@ -125,11 +126,31 @@`
		`"admin": False,`
		`"api_login": "",`
		`"api_token": "",`
		`- "api_token_expiration": date(1970, 1, 1),`
		`+ "api_token_expiration": datetime.date(1970, 1, 1),`
		`"openid_groups": None}`
		`for k, v in null.items():`
		`setattr(user, k, v)`

		`+ @classmethod`
		`+ def create_user_wrapper(cls, username, email, timezone=None):`
		`+ """`
		`+ Initial creation of Copr user (creates the API token, too).`
		`+ Create user + token configuration.`
		`+ """`
		`+ expiration_date_token = datetime.date.today() + \`
		`+ datetime.timedelta(`
		`+ days=app.config["API_TOKEN_EXPIRATION"])`
		`+`
		`+ copr64 = base64.b64encode(b"copr") + b"##"`
		`+ user = User(username=username, mail=email,`
		`+ timezone=timezone,`
		`+ api_login=copr64.decode("utf-8") + generate_api_token(`
		`+ app.config["API_TOKEN_LENGTH"] - len(copr64)),`
		`+ api_token=generate_api_token(`
		`+ app.config["API_TOKEN_LENGTH"]),`
		`+ api_token_expiration=expiration_date_token)`
		`+ return user`
		`+`

		`class UserDataDumper(object):`
		`def __init__(self, user):`

frontend/coprs_frontend/coprs/views/apiv3_ns/apiv3_general.py

file modified

+122 -1

		`@@ -1,6 +1,48 @@`
		`+ import os`
		`+ import re`
		`+`
		`import flask`
		`+`
		`+ from coprs import app, oid, db, models`
		`from coprs.views.apiv3_ns import apiv3_ns`
		`+ from coprs.exceptions import AccessRestricted`
		`from coprs.views.misc import api_login_required`
		`+ from coprs.logic.users_logic import UsersLogic`
		`+`
		`+`
		`+ def auth_check_response():`
		`+ """`
		`+ Used in misc and apiv3 for returning info about the user.`
		`+ """`
		`+ return flask.g.user.to_dict()`
		`+`
		`+`
		`+ def gssapi_login_action():`
		`+ """`
		`+ Redirect the successful log-in attempt, or return the JSON data that user`
		`+ expects.`
		`+ """`
		`+ if "web-ui" in flask.request.full_path:`
		`+ return flask.redirect(oid.get_next_url())`
		`+ return flask.jsonify(auth_check_response())`
		`+`
		`+`
		`+ def krb_straighten_username(krb_remote_user):`
		`+ """`
		`+ Cleanup the user's principal, and return just simple username. Remove`
		`+ disallowed characters for the service principals.`
		`+ """`
		`+ # Input should look like 'USERNAME@REALM.TLD', strip realm.`
		`+ username = re.sub(r'@.*', '', krb_remote_user)`
		`+`
		`+ # But USERNAME part can consist of USER/DOMAIN.TLD.`
		`+ # TODO: Do we need more clever thing here?`
		`+ username = re.sub('/', '_', username)`
		`+`
		`+ # Based on restrictions for project name: "letters, digits, underscores,`
		`+ # dashes and dots", it is worth limitting the username here, too.`
		`+ # TODO: Store this pattern on one place.`
		`+ return username if re.match(r"^[\w.-]+$", username) else None`


		`@apiv3_ns.route("/")`
		`@@ -11,4 +53,83 @@`
		`@apiv3_ns.route("/auth-check")`
		`@api_login_required`
		`def auth_check():`
		`- return flask.jsonify(flask.g.user.to_dict())`
		`+ return flask.jsonify(auth_check_response())`
		`+`
		`+`
		`+ def auth_403(message):`
		`+ """`
		`+ Return appropriately formatted GSSAPI 403 error for both web-ui and API`
		`+ """`
		`+ message = "Can't log-in using GSSAPI: " + message`
		`+ if "web-ui" in flask.request.full_path:`
		`+ return flask.render_template("403.html", message=message), 403`
frostyx commented 2 years ago My guess is that the `AccessRestricted` should be properly handled even for web-ui, so this function may be simply an oneliner `raise AccessRestricted(message)`. Maybe ...
		`+ raise AccessRestricted(message)`
		`+`
		`+`
		`+ @apiv3_ns.route("/gssapi_login/", methods=["GET"])`
		`+ @apiv3_ns.route("/gssapi_login/web-ui/", methods=["GET"])`
		`+ def gssapi_login():`
		`+ """`
		`+ Log-in using the GSSAPI/Kerberos credentials`
		`+`
		`+ Note that if we are able to get here, either the user is authenticated`
		`+ correctly, or apache is mis-configured and it does not perform KRB`
		`+ authentication at all (REMOTE_USER wouldn't be set, see below).`
		`+ """`
		`+`
		`+ # Already logged in?`
		`+ if flask.g.user is not None:`
		`+ return gssapi_login_action()`
		`+`
		`+ krb_config = app.config['KRB5_LOGIN']`
		`+`
		`+ if app.config["DEBUG"] and 'TEST_REMOTE_USER' in os.environ:`
		`+ # For local testing (without krb5 keytab and other configuration)`
		`+ flask.request.environ['REMOTE_USER'] = os.environ['TEST_REMOTE_USER']`
		`+`
		`+ if 'REMOTE_USER' not in flask.request.environ:`
		`+ nocred = "Kerberos authentication failed (no credentials provided)"`
		`+ return auth_403(nocred)`
		`+`
		`+ krb_username = flask.request.environ['REMOTE_USER']`
		`+ app.logger.debug("krb5 login attempt: " + krb_username)`
		`+ username = krb_straighten_username(krb_username)`
		`+ if not username:`
		`+ return auth_403("invalid krb5 username: " + krb_username)`
		`+`
		`+ krb_login = (`
		`+ models.Krb5Login.query`
		`+ .filter(models.Krb5Login.primary == username)`
		`+ .first()`
		`+ )`
		`+`
		`+ if krb_login:`
		`+ flask.g.user = krb_login.user`
		`+ flask.session['krb5_login'] = krb_login.user.name`
		`+ flask.flash("Welcome, {0}".format(flask.g.user.name), "success")`
		`+ return gssapi_login_action()`
		`+`
		`+ # We need to create row in 'krb5_login' table`
		`+ user = models.User.query.filter(models.User.username == username).first()`
		`+ if not user:`
		`+ if app.config["FAS_LOGIN"] is True:`
		`+ # We can not create a new user now because we wouldn't get the necessary`
		`+ # e-mail and groups info.`
		`+ return auth_403(`
		`+ "Valid GSSAPI authentication supplied for user '{}', but this "`
		`+ "user doesn't exist in the Copr build system. Please log-in "`
		`+ "using the web-UI (without GSSAPI) first.".format(username)`
		`+ )`
		`+ # Create the user in the database`
		`+ email = username + "@" + krb_config['email_domain']`
		`+ user = UsersLogic.create_user_wrapper(username, email)`
		`+ db.session.add(user)`
		`+`
		`+ krb_login = models.Krb5Login(user=user, primary=username)`
		`+ db.session.add(krb_login)`
		`+ db.session.commit()`
		`+`
		`+ flask.flash("Welcome, {0}".format(user.name), "success")`
		`+ flask.g.user = user`
		`+ flask.session['krb5_login'] = user.name`
		`+ return gssapi_login_action()`

frontend/coprs_frontend/coprs/views/misc.py

file modified

+3 -95

		`@@ -1,9 +1,7 @@`
		`- import os`
		`import base64`
		`import datetime`
		`import functools`
		`from functools import wraps, partial`
		`- import re`
		`from urllib.parse import urlparse`
		`import flask`

		`@@ -23,22 +21,6 @@`
		`from coprs.measure import checkpoint_start`


		`- def create_user_wrapper(username, email, timezone=None):`
		`- expiration_date_token = datetime.date.today() + \`
		`- datetime.timedelta(`
		`- days=flask.current_app.config["API_TOKEN_EXPIRATION"])`
		`-`
		`- copr64 = base64.b64encode(b"copr") + b"##"`
		`- user = models.User(username=username, mail=email,`
		`- timezone=timezone,`
		`- api_login=copr64.decode("utf-8") + helpers.generate_api_token(`
		`- app.config["API_TOKEN_LENGTH"] - len(copr64)),`
		`- api_token=helpers.generate_api_token(`
		`- app.config["API_TOKEN_LENGTH"]),`
		`- api_token_expiration=expiration_date_token)`
		`- return user`
		`-`
		`-`
		`def fed_raw_name(oidname):`
		`oidname_parse = urlparse(oidname)`
		`if not oidname_parse.netloc:`
		`@@ -47,20 +29,6 @@`
		`return oidname_parse.netloc.replace(".{0}".format(config_parse.netloc), "")`


		`- def krb_straighten_username(krb_remote_user):`
		`- # Input should look like 'USERNAME@REALM.TLD', strip realm.`
		`- username = re.sub(r'@.*', '', krb_remote_user)`
		`-`
		`- # But USERNAME part can consist of USER/DOMAIN.TLD.`
		`- # TODO: Do we need more clever thing here?`
		`- username = re.sub('/', '_', username)`
		`-`
		`- # Based on restrictions for project name: "letters, digits, underscores,`
		`- # dashes and dots", it is worth limitting the username here, too.`
		`- # TODO: Store this pattern on one place.`
		`- return username if re.match(r"^[\w.-]+$", username) else None`
		`-`
		`-`
		`@app.before_request`
		`def before_request():`
		`"""`
		`@@ -110,67 +78,6 @@`
		`misc = flask.Blueprint("misc", __name__)`


		`- @misc.route(app.config['KRB5_LOGIN_BASEURI'], methods=["GET"])`
		`- def krb5_login():`
		`- """`
		`- Handle the Kerberos authentication.`
		`-`
		`- Note that if we are able to get here, either the user is authenticated`
		`- correctly, or apache is mis-configured and it does not perform KRB`
		`- authentication at all. Note also, even if that can be considered ugly, we`
		`- are reusing oid's get_next_url feature with kerberos login.`
		`- """`
		`-`
		`- # Already logged in?`
		`- if flask.g.user is not None:`
		`- return flask.redirect(oid.get_next_url())`
		`-`
		`- krb_config = app.config['KRB5_LOGIN']`
		`-`
		`- if app.config["DEBUG"] and 'TEST_REMOTE_USER' in os.environ:`
		`- # For local testing (without krb5 keytab and other configuration)`
		`- flask.request.environ['REMOTE_USER'] = os.environ['TEST_REMOTE_USER']`
		`-`
		`- if 'REMOTE_USER' not in flask.request.environ:`
		`- nocred = "Kerberos authentication failed (no credentials provided)"`
		`- return flask.render_template("403.html", message=nocred), 403`
		`-`
		`- krb_username = flask.request.environ['REMOTE_USER']`
		`- app.logger.debug("krb5 login attempt: " + krb_username)`
		`- username = krb_straighten_username(krb_username)`
		`- if not username:`
		`- message = "invalid krb5 username: " + krb_username`
		`- return flask.render_template("403.html", message=message), 403`
		`-`
		`- krb_login = (`
		`- models.Krb5Login.query`
		`- .filter(models.Krb5Login.primary == username)`
		`- .first()`
		`- )`
		`- if krb_login:`
		`- flask.g.user = krb_login.user`
		`- flask.session['krb5_login'] = krb_login.user.name`
		`- flask.flash(u"Welcome, {0}".format(flask.g.user.name), "success")`
		`- return flask.redirect(oid.get_next_url())`
		`-`
		`- # We need to create row in 'krb5_login' table`
		`- user = models.User.query.filter(models.User.username == username).first()`
		`- if not user:`
		`- # Even the item in 'user' table does not exist, create _now_`
		`- email = username + "@" + krb_config['email_domain']`
		`- user = create_user_wrapper(username, email)`
		`- db.session.add(user)`
		`-`
		`- krb_login = models.Krb5Login(user=user, primary=username)`
		`- db.session.add(krb_login)`
		`- db.session.commit()`
		`-`
		`- flask.flash(u"Welcome, {0}".format(user.name), "success")`
		`- flask.g.user = user`
		`- flask.session['krb5_login'] = user.name`
		`- return flask.redirect(oid.get_next_url())`
		`-`
		`-`
		`def workaround_ipsilon_email_login_bug_handler(f):`
		`"""`
		`We are working around an ipislon issue when people log in with their email,`
		`@@ -242,7 +149,8 @@`
		`user = models.User.query.filter(`
		`models.User.username == username).first()`
		`if not user: # create if not created already`
		`- user = create_user_wrapper(username, resp.email, resp.timezone)`
		`+ user = UsersLogic.create_user_wrapper(username, resp.email,`
		`+ resp.timezone)`
		`else:`
		`user.mail = resp.email`
		`user.timezone = resp.timezone`
		`@@ -311,7 +219,7 @@`
		`def krb5_login_redirect(next=None):`
		`if app.config['KRB5_LOGIN']:`
		`# Pick the first one for now.`
		`- return flask.redirect(flask.url_for("misc.krb5_login",`
		`+ return flask.redirect(flask.url_for("apiv3_ns.krb5_login",`
		`next=next))`
		`flask.flash("Unable to pick krb5 login page", "error")`
		`return flask.redirect(flask.url_for("coprs_ns.coprs_show"))`

pylintrc

file modified

+3 -1

		`@@ -45,7 +45,9 @@`
		`# warn us if parent method doesn't provide the minimal amount of methods,`
		`# reported here: https://github.com/PyCQA/pylint/issues/4352`
		`# It's inconvenient to silence this per-class.`
		`- disable=import-error,too-few-public-methods`
		`+ # consider-using-f-string`
		`+ # "style" warnings, usage of the f-strings is not yet approved by our team`
		`+ disable=import-error,too-few-public-methods,consider-using-f-string`

		`[VARIABLES]`
		`# A regular expression matching names used for dummy variables (i.e. not used).`

pylintrc_clients

file modified

+5 -1

		`@@ -24,7 +24,11 @@`
		`# this error because our packaging CI would discover the problems anyways.`
		`# useless-object-inheritance`
		`# We need to keep compatibility with Python 2 for EL6 and EL7.`
		`- disable=import-error,useless-object-inheritance,super-with-arguments`
		`+ # consider-using-f-string`
		`+ # We still support Python 2.7 (EL7) for clients.`
		`+ # unspecified-encoding`
		`+ # Python2.7: TypeError: 'encoding' is an invalid keyword argument for this function`
		`+ disable=import-error,useless-object-inheritance,super-with-arguments,consider-using-f-string,unspecified-encoding`

		`[VARIABLES]`
		`# A regular expression matching names used for dummy variables (i.e. not used).`

python/copr/v3/__init__.py

file modified

+2 -2

		`@@ -16,7 +16,7 @@`
		`CoprValidationException,`
		`CoprNoConfigException,`
		`CoprConfigException,`
		`- CoprGssapiException)`
		`+ CoprAuthException)`


		`__all__ = [`
		`@@ -37,5 +37,5 @@`
		`"CoprValidationException",`
		`"CoprNoConfigException",`
		`"CoprConfigException",`
		`- "CoprGssapiException",`
		`+ "CoprAuthException",`
		`]`

python/copr/v3/exceptions.py

file modified

+2 -2

		`@@ -66,7 +66,7 @@`
		`pass`


		`- class CoprGssapiException(CoprException):`
		`+ class CoprAuthException(CoprException):`
		`"""`
		`- Exception thrown when the kerberos authentication failed.`
		`+ Copr authentication failure`
		`"""`

python/copr/v3/helpers.py

file modified

+2 -16

		`@@ -4,8 +4,6 @@`
		`import os`
		`import time`
		`import configparser`
		`- import requests`
		`- import requests_gssapi`
		`from munch import Munch`
		`from .exceptions import CoprConfigException, CoprException`

		`@@ -35,7 +33,8 @@`
		`for field in ["username", "login", "token", "copr_url", "gssapi"]:`
		`config[field] = raw_config["copr-cli"].get(field, None)`
		`config["encrypted"] = raw_config["copr-cli"].getboolean("encrypted", True)`
		`- config["gssapi"] = raw_config["copr-cli"].getboolean("gssapi", False)`
		`+ config["gssapi"] = raw_config["copr-cli"].getboolean("gssapi")`
		`+`

		`except configparser.Error as err:`
		`raise CoprConfigException("Bad configuration file: {0}".format(err))`
		`@@ -137,16 +136,3 @@`
		`if build.state != "succeeded":`
		`return False`
		`return True`
		`-`
		`-`
		`- def get_session_cookie(config):`
		`- """`
		`- Call an endpoint to check whether the user has a valid kerberos ticket.`
		`-`
		`- :return: Munch`
		`- """`
		`- url = config["copr_url"] + "/api_v3/gssapi_login/"`
		`- session = requests.Session()`
		`- response = session.get(url, auth=requests_gssapi.HTTPSPNEGOAuth(opportunistic_auth=True), allow_redirects=False)`
		`- cookies = response.cookies.get("session")`
		`- return cookies`

python/copr/v3/proxies/__init__.py

file modified

+90 -27

		`@@ -1,15 +1,20 @@`
		`- import os`
		`+ import errno`
		`import json`
		`- from datetime import datetime, timedelta`
		`+ import time`
		`+ import os`

		`try:`
		`from urllib.parse import urlparse`
		`except ImportError:`
		`from urlparse import urlparse`
		`- from ..helpers import config_from_file, get_session_cookie`
		`- from ..requests import Request, munchify`
		`- from ..helpers import for_all_methods, bind_proxy`
		`- from ..exceptions import CoprGssapiException`
		`+`
		`+ from future.utils import raise_from`
		`+`
		`+ import requests_gssapi`
		`+`
		`+ from ..requests import Request, munchify, requests, handle_errors`
		`+ from ..helpers import for_all_methods, bind_proxy, config_from_file`
		`+ from ..exceptions import CoprAuthException, CoprConfigException`


		`@for_all_methods(bind_proxy)`
		`@@ -21,6 +26,7 @@`
		`def __init__(self, config):`
		`self.config = config`
		`self._auth_token_cached = None`
		`+ self._auth_username = None`
		`self.request = Request(api_base_url=self.api_base_url, connection_attempts=config.get("connection_attempts", 1))`

		`@classmethod`
		`@@ -38,32 +44,78 @@`
		`return self._auth_token_cached`
		`if self.config.get("token"):`
		`self._auth_token_cached = self.config["login"], self.config["token"]`
		`+ self._auth_username = self.config.get("username")`
		`elif self.config.get("gssapi"):`
		`- self._auth_token_cached = self._get_session_cookie_via_gssapi()`
		`+ session_data = self._get_session_cookie_via_gssapi()`
		`+ self._auth_token_cached = session_data["session"]`
		`+ self._auth_username = session_data["name"]`
		`+ else:`
		`+ msg = "GSSAPI disabled and login:token is invalid ({0}:{1})".format(`
		`+ self.config.get("login", "NOT_SET"),`
		`+ self.config.get("token", "NOT_SET"),`
		`+ )`
		`+ raise CoprAuthException(msg)`
		`return self._auth_token_cached`

		`def _get_session_cookie_via_gssapi(self):`
		`- """Get session cookie using gssapi auth"""`
		`- session_cookie = None`
		`+ """`
		`+ Return the cached session for the configured username. If not already`
		`+ cached, new self.get_session_via_gssapi() is performed and result is`
		`+ cached into ~/.config/copr/<session_file>.`
		`+ """`
		`+ session_data = None`
		`url = urlparse(self.config["copr_url"]).netloc`
		`- path_to_session_cookie = os.path.join(os.path.expanduser("~"), '.cache', 'copr', url + '-cookie')`
		`- if os.path.exists(path_to_session_cookie):`
		`- with open(path_to_session_cookie, 'r') as file:`
		`- session_cookie = json.load(file)`
		`- if not session_cookie or datetime.strptime(session_cookie["expiration"], '%Y-%m-%d %H:%M:%S') <= datetime.now():`
		`- session_cookie = get_session_cookie(self.config)`
		`- if session_cookie:`
		`- cachedir = os.path.join(os.path.expanduser("~"), ".cache/copr")`
		`- if not os.path.exists(cachedir):`
		`- os.makedirs(cachedir)`
		`- with open(path_to_session_cookie, 'w') as file:`
		`- session_cookie_dict = {`
		`- "expiration": str(datetime.now() + timedelta(hours=10)).split('.', maxsplit=1)[0],`
		`- "value": session_cookie}`
		`- file.write(json.dumps(session_cookie_dict))`
		`- return session_cookie`
		`- raise CoprGssapiException("Operation requires api authentication")`
		`- return session_cookie["value"]`
		`+ cachedir = os.path.join(os.path.expanduser("~"), ".cache", "copr")`
		`+`
		`+ try:`
		`+ os.makedirs(cachedir)`
		`+ except OSError as err:`
		`+ if err.errno != errno.EEXIST:`
		`+ raise`
		`+`
		`+ session_file = os.path.join(cachedir, url+"-session")`
		`+`
		`+ if os.path.exists(session_file):`
		`+ with open(session_file, "r") as file:`
		`+ session_data = json.load(file)`
		`+`
		`+ if session_data and session_data["expiration"] > time.time():`
		`+ return session_data`
		`+`
		`+ # TODO: create Munch sub-class that returns serializable dict, we`
		`+ # have something like that in Cli: cli/copr_cli/util.py:serializable()`
		`+ session_data = self.get_session_via_gssapi()`
		`+ session_data = session_data.__dict__`
		`+ session_data.pop("__response__", None)`
		`+ session_data.pop("__proxy__", None)`
		`+`
		`+ with open(session_file, "w") as file:`
		`+ session_data["expiration"] = time.time() + 10*3600 # +10 hours`
		`+ file.write(json.dumps(session_data, indent=4) + "\n")`
		`+`
		`+ return session_data`
		`+`
		`+ def get_session_via_gssapi(self):`
		`+ """`
		`+ Obtain a _new_ session using GSSAPI route`
		`+`
		`+ :return: Munch, provides user's "id", "name", "session" cookie, and`
		`+ "expiration".`
		`+ """`
		`+ url = self.config["copr_url"] + "/api_3/gssapi_login/"`
		`+ session = requests.Session()`
		`+ auth = requests_gssapi.HTTPSPNEGOAuth(opportunistic_auth=True)`
		`+ try:`
		`+ response = session.get(url, auth=auth)`
		`+ except requests_gssapi.exceptions.SPNEGOExchangeError as err:`
		`+ msg = "Can not get session for {0} cookie via GSSAPI: {1}".format(`
		`+ self.config["copr_url"], err)`
		`+ raise_from(CoprAuthException(msg), err)`
		`+`
		`+ handle_errors(response)`
		`+ retval = munchify(response)`
		`+ retval.session = response.cookies.get("session")`
		`+ return retval`

		`def home(self):`
		`"""`
		`@@ -85,3 +137,14 @@`
		`self.request.auth = self.auth`
		`response = self.request.send(endpoint=endpoint)`
		`return munchify(response)`
		`+`
		`+`
		`+ def auth_username(self):`
		`+ """`
		`+ Return the username (string) assigned to this configuration. May`
		`+ contact the server and authenticate if needed.`
		`+ """`
		`+ if not self._auth_username:`
		`+ # perform authentication as a side effect`
		`+ _ = self.auth`
		`+ return self._auth_username`

python/copr/v3/requests.py

file modified

+5 -2

		`@@ -9,7 +9,7 @@`
		`from munch import Munch`
		`from future.utils import raise_from`
		`from requests_toolbelt.multipart.encoder import MultipartEncoder, MultipartEncoderMonitor`
		`- from .exceptions import CoprRequestException, CoprNoResultException, CoprTimeoutException, CoprGssapiException`
		`+ from .exceptions import CoprRequestException, CoprNoResultException, CoprTimeoutException, CoprAuthException`


		`GET = "GET"`
		`@@ -80,7 +80,7 @@`
		`try:`
		`response = session.request(**self._request_params)`
		`except requests_gssapi.exceptions.SPNEGOExchangeError as e:`
		`- raise_from(CoprGssapiException("Kerberos ticket has expired."), e)`
		`+ raise_from(CoprAuthException("GSSAPI authentication failed."), e)`
		`except requests.exceptions.ConnectionError:`
		`if i < self.connection_attempts:`
		`time.sleep(sleep)`
		`@@ -139,6 +139,9 @@`
		`if "error" not in response_json:`
		`return`

		`+ if response.status_code == 403:`
		`+ raise CoprAuthException(response_json["error"], response=response)`
		`+`
		`if response.status_code == 404:`
		`raise CoprNoResultException(response_json["error"], response=response)`

python/python-copr.spec

file modified

+1 -1

		`@@ -9,7 +9,7 @@`
		`%endif`

		`Name: python-copr`
		`- Version: 1.116.1.dev`
		`+ Version: 1.116.2.dev`
		`Release: 1%{?dist}`
		`Summary: Python interface for Copr`

no initial comment

zuul commented 2 years ago

Build succeeded.

csdiff-pylint : SUCCESS in 4m 44s

rebased onto e0bd214

2 years ago

zuul commented 2 years ago

Build failed. More information on how to proceed and troubleshoot errors available at https://fedoraproject.org/wiki/Zuul-based-ci

csdiff-pylint : FAILURE in 2m 22s

1 new commit added

frontend, python: put /gssapi_login/ page under apiv3

2 years ago

zuul commented 2 years ago

Build failed. More information on how to proceed and troubleshoot errors available at https://fedoraproject.org/wiki/Zuul-based-ci

csdiff-pylint : FAILURE in 2m 10s

1 new commit added

python, cli: make the auth more robust

2 years ago

zuul commented 2 years ago

Build failed. More information on how to proceed and troubleshoot errors available at https://fedoraproject.org/wiki/Zuul-based-ci

csdiff-pylint : FAILURE in 2m 13s

praiskup commented 2 years ago

This is going to fail the CI, still WIP. But the concept is hopefully OK for being reviewed?

rebased onto c445c1ef4e08603e28d48f03b854b404985075c7

2 years ago

8 new commits added

python, cli: make the auth more robust
frontend, python: put /gssapi_login/ page under apiv3
frontend: finalize the gssapi v3 route
frontend: package mod_auth_gssapi is recommended
frontend: fix the logging configuration
frontend, cli, docker: fix docs related to GSSAPI
cli: api: better error message when authentication fails
ci: disable consider-using-f-string PyLint warnings

2 years ago

zuul commented 2 years ago

Build failed. More information on how to proceed and troubleshoot errors available at https://fedoraproject.org/wiki/Zuul-based-ci

csdiff-pylint : FAILURE in 2m 06s

8 new commits added

python, cli: make the auth more robust
frontend, python: put /gssapi_login/ page under apiv3
frontend: finalize the gssapi v3 route
frontend: package mod_auth_gssapi is recommended
frontend: fix the logging configuration
frontend, cli, docker: fix docs related to GSSAPI
cli: api: better error message when authentication fails
ci: disable consider-using-f-string PyLint warnings

2 years ago

zuul commented 2 years ago

Build failed. More information on how to proceed and troubleshoot errors available at https://fedoraproject.org/wiki/Zuul-based-ci

csdiff-pylint : FAILURE in 1m 51s

8 new commits added

python, cli: make the auth more robust
frontend, python: put /gssapi_login/ page under apiv3
frontend: finalize the gssapi v3 route
frontend: package mod_auth_gssapi is recommended
frontend: fix the logging configuration
frontend, cli, docker: fix docs related to GSSAPI
cli: python: better error message when authentication fails
ci: disable consider-using-f-string PyLint warnings

2 years ago

zuul commented 2 years ago

Build failed. More information on how to proceed and troubleshoot errors available at https://fedoraproject.org/wiki/Zuul-based-ci

csdiff-pylint : FAILURE in 3m 02s

1 new commit added

cli: new-line separate the exit statuses in manual page

2 years ago

zuul commented 2 years ago

Build succeeded.

csdiff-pylint : SUCCESS in 1m 49s

rebased onto e58bd7f

2 years ago

zuul commented 2 years ago

Build succeeded.

csdiff-pylint : SUCCESS in 3m 34s

12 new commits added

frontend: the first login needs to be via ipsilon
cli: new-line separate the exit statuses in manual page
python, cli: make the auth more robust
frontend, python: put /gssapi_login/ page under apiv3
frontend: finalize the gssapi v3 route
frontend: package mod_auth_gssapi is recommended
frontend: fix the logging configuration
frontend, cli, docker: fix docs related to GSSAPI
cli: python: better error message when authentication fails
ci: disable 'unspecified-encoding' for clients
ci: disable consider-using-f-string PyLint warnings
ci: better ignore "missing-docstring" on PyLint 2.13+

2 years ago

zuul commented 2 years ago

Build succeeded.

csdiff-pylint : SUCCESS in 2m 06s

12 new commits added

frontend: the first login needs to be via ipsilon
cli: new-line separate the exit statuses in manual page
python, cli: make the auth more robust
frontend, python: put /gssapi_login/ page under apiv3
frontend: finalize the gssapi v3 route
frontend: package mod_auth_gssapi is recommended
frontend: fix the logging configuration
frontend, cli, docker: fix docs related to GSSAPI
cli: python: better error message when authentication fails
ci: disable 'unspecified-encoding' for clients
ci: disable consider-using-f-string PyLint warnings
ci: better ignore "missing-docstring" on PyLint 2.13+

2 years ago

12 new commits added

frontend: the first login needs to be via ipsilon
cli: new-line separate the exit statuses in manual page
python, cli: make the auth more robust
frontend, python: put /gssapi_login/ page under apiv3
frontend: finalize the gssapi v3 route
frontend: package mod_auth_gssapi is recommended
frontend: fix the logging configuration
frontend, cli, docker: fix docs related to GSSAPI
cli: python: better error message when authentication fails
ci: disable 'unspecified-encoding' for clients
ci: disable consider-using-f-string PyLint warnings
ci: better ignore "missing-docstring" on PyLint 2.13+

2 years ago

zuul commented 2 years ago

Build succeeded.

csdiff-pylint : SUCCESS in 2m 01s

1 new commit added

cli: ensure that (error) logging works even without --debug

2 years ago

zuul commented 2 years ago

Build failed. More information on how to proceed and troubleshoot errors available at https://fedoraproject.org/wiki/Zuul-based-ci

csdiff-pylint : FAILURE in 2m 31s

13 new commits added

cli: ensure that (error) logging works even without --debug
frontend: the first login needs to be via ipsilon
cli: new-line separate the exit statuses in manual page
python, cli: make the auth more robust
frontend, python: put /gssapi_login/ page under apiv3
frontend: finalize the gssapi v3 route
frontend: package mod_auth_gssapi is recommended
frontend: fix the logging configuration
frontend, cli, docker: fix docs related to GSSAPI
cli: python: better error message when authentication fails
ci: disable 'unspecified-encoding' for clients
ci: disable consider-using-f-string PyLint warnings
ci: better ignore "missing-docstring" on PyLint 2.13+

2 years ago

praiskup commented 2 years ago

IMO ready for review, finally. Sorry for so big PR.

zuul commented 2 years ago

Build succeeded.

csdiff-pylint : SUCCESS in 2m 56s

frostyx commented on line 64 of frontend/coprs_frontend/coprs/views/apiv3_ns/apiv3_general.py 2 years ago

My guess is that the AccessRestricted should be properly handled even for web-ui, so this function may be simply an oneliner raise AccessRestricted(message).

Maybe ...

frostyx commented 2 years ago

Heh, couple of times I wanted to point out some potential error and everything got fixed in some follow-up commit :-)

LGTM

praiskup commented 2 years ago

My guess is that the AccessRestricted should be properly handled even for web-ui, so this function may be simply an oneliner raise AccessRestricted(message).

I don't think it is handled; we are in the apiv3_ns code, which simply returns json-formatted errors?