Re: [Freeipa-devel] [PATCH 0035] Use default.conf as flag of IPA client being installed
On 02/21/2013 02:58 PM, Tomas Babej wrote: On 02/21/2013 01:50 PM, Martin Kosek wrote: On 02/21/2013 01:29 PM, Tomas Babej wrote: On 02/21/2013 12:47 PM, Martin Kosek wrote: On 02/20/2013 10:31 AM, Tomas Babej wrote: Hi, When installing / uninstalling IPA client, the checks that determine whether IPA client is installed now take the existence of /etc/ipa/default.conf into consideration. The client will not uninstall unless either something is backed up or /etc/ipa/default.conf file does exist. The client will not install if something is backed up or default.conf file does exist (unless it's installation on master). https://fedorahosted.org/freeipa/ticket/3331 Tomas Can we create a function testing if ipa client is installed to avoid duplication of the decision logic? Something like is_ipa_configured present in ipaserver/install/installutils.py. Keep in mind that we cannot use ipaserver package as it may not be present on client. Martin Moved to is_ipa_client_installed function to ipautils.py Updated patch attached. Tomas You just created a nice import loop: ... I made the function part of ipa-client-install script then. We probably will not need to check whether client is installed anywhere else. Tomas Thanks, this fixes the issue. ACK. Pushed to master, ipa-3-1. Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 0006 Remove check for alphabetic only characters from domain name validation
On 20.2.2013 11:03, Ana Krivokapic wrote:
On 02/18/2013 01:08 PM, Martin Kosek wrote:
On 02/18/2013 12:47 PM, Sumit Bose wrote:
On Mon, Feb 18, 2013 at 12:27:35PM +0100, Petr Spacek wrote:
On 15.2.2013 15:22, Ana Krivokapic wrote:
Hello,
The .isalpha() check in validate_domain_name() was too strict,
causing some commands like ipa dnsrecord-add to fail.
https://fedorahosted.org/freeipa/ticket/3385
I would add --force option rather than removing whole check, if it's possible.
Would it be possible to mention RFC in the error message? Something
like _('top level domain label must be alphabetic (RFC 1123 section
2.1)')
?
IMHO it is handy, because it educates users.
The problem is that this check is always done on the last component of
the domain_name even if it is just a sub-domain of the FreeIPA domain,
where e.g. numbers are valid characters.
At the beginning of validate_domain_name() a trailing '.' is stripped
away. iirc the trailing '.' is an indication for a complete, fully
qualified name. Would it work if the presence of the trailing '.' is
saved and the check is only done if there was a '.'?
bye,
Sumit
Sure. Though I am now not 100% sure that some IPA functions do not use this
validator with a fqdn hostname without trailing dot. If not, I am for fixing
this function as Sumit and Petr suggested.
Martin
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
After some thought, I decided to change the approach.
As pointed out by Sumit, the problem was that the validate_domain_name()
function did not distinguish between fqdn and non-fqdn domains
(subdomains of the IPA domain). The trailing dot is not a clear
indication either, because some IPA functions use this validator with an
fqdn without the trailing dot.
To fix this, I introduced an additional parameter to this function - a
flag which indicates whether the domain name is an fqdn or not. The is
.isalpha() check is then performed only in the case of an fqdn.
I also improved the error message to mention the relevant RFC, as
suggested by Petr.
Please don't forget to add --force switch. It could be handy.
--
Petr^2 Spacek
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCH] 0186 Change DNA magic value to -1 to make UID 999 usable
https://fedorahosted.org/freeipa/ticket/2886 This changes the DNA magic value to -1, and the corresponding IPA's parameters (gidnumber, uidnumber) to be optional (instead of autofill). Since the old clients still say 999 when they mean pick one I don't care, we need to detect them and change 999 to -1. For that there's a new capability, optional_uid_params. Behavior summary: With --uid 999: old client, old server: sends 999, creates random UID old client, new server: sends 999, creates random UID new client, old server: incompatible new client, new server: sends 999, creates UID 999 Without --uid: old client, old server: sends 999, creates random UID old client, new server: sends 999, creates random UID new client, old server: incompatible new client, new server: doesn't send UID, creates random UID Upgrade should work fine. I didn't test winsync as I don't have a Windows machine. -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 0186 Change DNA magic value to -1 to make UID 999 usable
On 02/22/2013 11:16 AM, Petr Viktorin wrote:
https://fedorahosted.org/freeipa/ticket/2886
This changes the DNA magic value to -1, and the corresponding IPA's
parameters (gidnumber, uidnumber) to be optional (instead of autofill).
Since the old clients still say 999 when they mean pick one I don't
care, we need to detect them and change 999 to -1. For that there's a
new capability, optional_uid_params.
Behavior summary:
With --uid 999:
old client, old server: sends 999, creates random UID
old client, new server: sends 999, creates random UID
new client, old server: incompatible
new client, new server: sends 999, creates UID 999
Without --uid:
old client, old server: sends 999, creates random UID
old client, new server: sends 999, creates random UID
new client, old server: incompatible
new client, new server: doesn't send UID, creates random UID
Upgrade should work fine.
I didn't test winsync as I don't have a Windows machine.
The patch is here
--
Petr³
From f5f7c2936f94ba332f4e8493ab9b785d5449eddb Mon Sep 17 00:00:00 2001
From: Petr Viktorin pvikt...@redhat.com
Date: Tue, 8 Jan 2013 04:10:35 -0500
Subject: [PATCH] Change DNA magic value to -1 to make UID 999 usable
Change user-add's uid gid parameters from autofill to optional.
Change the DNA magic value to -1.
For old clients, which will still send 999 when they want DNA
assignment, translate the 999 to -1. This is done via a new
capability, optional_uid_params.
Tests included
https://fedorahosted.org/freeipa/ticket/2886
---
API.txt| 13 ++--
VERSION|2 +-
daemons/ipa-sam/ipa_sam.c |2 +-
.../ipa-winsync/ipa-winsync-conf.ldif |4 +-
install/share/default-smb-group.ldif |2 +-
install/share/dna.ldif |2 +-
install/updates/20-dna.update | 10 +++
ipalib/capabilities.py |6 ++
ipalib/plugins/baseldap.py |2 +
ipalib/plugins/group.py|5 +-
ipalib/plugins/user.py | 34 +---
tests/test_install/1_add.update|4 +-
tests/test_xmlrpc/test_user_plugin.py | 86
13 files changed, 144 insertions(+), 28 deletions(-)
diff --git a/API.txt b/API.txt
index a43fce596a6e01ba1fc709242ede0303994a255d..1664f61493093ca2438536455e98b792572a953a 100644
--- a/API.txt
+++ b/API.txt
@@ -3416,7 +3416,7 @@ option: Str('cn', attribute=True, autofill=True, cli_name='cn', multivalue=False
option: Str('displayname', attribute=True, autofill=True, cli_name='displayname', multivalue=False, required=False)
option: Str('facsimiletelephonenumber', attribute=True, cli_name='fax', multivalue=True, required=False)
option: Str('gecos', attribute=True, autofill=True, cli_name='gecos', multivalue=False, required=False)
-option: Int('gidnumber', attribute=True, autofill=True, cli_name='gidnumber', default=999, minvalue=1, multivalue=False, required=True)
+option: Int('gidnumber', attribute=True, cli_name='gidnumber', minvalue=1, multivalue=False, required=False)
option: Str('givenname', attribute=True, cli_name='first', multivalue=False, required=True)
option: Str('homedirectory', attribute=True, cli_name='homedir', multivalue=False, required=False)
option: Str('initials', attribute=True, autofill=True, cli_name='initials', multivalue=False, required=False)
@@ -3440,7 +3440,7 @@ option: Str('st', attribute=True, cli_name='state', multivalue=False, required=F
option: Str('street', attribute=True, cli_name='street', multivalue=False, required=False)
option: Str('telephonenumber', attribute=True, cli_name='phone', multivalue=True, required=False)
option: Str('title', attribute=True, cli_name='title', multivalue=False, required=False)
-option: Int('uidnumber', attribute=True, autofill=True, cli_name='uid', default=999, minvalue=1, multivalue=False, required=True)
+option: Int('uidnumber', attribute=True, cli_name='uid', minvalue=1, multivalue=False, required=False)
option: Password('userpassword', attribute=True, cli_name='password', exclude='webui', multivalue=False, required=False)
option: Str('version?', exclude='webui')
output: Entry('result', type 'dict', Gettext('A dictionary representing an LDAP entry', domain='ipa', localedir=None))
@@ -3477,7 +3477,7 @@ option: Str('cn', attribute=True, autofill=False, cli_name='cn', multivalue=Fals
option: Str('displayname', attribute=True, autofill=False, cli_name='displayname', multivalue=False, query=True, required=False)
option: Str('facsimiletelephonenumber', attribute=True, autofill=False, cli_name='fax', multivalue=True, query=True, required=False)
option: Str('gecos', attribute=True, autofill=False, cli_name='gecos', multivalue=False, query=True, required=False)
-option: Int('gidnumber', attribute=True,
Re: [Freeipa-devel] [PATCH] 1087 Some missing v3 schema on upgrades
On 02/18/2013 10:00 PM, Rob Crittenden wrote: An objectclass and attribute are not being added on upgrades. Missing these causes the UI to not work. I also noticed a typo in the ordering of a number of the trust attributes so fix those as well. rob ACK, works for me. Pushed to master, ipa-3-1. Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCH] 0187 CLI: Do interactive prompting after a context is created
Hello, This fixes a regression introduced by one of my help patches (abe26d5). https://fedorahosted.org/freeipa/ticket/3453 -- Petr³ From 04d76fdeb4ae29665586aebbd7724b3b7d2fcbd7 Mon Sep 17 00:00:00 2001 From: Petr Viktorin pvikt...@redhat.com Date: Fri, 22 Feb 2013 07:25:03 -0500 Subject: [PATCH] cli: Do interactive prompting after a context is created Some commands require a connection for interactive prompting. Prompt after the connection is created. Option parsing is still done before connecting so that help can be printed out without a Kerberos ticket. https://fedorahosted.org/freeipa/ticket/3453 --- ipalib/cli.py |6 +++--- tests/test_cmdline/test_cli.py |3 ++- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/ipalib/cli.py b/ipalib/cli.py index f1d2f874319800d8e4e1576019e108e80836a4e9..d267170c5984eafc77de83c4e93962c72f5c513f 100644 --- a/ipalib/cli.py +++ b/ipalib/cli.py @@ -1039,9 +1039,8 @@ class cli(backend.Executioner): cmd = self.Command[name] return cmd -def argv_to_keyword_arguments(self, cmd, argv): +def process_keyword_arguments(self, cmd, kw): Get the keyword arguments for a Command -kw = self.parse(cmd, argv) if self.env.interactive: self.prompt_interactively(cmd, kw) kw = cmd.split_csv(**kw) @@ -1062,10 +1061,11 @@ class cli(backend.Executioner): if cmd is None: return name = cmd.name -kw = self.argv_to_keyword_arguments(cmd, argv[1:]) +kw = self.parse(cmd, argv[1:]) if not isinstance(cmd, frontend.Local): self.create_context() try: +kw = self.process_keyword_arguments(cmd, kw) result = self.execute(name, **kw) if callable(cmd.output_for_cli): for param in cmd.params(): diff --git a/tests/test_cmdline/test_cli.py b/tests/test_cmdline/test_cli.py index 4d730d582bf4622347e882158d72fef7b7bd6c06..58d73775c325dad2824697041a6f6605fde93d89 100644 --- a/tests/test_cmdline/test_cli.py +++ b/tests/test_cmdline/test_cli.py @@ -18,7 +18,8 @@ class TestCLIParsing(object): executioner = api.Backend.cli cmd = executioner.get_command(argv) -kw_got = executioner.argv_to_keyword_arguments(cmd, argv[1:]) +kw_got = executioner.parse(cmd, argv[1:]) +kw_got = executioner.process_keyword_arguments(cmd, kw_got) util.assert_deepequal(expected_command_name, cmd.name, 'Command name') util.assert_deepequal(kw_expected, kw_got) -- 1.7.7.6 ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES] 0183-0185 Drop CSV support
On 02/21/2013 06:08 PM, Petr Viktorin wrote: These patches remove CSV parsing from the client. Ticket: https://fedorahosted.org/freeipa/ticket/3352 Design: http://freeipa.org/page/V3/Drop_CSV The design page also talks about adding a warning for the user when they seem to use CSV, but this will need the JSON transport so it's not included in these patches. The csv flag is left on parameters so when that warning is added, we know for which params to show it. Generally, this works fine. This finally allows us to easily add records with quote or comma: # ipa dnsrecord-add example.com txt1 --txt-rec=foo, bar Record name: txt1 TXT record: foo, bar # ipa dnsrecord-add example.com txt1 --txt-rec='bar,bar' Record name: txt1 TXT record: foo, bar, bar,bar # ipa dnsrecord-show example.com txt1 --all --raw dn: idnsname=txt1,idnsname=example.com,cn=dns,dc=example,dc=com idnsname: txt1 txtrecord: foo, bar txtrecord: bar,bar objectclass: top objectclass: idnsrecord Just couple minor remarks: 1) Can we then update the csv flag description? It is now a bit misleading given its new function - csv: this multivalue attribute is given in CSV format 2) We need to also update our help, for example selfservice online help presents a CSV formatted option use: ipa selfservice-add --permissions=write --attrs=street,postalCode,l,c,st Users manage their own address Btw if we suggest the curly braces shortcut, we may also want to write that it applies for BASH shell only... Few other examples I saw: ipa group-add-member --users=test1,test2 localadmins Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 1087 Some missing v3 schema on upgrades
On 02/19/2013 08:23 PM, Simo Sorce wrote: On Tue, 2013-02-19 at 13:32 -0500, Rob Crittenden wrote: Jan Cholasta wrote: Hi, On 18.2.2013 22:00, Rob Crittenden wrote: An objectclass and attribute are not being added on upgrades. Missing these causes the UI to not work. I also noticed a typo in the ordering of a number of the trust attributes so fix those as well. rob The patch looks good, but I think errors like this will pop up from time to time, because we have to maintain the same thing in two places - the installation LDIFs and update files. Maybe we should start thinking about merging these two somehow, e.g. using the LDIFs for both installation and updates, with directives for the updater in specially formatted comments. Honza This idea came up long, long ago when we first added the updater very early in v2. The problem, as I recall, is that some schema is needed during the install so we need to ship it in ldif format, and the idea of splitting it didn't appeal to us. So perhaps what we should endeavor to do is add all new schema via updates and only update the schema files themselves if the schema is needed for a fresh install (since updates are done last). This also puts more schema into 99user.ldif which may or may not be desirable. Ron another option is to keep putting all updates only in schema files, and then have the updater validate the schema files. Validation would be: 1. Download schema from server (we already do this in the framework so it comes for free) 2. parse the schema files and check if each attribute and objectclass is present and in the correct form. 3. if any attribute is missing, we add it 4. if any attribute has been changed, we change it 5. same for object classes. This would allow us to keep everything just in schema files, and for now only updates would end up in 99.ldif I know there is also work in 389ds to improve schema validation and handling, so there is a chance in future we will have online interfaces to put data in multiple files w/o lumping everything in 99.ldif So by keeping stuff in schema files rather than arbitrary update files we are also sort of future proof. Finally keeping data in schema files instead of spreading it in updates should make it easier to keep an eye on the whole schema. The main issue I see is that this approach needs new code to analyze and compare schema files, however that shouldn't be overly hard. Simo. I think this is a great idea. Having schema updates on 2 or more separate spaces is error prone. attributeTypes or objectClasses update files may be confusing as we often have 2 and more replace: directives when we update objectClasses or attributeTypes more that one time. As for the LDIF file parsing, we could also use python-ldap's convenience classes which will make the comparing easier. I created a ticket to address this effort: https://fedorahosted.org/freeipa/ticket/3454 Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES] 0183-0185 Drop CSV support
On 02/22/2013 02:01 PM, Martin Kosek wrote: On 02/21/2013 06:08 PM, Petr Viktorin wrote: These patches remove CSV parsing from the client. Ticket: https://fedorahosted.org/freeipa/ticket/3352 Design: http://freeipa.org/page/V3/Drop_CSV The design page also talks about adding a warning for the user when they seem to use CSV, but this will need the JSON transport so it's not included in these patches. The csv flag is left on parameters so when that warning is added, we know for which params to show it. Generally, this works fine. This finally allows us to easily add records with quote or comma: # ipa dnsrecord-add example.com txt1 --txt-rec=foo, bar Record name: txt1 TXT record: foo, bar # ipa dnsrecord-add example.com txt1 --txt-rec='bar,bar' Record name: txt1 TXT record: foo, bar, bar,bar # ipa dnsrecord-show example.com txt1 --all --raw dn: idnsname=txt1,idnsname=example.com,cn=dns,dc=example,dc=com idnsname: txt1 txtrecord: foo, bar txtrecord: bar,bar objectclass: top objectclass: idnsrecord Just couple minor remarks: 1) Can we then update the csv flag description? It is now a bit misleading given its new function - csv: this multivalue attribute is given in CSV format 2) We need to also update our help, for example selfservice online help presents a CSV formatted option use: ipa selfservice-add --permissions=write --attrs=street,postalCode,l,c,st Users manage their own address Btw if we suggest the curly braces shortcut, we may also want to write that it applies for BASH shell only... Few other examples I saw: ipa group-add-member --users=test1,test2 localadmins Martin Just discovered another issue... We now crash when adding permissions/acis with csv: # ipa permission-add --attrs=member --permissions=read,write --type=group foo2 ipa: ERROR: an internal error has occurred We should report some meaningful error, otherwise users used to add permissions with csv values would be confused. Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES 0024-0025] Improvements to idrange.py
On 02/21/2013 02:22 PM, Martin Kosek wrote:
On 02/20/2013 03:19 PM, Tomas Babej wrote:
On Wed 20 Feb 2013 02:24:03 PM CET, Alexander Bokovoy wrote:
On Wed, 20 Feb 2013, Tomas Babej wrote:
On 12/21/2012 12:15 PM, Tomas Babej wrote:
Hi,
Sending updated and rebased versions of patches 0024 and 0025.
Tomas
Sending rebased version, these got quite rotten.
Thanks for updating them.
@@ -504,25 +515,37 @@ class idrange_mod(LDAPUpdate):
'not be found. Please specify the SID
directly '
'using dom-sid option.'))
-try:
-(old_dn, old_attrs) = ldap.get_entry(dn,
-['ipabaseid',
-'ipaidrangesize',
-'ipabaserid',
-'ipasecondarybaserid'])
-except errors.NotFound:
-self.obj.handle_not_found(*keys)
+if in_updated_attrs('ipanttrusteddomainsid'):
+if in_updated_attrs('ipasecondarybaserid'):
+raise errors.ValidationError(name='ID Range setup',
+error=_('Options dom_sid and secondary_rid_base
cannot '
+'be used together'))
Since we agreed to refer to options by their CLI name (--dom-sid and
--secondary-rid-base) in the other patch, it makes sense to use it
here too.
-if is_set('ipanttrusteddomainsid'):
-# Validate SID as the one of trusted domains
-
self.obj.validate_trusted_domain_sid(entry_attrs['ipanttrusteddomainsid'])
+if not in_updated_attrs('ipabaserid'):
+raise errors.ValidationError(name='ID Range setup',
+error=_('Options dom_sid and rid_base must '
+'be used together'))
Same here.
+# secondary base rid must be set if and only if base rid
is set
+if in_updated_attrs('ipasecondarybaserid') !=\
+in_updated_attrs('ipabaserid'):
+raise errors.ValidationError(name='ID Range setup',
+error=_('Options secondary_rid_base and rid_base
must '
+'be used together'))
Same here.
+dict(
+desc='Try to modify ID range %r so it has only primary
rid range set' % (testrange8),
+command=('idrange_mod', [testrange8],
+ dict(ipabaserid=testrange8_base_rid)),
+expected=errors.ValidationError(
+name='ID Range setup', error='Options
secondary_rid_base and rid_base must be used together'),
+),
And synchronize error message here too.
Thanks!
Sending the updated patch 0024.
Tomas
In patch 0024 your intention is OK, but the checking functions are not:
is_set = lambda x: (x in entry_attrs) and (x is not None)
+in_updated_attrs = lambda x: any((x in attrs and x is not None)
+ for attrs in (entry_attrs, old_attrs))
They return True even when the attribute is None because they check if *x* is
None and not if *attrs[x]* is None. Example:
# ipa idrange-add --base-id=120 --range-size=20 --rid-base=1000
--secondary-rid-base=100 local_range
Added ID range local_range
Range name: local_range
First Posix ID of the range: 120
Number of IDs in the range: 20
First RID of the corresponding RID range: 1000
First RID of the secondary RID range: 100
Range type: local domain range
This command should be NOOP, but is not:
# ipa idrange-mod local_range --dom-sid=
ipa: ERROR: invalid 'ID Range setup': Options dom-sid and secondary-rid-base
cannot be used together
Martin
Thanks for the catch, all checking functions fixed.
Sending the complete patchset, up to date.
Tomas
From 61fcd3db8a14a17ed5854dfb4a9f11e2bb06784e Mon Sep 17 00:00:00 2001
From: Tomas Babej tba...@redhat.com
Date: Wed, 20 Feb 2013 10:50:36 +0100
Subject: [PATCH] Add trusted domain range objectclass when using idrange-mod
When modifing the idrange, one was able to add ipa NT trusted
AD domain sid without objectclass ipatrustedaddomainrange being
added. This patch fixes the issue.
---
ipalib/plugins/idrange.py | 5 +
1 file changed, 5 insertions(+)
diff --git a/ipalib/plugins/idrange.py b/ipalib/plugins/idrange.py
index 97b9d2489b51cf1fc62f49e0d3faf9674851d68a..087a1b128256063194e39bfa2369fd66af4b516f 100644
--- a/ipalib/plugins/idrange.py
+++ b/ipalib/plugins/idrange.py
@@ -531,6 +531,11 @@ class idrange_mod(LDAPUpdate):
# perform this check only if the attribute was changed
self.obj.validate_trusted_domain_sid(
entry_attrs['ipanttrusteddomainsid'])
+
+ # Add trusted AD domain range object class, if it wasn't there
+if not 'ipatrustedaddomainrange' in
Re: [Freeipa-devel] [PATCHES] 0183-0185 Drop CSV support
On 02/22/2013 02:36 PM, Martin Kosek wrote: On 02/22/2013 02:01 PM, Martin Kosek wrote: On 02/21/2013 06:08 PM, Petr Viktorin wrote: These patches remove CSV parsing from the client. Ticket: https://fedorahosted.org/freeipa/ticket/3352 Design: http://freeipa.org/page/V3/Drop_CSV The design page also talks about adding a warning for the user when they seem to use CSV, but this will need the JSON transport so it's not included in these patches. The csv flag is left on parameters so when that warning is added, we know for which params to show it. Generally, this works fine. This finally allows us to easily add records with quote or comma: # ipa dnsrecord-add example.com txt1 --txt-rec=foo, bar Record name: txt1 TXT record: foo, bar # ipa dnsrecord-add example.com txt1 --txt-rec='bar,bar' Record name: txt1 TXT record: foo, bar, bar,bar # ipa dnsrecord-show example.com txt1 --all --raw dn: idnsname=txt1,idnsname=example.com,cn=dns,dc=example,dc=com idnsname: txt1 txtrecord: foo, bar txtrecord: bar,bar objectclass: top objectclass: idnsrecord Just couple minor remarks: 1) Can we then update the csv flag description? It is now a bit misleading given its new function - csv: this multivalue attribute is given in CSV format I have reworded that in patch 184: - - csv: this multivalue attribute is given in CSV format + - csv: this multivalue attribute used to be given in CSV format in CLI 2) We need to also update our help, for example selfservice online help presents a CSV formatted option use: ipa selfservice-add --permissions=write --attrs=street,postalCode,l,c,st Users manage their own address Btw if we suggest the curly braces shortcut, we may also want to write that it applies for BASH shell only... Few other examples I saw: ipa group-add-member --users=test1,test2 localadmins Attaching patch for that. I used the Bash shortcut when there were more than two items. I grepped through the sources to find examples that need changing, hopefully I've caught them all. Just discovered another issue... We now crash when adding permissions/acis with csv: # ipa permission-add --attrs=member --permissions=read,write --type=group foo2 ipa: ERROR: an internal error has occurred We should report some meaningful error, otherwise users used to add permissions with csv values would be confused. See ticket #3420, patch 0182. -- Petr³ From 8bcd00ca6b58a8c6d2f658db220659915db0b640 Mon Sep 17 00:00:00 2001 From: Petr Viktorin pvikt...@redhat.com Date: Fri, 22 Feb 2013 08:48:50 -0500 Subject: [PATCH] Update plugin docstrings (topic help) to reflect dropped CSV support https://fedorahosted.org/freeipa/ticket/3352 --- ipalib/plugins/aci.py |4 ++-- ipalib/plugins/delegation.py |2 +- ipalib/plugins/group.py|4 ++-- ipalib/plugins/hbacrule.py |2 +- ipalib/plugins/hbacsvcgroup.py |4 ++-- ipalib/plugins/hbactest.py |6 +++--- ipalib/plugins/hostgroup.py|4 ++-- ipalib/plugins/netgroup.py |2 +- ipalib/plugins/selfservice.py | 10 ++ ipalib/plugins/sudocmdgroup.py |2 +- 10 files changed, 21 insertions(+), 19 deletions(-) diff --git a/ipalib/plugins/aci.py b/ipalib/plugins/aci.py index 4bfcf258998de6b6470c86f3645b61a5d2f5816d..eddb26a471a2eee970e5e24e2219ddfec96b 100644 --- a/ipalib/plugins/aci.py +++ b/ipalib/plugins/aci.py @@ -101,10 +101,10 @@ command-line now (see last example). ipa aci-add --permissions=write --attrs=member --targetgroup=admins --group=editors --prefix=none Editors manage admins Add an ACI that allows members of the admins group to manage the street and zip code of those in the editors group: - ipa aci-add --permissions=write --memberof=editors --group=admins --attrs=street,postalcode --prefix=none admins edit the address of editors + ipa aci-add --permissions=write --memberof=editors --group=admins --attrs=street --attrs=postalcode --prefix=none admins edit the address of editors Add an ACI that allows the admins group manage the street and zipcode of those who work for the boss: - ipa aci-add --permissions=write --group=admins --attrs=street,postalcode --filter=(manager=uid=boss,cn=users,cn=accounts,dc=example,dc=com) --prefix=none Edit the address of those who work for the boss + ipa aci-add --permissions=write --group=admins --attrs=street --attrs=postalcode --filter=(manager=uid=boss,cn=users,cn=accounts,dc=example,dc=com) --prefix=none Edit the address of those who work for the boss Add an entirely new kind of record to IPA that isn't covered by any of the --type options, creating a permission: ipa permission-add --permissions=add --subtree=cn=*,cn=orange,cn=accounts,dc=example,dc=com --desc=Add Orange Entries add_orange diff --git a/ipalib/plugins/delegation.py b/ipalib/plugins/delegation.py index 6228cc50f03718a2cd147a57a853838976d62730..bab76ccbcbd920f8e98ef4c1f2da791a445d9254 100644 ---
Re: [Freeipa-devel] [PATCH] 0006 Remove check for alphabetic only characters from domain name validation
On 02/22/2013 10:19 AM, Petr Spacek wrote:
On 20.2.2013 11:03, Ana Krivokapic wrote:
On 02/18/2013 01:08 PM, Martin Kosek wrote:
On 02/18/2013 12:47 PM, Sumit Bose wrote:
On Mon, Feb 18, 2013 at 12:27:35PM +0100, Petr Spacek wrote:
On 15.2.2013 15:22, Ana Krivokapic wrote:
Hello,
The .isalpha() check in validate_domain_name() was too strict,
causing some commands like ipa dnsrecord-add to fail.
https://fedorahosted.org/freeipa/ticket/3385
I would add --force option rather than removing whole check, if
it's possible.
Would it be possible to mention RFC in the error message? Something
like _('top level domain label must be alphabetic (RFC 1123 section
2.1)')
?
IMHO it is handy, because it educates users.
The problem is that this check is always done on the last component of
the domain_name even if it is just a sub-domain of the FreeIPA domain,
where e.g. numbers are valid characters.
At the beginning of validate_domain_name() a trailing '.' is stripped
away. iirc the trailing '.' is an indication for a complete, fully
qualified name. Would it work if the presence of the trailing '.' is
saved and the check is only done if there was a '.'?
bye,
Sumit
Sure. Though I am now not 100% sure that some IPA functions do not
use this
validator with a fqdn hostname without trailing dot. If not, I am
for fixing
this function as Sumit and Petr suggested.
Martin
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
After some thought, I decided to change the approach.
As pointed out by Sumit, the problem was that the validate_domain_name()
function did not distinguish between fqdn and non-fqdn domains
(subdomains of the IPA domain). The trailing dot is not a clear
indication either, because some IPA functions use this validator with an
fqdn without the trailing dot.
To fix this, I introduced an additional parameter to this function - a
flag which indicates whether the domain name is an fqdn or not. The is
.isalpha() check is then performed only in the case of an fqdn.
I also improved the error message to mention the relevant RFC, as
suggested by Petr.
Please don't forget to add --force switch. It could be handy.
I added the --force switch to ipa dnsrecord-add and opened a new ticket
to handle the rest of the ipa commands that use domain name validation:
https://fedorahosted.org/freeipa/ticket/3455
Updated patch is attached.
--
Regards,
Ana Krivokapic
Associate Software Engineer
FreeIPA team
Red Hat Inc.
From d322f309f9e646489d5328f7bb417dc2387f33b6 Mon Sep 17 00:00:00 2001
From: Ana Krivokapic akriv...@redhat.com
Date: Fri, 22 Feb 2013 09:25:39 -0500
Subject: [PATCH] Improve domain name validation
Check for alphabetic only characters is now performed only in the
case of fully qualified domain names. This fixes the bug that caused
some ipa commands (e.g. ipa dnsrecord-add) to fail due to the presence
of numbers in the domain name.
This patch also adds --force option to ipa dnsrecord-add to allow
this command to skip domain name validation.
https://fedorahosted.org/freeipa/ticket/3385
---
ipalib/plugins/dns.py | 1 -
ipalib/util.py| 10 +-
2 files changed, 5 insertions(+), 6 deletions(-)
diff --git a/ipalib/plugins/dns.py b/ipalib/plugins/dns.py
index 61c2de3217e206bfcd3c68fa6a3f4d2611d0815e..5531f96687fdaabe39798f0cc2a72f4f28fdba68 100644
--- a/ipalib/plugins/dns.py
+++ b/ipalib/plugins/dns.py
@@ -2296,7 +2296,6 @@ class dnsrecord_add(LDAPCreate):
takes_options = LDAPCreate.takes_options + (
Flag('force',
label=_('Force'),
- flags=['no_option', 'no_output'],
doc=_('force NS record creation even if its hostname is not in DNS'),
),
dnsrecord.structured_flag,
diff --git a/ipalib/util.py b/ipalib/util.py
index 9ed27c84ec8d189507410ba141d8968fc38f674c..5a5af69eac68adc51ee5d55a8c570275f64c6aee 100644
--- a/ipalib/util.py
+++ b/ipalib/util.py
@@ -235,7 +235,7 @@ def validate_dns_label(dns_label, allow_underscore=False):
'DNS label may not start or end with -') \
% dict(underscore=underscore_err_msg))
-def validate_domain_name(domain_name, allow_underscore=False):
+def validate_domain_name(domain_name, allow_underscore=False, fqdn=True):
if domain_name.endswith('.'):
domain_name = domain_name[:-1]
@@ -244,9 +244,9 @@ def validate_domain_name(domain_name, allow_underscore=False):
# apply DNS name validator to every name part
map(lambda label:validate_dns_label(label,allow_underscore), domain_name)
-if not domain_name[-1].isalpha():
+if fqdn and not domain_name[-1].isalpha():
# see RFC 1123
-raise ValueError(_('top level domain label must be alphabetic'))
+raise ValueError(_('top level domain label must be alphabetic (RFC 1123 section 2.1)'))
def
Re: [Freeipa-devel] [PATCHES 0024-0025] Improvements to idrange.py
On 02/22/2013 03:01 PM, Tomas Babej wrote:
On 02/21/2013 02:22 PM, Martin Kosek wrote:
On 02/20/2013 03:19 PM, Tomas Babej wrote:
On Wed 20 Feb 2013 02:24:03 PM CET, Alexander Bokovoy wrote:
On Wed, 20 Feb 2013, Tomas Babej wrote:
On 12/21/2012 12:15 PM, Tomas Babej wrote:
Hi,
Sending updated and rebased versions of patches 0024 and 0025.
Tomas
Sending rebased version, these got quite rotten.
Thanks for updating them.
@@ -504,25 +515,37 @@ class idrange_mod(LDAPUpdate):
'not be found. Please specify the SID
directly '
'using dom-sid option.'))
-try:
-(old_dn, old_attrs) = ldap.get_entry(dn,
-['ipabaseid',
-'ipaidrangesize',
-'ipabaserid',
-'ipasecondarybaserid'])
-except errors.NotFound:
-self.obj.handle_not_found(*keys)
+if in_updated_attrs('ipanttrusteddomainsid'):
+if in_updated_attrs('ipasecondarybaserid'):
+raise errors.ValidationError(name='ID Range setup',
+error=_('Options dom_sid and secondary_rid_base
cannot '
+'be used together'))
Since we agreed to refer to options by their CLI name (--dom-sid and
--secondary-rid-base) in the other patch, it makes sense to use it
here too.
-if is_set('ipanttrusteddomainsid'):
-# Validate SID as the one of trusted domains
-
self.obj.validate_trusted_domain_sid(entry_attrs['ipanttrusteddomainsid'])
+if not in_updated_attrs('ipabaserid'):
+raise errors.ValidationError(name='ID Range setup',
+error=_('Options dom_sid and rid_base must '
+'be used together'))
Same here.
+# secondary base rid must be set if and only if base rid
is set
+if in_updated_attrs('ipasecondarybaserid') !=\
+in_updated_attrs('ipabaserid'):
+raise errors.ValidationError(name='ID Range setup',
+error=_('Options secondary_rid_base and rid_base
must '
+'be used together'))
Same here.
+dict(
+desc='Try to modify ID range %r so it has only primary
rid range set' % (testrange8),
+command=('idrange_mod', [testrange8],
+ dict(ipabaserid=testrange8_base_rid)),
+expected=errors.ValidationError(
+name='ID Range setup', error='Options
secondary_rid_base and rid_base must be used together'),
+),
And synchronize error message here too.
Thanks!
Sending the updated patch 0024.
Tomas
In patch 0024 your intention is OK, but the checking functions are not:
is_set = lambda x: (x in entry_attrs) and (x is not None)
+in_updated_attrs = lambda x: any((x in attrs and x is not None)
+ for attrs in (entry_attrs,
old_attrs))
They return True even when the attribute is None because they check if *x* is
None and not if *attrs[x]* is None. Example:
# ipa idrange-add --base-id=120 --range-size=20 --rid-base=1000
--secondary-rid-base=100 local_range
Added ID range local_range
Range name: local_range
First Posix ID of the range: 120
Number of IDs in the range: 20
First RID of the corresponding RID range: 1000
First RID of the secondary RID range: 100
Range type: local domain range
This command should be NOOP, but is not:
# ipa idrange-mod local_range --dom-sid=
ipa: ERROR: invalid 'ID Range setup': Options dom-sid and secondary-rid-base
cannot be used together
Martin
Thanks for the catch, all checking functions fixed.
Sending the complete patchset, up to date.
Tomas
I think I am being a nitpicker now (sorry), but this condition also fails if
the old_attrs has a setting, but I am removing it in a current -mod command:
# ipa idrange-add --base-id=120 --range-size=20 --rid-base=1000
--secondary-rid-base=100 local_range
Added ID range local_range
Range name: local_range
First Posix ID of the range: 120
Number of IDs in the range: 20
First RID of the corresponding RID range: 1000
First RID of the secondary RID range: 100
Range type: local domain range
# ipa idrange-mod local_range --dom-sid S-1-2 --secondary-rid-base=
ipa: ERROR: invalid 'ID Range setup': Options dom-sid and secondary-rid-base
cannot be used together
Martin
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 0182 Fix permission validation and normalization in aci.py
On 02/21/2013 01:58 PM, Petr Viktorin wrote: This patch fixes an internal error in the permission plugin that would become more noticeable when CSV is dropped. https://fedorahosted.org/freeipa/ticket/3420 ACK. Pushed to master, ipa-3-1. Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCH] 264-265 Web UI:Certificate pages
Note: static json files for testing and such will be updated soon (there
were several patch which changes API. I rather want to do one mass
regeneration than several minor ones in a short period of time.
1) [PATCH] Web UI:Certificate pages
Following pages were added to Web UI:
* certificated details
* certificate search
Certificate is not regular object so it gets no metadata. Therefore
artificial metadata were created for it to allow usage of search and
details facet.
Search and details facet were modified to allow removing of
add/remove/update/reset buttons - certificates have no mod operation and
they are not added by standard means.
User can revoke and restore certificated in details facet.
https://fedorahosted.org/freeipa/ticket/3419
2) [PATCH] Web UI:Choose different search option for cert-find
This extends certificate search page by search option select. Therefore
the search is not restricted to 'subject'.
It should be replaced by https://fedorahosted.org/freeipa/ticket/191 in
a future.
https://fedorahosted.org/freeipa/ticket/3419
--
Petr Vobornik
From 868040b866e6180f81fae2d8c7f50482e09824e4 Mon Sep 17 00:00:00 2001
From: Petr Vobornik pvobo...@redhat.com
Date: Fri, 22 Feb 2013 17:22:30 +0100
Subject: [PATCH] Web UI:Certificate pages
Following pages were added to Web UI:
* certificated details
* certificate search
Certificate is not regular object so it gets no metadata. Therefore artificial
metadata were created for it to allow usage of search and details facet.
Search and details facet were modified to allow removing of add/remove/update/
reset buttons - certificates have no mod operation and they are not added by
standard means.
User can revoke and restore certificated in details facet.
https://fedorahosted.org/freeipa/ticket/3419
---
install/ui/src/freeipa/certificate.js | 255 --
install/ui/src/freeipa/details.js | 24 ++--
install/ui/src/freeipa/facet.js | 1 +
install/ui/src/freeipa/search.js | 24 ++--
install/ui/src/freeipa/webui.js | 3 +-
install/ui/src/freeipa/widget.js | 3 +
install/ui/test/data/ipa_init.json| 7 +
ipalib/plugins/internal.py| 7 +
8 files changed, 294 insertions(+), 30 deletions(-)
diff --git a/install/ui/src/freeipa/certificate.js b/install/ui/src/freeipa/certificate.js
index f7bc843590ee621a071c764131dcf9ed9f5e75f6..1b6a129c3afeebd65182f8addf2884be66abffa5 100755
--- a/install/ui/src/freeipa/certificate.js
+++ b/install/ui/src/freeipa/certificate.js
@@ -19,7 +19,7 @@
* along with this program. If not, see http://www.gnu.org/licenses/.
*/
-define(['./ipa', './jquery', './dialog'], function(IPA, $) {
+define(['./ipa', './jquery','dojo/_base/lang', './dialog'], function(IPA, $, lang) {
IPA.cert = {};
@@ -486,6 +486,7 @@ IPA.cert.load_policy = function(spec) {
var that = IPA.facet_policy();
that.loader = IPA.build(spec.loader);
+that.has_reason = spec.has_reason;
that.post_load = function(data) {
@@ -499,7 +500,8 @@ IPA.cert.load_policy = function(spec) {
// initialize another load of certificate because current entity
// show commands don't contain revocation_reason so previous data
// might be slightly incorrect
-if (certificate certificate.certificate !IPA.cert.is_selfsign()) {
+if (!that.has_reason certificate certificate.certificate
+!IPA.cert.is_selfsign()) {
that.load_revocation_reason(certificate.serial_number);
}
};
@@ -672,9 +674,12 @@ IPA.cert.revoke_action = function(spec) {
var entity_label = that.entity_label || facet.entity.metadata.label_singular;
var entity_name = certificate.entity_info.name;
-var title = IPA.messages.objects.cert.revoke_certificate;
-title = title.replace('${entity}', entity_label);
-title = title.replace('${primary_key}', entity_name);
+var title = IPA.messages.objects.cert.revoke_certificate_simple;
+if (entity_name entity_label) {
+title = IPA.messages.objects.cert.revoke_certificate;
+title = title.replace('${entity}', entity_label);
+title = title.replace('${primary_key}', entity_name);
+}
that.dialog.title = title;
that.dialog.message = that.get_confirm_message(facet);
@@ -725,9 +730,12 @@ IPA.cert.restore_action = function(spec) {
var entity_label = that.entity_label || facet.entity.metadata.label_singular;
var entity_name = certificate.entity_info.name;
-var title = IPA.messages.objects.cert.restore_certificate;
-title = title.replace('${entity}', entity_label);
-title = title.replace('${primary_key}', entity_name);
+var title = IPA.messages.objects.cert.restore_certificate_simple;
+if (entity_name entity_label) {
+title = IPA.messages.objects.cert.restore_certificate;
+title =
Re: [Freeipa-devel] [PATCHES] 0183-0185 Drop CSV support
On 02/22/2013 03:05 PM, Petr Viktorin wrote: On 02/22/2013 02:36 PM, Martin Kosek wrote: On 02/22/2013 02:01 PM, Martin Kosek wrote: On 02/21/2013 06:08 PM, Petr Viktorin wrote: These patches remove CSV parsing from the client. Ticket: https://fedorahosted.org/freeipa/ticket/3352 Design: http://freeipa.org/page/V3/Drop_CSV The design page also talks about adding a warning for the user when they seem to use CSV, but this will need the JSON transport so it's not included in these patches. The csv flag is left on parameters so when that warning is added, we know for which params to show it. Generally, this works fine. This finally allows us to easily add records with quote or comma: # ipa dnsrecord-add example.com txt1 --txt-rec=foo, bar Record name: txt1 TXT record: foo, bar # ipa dnsrecord-add example.com txt1 --txt-rec='bar,bar' Record name: txt1 TXT record: foo, bar, bar,bar # ipa dnsrecord-show example.com txt1 --all --raw dn: idnsname=txt1,idnsname=example.com,cn=dns,dc=example,dc=com idnsname: txt1 txtrecord: foo, bar txtrecord: bar,bar objectclass: top objectclass: idnsrecord Just couple minor remarks: 1) Can we then update the csv flag description? It is now a bit misleading given its new function - csv: this multivalue attribute is given in CSV format I have reworded that in patch 184: - - csv: this multivalue attribute is given in CSV format + - csv: this multivalue attribute used to be given in CSV format in CLI 2) We need to also update our help, for example selfservice online help presents a CSV formatted option use: ipa selfservice-add --permissions=write --attrs=street,postalCode,l,c,st Users manage their own address Btw if we suggest the curly braces shortcut, we may also want to write that it applies for BASH shell only... Few other examples I saw: ipa group-add-member --users=test1,test2 localadmins Attaching patch for that. I used the Bash shortcut when there were more than two items. I grepped through the sources to find examples that need changing, hopefully I've caught them all. Just discovered another issue... We now crash when adding permissions/acis with csv: # ipa permission-add --attrs=member --permissions=read,write --type=group foo2 ipa: ERROR: an internal error has occurred We should report some meaningful error, otherwise users used to add permissions with csv values would be confused. See ticket #3420, patch 0182. Thanks for the fixes. ACK. Pushed to master. Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] DESIGN: Recover DNA Ranges
Design to allow one to recover DNA ranges when deleting a replica or just for normal range management. http://freeipa.org/page/V3/Recover_DNA_Ranges Supporting ticket https://fedorahosted.org/freeipa/ticket/3321 rob ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCH 0036] Make sure appropriate exit status is returned in make-test
Hi, The make-test script now exits with code 1 in case that any of the test cases that were run failed. Can we push this without a ticket under one-liner rule? Tomas From f4c6cad856be076d1c367edf2e9ced1b3c15b15a Mon Sep 17 00:00:00 2001 From: Tomas Babej tba...@redhat.com Date: Sat, 23 Feb 2013 00:41:58 +0100 Subject: [PATCH] Make sure appropriate exit status is returned in make-test The make-test script now returns 1 in case that any of the test cases that were run failed. --- make-test | 2 ++ 1 file changed, 2 insertions(+) diff --git a/make-test b/make-test index 02a17db9c039869dc11f48a3880841f1ad8a9cde..b39e4dbde410d64edc69791d617349cbb6e9c903 100755 --- a/make-test +++ b/make-test @@ -52,5 +52,7 @@ for pver in ran: print '' if fail: print '** FAIL **' +sys.exit(1) else: print '** pass **' +sys.exit(0) -- 1.8.1 ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
