Re: [Freeipa-devel] [PATCH 0406] admintool: Remove the option to override the log file

[Rob Crittenden]

Tomas Babej wrote:

Hi,

This option has been rarely used, and can be replaced by proper shell
output redirection.

https://fedorahosted.org/freeipa/ticket/5385


Should the ticket be re-opened?

I'm not opposed to removing it I guess, but how can you know it is 
rarely used? Nobody has provided one in a bug report perhaps?


The advantage of it over the shell is that it always logs in debug mode, 
so if something goes horribly wrong and the user didn't specify debug 
you still get the output vs having to re-run it and hope the same thing 
happens again.


rob

--
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

[Freeipa-devel] [PATCH 0453 - 0458] host-del: fix updatedns option

[Martin Basti]

https://fedorahosted.org/freeipa/ticket/5675

Patches attached.
From b013cce6bdfb7dbe703a4781e0dde407e1153c43 Mon Sep 17 00:00:00 2001
From: Martin Basti 
Date: Wed, 2 Mar 2016 13:44:22 +0100
Subject: [PATCH 1/6] host_del: fix removal of host records

Originally only the first A/ record is removed, and one other record. This commit fixes it
and all records are removed.

https://fedorahosted.org/freeipa/ticket/5675
---
 ipalib/plugins/host.py | 31 ++-
 1 file changed, 10 insertions(+), 21 deletions(-)

diff --git a/ipalib/plugins/host.py b/ipalib/plugins/host.py
index 6ff751ca88187bb37ac64ca291234eed56e26e6f..97c9e158851158c1ce96b5e3bc566a1135534942 100644
--- a/ipalib/plugins/host.py
+++ b/ipalib/plugins/host.py
@@ -35,7 +35,7 @@ from ipalib.plugins.service import (split_principal, validate_certificate,
 set_certificate_attrs, ticket_flags_params, update_krbticketflags,
 set_kerberos_attrs, rename_ipaallowedtoperform_from_ldap,
 rename_ipaallowedtoperform_to_ldap, revoke_certs)
-from ipalib.plugins.dns import (dns_container_exists, _record_types,
+from ipalib.plugins.dns import (dns_container_exists, _record_attributes,
 add_records_for_host_validation, add_records_for_host,
 get_reverse_zone)
 from ipalib import _, ngettext
@@ -772,26 +772,15 @@ class host_del(LDAPDelete):
 # Get all forward resources for this host
 records = api.Command['dnsrecord_find'](domain, idnsname=parts[0])['result']
 for record in records:
-if 'arecord' in record:
-remove_fwd_ptr(record['arecord'][0], parts[0],
-   domain, 'arecord')
-if 'record' in record:
-remove_fwd_ptr(record['record'][0], parts[0],
-   domain, 'record')
-else:
-# Try to delete all other record types too
-_attribute_types = [str('%srecord' % t.lower())
-for t in _record_types]
-for attr in _attribute_types:
-if attr not in ['arecord', 'record'] and attr in record:
-for val in record[attr]:
-if (val.endswith(parts[0]) or
-val.endswith(fqdn + '.')):
-delkw = {unicode(attr): val}
-api.Command['dnsrecord_del'](domain,
-record['idnsname'][0],
-**delkw)
-break
+for attr in _record_attributes:
+for val in record.get(attr, []):
+if attr in ('arecord', 'record'):
+remove_fwd_ptr(val, parts[0], domain, attr)
+elif (val.endswith(parts[0]) or
+val.endswith(fqdn + '.')):
+delkw = {unicode(attr): val}
+api.Command['dnsrecord_del'](
+domain, record['idnsname'][0], **delkw)
 
 if self.api.Command.ca_is_enabled()['result']:
 try:
-- 
2.5.5

From 32f35058dc86a1913fb4f515ef90ac0ae25a29fe Mon Sep 17 00:00:00 2001
From: Martin Basti 
Date: Wed, 2 Mar 2016 15:53:27 +0100
Subject: [PATCH 2/6] host_del: replace dns-record find command with show

Due the configuration of dnsrecord_find, it works as dnsrecord-show,
thus it can be replaced.

https://fedorahosted.org/freeipa/ticket/5675
---
 ipalib/plugins/host.py | 29 +
 1 file changed, 17 insertions(+), 12 deletions(-)

diff --git a/ipalib/plugins/host.py b/ipalib/plugins/host.py
index 97c9e158851158c1ce96b5e3bc566a1135534942..ef0738041e4fb72780b67f880028bf857c3f9485 100644
--- a/ipalib/plugins/host.py
+++ b/ipalib/plugins/host.py
@@ -769,18 +769,23 @@ class host_del(LDAPDelete):
 domain = result['idnsname'][0]
 except errors.NotFound:
 self.obj.handle_not_found(*keys)
-# Get all forward resources for this host
-records = api.Command['dnsrecord_find'](domain, idnsname=parts[0])['result']
-for record in records:
-for attr in _record_attributes:
-for val in record.get(attr, []):
-if attr in ('arecord', 'record'):
-remove_fwd_ptr(val, parts[0], domain, attr)
-elif (val.endswith(parts[0]) or
-val.endswith(fqdn + '.')):
-delkw = {unicode(attr): val}
-api.Command['dnsrecord_del'](
-domain, record['idnsname'][0], **delkw)
+else:
+  

[Freeipa-devel] [PATCH 0406] admintool: Remove the option to override the log file

[Tomas Babej]

Hi,

This option has been rarely used, and can be replaced by proper shell
output redirection.

https://fedorahosted.org/freeipa/ticket/5385

Tomas
From ee3b3d295e696488bef9abd16eb3108255afd0b0 Mon Sep 17 00:00:00 2001
From: Tomas Babej 
Date: Tue, 10 Nov 2015 14:20:45 +0100
Subject: [PATCH] admintool: Remove the option to override the log file

This has been rarely used, and can be replaced by proper shell output
redirection.

https://fedorahosted.org/freeipa/ticket/5385
---
 install/tools/man/ipa-kra-install.1|  3 ---
 install/tools/man/ipa-server-upgrade.1 |  3 ---
 ipapython/admintool.py | 17 ++---
 ipapython/install/cli.py   |  7 +--
 4 files changed, 7 insertions(+), 23 deletions(-)

diff --git a/install/tools/man/ipa-kra-install.1 b/install/tools/man/ipa-kra-install.1
index e3133eee188af0b613fca76b51d2f5b4f8d1ba7d..5bda4fe4b80947588ad7afde2c9f8fd81f320614 100644
--- a/install/tools/man/ipa-kra-install.1
+++ b/install/tools/man/ipa-kra-install.1
@@ -47,9 +47,6 @@ Enable debug output when more verbose output is needed
 .TP
 \fB\-q\fR, \fB\-\-quiet\fR
 Output only errors
-.TP
-\fB\-v\fR, \fB\-\-log-file\fR=\fFILE\fR
-Log to the given file
 .SH "EXIT STATUS"
 0 if the command was successful
 
diff --git a/install/tools/man/ipa-server-upgrade.1 b/install/tools/man/ipa-server-upgrade.1
index cbbdc590171bff0a88b67bcf1de961fd783ac35c..7f06e09fc4d181fa9a89772e7285d4a6567bc361 100644
--- a/install/tools/man/ipa-server-upgrade.1
+++ b/install/tools/man/ipa-server-upgrade.1
@@ -36,9 +36,6 @@ Print debugging information
 \fB\-q\fR, \fB\-\-quiet\fR
 Output only errors
 .TP
-\fB-\-log-file=FILE\fR
-Log to given file
-.TP
 
 .SH "EXIT STATUS"
 0 if the command was successful
diff --git a/ipapython/admintool.py b/ipapython/admintool.py
index e40f300b1318b7325eb2b39ec83970753d39c406..0c8a5d54676fac0704202cf183990115978cebed 100644
--- a/ipapython/admintool.py
+++ b/ipapython/admintool.py
@@ -113,8 +113,6 @@ class AdminTool(object):
 action="store_true", help="alias for --verbose (deprecated)")
 group.add_option("-q", "--quiet", dest="quiet", default=False,
 action="store_true", help="output only errors")
-group.add_option("--log-file", dest="log_file", default=None,
-metavar="FILE", help="log to the given file")
 parser.add_option_group(group)
 
 @classmethod
@@ -208,9 +206,8 @@ class AdminTool(object):
 :param _to_file: Setting this to false will disable logging to file.
 For internal use.
 
-If the --log-file option was given or if a filename is in
-self.log_file_name, the tool will log to that file. In this case,
-all messages are logged.
+If self.log_file_name is set, the tool will log to that file.
+In this case, all messages are logged.
 
 What is logged to the console depends on command-line options:
 the default is INFO; --quiet sets ERROR; --verbose sets DEBUG.
@@ -232,12 +229,8 @@ class AdminTool(object):
 self._setup_logging(log_file_mode=log_file_mode)
 
 def _setup_logging(self, log_file_mode='w', no_file=False):
-if no_file:
-log_file_name = None
-elif self.options.log_file:
-log_file_name = self.options.log_file
-else:
-log_file_name = self.log_file_name
+log_file_name = None if no_file else self.log_file_name
+
 if self.options.verbose:
 console_format = '%(name)s: %(levelname)s: %(message)s'
 verbose = True
@@ -249,10 +242,12 @@ class AdminTool(object):
 verbose = False
 else:
 verbose = True
+
 ipa_log_manager.standard_logging_setup(
 log_file_name, console_format=console_format,
 filemode=log_file_mode, debug=debug, verbose=verbose)
 self.log = ipa_log_manager.log_mgr.get_logger(self)
+
 if log_file_name:
 self.log.debug('Logging to %s' % log_file_name)
 elif not no_file:
diff --git a/ipapython/install/cli.py b/ipapython/install/cli.py
index aed0bc9fe12e0c56987a4e2f78d73f476dcfc2c8..37ede148b0cbde2f4c4ef46bddf39d13cbda6ed9 100644
--- a/ipapython/install/cli.py
+++ b/ipapython/install/cli.py
@@ -265,12 +265,7 @@ class ConfigureTool(admintool.AdminTool):
 index += 1
 
 def _setup_logging(self, log_file_mode='w', no_file=False):
-if no_file:
-log_file_name = None
-elif self.options.log_file:
-log_file_name = self.options.log_file
-else:
-log_file_name = self.log_file_name
+log_file_name = None if no_file else self.log_file_name
 ipa_log_manager.standard_logging_setup(log_file_name,
debug=self.options.verbose)
 self.log = ipa_log_manager.log_mgr.get_logger(self)
-- 
2.5.0

-- 
Manage your subscription for the Freeipa-devel 

Re: [Freeipa-devel] [PATCH 0143-0144] different errors/warnings for different LDAP limit type exceeded

[Martin Babinsky]

On 03/24/2016 04:14 PM, Martin Babinsky wrote:

On 03/22/2016 04:28 PM, Rob Crittenden wrote:

Martin Babinsky wrote:

On 03/21/2016 12:25 PM, Jan Cholasta wrote:

On 21.3.2016 10:17, Petr Spacek wrote:

On 18.3.2016 13:49, Rob Crittenden wrote:

Martin Babinsky wrote:

These patches implement behavior agreed upon during discussion of
https://fedorahosted.org/freeipa/ticket/5677

However I'm not sure if we want to push them into 4-3 branch (the
ticket
is triaged into 4.3.2 milestone) since they modify the framework
behavior quite a bit.

If there is no need to have it there (CC'ing Milan since he is the
reporter), I would retriage it into 4.4 milestone.



+ desc="while getting entries (search base: '{}',"
+ "filter: {})".format(base_dn, filter))

This is going to expose parts of the DIT in an error message to
users. We have
tried in the past to hide the implementation. I'd propose logging the
error
and making the exception less verbose.


I agree with Rob here, we shouldn't expose internal stuff in error
messages for users.

In this particular case, even if we included internal stuff in the
error
message, it should be the error message returned by the server rather
than this ad-hoc message.



IMHO it actually helps to print the DN. At very least the user can see
if the
error is happening always with the same DN or if it keeps changing.

In other words, for user it is not that important to understand
meaning of the
DN but it might be important to see if it is the same or not.


I can't imagine a situation where it would actually be useful for the
user (as opposed to the admin, who has access to logs) to know the base
DN of some arbitrary LDAP search operation. Could you give an example?


Right, attaching updated patches.


I may have suggested debug logging the detailed error. I was wrong. This
should log at the error level so it always appears in the logs. This may
be a spurious error and having the user turn on debug logging to capture
the reasons would be asking a lot.

rob

That's right, the user would then have to enable debug mode and re-run
the command. I have changed the log level to error as you suggested.




Bump for review.

--
Martin^3 Babinsky

--
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

[Freeipa-devel] [PATCH 0405] idviews: Add user certificate attribute to user ID overrides

[Tomas Babej]

Hi,

this extends the user ID overrides with capability to store the user
certificate.

https://fedorahosted.org/freeipa/ticket/4955

Tomas
From 4ab4ac5871f14d164544298fc5763321b8ef7558 Mon Sep 17 00:00:00 2001
From: Tomas Babej 
Date: Thu, 3 Mar 2016 15:14:10 +0100
Subject: [PATCH] idviews: Add user certificate attribute to user ID overrides

---
 ACI.txt  |  2 +-
 API.txt  |  6 --
 install/share/71idviews.ldif |  2 +-
 ipalib/plugins/idviews.py| 34 +++---
 4 files changed, 37 insertions(+), 7 deletions(-)

diff --git a/ACI.txt b/ACI.txt
index 24cb332ce6e10c82a5bfab76d084fb6c0277800d..ae00cf7a1b8e2ea0e33798993bb24dc5f06127e3 100644
--- a/ACI.txt
+++ b/ACI.txt
@@ -149,7 +149,7 @@ aci: (targetfilter = "(objectclass=ipahostgroup)")(version 3.0;acl "permission:S
 dn: cn=views,cn=accounts,dc=ipa,dc=example
 aci: (targetattr = "cn || createtimestamp || description || entryusn || gidnumber || ipaanchoruuid || modifytimestamp || objectclass")(targetfilter = "(objectclass=ipaGroupOverride)")(version 3.0;acl "permission:System: Read Group ID Overrides";allow (compare,read,search) userdn = "ldap:///all;;)
 dn: cn=views,cn=accounts,dc=ipa,dc=example
-aci: (targetattr = "createtimestamp || description || entryusn || gecos || gidnumber || homedirectory || ipaanchoruuid || ipaoriginaluid || ipasshpubkey || loginshell || modifytimestamp || objectclass || uid || uidnumber")(targetfilter = "(objectclass=ipaUserOverride)")(version 3.0;acl "permission:System: Read User ID Overrides";allow (compare,read,search) userdn = "ldap:///all;;)
+aci: (targetattr = "createtimestamp || description || entryusn || gecos || gidnumber || homedirectory || ipaanchoruuid || ipaoriginaluid || ipasshpubkey || loginshell || modifytimestamp || objectclass || uid || uidnumber || usercertificate")(targetfilter = "(objectclass=ipaUserOverride)")(version 3.0;acl "permission:System: Read User ID Overrides";allow (compare,read,search) userdn = "ldap:///all;;)
 dn: cn=ranges,cn=etc,dc=ipa,dc=example
 aci: (targetattr = "cn || createtimestamp || entryusn || ipabaseid || ipabaserid || ipaidrangesize || ipanttrusteddomainsid || iparangetype || ipasecondarybaserid || modifytimestamp || objectclass")(targetfilter = "(objectclass=ipaidrange)")(version 3.0;acl "permission:System: Read ID Ranges";allow (compare,read,search) userdn = "ldap:///all;;)
 dn: cn=views,cn=accounts,dc=ipa,dc=example
diff --git a/API.txt b/API.txt
index 5b75413f930d0e9caaffc68023bed8106d786653..34053640ccc0928ae76d9ae658a55e171478ceab 100644
--- a/API.txt
+++ b/API.txt
@@ -2429,7 +2429,7 @@ output: Entry('result', , Gettext('A dictionary representing an LDA
 output: Output('summary', (, ), None)
 output: PrimaryKey('value', None, None)
 command: idoverrideuser_add
-args: 2,15,3
+args: 2,16,3
 arg: Str('idviewcn', cli_name='idview', multivalue=False, primary_key=True, query=True, required=True)
 arg: Str('ipaanchoruuid', attribute=True, cli_name='anchor', multivalue=False, primary_key=True, required=True)
 option: Str('addattr*', cli_name='addattr', exclude='webui')
@@ -2446,6 +2446,7 @@ option: Flag('raw', autofill=True, cli_name='raw', default=False, exclude='webui
 option: Str('setattr*', cli_name='setattr', exclude='webui')
 option: Str('uid', attribute=True, cli_name='login', maxlength=255, multivalue=False, pattern='^[a-zA-Z0-9_.][a-zA-Z0-9_.-]{0,252}[a-zA-Z0-9_.$-]?$', required=False)
 option: Int('uidnumber', attribute=True, cli_name='uid', minvalue=1, multivalue=False, required=False)
+option: Bytes('usercertificate', attribute=True, cli_name='certificate', multivalue=True, required=False)
 option: Str('version?', exclude='webui')
 output: Entry('result', , Gettext('A dictionary representing an LDAP entry', domain='ipa', localedir=None))
 output: Output('summary', (, ), None)
@@ -2485,7 +2486,7 @@ output: ListOfEntries('result', (, ), Gettext('A list
 output: Output('summary', (, ), None)
 output: Output('truncated', , None)
 command: idoverrideuser_mod
-args: 2,18,3
+args: 2,19,3
 arg: Str('idviewcn', cli_name='idview', multivalue=False, primary_key=True, query=True, required=True)
 arg: Str('ipaanchoruuid', attribute=True, cli_name='anchor', multivalue=False, primary_key=True, query=True, required=True)
 option: Str('addattr*', cli_name='addattr', exclude='webui')
@@ -2505,6 +2506,7 @@ option: Flag('rights', autofill=True, default=False)
 option: Str('setattr*', cli_name='setattr', exclude='webui')
 option: Str('uid', attribute=True, autofill=False, cli_name='login', maxlength=255, multivalue=False, pattern='^[a-zA-Z0-9_.][a-zA-Z0-9_.-]{0,252}[a-zA-Z0-9_.$-]?$', required=False)
 option: Int('uidnumber', attribute=True, autofill=False, cli_name='uid', minvalue=1, multivalue=False, required=False)
+option: Bytes('usercertificate', attribute=True, autofill=False, cli_name='certificate', multivalue=True, required=False)
 option: Str('version?', exclude='webui')
 output: Entry('result', , 

Re: [Freeipa-devel] [TESTS][PATCH 0011] WebUI: Creating user without private group

[Pavel Vomacka]

On 03/31/2016 04:16 PM, Lenka Doudova wrote:



On 03/31/2016 12:42 PM, Pavel Vomacka wrote:



On 03/18/2016 11:24 AM, Lenka Doudova wrote:



On 03/10/2016 06:58 PM, Petr Vobornik wrote:

On 03/08/2016 01:17 PM, Lenka Doudova wrote:



On 03/08/2016 12:59 PM, Petr Vobornik wrote:

On 03/07/2016 04:29 PM, Pavel Vomacka wrote:



On 02/25/2016 03:08 PM, Lenka Doudova wrote:

Hi,

here's a patch for webUI tests that provides test for creating 
user

without private group.
Related to ticket https://fedorahosted.org/freeipa/ticket/4986

Since the option to specify GID when creating a user is not 
available
https://fedorahosted.org/freeipa/ticket/5505 the test creates a 
new
posix group, makes it a default user group instead of 
'ipausers' and

then attemps to create the user without private group. Returning
default user group value to 'ipausers' is provided even for 
cases when

the test fails so it would not block other tests from performing
properly.

Lenka



Hi,

ACK, works well.

Pavel^3 Vomacka



NACK, don't use naked except, specify at least 'Exception'
  +except:



Thanks, patch fixed according to Petr's review attached.

Lenka


Ticket 5505 was pushed. So the workaround can be removed. Do you 
prefer to do it in this patch?


Also, maybe it would be good to test both cases and check if the 
error is actually the right one.


Hi,

attaching patch fixed according to recently pushed changes.

Lenka

Hi,

NACK,

1) The data definition for user3 (user.DATA3) is not used anywhere. 
And the definition is actually the same as definition of user4. So, I 
think that it could be removed.


2) This is just a detail, but I would rather use 'combobox_input' or 
'combobox_textbox' as parameter name because the parameter actually 
doesn't represent the value of combobox.


Otherwise it works as expected.

--
Pavel^3 Vomacka


Hi,

thanks for comments, updated patch attached.

Lenka



Thank you, ACK.

--
Pavel^3 Vomacka

-- 
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

[Freeipa-devel] [patch 0035] ipatests: Add test case for requesting a certificate with full principal.

[Milan Kubík]

Patches attached.

https://fedorahosted.org/freeipa/ticket/5733

--
Milan Kubik

From 985814ef076a828ac59aeafd0598d87983edc809 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Milan=20Kub=C3=ADk?= 
Date: Fri, 1 Apr 2016 11:11:54 +0200
Subject: [PATCH] ipatests: Add test case for requesting a certificate with
 full principal.

Also fixes an issue in change_principal context manager that caused
a resource leak on test case failure.

https://fedorahosted.org/freeipa/ticket/5733
---
 .../test_xmlrpc/test_caacl_profile_enforcement.py |  8 
 ipatests/util.py  | 19 ++-
 2 files changed, 18 insertions(+), 9 deletions(-)

diff --git a/ipatests/test_xmlrpc/test_caacl_profile_enforcement.py b/ipatests/test_xmlrpc/test_caacl_profile_enforcement.py
index dca4151d614a4c2e2f5a09455426d117da4c1c80..a0b8d614cf6dd42b18eb03100a318e4a3fbfb4e0 100644
--- a/ipatests/test_xmlrpc/test_caacl_profile_enforcement.py
+++ b/ipatests/test_xmlrpc/test_caacl_profile_enforcement.py
@@ -130,6 +130,14 @@ class TestCertSignMIME(XMLRPC_test):
 api.Command.cert_request(csr, principal=smime_user,
  profile_id=smime_profile.name)
 
+@pytest.mark.xfail(strict=True, reason='freeipa ticket 5733')
+def test_sign_smime_csr_full_principal(self, smime_profile, smime_user):
+csr = generate_user_csr(smime_user)
+smime_user_principal = '@'.join((smime_user, api.env.realm))
+with change_principal(smime_user, SMIME_USER_PW):
+api.Command.cert_request(csr, principal=smime_user_principal,
+ profile_id=smime_profile.name)
+
 
 @pytest.mark.tier1
 class TestSignWithDisabledACL(XMLRPC_test):
diff --git a/ipatests/util.py b/ipatests/util.py
index 6aefe74d34fd7b1bd063c4b17c98af4840d6f042..118c47a12e0d97907cb559d716989a9ca6c5f304 100644
--- a/ipatests/util.py
+++ b/ipatests/util.py
@@ -696,17 +696,18 @@ def change_principal(user, password, client=None, path=None):
 
 client.Backend.rpcclient.disconnect()
 
-with private_ccache(ccache_name):
-kinit_password(user, password, ccache_name)
+try:
+with private_ccache(ccache_name):
+kinit_password(user, password, ccache_name)
+client.Backend.rpcclient.connect()
+
+try:
+yield
+finally:
+client.Backend.rpcclient.disconnect()
+finally:
 client.Backend.rpcclient.connect()
 
-try:
-yield
-finally:
-client.Backend.rpcclient.disconnect()
-
-client.Backend.rpcclient.connect()
-
 def get_group_dn(cn):
 return DN(('cn', cn), api.env.container_group, api.env.basedn)
 
-- 
2.8.0

From f3cb98f26551d342968281fb01d288e10cda85de Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Milan=20Kub=C3=ADk?= 
Date: Fri, 1 Apr 2016 11:11:54 +0200
Subject: [PATCH] ipatests: Add test case for requesting a certificate with
 full principal.

Also fixes an issue in change_principal context manager that caused
a resource leak on test case failure.

https://fedorahosted.org/freeipa/ticket/5733
---
 .../test_xmlrpc/test_caacl_profile_enforcement.py |  8 
 ipatests/util.py  | 19 ++-
 2 files changed, 18 insertions(+), 9 deletions(-)

diff --git a/ipatests/test_xmlrpc/test_caacl_profile_enforcement.py b/ipatests/test_xmlrpc/test_caacl_profile_enforcement.py
index 78262ae8c17716633ac33fcc8114f6b549066a42..98165c4919e719e72fd7a4aec977f12dacd79249 100644
--- a/ipatests/test_xmlrpc/test_caacl_profile_enforcement.py
+++ b/ipatests/test_xmlrpc/test_caacl_profile_enforcement.py
@@ -125,6 +125,14 @@ class TestCertSignMIME(XMLRPC_test):
 api.Command.cert_request(csr, principal=smime_user,
  profile_id=smime_profile.name)
 
+@pytest.mark.xfail(strict=True, reason='freeipa ticket 5733')
+def test_sign_smime_csr_full_principal(self, smime_profile, smime_user):
+csr = generate_user_csr(smime_user)
+smime_user_principal = '@'.join((smime_user, api.env.realm))
+with change_principal(smime_user, SMIME_USER_PW):
+api.Command.cert_request(csr, principal=smime_user_principal,
+ profile_id=smime_profile.name)
+
 
 @pytest.mark.tier1
 class TestSignWithDisabledACL(XMLRPC_test):
diff --git a/ipatests/util.py b/ipatests/util.py
index 4d99ff6e0a505cd3f75053f97caca9edbc802bcf..a3c4889c6fd0026bf5caa655170f785f571e09f5 100644
--- a/ipatests/util.py
+++ b/ipatests/util.py
@@ -687,13 +687,14 @@ def change_principal(user, password, client=None, path=None):
 
 client.Backend.rpcclient.disconnect()
 
-with private_ccache(ccache_name):
-kinit_password(user, password, ccache_name)
+try:
+with private_ccache(ccache_name):
+kinit_password(user, password, ccache_name)
+

Re: [Freeipa-devel] [SSSD] HOWTO: Troubleshooting SUDO

[Pavel Březina]

On 10/09/2015 01:35 PM, Pavel Březina wrote:

Hi,
I just submitted a sudo troubleshooting guide [1]. If you find anything
missing, please, let me know.

[1] https://fedorahosted.org/sssd/wiki/HOWTO_Troubleshoot_SUDO


Hi,
I added examples of sudo debug logs into the troubleshooting guide. You 
can find it useful when you investigating why sudo denies access when 
you expect that access is allowed.


https://fedorahosted.org/sssd/wiki/HOWTO_Troubleshoot_SUDO

--
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

Re: [Freeipa-devel] [TEST][Patch-0030]Next part of replica promotion tests

[Oleg Fayans]

Hi Martin,

Thanks for the review! The new version is attached

On 03/24/2016 06:08 PM, Martin Babinsky wrote:
> On 03/21/2016 01:51 PM, Oleg Fayans wrote:
>>
>>
>>
> Hi Oleg,
> 
> I have a few comments:
> 
> 1.)
> please make the commit message more clear, briefly describe what kind of
> test cases were added to the suite and maybe add a link to the test plan.

Done

> 
> 2.)
> I see negative test scenarios for attempting to issue
> 'ipa-csreplica-manage connect' and 'disconnect' under domain level 1.
> However, for full coverage there should be also a negative test case for
> 'ipa-csreplica-manage del' which should also issue error in domain level
> 1, see
> https://git.fedorahosted.org/cgit/freeipa.git/commit/install/tools/ipa-csreplica-manage?h=ipa-4-3=6119dbb9a915283434f718b38a70017e3ad00840
> 
> 
> Could you please add this case to the patch and also to the Test plan so
> that we have full coverage of this?

Done

> 
> 3.)
> test_one_command_installation exploded during client enrollment part on
> "Joining realm failed: incorrect password". This is probably caused by
> missing '-P', 'admin' option here:
> """
> +self.replicas[0].run_command(['ipa-replica-install', '-p',
> + self.master.config.admin_password,
> + '-n', self.master.domain.name,
> + '-r', self.master.domain.realm])
> +
> """

Fixed. Turned out, it's enough to just provide '-w'

> 
> 4.)
> I am not very happy about the organization of
> 'TestUnprivilegedUserPermissions' class.
> 
> For starters, I would add this whole block:
> """
> +password = self.master.config.dirman_password
> +new_password = '$ome0therPaaS'
> +replica = self.replicas[0]
> +adduser_stdin_text = "%s\n%s\n" %
> (self.master.config.admin_password,
> + self.master.config.admin_password)
> +user_kinit_stdin_text = "%s\n%s\n%s\n" % (password, new_password,
> +  new_password)
> +tasks.kinit_admin(self.master)
> +self.master.run_command(['ipa', 'user-add', 'testuser',
> '--password',
> + '--first', 'John', '--last', 'Donn'],
> +stdin_text=adduser_stdin_text)
> +# Now we need to change the password for the user
> +self.master.run_command(['kinit', 'testuser'],
> +stdin_text=user_kinit_stdin_text)
> +# And again kinit admin
> +tasks.kinit_admin(self.master)
> """
> 
> into 'install()' method, since it indeed sets-up the test harness. You
> can add the user name and password to class members so that you can then
> use them from the test cases. Which brings me to the second point: I
> know that the test plan mentions this as a single test case, but I would
> like this:
> 
> """
> +result1 = replica.run_command(['ipa-client-install', '-p',
> 'testuser',
> +   '-w', new_password,
> +   '--domain', replica.domain.name,
> +   '--realm', replica.domain.realm,
> '-U'],
> +  raiseonerr=False)
> +assert_error(result1, "No permission to join this host", 1)
> +tasks.install_client(self.master, replica)
> +result2 = replica.run_command(['ipa-replica-install', '-P',
> 'testuser',
> +   '-p', new_password,
> +   '-n', self.master.domain.name,
> +   '-r', self.master.domain.realm],
> +  raiseonerr=False)
> +assert_error(result2,
> + "Insufficient privileges to promote the server", 1)
> +self.master.run_command(['ipa', 'group-add-member', 'admins',
> + '--users=testuser'])
> +
> +replica.run_command(['ipa-replica-install', '-P', 'testuser',
> + '-p', new_password,
> + '-n', self.master.domain.name,
> + '-r', self.master.domain.realm])
> """
> 
> to be split into three separate test methods for the sake of clarity, e.g.:
> "test_client_enrollment_by_unprivileged_user"
> "test_replica_install_by_unprovileged_user"
> "test_replica_install_after_adding_to_admin_group"

I like that! Implemented.

> 
> 5.)
> """
> +result = self.replicas[0].run_command(['ipa-server-install',
> +   '--uninstall', '-U'],
> +  raiseonerr=False)
> +assert("Uninstallation leads to disconnected topology"
> +   in result.stderr_text)
> +self.replicas[0].run_command(['ipa-server-install', '--uninstall',
> +  '-U',
> '--ignore-topology-disconnect'])
> """