Re: [Freeipa-devel] [PATCH] 270 Improve migration NotFound error

2012-06-04 Thread Martin Kosek
On Mon, 2012-06-04 at 23:13 -0400, Rob Crittenden wrote:
> Martin Kosek wrote:
> > When no user/group was found, migration plugin reported an ambiguous
> > error about invalid container. But the root cause may be for example
> > in a wrong list of user/group objectclasses. Report both in the error
> > message to avoid user confusion.
> >
> > User/group objectclass attribute is now also marked as required.
> > Without the list of objectclasses, an invalid LDAP search is
> > produced.
> >
> > https://fedorahosted.org/freeipa/ticket/2206
> 
> ACK. The output is a lot readable, you might reconsider having it in 
> parens. A separate sentence or separated by a colon may be more readable.
> 
> rob

I tried different formats, but the former approach still seemed to me as
the most readable, so I kept that :-)

Pushed to master.

Martin

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


Re: [Freeipa-devel] [PATCH] 271 Fill new DNS zone update policy by default

2012-06-04 Thread Martin Kosek
On Mon, 2012-06-04 at 22:39 -0400, Rob Crittenden wrote:
> Martin Kosek wrote:
> > For security reasons, dynamic updates are not enabled for new DNS
> > zones. In order to enable the dynamic zone securely, user needs to
> > allow dynamic updates and create a zone update policy.
> >
> > The policy is not easy to construct for regular users, we should
> > rather fill it by default and let users just switch the policy
> > on or off.
> >
> > https://fedorahosted.org/freeipa/ticket/2441
> 
> I think the example should be something like:
> 
>Modify the zone to allow dynamic updates for hosts own records in 
> realm EXAMPLE.COM:
> ipa dnszone-mod example.com --dynamic-update=TRUE
> 
>This is the equivalent of:
> ipa dnszone-mod example.com --dynamic-update=TRUE \\
>  --update-policy="grant EXAMPLE.COM krb5-self * A; grant 
> EXAMPLE.COM krb5-self * ;"

Right, I did that change.

> 
> Otherwise ACK.
> 
> rob

Thanks. I also found out that I forgot to update DNS unit tests, so I
fixed that as well before pushing.

Pushed to master.

Martin

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


Re: [Freeipa-devel] [PATCH] 271 Fill new DNS zone update policy by default

2012-06-04 Thread William Brown

> I think the example should be something like:
> 
>   Modify the zone to allow dynamic updates for hosts own records in
> realm EXAMPLE.COM:
>ipa dnszone-mod example.com --dynamic-update=TRUE
> 
>   This is the equivalent of:
>ipa dnszone-mod example.com --dynamic-update=TRUE \\
> --update-policy="grant EXAMPLE.COM krb5-self * A; grant
> EXAMPLE.COM krb5-self * ;"
> 

What about reverse zones?

-- 
Sincerely,

William Brown

pgp.mit.edu
http://pgp.mit.edu:11371/pks/lookup?op=vindex&search=0x3C0AC6DAB2F928A2



signature.asc
Description: OpenPGP digital signature
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Re: [Freeipa-devel] [PATCH] 262-265 Enable psearch by default

2012-06-04 Thread Rob Crittenden

Martin Kosek wrote:

On Fri, 2012-05-25 at 17:14 +0200, Martin Kosek wrote:

On Fri, 2012-05-25 at 09:25 -0400, Rob Crittenden wrote:

Martin Kosek wrote:

This set of patches handles enabling psearch both for new installations
(patch 263) and upgraded IPA servers.

For upgraded IPA servers I needed to make sure that psearch is not
enabled for every IPA package update, but at most once, when a user
updates to IPA with this patch for the first time (patch 264). This is
enabled by a new State store located in /var/lib/ipa/sysupgrade (patch
262).

I also improved the way we handled SELinux sebool updates (patch 265),
this can make ipa-upgradeconfig to finish in 0.4 seconds and not in 150
seconds as previously. Details are in the patches.

Martin


262:
The sysupgrade directory isn't created by the RPM install:

mkdir -p %{buildroot}/%{_localstatedir}/cache/ipa/sysupgrade


Fixed.



263:

It looks like zone_refresh is simply disabled in bindinstance.py, why
not remove it completely?


zone_refresh is used by bindinstance.py. ipa-server-install or
ipa-dns-install may be configured to use zone refresh instead of
persistent search mechanism to update the zones (e.g. --zone-refresh
30).



264:

Small nit, worth doing case-insensitive compare of psearch enabled status?


Petr2 told me that arg value for boolean configuration option is
case-insensitive, so we can do that - fixed.



We're updating named.conf in place so I don't know that we need to reset
permissions. It at least shouldn't get modified by the write.


Right, I was being too defensive. I removed the check.

I made the upgrade more robust, now it won't crash for example when
named.conf does not exist. I also made sure the upgrade script works
correctly when the IPA is configured without DNS.

Martin


I rebased the patches for current master. I also slightly reworked patch
265, the error message printed in case of an unsuccessful setsebool was
not printed right.

Martin


Trailing whitespace in 264:

# git am /tmp/freeipa-mkosek-264-3-enable-psearch-on-upgrades.patch
Applying: Enable psearch on upgrades
/home/rcrit/redhat/freeipa-nossh/.git/rebase-apply/patch:108: trailing 
whitespace.
root_logger.error('Cannot update connections in %s: 
%s',

warning: 1 line adds whitespace errors.

I don't think the DNS detection is adequate in 264, testing for 
named.conf is not enough. What if someone is running a non-IPA DNS 
server on the box?


I know that I've recently done similar config changes but in 265 is 
using line.startswith() going to be fragile?


In 266 I'd merge in the ipa-upgradeconfig change into 265 or some other 
patch.


In the 'for setting, state' loop should it be catching a 
CalledProcessException rather than raw Exception? I think that is all 
that should be raised there.


I did an upgrade and it seemed to work ok, ended up with these scary 
messages in /var/log/messages:


Jun  4 23:39:17 localhost named[18753]: LDAP error: Can't contact LDAP 
server
Jun  4 23:39:17 localhost named[18753]: connection to the LDAP server 
was lost
Jun  4 23:39:17 localhost named[18753]: bind to LDAP server failed: 
Can't contact LDAP server
Jun  4 23:39:17 localhost named[18753]: ldap_psearch_watcher failed to 
handle LDAP connection error. Reconnection in 60s
Jun  4 23:39:17 localhost named[18753]: LDAP error: Can't contact LDAP 
server
Jun  4 23:39:17 localhost named[18753]: connection to the LDAP server 
was lost
Jun  4 23:39:17 localhost named[18753]: bind to LDAP server failed: 
Can't contact LDAP server
Jun  4 23:39:17 localhost ns-slapd[18798]: [04/Jun/2012:23:39:17 -0400] 
- Information: Non-Secure Port Disabled
Jun  4 23:40:17 localhost named[18753]: handle_connection_error failed 
to obtain ldap error code
Jun  4 23:40:17 localhost named[18753]: connection to the LDAP server 
was lost
Jun  4 23:40:17 localhost named[18753]: bind to LDAP server failed: 
Can't contact LDAP server
Jun  4 23:40:17 localhost named[18753]: ldap_psearch_watcher failed to 
handle LDAP connection error. Reconnection in 60s
Jun  4 23:41:17 localhost named[18753]: handle_connection_error failed 
to obtain ldap error code
Jun  4 23:41:17 localhost named[18753]: connection to the LDAP server 
was lost


DNS does seem to be working fine from the cli.

The tests are another matter. named crashed in 0:1.1.0-0.10.b2.fc17 in 
the test cleanup.


I upgraded to bind-dyndb-ldap-1.1.0-0.11.rc1.fc17 and got this stack trace:

Program received signal SIGABRT, Aborted.
[Switching to Thread 0x7f68e50db700 (LWP 19367)]
0x7f68e6188915 in raise () from /lib64/libc.so.6
(gdb) where
#0  0x7f68e6188915 in raise () from /lib64/libc.so.6
#1  0x7f68e618a0c8 in abort () from /lib64/libc.so.6
#2  0x7f68e91171fb in assertion_failed (file=,
line=, type=, cond=)
at ./main.c:219
#3  0x7f68e73a6c3a in isc_assertion_failed (
file=file@entry=0x7f68e8a82deb "zone.c", line=,
type=type@entry=isc_assertiontype_require,
cond=cond@entry=0x7f68e8a82fe7

Re: [Freeipa-devel] [PATCH] 270 Improve migration NotFound error

2012-06-04 Thread Rob Crittenden

Martin Kosek wrote:

When no user/group was found, migration plugin reported an ambiguous
error about invalid container. But the root cause may be for example
in a wrong list of user/group objectclasses. Report both in the error
message to avoid user confusion.

User/group objectclass attribute is now also marked as required.
Without the list of objectclasses, an invalid LDAP search is
produced.

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


ACK. The output is a lot readable, you might reconsider having it in 
parens. A separate sentence or separated by a colon may be more readable.


rob

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


Re: [Freeipa-devel] [PATCH] 147 Set network.http.sendRefererHeader to 2 on browser config

2012-06-04 Thread Rob Crittenden

Petr Vobornik wrote:

On 05/29/2012 11:29 PM, Rob Crittenden wrote:

Petr Vobornik wrote:

IPA web UI isn't functional when browser doesn't send http headers.

This patch adds a functionality which sets Firefox
network.http.sendRefererHeader configuration option to value '2' which
enables it.

Possible values:
http://kb.mozillazine.org/Network.http.sendRefererHeader

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


Should we also add a message when referer is missing to check this
setting in about:config?


I'm not sure what you have in mind. We set the referer option so why
would user check it afterwards?

Yes the ticket was about checking the option but: If user is configuring
the browser he wants the browser configured. So we should set all
options which are required. This is one of them. We have not been
notifying the user what was set, so I didn't add such notification for
this option now as well.

We might want to notify the user what options were changed but it's not
the topic of this ticket.


I was thinking more for already configured browsers who then later mess 
with this value. It fails in a very non-obvious way.


rob

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


Re: [Freeipa-devel] [PATCH] 492 Add options to reduce writes from KDC

2012-06-04 Thread Rob Crittenden

Simo Sorce wrote:

The original ldap driver we used up to 2.2 had 2 options admins could
set to limit the amount of writes to the database on certain auditing
related operations.
In particular disable_last_success is really important to reduce the
load on database servers.

I have implemented ticket #2734 with a little twist. Instead of adding
local options in krb5.conf I create global options in the LDAP tree, so
that all KDCs in the domain have the same configuration.

The 2 new options can be set in ipaConfigString attribute of the
cn=ipaConfig object under cn=etc,$SUFFIX

These are:
KDC:Disable Last Success
KDC:Disable Lockout

The first string if set will disable updating the krbLastSuccessfulAuth
field in the service/user entry.
The second one will prevent changing any of the Lockout related fields
and will effectively disable lockout policies.

I think we may want to set the first one by default in future.
The last successful auth field is not very interesting in general and is
cause for a lot of writes that pressure a lot the LDAP server and get
replicated everywhere with a storm multiplier effect we'd like to avoid.

The lockout one instead happen only when there are failed authentication
attempt, this means it never happens when keytabs are used for example.
And even with users it should happen rarely enough that traking lockouts
by default make leaving these writes on by default is a good tradeoff.

Note that simply setting the lockout policy to never lockout is *not*
equivalent to setting KDC:Disable Lockout, as it does not prevent writes
to the database.

I've tested setting KDC:Disable Last Success and it effectively prevent
MOD operation from showing up in the server access log.

Any change to these configuration options requires a reconnection from
the KDC to the LDAP server, the simplest way to cause that is to restart
the KDC service.

Simo.


In ipadb_get_global_configs() should there be a call to LOG_OOM()?

Also, if ipadb_simple_search() or ipadb_get_global_configs() fails 
should we log the result code when non-zero?


rob

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


Re: [Freeipa-devel] [PATCH] 271 Fill new DNS zone update policy by default

2012-06-04 Thread Rob Crittenden

Martin Kosek wrote:

For security reasons, dynamic updates are not enabled for new DNS
zones. In order to enable the dynamic zone securely, user needs to
allow dynamic updates and create a zone update policy.

The policy is not easy to construct for regular users, we should
rather fill it by default and let users just switch the policy
on or off.

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


I think the example should be something like:

  Modify the zone to allow dynamic updates for hosts own records in 
realm EXAMPLE.COM:

   ipa dnszone-mod example.com --dynamic-update=TRUE

  This is the equivalent of:
   ipa dnszone-mod example.com --dynamic-update=TRUE \\
--update-policy="grant EXAMPLE.COM krb5-self * A; grant 
EXAMPLE.COM krb5-self * ;"


Otherwise ACK.

rob

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


[Freeipa-devel] [PATCH] 151, 152 Removal of illegal options in association dialog

2012-06-04 Thread Petr Vobornik

[PATCH] 152 Removal of illegal options in association dialog:

Association dialogs were using non-existent options for find commands. 
It causes error when #2509 is implemented.


Now when creating a find command a check for options existence is 
performed. Option is not used if not present in metadata. It fixes the 
issue.


To be able to do the this check properly patch 151 is required.

[PATCH] 151 Change json serialization to serialize useful data:

json_metadata command creates and sends metadata needed by Web UI. It 
uses __json__ method for serialization of commands, options, objects... 
. A lot of data sent was useless for Web UI and some usefull information 
were missing. We

 * mostly CLI specific option attribues are not send.
 * attributes evaluated to false or None are not send
 * options which are send are not got from takes_aptions attribute but 
by get_options() method. It finally sends usefull option collection for 
commands part of metadata.


In the end the raw amount of data send is aproximately the same.

This patch is needed for Web UI to determine which option it can use in 
which commands.


https://fedorahosted.org/freeipa/ticket/2760
--
Petr Vobornik
From f8fb3dd8f477895bcbe4f4f733aadeb199ee24d0 Mon Sep 17 00:00:00 2001
From: Petr Vobornik 
Date: Wed, 30 May 2012 12:55:08 +0200
Subject: [PATCH] Change json serialization to serialize useful data

json_metadata command creates and sends metadata needed by Web UI. It uses __json__ method for serialization of commands, options, objects... . A lot of data sent was useless for Web UI and some usefull information were missing. We
 * mostly CLI specific option attribues are not send.
 * attributes evaluated to false or None are not send
 * options which are send are not got from takes_aptions attribute but by get_options() method. It finally sends usefull option collection for commands part of metadata.

In the end the raw amount of data send is aproximately the same.

This patch is needed for Web UI to determine which option it can use in which commands.

https://fedorahosted.org/freeipa/ticket/2760
---
 ipalib/frontend.py |   21 -
 ipalib/parameters.py   |   15 ++-
 ipalib/plugins/baseldap.py |9 ++---
 3 files changed, 40 insertions(+), 5 deletions(-)

diff --git a/ipalib/frontend.py b/ipalib/frontend.py
index 7c63b27ddb41e751692e47ebc334cb699606a74e..c28fa54ae933fe58833579b3e3aee0e1ad50f198 100644
--- a/ipalib/frontend.py
+++ b/ipalib/frontend.py
@@ -1012,13 +1012,32 @@ class Command(HasParam):
 
 # list of attributes we want exported to JSON
 json_friendly_attributes = (
-'name', 'takes_args', 'takes_options',
+'name', 'takes_args',
 )
 
+# list of options we want only to mention their presence and not to write
+# their attributes
+json_only_presence_options = (
+'all', 'raw', 'attrs', 'addattr', 'delattr', 'setattr', 'version',
+)
+
+def get_json_options(self):
+"""
+Get only options we want exported to JSON
+"""
+for option in self.get_options():
+if option.name not in self.json_only_presence_options:
+yield option
+else:
+yield { 'name': option.name }
+
 def __json__(self):
 json_dict = dict(
 (a, getattr(self, a)) for a in self.json_friendly_attributes
 )
+
+json_dict['takes_options'] = list(self.get_json_options())
+
 return json_dict
 
 class LocalOrRemote(Command):
diff --git a/ipalib/parameters.py b/ipalib/parameters.py
index 83a86544d5b66154944b9b279c042b638d3c57a5..47a4879aa44feac7357dd8cf4859b05d3884f521 100644
--- a/ipalib/parameters.py
+++ b/ipalib/parameters.py
@@ -958,15 +958,28 @@ class Param(ReadOnly):
 pass
 return self.default
 
+json_exclude_attrs = (
+'alwaysask', 'autofill', 'cli_name', 'cli_short_name', 'csv',
+'csv_separator', 'csv_skipspace', 'sortorder', 'falsehoods', 'truths',
+'version',
+)
+
 def __json__(self):
 json_dict = {}
 for (a, k, d) in self.kwargs:
+if a in self.json_exclude_attrs:
+continue
 if k in (callable, DefaultFrom):
 continue
 elif isinstance(getattr(self, a), frozenset):
 json_dict[a] = [k for k in getattr(self, a, [])]
 else:
-json_dict[a] = getattr(self, a, '')
+val = getattr(self, a, '')
+if val is None or not val:
+# ignore false and not set because lack of their presence is
+# the information itself
+continue;
+json_dict[a] = val
 json_dict['class'] = self.__class__.__name__
 json_dict['name'] = self.name
 json_dict['type'] = self.type.__name__
diff --git a/ipalib/plugins/baseldap.py b/ipalib/plugins/baseldap.py
index 2851f

[Freeipa-devel] [PATCH] 271 Fill new DNS zone update policy by default

2012-06-04 Thread Martin Kosek
For security reasons, dynamic updates are not enabled for new DNS
zones. In order to enable the dynamic zone securely, user needs to
allow dynamic updates and create a zone update policy.

The policy is not easy to construct for regular users, we should
rather fill it by default and let users just switch the policy
on or off.

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

>From bb4769b7860919cb43eef11891c9f14729a2f271 Mon Sep 17 00:00:00 2001
From: Martin Kosek 
Date: Mon, 4 Jun 2012 17:53:34 +0200
Subject: [PATCH] Fill new DNS zone update policy by default

For security reasons, dynamic updates are not enabled for new DNS
zones. In order to enable the dynamic zone securely, user needs to
allow dynamic updates and create a zone update policy.

The policy is not easy to construct for regular users, we should
rather fill it by default and let users just switch the policy
on or off.

https://fedorahosted.org/freeipa/ticket/2441
---
 API.txt   |2 +-
 VERSION   |2 +-
 ipalib/plugins/dns.py |   14 +++---
 ipalib/util.py|   29 +
 ipaserver/install/bindinstance.py |7 ---
 ipaserver/install/plugins/dns.py  |4 ++--
 6 files changed, 44 insertions(+), 14 deletions(-)

diff --git a/API.txt b/API.txt
index ba5aa1037e5d9b8661326afe4e6f984d52cc3cc8..501e8381450a7d203adf599746ecf372bdcb7043 100644
--- a/API.txt
+++ b/API.txt
@@ -1014,7 +1014,7 @@ option: Int('idnssoaexpire', attribute=True, autofill=True, cli_name='expire', d
 option: Int('idnssoaminimum', attribute=True, autofill=True, cli_name='minimum', default=3600, maxvalue=10800, minvalue=0, multivalue=False, required=True)
 option: Int('dnsttl', attribute=True, cli_name='ttl', multivalue=False, required=False)
 option: StrEnum('dnsclass', attribute=True, cli_name='class', multivalue=False, required=False, values=(u'IN', u'CS', u'CH', u'HS'))
-option: Str('idnsupdatepolicy', attribute=True, cli_name='update_policy', multivalue=False, required=False)
+option: Str('idnsupdatepolicy', attribute=True, autofill=True, cli_name='update_policy', multivalue=False, required=False)
 option: Bool('idnsallowdynupdate', attribute=True, autofill=True, cli_name='dynamic_update', default=False, multivalue=False, required=False)
 option: Str('idnsallowquery', attribute=True, autofill=True, cli_name='allow_query', default=u'any;', multivalue=False, required=False)
 option: Str('idnsallowtransfer', attribute=True, autofill=True, cli_name='allow_transfer', default=u'none;', multivalue=False, required=False)
diff --git a/VERSION b/VERSION
index 9e14c8cf46b8d39f955be952ce62173f4d9d453c..77340e02e91c91b45e5431810aac2a5c9d6237b6 100644
--- a/VERSION
+++ b/VERSION
@@ -79,4 +79,4 @@ IPA_DATA_VERSION=2010061412
 #  #
 
 IPA_API_VERSION_MAJOR=2
-IPA_API_VERSION_MINOR=37
+IPA_API_VERSION_MINOR=38
diff --git a/ipalib/plugins/dns.py b/ipalib/plugins/dns.py
index 1bf75427245e7435364ad5695e35426f5fd67be8..fb2810f10eeaca8cc688b535d9cf29e2929ab7d9 100644
--- a/ipalib/plugins/dns.py
+++ b/ipalib/plugins/dns.py
@@ -32,7 +32,8 @@ from ipalib.parameters import Flag, Bool, Int, Decimal, Str, StrEnum, Any
 from ipalib.plugins.baseldap import *
 from ipalib import _, ngettext
 from ipalib.util import (validate_zonemgr, normalize_zonemgr,
-validate_hostname, validate_dns_label, validate_domain_name)
+validate_hostname, validate_dns_label, validate_domain_name,
+get_dns_forward_zone_update_policy, get_dns_reverse_zone_update_policy)
 from ipapython.ipautil import valid_ip, CheckedIPAddress, is_host_resolvable
 from ldap import explode_dn
 
@@ -75,8 +76,7 @@ EXAMPLES:
--admin-email=ad...@example.com
 
  Modify the zone to allow dynamic updates for hosts own records in realm EXAMPLE.COM:
-   ipa dnszone-mod example.com --dynamic-update=TRUE \\
---update-policy="grant EXAMPLE.COM krb5-self * A; grant EXAMPLE.COM krb5-self * ;"
+   ipa dnszone-mod example.com --dynamic-update=TRUE
 
  Modify the zone to allow zone transfers for local network only:
ipa dnszone-mod example.com --allow-transfer=10.0.0.0/8
@@ -1510,6 +1510,12 @@ def dns_container_exists(ldap):
 return False
 return True
 
+def default_zone_update_policy(zone):
+if zone_is_reverse(zone):
+return get_dns_reverse_zone_update_policy(api.env.realm, zone)
+else:
+return get_dns_forward_zone_update_policy(api.env.realm)
+
 class dnszone(LDAPObject):
 """
 DNS Zone, container for resource records.
@@ -1611,6 +1617,8 @@ class dnszone(LDAPObject):
 cli_name='update_policy',
 label=_('BIND update policy'),
 doc=_('BIND update policy'),
+default_from=lambda idnsname: default_zone_update_policy(idnsname),
+autofill=True
 ),
 Bool('i

Re: [Freeipa-devel] [PATCH] 0057 Skip the fix_replica_memberof update plugin for non-root users

2012-06-04 Thread Simo Sorce
On Mon, 2012-06-04 at 17:22 +0200, Petr Viktorin wrote:
> An update plugin needed root privileges, and aborted the update if an 
> ordinary user user ran it.
> With this patch the plugin is skipped with a warning in that case.
> 
> https://fedorahosted.org/freeipa/ticket/2621

Hi Petr,
I am not sure I like the proposed solution.

If there is a legitimate reason to run this plugin as non-root (eg admin
user) then you should change the connection part to try to use GSSAPI
auth over ldap when non-root, not just throw a warning.

If there is no reason for anyone but root to run this script then we
should just abort if not root IMO.

Simo.

-- 
Simo Sorce * Red Hat, Inc * New York

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


[Freeipa-devel] [PATCH] 0057 Skip the fix_replica_memberof update plugin for non-root users

2012-06-04 Thread Petr Viktorin
An update plugin needed root privileges, and aborted the update if an 
ordinary user user ran it.

With this patch the plugin is skipped with a warning in that case.

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

--
PetrĀ³
From c525b9e90055ba01fee0a9402512c150cc2ced9d Mon Sep 17 00:00:00 2001
From: Petr Viktorin 
Date: Wed, 30 May 2012 08:08:24 -0400
Subject: [PATCH] Skip the fix_replica_memberof updater plugin for non-root
 users

The plugin does a SASL EXTERNAL bind, for which it needs root privileges.
Skip the plugin with a warning if run as a non-root user.

https://fedorahosted.org/freeipa/ticket/2621
---
 ipaserver/install/plugins/fix_replica_memberof.py |4 
 1 file changed, 4 insertions(+)

diff --git a/ipaserver/install/plugins/fix_replica_memberof.py b/ipaserver/install/plugins/fix_replica_memberof.py
index 04152d36021f7d962b335a7553861a13ba03a769..8dd3ed8b406e70cce55e7c338cdc0c5cdcfb4866 100644
--- a/ipaserver/install/plugins/fix_replica_memberof.py
+++ b/ipaserver/install/plugins/fix_replica_memberof.py
@@ -39,6 +39,10 @@ def execute(self, **options):
  'krbloginfailedcount')
 excludes = ('memberof', ) + totalexcludes
 
+if os.geteuid() != 0:
+self.log.warning("Updating replica memberof needs root privileges")
+return False, False, []  # No restart, no apply now, no updates
+
 # We need an IPAdmin connection to the backend
 conn = ipaldap.IPAdmin(api.env.host, ldapi=True, realm=api.env.realm)
 conn.do_external_bind(pwd.getpwuid(os.geteuid()).pw_name)
-- 
1.7.10.2

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

[Freeipa-devel] [PATCH] 0056 Framework for admin/install tools, with ipa-ldap-updater

2012-06-04 Thread Petr Viktorin

Currently, FreeIPA's install/admin scripts are long pieces of code
that aren't very reusable, importable, or testable.
They have been extended over time with features such as logging and
error handling, but since each tool was extended individually, there
is much inconsistency and code duplication.
This patch starts a framework which the admin tools can use, and
converts ipa-ldap-updater to use the framework.

In an earlier patch I found that improving a particular functionality in 
all the commands is not workable, so I want to tackle this one tool at a 
time.
I'm starting with ipa-ldap-updater, because it's pretty small, doesn't 
use DNs (I don't want conflicts with John's work), and has the 
interesting --upgrade option.



The framework does these tasks:
- Parse options
- Select tool to run (see below)
- Validate options
- Set up logging
- Run the tool code
- Handle any errors
- Log success/failure

The base class has some defaults for these that the tools can 
extend/override.



To handle the case where one script does two different things 
(ipa-ldap-updater with/without --upgrade, or ipa-server-install 
with/without --uninstall), I want to split the tool in two classes 
rather than have repeated ifs in the code.
This meant that option parsing (and initializing the parser) has to be 
done before creating an instance of the tool. I use a factory classmethod.



I put the admintool base class in ipapython/ as it should be useful for 
ipa-client-install as well.




First part of the work for:
https://fedorahosted.org/freeipa/ticket/2652

--
PetrĀ³
From d714df0d6c82081d50369471c6620b810bd15224 Mon Sep 17 00:00:00 2001
From: Petr Viktorin 
Date: Fri, 20 Apr 2012 04:39:59 -0400
Subject: [PATCH] Framework for admin/install tools, with ipa-ldap-updater

Currently, FreeIPA's install/admin scripts are long pieces of code
that aren't very reusable, importable, or testable.
They have been extended over time with features such as logging and
error handling, but since each tool was extended individually, there
is much inconsistency and code duplication.
This patch starts a framework which the admin tools can use, and
converts ipa-ldap-updater to use the framework.

Common tasks the tools do -- option parsing, validation, logging
setup, error handling -- are represented as methods. Individual
tools can extend, override or reuse the defaults as they see fit.

The ipa-ldap-updater has two modes (normal and --upgrade) that
don't share much functionality. They are represented by separate
classes. Option parsing, and selecting which class to run, happens
before they're instantiated.

All code is moved to importable modules to aid future testing. The
only thing that remains in the ipa-ldap-updater script is a two-line
call to the library.

First part of the work for:
https://fedorahosted.org/freeipa/ticket/2652
---
 install/tools/ipa-ldap-updater|  194 -
 ipapython/admintool.py|  221 +
 ipaserver/install/installutils.py |   89 ++---
 ipaserver/install/ipa_ldap_updater.py |  173 ++
 4 files changed, 457 insertions(+), 220 deletions(-)
 rewrite install/tools/ipa-ldap-updater (85%)
 create mode 100644 ipapython/admintool.py
 create mode 100644 ipaserver/install/ipa_ldap_updater.py

diff --git a/install/tools/ipa-ldap-updater b/install/tools/ipa-ldap-updater
dissimilarity index 85%
index bd2233a94241c28375b29cc10d60908238b8f176..0fc5a5bc448d04eb9ce8562ee1feebbf8f0fe356 100755
--- a/install/tools/ipa-ldap-updater
+++ b/install/tools/ipa-ldap-updater
@@ -1,169 +1,25 @@
-#!/usr/bin/python
-# Authors: Rob Crittenden 
-#
-# Copyright (C) 2008  Red Hat
-# see file 'COPYING' for use and warranty information
-#
-# This program is free software; you can redistribute it and/or modify
-# it under the terms of the GNU General Public License as published by
-# the Free Software Foundation, either version 3 of the License, or
-# (at your option) any later version.
-#
-# This program is distributed in the hope that it will be useful,
-# but WITHOUT ANY WARRANTY; without even the implied warranty of
-# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
-# GNU General Public License for more details.
-#
-# You should have received a copy of the GNU General Public License
-# along with this program.  If not, see .
-#
-
-# Documentation can be found at http://freeipa.org/page/LdapUpdate
-
-# TODO
-# save undo files?
-
-import os
-import sys
-try:
-from ipapython.config import IPAOptionParser
-from ipapython import ipautil, config
-from ipaserver.install import installutils
-from ipaserver.install.ldapupdate import LDAPUpdate, BadSyntax, UPDATES_DIR
-from ipaserver.install.upgradeinstance import IPAUpgrade
-from ipapython import sysrestore
-import krbV
-from ipalib import api
-from ipapython.ipa_log_manager import *
-except ImportError:
-print >>

[Freeipa-devel] [PATCH] 270 Improve migration NotFound error

2012-06-04 Thread Martin Kosek
When no user/group was found, migration plugin reported an ambiguous
error about invalid container. But the root cause may be for example
in a wrong list of user/group objectclasses. Report both in the error
message to avoid user confusion.

User/group objectclass attribute is now also marked as required.
Without the list of objectclasses, an invalid LDAP search is
produced.

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

>From 802450d88f7104cae6922a8a548ba69821d526a4 Mon Sep 17 00:00:00 2001
From: Martin Kosek 
Date: Mon, 4 Jun 2012 14:25:41 +0200
Subject: [PATCH] Improve migration NotFound error

When no user/group was found, migration plugin reported an ambiguous
error about invalid container. But the root cause may be for example
in a wrong list of user/group objectclasses. Report both in the error
message to avoid user confusion.

User/group objectclass attribute is now also marked as required.
Without the list of objectclasses, an invalid LDAP search is
produced.

https://fedorahosted.org/freeipa/ticket/2206
---
 API.txt |4 ++--
 VERSION |2 +-
 ipalib/plugins/migration.py |   17 -
 3 files changed, 15 insertions(+), 8 deletions(-)

diff --git a/API.txt b/API.txt
index ba5aa1037e5d9b8661326afe4e6f984d52cc3cc8..ba368bd6c478d3690c3b36bf559b6975e86d6e4a 100644
--- a/API.txt
+++ b/API.txt
@@ -1909,8 +1909,8 @@ arg: Password('bindpw', cli_name='password', confirm=False)
 option: Str('binddn?', autofill=True, cli_name='bind_dn', default=u'cn=directory manager')
 option: Str('usercontainer', autofill=True, cli_name='user_container', default=u'ou=people')
 option: Str('groupcontainer', autofill=True, cli_name='group_container', default=u'ou=groups')
-option: Str('userobjectclass*', autofill=True, cli_name='user_objectclass', csv=True, default=(u'person',))
-option: Str('groupobjectclass*', autofill=True, cli_name='group_objectclass', csv=True, default=(u'groupOfUniqueNames', u'groupOfNames'))
+option: Str('userobjectclass+', autofill=True, cli_name='user_objectclass', csv=True, default=(u'person',))
+option: Str('groupobjectclass+', autofill=True, cli_name='group_objectclass', csv=True, default=(u'groupOfUniqueNames', u'groupOfNames'))
 option: Str('userignoreobjectclass*', autofill=True, cli_name='user_ignore_objectclass', csv=True, default=())
 option: Str('userignoreattribute*', autofill=True, cli_name='user_ignore_attribute', csv=True, default=())
 option: Str('groupignoreobjectclass*', autofill=True, cli_name='group_ignore_objectclass', csv=True, default=())
diff --git a/VERSION b/VERSION
index 9e14c8cf46b8d39f955be952ce62173f4d9d453c..77340e02e91c91b45e5431810aac2a5c9d6237b6 100644
--- a/VERSION
+++ b/VERSION
@@ -79,4 +79,4 @@ IPA_DATA_VERSION=2010061412
 #  #
 
 IPA_API_VERSION_MAJOR=2
-IPA_API_VERSION_MINOR=37
+IPA_API_VERSION_MINOR=38
diff --git a/ipalib/plugins/migration.py b/ipalib/plugins/migration.py
index a7b0789756cc12cf976a83579fbc6bbc80cf0623..d2231c246392059f8704677426537658c701a59e 100644
--- a/ipalib/plugins/migration.py
+++ b/ipalib/plugins/migration.py
@@ -444,7 +444,7 @@ class migrate_ds(Command):
 default=u'ou=groups',
 autofill=True,
 ),
-Str('userobjectclass*',
+Str('userobjectclass+',
 cli_name='user_objectclass',
 label=_('User object class'),
 doc=_('Comma-separated list of objectclasses used to search for user entries in DS'),
@@ -452,7 +452,7 @@ class migrate_ds(Command):
 default=(u'person',),
 autofill=True,
 ),
-Str('groupobjectclass*',
+Str('groupobjectclass+',
 cli_name='group_objectclass',
 label=_('Group object class'),
 doc=_('Comma-separated list of objectclasses used to search for group entries in DS'),
@@ -619,8 +619,10 @@ can use their Kerberos accounts.''')
 for ldap_obj_name in self.migrate_order:
 ldap_obj = self.api.Object[ldap_obj_name]
 
-search_filter = construct_filter(self.migrate_objects[ldap_obj_name]['filter_template'],
- options[to_cli(self.migrate_objects[ldap_obj_name]['oc_option'])])
+template = self.migrate_objects[ldap_obj_name]['filter_template']
+oc_list = options[to_cli(self.migrate_objects[ldap_obj_name]['oc_option'])]
+search_filter = construct_filter(template, oc_list)
+
 exclude = options['exclude_%ss' % to_cli(ldap_obj_name)]
 context = dict(ds_ldap = ds_ldap)
 
@@ -637,7 +639,12 @@ can use their Kerberos accounts.''')
 except errors.NotFound:
 if not options.get('continue',False):
 raise errors.NotFound(
-reason=_('Container for %(container)s not found at %(search_base)s') % {'container': ldap_obj_na

Re: [Freeipa-devel] [PATCH] 0042-0048 AD trusts support (master)

2012-06-04 Thread Alexander Bokovoy

On Mon, 04 Jun 2012, Martin Kosek wrote:

I did another round of testing and this is what I found so far:

1) freeipa.spec.in was missing python-crypto BuildRequires (you fixed
that)

2) Unit tests need to be updated, currently there is about a dozen test
case errors, e.g. extra ipakrbprincipalalias attribute in services or
new ipakrbprincipal objectclass for hosts

Ok, will fix.


3) Replication did not work too well for me this time.
ipa-replica-install reported just one issue during installation process:

2012-06-04T09:42:51Z DEBUG   [24/30]: enabling S4U2Proxy delegation
2012-06-04T09:42:51Z DEBUG args=/usr/bin/ldapmodify -h
vm-057.idm.lab.bos.redhat.com -v -f /tmp/   tmpifHccf -x -D
cn=Directory Manager -y /tmp/tmppqaAdV
2012-06-04T09:42:51Z DEBUG stdout=
2012-06-04T09:42:51Z DEBUG
stderr=ldap_initialize( ldap://vm-057.idm.lab.bos.redhat.com )
ldapmodify: wrong attributeType at line 5, entry
"cn=ipa-http-delegation,cn=s4u2proxy,cn=etc,dc=idm,
dc=lab,dc=bos,dc=redhat,dc=com"

2012-06-04T09:42:51Z CRITICAL Failed to load replica-s4u2proxy.ldif:
Command '/usr/bin/ldapmodify -h   vm-057.idm.lab.bos.redhat.com -v
-f /tmp/tmpifHccf -x -D cn=Directory Manager -y /tmp/tmppqaAdV'
returned non-zero exit status 247

Found and fixed. The issue was in not following RFC2849 when specifying
multiple changetype operations, you need to split their definitions by a
single line with '-' on it.

I squashed the fix back to the original patch.


But this may be just a symptom of some bigger issue. After the
installation finished, DS did not start, it kept reporting Kerberos
issues:

[04/Jun/2012:05:46:00 -0400] set_krb5_creds - Could not get initial
credentials for principal
[ldap/vm-057.idm.lab.bos.redhat@idm.lab.bos.redhat.com] in keytab
[FILE:/etc/dirsrv/ds.keytab]: -1765328324 (Generic error (see e-text))
[04/Jun/2012:05:46:00 -0400] - slapd started.  Listening on All
Interfaces port 389 for LDAP requests
[04/Jun/2012:05:46:00 -0400] - Listening on All Interfaces port 636 for
LDAPS requests
[04/Jun/2012:05:46:00 -0400] - Listening
on /var/run/slapd-IDM-LAB-BOS-REDHAT-COM.socket for LDAPI requests
[04/Jun/2012:05:46:00 -0400] slapd_ldap_sasl_interactive_bind - Error:
could not perform interactive bind for id [] mech [GSSAPI]: LDAP error
-2 (Local error) (SASL(-1): generic failure: GSSAPI Error: Unspecified
GSS failure.  Minor code may provide more information (Credentials cache
file '/tmp/krb5cc_498' not found)) errno 0 (Success)
[04/Jun/2012:05:46:00 -0400] slapi_ldap_bind - Error: could not perform
interactive bind for id [] mech [GSSAPI]: error -2 (Local error)
[04/Jun/2012:05:46:00 -0400] NSMMReplicationPlugin -
agmt="cn=meTovm-125.idm.lab.bos.redhat.com" (vm-125:389): Replication
bind with GSSAPI auth failed: LDAP error -2 (Local error) (SASL(-1):
generic failure: GSSAPI Error: Unspecified GSS failure.  Minor code may
provide more information (Credentials cache file '/tmp/krb5cc_498' not
found))

When I run "ipactl restart", dirsrv started and I was able to kinit.

Maybe it is timing issue?



4) Patch "Add separate attribute to store trusted domain SID" still has
a wrong service part of the principal to be removed (s/ldap/cifs):

+dn3 = DN(u'cn=ipa-cifs-delegation-targets',
api.env.container_s4u2proxy, self.suffix)
+member_principal3 = "ldap/%(fqdn)s@%(realm)s" %
dict(fqdn=replica, realm=realm)
+

This leaves CIFS entry in the S4U2Proxy configuration even after replica
uninstallation.

Fixed and squashed back to the original patch.


Btw. these are the packages I use:
389-ds-base-1.2.10.4-2.fc17.x86_64
krb5-server-1.10-5.fc17.x86_64
samba4-4.0.0-123alpha21.fc17.x86_64

Same here. For me anything newer 1.2.10.4-2 will blow 389-ds.

--
/ Alexander Bokovoy

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


Re: [Freeipa-devel] [PATCH] 0042-0048 AD trusts support (master)

2012-06-04 Thread Martin Kosek
On Thu, 2012-04-12 at 17:16 +0200, Martin Kosek wrote:
> On Thu, 2012-04-12 at 18:08 +0300, Alexander Bokovoy wrote:
> > Hi Martin!
> > 
> > On Thu, 12 Apr 2012, Martin Kosek wrote:
> ...
> > >3) I would not try to import ipaserver.dcerpc every time the command is
> > >executed:
> > >+try:
> > >+import ipaserver.dcerpc
> > >+except Exception, e:
> > >+raise errors.NotFound(name=_('AD Trust setup'),
> > >+  reason=_('Cannot perform join operation without Samba
> > >4 python bindings installed'))
> > >
> > >I would rather do it once in the beginning and set a flag:
> > >
> > >try:
> > >import ipaserver.dcerpc
> > > _bindings_installed = True
> > >except Exception:
> > >_bindings_installed = False
> > >
> > >...
> > The idea was that this code is only executed on the server. We need to
> > differentiate between:
> > - running on client
> > - running on server, no samba4 python bindings
> > - running on server with samba4 python bindings
> > 
> > By making it executed all time you are affecting the client code as
> > well while with current approach it only affects server side.
> 
> Across our code base, this situation is currently solved with this
> condition:
> 
> if api.env.in_server and api.env.context in ['lite', 'server']:
> # try-import block
> 
> > 
> > 
> > >+def execute(self, *keys, **options):
> > >+# Join domain using full credentials and with random trustdom
> > >+# secret (will be generated by the join method)
> > >+trustinstance = None
> > >+if not _bindings_installed:
> > >+raise errors.NotFound(name=_('AD Trust setup'),
> > >+  reason=_('Cannot perform join operation without Samba
> > >4 python bindings installed'))
> > >
> > >
> > >4) Another import inside a function:
> > >+def arcfour_encrypt(key, data):
> > >+from Crypto.Cipher import ARC4
> > >+c = ARC4.new(key)
> > >+return c.encrypt(data)
> > Same here, it is only needed on server side.
> > 
> > Let us get consensus over 3) and 4) and I'll fix patches altogether (and
> > push).
> > 
> 
> Yeah, I would fix in the same way as 3).
> 
> Martin
> 

I did another round of testing and this is what I found so far:

1) freeipa.spec.in was missing python-crypto BuildRequires (you fixed
that)

2) Unit tests need to be updated, currently there is about a dozen test
case errors, e.g. extra ipakrbprincipalalias attribute in services or
new ipakrbprincipal objectclass for hosts

3) Replication did not work too well for me this time.
ipa-replica-install reported just one issue during installation process:

2012-06-04T09:42:51Z DEBUG   [24/30]: enabling S4U2Proxy delegation
2012-06-04T09:42:51Z DEBUG args=/usr/bin/ldapmodify -h
vm-057.idm.lab.bos.redhat.com -v -f /tmp/   tmpifHccf -x -D
cn=Directory Manager -y /tmp/tmppqaAdV
2012-06-04T09:42:51Z DEBUG stdout=
2012-06-04T09:42:51Z DEBUG
stderr=ldap_initialize( ldap://vm-057.idm.lab.bos.redhat.com )
ldapmodify: wrong attributeType at line 5, entry
"cn=ipa-http-delegation,cn=s4u2proxy,cn=etc,dc=idm,
dc=lab,dc=bos,dc=redhat,dc=com"

2012-06-04T09:42:51Z CRITICAL Failed to load replica-s4u2proxy.ldif:
Command '/usr/bin/ldapmodify -h   vm-057.idm.lab.bos.redhat.com -v
-f /tmp/tmpifHccf -x -D cn=Directory Manager -y /tmp/tmppqaAdV'
returned non-zero exit status 247


But this may be just a symptom of some bigger issue. After the
installation finished, DS did not start, it kept reporting Kerberos
issues:

[04/Jun/2012:05:46:00 -0400] set_krb5_creds - Could not get initial
credentials for principal
[ldap/vm-057.idm.lab.bos.redhat@idm.lab.bos.redhat.com] in keytab
[FILE:/etc/dirsrv/ds.keytab]: -1765328324 (Generic error (see e-text))
[04/Jun/2012:05:46:00 -0400] - slapd started.  Listening on All
Interfaces port 389 for LDAP requests
[04/Jun/2012:05:46:00 -0400] - Listening on All Interfaces port 636 for
LDAPS requests
[04/Jun/2012:05:46:00 -0400] - Listening
on /var/run/slapd-IDM-LAB-BOS-REDHAT-COM.socket for LDAPI requests
[04/Jun/2012:05:46:00 -0400] slapd_ldap_sasl_interactive_bind - Error:
could not perform interactive bind for id [] mech [GSSAPI]: LDAP error
-2 (Local error) (SASL(-1): generic failure: GSSAPI Error: Unspecified
GSS failure.  Minor code may provide more information (Credentials cache
file '/tmp/krb5cc_498' not found)) errno 0 (Success)
[04/Jun/2012:05:46:00 -0400] slapi_ldap_bind - Error: could not perform
interactive bind for id [] mech [GSSAPI]: error -2 (Local error)
[04/Jun/2012:05:46:00 -0400] NSMMReplicationPlugin -
agmt="cn=meTovm-125.idm.lab.bos.redhat.com" (vm-125:389): Replication
bind with GSSAPI auth failed: LDAP error -2 (Local error) (SASL(-1):
generic failure: GSSAPI Error: Unspecified GSS failure.  Minor code may
provide more information (Credentials cache file '/tmp/krb5cc_498' not
found))

When I run "ipactl restart", dirsrv started and I was able to kinit.

4) Patch "Add separate attribute

Re: [Freeipa-devel] [PATCH] 147 Set network.http.sendRefererHeader to 2 on browser config

2012-06-04 Thread Petr Vobornik

On 05/29/2012 11:29 PM, Rob Crittenden wrote:

Petr Vobornik wrote:

IPA web UI isn't functional when browser doesn't send http headers.

This patch adds a functionality which sets Firefox
network.http.sendRefererHeader configuration option to value '2' which
enables it.

Possible values: http://kb.mozillazine.org/Network.http.sendRefererHeader

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


Should we also add a message when referer is missing to check this
setting in about:config?


I'm not sure what you have in mind. We set the referer option so why 
would user check it afterwards?


Yes the ticket was about checking the option but: If user is configuring 
the browser he wants the browser configured. So we should set all 
options which are required. This is one of them. We have not been 
notifying the user what was set, so I didn't add such notification for 
this option now as well.


We might want to notify the user what options were changed but it's not 
the topic of this ticket.






I was also thinking about upgrading the configure.jar. We had a ticket
for it, which ended by documenting the steps.

https://fedorahosted.org/freeipa/ticket/2311
http://docs.fedoraproject.org/en-US/Fedora/16/html/FreeIPA_Guide/upgrading.html#ticket-delegation



I think the documentation is wrong. In it we are rebuilding the .jar
from /usr/share/ipa/html/preferences.html, this file is created on
server install and it is never updated therefore the .jar won't be
updated. The updated file is its template (the one changed in this
patch). The template output is created in
httpinstance.__setup_autoconfig() call.

For my development purposes I took this code and created a script which
rebuilds the .jar file (attached). Do we want to use it?


Yes, I think it is worth having this somewhere, even if just on the wiki.

rob



--
Petr Vobornik

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


Re: [Freeipa-devel] [PATCH] 146 Added cancel button to service unprovision dialog

2012-06-04 Thread Petr Vobornik

On 06/01/2012 02:40 AM, Endi Sukma Dewata wrote:

On 5/24/2012 4:11 AM, Petr Vobornik wrote:

Service unprovision dialog was missing a cancel button. The button was
added.

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


ACK.


Pushed to master.

--
Petr Vobornik

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


Re: [Freeipa-devel] [PATCH] 148 Removal of illegal options in JSON-RPC calls

2012-06-04 Thread Petr Vobornik

On 06/01/2012 02:44 AM, Endi Sukma Dewata wrote:

On 5/25/2012 9:57 AM, Petr Vobornik wrote:

Ticket https://fedorahosted.org/freeipa/ticket/2509 bans using non
existent options. If such option is supplied command ends with error. It
uncovered several cases in Web UI. This patch is fixing these cases.

Automember, Self-service and Delegation don't support 'pkey-only',
'size-limit' and 'rights' option. Pagination and rights check were
disabled for them.

Automount map adder dialog was sending options for indirect map even if
chosen type was direct (when those for indirect was filled earlier),
also it was sending non-existant 'method' option.

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

Note for reviewing: #2509 is partially done in Petr Viktorin's patch
#35. At this time it has a small issue regarding
automountmap_add_indirect command.


ACK. I suppose if those options are added later the UI can be updated
easily.


Pushed to master.

--
Petr Vobornik

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


Re: [Freeipa-devel] [PATCH] 149 Added links to netgroup member tables

2012-06-04 Thread Petr Vobornik

On 06/01/2012 02:44 AM, Endi Sukma Dewata wrote:

On 5/25/2012 11:23 AM, Petr Vobornik wrote:

Tables with members in netgroup were missing links for navigation to
associated details pages. This patch adds these links.

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


ACK.


Pushed to master.

--
Petr Vobornik

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


Re: [Freeipa-devel] [PATCH] 150 Text widget's dirty state is changed on various input methods

2012-06-04 Thread Petr Vobornik

On 06/01/2012 02:46 AM, Endi Sukma Dewata wrote:

On 5/28/2012 6:44 AM, Petr Vobornik wrote:

on_value_changed event in textboxes and textareas was raised only on
keyboard input. If user used different input method such as paste or
browser undo and redo functions widget's on_value_changed event wasn't
raised and so dirty state wasn't changed as well.

This patch adds listener to text's and textarea's 'input' event. Input
is a HTML 5 event which is raises on user initiated action.
Some of user initiated actions :
* Cut
* Copy
* Paste
* Undo
* Redo
* Clear
* Typing (like keyup)
* Form AutoFill
* User-invoked spellcheck corrections
* Input from Input Method Editor

It should be supported by all recent versions of major browsers. IE
doesn't support it up to version 8.

Listener for 'keyup' event was left in implementation for backward
compatibility with older browsers. This may cause firing on_value_change
twice but so far it shouldn't cause troubles.


Yeah, if it becomes a problem later you might need to check the browser
version and only listen to one of the events.

ACK.


Pushed to master.

--
Petr Vobornik

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


Re: [Freeipa-devel] [PATCHES] 138-145 Action panel for user password reset

2012-06-04 Thread Petr Vobornik

On 06/01/2012 02:40 AM, Endi Sukma Dewata wrote:

ACK. Looks good.


Pushed to master.



Some comments:

1. I suppose the select_action will always be the first action in any
header_actions, and the action doesn't actually do anything. You might
want to consider the '-- select action --' as part of the
IPA.action_list_widget and add it automatically in init_options(), that
way it doesn't have to be defined explicitly in all header_actions.


Can be. If I do it, I will do it configurable with default to use the 
'-- select action --'.




2. Ideally the Enable/Disable/Delete actions should only be enabled if
the user has rights to do that, but this depends on ticket #2187.


Yes. Also, I think enable and disable don't need #2187.



3. I noticed that the second argument of observer's notify() is always
the object that owns the observer. For example:

that.observer = IPA.observer();
that.observer.notify([arg], that);

Since the context doesn't change would it make sense to store the
context in the observer itself?

that.observer = IPA.observer({ context: that });

The arguments can also be passed as varargs:

that.observer.notify(arg);
that.observer.notify(arg1, arg2);


I like this idea. It simplifies things.




On 5/24/2012 3:54 AM, Petr Vobornik wrote:

This bunch of patches implements new concept: action panel and it's
implementation in user page.

First two patches refactorizes current action-list/control-buttons code
to prepare ground for following patches. Sorry for added review work
(could be done this way earlier).

Patch descriptions:

[PATCH] 138 Refactored action list and control buttons to use shared
list of actions

This is a first step for implementing action panels which will also use
the shared list of actions.

This effort changes the way how action list and control buttons are
defined. First all actions are defined on facet level - attribute
'actions' in spec file. Implementation of action list widget is not
specified on facet level. It is left in facet header. A list of action
names used in action list can be now specified in facet spec in
'header_actions' attribute.
Control buttons use similar concept. Facet by default is using
control_buttons_widget. Details and search facet are defining their own
default actions (refresh/add/remove/update/reset). Additional buttons
can be defined as array of action names on facet level in
control_buttons attribute.

state_evaluators and state_listeners were united. They are called
state_evaluators but they uses state_listener concept, they are attached
to an event. For former state_evaluator the event is post_load. They are
defined in spec in state attribute. State object purpose is to aggregate
states from all state evaluators. It offers changed event to which can
other objects subscribe. It also has summary evaluator which evaluation
conditions. Summary evaluator creates summary status with human readable
description. It can be used by facet header.

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


[PATCH] 139 Refactored entities to use changed actions concept

It's continuation of previous refactoring effort. This part is changing
specs in entities to used changed concept.

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

[PATCH] 140 Action panel

This patch implements action panel. Action panel is a box located in
facet details section which contains actions related to that
object/section.

In spec file can be configured actions and title used in action panel.
Default title is 'Actions'. Actions are specified by their name. They
have to be defined in action collection in facet.

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


[PATCH] 141 User password widget modified.

Currently the user password is shown as follows in the details page:
Password: Reset Password

This is inconsistent with the rest of the page because the 'Reset
Password' is an action, not the value of the password.

Now password is shown as follows:
Password: *** (if set)
Password: (if not set)

Reset password link was removed as well the dialog for reset password
was removed from password widget. The dialog was moved to its own object
and can be now showed independently. An action for showing this dialog
should be created.

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


[PATCH] 142 Action panel for user

This patch adds action panel to user account section. The panel contain
an action for reseting user password.

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

[PATCH] 143 Added missing i18n in action list and action panel

This patch adds strings to internal.py which were not translated in
action list/panel patches.

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


[PATCH] 144 Add shadow to dialog

This patch adds shadow to dialog used in Web UI. It looks cooler.

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

note: I didn't want to create new ticket just for this minor visual
enhancement.


[PATCH] 145 Enable reset password action according to attribute
perrmission

This patch creates