From 3b550bfbd3e07eb74b26066e19f355d481d5933c Mon Sep 17 00:00:00 2001 From: Adam Williamson Date: Dec 23 2023 23:24:41 +0000 Subject: [PATCH 1/5] openidc provider: respect secure=no This is useful in e.g. development environments where you don't want to deal with managing SSL certificates. Signed-off-by: Adam Williamson --- diff --git a/ipsilon/providers/openidcp.py b/ipsilon/providers/openidcp.py index 529ca59..d4d94c3 100644 --- a/ipsilon/providers/openidcp.py +++ b/ipsilon/providers/openidcp.py @@ -292,6 +292,8 @@ class Installer(ProviderInstaller): m.write(keyset.export()) proto = 'https' + if opts['secure'].lower() == 'no': + proto = 'http' url = '%s://%s%s/openidc/' % ( proto, opts['hostname'], opts['instanceurl']) From a3d5f82aa42e22bfa8adcebd9c07f612ae8ad221 Mon Sep 17 00:00:00 2001 From: Adam Williamson Date: Dec 23 2023 23:31:05 +0000 Subject: [PATCH 2/5] httpd config: Listen on port specified in hostname The tests often specify a port number as part of the --hostname arg to ipsilon-server-install, and this...partly works, at least it works far enough for the tests (e.g. it gets included in the various URLs that are generated and saved to ipsilon's internal config during install). But for real world use it's a bit incomplete. This makes it canonical by mentioning it in the help text and adding a Listen line to the Apache config we write. Doing this breaks the etcd test, because the tests write an httpd config that includes a Listen line of its own with the address specified, and also include the config written by ipsilon-server-install, so we wind up with a line like: Listen 45080 https in the i-s-i generated config, and one like: Listen 127.0.0.10:45080 https in the test-generated config. Those lines conflict and cause the server not to start up properly. Somehow, the socket wrappers used by most of the tests hide this problem so they pass, but the etcd test cannot use socket wrappers and fails. Dropping the address-specific line from the test-generated and relying on the i-s-i written line doesn't work either as it causes problems when tests try to run multiple httpd instances listening on different addresses, so we're left with only the option of hacking the line out of the i-s-i config in the test suite. Signed-off-by: Adam Williamson --- diff --git a/ipsilon/install/ipsilon-server-install b/ipsilon/install/ipsilon-server-install index b79db87..c68b376 100755 --- a/ipsilon/install/ipsilon-server-install +++ b/ipsilon/install/ipsilon-server-install @@ -97,6 +97,16 @@ def install(plugins, args): shutil.move(idp_conf, '%s.backup.%s' % (idp_conf, now)) if not os.path.exists(instance_conf): os.makedirs(instance_conf, 0o700) + proto = "https" + secure = True + if args['secure'] == "no": + proto = "http" + secure = False + listenline = "" + splithost = args['hostname'].split(':') + if len(splithost) > 1: + portnum = splithost[1] + listenline = "Listen %s %s" % (portnum, proto) confopts = {'instance': args['instance'], 'instanceurl': args['instanceurl'], 'needs_mount': args.get('needs_mount'), @@ -119,7 +129,8 @@ def install(plugins, args): 'samlsessionsdb': args['samlsessions_dburi'] or args[ 'database_url'] % {'datadir': args['data_dir'], 'dbname': 'saml2sessions'}, - 'secure': "False" if args['secure'] == "no" else "True", + 'secure': secure, + 'listenline': listenline, 'debugging': "True" if args['server_debugging'] else "False"} # Testing database sessions if 'session_type' in args: @@ -388,7 +399,7 @@ def parse_args(plugins): parser.add_argument('--authorization-order', dest='az_order', help='Comma separated list of authorization plugins') parser.add_argument('--hostname', - help="Machine's fully qualified host name") + help="Machine's fully qualified host name, can also include :port") parser.add_argument('--instance', default='idp', help="IdP instance name, each is a separate idp") parser.add_argument('--root-instance', default=False, action='store_true', diff --git a/templates/install/idp.conf b/templates/install/idp.conf index e2fffa7..ab9e29d 100644 --- a/templates/install/idp.conf +++ b/templates/install/idp.conf @@ -1,3 +1,5 @@ +${listenline} + Alias ${instanceurl}/ui ${staticdir}/ui Alias /.well-known ${wellknowndir} Alias ${instanceurl}/cache /var/cache/ipsilon diff --git a/tests/helpers/common.py b/tests/helpers/common.py index afbe09c..a0a3d38 100755 --- a/tests/helpers/common.py +++ b/tests/helpers/common.py @@ -264,6 +264,17 @@ basicConstraints = CA:false""" % {'certdir': os.path.join(self.testdir, stdout=self.stdout, stderr=self.stderr) os.symlink(os.path.join(self.rootdir, 'ipsilon'), os.path.join(self.testdir, 'lib', name, 'ipsilon')) + # drop the Listen line written by ipsilon-server-install from + # the config, as it will conflict with the address-specific + # Listen line written by setup_http and break stuff + isiconf = os.path.join(os.path.dirname(http_conf_file), "conf.d", "ipsilon-%s.conf" % name) + with open(isiconf, 'r', encoding='utf-8') as isiconfh: + lines = isiconfh.readlines() + with open(isiconf, 'w', encoding='utf-8') as isiconfh: + for line in lines: + if line.startswith("Listen"): + continue + isiconfh.write(line) return http_conf_file From ab59d6c643cefb93673247918fc4e9c2c3f67748 Mon Sep 17 00:00:00 2001 From: Adam Williamson Date: Dec 24 2023 00:11:49 +0000 Subject: [PATCH 3/5] httpd config: include ServerName directive This adds a ServerName line to the httpd config generated by ipsilon-server-install. As documented by Apache, not including this causes Apache to auto-detect it, and it will often get it wrong. Doing this is a bit "aggressive" because it's a directive that can only be specified once and the last specification of it wins, so this could theoretically surprise an admin. In practice I don't really think it's a problem, though, I don't really think people are in the habit of deploying authentication services on shared hosts or anything like that. It probably doesn't do a whole lot on an Ipsilon server anyhow, but it *is* used in the SSL redirect block we write into the file, so if it's not set correctly, that redirect block won't work right. Signed-off-by: Adam Williamson --- diff --git a/ipsilon/install/ipsilon-server-install b/ipsilon/install/ipsilon-server-install index c68b376..1daa54d 100755 --- a/ipsilon/install/ipsilon-server-install +++ b/ipsilon/install/ipsilon-server-install @@ -103,10 +103,13 @@ def install(plugins, args): proto = "http" secure = False listenline = "" + portseg = "" splithost = args['hostname'].split(':') if len(splithost) > 1: portnum = splithost[1] + portseg = ":%s" % portnum listenline = "Listen %s %s" % (portnum, proto) + servername = "%s://%s%s" % (proto, args['hostname'], portseg) confopts = {'instance': args['instance'], 'instanceurl': args['instanceurl'], 'needs_mount': args.get('needs_mount'), @@ -131,6 +134,7 @@ def install(plugins, args): 'dbname': 'saml2sessions'}, 'secure': secure, 'listenline': listenline, + 'servername': servername, 'debugging': "True" if args['server_debugging'] else "False"} # Testing database sessions if 'session_type' in args: diff --git a/templates/install/idp.conf b/templates/install/idp.conf index ab9e29d..a572f2c 100644 --- a/templates/install/idp.conf +++ b/templates/install/idp.conf @@ -1,5 +1,7 @@ ${listenline} +ServerName {servername} + Alias ${instanceurl}/ui ${staticdir}/ui Alias /.well-known ${wellknowndir} Alias ${instanceurl}/cache /var/cache/ipsilon From 770dc1d3c9e373c05523e754cb09341fa1e8d268 Mon Sep 17 00:00:00 2001 From: Adam Williamson Date: Dec 25 2023 00:05:15 +0000 Subject: [PATCH 4/5] openidcp: allow setting default attribute mapping at install This was not wired up to the install script, so it could only be set by the IPA helper. In some circumstances it might be nice to set it explicitly on the command line (e.g. when setting up a Bodhi development environment without using IPA). Signed-off-by: Adam Williamson --- diff --git a/ipsilon/helpers/ipa.py b/ipsilon/helpers/ipa.py index 19b2aea..cfa6ad4 100644 --- a/ipsilon/helpers/ipa.py +++ b/ipsilon/helpers/ipa.py @@ -154,7 +154,7 @@ class Installer(EnvHelpersInstaller): opts['info_sssd'] = 'yes' if not any(lm in opts['lm_order'] for lm in ('form', 'pam')): opts['lm_order'].append('pam') - if opts['openidc'] == 'yes': + if opts['openidc'] == 'yes' and not opts['openidc_default_attribute_mapping']: opts['openidc_default_attribute_mapping'] = [ ["*", "*"], ["_groups", "groups"], diff --git a/ipsilon/providers/openidcp.py b/ipsilon/providers/openidcp.py index d4d94c3..3bb3d8f 100644 --- a/ipsilon/providers/openidcp.py +++ b/ipsilon/providers/openidcp.py @@ -267,6 +267,8 @@ class Installer(ProviderInstaller): help='Salt to use for pairwise subject subjects') group.add_argument('--openidc-extensions', default='', help='List of OpenID Connect Extensions to enable') + group.add_argument('--openidc-default-attribute-mapping', default='', + help='OpenID Connect default attribute mapping (JSON list)') def configure(self, opts, changes): if opts['openidc'] != 'yes': @@ -319,7 +321,11 @@ class Installer(ProviderInstaller): 'idp subject salt': subject_salt} opt_dam = opts.get('openidc_default_attribute_mapping') if opt_dam: - config['default attribute mapping'] = json.dumps(opt_dam) + if isinstance(opt_dam, str): + config['default attribute mapping'] = opt_dam + else: + config['default attribute mapping'] = json.dumps(opt_dam) + po.save_plugin_config(config) # Update global config to add login plugin From c404ddb7fa40ede2b4a149216e432252381d55b4 Mon Sep 17 00:00:00 2001 From: Adam Williamson Date: Dec 25 2023 00:10:28 +0000 Subject: [PATCH 5/5] testauth: add a mechanism to specify groups via username This adds a mechanism to control testauth's reported group memberships via the username passed. You can pass a username like 'guest:groups=foo,bar', and you will be logged in as user 'guest' with (only) the group memberships 'foo' and 'bar'. This provides finer-grained options for testing group memberships beyond just setting a default group membership for all users. Signed-off-by: Adam Williamson --- diff --git a/ipsilon/login/authtest.py b/ipsilon/login/authtest.py index a3375f9..23b7a9f 100644 --- a/ipsilon/login/authtest.py +++ b/ipsilon/login/authtest.py @@ -18,19 +18,28 @@ class TestAuth(LoginFormBase): error = None if username and password: + groups = [] + if ":" in username: + userspec = username.split(":") + username = userspec[0] + for cmd in userspec[1:]: + if cmd.startswith("groups="): + groups = cmd[7:].split(",") + else: + err = f"testauth: unhandled username command {cmd} from username {username}" + cherrypy.log.error(err) if password == 'ipsilon': cherrypy.log("User %s successfully authenticated." % username) + if not groups: + groups = [username] + groups.extend(self.lm.groups or []) testdata = { 'givenname': 'Test User δΈ€', 'surname': username, 'fullname': 'Test User %s' % username, 'email': '%s@example.com' % username, - '_groups': [username] + '_groups': groups } - groups = self.lm.groups - if groups is not None: - self.debug('groups is %s' % repr(groups)) - testdata['_groups'].extend(groups) return self.lm.auth_successful(self.trans, username, 'password', testdata) else: @@ -82,6 +91,7 @@ Form based TEST login Manager, DO NOT EVER ACTIVATE IN PRODUCTION """ 'Extra groups') ) + @property def help_text(self): return self.get_config_value('help text') @@ -114,7 +124,7 @@ class Installer(LoginManagerInstaller): group.add_argument('--testauth', choices=['yes', 'no'], default='no', help='Configure PAM authentication') group.add_argument('--testauth-groups', action='store', - help='Extra groups for the testauth user') + help='Extra groups for all testauth users') def configure(self, opts, changes): if opts['testauth'] != 'yes':