Re: [Freeipa-devel] [PATCH 0406] admintool: Remove the option to override the log file
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
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: +# Get all forward resources for
[Freeipa-devel] [PATCH 0406] admintool: Remove the option to override the log file
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 mailing list: https://
Re: [Freeipa-devel] [PATCH 0143-0144] different errors/warnings for different LDAP limit type exceeded
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
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', , Gettext('A dict
Re: [Freeipa-devel] [TESTS][PATCH 0011] WebUI: Creating user without private group
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.
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) +client.Backend.rpcclient.connect() + +
Re: [Freeipa-devel] [SSSD] HOWTO: Troubleshooting SUDO
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
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&id=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']) > "