Re: [Freeipa-devel] dns.py question

2012-05-25 Thread Martin Kosek 
On Thu, 2012-05-24 at 12:07 -0400, John Dennis wrote:
 In dns_is_enabled it computes the base_dn but never uses it.
 Instead it uses self.base_dn, but as far as I can figure out there is no 
 self.base_dn in the class hierarchy (if so could you point me at it). 
 The access to self.base_dn is wrapped in a try/except block that doesn't 
 handle any errors so if in fact there was no self.base_dn that error 
 would be silently ignored.
 
 So is self.base_dn a typo and we're supposed to be using the local 
 base_dn that's computed but never used?
 
 Code snipped attached so it won't be mangled by line wrapping.
 

Hello John,

base_dn is defined in dns_is_enabled command class along with filter:

class dns_is_enabled(Command):
...
base_dn = 'cn=masters,cn=ipa,cn=etc,%s' % api.env.basedn
filter = '((objectClass=ipaConfigObject)(cn=DNS))'
...

def execute(self, *args, **options):
...
try:
ent = ldap.find_entries(filter=self.filter,
base_dn=self.base_dn)
if len(ent):
dns_enabled = True
except Exception, e:
pass

So self.base_dn and self.filter return correct value. Otherwise the
function would never return True as in my case:

 from ipalib import api
 api.bootstrap(context='cli_installer')
 api.finalize()
 api.Backend.xmlclient.connect()
ipa: INFO: trying https://vm-143.idm.lab.bos.redhat.com/ipa/xml
 api.Command['dns_is_enabled']()
ipa: INFO: Forwarding 'dns_is_enabled' to server
'http://vm-143.idm.lab.bos.redhat.com/ipa/xml'
{'result': True, 'value': u'', 'summary': None}

HTH,
Martin

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

Re: [Freeipa-devel] [PATCH] 0050 Fail on unknown Command options

2012-05-25 Thread Petr Vobornik 

On 05/16/2012 02:11 PM, Martin Kosek wrote:

On Wed, 2012-05-16 at 10:37 +0200, Petr Viktorin wrote:

On 05/16/2012 09:58 AM, Martin Kosek wrote:

On Tue, 2012-05-15 at 13:35 +0200, Petr Viktorin wrote:

On 05/15/2012 09:55 AM, Martin Kosek wrote:

On Mon, 2012-05-14 at 14:47 +0200, Petr Viktorin wrote:

The final part of rejecting unknown Command arguments: enable the
validation, add tests.
Also fix up things that were changed since the previous patches.

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



The patch looks OK so far. I just found an error in permission/aci
plugin - --subtree does not work when it matches a result:

# ipa permission-find --subtree=foo
-
0 permissions matched
-

Number of entries returned 0


ipa permission-find
--subtree='ldap:///ipauniqueid=*,cn=hbac,dc=idm,dc=lab,dc=bos,dc=redhat,dc=Com'
ipa: ERROR: Unknown option: subtree


Attaching fixed patch.


We should not pass **options to aci_show, it is too risky. There may be
other places where we don't use an option-safe approach that we want to
have fixed.


We shouldn't really pass **options to any command; listing everything
explicitly would be much safer. Unfortunately, in a lot of cases where
commands call other commands, it's currently done this way.




Martin



Attaching a rebased patch.



Yup, this one is fine. Now, I did not find issues in the patch itself,
tests are clean.

However, thanks to this new check I found issues in Web UI (automember,
selfservice, delegation screen) which use illegal options and which
should be fixed before we push your patch:

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

Martin



I found an issue in automountmap_add_indirect. It complains that 'key' 
is unknown option.

--
Petr Vobornik

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

Re: [Freeipa-devel] [PATCH] 0050 Fail on unknown Command options

2012-05-25 Thread Martin Kosek 
On Fri, 2012-05-25 at 09:20 +0200, Petr Vobornik wrote:
 On 05/16/2012 02:11 PM, Martin Kosek wrote:
  On Wed, 2012-05-16 at 10:37 +0200, Petr Viktorin wrote:
  On 05/16/2012 09:58 AM, Martin Kosek wrote:
  On Tue, 2012-05-15 at 13:35 +0200, Petr Viktorin wrote:
  On 05/15/2012 09:55 AM, Martin Kosek wrote:
  On Mon, 2012-05-14 at 14:47 +0200, Petr Viktorin wrote:
  The final part of rejecting unknown Command arguments: enable the
  validation, add tests.
  Also fix up things that were changed since the previous patches.
 
  https://fedorahosted.org/freeipa/ticket/2509
 
 
  The patch looks OK so far. I just found an error in permission/aci
  plugin - --subtree does not work when it matches a result:
 
  # ipa permission-find --subtree=foo
  -
  0 permissions matched
  -
  
  Number of entries returned 0
  
 
  ipa permission-find
  --subtree='ldap:///ipauniqueid=*,cn=hbac,dc=idm,dc=lab,dc=bos,dc=redhat,dc=Com'
  ipa: ERROR: Unknown option: subtree
 
  Attaching fixed patch.
 
  We should not pass **options to aci_show, it is too risky. There may be
  other places where we don't use an option-safe approach that we want to
  have fixed.
 
  We shouldn't really pass **options to any command; listing everything
  explicitly would be much safer. Unfortunately, in a lot of cases where
  commands call other commands, it's currently done this way.
 
 
 
  Martin
 
 
  Attaching a rebased patch.
 
 
  Yup, this one is fine. Now, I did not find issues in the patch itself,
  tests are clean.
 
  However, thanks to this new check I found issues in Web UI (automember,
  selfservice, delegation screen) which use illegal options and which
  should be fixed before we push your patch:
 
  https://fedorahosted.org/freeipa/ticket/2760
 
  Martin
 
 
 I found an issue in automountmap_add_indirect. It complains that 'key' 
 is unknown option.

I assume this is a cause and would need to be fixed in Petr3's patch:

 847 # Add a submount key
 848 self.api.Command['automountkey_add'](
 849 location, parentmap, automountkey=key, key=key,
 850 automountinformation='-fstype=autofs ldap:%s' %
map)

Martin

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

Re: [Freeipa-devel] [PATCH] 1022 normalize uid when in winsync

2012-05-25 Thread Martin Kosek 
On Thu, 2012-05-24 at 13:54 -0400, Rob Crittenden wrote:
 In case the uid that comes from AD is mixed-case we need to normalize it 
 to all lower. It should be safe using tolower() because we only allow 
 ASCII characters in uid.
 
 rob

I tested this with a winsync agreement with an AD and it worked fine, I
as able to change password and then log in as an AD-mixed-case user.

ACK, pushed to master.

Martin

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

Re: [Freeipa-devel] DNS zone serial number updates [#2554]: local SOA approach

2012-05-25 Thread Adam Tkac 
On Mon, May 07, 2012 at 10:29:12PM +0200, Petr Spacek wrote:
 Hello,
 
 on the last meeting there was another approach to $SUBJ$ discussed:
 
 Each DNS server will maintain its own serial number value
 independently from other servers.
 
 Pros:
 Should be simpler to implement; no DS plugin required.
 
 Cons:
 Slave DNS servers cannot fall-back to other masters, because of SOA
 serial inconsistency.
 
 
 Very basic implementation:
 1) Do not replicate idnsSoaSerial attribute
 2) Use persistent search to watch for incoming changes
 3) After each change increment local SOA serial number (and write
 it to LDAP - to survive DNS server restart)
 
 There is a problem with database updates if bind-dyndb-ldap plugin
 is not running, but its DS runs. In that case DS can replicate
 changes from other servers but there is no bind-dyndb-ldap plugin to
 modify serial number.
 
 I think it can happen during startup/shutdown phase or after BIND
 crash. In this case zone content can be changed without appropriate
 SOA serial update.
 
 Dumb solution:
 Always increment stored SOA serial number after DNS server start. It
 causes unnecessary zone transfer, but we are safe.
 
 Another solution (IPA+389 specific):
 Remember max(entryUSN) value computed from all records together with
 SOA serial. (Principle is the same as with modifyTimestamp described
 earlier in this thread.) It requires new LDAP object with two
 attributes:
 - assigned value of idnsSOASerial
 - highest entryUSN found
 DNS server after start can check if max(entryUSN) == recorded max
 value. If values do not match plugin bumps idnsSOASerial.
 
 If entryUSN is not supported by server, we can fall-back to bumping
 idnsSOASerial on every start-up.
 
 Did I miss anything? :-)
 
 It requires persistent search, but I resigned already.

After further discussion this seems like the best approach for me as well.

Regards, Adam

-- 
Adam Tkac, Red Hat, Inc.

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

Re: [Freeipa-devel] dns.py question

2012-05-25 Thread John Dennis 

On 05/25/2012 02:41 AM, Martin Kosek wrote:

base_dn is defined in dns_is_enabled command class along with filter:


Thanks Martin! I missed the fact it was defined in class scope, not 
method scope.  Duh on my part :-( I thought I was nuts, now I know for 
sure :-)


--
John Dennis jden...@redhat.com

Looking to carve out IT costs?
www.redhat.com/carveoutcosts/

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

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

2012-05-25 Thread Martin Kosek 
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
From 5a469d6fe8a69e49a8cbd5f844d201c2b0187cc8 Mon Sep 17 00:00:00 2001
From: Martin Kosek mko...@redhat.com
Date: Fri, 25 May 2012 14:11:25 +0200
Subject: [PATCH 1/4] Add sysupgrade state file

When IPA package is being updated, some of the configuration files
are also updated. Sometimes it may be useful to store upgrade meta
information for next package upgrades. For example an information
that some config file was already updated and we don't want to
update it again if user purposedly reverted the change.

This patch adds a new StateFile in /var/lib/ipa/sysupgrade which
is capable of holding this information. New sysupgrade.py module
was created to provide simple API to access the upgrade state
information.
---
 freeipa.spec.in  |4 +++
 install/Makefile.am  |3 ++
 install/tools/ipa-server-install |4 +++
 ipapython/sysrestore.py  |   45 +--
 ipaserver/install/sysupgrade.py  |   47 ++
 5 files changed, 90 insertions(+), 13 deletions(-)
 create mode 100644 ipaserver/install/sysupgrade.py

diff --git a/freeipa.spec.in b/freeipa.spec.in
index de93aecb6142f73528e6aefe89d6bfcb48fc036f..b62ee5803fb3aae0bd6cd77b4833b7b5a361a639 100644
--- a/freeipa.spec.in
+++ b/freeipa.spec.in
@@ -606,6 +606,7 @@ fi
 %attr(755,root,root) %{plugin_dir}/libipa_cldap.so
 %dir %{_localstatedir}/lib/ipa
 %attr(700,root,root) %dir %{_localstatedir}/lib/ipa/sysrestore
+%attr(700,root,root) %dir %{_localstatedir}/lib/ipa/sysupgrade
 %dir %{_localstatedir}/cache/ipa
 %attr(700,apache,apache) %dir %{_localstatedir}/cache/ipa/sessions
 %attr(755,root,root) %{_libdir}/krb5/plugins/kdb/ipadb.so
@@ -684,6 +685,9 @@ fi
 %ghost %attr(0644,root,apache) %config(noreplace) %{_sysconfdir}/ipa/ca.crt
 
 %changelog
+* Fri May 25 2012 Martin Kosek mko...@redhat.com - 2.99.0-30
+- Add directory /var/lib/ipa/sysupgrade for package upgrade metadata
+
 * Fri May 11 2012 Martin Kosek mko...@redhat.com - 2.99.0-29
 - Replace used DNS client library (acutil) with python-dns
 
diff --git a/install/Makefile.am b/install/Makefile.am
index 4d24d072dbda5817691ac21388edf196ee942c5d..5670f9bd9e29182adb6aebe92eaf72368bfb01cf 100644
--- a/install/Makefile.am
+++ b/install/Makefile.am
@@ -19,11 +19,14 @@ SUBDIRS =			\
 install-exec-local:
 	mkdir -p $(DESTDIR)$(localstatedir)/lib/ipa/sysrestore
 	chmod 700 $(DESTDIR)$(localstatedir)/lib/ipa/sysrestore
+	mkdir -p $(DESTDIR)$(localstatedir)/lib/ipa/sysupgrade
+	chmod 700 $(DESTDIR)$(localstatedir)/lib/ipa/sysupgrade
 	mkdir -p $(DESTDIR)$(localstatedir)/cache/ipa/sessions
 	chmod 700 $(DESTDIR)$(localstatedir)/cache/ipa/sessions
 
 uninstall-local:
 	-rmdir $(DESTDIR)$(localstatedir)/lib/ipa/sysrestore
+	-rmdir $(DESTDIR)$(localstatedir)/lib/ipa/sysupgrade
 	-rmdir $(DESTDIR)$(localstatedir)/lib/ipa
 	-rmdir $(DESTDIR)$(localstatedir)/cache/ipa/sessions
 	-rmdir $(DESTDIR)$(localstatedir)/cache/ipa
diff --git a/install/tools/ipa-server-install b/install/tools/ipa-server-install
index 2f06a9e879902eb1c2ac340757fcd1762959fe30..70ab127832486829f7b743f817a587ef43c70fdf 100755
--- a/install/tools/ipa-server-install
+++ b/install/tools/ipa-server-install
@@ -49,6 +49,7 @@ from ipaserver.install import ntpinstance
 from ipaserver.install import certs
 from ipaserver.install import cainstance
 from ipaserver.install import memcacheinstance
+from ipaserver.install import sysupgrade
 
 from ipaserver.install import service
 from ipapython import version
@@ -496,6 +497,9 @@ def uninstall():
 except CalledProcessError, e:
 print sys.stderr, Failed to set this machine hostname back to %s (%s). % (old_hostname, str(e))
 
+# remove upgrade state file
+sysupgrade.remove_upgrade_file()
+
 if fstore.has_files():
 root_logger.error('Some files have not been restored, see /var/lib/ipa/sysrestore/sysrestore.index')
 has_state = False
diff --git a/ipapython/sysrestore.py b/ipapython/sysrestore.py
index 82817acadce84af0371734db40ef27e74f575a0d..7720fd6e3da80cbf19bbadb62e4431d45d4fa482 100644
--- a/ipapython/sysrestore.py
+++ b/ipapython/sysrestore.py
@@ -41,7 +41,7 @@ SYSRESTORE_STATEFILE = sysrestore.state
 class FileStore:
 Class for handling backup and restore of files
 
-def __init__(self, path = SYSRESTORE_PATH):
+def __init__(self, path = SYSRESTORE_PATH, index_file =

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

2012-05-25 Thread Rob Crittenden 

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

263:

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


264:

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

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.


rob

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

Re: [Freeipa-devel] routing requests to local servers - DNS SRV [discussion needed]

2012-05-25 Thread Petr Spacek 

On 05/24/2012 08:00 PM, Dmitri Pal wrote:

On 05/24/2012 01:07 PM, Petr Spacek wrote:

Hello,

some time ago there was a request for DNS to support routing requests
to local servers. Any opinions if/when do it and proposals how to do
it are more than welcome. My best knowledge about this problem follows:


This request actually means differentiate answer to DNS query on
client's IP address basics.
Relevant thread on ipa-users-list:
https://www.redhat.com/archives/freeipa-users/2012-April/msg00070.html

First question is: Do we want to implement it?
(IMHO it is very important for large-scale deployments.)


It is related to IPA ticket #122 at architecture level:
IPA DNS integration should support GeoIP
https://fedorahosted.org/freeipa/ticket/122
IMHO it is better to support generic locations defined by user than
real Geo-IP:
- at least private IP addresses will be problematic
- real Geo-IP database is huge and can slow all things down


Bind-dyndb-ldap plugin supports BIND views already. It should be
possible to define multiple plugin instances with different ldap
search base and let it to serve records.

The main problem problem is managing DNS subtree in LDAP. With current
implementation you have to copy whole subtree to another place and
then change necessary records. Problem is with maintaining base
(non-overridden) records - you have to do same change n-times.


Adam and I discussed possible approaches. We agreed on stackable
approach:
- Store whole original DNS tree in one subtree, let say base.
- Create override subtrees for each locality = set of customized
records. Shared records are only in base. During DNS query
processing override subtree is searched first. If record does not
exist in override subtree, search will continue in base subtree.


Second question is how to realize these overrides:
- Let Directory Server to do the work. If DS supports this, it is
mostly done.
It do not require any change in bind-dyndb-ldap code. All
merges/overrides will be done on Directory server. Only
/etc/named.conf has to be adjusted with right search base and view
clause.

I asked on 389 mailing list and I'm waiting for reply:
http://lists.fedoraproject.org/pipermail/389-users/2012-May/014653.html


- Another approach is to add support directly to bind-dyndb-ldap, but
it is not so nice solution. In that case both LDAP search bases have
to be defined in /etc/named.conf. For each DNS query is necessary to
search override base first. If required record is not present then
is necessary to search through base subtree.
With persistent search it should be better, because all records are in
memory, but basic principle remains same.


Thanks for all opinions.



Petr^2 Spacek


May be I am oversimplifying things but it seems that it would make sense
to just have an extra multi-value attribute added to the DNS schema.
This attribute would contain a value that would allow the entry to be
included into the view. This way the base is the same and what the view
exposes is just a filter. The default view would have a filter that
matches all entries like now.


I assume that DNS server makes it decision based on the IP so it has a
call to get a view data. The ldap driver can return a filter. The DNS
server them makes a call using this filter to get all the records that
apply. I know it is too ldap centric. There is some abstraction in DNS
server and we do not want to change it. But the point is there are
probably two calls in the DNS server (have not actually confirmed):
lookup data X by IP to understand what the view is.
Pass data X to get the actual DNS entries.

I am saying that X can be filter.

Thoughts?


Special attribute sounds like a good idea. It is not realizable directly, but 
I will explore variants with some view attribute.


Current DNS name (name can potentially contain multiple records) structure 
is following:


dn: idnsname=_kerberos._tcp,idnsname=e.localnet,cn=dns,dc=e,dc=org
objectClass: idnsrecord
objectClass: top
idnsName: _kerberos._tcp
sRVRecord: 0 100 88 unused-4-107

DNS name is part of DN. It is not possible to have more objects with same DNS 
name and different attributes. This problem lead me to stackable approach.


I'm looking into Views in DS now.

Petr^2 Spacek

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

Re: [Freeipa-devel] routing requests to local servers - DNS SRV [discussion needed]

2012-05-25 Thread Simo Sorce 
On Thu, 2012-05-24 at 19:07 +0200, Petr Spacek wrote:
 Hello,
 
 some time ago there was a request for DNS to support routing requests to 
 local servers. Any opinions if/when do it and proposals how to do it are 
 more 
 than welcome. My best knowledge about this problem follows:
 
 
 This request actually means differentiate answer to DNS query on client's IP 
 address basics.
 Relevant thread on ipa-users-list:
 https://www.redhat.com/archives/freeipa-users/2012-April/msg00070.html
 
 First question is: Do we want to implement it?
 (IMHO it is very important for large-scale deployments.)

I am not really sure I like it, as it makes things quite difficult to
handle.

Please think how we are going to operate if you ahve a view dfined and a
client does a dyn DNS update.

 It is related to IPA ticket #122 at architecture level:
 IPA DNS integration should support GeoIP
 https://fedorahosted.org/freeipa/ticket/122
 IMHO it is better to support generic locations defined by user than real 
 Geo-IP:
 - at least private IP addresses will be problematic
 - real Geo-IP database is huge and can slow all things down

Geo-IP is good for some services, mostly internet facing high
availability stuff. For internal networks we should just use the
location code, as Geo-IP tagging makes no sense for private networks.
I say we should defer any work on Geo-IP tagging as we do not target
public facing high avilability IP serving with our DNS at the moemnt (Ie
I expect those people will delegate a zone to a dedicated set of DNS
servers instead).

Ah incidentally this means we need to start supporting zone delegation
from the management UI if we do not already support that.

 Bind-dyndb-ldap plugin supports BIND views already. It should be possible 
 to 
 define multiple plugin instances with different ldap search base and let it 
 to 
 serve records.

 The main problem problem is managing DNS subtree in LDAP. With current 
 implementation you have to copy whole subtree to another place and then 
 change 
 necessary records. Problem is with maintaining base (non-overridden) 
 records 
 - you have to do same change n-times.

Using different bases would require replicating the whole database
indeed with common entries you do not change per view. That's a lot of
effort, and very easy to screw up.
I think we really do not want to take this approach.

 Adam and I discussed possible approaches. We agreed on stackable approach:
 - Store whole original DNS tree in one subtree, let say base.
 - Create override subtrees for each locality = set of customized records. 
 Shared records are only in base. During DNS query processing override 
 subtree is searched first. If record does not exist in override subtree, 
 search will continue in base subtree.

Yes, this is my first thought too.

 Second question is how to realize these overrides:
 - Let Directory Server to do the work. If DS supports this, it is mostly done.

Why do you need dirsrv to do anything special ?
The simple way is to do subtree searches, if you get back more than one
result for a specific name then you know you have views and proceed to
filter out the one you want. The bonus here is that you can cache all
replies if you keep different caches per view.

The alternative is to add a 'viewname' or something in the filter, but
then you need to do 2 searches and fallbacks. This sounds more
expensive.

 It do not require any change in bind-dyndb-ldap code. All merges/overrides 
 will be done on Directory server.

Given we do persistent searches and we also do some caching in
bind-dyndb-ldap we almost certainly do not want to 'fool' it by
returning different values from DS w/o bind-dyndb-ldap knowledge.

  Only /etc/named.conf has to be adjusted with 
 right search base and view clause.
 
 I asked on 389 mailing list and I'm waiting for reply:
 http://lists.fedoraproject.org/pipermail/389-users/2012-May/014653.html
 
 
 - Another approach is to add support directly to bind-dyndb-ldap, but it is 
 not so nice solution.

Why ?

  In that case both LDAP search bases have to be defined 
 in /etc/named.conf. For each DNS query is necessary to search override base 
 first. If required record is not present then is necessary to search through 
 base subtree.

No, you do not need to do multiple searches, you just need to filter the
results by view.

It would make it very easy to manage if we added a 'dnsView' attribute
to overriding entries in the subtree. This would allow us to define a
new overriding entry and even assign it to multiple views if the
'dnsView' attribute is multivalued.

 With persistent search it should be better, because all records are in 
 memory, 
 but basic principle remains same.

psearch is one of the reasons we might want a dnsView attribute base
approach instead of playing games with DS.
I would also prefer to avoid adding more code to DS where the bind
plugin can easily handle this data itself. Another reason is that I
really want this plugin code to be

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

2012-05-25 Thread Petr Vobornik 
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.


--
Petr Vobornik
From aaa2066e0885b6c856edbf6ee65c6e9ebeeece2d Mon Sep 17 00:00:00 2001
From: Petr Vobornik pvobo...@redhat.com
Date: Fri, 25 May 2012 08:57:47 +0200
Subject: [PATCH] Removal of illegal options in JSON-RPC calls

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
---
 install/ui/aci.js|8 ++--
 install/ui/automember.js |4 
 install/ui/automount.js  |8 
 install/ui/details.js|   15 ++-
 4 files changed, 28 insertions(+), 7 deletions(-)

diff --git a/install/ui/aci.js b/install/ui/aci.js
index 27d9589aae551f76957066e22e79e2ac475ae488..953116c3f1e6f8450c0f8b308f1f2851b704e203 100644
--- a/install/ui/aci.js
+++ b/install/ui/aci.js
@@ -343,9 +343,11 @@ IPA.aci.selfservice_entity = function(spec) {
 that.entity_init();
 
 that.builder.search_facet({
-columns: [ 'aciname' ]
+columns: [ 'aciname' ],
+pagination: false
 }).
 details_facet({
+check_rights: false,
 sections: [
 {
 name: 'general',
@@ -386,9 +388,11 @@ IPA.aci.delegation_entity = function(spec) {
 that.entity_init();
 
 that.builder.search_facet({
-columns: [ 'aciname' ]
+columns: [ 'aciname' ],
+pagination: false
 }).
 details_facet({
+check_rights: false,
 sections: [
 {
 name: 'general',
diff --git a/install/ui/automember.js b/install/ui/automember.js
index df285973d0e51f5610406e6866eb76ec1f8ae088..6f8eba956b6631011c3bf0bde63615e2e8a6a046 100644
--- a/install/ui/automember.js
+++ b/install/ui/automember.js
@@ -58,6 +58,7 @@ IPA.automember.entity = function(spec) {
 group_type: 'group',
 label: IPA.messages.objects.automember.usergrouprules,
 details_facet: 'usergrouprule',
+pagination: false,
 columns: [
 'cn',
 'description'
@@ -69,6 +70,7 @@ IPA.automember.entity = function(spec) {
 group_type: 'hostgroup',
 label: IPA.messages.objects.automember.hostgrouprules,
 details_facet: 'hostgrouprule',
+pagination: false,
 columns: [
 'cn',
 'description'
@@ -80,6 +82,7 @@ IPA.automember.entity = function(spec) {
 group_type: 'group',
 label: IPA.messages.objects.automember.usergrouprule,
 disable_facet_tabs: true,
+check_rights: false,
 redirect_info: { tab: 'amgroup' }
 }).
 details_facet({
@@ -88,6 +91,7 @@ IPA.automember.entity = function(spec) {
 group_type: 'hostgroup',
 label: IPA.messages.objects.automember.hostgrouprule,
 disable_facet_tabs: true,
+check_rights: false,
 redirect_info: { tab: 'amhostgroup' }
 }).
 adder_dialog({
diff --git a/install/ui/automount.js b/install/ui/automount.js
index 5a4d5978f6e5215a3d9d7e25ab983f4c4bc30f36..3a4491d8b3325324ff8cc53b0ad606fb37e636d8 100644
--- a/install/ui/automount.js
+++ b/install/ui/automount.js
@@ -112,6 +112,7 @@ IPA.automount.map_entity = function(spec) {
 {
 type: 'radio',
 name: 'method',
+enabled: false, //don't use value in add command
 label: IPA.messages.objects.automountmap.map_type,
 options: [
 {
@@ -285,11 +286,15 @@ IPA.automountmap_adder_dialog =

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

2012-05-25 Thread Martin Kosek 
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
From a69c109d095e5bc96f5b2047b4d7fec2ea2f3917 Mon Sep 17 00:00:00 2001
From: Martin Kosek mko...@redhat.com
Date: Fri, 25 May 2012 14:11:25 +0200
Subject: [PATCH 1/4] Add sysupgrade state file

When IPA package is being updated, some of the configuration files
are also updated. Sometimes it may be useful to store upgrade meta
information for next package upgrades. For example an information
that some config file was already updated and we don't want to
update it again if user purposedly reverted the change.

This patch adds a new StateFile in /var/lib/ipa/sysupgrade which
is capable of holding this information. New sysupgrade.py module
was created to provide simple API to access the upgrade state
information.
---
 freeipa.spec.in  |5 
 install/Makefile.am  |3 ++
 install/tools/ipa-server-install |4 +++
 ipapython/sysrestore.py  |   45 +--
 ipaserver/install/sysupgrade.py  |   47 ++
 5 files changed, 91 insertions(+), 13 deletions(-)
 create mode 100644 ipaserver/install/sysupgrade.py

diff --git a/freeipa.spec.in b/freeipa.spec.in
index de93aecb6142f73528e6aefe89d6bfcb48fc036f..1f0ea73cef5cb18be32eda42404e50182671ec48 100644
--- a/freeipa.spec.in
+++ b/freeipa.spec.in
@@ -343,6 +343,7 @@ rm %{buildroot}/%{_libdir}/samba/pdb/ipasam.la
 # and link back.
 mkdir -p %{buildroot}/%{_sysconfdir}/ipa/html
 mkdir -p %{buildroot}/%{_localstatedir}/cache/ipa/sysrestore
+mkdir -p %{buildroot}/%{_localstatedir}/cache/ipa/sysupgrade
 mkdir %{buildroot}%{_usr}/share/ipa/html/
 ln -s ../../../..%{_sysconfdir}/ipa/html/ssbrowser.html \
 %{buildroot}%{_usr}/share/ipa/html/ssbrowser.html
@@ -606,6 +607,7 @@ fi
 %attr(755,root,root) %{plugin_dir}/libipa_cldap.so
 %dir %{_localstatedir}/lib/ipa
 %attr(700,root,root) %dir %{_localstatedir}/lib/ipa/sysrestore
+%attr(700,root,root) %dir %{_localstatedir}/lib/ipa/sysupgrade
 %dir %{_localstatedir}/cache/ipa
 %attr(700,apache,apache) %dir %{_localstatedir}/cache/ipa/sessions
 %attr(755,root,root) %{_libdir}/krb5/plugins/kdb/ipadb.so
@@ -684,6 +686,9 @@ fi
 %ghost %attr(0644,root,apache) %config(noreplace) %{_sysconfdir}/ipa/ca.crt
 
 %changelog
+* Fri May 25 2012 Martin Kosek mko...@redhat.com - 2.99.0-30
+- Add directory /var/lib/ipa/sysupgrade for package upgrade metadata
+
 * Fri May 11 2012 Martin Kosek mko...@redhat.com - 2.99.0-29
 - Replace used DNS client library (acutil) with python-dns
 
diff --git a/install/Makefile.am b/install/Makefile.am
index 4d24d072dbda5817691ac21388edf196ee942c5d..5670f9bd9e29182adb6aebe92eaf72368bfb01cf 100644
--- a/install/Makefile.am
+++ b/install/Makefile.am
@@ -19,11 +19,14 @@ SUBDIRS =			\
 install-exec-local:
 	mkdir -p $(DESTDIR)$(localstatedir)/lib/ipa/sysrestore
 	chmod 700 $(DESTDIR)$(localstatedir)/lib/ipa/sysrestore
+	mkdir -p $(DESTDIR)$(localstatedir)/lib/ipa/sysupgrade
+	chmod 700 $(DESTDIR)$(localstatedir)/lib/ipa/sysupgrade
 	mkdir -p $(DESTDIR)$(localstatedir)/cache/ipa/sessions
 	chmod 700 $(DESTDIR)$(localstatedir)/cache/ipa/sessions
 
 uninstall-local:
 	-rmdir $(DESTDIR)$(localstatedir)/lib/ipa/sysrestore
+	-rmdir $(DESTDIR)$(localstatedir)/lib/ipa/sysupgrade
 	-rmdir

[Freeipa-devel] [PATCH] 0054 Provide a better error message when deleting nonexistent attributes

2012-05-25 Thread Petr Viktorin 
This fixes misleading/invalid error messages given when using 
--delattr to delete values from an attribute that doesn't exist on the 
entry.

Please see the trac comment for details.

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


--
Petr³


From 09725d75cff2910f984cd571dc9565f58a072c8a Mon Sep 17 00:00:00 2001
From: Petr Viktorin pvikt...@redhat.com
Date: Fri, 11 May 2012 04:00:30 -0400
Subject: [PATCH] Provide a better error message when deleting nonexistent
 attributes

If --delattr is used on an attribute that's not present on an entry,
and --{set,add}attr isn't being used on that same attribute,
say that there's no such attribute instead of attribute does
not contain value.

https://fedorahosted.org/freeipa/ticket/2699
---
 ipalib/plugins/baseldap.py |   10 ++
 tests/test_xmlrpc/test_attr.py |   33 +
 2 files changed, 43 insertions(+)

diff --git a/ipalib/plugins/baseldap.py b/ipalib/plugins/baseldap.py
index 2851f0f270d9e2bdba4780cc7bf308a76e180fd2..6cfcbe67810d38a1f3085c3781273335814a429f 100644
--- a/ipalib/plugins/baseldap.py
+++ b/ipalib/plugins/baseldap.py
@@ -901,6 +901,16 @@ def process_attr_options(self, entry_attrs, dn, keys, options):
 )
 except errors.NotFound:
 self.obj.handle_not_found(*keys)
+
+# Provide a nice error message when user tries to delete an
+# attribute that does not exist on the entry (and user is not
+# adding it)
+names = set(n.lower() for n in old_entry)
+del_nonexisting = delattrs - (names | setattrs | addattrs)
+if del_nonexisting:
+raise errors.ValidationError(name=del_nonexisting.pop(),
+error=_('No such attribute on this entry'))
+
 for attr in needldapattrs:
 entry_attrs[attr] = old_entry.get(attr, [])
 
diff --git a/tests/test_xmlrpc/test_attr.py b/tests/test_xmlrpc/test_attr.py
index 5916ebd2d0f3b7bce8fc3160bce86d703145880d..2c94402804695b1e5ad8fe1389ceb0f220c75d67 100644
--- a/tests/test_xmlrpc/test_attr.py
+++ b/tests/test_xmlrpc/test_attr.py
@@ -517,4 +517,37 @@ class test_attr(Declarative):
 error='must be an integer'),
 ),
 
+dict(
+desc='Try deleting bogus attribute',
+command=('config_mod', [], dict(delattr=u'bogusattribute=xyz')),
+expected=errors.ValidationError(name='bogusattribute',
+error='No such attribute on this entry'),
+),
+
+dict(
+desc='Try deleting empty attribute',
+command=('config_mod', [],
+dict(delattr=u'ipaCustomFields=See Also,seealso,false')),
+expected=errors.ValidationError(name='ipacustomfields',
+error='No such attribute on this entry'),
+),
+
+dict(
+desc='Set and delete one value, plus try deleting a missing one',
+command=('config_mod', [], dict(
+delattr=[u'ipaCustomFields=See Also,seealso,false',
+u'ipaCustomFields=Country,c,false'],
+addattr=u'ipaCustomFields=See Also,seealso,false')),
+expected=errors.AttrValueNotFound(attr='ipacustomfields',
+value='Country,c,false'),
+),
+
+dict(
+desc='Try to delete an operational attribute with --delattr',
+command=('config_mod', [], dict(
+delattr=u'creatorsName=cn=directory manager')),
+expected=errors.DatabaseError(
+desc='Server is unwilling to perform', info=''),
+),
+
 ]
-- 
1.7.10.2

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

[Freeipa-devel] [PATCH] 0055 Add more automount tests

2012-05-25 Thread Petr Viktorin 
Martin and Petr Voborník found an automount plugin bug in my patch 50. I 
checked the automount test coverage and found that it isn't that great – 
not only automount-key-add-indirect with the parentmap flag, but the 
import and tofiles commands weren't tested at all.


This patch makes the situation a bit better.


--
Petr³
From c7b466b0481eac481c61100c91213a5df257e6c8 Mon Sep 17 00:00:00 2001
From: Petr Viktorin pvikt...@redhat.com
Date: Fri, 25 May 2012 10:53:23 -0400
Subject: [PATCH] Add more automount tests

This adds tests for the automountlocation_tofiles and
automountlocation_import commands, and to automountmap_add_indirect
with the --parentmap option.

The tofiles test checks not only the XML-RPC output, but also the
output_for_cli method.

The import tests load data from tofiles output to the directory
and check that tofiles output matches.
This only works when all maps are connected to auto.master.

Two minor touches to the automount plugin itself: remove an extra
space, and don't hide the traceback when re-raising an exception.
---
 ipalib/plugins/automount.py|4 +-
 tests/test_xmlrpc/test_automount_plugin.py |  227 +++-
 2 files changed, 223 insertions(+), 8 deletions(-)

diff --git a/ipalib/plugins/automount.py b/ipalib/plugins/automount.py
index d3451314cd8740fab5e0dc0d64189a844e082813..5c9f42b4c9812971e4fa03d4700b20ec70a0f81a 100644
--- a/ipalib/plugins/automount.py
+++ b/ipalib/plugins/automount.py
@@ -349,7 +349,7 @@ def output_for_cli(self, textui, result, *keys, **options):
 textui.print_plain(_('maps not connected to /etc/auto.master:'))
 for m in orphanmaps:
 textui.print_plain('---')
-textui.print_plain('/etc/%s: ' % m['automountmapname'])
+textui.print_plain('/etc/%s:' % m['automountmapname'])
 for k in orphankeys:
 if len(k) == 0: continue
 dn = DN(k[0]['dn'])
@@ -391,7 +391,7 @@ def __read_mapfile(self, filename):
 reason=_('File %(file)s not found') % {'file': filename}
 )
 else:
-raise e
+raise
 return map
 
 def forward(self, *args, **options):
diff --git a/tests/test_xmlrpc/test_automount_plugin.py b/tests/test_xmlrpc/test_automount_plugin.py
index 0dd65d8704cb7d2fbaa1e5127db4089ad243308e..d6c2899796edc983e7196e137a2853df377cd934 100644
--- a/tests/test_xmlrpc/test_automount_plugin.py
+++ b/tests/test_xmlrpc/test_automount_plugin.py
@@ -22,14 +22,90 @@
 
 
 import sys
-from nose.tools import raises, assert_raises  # pylint: disable=E0611
+import textwrap
+import tempfile
+import shutil
 
-from xmlrpc_test import XMLRPC_test, assert_attr_equal
 from ipalib import api
 from ipalib import errors
+from ipalib.dn import DN
 
+from nose.tools import raises, assert_raises  # pylint: disable=E0611
+from xmlrpc_test import XMLRPC_test, assert_attr_equal
+from tests.util import assert_deepequal
 
-class test_automount(XMLRPC_test):
+
+class MockTextui(list):
+Collects output lines
+# Extend the mock object if other textui methods are called
+def print_plain(self, line):
+self.append(unicode(line))
+
+
+class AutomountTest(XMLRPC_test):
+Provides common functionality for automount tests
+def check_tofiles(self):
+Check automountlocation_tofiles output against self.tofiles_output
+
+res = api.Command['automountlocation_tofiles'](self.locname)
+
+mock_ui = MockTextui()
+command = api.Command['automountlocation_tofiles']
+command.output_for_cli(mock_ui, res, self.locname)
+expected_output = self.tofiles_output
+assert_deepequal(expected_output, u'\n'.join(mock_ui))
+
+def check_import_roundtrip(self):
+Check automountlocation_tofiles/automountlocation_import roundtrip
+
+Loads self.tofiles_output (which should correspond to
+automountlocation_tofiles output), then checks the resulting map
+against tofiles_output again.
+Do not use this if the test creates maps that aren't connected to
+auto.master -- these can't be imported successfully.
+
+conf_directory = tempfile.mkdtemp()
+
+# Parse the tofiles_output into individual files, replace /etc/ by
+# our temporary directory name
+current_file = None
+for line in self.tofiles_output.splitlines():
+line = line.replace('/etc/',  '%s/' % conf_directory)
+if line.startswith(conf_directory) and line.endswith(':'):
+current_file = open(line.rstrip(':'), 'w')
+elif '' in line:
+current_file.close()
+elif line.startswith('maps not connected to '):
+break
+else:
+current_file.write(line + '\n')
+current_file.close()
+
+self.failsafe_add(api.Object.automountlocation,

Re: [Freeipa-devel] [PATCH] 0050 Fail on unknown Command options

2012-05-25 Thread Petr Viktorin 

On 05/25/2012 09:26 AM, Martin Kosek wrote:

On Fri, 2012-05-25 at 09:20 +0200, Petr Vobornik wrote:

On 05/16/2012 02:11 PM, Martin Kosek wrote:

On Wed, 2012-05-16 at 10:37 +0200, Petr Viktorin wrote:

On 05/16/2012 09:58 AM, Martin Kosek wrote:

On Tue, 2012-05-15 at 13:35 +0200, Petr Viktorin wrote:

On 05/15/2012 09:55 AM, Martin Kosek wrote:

On Mon, 2012-05-14 at 14:47 +0200, Petr Viktorin wrote:

The final part of rejecting unknown Command arguments: enable the
validation, add tests.
Also fix up things that were changed since the previous patches.

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



The patch looks OK so far. I just found an error in permission/aci
plugin - --subtree does not work when it matches a result:

# ipa permission-find --subtree=foo
-
0 permissions matched
-

Number of entries returned 0


 ipa permission-find
--subtree='ldap:///ipauniqueid=*,cn=hbac,dc=idm,dc=lab,dc=bos,dc=redhat,dc=Com'
ipa: ERROR: Unknown option: subtree


Attaching fixed patch.


We should not pass **options to aci_show, it is too risky. There may be
other places where we don't use an option-safe approach that we want to
have fixed.


We shouldn't really pass **options to any command; listing everything
explicitly would be much safer. Unfortunately, in a lot of cases where
commands call other commands, it's currently done this way.




Martin



Attaching a rebased patch.



Yup, this one is fine. Now, I did not find issues in the patch itself,
tests are clean.

However, thanks to this new check I found issues in Web UI (automember,
selfservice, delegation screen) which use illegal options and which
should be fixed before we push your patch:

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

Martin



I found an issue in automountmap_add_indirect. It complains that 'key'
is unknown option.


I assume this is a cause and would need to be fixed in Petr3's patch:

  847 # Add a submount key
  848 self.api.Command['automountkey_add'](
  849 location, parentmap, automountkey=key, key=key,
  850 automountinformation='-fstype=autofs ldap:%s' %
map)

Martin



Thanks!
Fixed and rebased. I'll test this code path in a separate patch.

--
Petr³
From b0019124b7b4641660dc095a43d0add37da92fa8 Mon Sep 17 00:00:00 2001
From: Petr Viktorin pvikt...@redhat.com
Date: Tue, 17 Apr 2012 12:42:35 -0400
Subject: [PATCH] Fail on unknown Command options

When unknown keyword arguments are passed to a Command, raise an
error instead of ignoring them.

Options used when IPA calls its commands internally are listed
in a new Command attribute called internal_options, and allowed.

Previous patches (0b01751c, c45174d6, c5689e7f) made IPA not use
unknown keyword arguments in its own commands and tests, but since
that some violations were reintroduced in permission_find and tests.
Fix those.

Tests included; both a frontend unittest and a XML-RPC test via the
ping plugin (which was untested previously).

https://fedorahosted.org/freeipa/ticket/2509
---
 ipalib/frontend.py  |   13 +--
 ipalib/plugins/aci.py   |2 ++
 ipalib/plugins/automount.py |6 +++-
 ipalib/plugins/permission.py|   46 +++-
 tests/test_cmdline/test_cli.py  |6 ++--
 tests/test_ipalib/test_frontend.py  |5 +++
 tests/test_xmlrpc/test_host_plugin.py   |2 +-
 tests/test_xmlrpc/test_netgroup_plugin.py   |6 ++--
 tests/test_xmlrpc/test_permission_plugin.py |   12 +++
 tests/test_xmlrpc/test_ping_plugin.py   |   52 +++
 10 files changed, 122 insertions(+), 28 deletions(-)
 create mode 100644 tests/test_xmlrpc/test_ping_plugin.py

diff --git a/ipalib/frontend.py b/ipalib/frontend.py
index e8a84eabe0ccb981eabcc6b5d5e89f80c4135fe8..7c63b27ddb41e751692e47ebc334cb699606a74e 100644
--- a/ipalib/frontend.py
+++ b/ipalib/frontend.py
@@ -29,7 +29,8 @@
 from output import Output, Entry, ListOfEntries
 from text import _, ngettext
 
-from errors import ZeroArgumentError, MaxArgumentError, OverlapError, RequiresRoot, VersionError, RequirementError
+from errors import (ZeroArgumentError, MaxArgumentError, OverlapError,
+RequiresRoot, VersionError, RequirementError, OptionError)
 from errors import InvocationError
 from constants import TYPE_ERROR
 from ipapython.version import API_VERSION
@@ -404,6 +405,8 @@ class Command(HasParam):
 output_params = Plugin.finalize_attr('output_params')
 has_output_params = tuple()
 
+internal_options = tuple()
+
 msg_summary = None
 msg_truncated = _('Results are truncated, try a more specific search')
 
@@ -520,7 +523,13 @@ def __args_2_params(self, values):
 def __options_2_params(self, options):
 for name in self.params:
 if name in options:
-yield

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

2012-05-25 Thread Petr Vobornik 

On 05/25/2012 04:57 PM, 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.



I found more used non existing options in association adder dialog code: 
in hbac hbacsvcgroup_find: no_hbacsvc. I will investigate these cases 
more deeply.


--
Petr Vobornik

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

Re: [Freeipa-devel] [PATCH] 79 SSH configuration fixes

2012-05-25 Thread Martin Kosek 
On Wed, 2012-05-23 at 11:16 +0200, Jan Cholasta wrote:
 Hi,
 
 this fixes https://fedorahosted.org/freeipa/ticket/2769 as well as some 
 other issues with SSH configuration in ipa-client-install.
 
 Honza
 

This fixed the basic functionality, but I discovered another issue
(quite serious one).

With /usr/bin/sss_ssh_knownhostsproxy as ssh ProxyCommand, I cannot
connect to remove client which does not have valid reverse record.
Without the proxy command, it works fine. I logged a Bugzilla for this
issue:
https://bugzilla.redhat.com/show_bug.cgi?id=825316

Martin

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

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

2012-05-25 Thread Petr Vobornik 
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
--
Petr Vobornik
From c89f67b1a457e99fa3ee0f2be89250222f5b1f52 Mon Sep 17 00:00:00 2001
From: Petr Vobornik pvobo...@redhat.com
Date: Fri, 25 May 2012 18:19:16 +0200
Subject: [PATCH] Added links to netgroup member tables

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
---
 install/ui/netgroup.js |   12 
 1 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/install/ui/netgroup.js b/install/ui/netgroup.js
index 05f9d0ddc9c6a35b3b63275d6137fd6bff74f3d3..f7dccbeee97bd11f787575dc9b82697bce73a12c 100644
--- a/install/ui/netgroup.js
+++ b/install/ui/netgroup.js
@@ -166,7 +166,8 @@ IPA.netgroup.details_facet = function(spec) {
 columns: [
 {
 name: 'memberuser_user',
-label: IPA.messages.objects.netgroup.users
+label: IPA.messages.objects.netgroup.users,
+link: true
 }
 ]
 },
@@ -181,7 +182,8 @@ IPA.netgroup.details_facet = function(spec) {
 columns: [
 {
 name: 'memberuser_group',
-label: IPA.messages.objects.netgroup.usergroups
+label: IPA.messages.objects.netgroup.usergroups,
+link: true
 }
 ]
 }
@@ -253,7 +255,8 @@ IPA.netgroup.details_facet = function(spec) {
 columns: [
 {
 name: 'memberhost_host',
-label: IPA.messages.objects.netgroup.hosts
+label: IPA.messages.objects.netgroup.hosts,
+link: true
 },
 {
 name: 'externalhost',
@@ -274,7 +277,8 @@ IPA.netgroup.details_facet = function(spec) {
 columns: [
 {
 name: 'memberhost_hostgroup',
-label: IPA.messages.objects.netgroup.hostgroups
+label: IPA.messages.objects.netgroup.hostgroups,
+link: true
 }
 ]
 }
-- 
1.7.7.6

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

Re: [Freeipa-devel] routing requests to local servers - DNS SRV [discussion needed]

2012-05-25 Thread Simo Sorce 
On Fri, 2012-05-25 at 15:52 +0200, Petr Spacek wrote:
 On 05/24/2012 08:00 PM, Dmitri Pal wrote:
  On 05/24/2012 01:07 PM, Petr Spacek wrote:
  Hello,
 
  some time ago there was a request for DNS to support routing requests
  to local servers. Any opinions if/when do it and proposals how to do
  it are more than welcome. My best knowledge about this problem follows:
 
 
  This request actually means differentiate answer to DNS query on
  client's IP address basics.
  Relevant thread on ipa-users-list:
  https://www.redhat.com/archives/freeipa-users/2012-April/msg00070.html
 
  First question is: Do we want to implement it?
  (IMHO it is very important for large-scale deployments.)
 
 
  It is related to IPA ticket #122 at architecture level:
  IPA DNS integration should support GeoIP
  https://fedorahosted.org/freeipa/ticket/122
  IMHO it is better to support generic locations defined by user than
  real Geo-IP:
  - at least private IP addresses will be problematic
  - real Geo-IP database is huge and can slow all things down
 
 
  Bind-dyndb-ldap plugin supports BIND views already. It should be
  possible to define multiple plugin instances with different ldap
  search base and let it to serve records.
 
  The main problem problem is managing DNS subtree in LDAP. With current
  implementation you have to copy whole subtree to another place and
  then change necessary records. Problem is with maintaining base
  (non-overridden) records - you have to do same change n-times.
 
 
  Adam and I discussed possible approaches. We agreed on stackable
  approach:
  - Store whole original DNS tree in one subtree, let say base.
  - Create override subtrees for each locality = set of customized
  records. Shared records are only in base. During DNS query
  processing override subtree is searched first. If record does not
  exist in override subtree, search will continue in base subtree.
 
 
  Second question is how to realize these overrides:
  - Let Directory Server to do the work. If DS supports this, it is
  mostly done.
  It do not require any change in bind-dyndb-ldap code. All
  merges/overrides will be done on Directory server. Only
  /etc/named.conf has to be adjusted with right search base and view
  clause.
 
  I asked on 389 mailing list and I'm waiting for reply:
  http://lists.fedoraproject.org/pipermail/389-users/2012-May/014653.html
 
 
  - Another approach is to add support directly to bind-dyndb-ldap, but
  it is not so nice solution. In that case both LDAP search bases have
  to be defined in /etc/named.conf. For each DNS query is necessary to
  search override base first. If required record is not present then
  is necessary to search through base subtree.
  With persistent search it should be better, because all records are in
  memory, but basic principle remains same.
 
 
  Thanks for all opinions.
  
  Petr^2 Spacek
 
  May be I am oversimplifying things but it seems that it would make sense
  to just have an extra multi-value attribute added to the DNS schema.
  This attribute would contain a value that would allow the entry to be
  included into the view. This way the base is the same and what the view
  exposes is just a filter. The default view would have a filter that
  matches all entries like now.
 
 
  I assume that DNS server makes it decision based on the IP so it has a
  call to get a view data. The ldap driver can return a filter. The DNS
  server them makes a call using this filter to get all the records that
  apply. I know it is too ldap centric. There is some abstraction in DNS
  server and we do not want to change it. But the point is there are
  probably two calls in the DNS server (have not actually confirmed):
  lookup data X by IP to understand what the view is.
  Pass data X to get the actual DNS entries.
 
  I am saying that X can be filter.
 
  Thoughts?
 
 Special attribute sounds like a good idea. It is not realizable directly, but 
 I will explore variants with some view attribute.
 
 Current DNS name (name can potentially contain multiple records) structure 
 is following:
 
 dn: idnsname=_kerberos._tcp,idnsname=e.localnet,cn=dns,dc=e,dc=org
 objectClass: idnsrecord
 objectClass: top
 idnsName: _kerberos._tcp
 sRVRecord: 0 100 88 unused-4-107
 
 DNS name is part of DN. It is not possible to have more objects with same DNS 
 name and different attributes. This problem lead me to stackable approach.

Yes, and we can also use multiple attributes in the same tree, although
for clarity I probably prefer the subtree approach.

So a few options:

1. all in the same subtree:
# Normal object
dn: idnsname=bar,idnsname=foo.org,cn=dns,dc=foo,dc=org
objectClass: idnsrecord
objectClass: top
idnsName: bar
aRecord: 192.168.12.34
dNSTTL: 1200

# Object belongin to the 'DMZ' view
dn:cn=DMZ-bar,idnsname=foo.org,cn=dns,dc=foo,dc=org
objectClass: idnsrecord
objectClass: top
objectClass: nsContainer
cn: DMZ-bar
idnsName: bar
aRecord: 5.6.7.8
dNSTTL: 3600
idnsView: DMZ


NOTE: I had to

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

2012-05-25 Thread Simo Sorce 
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.

-- 
Simo Sorce * Red Hat, Inc * New York
From 1bb57926c7156be7b731e4920ee202c3307e3fb6 Mon Sep 17 00:00:00 2001
From: Simo Sorce sso...@redhat.com
Date: Wed, 23 May 2012 12:35:44 -0400
Subject: [PATCH] Add support for disabling KDC writes

Add two global ipaConfig options to disable undesirable writes that have
performance impact.
The KDC:Disable Last Success will disable writing back to ldap the last
successful AS Request time (successful kinit)
The KDC:Disable Lockout will disable completely writing back lockout
related data. This means lockout policies will stop working.
---
 API.txt|2 +-
 daemons/ipa-kdb/ipa_kdb.c  |   66 
 daemons/ipa-kdb/ipa_kdb.h  |2 +
 daemons/ipa-kdb/ipa_kdb_audit_as.c |7 
 ipalib/plugins/config.py   |3 +-
 5 files changed, 78 insertions(+), 2 deletions(-)

diff --git a/API.txt b/API.txt
index 82a6bc65c1e1936f94f4c29b58a34972237f4eb3..6559e8133a0a4a7b1cc005608bd65436d033a10d 100644
--- a/API.txt
+++ b/API.txt
@@ -459,7 +459,7 @@ option: Bool('ipamigrationenabled', attribute=True, autofill=False, cli_name='en
 option: Str('ipagroupobjectclasses', attribute=True, autofill=False, cli_name='groupobjectclasses', csv=True, multivalue=True, required=False)
 option: Str('ipauserobjectclasses', attribute=True, autofill=False, cli_name='userobjectclasses', csv=True, multivalue=True, required=False)
 option: Int('ipapwdexpadvnotify', attribute=True, autofill=False, cli_name='pwdexpnotify', minvalue=0, multivalue=False, required=False)
-option: StrEnum('ipaconfigstring', attribute=True, autofill=False, cli_name='ipaconfigstring', csv=True, multivalue=True, required=False, values=(u'AllowLMhash', u'AllowNThash'))
+option: StrEnum('ipaconfigstring', attribute=True, autofill=False, cli_name='ipaconfigstring', csv=True, multivalue=True, required=False, values=(u'AllowLMhash', u'AllowNThash', u'KDC:Disable Last Success', u'KDC:Disable Lockout'))
 option: Str('ipaselinuxusermaporder', attribute=True, autofill=False, cli_name='ipaselinuxusermaporder', multivalue=False, required=False)
 option: Str('ipaselinuxusermapdefault', attribute=True, autofill=False, cli_name='ipaselinuxusermapdefault', multivalue=False, required=False)
 option: Str('setattr*', cli_name='setattr', exclude='webui')
diff --git a/daemons/ipa-kdb/ipa_kdb.c b/daemons/ipa-kdb/ipa_kdb.c
index ed87d6fef246ca4566ba87af40d2e41fd20f757f..3527cefa10df67d3f17c730ab4483410c736a44f 100644
--- a/daemons/ipa-kdb/ipa_kdb.c
+++ b/daemons/ipa-kdb/ipa_kdb.c
@@ -159,6 +159,65 @@ done:
 return base;
 }
 
+int ipadb_get_global_configs(struct ipadb_context *ipactx)
+{
+char *attrs[] = { ipaConfigString, NULL };
+struct berval **vals = NULL;
+LDAPMessage *res = NULL;
+LDAPMessage *first;
+char *base = NULL;
+int i;
+int ret;
+
+ret = asprintf(base, cn=ipaConfig,cn=etc,%s, ipactx-base);
+if (ret == -1) {
+ret = ENOMEM;
+goto done;
+}
+
+ret = ipadb_simple_search(ipactx, base, LDAP_SCOPE_BASE,
+  (objectclass=*), attrs,