Re: [Freeipa-devel] [PATCH] 233 Fix ipa-replica-manage TLS connection error

2012-03-14 Thread Martin Kosek
On Tue, 2012-03-13 at 16:08 -0400, Rob Crittenden wrote:
 Rich Megginson wrote:
  On 03/08/2012 05:33 AM, Martin Kosek wrote:
  New version of openldap (openldap-2.4.26-6.fc16.x86_64) changed its
  ABI and broke our TLS connection in ipa-replica-manage. This makes
  it impossible to connect for example to Active Directory to set up
  a winsync replication. We always receive a connection error stating
  that Peer's certificate is not recognized even though we pass
  a correct certificate.
 
  This patch fixes the way we set up TLS. The change is backwards
  compatible with older versions of openldap.
 
  https://fedorahosted.org/freeipa/ticket/2500
  ack
 
 ACK here too, works fine with old and new openldap.
 
 rob

I fixed a typo in a comment and pushed to master, ipa-2-2.

Martin

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


Re: [Freeipa-devel] [PATCH] 105 Fixed checkbox value in table without pkey

2012-03-14 Thread Petr Vobornik

On 03/09/2012 05:29 AM, Endi Sukma Dewata wrote:

On 3/8/2012 3:47 AM, Petr Vobornik wrote:

When a table is displaying a record set without entity's pkey attribute.
A checkbox value isn't properly prepared. This patch adds the
preparation (converts value to string).

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


ACK.


Pushed to master, ipa-2-2.

--
Petr Vobornik

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


Re: [Freeipa-devel] [PATCH] 104 Fixed mask validation in network_validator

2012-03-14 Thread Petr Vobornik

On 03/09/2012 05:29 AM, Endi Sukma Dewata wrote:

On 3/7/2012 10:15 AM, Petr Vobornik wrote:

Attaching patch file.

On 03/07/2012 05:10 PM, Petr Vobornik wrote:

Network validator allowed invalid mask format:
* leading zeros: 192.168.0.1/0024
* trailing chars: 192.168.0.1/24abcd

It was fixed.

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


ACK.


Pushed to master, ipa-2-2.

--
Petr Vobornik

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


[Freeipa-devel] [PATCH] 18 Typos in FreeIPA messages

2012-03-14 Thread Ondrej Hamada

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

Rebased patch sent by Yuri Chornoivan (yurc...@ukr.net). Fixes 'occured'
and 'commond' typos in FreeIPA messages.

Longtitude/Longitude typo was already corrected in patch for ticket 
#2382 https://fedorahosted.org/freeipa/ticket/2382.


--
Regards,

Ondrej Hamada
FreeIPA team
jabber: oh...@jabbim.cz
IRC: ohamada

From 8cdd8d2000167a1db924f3eb73d50555ffc32768 Mon Sep 17 00:00:00 2001
From: Ondrej Hamada oham...@redhat.com
Date: Wed, 14 Mar 2012 13:16:29 +0100
Subject: [PATCH] Typos in FreeIPA messages

Rebased patch sent by Yuri Chornoivan (yurc...@ukr.net). Fixes 'occured'
and 'commond' typos in FreeIPA messages.

https://fedorahosted.org/freeipa/ticket/2526
---
 install/ui/test/data/ipa_init.json |4 ++--
 ipalib/plugins/internal.py |2 +-
 ipalib/plugins/selinuxusermap.py   |2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/install/ui/test/data/ipa_init.json b/install/ui/test/data/ipa_init.json
index 0182aab733a5541d3149ea582bd975faf04db10a..1010cbfd68d99b81ebcf452f0a95bb8affc134cc 100644
--- a/install/ui/test/data/ipa_init.json
+++ b/install/ui/test/data/ipa_init.json
@@ -208,7 +208,7 @@
 ptr_redir_zone: Zone found: ${zone},
 ptr_redir_zone_err: Target reverse zone not found.,
 ptr_redir_zones: Fetching DNS zones.,
-ptr_redir_zones_err: An error occurd while fetching dns zones.,
+ptr_redir_zones_err: An error occurred while fetching dns zones.,
 redirection_dnszone: You will be redirected to DNS Zone.,
 standard: Standard Record Types,
 title: Records for DNS Zone,
@@ -608,4 +608,4 @@
 }
 ]
 }
-}
\ No newline at end of file
+}
diff --git a/ipalib/plugins/internal.py b/ipalib/plugins/internal.py
index deff866eee1c073f3f786686fd2e74f9261ba6b4..bad75aeac32f2f6409ba36fe5ffe83a53f8b2b72 100644
--- a/ipalib/plugins/internal.py
+++ b/ipalib/plugins/internal.py
@@ -345,7 +345,7 @@ class i18n_messages(Command):
 ptr_redir_zone: _(Zone found: ${zone}),
 ptr_redir_zone_err: _(Target reverse zone not found.),
 ptr_redir_zones: _(Fetching DNS zones.),
-ptr_redir_zones_err: _(An error occurd while fetching dns zones.),
+ptr_redir_zones_err: _(An error occurred while fetching dns zones.),
 redirection_dnszone: _(You will be redirected to DNS Zone.),
 standard: _(Standard Record Types),
 title: _(Records for DNS Zone),
diff --git a/ipalib/plugins/selinuxusermap.py b/ipalib/plugins/selinuxusermap.py
index ee9a8133f8bd8f164cab3337714a7dec8d3aa05f..e33e1016192d62312aa5f4f0dcdbafea23327216 100644
--- a/ipalib/plugins/selinuxusermap.py
+++ b/ipalib/plugins/selinuxusermap.py
@@ -65,7 +65,7 @@ EXAMPLES:
 SEEALSO:
 
  The list controlling the order in which the SELinux user map is applied
- and the default SELinux user are available in the config-show commond.
+ and the default SELinux user are available in the config-show command.
 )
 
 notboth_err = _('HBAC rule and local members cannot both be set')
-- 
1.7.6.5

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

Re: [Freeipa-devel] [PATCH] [WIP] Cross-realm trusts with AD

2012-03-14 Thread Alexander Bokovoy

On Tue, 13 Mar 2012, Simo Sorce wrote:

2. samba4 4.0.0-102alpha18 has one minor bug in systemd service
(https://fedorahosted.org/freeipa/ticket/2523), you'd need to add

ExecStartPre=/bin/mkdir -p /run/samba

before ExecStart= stanza to get it working with tmpfs-based /run in
Fedora 17.


This is wrong.
Please add a file in /etc/tmpfiles.d/samba.conf

Contents should be:
d /var/run/samba  644 root root

(adjust permission and ownership accordingly).

This file needs to be added to the samba4 package (and the samba3
package as well ?)

samba4-4.0.0-103alpha18 (in ipa-devel) now includes proper tmpfiles.d
file. With 755 permissions, obviously.

--
/ Alexander Bokovoy

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


Re: [Freeipa-devel] [PATCH] 924 display both hex and decimal serial numbers

2012-03-14 Thread Jan Cholasta

On 13.3.2012 22:57, Rob Crittenden wrote:

Jan Cholasta wrote:

On 7.3.2012 17:12, Rob Crittenden wrote:

Petr Vobornik wrote:

On 03/06/2012 09:56 PM, Rob Crittenden wrote:

Rob Crittenden wrote:

Jan Cholasta wrote:

Dne 18.1.2012 00:04, Rob Crittenden napsal(a):

Jan Cholasta wrote:

Dne 16.1.2012 22:02, Rob Crittenden napsal(a):

Rob Crittenden wrote:

Jan Cholasta wrote:

Dne 13.1.2012 20:53, Rob Crittenden napsal(a):

When viewing a certificate it will show the serial number as
hex
(dec).

# ipa service-show HTTP/rawhide.example.com
Principal: HTTP/rawhide.example@example.com
Certificate: [snip]
Keytab: True
Managed by: rawhide.example.com
Subject: CN=rawhide.example.com,O=EXAMPLE.COM
Serial Number: 0x403 (1027)
Issuer: CN=EXAMPLE.COM Certificate Authority
Not Before: Fri Jan 13 15:00:44 2012 UTC
Not After: Thu Jan 13 15:00:44 2022 UTC
Fingerprint (MD5):
e5:43:17:0d:8d:af:d6:69:d8:fb:eb:ca:79:fb:47:69
Fingerprint (SHA1):
c2:9e:8e:de:42:c9:4a:29:cc:b0:a0:de:57:c7:b7:d8:f9:b5:fe:e6

rob



NACK

Displaying a host or a service in the webUI fails with IPA
error
3009:
invalid 'serial_number': Decimal or hexadecimal number is
required
for
serial number.

I would suggest to do the nifty formatting of serial numbers on
the
client side, that would fix the webUI issue, allow non-IPA
clients to
parse the number without dissecting the string representation
of it
and
probably also save me a hack in the type conversion overhaul.
You
could
for example add a parameter flag like format_serial_number to
indicate
to the client that it should format the value as a serial
number.

Honza



Well, we want to do as little client formatting as possible. The
idea is
to have a very thin client.


It doesn't seem right to me to enforce this specific
representation of
what is really just an integer at the API level. Doing a little
formatting on the client side won't make the client(s)
particularly
fat,
will it?


Yes. The current code just outputs labels and data. There is no
if it
is this attribute then do that logic.



IMHO there is too much stuff done on server that would make more
sense
to do on client anyway (especially CLI-specific stuff such as CSV
parsing). What is the reason we want such a thin client?


To avoid double work such that every time we want a formatting
change we
have to change it in multiple places. This lesson was learned in
v1.


I believe there should be clear separation of presentation and
content,
but perhaps I'm a little bit too idealistic :-).


You have a point, serial number is defined as an integer.
Perhaps we
should revisit this decision to display hex at all.






I'll look into fixing the UI side.


I don't see this error in services, it displays correctly. I'm
not
sure
if it is my browser or what but hosts don't display much of
anything
for
me.

rob


I have just checked both master and ipa-2-2 and I'm getting the
same
error message (tested in Firefox 9.0.1) when viewing details of a
host
or a service with the usercertificate attribute set.

BTW, wouldn't it make sense to format serial numbers in the cert
plugin
in the same way?


Perhaps. Like I said, I'm not really in favor of this change.

rob


Maybe we can do a compromise of some sort. What about allowing the
client to specify with each request what representation/formatting
the
server should use for the resulting entries and attributes?


That would be mighty flexible but would open a new can of worms. I
think
long term I'd like to be able to request what attributes to see (ala
ldapsearch) but that too is a bit out of scope.

This comes down to Output being rather loosely defined and we already
have a ticket open on that. It basically just defines the broad
types of
data to be returned (string, list, dict, etc) but not the internal
components of complex types.


Took a new approach and created a new output attribute,
serial_number_hex, that is displayed separately.

UI portion added as well.



ACK for the UI part. I attached a patch which extends UI static testing
data - to keep things in solid state.

I think this approach is still evil (as the whole ticket) but I don't
have a better solution (in CLI).


We are in agreement.


Question:
Isn't the '0x' part a bit redundant? The label already says '(hex)'.
However I can buy a 'It is a convention.' argument.


Yes, I did it for convention, plus to avoid confusion for the case where
it looks like a decimal number but isn't, e.g. 10. If you saw:

Serial number: 16
Serial number (hex): 10

It might be confusing. 0x10 would be clearer.

rob


This patch works for me, but let me repeat myself:

 BTW, wouldn't it make sense to format serial numbers in the cert
 plugin in the same way?

Honza



Updated.

rob



Thanks, ACK.

Honza

--
Jan Cholasta

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


Re: [Freeipa-devel] [PATCH] 107 Fixed evaluating checkbox dirty status

2012-03-14 Thread Endi Sukma Dewata

ACK. I have some comments below.

On 3/9/2012 11:20 AM, Petr Vobornik wrote:

Problem:
When value in checkbox is modified twice in a row (so it is at its
original value) an 'undo' button is still visible even when it shouldn't
be.

Cause:
IPA server sends boolean values as 'TRUE' or 'FALSE' (strings).
Checkbox_widget converts them to JavaScript? boolean (true, false). Save
method in checkbox_widget is returning array with a boolean. So
test_dirty method always evaluates to dirty because 'FALSE' != false.

This patch is fixing the problem.

Note (future enhancements):
As we were talking before about making fields less dependent on widget
types. The dependency comes from the fact that dirty evaluation is in
field. I plan to move the core to widget. I'm thinking about something
like:
Widget would have:
* input value parser - ie for parsing strings to booleans - configurable
* original value (parsed)
* inner state (inner_save())
* is_dirty() - compare inner state with original value
* output formatter - can make boolean back to strings (just example) -
configurable
* save() would return array of formatted values

Field:
* load(record) would pick values from record as now
* is_dirty - needed for facets. Would be: dirty = 'one of associated
widgets is dirty'
* save() - merge associated widgets values - usually only on array from
one widget

Maybe input and output formatters should be in field.


We might need it in both field and widget. There are 3 different values 
that we need to consider:

 * stored value (LDAP), e.g. TRUE/FALSE
 * internal value (JavaScript), e.g. true/false
 * external value (human readable), e.g. True/False

The field will be responsible for converting between stored and internal 
value. The widget will be responsible for converting between internal  
external value if needed.


Suppose we want to display a boolean attribute using a checkbox. We will 
need a boolean field which will convert TRUE/FALSE into true/false. Then 
we also need a standard checkbox that displays true as checked and false 
as unchecked.


Suppose we want to display the boolean attribute in a table column. We 
should be able to use the same boolean field, but then we use a boolean 
column that converts true/false into True/False.


--
Endi S. Dewata

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


Re: [Freeipa-devel] [PATCH] 108 Better hbactest validation message

2012-03-14 Thread Endi Sukma Dewata

On 3/12/2012 8:22 AM, Petr Vobornik wrote:

HBAC Test validation message now contains all missing values in form of
list of links instead of general 'missing values' message and
redirection to first missing value's facet.

When a link is clicked user is redirected to value's facet.

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

Note: In a list I chose to display an option label instead of facet
label. IMHO it seems better with message 'Missing values'.


It looks good. ACK.

--
Endi S. Dewata

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


Re: [Freeipa-devel] [PATCH] 924 display both hex and decimal serial numbers

2012-03-14 Thread Endi Sukma Dewata

On 3/7/2012 9:57 AM, Petr Vobornik wrote:

On 03/06/2012 09:56 PM, Rob Crittenden wrote:

UI portion added as well.


ACK for the UI part. I attached a patch which extends UI static testing
data - to keep things in solid state.


ACK for Petr's patch #101.

--
Endi S. Dewata

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


Re: [Freeipa-devel] [PATCH] [WIP] Cross-realm trusts with AD

2012-03-14 Thread Simo Sorce
On Wed, 2012-03-14 at 15:36 +0200, Alexander Bokovoy wrote:
 On Tue, 13 Mar 2012, Simo Sorce wrote:
  2. samba4 4.0.0-102alpha18 has one minor bug in systemd service
  (https://fedorahosted.org/freeipa/ticket/2523), you'd need to add
 
  ExecStartPre=/bin/mkdir -p /run/samba
 
  before ExecStart= stanza to get it working with tmpfs-based /run in
  Fedora 17.
 
 This is wrong.
 Please add a file in /etc/tmpfiles.d/samba.conf
 
 Contents should be:
 d /var/run/samba  644 root root
 
 (adjust permission and ownership accordingly).
 
 This file needs to be added to the samba4 package (and the samba3
 package as well ?)
 samba4-4.0.0-103alpha18 (in ipa-devel) now includes proper tmpfiles.d
 file. With 755 permissions, obviously.
 

Great!
thanks.

Simo.

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

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


Re: [Freeipa-devel] [PATCH] Fixed rpm build warning - extension.js listed twice

2012-03-14 Thread Petr Viktorin

On 03/14/2012 03:40 PM, Petr Vobornik wrote:

First, I have to say that I'm new to rpm specs and I don't like this
patch but I don't have better solution.

Problem:
Building the ipa rpms returns this:
warning: File listed twice: /usr/share/ipa/ui/extension.js

Cause:
This is because of a glob:
%{_usr}/share/ipa/ui/*.js

and then more specifically:
%config(noreplace) %{_usr}/share/ipa/ui/extension.js

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

Solution thoughts::

1) best way would be limit glob like in bash ie:
'%{_usr}/share/ipa/ui/!(extension).js'. It doesn't work in spec file.
2) I don't want to specify each JavaScript file in spec - would have to
alter spec each time when adding new one.

-- I found nothing better than to use file list.

Filelist generation:
I'm creating it in %prep phase from install/ui/Makefile.am:
grep .js install/ui/Makefile.am | grep -v 'extension.js' | sed -e
's/[\t]*\\//' -e 's|[\t]*|%{_usr}/share/ipa/ui/|'

It would also be possible to generate the list from listing the directory:

find install/ui/ -maxdepth 1 -type f -name *.js ! -name develop.js !
-name extension.js | sed 's|^install|%{usr}/share/ipa|g'

Why from Makefile?:
The list of .js files in git and Makefile differs. Therefore listing
from dir also needs to create exceptions for not used files (develop.js)
- bad in long term.


Slightly related: Have you thought about concatenating the JS files 
(automatically), and serving them all as one file?

http://developer.yahoo.com/performance/rules.html#num_http

--
PetrĀ³

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


Re: [Freeipa-devel] [PATCH] Fixed rpm build warning - extension.js listed twice

2012-03-14 Thread Petr Vobornik

On 03/14/2012 04:06 PM, Petr Viktorin wrote:

On 03/14/2012 03:40 PM, Petr Vobornik wrote:

First, I have to say that I'm new to rpm specs and I don't like this
patch but I don't have better solution.

Problem:
Building the ipa rpms returns this:
warning: File listed twice: /usr/share/ipa/ui/extension.js

Cause:
This is because of a glob:
%{_usr}/share/ipa/ui/*.js

and then more specifically:
%config(noreplace) %{_usr}/share/ipa/ui/extension.js

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

Solution thoughts::

1) best way would be limit glob like in bash ie:
'%{_usr}/share/ipa/ui/!(extension).js'. It doesn't work in spec file.
2) I don't want to specify each JavaScript file in spec - would have to
alter spec each time when adding new one.

-- I found nothing better than to use file list.

Filelist generation:
I'm creating it in %prep phase from install/ui/Makefile.am:
grep .js install/ui/Makefile.am | grep -v 'extension.js' | sed -e
's/[\t]*\\//' -e 's|[\t]*|%{_usr}/share/ipa/ui/|'

It would also be possible to generate the list from listing the
directory:

find install/ui/ -maxdepth 1 -type f -name *.js ! -name develop.js !
-name extension.js | sed 's|^install|%{usr}/share/ipa|g'

Why from Makefile?:
The list of .js files in git and Makefile differs. Therefore listing
from dir also needs to create exceptions for not used files (develop.js)
- bad in long term.


Slightly related: Have you thought about concatenating the JS files
(automatically), and serving them all as one file?
http://developer.yahoo.com/performance/rules.html#num_http

We have a ticket for that (slightly differ, but the idea is mentioned 
there) https://fedorahosted.org/freeipa/ticket/112 . Performance-wise, 
bigger issue is fetching metadata.


Web UI is single page app so it fetches all in first request and then it 
uses AJAX calls to get things done so it would not be a big benefit.



This ticket:
I will use Endi's idea and move extension.js to subfolder.

--
Petr Vobornik

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


Re: [Freeipa-devel] [PATCH] 16 Netgroup nisdomain and hosts validation

2012-03-14 Thread Ondrej Hamada

On 03/09/2012 04:34 PM, Martin Kosek wrote:

On Thu, 2012-03-08 at 14:52 +0100, Ondrej Hamada wrote:

Netgroup nisdomain and hosts validation

nisdomain validation:
Added pattern to the 'nisdomain' parameter to validate the specified
nisdomain name. According to most common use cases the same patter as
for netgroup should fit. Unit-tests added.

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

hosts validation:
Added precallback to netgroup_add_member. It validates the specified
hostnames and raises ValidationError exception for invalid hostnames.
Unit-test added.

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


I checked the host validation part and it could be improved. Issue
described in #2447 (you have switched the ticket IDs) affects all
objects that allow external hosts, users, ..., i.e. those who call
add_external_post_callback in their post_callback.

Should we fix all of these when we deal with this issue? Otherwise user
could do something like this:
# ipa sudorule-add-user foo --users=a+b
   Rule name: foo
   Enabled: TRUE
   External User: a+b

We could create a similar function called add_external_pre_callback()
and pass it attribute name and validating function (which would be
common with the linked object). It would then do the validation for all
these affected objects consistently and without redundant code.

I didn't liked much the implemented pre_callback anyway

+def pre_callback(self, ldap, dn, found, not_found, *keys,
**options):
+# validate entered hostnames
+if 'host' in options:
+invalid_hostnames=[]
+for hostname in options['host']:
+try:
+validate_hostname(hostname, False)
+except ValueError:
+invalid_hostnames.append(hostname)
+if invalid_hostnames:
+raise errors.ValidationError(name='host',
error='hostnames:\%s\ contain invalid characters' %
','.join(invalid_hostnames))
+return dn

I would rather raise the ValidationError with the first invalid hostname
and tell what's wrong (function validate_hostname tells it to you). If
you go with the proposed approach, you wouldn't have to deal with
formatting error messages, you would just raise the one returned by the
validator shared with the linked LDAP object (hostname, user, ...).

Martin


external_pre_callback function seems as a good idea, but there is a 
problem how to get the validators for various LDAP objects. For the 
hostname we already have one in ipalib.utils, but for the uid or group 
name we use only patterns specified in the parameter objects.


Below I propose solution how to use the already defined parameter 
objects for validation (the only problem is that I have to assume, that 
it is always the first parameter in takes_params). Do you think this is 
a good approach?


def add_external_pre_callback(memberattr, membertype, externalattr, 
ldap, dn, found, not_found, *keys, **options):


Pre callback to validate external members.

if membertype in options:
validator = api.Object[membertype].takes_params[0]
for value in options[membertype]:
try:
validator(value)
except errors.ValidationError as e:
error_msg = e[(e.find(':')+1):]
raise errors.ValidationError(name=membertype, 
error=e[e.find(':')+1:])

return dn

--
Regards,

Ondrej Hamada
FreeIPA team
jabber: oh...@jabbim.cz
IRC: ohamada

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


[Freeipa-devel] [PATCH] 0027 Use valid argument names in tests

2012-03-14 Thread Petr Viktorin
This patch depends on my patch 0024 (but I can rebase if it needs to be 
pushed earlied).


It fixes some of the test bugs that would be found by a fix for 
https://fedorahosted.org/freeipa/ticket/2509 (Unknown Command arguments 
are allowed (and ignored)).


As you can see it's quite easy, without validation, to use a wrong name 
for an argument.


Unfortunately, some of our plugins (automount, dns, delegation, 
permission, selfservice) take advantage of the current sloppy 
validation, so they'll need some untangling for 3.x to make them more 
robust and testable. For now I'm just touching the tests, so we don't 
get new bugs in production code.


--
PetrĀ³
From e794bf4fe5d55e3313ec239e29397aaf934bb07d Mon Sep 17 00:00:00 2001
From: Petr Viktorin pvikt...@redhat.com
Date: Wed, 14 Mar 2012 11:33:41 -0400
Subject: [PATCH] Use valid argument names in tests

Some of our tests used unintended extra options, or options with
misspelled, wrongly copy-pasted or otherwise bad names. These are
ignored, so the intended argument was treated as missing. The test
itself can still pass but may be rendered ineffective or fragile.

This fixes some such errors. Actual rejecting of unknown arguments
is deferred for later (ticket #2509).
---
 tests/test_xmlrpc/test_automember_plugin.py |5 +
 tests/test_xmlrpc/test_delegation_plugin.py |2 +-
 tests/test_xmlrpc/test_hbac_plugin.py   |6 +++---
 tests/test_xmlrpc/test_hbactest_plugin.py   |2 +-
 tests/test_xmlrpc/test_pwpolicy.py  |2 +-
 tests/test_xmlrpc/test_role_plugin.py   |2 +-
 tests/test_xmlrpc/test_sudorule_plugin.py   |6 +++---
 7 files changed, 11 insertions(+), 14 deletions(-)

diff --git a/tests/test_xmlrpc/test_automember_plugin.py b/tests/test_xmlrpc/test_automember_plugin.py
index 1241bd669c597c1363b86c4f378472031c3e6682..265c3af861b0bfd5ffa102e5fbb3a81857292996 100644
--- a/tests/test_xmlrpc/test_automember_plugin.py
+++ b/tests/test_xmlrpc/test_automember_plugin.py
@@ -718,10 +718,7 @@ class test_automember(Declarative):
 dict(
 desc='Retrieve default automember group for groups',
 command=(
-'automember_default_group_show', [], dict(
-type=u'group',
-automemberdefaultgroup=defaultgroup1,
-)
+'automember_default_group_show', [], dict(type=u'group')
 ),
 expected=dict(
 result=dict(
diff --git a/tests/test_xmlrpc/test_delegation_plugin.py b/tests/test_xmlrpc/test_delegation_plugin.py
index db5f7186527d2e0c6567dd5a727e878144bd3020..0c32b0447d113cc8e377c8fe139e11ee05e2c8ba 100644
--- a/tests/test_xmlrpc/test_delegation_plugin.py
+++ b/tests/test_xmlrpc/test_delegation_plugin.py
@@ -45,7 +45,7 @@ class test_delegation(Declarative):
 
 dict(
 desc='Try to update non-existent %r' % delegation1,
-command=('delegation_mod', [delegation1], dict(description=u'Foo')),
+command=('delegation_mod', [delegation1], dict(group=u'admins')),
 expected=errors.NotFound(reason='no such entry'),
 ),
 
diff --git a/tests/test_xmlrpc/test_hbac_plugin.py b/tests/test_xmlrpc/test_hbac_plugin.py
index 268c8d9d2217de71a936f691e5dc1666e9afb4b1..a75360636f9f344335cb4fd41df1c44ebe7e770a 100644
--- a/tests/test_xmlrpc/test_hbac_plugin.py
+++ b/tests/test_xmlrpc/test_hbac_plugin.py
@@ -123,7 +123,7 @@ class test_hbac(XMLRPC_test):
 Test searching for HBAC rules using `xmlrpc.hbacrule_find`.
 
 ret = api.Command['hbacrule_find'](
-name=self.rule_name, accessruletype=self.rule_type,
+cn=self.rule_name, accessruletype=self.rule_type,
 description=self.rule_desc_mod
 )
 assert ret['truncated'] is False
@@ -155,7 +155,7 @@ class test_hbac(XMLRPC_test):
 self.test_sourcehostgroup, description=u'desc'
 )
 self.failsafe_add(api.Object.hbacsvc,
-self.test_service, description=u'desc', force=True
+self.test_service, description=u'desc',
 )
 
 def test_8_hbacrule_add_user(self):
@@ -424,7 +424,7 @@ class test_hbac(XMLRPC_test):
 
 api.Command['hbacrule_mod'](self.rule_name, usercategory=u'all')
 try:
-api.Command['hbacrule_add_user'](self.rule_name, users='admin')
+api.Command['hbacrule_add_user'](self.rule_name, user='admin')
 finally:
 api.Command['hbacrule_mod'](self.rule_name, usercategory=u'')
 
diff --git a/tests/test_xmlrpc/test_hbactest_plugin.py b/tests/test_xmlrpc/test_hbactest_plugin.py
index 5d829b14c4fa6e7d96db36b64afe38a171096142..bc12e8974dc4591afa3b4a5f9e68df9ed32e2231 100644
--- a/tests/test_xmlrpc/test_hbactest_plugin.py
+++ b/tests/test_xmlrpc/test_hbactest_plugin.py
@@ -79,7 +79,7 @@ class test_hbactest(XMLRPC_test):
 self.test_sourcehostgroup, description=u'desc'
 )
 

[Freeipa-devel] Please review: take 2: Ticket #1891 - Rewrite IPA plugins to take advantage of the single transaction

2012-03-14 Thread Rich Megginson




freeipa-rmeggins-0004-Rewrite-IPA-plugins-to-take-advantage-of.patch
Description: application/mbox
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Re: [Freeipa-devel] [PATCH] 982 tweak to no_init patch

2012-03-14 Thread Rob Crittenden

Martin Kosek wrote:

On Wed, 2012-03-07 at 16:50 -0500, Rob Crittenden wrote:

I discovered today that cert-request was failing with an untrusted CA error.

The problem had to do with the NSS no_init patch. We were setting dbdir
in the connection object too soon so it was comparing itself to itself
and always determined that NSS was initialized just fine. This needs to
be moved after the check.

To test this you need a master, a replica and a client with DNS set up
and SRV records for both servers.

You need two or more servers so we run the ping() test. This is where
the client was failing before. What would happen is this:

- initialize NSS
- run ping() against a server
- prepare request
- initialize NSS
- FAIL

That second initialization isn't needed and is correctly caught by the
code with this patch.

You need to test that a client enrollment works and that ipa
cert-request works.

cert-request was failing because we initialize NSS with nodb so we can
load the CSR for validation. Because dbdir was set too early in the
connection we were getting no_init set improperly and nss_shutdown()
wasn't being called.

rob


Works for me, ACK.

Please enhance testing instructions in the ticket. I had some issues
reproducing the problem myself, but your advice sent off-list helped me.
This should be enough.

Martin




pushed to master and ipa-2-2

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


Re: [Freeipa-devel] [PATCH] 0023 Don't crash when searching with empty relationship options

2012-03-14 Thread Rob Crittenden

Petr Viktorin wrote:

See commit message.

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



ACK, pushed to master and ipa-2-2

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


Re: [Freeipa-devel] Please review: take 2: Ticket #1891 - Rewrite IPA plugins to take advantage of the single transaction

2012-03-14 Thread Noriko Hosoi

ack.

Rich Megginson wrote:




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


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

Re: [Freeipa-devel] [PATCH] 983 add subject key identifier

2012-03-14 Thread Rob Crittenden

Martin Kosek wrote:

On Wed, 2012-03-07 at 17:49 -0500, Rob Crittenden wrote:

Add subject key identifier to the dogtag server cert profile.

This will add it on upgrades too and any new certs issued will have a
subject key identifier set.

If the user has customized the profile themselves then this won't be
applied.

rob


NACK

I found few issues with the patch:

1) There is an extraneous pdb statement:
+import pdb; pdb.set_trace()

2) A name of config file should be put to some variable once and not
created every time again in enable_subject_key_identifier. It would be
much more readable and less error prone:
+installutils.set_directive('/var/lib/%
s/profiles/ca/caIPAserviceCert.cfg' % PKI_INSTANCE_NAME,
'policyset.serverCertSet.list', '1,2,3,4,5,6,7,8,10', quotes=False,
separator='=')
+installutils.set_directive('/var/lib/%
s/profiles/ca/caIPAserviceCert.cfg' % PKI_INSTANCE_NAME,
'policyset.serverCertSet.10.constraint.class_id', 'noConstraintImpl',
quotes=False, separator='=')
...

3) We do not handle gracefully missing config file. This is what happens
when replica without CA is upgraded:
# rpm -Uvh --force /home/mkosek/dist-review/rpms/freeipa-*
Preparing...### [100%]
1:freeipa-python ### [ 17%]
2:freeipa-client ### [ 33%]
3:freeipa-admintools ### [ 50%]
4:freeipa-server ### [ 67%]
Upgraded /etc/httpd/conf.d/ipa-pki-proxy.conf to version 1
Traceback (most recent call last):
   File /usr/sbin/ipa-upgradeconfig, line 301, inmodule
 sys.exit(main())
   File /usr/sbin/ipa-upgradeconfig, line 297, in main
 upgrade_ipa_profile(krbctx.default_realm)
   File /usr/sbin/ipa-upgradeconfig, line 243, in upgrade_ipa_profile
 if ca.enable_subject_key_identifier():
   File /usr/lib/python2.7/site-packages/ipaserver/install/cainstance.py, 
line 1079, in enable_subject_key_identifier
 setlist = 
installutils.get_directive('/var/lib/%s/profiles/ca/caIPAserviceCert.cfg' % 
PKI_INSTANCE_NAME, 'policyset.serverCertSet.list', separator='=')
   File /usr/lib/python2.7/site-packages/ipaserver/install/installutils.py, 
line 429, in get_directive
 fd = open(filename, r)
IOError: [Errno 2] No such file or directory: 
'/var/lib/pki-ca/profiles/ca/caIPAserviceCert.cfg'
5:freeipa-server-selinux ### [ 83%]
6:freeipa-debuginfo  ### [100%]

  1. Martin



I think this should do it.

rob
From e24088093029c1d8b55f487f009547d214bce568 Mon Sep 17 00:00:00 2001
From: Rob Crittenden rcrit...@redhat.com
Date: Wed, 7 Mar 2012 17:46:33 -0500
Subject: [PATCH] Add subject key identifier to the dogtag server cert
 profile.

This will add it on upgrades too and any new certs issued will have
a subject key identifier set.

If the user has customized the profile themselves then this won't be
applied.

https://fedorahosted.org/freeipa/ticket/2446
---
 install/tools/ipa-upgradeconfig |   13 ++
 ipaserver/install/cainstance.py |   47 +-
 2 files changed, 58 insertions(+), 2 deletions(-)

diff --git a/install/tools/ipa-upgradeconfig b/install/tools/ipa-upgradeconfig
index a23489f406f29db4b8f33c153cccb1121675eb61..40a2b68ce89b58b98077428783a98e3060674665 100644
--- a/install/tools/ipa-upgradeconfig
+++ b/install/tools/ipa-upgradeconfig
@@ -31,6 +31,8 @@ try:
 from ipaserver.install import httpinstance
 from ipaserver.install import memcacheinstance
 from ipaserver.install import service
+from ipaserver.install import cainstance
+from ipaserver.install import certs
 import ldap
 import krbV
 import re
@@ -233,6 +235,15 @@ def cleanup_kdc():
 if fstore.has_file(filename):
 fstore.untrack_file(filename)
 
+def upgrade_ipa_profile(realm):
+
+Update the IPA Profile provided by dogtag
+
+ca = cainstance.CAInstance(realm, certs.NSS_DIR)
+if ca.is_configured():
+if ca.enable_subject_key_identifier():
+ca.restart()
+
 def main():
 
 Get some basics about the system. If getting those basics fail then
@@ -284,6 +295,8 @@ def main():
 pass
 
 cleanup_kdc()
+upgrade_ipa_profile(krbctx.default_realm)
+
 try:
 if __name__ == __main__:
 sys.exit(main())
diff --git a/ipaserver/install/cainstance.py b/ipaserver/install/cainstance.py
index 3afc705ddc677c035d63c804d4a28737c03d8352..f953100be9d8e99abf402ae8453ca39a26758da1 100644
--- a/ipaserver/install/cainstance.py
+++ b/ipaserver/install/cainstance.py
@@ -72,6 +72,7 @@ EE_CLIENT_AUTH_PORT=9446
 UNSECURE_PORT=9180
 TOMCAT_SERVER_PORT=9701
 
+IPA_SERVICE_PROFILE = '/var/lib/%s/profiles/ca/caIPAserviceCert.cfg' % PKI_INSTANCE_NAME
 
 # We need to reset the 

[Freeipa-devel] [PATCH] 44 Add Automember Test to simulate logic decisions

2012-03-14 Thread JR Aquino
This will be _very_ helpful for testing automember logic against potential 
users / hosts.

This patch addes a new plugin to FreeIPA that tests automember logic decisions
https://fedorahosted.org/freeipa/ticket/2535

~
Jr Aquino | Sr. Information Security Specialist
GIAC Certified Incident Handler | GIAC WebApp Penetration Tester
Citrix Online | 7408 Hollister Avenue | Goleta, CA 
93117x-apple-data-detectors://0/0
T:  +1 805.690.3478tel:+1%C2%A0805.690.3478
C: +1 805.717.0365tel:+1%20805.717.0365
jr.aqu...@citrixonline.commailto:jr.aqu...@citrixonline.com
http://www.citrixonline.comhttp://www.citrixonline.com/


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


Re: [Freeipa-devel] [PATCH] 924 display both hex and decimal serial numbers

2012-03-14 Thread Rob Crittenden

Endi Sukma Dewata wrote:

On 3/7/2012 9:57 AM, Petr Vobornik wrote:

On 03/06/2012 09:56 PM, Rob Crittenden wrote:

UI portion added as well.


ACK for the UI part. I attached a patch which extends UI static testing
data - to keep things in solid state.


ACK for Petr's patch #101.



Pushed both patches to ipa-2-2 and master.

rob

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


Re: [Freeipa-devel] [PATCH] 981 set httpd_manage_ipa

2012-03-14 Thread Rob Crittenden

Alexander Bokovoy wrote:

On Mon, 12 Mar 2012, Rob Crittenden wrote:

Rob Crittenden wrote:

Alexander Bokovoy wrote:

On Mon, 12 Mar 2012, Rob Crittenden wrote:

Alexander Bokovoy wrote:

On Wed, 07 Mar 2012, Rob Crittenden wrote:


Set SELinux boolean httpd_manage_ipa so ipa_memcached will work in
enforcing mode.

This is being done in the HTTP instance so we can set both booleans
in one step and save a bit of time (it is still slow).

I would prefer all platform-specific manipulations of security
policies to be moved to platform-specific module.

Make a HTTP class there (like I did dirsrv class in systemd
backend) and perform manipulations on service enable.

This way main code will stay clear of platform-specific code.

Sorry for not looking into the issue before.



I'd prefer to keep the change simple for now and do the big move post
2.2.

ACK on condition you'd file a ticket for the post 2.2 work.

:)


Filed this https://fedorahosted.org/freeipa/ticket/2519

I found an issue with this patch that I need to address, will submit a
replacement.

rob


Handle things better if a boolean doesn't exist.

Lucky that setsebool takes multiple booleans at the same time...
Maybe it would make sense to merge bools upon recover?

Otherwise ACK.



pushed to master and ipa-2-2

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


Re: [Freeipa-devel] [PATCH] 232 Treat UPGs correctly in winsync replication

2012-03-14 Thread Rob Crittenden

Martin Kosek wrote:

There are some test hints attached to the ticket.
---
IPA winsync plugin failed to replicate users when default user group
was non-posix even though User Private Groups (UPG) were enabled
on the server. Both their uidNumber and gidNumber were empty and
they missed essential object classes. When the default user group
was made posix and UPG was disabled it did not set gidNumber to
the default group gidNumber.

This patch improves this behavior to set gidNumber correctly
according to UPG configuration and the default group status
(posix/non-posix). 4 situations can occur, the following list
specifies what value is assigned to user gidNumber:
  1) Default group posix, UPG enabled: gidNumber = UPG gidNumber
  2) Default group posix, UPG disabled: gidNumber = default
 group gidNumber
  3) Default group non-posix, UPG enabled: gidNumber = UPG gidNumber
  4) Default group non-posix, UPG disabled: an error is printed to
 the dirsrv log as the gidNumber cannot be retrieved. User
 is replicated in the same way as before this patch, i.e.
 without essential object classes.

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


ACK, works as advertised.

rob

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