URL: https://github.com/freeipa/freeipa/pull/2557 Author: tiran Title: #2557: [Backport][ipa-4-7] Fix ipa user-add --radius=radiusserver Action: opened
PR body: """ This PR was opened automatically because PR #2530 was pushed to master and backport to ipa-4-7 is required. """ To pull the PR as Git branch: git remote add ghfreeipa https://github.com/freeipa/freeipa git fetch ghfreeipa pull/2557/head:pr2557 git checkout pr2557
From 27011e597638e51fd82b17ea4edc8075dee8ea37 Mon Sep 17 00:00:00 2001 From: Florence Blanc-Renaud <f...@redhat.com> Date: Tue, 6 Nov 2018 16:43:29 +0100 Subject: [PATCH 1/4] ipa user-add: add optional objectclass for radius-username The command "ipa user-add --radius-username" fails with ipa: ERROR: attribute "ipatokenRadiusUserName" not allowed because it does not add the objectclass ipatokenradiusproxyuser that is required by the attribute ipatokenradiususername. The issue happens with ipa user-add / stageuser-add / user-mod / stageuser-mod. The fix adds the objectclass when needed in the pre_common_callback method of baseuser_add and baseuser_mod (ensuring that user and stageuser commands are fixed). Fixes https://pagure.io/freeipa/issue/7569 --- ipaserver/plugins/baseuser.py | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/ipaserver/plugins/baseuser.py b/ipaserver/plugins/baseuser.py index eed750e68e..1b01758010 100644 --- a/ipaserver/plugins/baseuser.py +++ b/ipaserver/plugins/baseuser.py @@ -485,6 +485,9 @@ def pre_common_callback(self, ldap, dn, entry_attrs, attrs_list, *keys, assert isinstance(dn, DN) set_krbcanonicalname(entry_attrs) self.obj.convert_usercertificate_pre(entry_attrs) + if entry_attrs.get('ipatokenradiususername', None): + add_missing_object_class(ldap, u'ipatokenradiusproxyuser', dn, + entry_attrs, update=False) def post_common_callback(self, ldap, dn, entry_attrs, *keys, **options): assert isinstance(dn, DN) @@ -573,8 +576,10 @@ def check_userpassword(self, entry_attrs, **options): setattr(context, 'randompassword', entry_attrs['userpassword']) def check_objectclass(self, ldap, dn, entry_attrs): - if ('ipasshpubkey' in entry_attrs or 'ipauserauthtype' in entry_attrs - or 'userclass' in entry_attrs or 'ipatokenradiusconfiglink' in entry_attrs): + # Some attributes may require additional object classes + special_attrs = {'ipasshpubkey', 'ipauserauthtype', 'userclass', + 'ipatokenradiusconfiglink', 'ipatokenradiususername'} + if special_attrs.intersection(entry_attrs): if 'objectclass' in entry_attrs: obj_classes = entry_attrs['objectclass'] else: @@ -602,6 +607,15 @@ def check_objectclass(self, ldap, dn, entry_attrs): answer = self.api.Object['radiusproxy'].get_dn_if_exists(cl) entry_attrs['ipatokenradiusconfiglink'] = answer + # Note: we could have used the method add_missing_object_class + # but since the data is already fetched and lowercased in + # obj_classes, it is more efficient to use the same approach + # as the code right above these lines + if 'ipatokenradiususername' in entry_attrs: + if 'ipatokenradiusproxyuser' not in obj_classes: + entry_attrs['objectclass'].append( + 'ipatokenradiusproxyuser') + def pre_common_callback(self, ldap, dn, entry_attrs, attrs_list, *keys, **options): assert isinstance(dn, DN) From 61344fe63572b50021b73c74ea3adfe962cf3f88 Mon Sep 17 00:00:00 2001 From: Florence Blanc-Renaud <f...@redhat.com> Date: Wed, 7 Nov 2018 11:22:25 +0100 Subject: [PATCH 2/4] tests: add xmlrpc test for ipa user-add --radius-username Add a xmlrpc test for ipa user-add/user-mod --radius-username The command were previously failing because the objectclass ipatokenradiusproxyuser was not automatically added when the attribute ipatokenRadiusUserName was added to the entry. The test ensures that the command is now succeeding. Related to https://pagure.io/freeipa/issue/7569 --- ipatests/test_xmlrpc/test_user_plugin.py | 29 ++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/ipatests/test_xmlrpc/test_user_plugin.py b/ipatests/test_xmlrpc/test_user_plugin.py index 08d73f4b8f..06a67cfb6b 100644 --- a/ipatests/test_xmlrpc/test_user_plugin.py +++ b/ipatests/test_xmlrpc/test_user_plugin.py @@ -138,6 +138,19 @@ def user_npg2(request, group): return tracker.make_fixture(request) +@pytest.fixture(scope='class') +def user_radius(request): + """ User tracker fixture for testing users with radius user name """ + tracker = UserTracker(name=u'radiususer', givenname=u'radiususer', + sn=u'radiususer1', + ipatokenradiususername=u'radiususer') + tracker.track_create() + tracker.attrs.update( + objectclass=objectclasses.user + [u'ipatokenradiusproxyuser'] + ) + return tracker.make_fixture(request) + + @pytest.fixture(scope='class') def group(request): tracker = GroupTracker(name=u'group1') @@ -448,6 +461,15 @@ def test_rename_to_invalid_login(self, user): error=u'may only include letters, numbers, _, -, . and $')): command() + def test_add_radius_username(self, user): + """ Test for ticket 7569: Try to add --radius-username """ + user.ensure_exists() + command = user.make_update_command( + updates=dict(ipatokenradiususername=u'radiususer') + ) + command() + user.delete() + @pytest.mark.tier1 class TestCreate(XMLRPC_test): @@ -663,6 +685,13 @@ def test_create_with_numeric_only_username(self): )): testuser.create() + def test_create_with_radius_username(self, user_radius): + """Test for issue 7569: try to create a user with --radius-username""" + command = user_radius.make_create_command() + result = command() + user_radius.check_create(result) + user_radius.delete() + @pytest.mark.tier1 class TestUserWithGroup(XMLRPC_test): From 67def9213fb7dafc68e00832699f950150f7bd3b Mon Sep 17 00:00:00 2001 From: Florence Blanc-Renaud <f...@redhat.com> Date: Wed, 7 Nov 2018 16:58:04 +0100 Subject: [PATCH 3/4] radiusproxy: add permission for reading radius proxy servers A non-admin user which has the "User Administrator" role cannot add a user with ipa user-add --radius=<proxy> because the call needs to read the radius proxy server entries. The fix adds a System permission for reading radius proxy server entries (all attributes except the ipatokenradiussecret). This permission is added to the already existing privileges "User Administrators" and "Stage User Administrators", so that the role "User Administrator" can call ipa [stage]user-add|mod --radius=<proxy> Fixes: https://pagure.io/freeipa/issue/7570 --- ACI.txt | 2 ++ ipaserver/plugins/radiusproxy.py | 19 +++++++++++++++++++ 2 files changed, 21 insertions(+) diff --git a/ACI.txt b/ACI.txt index e5134a55f8..82820ca210 100644 --- a/ACI.txt +++ b/ACI.txt @@ -234,6 +234,8 @@ dn: cn=IPA.EXAMPLE,cn=kerberos,dc=ipa,dc=example aci: (targetattr = "krbmaxpwdlife || krbminpwdlife || krbpwdfailurecountinterval || krbpwdhistorylength || krbpwdlockoutduration || krbpwdmaxfailure || krbpwdmindiffchars || krbpwdminlength")(targetfilter = "(objectclass=krbpwdpolicy)")(version 3.0;acl "permission:System: Modify Group Password Policy";allow (write) groupdn = "ldap:///cn=System: Modify Group Password Policy,cn=permissions,cn=pbac,dc=ipa,dc=example";) dn: cn=IPA.EXAMPLE,cn=kerberos,dc=ipa,dc=example aci: (targetattr = "cn || cospriority || createtimestamp || entryusn || krbmaxpwdlife || krbminpwdlife || krbpwdfailurecountinterval || krbpwdhistorylength || krbpwdlockoutduration || krbpwdmaxfailure || krbpwdmindiffchars || krbpwdminlength || modifytimestamp || objectclass")(targetfilter = "(objectclass=krbpwdpolicy)")(version 3.0;acl "permission:System: Read Group Password Policy";allow (compare,read,search) groupdn = "ldap:///cn=System: Read Group Password Policy,cn=permissions,cn=pbac,dc=ipa,dc=example";) +dn: cn=radiusproxy,dc=ipa,dc=example +aci: (targetattr = "cn || createtimestamp || description || entryusn || ipatokenradiusretries || ipatokenradiusserver || ipatokenradiustimeout || ipatokenusermapattribute || modifytimestamp || objectclass")(targetfilter = "(objectclass=ipatokenradiusconfiguration)")(version 3.0;acl "permission:System: Read Radius Servers";allow (compare,read,search) groupdn = "ldap:///cn=System: Read Radius Servers,cn=permissions,cn=pbac,dc=ipa,dc=example";) dn: cn=Realm Domains,cn=ipa,cn=etc,dc=ipa,dc=example aci: (targetattr = "associateddomain")(targetfilter = "(objectclass=domainrelatedobject)")(version 3.0;acl "permission:System: Modify Realm Domains";allow (write) groupdn = "ldap:///cn=System: Modify Realm Domains,cn=permissions,cn=pbac,dc=ipa,dc=example";) dn: cn=Realm Domains,cn=ipa,cn=etc,dc=ipa,dc=example diff --git a/ipaserver/plugins/radiusproxy.py b/ipaserver/plugins/radiusproxy.py index f638431f69..7b853555f0 100644 --- a/ipaserver/plugins/radiusproxy.py +++ b/ipaserver/plugins/radiusproxy.py @@ -29,6 +29,7 @@ from ipalib.plugable import Registry from ipalib.util import validate_hostname, validate_ipaddr from ipalib.errors import ValidationError +from ipapython.dn import DN import re __doc__ = _(""" @@ -147,6 +148,24 @@ class radiusproxy(LDAPObject): ), ) + managed_permissions = { + 'System: Read Radius Servers': { + 'replaces_global_anonymous_aci': True, + 'ipapermright': {'read', 'search', 'compare'}, + 'ipapermdefaultattr': { + 'cn', 'objectclass', 'ipatokenradiusserver', 'description', + 'ipatokenradiustimeout', 'ipatokenradiusretries', + 'ipatokenusermapattribute' + }, + 'ipapermlocation': DN(container_dn, api.env.basedn), + 'ipapermtargetfilter': { + '(objectclass=ipatokenradiusconfiguration)'}, + 'default_privileges': { + 'User Administrators', + 'Stage User Administrators'}, + } + } + @register() class radiusproxy_add(LDAPCreate): __doc__ = _('Add a new RADIUS proxy server.') From 75ac01dacbeb37d527f30c55cf8b75fe17340389 Mon Sep 17 00:00:00 2001 From: Florence Blanc-Renaud <f...@redhat.com> Date: Wed, 7 Nov 2018 17:02:31 +0100 Subject: [PATCH 4/4] ipatests: add integration test for "Read radius servers" perm Add a new integration test for the following scenario: - create a user with the "User Administrator" role - as this user, create a user with a --radius=<radius_proxy_server> This scenario was previously failing because ipa user-add --radius requires read access to the radius server entries, and there was no permission granting this access. Related to https://pagure.io/freeipa/issue/7570 --- .../test_integration/test_user_permissions.py | 43 +++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/ipatests/test_integration/test_user_permissions.py b/ipatests/test_integration/test_user_permissions.py index 38e72fd9d2..13a0c983a2 100644 --- a/ipatests/test_integration/test_user_permissions.py +++ b/ipatests/test_integration/test_user_permissions.py @@ -98,6 +98,49 @@ def test_stageuser_show_as_alternate_admin(self): result = self.master.run_command(['ipa', 'stageuser-show', stageuser]) assert 'Kerberos keys available: True' in result.stdout_text + def test_user_add_withradius(self): + """ + Test that a user with User Administrator role can call + ipa user-add --radius myradius + to create a user with an assigned Radius Proxy Server. + + This is a test case for issue 7570 + """ + # kinit admin + tasks.kinit_admin(self.master) + + # Create a radius proxy server + radiusproxy = 'myradius' + secret = 'Secret123' + radius_secret_confirmation = "%s\n%s\n" % (secret, secret) + self.master.run_command( + ['ipa', 'radiusproxy-add', radiusproxy, + '--server', 'radius.example.com', '--secret'], + stdin_text=radius_secret_confirmation) + + # Create a user with 'User Administrator' role + altuser = 'specialuser' + password = 'SpecialUser123' + password_confirmation = "%s\n%s\n" % (password, password) + self.master.run_command( + ['ipa', 'user-add', altuser, '--first', altuser, '--last', altuser, + '--password'], + stdin_text=password_confirmation) + self.master.run_command( + ['ipa', 'role-add-member', "User Administrator", + '--user', altuser]) + + # kinit as altuser to initialize the password + altuser_kinit = "%s\n%s\n%s\n" % (password, password, password) + self.master.run_command(['kinit', altuser], stdin_text=altuser_kinit) + # call ipa user-add with --radius=... + # this call requires read access to radius proxy servers + self.master.run_command( + ['ipa', 'user-add', '--first', 'test', '--last', 'test', + '--user-auth-type', 'radius', '--radius-username', 'testradius', + 'testradius', '--radius', radiusproxy]) + + class TestInstallClientNoAdmin(IntegrationTest): num_clients = 1
_______________________________________________ FreeIPA-devel mailing list -- freeipa-devel@lists.fedorahosted.org To unsubscribe send an email to freeipa-devel-le...@lists.fedorahosted.org Fedora Code of Conduct: https://getfedora.org/code-of-conduct.html List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedorahosted.org/archives/list/freeipa-devel@lists.fedorahosted.org
