Re: [Freeipa-devel] [PATCH] 0056 Support requests for DOMAIN$ account for trusted domain in ipasam module

2012-06-28 Thread Martin Kosek
On 06/27/2012 05:59 PM, Sumit Bose wrote:
 On Wed, Jun 27, 2012 at 05:36:51PM +0300, Alexander Bokovoy wrote:
 Hi,

 Windows 2008R2 attempts to authenticate as DOMAIN$ with trust password
 when trust is established. Change ipasam module to consider DOMAIN$ when
 checking for trusted domain accounts as current code only checks for
 DOMAIN. (ending with dot).
 
 ACK
 
 bye,
 Sumit
 

 https://fedorahosted.org/freeipa/ticket/2870
 -- 
 / Alexander Bokovoy
 

Pushed to master (I just removed one trailing whitespace before that).

Martin

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


Re: [Freeipa-devel] [PATCH] 0055 Add error condition handling to SASL bind callback in ipasam module

2012-06-28 Thread Martin Kosek
On 06/27/2012 06:12 PM, Sumit Bose wrote:
 On Wed, Jun 27, 2012 at 07:09:03PM +0300, Alexander Bokovoy wrote:
 On Wed, 27 Jun 2012, Sumit Bose wrote:
 On Wed, Jun 27, 2012 at 05:29:07PM +0300, Alexander Bokovoy wrote:
 Hi,

 attached patch adds comprehensive error condition handling to SASL bind
 callback in ipasam module. The callback is doing keytab-based auth
 against FreeIPA LDAP server and original version lacked error checks on
 purpose.

 The patch is working find, but I would like to ask you to consider the
 following two changes:
 Yep. Completely overlooked that I've already got the service principal.
 Thanks!

 Updated patch attached.

 Please note that patches 0055 and 0056 should be applied in the reverse
 order. Sorry for confusion.
 
 ACK
 
 bye,
 Sumit

Pushed to master.

Martin

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


Re: [Freeipa-devel] [PATCH] Add sidgen postop and task

2012-06-28 Thread Martin Kosek
On 06/27/2012 06:27 PM, Alexander Bokovoy wrote:
 On Mon, 25 Jun 2012, Sumit Bose wrote:
 Hi,

 this patch added support to automatically create SIDs for local objects
 as described in ticket https://fedorahosted.org/freeipa/ticket/2825.

 The post-operation plugin adds the SID and if necessary the needed
 objectclass for a newly created object.
 ACK.
 
 Works for me in tests.

Pushed to master.

Martin


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


Re: [Freeipa-devel] [PATCH] Filter groups in the PAC

2012-06-28 Thread Martin Kosek
On 06/27/2012 06:28 PM, Alexander Bokovoy wrote:
 On Tue, 26 Jun 2012, Sumit Bose wrote:
 Hi,

 this patch contains the KDC part of the external groups handling. If
 group SIDs from the PAC can be found in the ipaExternalGroup objects and
 the external groups are member of local groups, the SIDs of the local
 groups are added to the PAC. If the PAC this then read by the SSSD pac
 responder the user from the PAC is added to the local groups on the
 client.
 ACK. There were code-related comments from Simo yesterday on IRC but it
 was agreed to solve those in separate patches.
 

Pushed to master.

Martin

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


Re: [Freeipa-devel] [PATCH] 18 Add external domain extop DS plugin

2012-06-28 Thread Martin Kosek
On 06/27/2012 06:38 PM, Alexander Bokovoy wrote:
 On Wed, 27 Jun 2012, Sumit Bose wrote:
 On Wed, Jun 13, 2012 at 12:37:49PM +0200, Sumit Bose wrote:
 On Wed, Jun 13, 2012 at 12:26:43PM +0200, Sumit Bose wrote:
  On Mon, Jun 11, 2012 at 05:46:17PM +0300, Alexander Bokovoy wrote:
   On Thu, 07 Jun 2012, Sumit Bose wrote:
   On Fri, Mar 23, 2012 at 01:52:34PM +0100, Sumit Bose wrote:
   Hi,
   
   these two patches introduce a new extended operation to the IPA server
   which can be used by clients in the IPA domain to obtain information
   about users and groups from trusted domains. Currently this exop is 
   used
   by the sssd sub-domain patch to map user names from a trusted AD 
   domain
   to a SID and back. There is also some code for other kind of requests
   which might become useful in future, e.g. with trusted IPA domain.
   
   I added some unit test and added check for the check unit test 
   framework
   for C (http://check.sourceforge.net/) which is used by sssd as well. I
   modified the spec file that the test is run during the build of the
   packages. I hope this is ok.
   
   The patches depend on the idmap library patch which was ACKed recently
   on sssd-devel and as mentioned before the sub-domain patches on
   sssd-devel can only be fully tested with an IPA server which has these
   patches applied.
   
   Since Alexander is currently rewriting parts of the 
   ipa-adtrust-install
   utility I stand back from adding activation code for the exop to
   ipa-adtrust-install and will send a patch when Alexander's changes are
   available. So currently extdom-extop-conf.ldif has to be loaded 
   manually
   after replacing $SUFFIX to activate the new exop.
   
   bye,
   Sumit
   
   Please find a rebased version of the patches which work on top of
   Alexander's latest series of patches. The patches now also contain the
   loading of extdom-extop-conf.ldif and the activation of winbind.
   Thanks for the rebase.
  
   Few comments.
  
   1.The extdom plugin should support IDMAP_BOTH. We do provide user 
   private
   groups so in our case it should be viewed as preferred output. Thus you
   would need to add new response type to cover this case.
 
  Currently the plugin only uses winbind to map SIDs to names and back and
  in the returned user data the user private groups are already respected
  by setting the GID to the UID. On the client side sssd handles the
  trusted domains a mpg (magic private group) domains.
 
  
   2. I have tried to look at the plugin description from point of view of
   a system administrator and I failed to understand what it does:
   +#define IPA_EXTDOM_PLUGIN_NAME   ipa-extdom-extop
   +#define IPA_EXTDOM_FEATURE_DESC  IPA EXTDOM ID mapper
   +#define IPA_EXTDOM_PLUGIN_DESC   IPA EXTDOM ID mapper Extended
 Operation plugin
  
   In the ipa-extdom-extop-conf.ldif you have following description:
   +nsslapd-plugindescription: Support resolving IDs in trusted domains to
 names and back
   Probably it is better to reuse the same description in
 IPA_EXTDOM_PLUGIN_DESC?
  
   This is a minor point but EXTDOM itself is vague. Maybe we should be
 more clear
   and call it 'IPA trusted domain ID mapper' as it really limits itself to
   only trusted domains? We don't dispatch winbind request if the domain is
   not found in our list of trusted domains.
 
  I have updated the descriptions. I prefer the EXTDOM prefix because
  there might be future use cases where we might want to get some data
  from other domains without trust. But I'm happy to change it if you like
  a different prefix better.
 
  
   3. Could you please define the oid in ipa_extdom.h so that it could be
   useful for client code as well?
   +#define EXOP_EXTDOM_OID 2.16.840.1.113730.3.8.10.4
 
  done
 
  New version attached.

 ah. sorry, forgot to squash in some changes.

 Additionally I moved the binary to the freeipa-server-trust-ad package
 to avoid additional dependencies in the freeipa-server package.

 bye,
 Sumit

 
  
   4. Do we have 'check' tool in RHEL6?
 
  yes, current version is check-0.9.8-1.1.el6
 
  Thank you for the review.
 
  bye,
  Sumit
   --
   / Alexander Bokovoy

 rebased version with some changes by Alexander attached.
 ACK from my side. Works in tests I've run.

Patch 17 pushed to master.

Patch 18 does not apply. I also have one question related to this patch:

We added a winbind service to ADTRUSTInstance which is now being configured as
a part of ipa-adtrust-install. To make this cleaner, we may want to write
winbind's own service.Service class which would do the necessary configuration
and could be also better expanded in the future.

Martin

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


Re: [Freeipa-devel] [PATCH] 1030 Fedora 18 compatibility

2012-06-28 Thread Martin Kosek
On 06/27/2012 07:46 PM, Rob Crittenden wrote:
 I found a few minor issues when building and installing the master branch on
 Fedora 18. This patch should address it.
 
 rob
 

1) This will fail for on F17-F18 upgrades, we need to bump VERSION in
ipa-rewrite.conf.

Besides that, ipa-upgradeconfig needs to be fixed, otherwise it will crash
during ipa-rewrite.conf upgrade - ${AUTOREDIR} variable is not set.

However, this variable will need to be figured out from current
ipa-rewrite.conf contents as it depends on whether the IPA server was installed
with --no-ui-redirect or not.

2) Shouldn't we bump tomcat6 version as well since we depend on the tomcat6
fixed in BZ 831464?

3) %changelog entry is missing

Martin

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


[Freeipa-devel] Build failure in ipa_sam

2012-06-28 Thread William Brown
Making all in ipa-sam
make[4]: Entering directory
`/home/william/development/freeipa/rpmbuild/BUILD/freeipa-2.99.0GIT8ce7330/daemons/ipa-sam'
/bin/sh ../libtool --tag=CC   --mode=compile gcc -DHAVE_CONFIG_H -I.
-I.. -I. -I. -I/usr/include/samba-4.0 -DPREFIX=\/usr\
-DBINDIR=\/usr/bin\ -DLIBDIR=\/usr/lib64\
-DLIBEXECDIR=\/usr/libexec\ -DDATADIR=\/usr/share\
-DLDAPIDIR=\/var/run\ -DHAVE_LDAP -I ../../util  -Wall -Wshadow
-Wstrict-prototypes -Wpointer-arith -Wcast-align
-Werror-implicit-function-declaration  -DWITH_OPENLDAP
-I/usr/include/nspr4 -I/usr/include/nss3 -DUSE_OPENLDAP
-DHAVE_IMMEDIATE_STRUCTURES=1 -I/usr/include/samba-4.0
-DHAVE_IMMEDIATE_STRUCTURES=1 -D_GNU_SOURCE=1 -I/usr/include/samba-4.0
   -I/usr/include/nspr4 -I/usr/include/nss3  -Wall -Wshadow
-Wstrict-prototypes -Wpointer-arith -Wcast-align
-Werror-implicit-function-declaration  -O2 -g -pipe -Wall
-Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector
--param=ssp-buffer-size=4  -m64 -mtune=generic -MT ipa_sam.lo -MD -MP
-MF .deps/ipa_sam.Tpo -c -o ipa_sam.lo ipa_sam.c
libtool: compile:  gcc -DHAVE_CONFIG_H -I. -I.. -I. -I.
-I/usr/include/samba-4.0 -DPREFIX=\/usr\ -DBINDIR=\/usr/bin\
-DLIBDIR=\/usr/lib64\ -DLIBEXECDIR=\/usr/libexec\
-DDATADIR=\/usr/share\ -DLDAPIDIR=\/var/run\ -DHAVE_LDAP -I
../../util -Wall -Wshadow -Wstrict-prototypes -Wpointer-arith
-Wcast-align -Werror-implicit-function-declaration -DWITH_OPENLDAP
-I/usr/include/nspr4 -I/usr/include/nss3 -DUSE_OPENLDAP
-DHAVE_IMMEDIATE_STRUCTURES=1 -I/usr/include/samba-4.0
-DHAVE_IMMEDIATE_STRUCTURES=1 -D_GNU_SOURCE=1 -I/usr/include/samba-4.0
-I/usr/include/nspr4 -I/usr/include/nss3 -Wall -Wshadow
-Wstrict-prototypes -Wpointer-arith -Wcast-align
-Werror-implicit-function-declaration -O2 -g -pipe -Wall
-Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector
--param=ssp-buffer-size=4 -m64 -mtune=generic -MT ipa_sam.lo -MD -MP -MF
.deps/ipa_sam.Tpo -c ipa_sam.c  -fPIC -DPIC -o .libs/ipa_sam.o
ipa_sam.c:510:17: warning: 'struct unixid' declared inside parameter
list [enabled by default]
ipa_sam.c:510:17: warning: its scope is only this definition or
declaration, which is probably not what you want [enabled by default]
ipa_sam.c: In function 'ldapsam_sid_to_id':
ipa_sam.c:583:3: error: implicit declaration of function
'unixid_from_gid' [-Werror=implicit-function-declaration]
ipa_sam.c:598:2: error: implicit declaration of function
'unixid_from_uid' [-Werror=implicit-function-declaration]
ipa_sam.c: In function 'set_krb_princ':
ipa_sam.c:1456:8: warning: unused variable 'inp' [-Wunused-variable]
ipa_sam.c: In function 'ldap_sasl_interact':
ipa_sam.c:3100:18: warning: unused variable 'krberr' [-Wunused-variable]
ipa_sam.c:3099:8: warning: unused variable 'outname' [-Wunused-variable]
ipa_sam.c:3098:15: warning: unused variable 'krbctx' [-Wunused-variable]
ipa_sam.c: At top level:
ipa_sam.c:3126:1: warning: function declaration isn't a prototype
[-Wstrict-prototypes]
ipa_sam.c:3127:1: warning: function declaration isn't a prototype
[-Wstrict-prototypes]
ipa_sam.c: In function 'bind_callback':
ipa_sam.c:3131:18: warning: variable 'rc' set but not used
[-Wunused-but-set-variable]
ipa_sam.c: In function 'pdb_init_ipasam':
ipa_sam.c:3355:27: warning: assignment from incompatible pointer type
[enabled by default]
cc1: some warnings being treated as errors
make[4]: *** [ipa_sam.lo] Error 1
make[4]: Leaving directory
`/home/william/development/freeipa/rpmbuild/BUILD/freeipa-2.99.0GIT8ce7330/daemons/ipa-sam'
make[3]: *** [all-recursive] Error 1
make[3]: Leaving directory
`/home/william/development/freeipa/rpmbuild/BUILD/freeipa-2.99.0GIT8ce7330/daemons'
make[2]: *** [all] Error 2
make[2]: Leaving directory
`/home/william/development/freeipa/rpmbuild/BUILD/freeipa-2.99.0GIT8ce7330/daemons'
make[1]: *** [all] Error 1
make[1]: Leaving directory
`/home/william/development/freeipa/rpmbuild/BUILD/freeipa-2.99.0GIT8ce7330'
error: Bad exit status from /var/tmp/rpm-tmp.tskROb (%build)


RPM build errors:
Bad exit status from /var/tmp/rpm-tmp.tskROb (%build)
make: *** [rpms] Error 1


Git master (Not my feature branches)
-- 
Sincerely,

William Brown

pgp.mit.edu
http://pgp.mit.edu:11371/pks/lookup?op=vindexsearch=0x3C0AC6DAB2F928A2




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

Re: [Freeipa-devel] Build failure in ipa_sam

2012-06-28 Thread Alexander Bokovoy

On Thu, 28 Jun 2012, William Brown wrote:

Making all in ipa-sam
make[4]: Entering directory
`/home/william/development/freeipa/rpmbuild/BUILD/freeipa-2.99.0GIT8ce7330/daemons/ipa-sam'
/bin/sh ../libtool --tag=CC   --mode=compile gcc -DHAVE_CONFIG_H -I.
-I.. -I. -I. -I/usr/include/samba-4.0 -DPREFIX=\/usr\
-DBINDIR=\/usr/bin\ -DLIBDIR=\/usr/lib64\
-DLIBEXECDIR=\/usr/libexec\ -DDATADIR=\/usr/share\
-DLDAPIDIR=\/var/run\ -DHAVE_LDAP -I ../../util  -Wall -Wshadow
-Wstrict-prototypes -Wpointer-arith -Wcast-align
-Werror-implicit-function-declaration  -DWITH_OPENLDAP
-I/usr/include/nspr4 -I/usr/include/nss3 -DUSE_OPENLDAP
-DHAVE_IMMEDIATE_STRUCTURES=1 -I/usr/include/samba-4.0
-DHAVE_IMMEDIATE_STRUCTURES=1 -D_GNU_SOURCE=1 -I/usr/include/samba-4.0
  -I/usr/include/nspr4 -I/usr/include/nss3  -Wall -Wshadow
-Wstrict-prototypes -Wpointer-arith -Wcast-align
-Werror-implicit-function-declaration  -O2 -g -pipe -Wall
-Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector
--param=ssp-buffer-size=4  -m64 -mtune=generic -MT ipa_sam.lo -MD -MP
-MF .deps/ipa_sam.Tpo -c -o ipa_sam.lo ipa_sam.c
libtool: compile:  gcc -DHAVE_CONFIG_H -I. -I.. -I. -I.
-I/usr/include/samba-4.0 -DPREFIX=\/usr\ -DBINDIR=\/usr/bin\
-DLIBDIR=\/usr/lib64\ -DLIBEXECDIR=\/usr/libexec\
-DDATADIR=\/usr/share\ -DLDAPIDIR=\/var/run\ -DHAVE_LDAP -I
../../util -Wall -Wshadow -Wstrict-prototypes -Wpointer-arith
-Wcast-align -Werror-implicit-function-declaration -DWITH_OPENLDAP
-I/usr/include/nspr4 -I/usr/include/nss3 -DUSE_OPENLDAP
-DHAVE_IMMEDIATE_STRUCTURES=1 -I/usr/include/samba-4.0
-DHAVE_IMMEDIATE_STRUCTURES=1 -D_GNU_SOURCE=1 -I/usr/include/samba-4.0
-I/usr/include/nspr4 -I/usr/include/nss3 -Wall -Wshadow
-Wstrict-prototypes -Wpointer-arith -Wcast-align
-Werror-implicit-function-declaration -O2 -g -pipe -Wall
-Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector
--param=ssp-buffer-size=4 -m64 -mtune=generic -MT ipa_sam.lo -MD -MP -MF
.deps/ipa_sam.Tpo -c ipa_sam.c  -fPIC -DPIC -o .libs/ipa_sam.o
ipa_sam.c:510:17: warning: 'struct unixid' declared inside parameter
list [enabled by default]

Missing 'struct unixid' means you don't have newer samba4 packages.


ipa_sam.c:510:17: warning: its scope is only this definition or
declaration, which is probably not what you want [enabled by default]
ipa_sam.c: In function 'ldapsam_sid_to_id':
ipa_sam.c:583:3: error: implicit declaration of function
'unixid_from_gid' [-Werror=implicit-function-declaration]
ipa_sam.c:598:2: error: implicit declaration of function
'unixid_from_uid' [-Werror=implicit-function-declaration]
ipa_sam.c: In function 'set_krb_princ':
ipa_sam.c:1456:8: warning: unused variable 'inp' [-Wunused-variable]
ipa_sam.c: In function 'ldap_sasl_interact':
ipa_sam.c:3100:18: warning: unused variable 'krberr' [-Wunused-variable]
ipa_sam.c:3099:8: warning: unused variable 'outname' [-Wunused-variable]
ipa_sam.c:3098:15: warning: unused variable 'krbctx' [-Wunused-variable]
ipa_sam.c: At top level:
ipa_sam.c:3126:1: warning: function declaration isn't a prototype
[-Wstrict-prototypes]
ipa_sam.c:3127:1: warning: function declaration isn't a prototype
[-Wstrict-prototypes]
ipa_sam.c: In function 'bind_callback':
ipa_sam.c:3131:18: warning: variable 'rc' set but not used
[-Wunused-but-set-variable]
ipa_sam.c: In function 'pdb_init_ipasam':
ipa_sam.c:3355:27: warning: assignment from incompatible pointer type
[enabled by default]
cc1: some warnings being treated as errors
make[4]: *** [ipa_sam.lo] Error 1
make[4]: Leaving directory
`/home/william/development/freeipa/rpmbuild/BUILD/freeipa-2.99.0GIT8ce7330/daemons/ipa-sam'
make[3]: *** [all-recursive] Error 1
make[3]: Leaving directory
`/home/william/development/freeipa/rpmbuild/BUILD/freeipa-2.99.0GIT8ce7330/daemons'
make[2]: *** [all] Error 2
make[2]: Leaving directory
`/home/william/development/freeipa/rpmbuild/BUILD/freeipa-2.99.0GIT8ce7330/daemons'
make[1]: *** [all] Error 1
make[1]: Leaving directory
`/home/william/development/freeipa/rpmbuild/BUILD/freeipa-2.99.0GIT8ce7330'
error: Bad exit status from /var/tmp/rpm-tmp.tskROb (%build)


RPM build errors:
   Bad exit status from /var/tmp/rpm-tmp.tskROb (%build)
make: *** [rpms] Error 1


Git master (Not my feature branches)

Make sure you have at least samba4 beta1 packages installed as required by the
freeipa.spec.in file. I have samba4-devel-4.0.0-124beta1.fc17.x86_64 from 
ipa-devel
repo.

Also 8ce7330 is not git master HEAD:
$ git log --oneline 8ce733..HEAD
ac6afd3 Add configure check for C Unit-Test framework check
dc3491e Filter groups in the PAC
65ad261 Add sidgen postop and task
6356747 Add error condition handling to the SASL bind callback in ipasam
761cb71 Support requests for DOMAIN$ account for trusted domains in ipasam 
module
db4c946 Defer adding ipa-cifs-delegation-targets until the Updates phase.

--
/ Alexander Bokovoy

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


Re: [Freeipa-devel] [DRAFT2] Per-domain DNS update permissions

2012-06-28 Thread Petr Viktorin

On 06/27/2012 06:01 PM, Petr Viktorin wrote:

On 06/27/2012 02:50 PM, Martin Kosek wrote:

On 06/25/2012 08:50 PM, Rob Crittenden wrote:

Simo Sorce wrote:

On Fri, 2012-06-22 at 14:25 +0200, Martin Kosek wrote:

On 06/22/2012 02:23 PM, Simo Sorce wrote:

On Fri, 2012-06-22 at 12:20 +0200, Martin Kosek wrote:

On 06/18/2012 05:37 PM, Rob Crittenden wrote:

Martin Kosek wrote:

On Fri, 2012-06-15 at 10:15 -0400, Simo Sorce wrote:

On Fri, 2012-06-15 at 15:22 +0200, Martin Kosek wrote:

Hello all,

In a scope of ticket 2511 I would like to implement an
ability to
delegate a DNS update permissions to chosen user (or host)
without
having to give the user full Update DNS Entries privileges,
i.e.
allow
him to modify any DNS zone or record.

So far, this is what I would like to do (comments welcome):

1) Create new objectclass idnsManagedZone with managedBy
attribute
in MAY list
2) Create new DNS commands:
 a] dnszone-add-managedby [--users=USERS] [--hosts=HOSTS]
 b] dnszone-remove-managedby [--users=USERS] [--hosts=HOSTS]
 - these commands would add/remove chosen user/host DN to
managedBy
attribute in chosen DNS zone
3) Add new generic ACIs to cn=dns,$SUFFIX:
aci: (target = ldap:///idnsname=*,cn=dns,$SUFFIX;)(version
3.0;acl
Users and hosts can add DNS entries;allow (add) userattr =
parent[1].managedby#USERDN;)
... add similar ACIs for UPDATE, REMOVE access

With these steps done, all that an administrator would need
to do to
delegate a management of a DNS zone example.com is to run this
command:
$ ipa dnszone-add-managedby example.com --users=fbar

The only downside I found so far is that the user would
already need to
have Read DNS Entries permission assigned, otherwise he
would not be
able to actually read DNS entries (allow rules can't take
precedence
over deny rule we implemented to deny public access to DNS
tree).

An admin could of course create a special privilege and role
with just
Read DNS Entries permission and then assign it to relevant
users/groups, but this looks awkward. Any idea to make this
simpler?
Maybe creating a group dns readers by default which would
allow such
access?


Change the deny rule to deny to everyone except the user in
parent[1].managedby#USERDN ?

Simo.



Good idea, I will do that. I will just use
parent[0,1].managedby#USERDN so that user can also read the zone
record. This way, a selected user will have read/write access
to the
chosen zone only, which is exactly what we want to achieve.


Yes, this sounds workable to me too.

rob



There were some second thoughts about the proposed design, which
I would
like to discuss so that we can eventually accept another (better)
solution for this feature.

The main concern here was that proposed solution (based on user
list in
managedBy attribute in DNS zone) is not in line with the rest of
permissionprivilege architecture in IPA.

Here is another idea how to address the feature (I tested it and it
would work):
1) Get rid of the deny rule on cn=dns,$SUFFIX by modifying global
access
rule (a working patch attached) to avoid current and future
issues with
extending ACIs (deny rules are evil).

2) Add new Managed Entry Definition and Template to automatically
add
Manage DNS zone $idsname permission. These could be used with
standard
IPA privileges, roles and thus could be assigned to users, groups,
hosts, hostgroups...

3) New DNS zone managedBy attribute won't be manageable by user,
but it
will hold a DN of the managed Permission entry

4) Add the following ACIs to cn=dns,$SUFFIX:
aci: (targetattr = *)
(version 3.0; acl Read DNS entries; allow (read,search,compare)
userattr = parent[0,1].managedby#GROUPDN;)

aci: (target = ldap:///idnsname=*,cn=dns,$SUFFIX;)
(version 3.0;acl Add dns entries;allow (add)
userattr = parent[1].managedby#GROUPDN;)

aci: (target = ldap:///idnsname=*,cn=dns,$SUFFIX;)
(version 3.0;acl Remove DNS entries;allow (delete)
userattr = parent[1].managedby#GROUPDN;)

aci: (targetattr = idnsname || cn || idnsallowdynupdate ||
dnsttl ||
dnsclass || arecord || record || a6record || nsrecord ||
cnamerecord
|| ptrrecord || srvrecord || txtrecord || mxrecord || mdrecord   ||
hinforecord || minforecord || afsdbrecord || sigrecord ||
keyrecord ||
locrecord || nxtrecord || naptrrecord || kxrecord ||
certrecord ||
dnamerecord || dsrecord || sshfprecord || rrsigrecord ||
nsecrecord ||
idnsname || idnszoneactive || idnssoamname || idnssoarname ||
idnssoaserial || idnssoarefresh || idnssoaretry || idnssoaexpire ||
idnssoaminimum || idnsupdatepolicy || idnsallowquery ||
idnsallowtransfer || idnsallowsyncptr || idnsforwardpolicy ||
idnsforwarders)
(target = ldap:///idnsname=*,cn=dns,$SUFFIX;)(version 3.0;acl
Update
DNS Entries;allow (write) userattr =
parent[0,1].managedby#GROUPDN;)

I needed to add permission DN to the managedBy attribute so that
I could
create just one set of generic ACIs without having to create a
set of
ACIs for every new zone and thus let users with Update DNS entries
permission have a write 

Re: [Freeipa-devel] Build failure in ipa_sam

2012-06-28 Thread William Brown
On 28/06/12 18:43, Alexander Bokovoy wrote:
 On Thu, 28 Jun 2012, William Brown wrote:
 Making all in ipa-sam
 make[4]: Entering directory
 `/home/william/development/freeipa/rpmbuild/BUILD/freeipa-2.99.0GIT8ce7330/daemons/ipa-sam'

 /bin/sh ../libtool --tag=CC   --mode=compile gcc -DHAVE_CONFIG_H -I.
 -I.. -I. -I. -I/usr/include/samba-4.0 -DPREFIX=\/usr\
 -DBINDIR=\/usr/bin\ -DLIBDIR=\/usr/lib64\
 -DLIBEXECDIR=\/usr/libexec\ -DDATADIR=\/usr/share\
 -DLDAPIDIR=\/var/run\ -DHAVE_LDAP -I ../../util  -Wall -Wshadow
 -Wstrict-prototypes -Wpointer-arith -Wcast-align
 -Werror-implicit-function-declaration  -DWITH_OPENLDAP
 -I/usr/include/nspr4 -I/usr/include/nss3 -DUSE_OPENLDAP
 -DHAVE_IMMEDIATE_STRUCTURES=1 -I/usr/include/samba-4.0
 -DHAVE_IMMEDIATE_STRUCTURES=1 -D_GNU_SOURCE=1 -I/usr/include/samba-4.0
   -I/usr/include/nspr4 -I/usr/include/nss3  -Wall -Wshadow
 -Wstrict-prototypes -Wpointer-arith -Wcast-align
 -Werror-implicit-function-declaration  -O2 -g -pipe -Wall
 -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector
 --param=ssp-buffer-size=4  -m64 -mtune=generic -MT ipa_sam.lo -MD -MP
 -MF .deps/ipa_sam.Tpo -c -o ipa_sam.lo ipa_sam.c
 libtool: compile:  gcc -DHAVE_CONFIG_H -I. -I.. -I. -I.
 -I/usr/include/samba-4.0 -DPREFIX=\/usr\ -DBINDIR=\/usr/bin\
 -DLIBDIR=\/usr/lib64\ -DLIBEXECDIR=\/usr/libexec\
 -DDATADIR=\/usr/share\ -DLDAPIDIR=\/var/run\ -DHAVE_LDAP -I
 ../../util -Wall -Wshadow -Wstrict-prototypes -Wpointer-arith
 -Wcast-align -Werror-implicit-function-declaration -DWITH_OPENLDAP
 -I/usr/include/nspr4 -I/usr/include/nss3 -DUSE_OPENLDAP
 -DHAVE_IMMEDIATE_STRUCTURES=1 -I/usr/include/samba-4.0
 -DHAVE_IMMEDIATE_STRUCTURES=1 -D_GNU_SOURCE=1 -I/usr/include/samba-4.0
 -I/usr/include/nspr4 -I/usr/include/nss3 -Wall -Wshadow
 -Wstrict-prototypes -Wpointer-arith -Wcast-align
 -Werror-implicit-function-declaration -O2 -g -pipe -Wall
 -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector
 --param=ssp-buffer-size=4 -m64 -mtune=generic -MT ipa_sam.lo -MD -MP -MF
 .deps/ipa_sam.Tpo -c ipa_sam.c  -fPIC -DPIC -o .libs/ipa_sam.o
 ipa_sam.c:510:17: warning: 'struct unixid' declared inside parameter
 list [enabled by default]
 Missing 'struct unixid' means you don't have newer samba4 packages.
 
 ipa_sam.c:510:17: warning: its scope is only this definition or
 declaration, which is probably not what you want [enabled by default]
 ipa_sam.c: In function 'ldapsam_sid_to_id':
 ipa_sam.c:583:3: error: implicit declaration of function
 'unixid_from_gid' [-Werror=implicit-function-declaration]
 ipa_sam.c:598:2: error: implicit declaration of function
 'unixid_from_uid' [-Werror=implicit-function-declaration]
 ipa_sam.c: In function 'set_krb_princ':
 ipa_sam.c:1456:8: warning: unused variable 'inp' [-Wunused-variable]
 ipa_sam.c: In function 'ldap_sasl_interact':
 ipa_sam.c:3100:18: warning: unused variable 'krberr' [-Wunused-variable]
 ipa_sam.c:3099:8: warning: unused variable 'outname' [-Wunused-variable]
 ipa_sam.c:3098:15: warning: unused variable 'krbctx' [-Wunused-variable]
 ipa_sam.c: At top level:
 ipa_sam.c:3126:1: warning: function declaration isn't a prototype
 [-Wstrict-prototypes]
 ipa_sam.c:3127:1: warning: function declaration isn't a prototype
 [-Wstrict-prototypes]
 ipa_sam.c: In function 'bind_callback':
 ipa_sam.c:3131:18: warning: variable 'rc' set but not used
 [-Wunused-but-set-variable]
 ipa_sam.c: In function 'pdb_init_ipasam':
 ipa_sam.c:3355:27: warning: assignment from incompatible pointer type
 [enabled by default]
 cc1: some warnings being treated as errors
 make[4]: *** [ipa_sam.lo] Error 1
 make[4]: Leaving directory
 `/home/william/development/freeipa/rpmbuild/BUILD/freeipa-2.99.0GIT8ce7330/daemons/ipa-sam'

 make[3]: *** [all-recursive] Error 1
 make[3]: Leaving directory
 `/home/william/development/freeipa/rpmbuild/BUILD/freeipa-2.99.0GIT8ce7330/daemons'

 make[2]: *** [all] Error 2
 make[2]: Leaving directory
 `/home/william/development/freeipa/rpmbuild/BUILD/freeipa-2.99.0GIT8ce7330/daemons'

 make[1]: *** [all] Error 1
 make[1]: Leaving directory
 `/home/william/development/freeipa/rpmbuild/BUILD/freeipa-2.99.0GIT8ce7330'

 error: Bad exit status from /var/tmp/rpm-tmp.tskROb (%build)


 RPM build errors:
Bad exit status from /var/tmp/rpm-tmp.tskROb (%build)
 make: *** [rpms] Error 1


 Git master (Not my feature branches)
 Make sure you have at least samba4 beta1 packages installed as required
 by the
 freeipa.spec.in file. I have samba4-devel-4.0.0-124beta1.fc17.x86_64
 from ipa-devel
 repo.

Is that the repo at:

[ipa-devel]
name=IPA development $releasever - $basearch
baseurl=http://jdennis.fedorapeople.org/ipa-devel/fedora/$releasever/$basearch/os/
enabled=0
gpgcheck=0


 
 Also 8ce7330 is not git master HEAD:
 $ git log --oneline 8ce733..HEAD
 ac6afd3 Add configure check for C Unit-Test framework check
 dc3491e Filter groups in the PAC
 65ad261 Add sidgen postop and task
 6356747 Add error condition handling to the SASL bind callback in ipasam
 761cb71 Support requests 

Re: [Freeipa-devel] Build failure in ipa_sam

2012-06-28 Thread Alexander Bokovoy

On Thu, 28 Jun 2012, William Brown wrote:

On 28/06/12 18:43, Alexander Bokovoy wrote:

On Thu, 28 Jun 2012, William Brown wrote:

Making all in ipa-sam
make[4]: Entering directory
`/home/william/development/freeipa/rpmbuild/BUILD/freeipa-2.99.0GIT8ce7330/daemons/ipa-sam'

/bin/sh ../libtool --tag=CC   --mode=compile gcc -DHAVE_CONFIG_H -I.
-I.. -I. -I. -I/usr/include/samba-4.0 -DPREFIX=\/usr\
-DBINDIR=\/usr/bin\ -DLIBDIR=\/usr/lib64\
-DLIBEXECDIR=\/usr/libexec\ -DDATADIR=\/usr/share\
-DLDAPIDIR=\/var/run\ -DHAVE_LDAP -I ../../util  -Wall -Wshadow
-Wstrict-prototypes -Wpointer-arith -Wcast-align
-Werror-implicit-function-declaration  -DWITH_OPENLDAP
-I/usr/include/nspr4 -I/usr/include/nss3 -DUSE_OPENLDAP
-DHAVE_IMMEDIATE_STRUCTURES=1 -I/usr/include/samba-4.0
-DHAVE_IMMEDIATE_STRUCTURES=1 -D_GNU_SOURCE=1 -I/usr/include/samba-4.0
  -I/usr/include/nspr4 -I/usr/include/nss3  -Wall -Wshadow
-Wstrict-prototypes -Wpointer-arith -Wcast-align
-Werror-implicit-function-declaration  -O2 -g -pipe -Wall
-Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector
--param=ssp-buffer-size=4  -m64 -mtune=generic -MT ipa_sam.lo -MD -MP
-MF .deps/ipa_sam.Tpo -c -o ipa_sam.lo ipa_sam.c
libtool: compile:  gcc -DHAVE_CONFIG_H -I. -I.. -I. -I.
-I/usr/include/samba-4.0 -DPREFIX=\/usr\ -DBINDIR=\/usr/bin\
-DLIBDIR=\/usr/lib64\ -DLIBEXECDIR=\/usr/libexec\
-DDATADIR=\/usr/share\ -DLDAPIDIR=\/var/run\ -DHAVE_LDAP -I
../../util -Wall -Wshadow -Wstrict-prototypes -Wpointer-arith
-Wcast-align -Werror-implicit-function-declaration -DWITH_OPENLDAP
-I/usr/include/nspr4 -I/usr/include/nss3 -DUSE_OPENLDAP
-DHAVE_IMMEDIATE_STRUCTURES=1 -I/usr/include/samba-4.0
-DHAVE_IMMEDIATE_STRUCTURES=1 -D_GNU_SOURCE=1 -I/usr/include/samba-4.0
-I/usr/include/nspr4 -I/usr/include/nss3 -Wall -Wshadow
-Wstrict-prototypes -Wpointer-arith -Wcast-align
-Werror-implicit-function-declaration -O2 -g -pipe -Wall
-Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector
--param=ssp-buffer-size=4 -m64 -mtune=generic -MT ipa_sam.lo -MD -MP -MF
.deps/ipa_sam.Tpo -c ipa_sam.c  -fPIC -DPIC -o .libs/ipa_sam.o
ipa_sam.c:510:17: warning: 'struct unixid' declared inside parameter
list [enabled by default]

Missing 'struct unixid' means you don't have newer samba4 packages.


ipa_sam.c:510:17: warning: its scope is only this definition or
declaration, which is probably not what you want [enabled by default]
ipa_sam.c: In function 'ldapsam_sid_to_id':
ipa_sam.c:583:3: error: implicit declaration of function
'unixid_from_gid' [-Werror=implicit-function-declaration]
ipa_sam.c:598:2: error: implicit declaration of function
'unixid_from_uid' [-Werror=implicit-function-declaration]
ipa_sam.c: In function 'set_krb_princ':
ipa_sam.c:1456:8: warning: unused variable 'inp' [-Wunused-variable]
ipa_sam.c: In function 'ldap_sasl_interact':
ipa_sam.c:3100:18: warning: unused variable 'krberr' [-Wunused-variable]
ipa_sam.c:3099:8: warning: unused variable 'outname' [-Wunused-variable]
ipa_sam.c:3098:15: warning: unused variable 'krbctx' [-Wunused-variable]
ipa_sam.c: At top level:
ipa_sam.c:3126:1: warning: function declaration isn't a prototype
[-Wstrict-prototypes]
ipa_sam.c:3127:1: warning: function declaration isn't a prototype
[-Wstrict-prototypes]
ipa_sam.c: In function 'bind_callback':
ipa_sam.c:3131:18: warning: variable 'rc' set but not used
[-Wunused-but-set-variable]
ipa_sam.c: In function 'pdb_init_ipasam':
ipa_sam.c:3355:27: warning: assignment from incompatible pointer type
[enabled by default]
cc1: some warnings being treated as errors
make[4]: *** [ipa_sam.lo] Error 1
make[4]: Leaving directory
`/home/william/development/freeipa/rpmbuild/BUILD/freeipa-2.99.0GIT8ce7330/daemons/ipa-sam'

make[3]: *** [all-recursive] Error 1
make[3]: Leaving directory
`/home/william/development/freeipa/rpmbuild/BUILD/freeipa-2.99.0GIT8ce7330/daemons'

make[2]: *** [all] Error 2
make[2]: Leaving directory
`/home/william/development/freeipa/rpmbuild/BUILD/freeipa-2.99.0GIT8ce7330/daemons'

make[1]: *** [all] Error 1
make[1]: Leaving directory
`/home/william/development/freeipa/rpmbuild/BUILD/freeipa-2.99.0GIT8ce7330'

error: Bad exit status from /var/tmp/rpm-tmp.tskROb (%build)


RPM build errors:
   Bad exit status from /var/tmp/rpm-tmp.tskROb (%build)
make: *** [rpms] Error 1


Git master (Not my feature branches)

Make sure you have at least samba4 beta1 packages installed as required
by the
freeipa.spec.in file. I have samba4-devel-4.0.0-124beta1.fc17.x86_64
from ipa-devel
repo.


Is that the repo at:

[ipa-devel]
name=IPA development $releasever - $basearch
baseurl=http://jdennis.fedorapeople.org/ipa-devel/fedora/$releasever/$basearch/os/
enabled=0
gpgcheck=0

Yes.

Note that FreeIPA git master is only working on F17 with packages from the
ipa-devel repo.

--
/ Alexander Bokovoy

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


Re: [Freeipa-devel] [PATCH] Per-domain DNS update permissions

2012-06-28 Thread Martin Kosek
On 06/28/2012 11:20 AM, Petr Viktorin wrote:
 On 06/27/2012 06:01 PM, Petr Viktorin wrote:
 On 06/27/2012 02:50 PM, Martin Kosek wrote:
 On 06/25/2012 08:50 PM, Rob Crittenden wrote:
 Simo Sorce wrote:
 On Fri, 2012-06-22 at 14:25 +0200, Martin Kosek wrote:
 On 06/22/2012 02:23 PM, Simo Sorce wrote:
 On Fri, 2012-06-22 at 12:20 +0200, Martin Kosek wrote:
 On 06/18/2012 05:37 PM, Rob Crittenden wrote:
 Martin Kosek wrote:
 On Fri, 2012-06-15 at 10:15 -0400, Simo Sorce wrote:
 On Fri, 2012-06-15 at 15:22 +0200, Martin Kosek wrote:
 Hello all,

 In a scope of ticket 2511 I would like to implement an
 ability to
 delegate a DNS update permissions to chosen user (or host)
 without
 having to give the user full Update DNS Entries privileges,
 i.e.
 allow
 him to modify any DNS zone or record.

 So far, this is what I would like to do (comments welcome):

 1) Create new objectclass idnsManagedZone with managedBy
 attribute
 in MAY list
 2) Create new DNS commands:
  a] dnszone-add-managedby [--users=USERS] [--hosts=HOSTS]
  b] dnszone-remove-managedby [--users=USERS] [--hosts=HOSTS]
  - these commands would add/remove chosen user/host DN to
 managedBy
 attribute in chosen DNS zone
 3) Add new generic ACIs to cn=dns,$SUFFIX:
 aci: (target = ldap:///idnsname=*,cn=dns,$SUFFIX;)(version
 3.0;acl
 Users and hosts can add DNS entries;allow (add) userattr =
 parent[1].managedby#USERDN;)
 ... add similar ACIs for UPDATE, REMOVE access

 With these steps done, all that an administrator would need
 to do to
 delegate a management of a DNS zone example.com is to run this
 command:
 $ ipa dnszone-add-managedby example.com --users=fbar

 The only downside I found so far is that the user would
 already need to
 have Read DNS Entries permission assigned, otherwise he
 would not be
 able to actually read DNS entries (allow rules can't take
 precedence
 over deny rule we implemented to deny public access to DNS
 tree).

 An admin could of course create a special privilege and role
 with just
 Read DNS Entries permission and then assign it to relevant
 users/groups, but this looks awkward. Any idea to make this
 simpler?
 Maybe creating a group dns readers by default which would
 allow such
 access?

 Change the deny rule to deny to everyone except the user in
 parent[1].managedby#USERDN ?

 Simo.


 Good idea, I will do that. I will just use
 parent[0,1].managedby#USERDN so that user can also read the zone
 record. This way, a selected user will have read/write access
 to the
 chosen zone only, which is exactly what we want to achieve.

 Yes, this sounds workable to me too.

 rob


 There were some second thoughts about the proposed design, which
 I would
 like to discuss so that we can eventually accept another (better)
 solution for this feature.

 The main concern here was that proposed solution (based on user
 list in
 managedBy attribute in DNS zone) is not in line with the rest of
 permissionprivilege architecture in IPA.

 Here is another idea how to address the feature (I tested it and it
 would work):
 1) Get rid of the deny rule on cn=dns,$SUFFIX by modifying global
 access
 rule (a working patch attached) to avoid current and future
 issues with
 extending ACIs (deny rules are evil).

 2) Add new Managed Entry Definition and Template to automatically
 add
 Manage DNS zone $idsname permission. These could be used with
 standard
 IPA privileges, roles and thus could be assigned to users, groups,
 hosts, hostgroups...

 3) New DNS zone managedBy attribute won't be manageable by user,
 but it
 will hold a DN of the managed Permission entry

 4) Add the following ACIs to cn=dns,$SUFFIX:
 aci: (targetattr = *)
 (version 3.0; acl Read DNS entries; allow (read,search,compare)
 userattr = parent[0,1].managedby#GROUPDN;)

 aci: (target = ldap:///idnsname=*,cn=dns,$SUFFIX;)
 (version 3.0;acl Add dns entries;allow (add)
 userattr = parent[1].managedby#GROUPDN;)

 aci: (target = ldap:///idnsname=*,cn=dns,$SUFFIX;)
 (version 3.0;acl Remove DNS entries;allow (delete)
 userattr = parent[1].managedby#GROUPDN;)

 aci: (targetattr = idnsname || cn || idnsallowdynupdate ||
 dnsttl ||
 dnsclass || arecord || record || a6record || nsrecord ||
 cnamerecord
 || ptrrecord || srvrecord || txtrecord || mxrecord || mdrecord   ||
 hinforecord || minforecord || afsdbrecord || sigrecord ||
 keyrecord ||
 locrecord || nxtrecord || naptrrecord || kxrecord ||
 certrecord ||
 dnamerecord || dsrecord || sshfprecord || rrsigrecord ||
 nsecrecord ||
 idnsname || idnszoneactive || idnssoamname || idnssoarname ||
 idnssoaserial || idnssoarefresh || idnssoaretry || idnssoaexpire ||
 idnssoaminimum || idnsupdatepolicy || idnsallowquery ||
 idnsallowtransfer || idnsallowsyncptr || idnsforwardpolicy ||
 idnsforwarders)
 (target = ldap:///idnsname=*,cn=dns,$SUFFIX;)(version 3.0;acl
 Update
 DNS Entries;allow (write) userattr =
 parent[0,1].managedby#GROUPDN;)

 I needed to add permission DN to the managedBy attribute so that
 I could
 create 

Re: [Freeipa-devel] [SOLVED] Build failure in ipa_sam

2012-06-28 Thread William Brown
On 28/06/12 19:35, Alexander Bokovoy wrote:
 On Thu, 28 Jun 2012, William Brown wrote:
 On 28/06/12 18:43, Alexander Bokovoy wrote:
 On Thu, 28 Jun 2012, William Brown wrote:
 Making all in ipa-sam
 make[4]: Entering directory
 `/home/william/development/freeipa/rpmbuild/BUILD/freeipa-2.99.0GIT8ce7330/daemons/ipa-sam'


 /bin/sh ../libtool --tag=CC   --mode=compile gcc -DHAVE_CONFIG_H -I.
 -I.. -I. -I. -I/usr/include/samba-4.0 -DPREFIX=\/usr\
 -DBINDIR=\/usr/bin\ -DLIBDIR=\/usr/lib64\
 -DLIBEXECDIR=\/usr/libexec\ -DDATADIR=\/usr/share\
 -DLDAPIDIR=\/var/run\ -DHAVE_LDAP -I ../../util  -Wall -Wshadow
 -Wstrict-prototypes -Wpointer-arith -Wcast-align
 -Werror-implicit-function-declaration  -DWITH_OPENLDAP
 -I/usr/include/nspr4 -I/usr/include/nss3 -DUSE_OPENLDAP
 -DHAVE_IMMEDIATE_STRUCTURES=1 -I/usr/include/samba-4.0
 -DHAVE_IMMEDIATE_STRUCTURES=1 -D_GNU_SOURCE=1 -I/usr/include/samba-4.0
   -I/usr/include/nspr4 -I/usr/include/nss3  -Wall -Wshadow
 -Wstrict-prototypes -Wpointer-arith -Wcast-align
 -Werror-implicit-function-declaration  -O2 -g -pipe -Wall
 -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector
 --param=ssp-buffer-size=4  -m64 -mtune=generic -MT ipa_sam.lo -MD -MP
 -MF .deps/ipa_sam.Tpo -c -o ipa_sam.lo ipa_sam.c
 libtool: compile:  gcc -DHAVE_CONFIG_H -I. -I.. -I. -I.
 -I/usr/include/samba-4.0 -DPREFIX=\/usr\ -DBINDIR=\/usr/bin\
 -DLIBDIR=\/usr/lib64\ -DLIBEXECDIR=\/usr/libexec\
 -DDATADIR=\/usr/share\ -DLDAPIDIR=\/var/run\ -DHAVE_LDAP -I
 ../../util -Wall -Wshadow -Wstrict-prototypes -Wpointer-arith
 -Wcast-align -Werror-implicit-function-declaration -DWITH_OPENLDAP
 -I/usr/include/nspr4 -I/usr/include/nss3 -DUSE_OPENLDAP
 -DHAVE_IMMEDIATE_STRUCTURES=1 -I/usr/include/samba-4.0
 -DHAVE_IMMEDIATE_STRUCTURES=1 -D_GNU_SOURCE=1 -I/usr/include/samba-4.0
 -I/usr/include/nspr4 -I/usr/include/nss3 -Wall -Wshadow
 -Wstrict-prototypes -Wpointer-arith -Wcast-align
 -Werror-implicit-function-declaration -O2 -g -pipe -Wall
 -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector
 --param=ssp-buffer-size=4 -m64 -mtune=generic -MT ipa_sam.lo -MD -MP
 -MF
 .deps/ipa_sam.Tpo -c ipa_sam.c  -fPIC -DPIC -o .libs/ipa_sam.o
 ipa_sam.c:510:17: warning: 'struct unixid' declared inside parameter
 list [enabled by default]
 Missing 'struct unixid' means you don't have newer samba4 packages.

 ipa_sam.c:510:17: warning: its scope is only this definition or
 declaration, which is probably not what you want [enabled by default]
 ipa_sam.c: In function 'ldapsam_sid_to_id':
 ipa_sam.c:583:3: error: implicit declaration of function
 'unixid_from_gid' [-Werror=implicit-function-declaration]
 ipa_sam.c:598:2: error: implicit declaration of function
 'unixid_from_uid' [-Werror=implicit-function-declaration]
 ipa_sam.c: In function 'set_krb_princ':
 ipa_sam.c:1456:8: warning: unused variable 'inp' [-Wunused-variable]
 ipa_sam.c: In function 'ldap_sasl_interact':
 ipa_sam.c:3100:18: warning: unused variable 'krberr'
 [-Wunused-variable]
 ipa_sam.c:3099:8: warning: unused variable 'outname'
 [-Wunused-variable]
 ipa_sam.c:3098:15: warning: unused variable 'krbctx'
 [-Wunused-variable]
 ipa_sam.c: At top level:
 ipa_sam.c:3126:1: warning: function declaration isn't a prototype
 [-Wstrict-prototypes]
 ipa_sam.c:3127:1: warning: function declaration isn't a prototype
 [-Wstrict-prototypes]
 ipa_sam.c: In function 'bind_callback':
 ipa_sam.c:3131:18: warning: variable 'rc' set but not used
 [-Wunused-but-set-variable]
 ipa_sam.c: In function 'pdb_init_ipasam':
 ipa_sam.c:3355:27: warning: assignment from incompatible pointer type
 [enabled by default]
 cc1: some warnings being treated as errors
 make[4]: *** [ipa_sam.lo] Error 1
 make[4]: Leaving directory
 `/home/william/development/freeipa/rpmbuild/BUILD/freeipa-2.99.0GIT8ce7330/daemons/ipa-sam'


 make[3]: *** [all-recursive] Error 1
 make[3]: Leaving directory
 `/home/william/development/freeipa/rpmbuild/BUILD/freeipa-2.99.0GIT8ce7330/daemons'


 make[2]: *** [all] Error 2
 make[2]: Leaving directory
 `/home/william/development/freeipa/rpmbuild/BUILD/freeipa-2.99.0GIT8ce7330/daemons'


 make[1]: *** [all] Error 1
 make[1]: Leaving directory
 `/home/william/development/freeipa/rpmbuild/BUILD/freeipa-2.99.0GIT8ce7330'


 error: Bad exit status from /var/tmp/rpm-tmp.tskROb (%build)


 RPM build errors:
Bad exit status from /var/tmp/rpm-tmp.tskROb (%build)
 make: *** [rpms] Error 1


 Git master (Not my feature branches)
 Make sure you have at least samba4 beta1 packages installed as required
 by the
 freeipa.spec.in file. I have samba4-devel-4.0.0-124beta1.fc17.x86_64
 from ipa-devel
 repo.

 Is that the repo at:

 [ipa-devel]
 name=IPA development $releasever - $basearch
 baseurl=http://jdennis.fedorapeople.org/ipa-devel/fedora/$releasever/$basearch/os/

 enabled=0
 gpgcheck=0
 Yes.
 
 Note that FreeIPA git master is only working on F17 with packages from the
 ipa-devel repo.
 

Yep I'm on f17.

Updated ipa-devel, working now. Thanks.

-- 
Sincerely,

William Brown

pgp.mit.edu

Re: [Freeipa-devel] [PATCH] 18 Add external domain extop DS plugin

2012-06-28 Thread Martin Kosek
On 06/28/2012 12:19 PM, Sumit Bose wrote:
 On Thu, Jun 28, 2012 at 09:52:14AM +0200, Martin Kosek wrote:
 On 06/27/2012 06:38 PM, Alexander Bokovoy wrote:
 On Wed, 27 Jun 2012, Sumit Bose wrote:
 On Wed, Jun 13, 2012 at 12:37:49PM +0200, Sumit Bose wrote:
 On Wed, Jun 13, 2012 at 12:26:43PM +0200, Sumit Bose wrote:
 On Mon, Jun 11, 2012 at 05:46:17PM +0300, Alexander Bokovoy wrote:
 On Thu, 07 Jun 2012, Sumit Bose wrote:
 On Fri, Mar 23, 2012 at 01:52:34PM +0100, Sumit Bose wrote:
 Hi,

 these two patches introduce a new extended operation to the IPA server
 which can be used by clients in the IPA domain to obtain information
 about users and groups from trusted domains. Currently this exop is 
 used
 by the sssd sub-domain patch to map user names from a trusted AD 
 domain
 to a SID and back. There is also some code for other kind of requests
 which might become useful in future, e.g. with trusted IPA domain.

 I added some unit test and added check for the check unit test 
 framework
 for C (http://check.sourceforge.net/) which is used by sssd as well. I
 modified the spec file that the test is run during the build of the
 packages. I hope this is ok.

 The patches depend on the idmap library patch which was ACKed recently
 on sssd-devel and as mentioned before the sub-domain patches on
 sssd-devel can only be fully tested with an IPA server which has these
 patches applied.

 Since Alexander is currently rewriting parts of the 
 ipa-adtrust-install
 utility I stand back from adding activation code for the exop to
 ipa-adtrust-install and will send a patch when Alexander's changes are
 available. So currently extdom-extop-conf.ldif has to be loaded 
 manually
 after replacing $SUFFIX to activate the new exop.

 bye,
 Sumit

 Please find a rebased version of the patches which work on top of
 Alexander's latest series of patches. The patches now also contain the
 loading of extdom-extop-conf.ldif and the activation of winbind.
 Thanks for the rebase.

 Few comments.

 1.The extdom plugin should support IDMAP_BOTH. We do provide user 
 private
 groups so in our case it should be viewed as preferred output. Thus you
 would need to add new response type to cover this case.

 Currently the plugin only uses winbind to map SIDs to names and back and
 in the returned user data the user private groups are already respected
 by setting the GID to the UID. On the client side sssd handles the
 trusted domains a mpg (magic private group) domains.


 2. I have tried to look at the plugin description from point of view of
 a system administrator and I failed to understand what it does:
 +#define IPA_EXTDOM_PLUGIN_NAME   ipa-extdom-extop
 +#define IPA_EXTDOM_FEATURE_DESC  IPA EXTDOM ID mapper
 +#define IPA_EXTDOM_PLUGIN_DESC   IPA EXTDOM ID mapper Extended
 Operation plugin

 In the ipa-extdom-extop-conf.ldif you have following description:
 +nsslapd-plugindescription: Support resolving IDs in trusted domains to
 names and back
 Probably it is better to reuse the same description in
 IPA_EXTDOM_PLUGIN_DESC?

 This is a minor point but EXTDOM itself is vague. Maybe we should be
 more clear
 and call it 'IPA trusted domain ID mapper' as it really limits itself to
 only trusted domains? We don't dispatch winbind request if the domain is
 not found in our list of trusted domains.

 I have updated the descriptions. I prefer the EXTDOM prefix because
 there might be future use cases where we might want to get some data
 from other domains without trust. But I'm happy to change it if you like
 a different prefix better.


 3. Could you please define the oid in ipa_extdom.h so that it could be
 useful for client code as well?
 +#define EXOP_EXTDOM_OID 2.16.840.1.113730.3.8.10.4

 done

 New version attached.

 ah. sorry, forgot to squash in some changes.

 Additionally I moved the binary to the freeipa-server-trust-ad package
 to avoid additional dependencies in the freeipa-server package.

 bye,
 Sumit



 4. Do we have 'check' tool in RHEL6?

 yes, current version is check-0.9.8-1.1.el6

 Thank you for the review.

 bye,
 Sumit
 --
 / Alexander Bokovoy

 rebased version with some changes by Alexander attached.
 ACK from my side. Works in tests I've run.

 Patch 17 pushed to master.

 Patch 18 does not apply. I also have one question related to this patch:
 
 a rebased version is attached.
 

 We added a winbind service to ADTRUSTInstance which is now being configured 
 as
 a part of ipa-adtrust-install. To make this cleaner, we may want to write
 winbind's own service.Service class which would do the necessary 
 configuration
 and could be also better expanded in the future.
 
 Currently none of the configuration steps are done exclusively for
 winbind, e.g. winbind will use the same credential as the smbd to access
 the directory server. I would agree to create an class for winbind if it
 turns out that we have to add special winbind options, but for now we
 only need to start the winbind process.

Ok, lets keep current 

Re: [Freeipa-devel] [PATCH] 18 Add external domain extop DS plugin

2012-06-28 Thread Martin Kosek
On 06/28/2012 01:09 PM, Martin Kosek wrote:
 On 06/28/2012 12:19 PM, Sumit Bose wrote:
 On Thu, Jun 28, 2012 at 09:52:14AM +0200, Martin Kosek wrote:
 On 06/27/2012 06:38 PM, Alexander Bokovoy wrote:
 On Wed, 27 Jun 2012, Sumit Bose wrote:
 On Wed, Jun 13, 2012 at 12:37:49PM +0200, Sumit Bose wrote:
 On Wed, Jun 13, 2012 at 12:26:43PM +0200, Sumit Bose wrote:
 On Mon, Jun 11, 2012 at 05:46:17PM +0300, Alexander Bokovoy wrote:
 On Thu, 07 Jun 2012, Sumit Bose wrote:
 On Fri, Mar 23, 2012 at 01:52:34PM +0100, Sumit Bose wrote:
 Hi,

 these two patches introduce a new extended operation to the IPA 
 server
 which can be used by clients in the IPA domain to obtain information
 about users and groups from trusted domains. Currently this exop is 
 used
 by the sssd sub-domain patch to map user names from a trusted AD 
 domain
 to a SID and back. There is also some code for other kind of requests
 which might become useful in future, e.g. with trusted IPA domain.

 I added some unit test and added check for the check unit test 
 framework
 for C (http://check.sourceforge.net/) which is used by sssd as well. 
 I
 modified the spec file that the test is run during the build of the
 packages. I hope this is ok.

 The patches depend on the idmap library patch which was ACKed 
 recently
 on sssd-devel and as mentioned before the sub-domain patches on
 sssd-devel can only be fully tested with an IPA server which has 
 these
 patches applied.

 Since Alexander is currently rewriting parts of the 
 ipa-adtrust-install
 utility I stand back from adding activation code for the exop to
 ipa-adtrust-install and will send a patch when Alexander's changes 
 are
 available. So currently extdom-extop-conf.ldif has to be loaded 
 manually
 after replacing $SUFFIX to activate the new exop.

 bye,
 Sumit

 Please find a rebased version of the patches which work on top of
 Alexander's latest series of patches. The patches now also contain the
 loading of extdom-extop-conf.ldif and the activation of winbind.
 Thanks for the rebase.

 Few comments.

 1.The extdom plugin should support IDMAP_BOTH. We do provide user 
 private
 groups so in our case it should be viewed as preferred output. Thus you
 would need to add new response type to cover this case.

 Currently the plugin only uses winbind to map SIDs to names and back and
 in the returned user data the user private groups are already respected
 by setting the GID to the UID. On the client side sssd handles the
 trusted domains a mpg (magic private group) domains.


 2. I have tried to look at the plugin description from point of view of
 a system administrator and I failed to understand what it does:
 +#define IPA_EXTDOM_PLUGIN_NAME   ipa-extdom-extop
 +#define IPA_EXTDOM_FEATURE_DESC  IPA EXTDOM ID mapper
 +#define IPA_EXTDOM_PLUGIN_DESC   IPA EXTDOM ID mapper Extended
 Operation plugin

 In the ipa-extdom-extop-conf.ldif you have following description:
 +nsslapd-plugindescription: Support resolving IDs in trusted domains 
 to
 names and back
 Probably it is better to reuse the same description in
 IPA_EXTDOM_PLUGIN_DESC?

 This is a minor point but EXTDOM itself is vague. Maybe we should be
 more clear
 and call it 'IPA trusted domain ID mapper' as it really limits itself 
 to
 only trusted domains? We don't dispatch winbind request if the domain 
 is
 not found in our list of trusted domains.

 I have updated the descriptions. I prefer the EXTDOM prefix because
 there might be future use cases where we might want to get some data
 from other domains without trust. But I'm happy to change it if you like
 a different prefix better.


 3. Could you please define the oid in ipa_extdom.h so that it could be
 useful for client code as well?
 +#define EXOP_EXTDOM_OID 2.16.840.1.113730.3.8.10.4

 done

 New version attached.

 ah. sorry, forgot to squash in some changes.

 Additionally I moved the binary to the freeipa-server-trust-ad package
 to avoid additional dependencies in the freeipa-server package.

 bye,
 Sumit



 4. Do we have 'check' tool in RHEL6?

 yes, current version is check-0.9.8-1.1.el6

 Thank you for the review.

 bye,
 Sumit
 --
 / Alexander Bokovoy

 rebased version with some changes by Alexander attached.
 ACK from my side. Works in tests I've run.

 Patch 17 pushed to master.

 Patch 18 does not apply. I also have one question related to this patch:

 a rebased version is attached.


 We added a winbind service to ADTRUSTInstance which is now being configured 
 as
 a part of ipa-adtrust-install. To make this cleaner, we may want to write
 winbind's own service.Service class which would do the necessary 
 configuration
 and could be also better expanded in the future.

 Currently none of the configuration steps are done exclusively for
 winbind, e.g. winbind will use the same credential as the smbd to access
 the directory server. I would agree to create an class for winbind if it
 turns out that we have to add special winbind options, but for now we
 only need 

Re: [Freeipa-devel] [PATCH] 18 Add external domain extop DS plugin

2012-06-28 Thread Sumit Bose
On Thu, Jun 28, 2012 at 01:51:28PM +0200, Martin Kosek wrote:
 On 06/28/2012 01:09 PM, Martin Kosek wrote:
  On 06/28/2012 12:19 PM, Sumit Bose wrote:
  On Thu, Jun 28, 2012 at 09:52:14AM +0200, Martin Kosek wrote:
  On 06/27/2012 06:38 PM, Alexander Bokovoy wrote:
  On Wed, 27 Jun 2012, Sumit Bose wrote:
  On Wed, Jun 13, 2012 at 12:37:49PM +0200, Sumit Bose wrote:
  On Wed, Jun 13, 2012 at 12:26:43PM +0200, Sumit Bose wrote:
  On Mon, Jun 11, 2012 at 05:46:17PM +0300, Alexander Bokovoy wrote:
  On Thu, 07 Jun 2012, Sumit Bose wrote:
  On Fri, Mar 23, 2012 at 01:52:34PM +0100, Sumit Bose wrote:
  Hi,
 
  these two patches introduce a new extended operation to the IPA 
  server
  which can be used by clients in the IPA domain to obtain 
  information
  about users and groups from trusted domains. Currently this exop 
  is used
  by the sssd sub-domain patch to map user names from a trusted AD 
  domain
  to a SID and back. There is also some code for other kind of 
  requests
  which might become useful in future, e.g. with trusted IPA domain.
 
  I added some unit test and added check for the check unit test 
  framework
  for C (http://check.sourceforge.net/) which is used by sssd as 
  well. I
  modified the spec file that the test is run during the build of the
  packages. I hope this is ok.
 
  The patches depend on the idmap library patch which was ACKed 
  recently
  on sssd-devel and as mentioned before the sub-domain patches on
  sssd-devel can only be fully tested with an IPA server which has 
  these
  patches applied.
 
  Since Alexander is currently rewriting parts of the 
  ipa-adtrust-install
  utility I stand back from adding activation code for the exop to
  ipa-adtrust-install and will send a patch when Alexander's changes 
  are
  available. So currently extdom-extop-conf.ldif has to be loaded 
  manually
  after replacing $SUFFIX to activate the new exop.
 
  bye,
  Sumit
 
  Please find a rebased version of the patches which work on top of
  Alexander's latest series of patches. The patches now also contain 
  the
  loading of extdom-extop-conf.ldif and the activation of winbind.
  Thanks for the rebase.
 
  Few comments.
 
  1.The extdom plugin should support IDMAP_BOTH. We do provide user 
  private
  groups so in our case it should be viewed as preferred output. Thus 
  you
  would need to add new response type to cover this case.
 
  Currently the plugin only uses winbind to map SIDs to names and back 
  and
  in the returned user data the user private groups are already 
  respected
  by setting the GID to the UID. On the client side sssd handles the
  trusted domains a mpg (magic private group) domains.
 
 
  2. I have tried to look at the plugin description from point of view 
  of
  a system administrator and I failed to understand what it does:
  +#define IPA_EXTDOM_PLUGIN_NAME   ipa-extdom-extop
  +#define IPA_EXTDOM_FEATURE_DESC  IPA EXTDOM ID mapper
  +#define IPA_EXTDOM_PLUGIN_DESC   IPA EXTDOM ID mapper Extended
  Operation plugin
 
  In the ipa-extdom-extop-conf.ldif you have following description:
  +nsslapd-plugindescription: Support resolving IDs in trusted 
  domains to
  names and back
  Probably it is better to reuse the same description in
  IPA_EXTDOM_PLUGIN_DESC?
 
  This is a minor point but EXTDOM itself is vague. Maybe we should be
  more clear
  and call it 'IPA trusted domain ID mapper' as it really limits 
  itself to
  only trusted domains? We don't dispatch winbind request if the 
  domain is
  not found in our list of trusted domains.
 
  I have updated the descriptions. I prefer the EXTDOM prefix because
  there might be future use cases where we might want to get some data
  from other domains without trust. But I'm happy to change it if you 
  like
  a different prefix better.
 
 
  3. Could you please define the oid in ipa_extdom.h so that it could 
  be
  useful for client code as well?
  +#define EXOP_EXTDOM_OID 2.16.840.1.113730.3.8.10.4
 
  done
 
  New version attached.
 
  ah. sorry, forgot to squash in some changes.
 
  Additionally I moved the binary to the freeipa-server-trust-ad package
  to avoid additional dependencies in the freeipa-server package.
 
  bye,
  Sumit
 
 
 
  4. Do we have 'check' tool in RHEL6?
 
  yes, current version is check-0.9.8-1.1.el6
 
  Thank you for the review.
 
  bye,
  Sumit
  --
  / Alexander Bokovoy
 
  rebased version with some changes by Alexander attached.
  ACK from my side. Works in tests I've run.
 
  Patch 17 pushed to master.
 
  Patch 18 does not apply. I also have one question related to this patch:
 
  a rebased version is attached.
 
 
  We added a winbind service to ADTRUSTInstance which is now being 
  configured as
  a part of ipa-adtrust-install. To make this cleaner, we may want to write
  winbind's own service.Service class which would do the necessary 
  configuration
  and could be also better expanded in the future.
 
  Currently none of the configuration steps are done exclusively for

Re: [Freeipa-devel] [PATCH] External group membership for trusted domains

2012-06-28 Thread Alexander Bokovoy

On Wed, 27 Jun 2012, Alexander Bokovoy wrote:

On Wed, 27 Jun 2012, Petr Viktorin wrote:

On 06/27/2012 12:36 PM, Sumit Bose wrote:

On Wed, Jun 27, 2012 at 12:56:56PM +0300, Alexander Bokovoy wrote:

On Mon, 25 Jun 2012, Alexander Bokovoy wrote:

On Mon, 25 Jun 2012, Sumit Bose wrote:

Hi Alexander,

On Thu, Jun 21, 2012 at 06:26:02PM +0300, Alexander Bokovoy wrote:

Hi!

Attached is the patch to support external group membership for trusted
domains. This is needed to get proper group membership with the work
Sumit and Jan are doing on both IPA and SSSD sides.

We already have ipaExternalGroup class that includes ipaExternalMember
attribute (multivalued case-insensitive string). The group that has
ipaExternalGroup object class will have to be non-POSIX and
ipaExternalMember
attribute will contain security identifiers (SIDs) of members from
trusted domains.

The patch takes care of three things:
1. Extends 'ipa group-add' with --external option to add
  ipaExternalGroup object class to a new group
2. Modifies 'ipa group-add-member' to accept --external CSV argument
  to specify SIDs
3. Modifies 'ipa group-del-member' to allow removing external members.


thank you for the patch, it works as expected, but I have a few
comments:

- there is a trailing whitespace at the end of the This means we can't
check the correctness of a trusted domain SIDs line
- when using ipa group-add-member with --external there are still prompt
for [member user] and [member group], can those be suppressed?
- with ipa group-mod --posix it is possible to add the posxiGroup
objectclass together with a GID to the extern group object. This
should result in an error and also the other way round, adding
--external to Posix groups.

Updated patch is attached. It fixes whitespace and group-mod.

New revision.


Thank you. This version works well in my tests, so ACK.

It would be nice if someone can have a short look at the changes to
baseldap.py to see if there are any unexpected side effects.

bye,
Sumit




I'm concerned about this:

membername = entry[0].lower()
member_dn = api.Object[membertype].get_dn(membername)
if membername not in external_entries and \
+  entry[0] not in external_entries and \
  member_dn not in members:

Do you want to do a case-insensitive compare here? In that case it 
would be better to do:


  lowercase_external_entries = set(e.lower() for e in external_entries)
  if membername not in lowercase_external_entries ...

instead of comparing the lowercased entry and the entry itself to 
the original list.

The `in` operator is also faster on a set.

Given that this list going to be short (~dozen members in most cases) it
is affordable to produce new set. I'll change it.

You should also update the `elif membername in external_entries` 
block below this one.

There's a similar situation in remove_external_post_callback.

Anyway, if you ran into a situation where the `entry[0] not in 
external_entries` check is needed, there should be a test for it.

Originally this callback was forcing all references to lower case before
comparing. This was applied both to existing and truly external
references. However, for external references we cannot guarantee that
lower case is the right one -- and, indeed, with SIDs one has to follow
SID format which is S-1-* so lowcasing the value is not possible as that
value will be used by SSSD and other sides (DCERPC requests) which don't
expect it to break the format.

Thus I tried to keep the format.

I've added several tests:
1. Create group with external membership
2. Attempt to convert posix group to external one
3. Attempt to convert external group to posix
4. Attempt to add external member to it.
5. Delete external membership group to avoid disturbing other tests
   (group-find, etc) that depend on number of groups.

In the #4 I'm only checking that we are getting exceptions --
either ValidationError or NotFound. You can't do more without setting up
the full trust.

Even to do that I had to introduce new type of checkers -- checkers that
can be activated with a 'expected' attribute being a callable in a
declarative test definition in xmlrpc tests. This is an easiest way
to deal with multiple exceptions -- just define a lambda that tests
various conditions and let it be executed by the checker.

I think something is rotten with add_external_post_callback: it's 
defined as add_external_post_callback(... *keys, **options), but 
invariably called as add_external_post_callback(... keys, 
options). That existed before the patch, though, so I guess it 
warrants a separate ticket.



I also have a few obligatory style nitpicks.

For line continuation, instead of backslashes:

   if membername not in external_entries and \
 entry[0] not in external_entries and \
 member_dn not in members:

we prefer parentheses:

   if (membername not in external_entries and
   entry[0] not in external_entries and
   member_dn not in members):

Don't shoot the 

Re: [Freeipa-devel] [PATCH] Filter groups in the PAC

2012-06-28 Thread Sumit Bose
On Wed, Jun 27, 2012 at 07:28:11PM +0300, Alexander Bokovoy wrote:
 On Tue, 26 Jun 2012, Sumit Bose wrote:
 Hi,
 
 this patch contains the KDC part of the external groups handling. If
 group SIDs from the PAC can be found in the ipaExternalGroup objects and
 the external groups are member of local groups, the SIDs of the local
 groups are added to the PAC. If the PAC this then read by the SSSD pac
 responder the user from the PAC is added to the local groups on the
 client.
 ACK. There were code-related comments from Simo yesterday on IRC but it
 was agreed to solve those in separate patches.

Thank you for the review, I created
https://fedorahosted.org/freeipa/ticket/2881 for the discussed
performance improvements.

bye,
Sumit

 
 -- 
 / Alexander Bokovoy

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


Re: [Freeipa-devel] [PATCH] External group membership for trusted domains

2012-06-28 Thread Petr Viktorin

On 06/28/2012 02:16 PM, Alexander Bokovoy wrote:

On Wed, 27 Jun 2012, Alexander Bokovoy wrote:

On Wed, 27 Jun 2012, Petr Viktorin wrote:

On 06/27/2012 12:36 PM, Sumit Bose wrote:

On Wed, Jun 27, 2012 at 12:56:56PM +0300, Alexander Bokovoy wrote:

On Mon, 25 Jun 2012, Alexander Bokovoy wrote:

On Mon, 25 Jun 2012, Sumit Bose wrote:

Hi Alexander,

On Thu, Jun 21, 2012 at 06:26:02PM +0300, Alexander Bokovoy wrote:

Hi!

Attached is the patch to support external group membership for
trusted
domains. This is needed to get proper group membership with the
work
Sumit and Jan are doing on both IPA and SSSD sides.

We already have ipaExternalGroup class that includes
ipaExternalMember
attribute (multivalued case-insensitive string). The group that has
ipaExternalGroup object class will have to be non-POSIX and
ipaExternalMember
attribute will contain security identifiers (SIDs) of members from
trusted domains.

The patch takes care of three things:
1. Extends 'ipa group-add' with --external option to add
  ipaExternalGroup object class to a new group
2. Modifies 'ipa group-add-member' to accept --external CSV
argument
  to specify SIDs
3. Modifies 'ipa group-del-member' to allow removing external
members.


thank you for the patch, it works as expected, but I have a few
comments:

- there is a trailing whitespace at the end of the This means we
can't
check the correctness of a trusted domain SIDs line
- when using ipa group-add-member with --external there are still
prompt
for [member user] and [member group], can those be suppressed?
- with ipa group-mod --posix it is possible to add the posxiGroup
objectclass together with a GID to the extern group object. This
should result in an error and also the other way round, adding
--external to Posix groups.

Updated patch is attached. It fixes whitespace and group-mod.

New revision.


Thank you. This version works well in my tests, so ACK.

It would be nice if someone can have a short look at the changes to
baseldap.py to see if there are any unexpected side effects.

bye,
Sumit




I'm concerned about this:

membername = entry[0].lower()
member_dn = api.Object[membertype].get_dn(membername)
if membername not in external_entries and \
+  entry[0] not in external_entries and \
  member_dn not in members:

Do you want to do a case-insensitive compare here? In that case it
would be better to do:

  lowercase_external_entries = set(e.lower() for e in external_entries)
  if membername not in lowercase_external_entries ...

instead of comparing the lowercased entry and the entry itself to the
original list.
The `in` operator is also faster on a set.

Given that this list going to be short (~dozen members in most cases) it
is affordable to produce new set. I'll change it.


You should also update the `elif membername in external_entries`
block below this one.
There's a similar situation in remove_external_post_callback.

Anyway, if you ran into a situation where the `entry[0] not in
external_entries` check is needed, there should be a test for it.

Originally this callback was forcing all references to lower case before
comparing. This was applied both to existing and truly external
references. However, for external references we cannot guarantee that
lower case is the right one -- and, indeed, with SIDs one has to follow
SID format which is S-1-* so lowcasing the value is not possible as that
value will be used by SSSD and other sides (DCERPC requests) which don't
expect it to break the format.

Thus I tried to keep the format.

I've added several tests:
1. Create group with external membership
2. Attempt to convert posix group to external one
3. Attempt to convert external group to posix
4. Attempt to add external member to it.
5. Delete external membership group to avoid disturbing other tests
   (group-find, etc) that depend on number of groups.

In the #4 I'm only checking that we are getting exceptions --
either ValidationError or NotFound. You can't do more without setting up
the full trust.

Even to do that I had to introduce new type of checkers -- checkers that
can be activated with a 'expected' attribute being a callable in a
declarative test definition in xmlrpc tests. This is an easiest way
to deal with multiple exceptions -- just define a lambda that tests
various conditions and let it be executed by the checker.


I think something is rotten with add_external_post_callback: it's
defined as add_external_post_callback(... *keys, **options), but
invariably called as add_external_post_callback(... keys, options).
That existed before the patch, though, so I guess it warrants a
separate ticket.


I also have a few obligatory style nitpicks.

For line continuation, instead of backslashes:

   if membername not in external_entries and \
 entry[0] not in external_entries and \
 member_dn not in members:

we prefer parentheses:

   if (membername not in external_entries and
   entry[0] not in external_entries and
   member_dn 

Re: [Freeipa-devel] [PATCH] External group membership for trusted domains

2012-06-28 Thread Alexander Bokovoy

On Thu, 28 Jun 2012, Petr Viktorin wrote:

On 06/28/2012 02:16 PM, Alexander Bokovoy wrote:

On Wed, 27 Jun 2012, Alexander Bokovoy wrote:

On Wed, 27 Jun 2012, Petr Viktorin wrote:

On 06/27/2012 12:36 PM, Sumit Bose wrote:

On Wed, Jun 27, 2012 at 12:56:56PM +0300, Alexander Bokovoy wrote:

On Mon, 25 Jun 2012, Alexander Bokovoy wrote:

On Mon, 25 Jun 2012, Sumit Bose wrote:

Hi Alexander,

On Thu, Jun 21, 2012 at 06:26:02PM +0300, Alexander Bokovoy wrote:

Hi!

Attached is the patch to support external group membership for
trusted
domains. This is needed to get proper group membership with the
work
Sumit and Jan are doing on both IPA and SSSD sides.

We already have ipaExternalGroup class that includes
ipaExternalMember
attribute (multivalued case-insensitive string). The group that has
ipaExternalGroup object class will have to be non-POSIX and
ipaExternalMember
attribute will contain security identifiers (SIDs) of members from
trusted domains.

The patch takes care of three things:
1. Extends 'ipa group-add' with --external option to add
 ipaExternalGroup object class to a new group
2. Modifies 'ipa group-add-member' to accept --external CSV
argument
 to specify SIDs
3. Modifies 'ipa group-del-member' to allow removing external
members.


thank you for the patch, it works as expected, but I have a few
comments:

- there is a trailing whitespace at the end of the This means we
can't
check the correctness of a trusted domain SIDs line
- when using ipa group-add-member with --external there are still
prompt
for [member user] and [member group], can those be suppressed?
- with ipa group-mod --posix it is possible to add the posxiGroup
objectclass together with a GID to the extern group object. This
should result in an error and also the other way round, adding
--external to Posix groups.

Updated patch is attached. It fixes whitespace and group-mod.

New revision.


Thank you. This version works well in my tests, so ACK.

It would be nice if someone can have a short look at the changes to
baseldap.py to see if there are any unexpected side effects.

bye,
Sumit




I'm concerned about this:

   membername = entry[0].lower()
   member_dn = api.Object[membertype].get_dn(membername)
   if membername not in external_entries and \
+  entry[0] not in external_entries and \
 member_dn not in members:

Do you want to do a case-insensitive compare here? In that case it
would be better to do:

 lowercase_external_entries = set(e.lower() for e in external_entries)
 if membername not in lowercase_external_entries ...

instead of comparing the lowercased entry and the entry itself to the
original list.
The `in` operator is also faster on a set.

Given that this list going to be short (~dozen members in most cases) it
is affordable to produce new set. I'll change it.


You should also update the `elif membername in external_entries`
block below this one.
There's a similar situation in remove_external_post_callback.

Anyway, if you ran into a situation where the `entry[0] not in
external_entries` check is needed, there should be a test for it.

Originally this callback was forcing all references to lower case before
comparing. This was applied both to existing and truly external
references. However, for external references we cannot guarantee that
lower case is the right one -- and, indeed, with SIDs one has to follow
SID format which is S-1-* so lowcasing the value is not possible as that
value will be used by SSSD and other sides (DCERPC requests) which don't
expect it to break the format.

Thus I tried to keep the format.

I've added several tests:
1. Create group with external membership
2. Attempt to convert posix group to external one
3. Attempt to convert external group to posix
4. Attempt to add external member to it.
5. Delete external membership group to avoid disturbing other tests
  (group-find, etc) that depend on number of groups.

In the #4 I'm only checking that we are getting exceptions --
either ValidationError or NotFound. You can't do more without setting up
the full trust.

Even to do that I had to introduce new type of checkers -- checkers that
can be activated with a 'expected' attribute being a callable in a
declarative test definition in xmlrpc tests. This is an easiest way
to deal with multiple exceptions -- just define a lambda that tests
various conditions and let it be executed by the checker.


I think something is rotten with add_external_post_callback: it's
defined as add_external_post_callback(... *keys, **options), but
invariably called as add_external_post_callback(... keys, options).
That existed before the patch, though, so I guess it warrants a
separate ticket.


I also have a few obligatory style nitpicks.

For line continuation, instead of backslashes:

  if membername not in external_entries and \
entry[0] not in external_entries and \
member_dn not in members:

we prefer parentheses:

  if (membername not in external_entries and
  entry[0] not in 

Re: [Freeipa-devel] [PATCH] Per-domain DNS update permissions

2012-06-28 Thread Petr Viktorin

On 06/28/2012 12:53 PM, Martin Kosek wrote:

On 06/28/2012 11:20 AM, Petr Viktorin wrote:

On 06/27/2012 06:01 PM, Petr Viktorin wrote:

On 06/27/2012 02:50 PM, Martin Kosek wrote:

On 06/25/2012 08:50 PM, Rob Crittenden wrote:

Simo Sorce wrote:

On Fri, 2012-06-22 at 14:25 +0200, Martin Kosek wrote:

On 06/22/2012 02:23 PM, Simo Sorce wrote:

On Fri, 2012-06-22 at 12:20 +0200, Martin Kosek wrote:

On 06/18/2012 05:37 PM, Rob Crittenden wrote:

Martin Kosek wrote:

On Fri, 2012-06-15 at 10:15 -0400, Simo Sorce wrote:

On Fri, 2012-06-15 at 15:22 +0200, Martin Kosek wrote:

Hello all,

In a scope of ticket 2511 I would like to implement an
ability to
delegate a DNS update permissions to chosen user (or host)
without
having to give the user full Update DNS Entries privileges,
i.e.
allow
him to modify any DNS zone or record.

So far, this is what I would like to do (comments welcome):

1) Create new objectclass idnsManagedZone with managedBy
attribute
in MAY list
2) Create new DNS commands:
  a] dnszone-add-managedby [--users=USERS] [--hosts=HOSTS]
  b] dnszone-remove-managedby [--users=USERS] [--hosts=HOSTS]
  - these commands would add/remove chosen user/host DN to
managedBy
attribute in chosen DNS zone
3) Add new generic ACIs to cn=dns,$SUFFIX:
aci: (target = ldap:///idnsname=*,cn=dns,$SUFFIX;)(version
3.0;acl
Users and hosts can add DNS entries;allow (add) userattr =
parent[1].managedby#USERDN;)
... add similar ACIs for UPDATE, REMOVE access

With these steps done, all that an administrator would need
to do to
delegate a management of a DNS zone example.com is to run this
command:
$ ipa dnszone-add-managedby example.com --users=fbar

The only downside I found so far is that the user would
already need to
have Read DNS Entries permission assigned, otherwise he
would not be
able to actually read DNS entries (allow rules can't take
precedence
over deny rule we implemented to deny public access to DNS
tree).

An admin could of course create a special privilege and role
with just
Read DNS Entries permission and then assign it to relevant
users/groups, but this looks awkward. Any idea to make this
simpler?
Maybe creating a group dns readers by default which would
allow such
access?


Change the deny rule to deny to everyone except the user in
parent[1].managedby#USERDN ?

Simo.



Good idea, I will do that. I will just use
parent[0,1].managedby#USERDN so that user can also read the zone
record. This way, a selected user will have read/write access
to the
chosen zone only, which is exactly what we want to achieve.


Yes, this sounds workable to me too.

rob



There were some second thoughts about the proposed design, which
I would
like to discuss so that we can eventually accept another (better)
solution for this feature.

The main concern here was that proposed solution (based on user
list in
managedBy attribute in DNS zone) is not in line with the rest of
permissionprivilege architecture in IPA.

Here is another idea how to address the feature (I tested it and it
would work):
1) Get rid of the deny rule on cn=dns,$SUFFIX by modifying global
access
rule (a working patch attached) to avoid current and future
issues with
extending ACIs (deny rules are evil).

2) Add new Managed Entry Definition and Template to automatically
add
Manage DNS zone $idsname permission. These could be used with
standard
IPA privileges, roles and thus could be assigned to users, groups,
hosts, hostgroups...

3) New DNS zone managedBy attribute won't be manageable by user,
but it
will hold a DN of the managed Permission entry

4) Add the following ACIs to cn=dns,$SUFFIX:
aci: (targetattr = *)
(version 3.0; acl Read DNS entries; allow (read,search,compare)
userattr = parent[0,1].managedby#GROUPDN;)

aci: (target = ldap:///idnsname=*,cn=dns,$SUFFIX;)
(version 3.0;acl Add dns entries;allow (add)
userattr = parent[1].managedby#GROUPDN;)

aci: (target = ldap:///idnsname=*,cn=dns,$SUFFIX;)
(version 3.0;acl Remove DNS entries;allow (delete)
userattr = parent[1].managedby#GROUPDN;)

aci: (targetattr = idnsname || cn || idnsallowdynupdate ||
dnsttl ||
dnsclass || arecord || record || a6record || nsrecord ||
cnamerecord
|| ptrrecord || srvrecord || txtrecord || mxrecord || mdrecord   ||
hinforecord || minforecord || afsdbrecord || sigrecord ||
keyrecord ||
locrecord || nxtrecord || naptrrecord || kxrecord ||
certrecord ||
dnamerecord || dsrecord || sshfprecord || rrsigrecord ||
nsecrecord ||
idnsname || idnszoneactive || idnssoamname || idnssoarname ||
idnssoaserial || idnssoarefresh || idnssoaretry || idnssoaexpire ||
idnssoaminimum || idnsupdatepolicy || idnsallowquery ||
idnsallowtransfer || idnsallowsyncptr || idnsforwardpolicy ||
idnsforwarders)
(target = ldap:///idnsname=*,cn=dns,$SUFFIX;)(version 3.0;acl
Update
DNS Entries;allow (write) userattr =
parent[0,1].managedby#GROUPDN;)

I needed to add permission DN to the managedBy attribute so that
I could
create just one set of generic ACIs without having to create a
set 

Re: [Freeipa-devel] [PATCH] Per-domain DNS update permissions

2012-06-28 Thread Martin Kosek
On 06/28/2012 03:20 PM, Petr Viktorin wrote:
 On 06/28/2012 12:53 PM, Martin Kosek wrote:
 On 06/28/2012 11:20 AM, Petr Viktorin wrote:
 On 06/27/2012 06:01 PM, Petr Viktorin wrote:
 On 06/27/2012 02:50 PM, Martin Kosek wrote:
 On 06/25/2012 08:50 PM, Rob Crittenden wrote:
 Simo Sorce wrote:
 On Fri, 2012-06-22 at 14:25 +0200, Martin Kosek wrote:
 On 06/22/2012 02:23 PM, Simo Sorce wrote:
 On Fri, 2012-06-22 at 12:20 +0200, Martin Kosek wrote:
 On 06/18/2012 05:37 PM, Rob Crittenden wrote:
 Martin Kosek wrote:
 On Fri, 2012-06-15 at 10:15 -0400, Simo Sorce wrote:
 On Fri, 2012-06-15 at 15:22 +0200, Martin Kosek wrote:
 Hello all,

 In a scope of ticket 2511 I would like to implement an
 ability to
 delegate a DNS update permissions to chosen user (or host)
 without
 having to give the user full Update DNS Entries privileges,
 i.e.
 allow
 him to modify any DNS zone or record.

 So far, this is what I would like to do (comments welcome):

 1) Create new objectclass idnsManagedZone with managedBy
 attribute
 in MAY list
 2) Create new DNS commands:
   a] dnszone-add-managedby [--users=USERS] [--hosts=HOSTS]
   b] dnszone-remove-managedby [--users=USERS] [--hosts=HOSTS]
   - these commands would add/remove chosen user/host DN to
 managedBy
 attribute in chosen DNS zone
 3) Add new generic ACIs to cn=dns,$SUFFIX:
 aci: (target = ldap:///idnsname=*,cn=dns,$SUFFIX;)(version
 3.0;acl
 Users and hosts can add DNS entries;allow (add) userattr =
 parent[1].managedby#USERDN;)
 ... add similar ACIs for UPDATE, REMOVE access

 With these steps done, all that an administrator would need
 to do to
 delegate a management of a DNS zone example.com is to run this
 command:
 $ ipa dnszone-add-managedby example.com --users=fbar

 The only downside I found so far is that the user would
 already need to
 have Read DNS Entries permission assigned, otherwise he
 would not be
 able to actually read DNS entries (allow rules can't take
 precedence
 over deny rule we implemented to deny public access to DNS
 tree).

 An admin could of course create a special privilege and role
 with just
 Read DNS Entries permission and then assign it to relevant
 users/groups, but this looks awkward. Any idea to make this
 simpler?
 Maybe creating a group dns readers by default which would
 allow such
 access?

 Change the deny rule to deny to everyone except the user in
 parent[1].managedby#USERDN ?

 Simo.


 Good idea, I will do that. I will just use
 parent[0,1].managedby#USERDN so that user can also read the zone
 record. This way, a selected user will have read/write access
 to the
 chosen zone only, which is exactly what we want to achieve.

 Yes, this sounds workable to me too.

 rob


 There were some second thoughts about the proposed design, which
 I would
 like to discuss so that we can eventually accept another (better)
 solution for this feature.

 The main concern here was that proposed solution (based on user
 list in
 managedBy attribute in DNS zone) is not in line with the rest of
 permissionprivilege architecture in IPA.

 Here is another idea how to address the feature (I tested it and it
 would work):
 1) Get rid of the deny rule on cn=dns,$SUFFIX by modifying global
 access
 rule (a working patch attached) to avoid current and future
 issues with
 extending ACIs (deny rules are evil).

 2) Add new Managed Entry Definition and Template to automatically
 add
 Manage DNS zone $idsname permission. These could be used with
 standard
 IPA privileges, roles and thus could be assigned to users, groups,
 hosts, hostgroups...

 3) New DNS zone managedBy attribute won't be manageable by user,
 but it
 will hold a DN of the managed Permission entry

 4) Add the following ACIs to cn=dns,$SUFFIX:
 aci: (targetattr = *)
 (version 3.0; acl Read DNS entries; allow (read,search,compare)
 userattr = parent[0,1].managedby#GROUPDN;)

 aci: (target = ldap:///idnsname=*,cn=dns,$SUFFIX;)
 (version 3.0;acl Add dns entries;allow (add)
 userattr = parent[1].managedby#GROUPDN;)

 aci: (target = ldap:///idnsname=*,cn=dns,$SUFFIX;)
 (version 3.0;acl Remove DNS entries;allow (delete)
 userattr = parent[1].managedby#GROUPDN;)

 aci: (targetattr = idnsname || cn || idnsallowdynupdate ||
 dnsttl ||
 dnsclass || arecord || record || a6record || nsrecord ||
 cnamerecord
 || ptrrecord || srvrecord || txtrecord || mxrecord || mdrecord   ||
 hinforecord || minforecord || afsdbrecord || sigrecord ||
 keyrecord ||
 locrecord || nxtrecord || naptrrecord || kxrecord ||
 certrecord ||
 dnamerecord || dsrecord || sshfprecord || rrsigrecord ||
 nsecrecord ||
 idnsname || idnszoneactive || idnssoamname || idnssoarname ||
 idnssoaserial || idnssoarefresh || idnssoaretry || idnssoaexpire ||
 idnssoaminimum || idnsupdatepolicy || idnsallowquery ||
 idnsallowtransfer || idnsallowsyncptr || idnsforwardpolicy ||
 idnsforwarders)
 (target = ldap:///idnsname=*,cn=dns,$SUFFIX;)(version 3.0;acl
 Update
 DNS Entries;allow (write) userattr =
 

Re: [Freeipa-devel] [PATCH] 162 Web UI password is going to expire in n days notification

2012-06-28 Thread Petr Vobornik

On 06/27/2012 04:33 PM, Petr Vobornik wrote:

On 06/27/2012 03:54 AM, Endi Sukma Dewata wrote:

On 6/26/2012 9:46 AM, Petr Vobornik wrote:

This is patch is more like a draft.

I'm not sure where to display the 'password is going to expire'
notification.

I was deciding between:
1) red bold text in Web UI header
2) popup dialog after Web UI initialization

I don't like unwanted pop-up dialogs so I used first option.


If we only support 1 short message I'd prefer option #1. Some users
might not want to reset the password immediately, so they need to be
constantly reminded about the password expiration.

If the message is too long, or we want to support multiple messages (not
just for password expiration), we can show a message icon like in the
upper right corner of Fedora desktop. When you click it it will open a
dialog box listing all messages. In this dialog you can delete each
message. The icon will disappear only if there's no message left.


I didn't make a 'password reset link' because it is done in user's
detail page and a link there is right next to this notification.


I'd say the message should include a link, something like this:

Your password will expire in n days. [Reset your password].

The link is important because:

* Without the link the message doesn't tell you what to do or how
to reset the password.
* Users might not realize that the [Logged In As: user] is a link
that can bring them to their profile page.
* Even if they're in the right page, they might not know there's a
reset password link in the action panel.

The [Reset your password] link can open the Password Reset dialog for
the current user, regardless of the current page. To avoid confusion the
dialog probably should be changed to show the username of the user being
updated.

What do you think?



I like it.

I'll add the reset link next to the message. If we encounter more cases
we can moved it to the notification icon functionality.

I found more non-existing options used and also #2876. I'll return to
this ticket when I fix these regression.


Here's an updated version.

It displays the warning with reset link next to it. I slightly modified 
user_password_dialog to be usable alone.


--
Petr Vobornik
From d85cb42d30b27904cc1afa1ae416c78c1ca6985a Mon Sep 17 00:00:00 2001
From: Petr Vobornik pvobo...@redhat.com
Date: Tue, 26 Jun 2012 16:19:58 +0200
Subject: [PATCH] Web UI password is going to expire in n days notification

This patch adds pending password expiration notification support to Web UI. When user's password is going to expire in less or equal than configure days a bold red text 'Your password expires in N days.' and a link 'Reset your password' are shown in Web UI's header (on the left next to 'Logged in as...').

Clicking on 'Reset your password link' opens IPA.user_password_dialog. Successful reset of own password will reload user's information (whoami) and update header (it will most likely hide the warning and link).

https://fedorahosted.org/freeipa/ticket/2625
---
 install/ui/details.js  |2 +-
 install/ui/index.html  |1 +
 install/ui/ipa.css |   12 
 install/ui/ipa.js  |  111 +---
 install/ui/test/data/ipa_init.json |   57 ++-
 install/ui/user.js |   52 +
 install/ui/widget.js   |   25 +
 ipalib/plugins/internal.py |2 +
 8 files changed, 217 insertions(+), 45 deletions(-)

diff --git a/install/ui/details.js b/install/ui/details.js
index 65de284423698bd6f8288fe5f84759d56a47d1b0..618d02f573e38f2573b532440265ff934e91f8c3 100644
--- a/install/ui/details.js
+++ b/install/ui/details.js
@@ -125,7 +125,7 @@ IPA.section_builder = function(spec) {
 section_spec.entity = that.container.entity;
 section_spec.facet = that.container;
 
-if (!section_spec.label  section_spec.name) {
+if (!section_spec.label  section_spec.name  that.container.entity) {
 var obj_messages = IPA.messages.objects[that.container.entity.name];
 section_spec.label = obj_messages[section_spec.name];
 }
diff --git a/install/ui/index.html b/install/ui/index.html
index 653704b7bca323febb7c5874e24b47034f9c898e..33c0923c197d53824d79591488b18dda2ea90ad2 100644
--- a/install/ui/index.html
+++ b/install/ui/index.html
@@ -70,6 +70,7 @@
 a href=#img src=images/ipa-logo.png /img src=images/ipa-banner.png //a
 /span
 span class=header-right
+span class=header-passwordexpires/span
 span id=loggedinas class=header-loggedinas
 a href=#span id=login_headerLogged in as/span: strongu...@freeipa.org/strong/a
 /span
diff --git a/install/ui/ipa.css b/install/ui/ipa.css
index c0cec89a44532e22103474910ae18110597d76ac..dac345ac39c92c30aa6b8c9754eb325c41152c28 100644
--- a/install/ui/ipa.css
+++ b/install/ui/ipa.css
@@ -240,6 

Re: [Freeipa-devel] [PATCH] External group membership for trusted domains

2012-06-28 Thread Petr Viktorin

On 06/28/2012 02:58 PM, Alexander Bokovoy wrote:

On Thu, 28 Jun 2012, Petr Viktorin wrote:

On 06/28/2012 02:16 PM, Alexander Bokovoy wrote:

On Wed, 27 Jun 2012, Alexander Bokovoy wrote:

On Wed, 27 Jun 2012, Petr Viktorin wrote:

On 06/27/2012 12:36 PM, Sumit Bose wrote:

On Wed, Jun 27, 2012 at 12:56:56PM +0300, Alexander Bokovoy wrote:

On Mon, 25 Jun 2012, Alexander Bokovoy wrote:

On Mon, 25 Jun 2012, Sumit Bose wrote:

Hi Alexander,

On Thu, Jun 21, 2012 at 06:26:02PM +0300, Alexander Bokovoy wrote:

Hi!

Attached is the patch to support external group membership for
trusted
domains. This is needed to get proper group membership with the
work
Sumit and Jan are doing on both IPA and SSSD sides.

We already have ipaExternalGroup class that includes
ipaExternalMember
attribute (multivalued case-insensitive string). The group
that has
ipaExternalGroup object class will have to be non-POSIX and
ipaExternalMember
attribute will contain security identifiers (SIDs) of members
from
trusted domains.

The patch takes care of three things:
1. Extends 'ipa group-add' with --external option to add
 ipaExternalGroup object class to a new group
2. Modifies 'ipa group-add-member' to accept --external CSV
argument
 to specify SIDs
3. Modifies 'ipa group-del-member' to allow removing external
members.


thank you for the patch, it works as expected, but I have a few
comments:

- there is a trailing whitespace at the end of the This means we
can't
check the correctness of a trusted domain SIDs line
- when using ipa group-add-member with --external there are still
prompt
for [member user] and [member group], can those be suppressed?
- with ipa group-mod --posix it is possible to add the posxiGroup
objectclass together with a GID to the extern group object. This
should result in an error and also the other way round, adding
--external to Posix groups.

Updated patch is attached. It fixes whitespace and group-mod.

New revision.


Thank you. This version works well in my tests, so ACK.

It would be nice if someone can have a short look at the changes to
baseldap.py to see if there are any unexpected side effects.

bye,
Sumit




I'm concerned about this:

   membername = entry[0].lower()
   member_dn = api.Object[membertype].get_dn(membername)
   if membername not in external_entries and \
+  entry[0] not in external_entries and \
 member_dn not in members:

Do you want to do a case-insensitive compare here? In that case it
would be better to do:

 lowercase_external_entries = set(e.lower() for e in external_entries)
 if membername not in lowercase_external_entries ...

instead of comparing the lowercased entry and the entry itself to the
original list.
The `in` operator is also faster on a set.

Given that this list going to be short (~dozen members in most
cases) it
is affordable to produce new set. I'll change it.


You should also update the `elif membername in external_entries`
block below this one.
There's a similar situation in remove_external_post_callback.

Anyway, if you ran into a situation where the `entry[0] not in
external_entries` check is needed, there should be a test for it.

Originally this callback was forcing all references to lower case
before
comparing. This was applied both to existing and truly external
references. However, for external references we cannot guarantee that
lower case is the right one -- and, indeed, with SIDs one has to follow
SID format which is S-1-* so lowcasing the value is not possible as
that
value will be used by SSSD and other sides (DCERPC requests) which
don't
expect it to break the format.

Thus I tried to keep the format.

I've added several tests:
1. Create group with external membership
2. Attempt to convert posix group to external one
3. Attempt to convert external group to posix
4. Attempt to add external member to it.
5. Delete external membership group to avoid disturbing other tests
  (group-find, etc) that depend on number of groups.

In the #4 I'm only checking that we are getting exceptions --
either ValidationError or NotFound. You can't do more without
setting up
the full trust.

Even to do that I had to introduce new type of checkers -- checkers
that
can be activated with a 'expected' attribute being a callable in a
declarative test definition in xmlrpc tests. This is an easiest way
to deal with multiple exceptions -- just define a lambda that tests
various conditions and let it be executed by the checker.


I think something is rotten with add_external_post_callback: it's
defined as add_external_post_callback(... *keys, **options), but
invariably called as add_external_post_callback(... keys, options).
That existed before the patch, though, so I guess it warrants a
separate ticket.


I also have a few obligatory style nitpicks.

For line continuation, instead of backslashes:

  if membername not in external_entries and \
entry[0] not in external_entries and \
member_dn not in members:

we prefer parentheses:

  if (membername not in 

Re: [Freeipa-devel] [PATCH] External group membership for trusted domains

2012-06-28 Thread Martin Kosek
On 06/28/2012 04:50 PM, Petr Viktorin wrote:
 On 06/28/2012 02:58 PM, Alexander Bokovoy wrote:
 On Thu, 28 Jun 2012, Petr Viktorin wrote:
 On 06/28/2012 02:16 PM, Alexander Bokovoy wrote:
 On Wed, 27 Jun 2012, Alexander Bokovoy wrote:
 On Wed, 27 Jun 2012, Petr Viktorin wrote:
 On 06/27/2012 12:36 PM, Sumit Bose wrote:
 On Wed, Jun 27, 2012 at 12:56:56PM +0300, Alexander Bokovoy wrote:
 On Mon, 25 Jun 2012, Alexander Bokovoy wrote:
 On Mon, 25 Jun 2012, Sumit Bose wrote:
 Hi Alexander,

 On Thu, Jun 21, 2012 at 06:26:02PM +0300, Alexander Bokovoy wrote:
 Hi!

 Attached is the patch to support external group membership for
 trusted
 domains. This is needed to get proper group membership with the
 work
 Sumit and Jan are doing on both IPA and SSSD sides.

 We already have ipaExternalGroup class that includes
 ipaExternalMember
 attribute (multivalued case-insensitive string). The group
 that has
 ipaExternalGroup object class will have to be non-POSIX and
 ipaExternalMember
 attribute will contain security identifiers (SIDs) of members
 from
 trusted domains.

 The patch takes care of three things:
 1. Extends 'ipa group-add' with --external option to add
  ipaExternalGroup object class to a new group
 2. Modifies 'ipa group-add-member' to accept --external CSV
 argument
  to specify SIDs
 3. Modifies 'ipa group-del-member' to allow removing external
 members.

 thank you for the patch, it works as expected, but I have a few
 comments:

 - there is a trailing whitespace at the end of the This means we
 can't
 check the correctness of a trusted domain SIDs line
 - when using ipa group-add-member with --external there are still
 prompt
 for [member user] and [member group], can those be suppressed?
 - with ipa group-mod --posix it is possible to add the posxiGroup
 objectclass together with a GID to the extern group object. This
 should result in an error and also the other way round, adding
 --external to Posix groups.
 Updated patch is attached. It fixes whitespace and group-mod.
 New revision.

 Thank you. This version works well in my tests, so ACK.

 It would be nice if someone can have a short look at the changes to
 baseldap.py to see if there are any unexpected side effects.

 bye,
 Sumit



 I'm concerned about this:

membername = entry[0].lower()
member_dn = api.Object[membertype].get_dn(membername)
if membername not in external_entries and \
 +  entry[0] not in external_entries and \
  member_dn not in members:

 Do you want to do a case-insensitive compare here? In that case it
 would be better to do:

  lowercase_external_entries = set(e.lower() for e in external_entries)
  if membername not in lowercase_external_entries ...

 instead of comparing the lowercased entry and the entry itself to the
 original list.
 The `in` operator is also faster on a set.
 Given that this list going to be short (~dozen members in most
 cases) it
 is affordable to produce new set. I'll change it.

 You should also update the `elif membername in external_entries`
 block below this one.
 There's a similar situation in remove_external_post_callback.

 Anyway, if you ran into a situation where the `entry[0] not in
 external_entries` check is needed, there should be a test for it.
 Originally this callback was forcing all references to lower case
 before
 comparing. This was applied both to existing and truly external
 references. However, for external references we cannot guarantee that
 lower case is the right one -- and, indeed, with SIDs one has to follow
 SID format which is S-1-* so lowcasing the value is not possible as
 that
 value will be used by SSSD and other sides (DCERPC requests) which
 don't
 expect it to break the format.

 Thus I tried to keep the format.

 I've added several tests:
 1. Create group with external membership
 2. Attempt to convert posix group to external one
 3. Attempt to convert external group to posix
 4. Attempt to add external member to it.
 5. Delete external membership group to avoid disturbing other tests
   (group-find, etc) that depend on number of groups.

 In the #4 I'm only checking that we are getting exceptions --
 either ValidationError or NotFound. You can't do more without
 setting up
 the full trust.

 Even to do that I had to introduce new type of checkers -- checkers
 that
 can be activated with a 'expected' attribute being a callable in a
 declarative test definition in xmlrpc tests. This is an easiest way
 to deal with multiple exceptions -- just define a lambda that tests
 various conditions and let it be executed by the checker.

 I think something is rotten with add_external_post_callback: it's
 defined as add_external_post_callback(... *keys, **options), but
 invariably called as add_external_post_callback(... keys, options).
 That existed before the patch, though, so I guess it warrants a
 separate ticket.


 I also have a few obligatory style nitpicks.

 For line continuation, instead of backslashes:

   if membername not in 

[Freeipa-devel] [PATCH] [WIP] 281 Enable SOA serial autoincrement

2012-06-28 Thread Martin Kosek
This patch enables currently developed SOA serial autoincrement feature in
bind-dyndb-ldap. The patch may be updated if any assumptions about this feature
are changed (or somebody finds a bug).

---

SOA serial autoincrement is a requirement for major DNS features,
e.g. zone transfers or DNSSEC. Enable it by default in named.conf
both for new and upgraded installations. Name of the bind-dyndb-ldap
option is serial_autoincrement.

From now on, idnsSOAserial attribute also has to be put to
replication agreement exclude list as serial will be incremented
on each DNS server separately and won't be shared. Exclude list
has to be updated both for new replication agreements and the
current ones.

https://fedorahosted.org/freeipa/ticket/2554
From 52f37bef0bc4c31919397026785818519f053a3e Mon Sep 17 00:00:00 2001
From: Martin Kosek mko...@redhat.com
Date: Thu, 28 Jun 2012 16:46:48 +0200
Subject: [PATCH] Enable SOA serial autoincrement

SOA serial autoincrement is a requirement for major DNS features,
e.g. zone transfers or DNSSEC. Enable it by default in named.conf
both for new and upgraded installations. Name of the bind-dyndb-ldap
option is serial_autoincrement.

From now on, idnsSOAserial attribute also has to be put to
replication agreement exclude list as serial will be incremented
on each DNS server separately and won't be shared. Exclude list
has to be updated both for new replication agreements and the
current ones.

https://fedorahosted.org/freeipa/ticket/2554
---
 install/share/bind.named.conf.template|1 +
 install/tools/ipa-dns-install |   10 +++-
 install/tools/ipa-server-install  |   12 -
 install/tools/ipa-upgradeconfig   |   57 +++--
 install/tools/man/ipa-dns-install.1   |5 +-
 install/tools/man/ipa-server-install.1|5 +-
 ipalib/plugins/dns.py |   11 +++-
 ipaserver/install/bindinstance.py |   11 ++--
 ipaserver/install/plugins/fix_replica_memberof.py |   47 ++---
 ipaserver/install/replication.py  |2 +-
 10 files changed, 129 insertions(+), 32 deletions(-)

diff --git a/install/share/bind.named.conf.template b/install/share/bind.named.conf.template
index f133b089a9eb428e9ad76b66a3ff162b45e5a779..9fdd91319947f6cfd3034f8d2a4fe8bb60d1af77 100644
--- a/install/share/bind.named.conf.template
+++ b/install/share/bind.named.conf.template
@@ -46,4 +46,5 @@ dynamic-db ipa {
 	arg sasl_user DNS/$FQDN;
 	arg zone_refresh $ZONE_REFRESH;
 	arg psearch $PERSISTENT_SEARCH;
+	arg serial_autoincrement $SERIAL_AUTOINCREMENT;
 };
diff --git a/install/tools/ipa-dns-install b/install/tools/ipa-dns-install
index 6e9b9989792aba6f7607348da4693cf605dc0b76..47bffdf8354fa509d64af0ba0e15d5880010e425 100755
--- a/install/tools/ipa-dns-install
+++ b/install/tools/ipa-dns-install
@@ -62,6 +62,9 @@ def parse_options():
   default=0, type=int,
   help=When set to non-zero the name server will use DNS zone 
detection based on polling instead of a persistent search)
+parser.add_option(--no-serial-autoincrement, dest=serial_autoincrement,
+  default=True, action=store_false,
+  help=Do not enable SOA serial autoincrement)
 parser.add_option(-U, --unattended, dest=unattended, action=store_true,
   default=False, help=unattended installation never prompts the user)
 
@@ -85,6 +88,10 @@ def parse_options():
 if options.zone_notif:
 print sys.stderr, WARNING: --zone-notif option is deprecated and has no effect
 
+if options.serial_autoincrement and not options.persistent_search:
+parser.error('persistent search feature is required for '
+ 'DNS SOA serial autoincrement')
+
 return safe_options, options
 
 def main():
@@ -224,7 +231,8 @@ def main():
 bind.setup(api.env.host, ip_address, api.env.realm, api.env.domain,
dns_forwarders, conf_ntp, reverse_zone, zonemgr=options.zonemgr,
zone_refresh=options.zone_refresh,
-   persistent_search=options.persistent_search)
+   persistent_search=options.persistent_search,
+   serial_autoincrement=options.serial_autoincrement)
 bind.create_instance()
 
 # Restart http instance to make sure that python-dns has the right resolver
diff --git a/install/tools/ipa-server-install b/install/tools/ipa-server-install
index 6dc02f684e0b6fc21f150eadef1c286f2a326233..2c409eacba18f809d8254876ecebafc27fe931b4 100755
--- a/install/tools/ipa-server-install
+++ b/install/tools/ipa-server-install
@@ -210,7 +210,10 @@ def parse_options():
   default=False,
   help=Do not use DNS for hostname lookup during installation)
 dns_group.add_option(--no-dns-sshfp, dest=create_sshfp, default=True, action=store_false,
-  

[Freeipa-devel] test_changepw is failing on master

2012-06-28 Thread John Dennis
tests/test_ipaserver/test_changepw.py is failing on master. Could 
someone who is familiar with the code take a look and see what's wrong.


Thanks,

John

== 




FAIL: tests.test_ipaserver.test_changepw.test_changepw.test_invalid_auth 




-- 




Traceback (most recent call last): 




  File /usr/lib/python2.7/site-packages/nose/case.py, line 197, in 
runTest 



self.test(*self.arg) 




  File 
/home/jdennis/src/freeipa.ref/tests/test_ipaserver/test_changepw.py, 
line 86, in test_invalid_auth 



assert_equal(response.getheader('X-IPA-Pwchange-Result'), 
'invalid-password') 



  File /home/jdennis/src/freeipa.ref/tests/util.py, line 107, in 
assert_equal 



assert val1 == val2, '%r != %r' % (val1, val2) 




AssertionError: 'error' != 'invalid-password' 








== 




FAIL: 
tests.test_ipaserver.test_changepw.test_changepw.test_pwpolicy_error 




-- 




Traceback (most recent call last): 




  File /usr/lib/python2.7/site-packages/nose/case.py, line 197, in 
runTest 



self.test(*self.arg) 




  File 
/home/jdennis/src/freeipa.ref/tests/test_ipaserver/test_changepw.py, 
line 95, in test_pwpolicy_error 



assert_equal(response.getheader('X-IPA-Pwchange-Result'), 
'policy-error') 



  File /home/jdennis/src/freeipa.ref/tests/util.py, line 107, in 
assert_equal 



assert val1 == val2, '%r != %r' % (val1, val2) 




AssertionError: 'error' != 'policy-error' 








== 




FAIL: 
tests.test_ipaserver.test_changepw.test_changepw.test_pwpolicy_success 




-- 




Traceback (most recent call last): 




  File /usr/lib/python2.7/site-packages/nose/case.py, line 197, in 
runTest 



self.test(*self.arg) 




  File 
/home/jdennis/src/freeipa.ref/tests/test_ipaserver/test_changepw.py, 
line 106, in test_pwpolicy_success 



assert_equal(response.getheader('X-IPA-Pwchange-Result'), 'ok') 




  File /home/jdennis/src/freeipa.ref/tests/util.py, line 107, in 
assert_equal 



assert val1 == val2, '%r != %r' % (val1, val2) 




AssertionError: 'error' != 'ok' 








-- 




Ran 4 tests in 31.398s 








FAILED (failures=3) 




== 




FAILED under '/usr/bin/python2.7'

--
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


Re: [Freeipa-devel] [PATCH] 163 Refactored association facet to use facet buttons with actions

2012-06-28 Thread Endi Sukma Dewata

On 6/27/2012 11:19 AM, Petr Vobornik wrote:

Association facet was refactored to use new concept of control buttons.
It is the last facet type which don't use this concept.
It fixes regression introduced by previous refactoring of table facet
(delete button was never enabled).

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


ACK.

--
Endi S. Dewata


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


Re: [Freeipa-devel] [PATCH] 164 Continuation of removing of not supported command options from Web UI

2012-06-28 Thread Endi Sukma Dewata

On 6/27/2012 11:22 AM, Petr Vobornik wrote:

This patch removes following non-existing command options:
  * all,rights in host_disable
  * record_type in dns_record_add
  * all,rights in various xxx_remove_xxx commands used in
rule_association_table_field (removing association)

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


ACK.

--
Endi S. Dewata


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


Re: [Freeipa-devel] [PATCH] 162 Web UI password is going to expire in n days notification

2012-06-28 Thread Endi Sukma Dewata

On 6/28/2012 8:59 AM, Petr Vobornik wrote:

On 06/27/2012 04:33 PM, Petr Vobornik wrote:

On 06/27/2012 03:54 AM, Endi Sukma Dewata wrote:

On 6/26/2012 9:46 AM, Petr Vobornik wrote:

This is patch is more like a draft.

I'm not sure where to display the 'password is going to expire'
notification.

I was deciding between:
1) red bold text in Web UI header
2) popup dialog after Web UI initialization

I don't like unwanted pop-up dialogs so I used first option.


If we only support 1 short message I'd prefer option #1. Some users
might not want to reset the password immediately, so they need to be
constantly reminded about the password expiration.

If the message is too long, or we want to support multiple messages (not
just for password expiration), we can show a message icon like in the
upper right corner of Fedora desktop. When you click it it will open a
dialog box listing all messages. In this dialog you can delete each
message. The icon will disappear only if there's no message left.


I didn't make a 'password reset link' because it is done in user's
detail page and a link there is right next to this notification.


I'd say the message should include a link, something like this:

Your password will expire in n days. [Reset your password].

The link is important because:

* Without the link the message doesn't tell you what to do or how
to reset the password.
* Users might not realize that the [Logged In As: user] is a link
that can bring them to their profile page.
* Even if they're in the right page, they might not know there's a
reset password link in the action panel.

The [Reset your password] link can open the Password Reset dialog for
the current user, regardless of the current page. To avoid confusion the
dialog probably should be changed to show the username of the user being
updated.

What do you think?



I like it.

I'll add the reset link next to the message. If we encounter more cases
we can moved it to the notification icon functionality.

I found more non-existing options used and also #2876. I'll return to
this ticket when I fix these regression.


Here's an updated version.

It displays the warning with reset link next to it. I slightly modified
user_password_dialog to be usable alone.


ACK. I have some suggestions below.

The reset link probably should be blue to be consistent with the other 
links, but maybe it doesn't go well with dark background. Or you can 
make the link red (and the whole message clickable) to make sure it's 
clear that it's part of the error message, not a permanent part of the 
UI like the Logged in as. Or make the link message more explicit: 
Click here to reset your password.


When you reset the password, a confirmation dialog will appear on top of 
the password reset dialog. I think in general we should avoid nesting 
dialog. So the password dialog should be closed first then open the 
confirmation/error dialog. The password dialog can be reopened if 
something goes wrong.


Btw, the style of the confirmation dialog is different from the other 
dialog. Is this intentional?


One thing I noticed also, when you login with expired password, after 
you reset the password it will briefly show the login page again before 
showing the UI. I think the login dialog should close immediately once 
you click the button.


--
Endi S. Dewata


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


Re: [Freeipa-devel] [PATCH] 165 Display loginas information only after login

2012-06-28 Thread Endi Sukma Dewata

On 6/28/2012 9:07 AM, Petr Vobornik wrote:

Message 'Logged in as: u...@freeipa.org' was displayed before user was
logged in. It was wrong.

Now 'Logged in as: XXX' is displayed only when user XXX is logged in. So
no more u...@freeipa.org :) .

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


It might be better to use visibility instead of display to reserve the 
space. Right now the password expiration warning will initially appear 
on the right, then shift to the left when the Logged in as appears.


--
Endi S. Dewata


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