On 07/14/2016 11:25 AM, Lenka Doudova wrote:
On 07/14/2016 09:20 AM, Lenka Doudova wrote:
On 07/13/2016 04:48 PM, Milan Kubík wrote:
On 07/11/2016 01:34 PM, Lenka Doudova wrote:
On 07/08/2016 02:24 PM, Milan Kubík wrote:
On 07/01/2016 05:13 PM, Lenka Doudova wrote:
On 07/01/2016 02:42 PM, Milan Kubík wrote:
On 06/16/2016 03:23 PM, Lenka Doudova wrote:
Hi,
attached are tests for authentication indicators. Please note:
1. newly created service tracker is not exactly complete, list
of unimplemented methods is in doc. These methods can be filled
in when existing declarative tests are refactored.
2. patch 0015 depends on 0014, so it should not be pushed
without it.
Lenka
patch 0014:
In the update method, what happens when the updated attributes
contain addattr? It is not clear to me. Is it necessary?
Example:
ipa service-mod SRV --addattr="authind=radius"
Result:
The way the tracker works, this adds
/u'addattr="authind=radius"'/ to the list of expected results
(result of /self.attrs.update(updates)/. Of course nothing like
that appears anywhere, so in case there's the /--addattr/ option,
it's necessary to ensure it won't get to the /self.attrs/ atribute.
patch 0015:
host1 and service2 do not tell anything about the purpose of the
fixture. Please assign more descriptive names to them.
Why do the fixtures have 'function' scope? Does the service
entry exist during the second and third test case?
Renamed.
patch 0016:
Per offline discussion, admin user has no special privileges
here, LGTM.
--
Milan Kubik
Thanks for review, fixed patches (14.2 and 15.2) attached.
Lenka
NACK,
the update method of ServiceTracker creates the entry if it
doesn't exist. Why? I know the base class has this problem also
[1], though.
Given this will be addressed, the fixtures in the xmlrpc test will
fail since the fixture scope is wrong - function instead of class.
[1]: https://fedorahosted.org/freeipa/ticket/6045
--
Milan Kubik
Hi,
fixed patch attached. I chose to leave the scope as it was (in case
one test breaks and entry is not even created, the other tests can
still be successful), but I removed the creation command from
ServiceTracker update method and called it directly in the tests,
so there shouldn't be hidden actions.
Lenka
Thanks for the updated patches. There are still some issues I
haven't noticed before:
service tracker: track_create method doesn't set self.exists flag
which is needed
In the xmlrpc test method `test_adding_second_indicator` uses
'addattr' to set the indicator, why?
--
Milan Kubik
Hi,
fixes for comments in attached patches.
After offline discussion, I removed the usage of "addattr" and
replaced it with test to add two indicators in one command.
Lenka
One more problem was pointed out to me offline, attaching changed
patch 0014.
Lenka
Resending the complete patch set.
L.
From aaf2afa849b3156603e057d99fe7f31247b08725 Mon Sep 17 00:00:00 2001
From: Lenka Doudova <ldoud...@redhat.com>
Date: Wed, 15 Jun 2016 13:40:00 +0200
Subject: [PATCH] Tests: Tracker class for services
Provides basic service tracker, so far for purposes of [1].
Tracker is not complete, some methods will need to be added in case of service test refactoring.
[1] https://fedorahosted.org/freeipa/ticket/433
---
ipatests/test_xmlrpc/tracker/service_plugin.py | 152 +++++++++++++++++++++++++
1 file changed, 152 insertions(+)
create mode 100644 ipatests/test_xmlrpc/tracker/service_plugin.py
diff --git a/ipatests/test_xmlrpc/tracker/service_plugin.py b/ipatests/test_xmlrpc/tracker/service_plugin.py
new file mode 100644
index 0000000000000000000000000000000000000000..89c27e82161b70a77ac4e45ffd8b60d3da657163
--- /dev/null
+++ b/ipatests/test_xmlrpc/tracker/service_plugin.py
@@ -0,0 +1,152 @@
+# -*- coding: utf-8 -*-
+#
+# Copyright (C) 2016 FreeIPA Contributors see COPYING for license
+#
+
+import six
+
+from ipalib import api
+from ipatests.test_xmlrpc.tracker.base import Tracker
+from ipatests.test_xmlrpc.xmlrpc_test import fuzzy_uuid
+from ipatests.test_xmlrpc import objectclasses
+from ipatests.util import assert_deepequal
+from ipapython.dn import DN
+
+if six.PY3:
+ unicode = str
+
+
+class ServiceTracker(Tracker):
+ """
+ Tracker class for service plugin
+
+ So far does not include methods for these commands:
+ service-add-host
+ service-remove-host
+ service-allow-retrieve-keytab
+ service-disallow-retrieve-keytab
+ service-allow-create-keytab
+ service-disallow-create-keytab
+ service-disable
+ service-add-cert
+ service-remove-cert
+ """
+
+ retrieve_keys = {
+ u'dn', u'krbprincipalname', u'usercertificate', u'has_keytab',
+ u'ipakrbauthzdata', u'ipaallowedtoperform', u'subject',
+ u'managedby', u'serial_number', u'serial_number_hex', u'issuer',
+ u'valid_not_before', u'valid_not_after', u'md5_fingerprint',
+ u'sha1_fingerprint', u'krbprincipalauthind', u'managedby_host',
+ u'krbcanonicalname'}
+ retrieve_all_keys = retrieve_keys | {
+ u'ipaKrbPrincipalAlias', u'ipaUniqueID', u'krbExtraData',
+ u'krbLastPwdChange', u'krbLoginFailedCount', u'memberof',
+ u'objectClass', u'ipakrbrequirespreauth',
+ u'ipakrbokasdelegate'}
+
+ create_keys = (retrieve_keys | {u'objectclass', u'ipauniqueid'}) - {
+ u'usercertificate', u'has_keytab'}
+ update_keys = retrieve_keys - {u'dn'}
+
+ def __init__(self, name, host_fqdn, options):
+ super(ServiceTracker, self).__init__(default_version=None)
+ self._name = "{0}/{1}@{2}".format(name, host_fqdn, api.env.realm)
+ self.dn = DN(
+ ('krbprincipalname', self.name), api.env.container_service,
+ api.env.basedn)
+ self.host_fqdn = host_fqdn
+ self.options = options
+
+ @property
+ def name(self):
+ return self._name
+
+ def make_create_command(self, force=True):
+ """ Make function that creates a service """
+ return self.make_command('service_add', self.name,
+ force=force, **self.options)
+
+ def make_delete_command(self):
+ """ Make function that deletes a service """
+ return self.make_command('service_del', self.name)
+
+ def make_retrieve_command(self, all=False, raw=False):
+ """ Make function that retrieves a service """
+ return self.make_command('service_show', self.name, all=all)
+
+ def make_find_command(self, *args, **kwargs):
+ """ Make function that searches for a service"""
+ return self.make_command('service_find', *args, **kwargs)
+
+ def make_update_command(self, updates):
+ """ Make function that updates a service """
+
+ return self.make_command('service_mod', self.name, **updates)
+
+ def track_create(self, **options):
+ """ Update expected state for service creation """
+ self.attrs = {
+ u'dn': self.dn,
+ u'krbprincipalname': [u'{0}'.format(self.name)],
+ u'objectclass': objectclasses.service,
+ u'ipauniqueid': [fuzzy_uuid],
+ u'managedby_host': [self.host_fqdn],
+ u'krbcanonicalname': [u'{0}'.format(self.name)]
+ }
+
+ for key in self.options:
+ self.attrs[key] = [self.options[key]]
+
+ self.exists = True
+
+ def check_create(self, result):
+ """ Check service-add command result """
+ assert_deepequal({
+ u'value': u'{0}'.format(self.name),
+ u'summary': u'Added service "{0}"'.format(self.name),
+ u'result': self.filter_attrs(self.create_keys)
+ }, result)
+
+ def check_delete(self, result):
+ """ Check service-del command result """
+ assert_deepequal({
+ u'value': [u'{0}'.format(self.name)],
+ u'summary': u'Deleted service "{0}"'.format(self.name),
+ u'result': {u'failed': []}
+ }, result)
+
+ def check_retrieve(self, result, all=False, raw=False):
+ """ Check service-show command result """
+ if all:
+ expected = self.filter_attrs(self.retrieve_all_keys)
+ else:
+ expected = self.filter_attrs(self.retrieve_keys)
+
+ assert_deepequal({
+ u'value': u'{0}'.format(self.name),
+ u'summary': None,
+ u'result': expected,
+ }, result)
+
+ def check_find(self, result, all=False, raw=False):
+ """ Check service-find command result """
+ if all:
+ expected = self.filter_attrs(self.retrieve_all_keys)
+ else:
+ expected = self.filter_attrs(self.retrieve_keys)
+
+ assert_deepequal({
+ u'count': 1,
+ u'truncated': False,
+ u'summary': u'1 service matched',
+ u'result': [expected]
+ }, result)
+
+ def check_update(self, result, extra_keys=()):
+ """ Check service-mod command result """
+ assert_deepequal({
+ u'value': u'{0}'.format(self.name),
+ u'summary': u'Modified service "{0}"'.format(self.name),
+ u'result': self.filter_attrs(self.update_keys | set(extra_keys))
+ }, result)
--
2.7.4
From c50301079954c9dc1f98d4b2ee9af924cbb9429a Mon Sep 17 00:00:00 2001
From: Lenka Doudova <ldoud...@redhat.com>
Date: Wed, 15 Jun 2016 13:44:16 +0200
Subject: [PATCH 2/2] Tests: Authentication indicators xmlrpc tests
https://fedorahosted.org/freeipa/ticket/433
---
ipatests/test_xmlrpc/test_service_plugin.py | 45 ++++++++++++++++++++++++++++-
1 file changed, 44 insertions(+), 1 deletion(-)
diff --git a/ipatests/test_xmlrpc/test_service_plugin.py b/ipatests/test_xmlrpc/test_service_plugin.py
index 54ae55963c5ba45fb36dd6bb37416e33e9e49aa1..56e2c7a7a0570cb15dc5d27dcdba488da089350f 100644
--- a/ipatests/test_xmlrpc/test_service_plugin.py
+++ b/ipatests/test_xmlrpc/test_service_plugin.py
@@ -24,10 +24,14 @@ Test the `ipaserver/plugins/service.py` module.
from ipalib import api, errors, x509
from ipatests.test_xmlrpc.xmlrpc_test import Declarative, fuzzy_uuid, fuzzy_hash
from ipatests.test_xmlrpc.xmlrpc_test import fuzzy_digits, fuzzy_date, fuzzy_issuer
-from ipatests.test_xmlrpc.xmlrpc_test import fuzzy_hex
+from ipatests.test_xmlrpc.xmlrpc_test import fuzzy_hex, XMLRPC_test
from ipatests.test_xmlrpc import objectclasses
from ipatests.test_xmlrpc.testcert import get_testcert
from ipatests.test_xmlrpc.test_user_plugin import get_user_result, get_group_dn
+
+from ipatests.test_xmlrpc.tracker.service_plugin import ServiceTracker
+from ipatests.test_xmlrpc.tracker.host_plugin import HostTracker
+
import base64
from ipapython.dn import DN
import pytest
@@ -1262,3 +1266,42 @@ class test_service_allowed_to(Declarative):
),
),
]
+
+
+@pytest.fixture(scope='function')
+def indicators_host(request):
+ tracker = HostTracker(name=u'testhost1', fqdn=fqdn1)
+ return tracker.make_fixture(request)
+
+
+@pytest.fixture(scope='function')
+def indicators_service(request):
+ tracker = ServiceTracker(
+ name=u'SRV1', host_fqdn=fqdn1, options={
+ u'krbprincipalauthind': u'otp'})
+ return tracker.make_fixture(request)
+
+
+@pytest.mark.tier1
+class TestAuthenticationIndicators(XMLRPC_test):
+ def test_create_service_with_otp_indicator(
+ self, indicators_host, indicators_service):
+ """ Since range of authentication indicator values is not limited,
+ only 'otp' option is tested """
+ indicators_host.create()
+ indicators_service.create()
+
+ def test_adding_second_indicator(
+ self, indicators_host, indicators_service):
+ indicators_host.create()
+ indicators_service.create()
+ indicators_service.update(
+ updates={u'krbprincipalauthind': [u'otp', u'radius']})
+
+ def test_update_indicator(self, indicators_host, indicators_service):
+ indicators_host.create()
+ indicators_service.create()
+ indicators_service.update(
+ updates={u'krbprincipalauthind': u'radius'},
+ expected_updates={u'krbprincipalauthind': [u'radius']}
+ )
--
2.7.4
From 8de3d5dcba66569700f0572e35c2b622ea46228c Mon Sep 17 00:00:00 2001
From: Lenka Doudova <ldoud...@redhat.com>
Date: Wed, 15 Jun 2016 13:45:59 +0200
Subject: [PATCH] Tests: Authentication indicators integration tests
https://fedorahosted.org/freeipa/ticket/433
---
.../test_integration/test_service_permissions.py | 56 ++++++++++++++++++++++
1 file changed, 56 insertions(+)
diff --git a/ipatests/test_integration/test_service_permissions.py b/ipatests/test_integration/test_service_permissions.py
index 3d4a50d324ae71006bab3fafb966c60ab1574e13..59f46971303d7cda39dcd320a729a96abbc07b30 100644
--- a/ipatests/test_integration/test_service_permissions.py
+++ b/ipatests/test_integration/test_service_permissions.py
@@ -80,3 +80,59 @@ class TestServicePermissions(IntegrationTest):
self.master.run_command(['ipa', 'service-del', service_name])
self.master.run_command(['ipa', 'user-del', 'tuser'])
+
+
+class TestServiceAuthenticationIndicators(IntegrationTest):
+ topology = 'star'
+
+ def test_service_access(self):
+ """ Test that user is granted access when authenticated using
+ credentials that are sufficient for a service, and denied access
+ when using insufficient credentials"""
+
+ service_name = 'testservice/%s@%s' % (self.master.hostname,
+ self.master.domain.realm)
+
+ keytab_file = os.path.join(self.master.config.test_dir,
+ 'testservice_keytab')
+
+ # Prepare a service without authentication indicator
+ self.master.run_command(['ipa', 'service-add', service_name])
+
+ self.master.run_command(['ipa-getkeytab',
+ '-p', service_name,
+ '-k', keytab_file])
+
+ # Set authentication-type for admin user
+ self.master.run_command(['ipa', 'user-mod', 'admin',
+ '--user-auth-type=password',
+ '--user-auth-type=otp'])
+
+ # Authenticate
+ self.master.run_command(['kinit', '-k', service_name,
+ '-t', keytab_file])
+
+ # Verify access to service is granted
+ result = self.master.run_command(['kvno', service_name],
+ raiseonerr=False)
+ assert result.returncode == 0
+
+ # Obtain admin ticket to be able to update service
+ tasks.kinit_admin(self.master)
+
+ # Modify service to have authentication indicator
+ self.master.run_command(['ipa', 'service-mod', service_name,
+ '--auth-ind=otp'])
+
+ self.master.run_command(['ipa-getkeytab',
+ '-p', service_name,
+ '-k', keytab_file])
+
+ # Authenticate
+ self.master.run_command(['kinit', '-k', service_name,
+ '-t', keytab_file])
+
+ # Verify access to service is rejected
+ result = self.master.run_command(['kvno', service_name],
+ raiseonerr=False)
+ assert result.returncode > 0
--
2.7.4
--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
