Re: [Freeipa-devel] User life cycle: question regarding the design

2014-05-22 Thread thierry bordaz

On 05/21/2014 09:06 PM, Martin Kosek wrote:

On 05/21/2014 08:14 PM, Simo Sorce wrote:

On Wed, 2014-05-21 at 16:01 +0200, thierry bordaz wrote:

Hello,

 Thanks for all these detailed descriptions.
 Just to be sure to be on the same page, here is my 
understanding of

 the provisioning templates and placeholder definitions. An
 administrator can provide a provisioning template. I suppose it
 would be a file containing a lines of placeholder definitions.

   * Where is located the template file ? Is there a standard
 repository where templates are put ? (somewhere under 
/etc/ipa/* ?)


FreeIPA is a multi-master system, a file stored in a file would be
extremely cumbersome to use as it would require the admin to manually
copy it for every new replica and then keep it in sync.
It would also make it hard to change 'on-line'.

Placeholders should be defined in an object similar to
cn=ipaConfig,cn=etc,$suffix


   * Is there an already defined syntax for the provisionning
 template. ('$' is separator attr/value, %{attr} is 
substitute

 pattern...). If not, is it possible to user ':space ' as
 separator ?


Using initial and final ? like in Martin's example doesn't work ?


   * What is the priority. The user can provide the 'homeDirectory'
 through different methods. Is it ok to use the following 
order:

   o the CLI option
   o the provisionning template
   o the default config value (in cn=ipaConfig,cn=etc,$SUFFIX)

 For example, if it exists the provisioning template:
 /etc/ipa/provisioning/shell-user.template

 roomnumber$-2
 homeDirectory$/home/net/shell-%{uid}
 loginShell$?shell-plugin-autogenerate?


I do not understand this, we are not building a templating engine here,
you only have 2 options:
1) a required (MUST) attribute has an explicit value
2) a require (MUST) attribute has a placeholder value

the placeholder value is fixed per type, and what it is substituted with
uses the same rules as the current code uses to autogenerate values.


 the command: ipa user-add tuser
 --homedir=/tmp/tuser--roomnumber=1234 --to-stage would create a
 staging entry:

 dn: uid=tuser,cn=staged users,cn=provisioning,$SUFFIX
 ...
 roomNumber: 1234
 homeDirectory: /tmp/tuser
 loginShell: shell-plugin-autogenerate


loginShell is a MAY attribute, not a MUST attribute, so nothing should
be stored at all in the staged entry unless explicitly provided for by
the admin.


 Then a private DS plugin (catching shell-plugin-autogenerate)
 generate the loginShell value when the entry becomes active.


 the command: ipa user-add tuser --homedir=/tmp/tuser--to-stage 
would

 create a staging entry:

 dn: uid=tuser,cn=staged users,cn=provisioning,$SUFFIX
 ...
 roomNumber: -2
 homeDirectory: /tmp/tuser
 loginShell: shell-plugin-autogenerate


roomNumber is also a MAY, so what would cause it to be set at -2, and
why ?

 the command: ipa user-add tuser --to-stage would create a 
staging entry:


 dn: uid=tuser,cn=staged users,cn=provisioning,$SUFFIX
 ...
 roomNumber: -2
 homeDirectory: /home/net/shell-tuser
 loginShell: shell-plugin-autogenerate


homeDirectory should be something like: ?placeholder? IMO, we do not
really want to play templating here.


 In case the provisioning template does not define 'homeDirectory',
 then the created entry would take the value from the ipa config
 definition:


that value should be taken and applied at the time the user is unstaged
and brought in the actual tree, not at the time a user is staged.

HTH,
Simo.



Hello Thierry and Simo,

I think Thierry was confused with this part of the design:


This format of placeholders gives enough space for future 
enhancements. For example, Administrator could configure a new 
template myhomedirtemplate$/home/net/%{uid} and use it in the staged 
LDAP entry. Such value would be replaced by /home/net/tuser if user 
uid attribute is set to tuser



My intention when writing this design was to enable future use of 
configurable placeholders, i.e. a value ?someplaceholder? could be 
turn into /custom/path/%{uid}. But I meant that this can be 
considered as a future enhancement. For now, I think implementing a 
placeholder -1 for numerical values and ?autogenerate? for string 
ones a good start.


Martin

Hello Martin and Simo,

   Thanks for your feedbacks. I liked the idea of configurable
   placeholders and I was thinking it already existed a kind of
   template engine that I had to follow. Now I understand that in  a
   first step, I have to make it run only with  '-1|?autogenerate?'
   placeholders and for MUST attribute only. Sorry for the confusion.

   Thierry

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

Re: [Freeipa-devel] [PATCH] 630 rpcserver: login_password datetime fix in expiration check

2014-05-22 Thread Tomas Babej

On 05/07/2014 04:37 PM, Petr Vobornik wrote:
 On 7.5.2014 16:30, Tomas Babej wrote:

 On 05/07/2014 04:26 PM, Petr Vobornik wrote:
 On 7.5.2014 16:01, Tomas Babej wrote:

 On 05/07/2014 03:47 PM, Petr Vobornik wrote:
 krbpasswordexpiration conversion to number of second since epoch
 failed
 because now we get datetime object instead of string.

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


 NACK, I don't think this is the right approach. This does not leverage
 the simplicity which the DateTime parameter refactoring provides.

 Instead of converting the datetime to the number of the seconds since
 epoch, and getting the current time represented by the number of
 seconds
 since the epoch (using time.time()), why not use datetime module and
 datetime.datetime.now() to get the current time?

 Then you could simplify this:

 +exp = time.mktime(expiration.timetuple())
 +if exp = time.time():

 to this:

 +if expiration = datetime.datetime.now()


 Good point, fixed


 Can you also please remove the (now) unused import for module time?

 done


ACK.

-- 
Tomas Babej
Associate Software Engineer | Red Hat | Identity Management
RHCE | Brno Site | IRC: tbabej | freeipa.org 

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


Re: [Freeipa-devel] Status/Question about User life cycle

2014-05-22 Thread Petr Viktorin

On 05/21/2014 10:00 PM, Dmitri Pal wrote:

On 05/19/2014 10:45 AM, thierry bordaz wrote:

On 05/19/2014 04:44 PM, Jan Cholasta wrote:

On 19.5.2014 16:34, thierry bordaz wrote:

On 05/19/2014 04:22 PM, Jan Cholasta wrote:

On 19.5.2014 16:03, thierry bordaz wrote:

On 05/19/2014 03:54 PM, Jan Cholasta wrote:

On 19.5.2014 15:19, Petr Viktorin wrote:

Hello list,
Here's a conversation that started internally. I'm making it
public.

On 05/19/2014 01:00 PM, Martin Kosek wrote:

On 05/19/2014 12:46 PM, Petr Viktorin wrote:

On 05/19/2014 08:25 AM, Martin Kosek wrote:

On 05/19/2014 08:24 AM, Martin Kosek wrote:

On 05/16/2014 04:48 PM, thierry bordaz wrote:

Hello Martin,

 I am getting familiar with the freeipa CLI code and
started
 implemented '--to-stage' and '--from-stage'. This
really an
 impressive set of code :-)


Great! :-)


 I completed 'to-stage' and testing '--from-stage'.

 I have a question regarding the '--from-stage' syntax.
'uid'
is a
 mandatory argument to 'user-add' subcommand. In the
design the
 '--from-stage' option is described with:

 ipa user-add --from-stage=tuser


Note, the design is here:
http://www.freeipa.org/page/V4/User_Life-Cycle_Management


 But as 'uid' is mandatory the command should rather be

 ipa user-add tuser --from-stage=tuser

 In that case the option value for '--from-stage' is not
required and
 the command should be

 ipa user-add tuser --from-stage

 Is that ok if I implement the command like above or did I
miss
 something ?

 regards
 thierry


Hmm, no, I think you are right.  We can change --from-stage to
just
Bool
parameter. When it is true, it'd mean that get_dn or
pre-callback
should
retrieve the record from stage and use all it's attributes (and
add
standard
default attributes values on top of that).

Also CC-ing Petr Viktorin for reference.


This operation can't change the user's attributes, can it?
I.e., we
don't
support something like:
 ipa user-add tuser --from-stage --phone=123456789
--email=newem...@example.com
If this is the case, what's the reason for using user-add for
this?
Wouldn't it
be better to make this a separate command, say:
 ipa user-activate tuser
 ipa user-activate tuser --from-deleted
 ipa user-activate tuser --from-deleted --to-staged


+1, I would even go as far as having separate commands for staged
and
deleted users, e.g.:

ipa user-unstage tuser
ipa user-undelete tuser
ipa user-undelete tuser --to-staged


A deleted entry has already been active so it contains already set
attributes while the pure staged entries are almost empty boxes.
But
from an administrator point of view, both staged/deleted entries are
inactive. What would be the advantages of two separated commands ?


You just said it yourself: activating/unstaging a user is quite
different from undeleting a user. Cramming multiple different
operations in a single command is bad design IMHO.


Ok I understand.
I believe that deleted entries and staged entries will be in the same
container (provisioning).


The design page mentions cn=staged
users,cn=accounts,cn=provisioning,$SUFFIX and cn=deleted
users,cn=accounts,cn=provisioning,$SUFFIX, which are two different
containers.


Oppsss.. Sorry for the confusion :-[



So we may have at least those two possibilities:

  * ipa user-activate tuser [--from-staging|--from-delete]
  * ipa user-unstage tuser
ipa user-undelete tuser



I like the idea of different verbs for different hives.
Something like:

Adding directly to stage via CLI: ipa user-stage
Removing from stage: user-unstage (user is gone)
Stage to Main - activate; - deactivate
Main to delete - del; -restore or undelete
Delete to stage - I think we can use ipa user-stage command with
--deleted=user or similar


To be honest, I don't like this idea.
Too many names are confusing, if we can find a consistent option to cut 
the number of names down we should do it.
IMO This is the worst part of Git: 
http://assets.osteele.com/images/2008/git-transport.png . We can do better.


Another good thing would be if options did not affect the applicability 
of other options (too much). For example in your proposal there'd be 
something like:

ipa user-stage tuser --first=abc --last=xyz --phone=123 ..
ipa user-stage --deleted=tuser  # no attribute options allowed
We should avoid this, if only for the reason that it makes the help text 
confusing.



My proposal would be that the move commands use the verb for the target 
and an option for the source, and add/mod use an option for the container:


1) adding a new user
(to active)   ipa user-add tuser ...
(to stage)ipa user-add tuser --staged ...
(to deleted)  ipa user-add tuser --deleted ...  (*)

2) moving to main
(from stage)  ipa user-activate tuser  (**)
(from del)ipa user-activate tuser --deleted

3) moving to deleted
(from active) ipa user-del tuser
(from stage)  ipa user-del tuser --staged

4) 

[Freeipa-devel] [PATCH] 0550 ipalib.cli: Add filename argument to ipa console

2014-05-22 Thread Petr Viktorin

Hello,
I find the `ipa console` command quite useful for testing, and it's 
bothered me that it can't execute a script. Fixing this helps me. Would 
it help anyone else?


This would need a ticket + design doc before it's pushed.


Compare:
$ (echo 'print 1'; echo 'print 2') | ipa console
(Custom IPA interactive Python console)
 1
 2

 [no newline]

$ ipa console (echo 'print 1'; echo 'print 2')
1
2


--
Petr³
From 32038c91e005f7d66926f887e6a21bb74350b897 Mon Sep 17 00:00:00 2001
From: Petr Viktorin pvikt...@redhat.com
Date: Thu, 15 May 2014 15:42:48 +0200
Subject: [PATCH] ipalib.cli: Add filename argument to ipa console

This allows writing simple IPA scripts using the shebang
#! /usr/bin/ipa console
---
 ipalib/cli.py | 31 +--
 1 file changed, 25 insertions(+), 6 deletions(-)

diff --git a/ipalib/cli.py b/ipalib/cli.py
index f03db9c612b6e36e0ceda62a5e15c08261950dbe..ea47c7bb080fc30342cfb99f5bf5484b03929983 100644
--- a/ipalib/cli.py
+++ b/ipalib/cli.py
@@ -30,6 +30,7 @@
 import termios
 import struct
 import base64
+import traceback
 try:
 #pylint: disable=F0401
 import default_encoding_utf8
@@ -875,15 +876,33 @@ def run(self, command_name, **options):
 
 
 class console(frontend.Command):
-Start the IPA interactive Python console.
+Start the IPA interactive Python console, or run a script.
 
+An IPA API object is initialized and made available
+in the `api` global variable.
+
+
+takes_args = ('filename?',)
 has_output = tuple()
 
-def run(self, **options):
-code.interact(
-'(Custom IPA interactive Python console)',
-local=dict(api=self.api)
-)
+def run(self, filename=None, **options):
+local = dict(api=self.api)
+if filename:
+try:
+script = open(filename)
+except IOError as e:
+exit(%s: %s % (e.filename, e.strerror))
+try:
+with script:
+exec(script, globals(), local)
+except:
+traceback.print_exc()
+exit(1)
+else:
+code.interact(
+'(Custom IPA interactive Python console)',
+local=local
+)
 
 
 class show_api(frontend.Command):
-- 
1.9.0

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

[Freeipa-devel] [PATCH] 0551 ldap2.find_entries: Do not modify attrs_list in-place

2014-05-22 Thread Petr Viktorin

This fixes https://fedorahosted.org/freeipa/ticket/4349.

See the ticket for a description.

--
Petr³
From 423a7337dcd10cc88b2fb90872923bb21ada4713 Mon Sep 17 00:00:00 2001
From: Petr Viktorin pvikt...@redhat.com
Date: Fri, 16 May 2014 13:18:36 +0200
Subject: [PATCH] ldap2.find_entries: Do not modify attrs_list in-place

dap2.find_entries modified the passed in attrs_list to remove
the virtual attributes memberindirect and memberofindirect
before passing the list to LDAP. This means that a call like
ldap2.get_entry(dn, attrs_list=some_framework_object.default_attributes)
would permanently remove the virtual attributes from
some_framework_object's definition.

Create a copy of the list instead.

https://fedorahosted.org/freeipa/ticket/4349
---
 ipaserver/plugins/ldap2.py | 15 +--
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/ipaserver/plugins/ldap2.py b/ipaserver/plugins/ldap2.py
index 17bd8411804b6d2bb89c61e9386f254a2c56d5f0..03ab2dbfe953440f9279137db30eb92d2606e18b 100644
--- a/ipaserver/plugins/ldap2.py
+++ b/ipaserver/plugins/ldap2.py
@@ -186,12 +186,15 @@ def find_entries(self, filter=None, attrs_list=None, base_dn=None,
 has_memberindirect = False
 has_memberofindirect = False
 if attrs_list:
-if 'memberindirect' in attrs_list:
-has_memberindirect = True
-attrs_list.remove('memberindirect')
-if 'memberofindirect' in attrs_list:
-has_memberofindirect = True
-attrs_list.remove('memberofindirect')
+new_attrs_list = []
+for attr_name in attrs_list:
+if attr_name == 'memberindirect':
+has_memberindirect = True
+elif attr_name == 'memberofindirect':
+has_memberofindirect = True
+else:
+new_attrs_list.append(attr_name)
+attrs_list = new_attrs_list
 
 res, truncated = super(ldap2, self).find_entries(
 filter=filter, attrs_list=attrs_list, base_dn=base_dn, scope=scope,
-- 
1.9.0

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

[Freeipa-devel] [PATCHES] 0552-0554 Upgrading write permissions

2014-05-22 Thread Petr Viktorin

Hello,
Here I start upgrading  the existing default permissions to the new 
Managed style.


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

The patches rely on my patch 0551 
(https://fedorahosted.org/freeipa/ticket/4349)
You may run into what seems to be a 389 bug. If you get a Midair 
Collision (NO_SUCH_ATTRIBUTE) error, restart the DS and try running 
ipa-ldap-updater again. I'm working with Ludwig on this one.




The operation is now described at 
http://www.freeipa.org/page/V4/Managed_Read_permissions#Replacing_legacy_default_permissions


If there user has modified an old default permission, a warning is 
logged the replacement permission is not added/updated. The user needs 
to evaluate the situation: either update the old permission to match the 
original default, or remove the old permission, and then run 
ipa-ldap-updater will create the new one.

Is bailing out the right thing to do if the old entry was modified?
It could be possible to parse the permission, figure out the changes the 
user made, and apply them to the new one, but that seems like too much 
guesswork to me.
On the other hand, my approach has a downside as well: if the 
'memberallowcmd' attribute was removed from 'Modify Sudo rule', there's 
now no way to upgrade while allowing access but keeping that attribute 
off-limits, short of writing deny a ACI by hand. How big a problem is 
this? It might be worth it to create a special tool that upgrades a 
single permission and allows setting the excluded/included attributes 
explicitly.




There are some interesting scenarios to think about with respect to 
upgrades and user changes:


* Upgrade to old version, e.g.
  - have IPA 3.2 master, IPA 3.2 replica
  - upgrade master to 4.0 (old permissions are updated)
  - then upgrade replica to 3.3 (old permissions are added again!)

This is AFAIK not supported but it does happen.
We can't change old IPA versions, so any upgrade to a pre-4.0 IPA will 
always add the old permissions, but with this patch, a subsequent 
upgrade to 4.0+, or running a 4.0+ ipa-ldap-update, will remove the old 
permissions again.


Tied to that is another scenario:

* Re-create permissions with old names
  - have IPA 4.0 master
  - Create a permission named 'Modify Sudo rule'
  - Upgrade to IPA 4.1

Here we need to make sure the new permission is *not* removed, because a 
new 'Modify Sudo rule' permission is no longer special in any way. To 
ensure this the updater only removes old-style permissions.


One thing that can happen when 4.0 masters are still mixed with 3.x is 
that an old permission named 'Modify Sudo rule' is added on the old 
server. Any update to 4.0+ will remove that.
Old-style default permissions were sorta-kinda managed by IPA itself 
anyway, so users should expect this. We should still point it out in the 
docs though, since I expect some users to start messing with the 
permissions before upgrading all of the infrastructure to 4.0.



The second patch upgrades sudorule permissions, this server as an 
example of how the  will work.

The third patch fixes https://fedorahosted.org/freeipa/ticket/4344


--
Petr³
From c56f12a069ebcc21a292a95f00771d1a81d6a09c Mon Sep 17 00:00:00 2001
From: Petr Viktorin pvikt...@redhat.com
Date: Wed, 14 May 2014 16:08:28 +0200
Subject: [PATCH] Add mechanism for updating permissions to managed

Part of the work for: https://fedorahosted.org/freeipa/ticket/4346
---
 .../install/plugins/update_managed_permissions.py  | 69 +++---
 1 file changed, 62 insertions(+), 7 deletions(-)

diff --git a/ipaserver/install/plugins/update_managed_permissions.py b/ipaserver/install/plugins/update_managed_permissions.py
index bffd9bbf434e76c9c6d74d0167a718acc96a54b1..3fcbc8597460c7338b74111bd5fc4ba6a9af9bb4 100644
--- a/ipaserver/install/plugins/update_managed_permissions.py
+++ b/ipaserver/install/plugins/update_managed_permissions.py
@@ -64,15 +64,20 @@
 * non_object
   - If true, no object-specific defaults are used (e.g. for
 ipapermtargetfilter, ipapermlocation).
+* replaces
+  - A list of ACIs corresponding to legacy default permissions replaced
+by this permission.
 
 No other keys are allowed in the template
 
 
 from ipalib import api, errors
-from ipapython.dn import DN
+from ipalib.aci import ACI
 from ipalib.plugable import Registry
 from ipalib.plugins import aci
-from ipalib.plugins.permission import permission
+from ipalib.plugins.permission import permission, permission_del
+from ipapython import ipautil
+from ipapython.dn import DN
 from ipaserver.plugins.ldap2 import ldap2
 from ipaserver.install.plugins import LAST
 from ipaserver.install.plugins.baseupdate import PostUpdate
@@ -158,6 +163,17 @@
 }
 
 
+def get_aci_names(acistrs):
+names = []
+for acistr in acistrs:
+aci = ACI(acistr)
+prefix, sep, name = aci.name.partition(':')
+assert prefix == 'permission'
+assert sep
+names.append(name)
+return names
+
+
 @register()
 class 

Re: [Freeipa-devel] [PATCH] 0551 ldap2.find_entries: Do not modify attrs_list in-place

2014-05-22 Thread Jan Cholasta

On 22.5.2014 15:07, Petr Viktorin wrote:

This fixes https://fedorahosted.org/freeipa/ticket/4349.

See the ticket for a description.


Looks OK to me, ACK.

--
Jan Cholasta

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


Re: [Freeipa-devel] User life cycle: question regarding the design

2014-05-22 Thread Martin Kosek
On 05/21/2014 10:11 PM, Dmitri Pal wrote:
 On 05/21/2014 03:06 PM, Martin Kosek wrote:
 On 05/21/2014 08:14 PM, Simo Sorce wrote:
 On Wed, 2014-05-21 at 16:01 +0200, thierry bordaz wrote:
 Hello,

  Thanks for all these detailed descriptions.
  Just to be sure to be on the same page, here is my understanding of
  the provisioning templates and placeholder definitions. An
  administrator can provide a provisioning template. I suppose it
  would be a file containing a lines of placeholder definitions.

* Where is located the template file ? Is there a standard
  repository where templates are put ? (somewhere under /etc/ipa/* 
 ?)

 FreeIPA is a multi-master system, a file stored in a file would be
 extremely cumbersome to use as it would require the admin to manually
 copy it for every new replica and then keep it in sync.
 It would also make it hard to change 'on-line'.

 Placeholders should be defined in an object similar to
 cn=ipaConfig,cn=etc,$suffix

* Is there an already defined syntax for the provisionning
  template. ('$' is separator attr/value, %{attr} is substitute
  pattern...). If not, is it possible to user ':space ' as
  separator ?

 Using initial and final ? like in Martin's example doesn't work ?

* What is the priority. The user can provide the 'homeDirectory'
  through different methods. Is it ok to use the following order:
o the CLI option
o the provisionning template
o the default config value (in cn=ipaConfig,cn=etc,$SUFFIX)

  For example, if it exists the provisioning template:
  /etc/ipa/provisioning/shell-user.template

  roomnumber$-2
  homeDirectory$/home/net/shell-%{uid}
  loginShell$?shell-plugin-autogenerate?

 I do not understand this, we are not building a templating engine here,
 you only have 2 options:
 1) a required (MUST) attribute has an explicit value
 2) a require (MUST) attribute has a placeholder value

 the placeholder value is fixed per type, and what it is substituted with
 uses the same rules as the current code uses to autogenerate values.

  the command: ipa user-add tuser
  --homedir=/tmp/tuser--roomnumber=1234 --to-stage would create a
  staging entry:

  dn: uid=tuser,cn=staged users,cn=provisioning,$SUFFIX
  ...
  roomNumber: 1234
  homeDirectory: /tmp/tuser
  loginShell: shell-plugin-autogenerate

 loginShell is a MAY attribute, not a MUST attribute, so nothing should
 be stored at all in the staged entry unless explicitly provided for by
 the admin.

  Then a private DS plugin (catching shell-plugin-autogenerate)
  generate the loginShell value when the entry becomes active.


  the command: ipa user-add tuser --homedir=/tmp/tuser--to-stage would
  create a staging entry:

  dn: uid=tuser,cn=staged users,cn=provisioning,$SUFFIX
  ...
  roomNumber: -2
  homeDirectory: /tmp/tuser
  loginShell: shell-plugin-autogenerate

 roomNumber is also a MAY, so what would cause it to be set at -2, and
 why ?

  the command: ipa user-add tuser --to-stage would create a staging 
 entry:

  dn: uid=tuser,cn=staged users,cn=provisioning,$SUFFIX
  ...
  roomNumber: -2
  homeDirectory: /home/net/shell-tuser
  loginShell: shell-plugin-autogenerate

 homeDirectory should be something like: ?placeholder? IMO, we do not
 really want to play templating here.

  In case the provisioning template does not define 'homeDirectory',
  then the created entry would take the value from the ipa config
  definition:

 that value should be taken and applied at the time the user is unstaged
 and brought in the actual tree, not at the time a user is staged.

 HTH,
 Simo.


 Hello Thierry and Simo,

 I think Thierry was confused with this part of the design:

 
 This format of placeholders gives enough space for future enhancements. For
 example, Administrator could configure a new template
 myhomedirtemplate$/home/net/%{uid} and use it in the staged LDAP entry.
 Such value would be replaced by /home/net/tuser if user uid attribute is set
 to tuser
 

 My intention when writing this design was to enable future use of
 configurable placeholders, i.e. a value ?someplaceholder? could be turn
 into /custom/path/%{uid}. But I meant that this can be considered as a
 future enhancement. For now, I think implementing a placeholder -1 for
 numerical values and ?autogenerate? for string ones a good start.

 Martin

 ___
 Freeipa-devel mailing list
 Freeipa-devel@redhat.com
 https://www.redhat.com/mailman/listinfo/freeipa-devel
 
 
 Please consider the flow: user added staged - activated/moved to main tree -
 deleted/moved to deleted tree - staged back
 At the first step his IPA user ID and UID should be undefined and 
 autogenerated.
 On the last step his IPAUserID and UID should be preserved. The main 

Re: [Freeipa-devel] [PATCH] 0544 Remove the global anonymous read ACI

2014-05-22 Thread Petr Viktorin

On 05/21/2014 08:08 AM, Martin Kosek wrote:

On 05/19/2014 03:27 PM, Petr Viktorin wrote:

On 05/16/2014 02:00 PM, Martin Kosek wrote:

On 04/29/2014 11:02 PM, Petr Viktorin wrote:

I didn't test this as much as I'd like to, but it might come in handy when
testing my earlier patches.

The ACI is removed in the managed permissions plugin because I want to make
sure it's done after all the managed permission updates, which query it.


It worked in my case (I tested upgrade from 3.3.5). What do we do about other
permissions we will want to remove? I am talking about following ACIs:

- no anonymous access to roles
- no anonymous access to sudo
- no anonymous access to hbac
- no anonymous access to member information

I would like to remove them in 544 as well as otherwise they would bias the
testing.


Right. Here is the updated patch.


I tested upgrade from 3.3.5 to 4.0 and in SUFFIX I still had some of the ACIs 
left:

(targetattr = *)(target =
ldap:///cn=*,cn=roles,cn=accounts,dc=mkosek-fedora20,dc=test;)(version 3.0;
acl No anonymous access to roles; deny (read,search,compare) userdn !=
ldap:///all;;)

(targetattr = *)(target =
ldap:///cn=*,ou=SUDOers,dc=mkosek-fedora20,dc=test;)(version 3.0; acl No
anonymous access to sudo; deny (read,search,compare) userdn != ldap:///all;;)

The problem is that you used your testing suffix instead of suffix variable.


Shame on me. I've updated  rebased the patch.

I've also made a git hook yell at me when I commit something containing 
BRQ, hopefully this won't happen again.


--
Petr³

From 0802e5ae783703c6f1d05ac3f961e41233884a10 Mon Sep 17 00:00:00 2001
From: Petr Viktorin pvikt...@redhat.com
Date: Tue, 29 Apr 2014 21:46:26 +0200
Subject: [PATCH] Remove the global anonymous read ACI
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Also remove
- the deny ACIs that implemented exceptions to it:
  - no anonymous access to roles
  - no anonymous access to member information
  - no anonymous access to hbac
  - no anonymous access to sudo (2×)
- its updater plugin

Part of the work for: https://fedorahosted.org/freeipa/ticket/3566
---
 install/share/default-aci.ldif | 13 ---
 install/share/delegation.ldif  |  5 --
 install/updates/20-aci.update  | 11 +++
 install/updates/60-trusts.update   |  1 -
 ipaserver/install/plugins/update_anonymous_aci.py  | 96 --
 .../install/plugins/update_managed_permissions.py  | 19 +
 6 files changed, 30 insertions(+), 115 deletions(-)
 delete mode 100644 ipaserver/install/plugins/update_anonymous_aci.py

diff --git a/install/share/default-aci.ldif b/install/share/default-aci.ldif
index 480facf3294c593c6a2bcf326e20c32157d6d3c6..04fc185f785ee71246c6cc4f958c754158f16302 100644
--- a/install/share/default-aci.ldif
+++ b/install/share/default-aci.ldif
@@ -3,10 +3,7 @@
 dn: $SUFFIX
 changetype: modify
 add: aci
-aci: (targetfilter = ((!(objectClass=ipaToken))(!(objectClass=ipatokenTOTP))(!(objectClass=ipatokenHOTP))(!(objectClass=ipatokenRadiusConfiguration(target != ldap:///idnsname=*,cn=dns,$SUFFIX;)(targetattr != userPassword || krbPrincipalKey || sambaLMPassword || sambaNTPassword || passwordHistory || krbMKey || userPKCS12 || ipaNTHash || ipaNTTrustAuthOutgoing || ipaNTTrustAuthIncoming)(version 3.0; acl Enable Anonymous access; allow (read, search, compare) userdn = ldap:///anyone;;)
-aci: (targetattr = memberOf || memberHost || memberUser)(version 3.0; acl No anonymous access to member information; deny (read,search,compare) userdn != ldap:///all;;)
 aci: (targetattr = userpassword || krbprincipalkey || sambalmpassword || sambantpassword)(version 3.0; acl selfservice:Self can write own password; allow (write) userdn=ldap:///self;;)
-aci: (targetattr = *)(target = ldap:///cn=*,ou=SUDOers,$SUFFIX;)(version 3.0; acl No anonymous access to sudo; deny (read,search,compare) userdn != ldap:///all;;)
 
 dn: $SUFFIX
 changetype: modify
@@ -65,16 +62,6 @@ dn: cn=computers,cn=accounts,$SUFFIX
 add: aci
 aci: (targetattr = krbPrincipalKey || krbLastPwdChange)(target = ldap:///fqdn=*,cn=computers,cn=accounts,$SUFFIX;)(version 3.0;acl Admins can manage host keytab;allow (write) groupdn = ldap:///cn=admins,cn=groups,cn=accounts,$SUFFIX;;)
 
-dn: cn=hbac,$SUFFIX
-changetype: modify
-add: aci
-aci: (targetattr = *)(version 3.0; acl No anonymous access to hbac; deny (read,search,compare) userdn != ldap:///all;;)
-
-dn: cn=sudo,$SUFFIX
-changetype: modify
-add: aci
-aci: (targetattr = *)(version 3.0; acl No anonymous access to sudo; deny (read,search,compare) userdn != ldap:///all;;)
-
 # This is used for the host/service one-time passwordn and keytab indirectors.
 # We can do a query on a DN to see if an attribute exists.
 dn: cn=accounts,$SUFFIX
diff --git a/install/share/delegation.ldif b/install/share/delegation.ldif
index 7bd4e1e2d93b1dde4122ad1bfbe889625d983544..43d13974ffd63ea6ee554c815b911715609149b8 

Re: [Freeipa-devel] [PATCH] 0550 ipalib.cli: Add filename argument to ipa console

2014-05-22 Thread Nathaniel McCallum
On Thu, 2014-05-22 at 15:07 +0200, Petr Viktorin wrote:
 Hello,
 I find the `ipa console` command quite useful for testing, and it's 
 bothered me that it can't execute a script. Fixing this helps me. Would 
 it help anyone else?
 
 This would need a ticket + design doc before it's pushed.
 
 
 Compare:
 $ (echo 'print 1'; echo 'print 2') | ipa console
 (Custom IPA interactive Python console)
   1
   2
 
   [no newline]
 
 $ ipa console (echo 'print 1'; echo 'print 2')
 1
 2

Looks helpful to me. ACK

Also, FTR, at first I thought it looked weird to have the open() outside
of the with statement. But this is because you only want to catch the
IOError during the open() operation. So the behavior in this patch is
correct.

Nathaniel

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


Re: [Freeipa-devel] [PATCH 0052] Only specify the ipatokenuniqueid default in the add operation

2014-05-22 Thread Nathaniel McCallum
On Tue, 2014-05-13 at 12:55 -0400, Nathaniel McCallum wrote:
 On Tue, 2014-05-13 at 16:47 +0200, Jan Cholasta wrote:
  On 12.5.2014 20:50, Nathaniel McCallum wrote:
   On Mon, 2014-05-12 at 18:40 +0200, Misnyovszki Adam wrote:
   On Tue, 06 May 2014 11:46:14 -0400
   Nathaniel McCallum npmccal...@redhat.com wrote:
  
   On Tue, 2014-05-06 at 11:38 -0400, Nathaniel McCallum wrote:
   On Tue, 2014-05-06 at 17:34 +0200, Petr Vobornik wrote:
   On 6.5.2014 17:13, Nathaniel McCallum wrote:
   On Tue, 2014-05-06 at 17:04 +0200, Petr Vobornik wrote:
   On 6.5.2014 16:51, Nathaniel McCallum wrote:
   Specifying the default in the LDAP Object causes the
   parameter to be specified for non-add operations. This is
   especially problematic when performing the modify operation
   as it causes the primary key to change for every modification.
  
   https://fedorahosted.org/freeipa/ticket/4227
  
  
   shouldn't removal of `autofill=True,` be enough?
  
   Removing autofill=True results in the default not being used
   for the otptoken-add operation. That may be a different bug
   (I'm not sure what the expectation of autofill is).
  
   Nathaniel
  
  
   Seems to work form me with:
  
   diff --git a/ipalib/plugins/otptoken.py
   b/ipalib/plugins/otptoken.py index f68ea7d..623f1f1 100644
   --- a/ipalib/plugins/otptoken.py
   +++ b/ipalib/plugins/otptoken.py
   @@ -121,9 +121,7 @@ class otptoken(LDAPObject):
  cli_name='id',
  label=_('Unique ID'),
  default_from=lambda: unicode(uuid.uuid4()),
   -autofill=True,
  primary_key=True,
   -flags=('optional_create'),
  ),
  StrEnum('type?',
  label=_('Type'),
  
   Doing this causes the ipa otptoken-add command to prompt for the
   Unique ID. This may be the desired behavior, but it is not how it
   worked previously (no prompt).
  
   Here is an alternate patch for this second approach. I have no strong
   opinion on the correct behavior here.
  
   Nathaniel
  
   IMO you should update API.txt with ./makeapi
  
   Running ./makeapi results in no changes to API.txt.
  
  This is not right, there *are* changes in the API and build fails for me 
  becase API.txt is not updated.
 
 I think maybe I ran it from the wrong branch. Fixed.

I still need a review of this. It is pretty trivial.

Nathaniel

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


Re: [Freeipa-devel] [PATCHES] 0540-0542 Add managed read permissions to user

2014-05-22 Thread Petr Viktorin

On 05/21/2014 12:14 PM, Simo Sorce wrote:

On Wed, 2014-05-21 at 08:03 +0200, Martin Kosek wrote:

On 05/16/2014 04:33 PM, Petr Viktorin wrote:

On 05/16/2014 01:54 PM, Martin Kosek wrote:

On 04/29/2014 11:00 PM, Petr Viktorin wrote:

Patch 0540 adds a bunch of managed read ACIs for user, as discussed previously
[0].

Patch 0541 is some minor refactoring for the next part.

Patch 0542 sets the read acces to addressbook attributes to anonymous when
upgrading from pre-4.0.
I first this by checking if the update is run from ipa-server-install or not,
but then I realized the logic I want is simple: if the global anon read ACI
exists, we want to preserve its spirit by setting addressbook attribute ACI to
anonymous.


[0] http://www.redhat.com/archives/freeipa-devel/2014-April/msg00363.html et
al.



540:

Looks good! The only attributes I am concerned about are special IPA attributes:

- ipauniqueid
- ipasshpubkey
- ipauserauthtype
- userclass

I personally do not think they should be included in POSIX attributes
permissions, they are far from POSIX definition...

What about creating one more permission System: Read User IPA Attributes as
these are specific to FreeIPA use and allowing that permission for all
authenticated users?


Sounds reasonable. I assume we want this one to be also set to anonymous when
upgrading from old versions.
Attaching updated patches.


Ok, looks good.

I am now just pondering whether System: Read User POSIX Attributes is the
right name for the permission as there are not just POSIX attributes, but also
attributes from organizationalPerson or inetOrgPerson objectclasses.

Maybe we should name it System: Read User Core Attributes or System: Read
User Basic Attributes? Simo, any preference?


We could use: System: Read User Standard Attributes


I've used this one, then.



but the 'posix' version is also ok to me.


On Wed, 2014-05-21 at 08:03 +0200, Martin Kosek wrote:

Also, I just realized we forgot memberOf attribute - it needs to be available
to authenticated users otherwise group membership will fall apart.


Good catch. Added.

--
Petr³

From f02ca92737e03eb9872ab87ce039766a6372dbe4 Mon Sep 17 00:00:00 2001
From: Petr Viktorin pvikt...@redhat.com
Date: Wed, 26 Mar 2014 17:11:23 +0100
Subject: [PATCH] Add managed read permissions to user

Part of the work for: https://fedorahosted.org/freeipa/ticket/3566
---
 ipalib/plugins/user.py | 70 ++
 1 file changed, 70 insertions(+)

diff --git a/ipalib/plugins/user.py b/ipalib/plugins/user.py
index d9c7c6c858aa0a4927efc01fb41b535b7bb04ba2..76efdc8941f70155c11553532dedc5656c4efcd0 100644
--- a/ipalib/plugins/user.py
+++ b/ipalib/plugins/user.py
@@ -233,6 +233,76 @@ class user(LDAPObject):
 bindable = True
 password_attributes = [('userpassword', 'has_password'),
('krbprincipalkey', 'has_keytab')]
+managed_permissions = {
+'System: Read User Standard Attributes': {
+'replaces_global_anonymous_aci': True,
+'ipapermbindruletype': 'anonymous',
+'ipapermright': {'read', 'search', 'compare'},
+'ipapermdefaultattr': {
+'objectclass', 'cn', 'sn', 'description', 'title', 'uid',
+'displayname', 'givenname', 'initials', 'manager', 'gecos',
+'gidnumber', 'homedirectory', 'loginshell', 'uidnumber'
+},
+},
+'System: Read User Addressbook Attributes': {
+'replaces_global_anonymous_aci': True,
+'ipapermbindruletype': 'all',
+'ipapermright': {'read', 'search', 'compare'},
+'ipapermdefaultattr': {
+'seealso', 'telephonenumber',
+'fax', 'l', 'ou', 'st', 'postalcode', 'street',
+'destinationindicator', 'internationalisdnnumber',
+'physicaldeliveryofficename', 'postaladdress', 'postofficebox',
+'preferreddeliverymethod', 'registeredaddress',
+'teletexterminalidentifier', 'telexnumber', 'x121address',
+'carlicense', 'departmentnumber', 'employeenumber',
+'employeetype', 'preferredlanguage', 'mail', 'mobile', 'pager',
+'audio', 'businesscategory', 'homephone', 'homepostaladdress',
+'jpegphoto', 'labeleduri', 'o', 'photo', 'roomnumber',
+'secretary', 'usercertificate', 'userpkcs12',
+'usersmimecertificate', 'x500uniqueidentifier',
+'inetuserhttpurl', 'inetuserstatus',
+},
+},
+'System: Read User IPA Attributes': {
+'replaces_global_anonymous_aci': True,
+'ipapermbindruletype': 'all',
+'ipapermright': {'read', 'search', 'compare'},
+'ipapermdefaultattr': {
+'ipauniqueid', 'ipasshpubkey', 'ipauserauthtype', 'userclass',
+},
+},
+'System: Read User Kerberos Attributes': {
+ 

Re: [Freeipa-devel] [PATCH 0048] Default the token owner to the person adding the token

2014-05-22 Thread Nathaniel McCallum
I still need a review on this.

On Wed, 2014-05-07 at 10:06 -0400, Nathaniel McCallum wrote:
 On Wed, 2014-05-07 at 15:54 +0200, Petr Vobornik wrote:
  On 6.5.2014 17:07, Nathaniel McCallum wrote:
   On Tue, 2014-05-06 at 16:11 +0200, Jan Cholasta wrote:
   On 6.5.2014 15:16, Nathaniel McCallum wrote:
   On Tue, 2014-05-06 at 13:46 +0200, Jan Cholasta wrote:
   Hi,
  
   On 5.5.2014 18:40, Nathaniel McCallum wrote:
   Creating tokens for yourself is the most common operation. Making this
   the default optimizes for the common case.
  
   The user-find call should be inside the if statement.
  
   This is actually for a reason. See my patch 0049 for further context.
  
   IMO something like this would be better:
  
 if 'ipatokenowner' not in entry_attrs or 'ipatokenprotected' not in
   entry_attrs:
 result = self.api.Command.user_find(whoami=True)['result']
 if result:
 cur_uid = result[0]['uid'][0]
 prev_uid = entry_attrs.setdefault('ipatokenowner', cur_uid)
 if cur_uid != prev_uid:
 entry_attrs.setdefault('ipatokenprotected', True)
  
   Fixed (see also my new revision of patch 0049).
  
   Nathaniel
  
  
  I assume that this won't allow to create a token without an owner. Do we 
  want to have this restriction?
  
  Usecase: import a batch of hw tokens
 
 This case is currently very much on my radar (I'm finishing the import
 script now). To set no owner, just use --owner=. We are testing for
 key presence here, not the value of the key. So if the key is present
 with an empty value, no owner will be set.
 
 FYI, the import format (RFC 6030) also permits a mechanism for declaring
 ownership in DN format.
 
 Nathaniel
 
 ___
 Freeipa-devel mailing list
 Freeipa-devel@redhat.com
 https://www.redhat.com/mailman/listinfo/freeipa-devel


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


Re: [Freeipa-devel] [PATCH 0047] kdb: Don't provide password expiration when using only RADIUS

2014-05-22 Thread Nathaniel McCallum
On Fri, 2014-05-02 at 17:49 -0400, Nathaniel McCallum wrote:
 If the KDC doesn't use the FreeIPA password for authentication, then it
 is futile to provide this information. Doing so will only confuse the
 user. It also causes password change dialogues when the password is
 irrelevant.
 
 https://fedorahosted.org/freeipa/ticket/4299

This new version fixes a small logic bug. This should be an easy review.

Nathaniel
From 9764b91aa976ca1ed48885d5ace555b6b263080a Mon Sep 17 00:00:00 2001
From: Nathaniel McCallum npmccal...@redhat.com
Date: Fri, 2 May 2014 14:55:07 -0400
Subject: [PATCH] kdb: Don't provide password expiration when using only RADIUS

If the KDC doesn't use the FreeIPA password for authentication, then it is
futile to provide this information. Doing so will only confuse the user. It
also causes password change dialogues when the password is irrelevant.

https://fedorahosted.org/freeipa/ticket/4299
---
 daemons/ipa-kdb/ipa_kdb_principals.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/daemons/ipa-kdb/ipa_kdb_principals.c b/daemons/ipa-kdb/ipa_kdb_principals.c
index f0be76ea7b36efe3540429f7e31ffbc582edd060..d2be98886ef865eaabf7d5935994281ec262a2c8 100644
--- a/daemons/ipa-kdb/ipa_kdb_principals.c
+++ b/daemons/ipa-kdb/ipa_kdb_principals.c
@@ -429,6 +429,10 @@ static krb5_error_code ipadb_parse_ldap_entry(krb5_context kcontext,
 switch (ret) {
 case 0:
 entry-pw_expiration = restime;
+
+/* If we are using only RADIUS, we don't know expiration. */
+if (ua == IPADB_USER_AUTH_RADIUS)
+entry-pw_expiration = 0;
 case ENOENT:
 break;
 default:
-- 
1.9.3

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

[Freeipa-devel] User life cycle: plugins scope for staged users

2014-05-22 Thread thierry bordaz

Hello,

   In order to provision staged users (account inactivated) with there
   initial values:

   /usr/bin/ipa user-add tb20 --to-stage --first=tb20 --last=tb20
   -
   Added user tb20
   -
  User login: tb20
  First name: tb20
  Last name: tb20
  Full name: tb20 tb20
  Display name: tb20 tb20
  Initials: tt
  Home directory: /home/tb20
  GECOS: tb20 tb20
  Login shell: /bin/sh
  Kerberos principal: t...@idm.lab.bos.redhat.com
  Email address: t...@idm.lab.bos.redhat.com
  UID: -1
  GID: -1
  Account disabled: true
  Password: False
  Kerberos keys available: False

   ldapsearch -LLL -h localhost -p 389 -D cn=directory manager -w
   Secret123 -b dc=idm,dc=lab,dc=bos,dc=redhat,dc=com uid=tb20
   dn: uid=tb20,cn=staged
   users,cn=accounts,cn=provisioning,dc=idm,dc=lab,dc=bos,
 dc=redhat,dc=com
   displayName: tb20 tb20
   cn: tb20 tb20
   objectClass: top
   objectClass: person
   objectClass: organizationalperson
   objectClass: inetorgperson
   objectClass: inetuser
   objectClass: posixaccount
   objectClass: krbprincipalaux
   objectClass: krbticketpolicyaux
   objectClass: ipaobject
   objectClass: ipasshuser
   objectClass: ipaSshGroupOfPubKeys
   loginShell: /bin/sh
   uidNumber: -1
   ipaUniqueID: autogenerate
   gidNumber: -1
   gecos: tb20 tb20
   sn: tb20
   homeDirectory: /home/tb20
   uid: tb20
   mail: t...@idm.lab.bos.redhat.com
   krbPrincipalName: t...@idm.lab.bos.redhat.com
   givenName: tb20
   initials: tt

   I needed to resctrict the scope of the following plugins:

   dn: cn=ipaUniqueID uniqueness,cn=plugins,cn=config
   nsslapd-pluginarg1:
   cn=accounts,dc=idm,dc=lab,dc=bos,dc=redhat,dc=com

   dn: cn=IPA Unique IDs,cn=IPA UUID,cn=plugins,cn=confi
   ipauuidscope: cn=accounts,dc=idm,dc=lab,dc=bos,dc=redhat,dc=com

   dn: cn=Posix IDs,cn=Distributed Numeric Assignment
   Plugin,cn=plugins,cn=config
   dnaScope: cn=accounts,dc=idm,dc=lab,dc=bos,dc=redhat,dc=com

   dn: cn=MemberOf Plugin,cn=plugins,cn=config
   memberofentryscope:
   cn=accounts,dc=idm,dc=lab,dc=bos,dc=redhat,dc=com

   In fact I need them to not modify the added entry when it is added
   under cn=staged users,cn=accounts,cn=provisioning,$SUFFIX.
   Now is it possible to limit those plugins scope to the 'cn=accounts'
   part of the tree ? I guess not.
   If it is not possible, a solution is to make the scope multi-valued
   attributes or to introduce a new config attribute '*notInScope' also
   multi-valued.
   A problem is the 'cn=ipaUniqueID uniqueness' that rely on the
   'attribute uniqueness' plugin with a argv[ ], not really convenient
   to pass 2 multivalued attributes.

   If anyone is having others solutions it would help me a lot :-)

   thanks
   thierry





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

Re: [Freeipa-devel] Status/Question about User life cycle

2014-05-22 Thread Martin Kosek
On 05/22/2014 10:47 AM, Petr Viktorin wrote:
 On 05/21/2014 10:00 PM, Dmitri Pal wrote:
 On 05/19/2014 10:45 AM, thierry bordaz wrote:
 On 05/19/2014 04:44 PM, Jan Cholasta wrote:
 On 19.5.2014 16:34, thierry bordaz wrote:
 On 05/19/2014 04:22 PM, Jan Cholasta wrote:
 On 19.5.2014 16:03, thierry bordaz wrote:
 On 05/19/2014 03:54 PM, Jan Cholasta wrote:
 On 19.5.2014 15:19, Petr Viktorin wrote:
 Hello list,
 Here's a conversation that started internally. I'm making it
 public.

 On 05/19/2014 01:00 PM, Martin Kosek wrote:
 On 05/19/2014 12:46 PM, Petr Viktorin wrote:
 On 05/19/2014 08:25 AM, Martin Kosek wrote:
 On 05/19/2014 08:24 AM, Martin Kosek wrote:
 On 05/16/2014 04:48 PM, thierry bordaz wrote:
 Hello Martin,

  I am getting familiar with the freeipa CLI code and
 started
  implemented '--to-stage' and '--from-stage'. This
 really an
  impressive set of code :-)

 Great! :-)

  I completed 'to-stage' and testing '--from-stage'.

  I have a question regarding the '--from-stage' syntax.
 'uid'
 is a
  mandatory argument to 'user-add' subcommand. In the
 design the
  '--from-stage' option is described with:

  ipa user-add --from-stage=tuser

 Note, the design is here:
 http://www.freeipa.org/page/V4/User_Life-Cycle_Management

  But as 'uid' is mandatory the command should rather be

  ipa user-add tuser --from-stage=tuser

  In that case the option value for '--from-stage' is not
 required and
  the command should be

  ipa user-add tuser --from-stage

  Is that ok if I implement the command like above or did I
 miss
  something ?

  regards
  thierry

 Hmm, no, I think you are right.  We can change --from-stage to
 just
 Bool
 parameter. When it is true, it'd mean that get_dn or
 pre-callback
 should
 retrieve the record from stage and use all it's attributes (and
 add
 standard
 default attributes values on top of that).

 Also CC-ing Petr Viktorin for reference.

 This operation can't change the user's attributes, can it?
 I.e., we
 don't
 support something like:
  ipa user-add tuser --from-stage --phone=123456789
 --email=newem...@example.com
 If this is the case, what's the reason for using user-add for
 this?
 Wouldn't it
 be better to make this a separate command, say:
  ipa user-activate tuser
  ipa user-activate tuser --from-deleted
  ipa user-activate tuser --from-deleted --to-staged

 +1, I would even go as far as having separate commands for staged
 and
 deleted users, e.g.:

 ipa user-unstage tuser
 ipa user-undelete tuser
 ipa user-undelete tuser --to-staged

 A deleted entry has already been active so it contains already set
 attributes while the pure staged entries are almost empty boxes.
 But
 from an administrator point of view, both staged/deleted entries are
 inactive. What would be the advantages of two separated commands ?

 You just said it yourself: activating/unstaging a user is quite
 different from undeleting a user. Cramming multiple different
 operations in a single command is bad design IMHO.

 Ok I understand.
 I believe that deleted entries and staged entries will be in the same
 container (provisioning).

 The design page mentions cn=staged
 users,cn=accounts,cn=provisioning,$SUFFIX and cn=deleted
 users,cn=accounts,cn=provisioning,$SUFFIX, which are two different
 containers.

 Oppsss.. Sorry for the confusion :-[

 So we may have at least those two possibilities:

   * ipa user-activate tuser [--from-staging|--from-delete]
   * ipa user-unstage tuser
 ipa user-undelete tuser


 I like the idea of different verbs for different hives.
 Something like:

 Adding directly to stage via CLI: ipa user-stage
 Removing from stage: user-unstage (user is gone)
 Stage to Main - activate; - deactivate
 Main to delete - del; -restore or undelete
 Delete to stage - I think we can use ipa user-stage command with
 --deleted=user or similar
 
 To be honest, I don't like this idea.
 Too many names are confusing, if we can find a consistent option to cut the
 number of names down we should do it.
 IMO This is the worst part of Git:
 http://assets.osteele.com/images/2008/git-transport.png . We can do better.
 
 Another good thing would be if options did not affect the applicability of
 other options (too much). For example in your proposal there'd be something 
 like:
 ipa user-stage tuser --first=abc --last=xyz --phone=123 ..
 ipa user-stage --deleted=tuser  # no attribute options allowed
 We should avoid this, if only for the reason that it makes the help text
 confusing.
 
 
 My proposal would be that the move commands use the verb for the target and an
 option for the source, and add/mod use an option for the container:
 
 1) adding a new user
 (to active)   ipa user-add tuser ...
 (to stage)ipa user-add tuser --staged ...
 (to deleted)  ipa user-add tuser --deleted ...  (*)
 
 2) moving to main
 (from stage)  ipa user-activate tuser  (**)
 (from del)

Re: [Freeipa-devel] [PATCH 0047] kdb: Don't provide password expiration when using only RADIUS

2014-05-22 Thread Alexander Bokovoy

On Thu, 22 May 2014, Nathaniel McCallum wrote:

On Fri, 2014-05-02 at 17:49 -0400, Nathaniel McCallum wrote:

If the KDC doesn't use the FreeIPA password for authentication, then it
is futile to provide this information. Doing so will only confuse the
user. It also causes password change dialogues when the password is
irrelevant.

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


This new version fixes a small logic bug. This should be an easy review.

ACK.




Nathaniel



From 9764b91aa976ca1ed48885d5ace555b6b263080a Mon Sep 17 00:00:00 2001

From: Nathaniel McCallum npmccal...@redhat.com
Date: Fri, 2 May 2014 14:55:07 -0400
Subject: [PATCH] kdb: Don't provide password expiration when using only RADIUS

If the KDC doesn't use the FreeIPA password for authentication, then it is
futile to provide this information. Doing so will only confuse the user. It
also causes password change dialogues when the password is irrelevant.

https://fedorahosted.org/freeipa/ticket/4299
---
daemons/ipa-kdb/ipa_kdb_principals.c | 4 
1 file changed, 4 insertions(+)

diff --git a/daemons/ipa-kdb/ipa_kdb_principals.c 
b/daemons/ipa-kdb/ipa_kdb_principals.c
index 
f0be76ea7b36efe3540429f7e31ffbc582edd060..d2be98886ef865eaabf7d5935994281ec262a2c8
 100644
--- a/daemons/ipa-kdb/ipa_kdb_principals.c
+++ b/daemons/ipa-kdb/ipa_kdb_principals.c
@@ -429,6 +429,10 @@ static krb5_error_code ipadb_parse_ldap_entry(krb5_context 
kcontext,
switch (ret) {
case 0:
entry-pw_expiration = restime;
+
+/* If we are using only RADIUS, we don't know expiration. */
+if (ua == IPADB_USER_AUTH_RADIUS)
+entry-pw_expiration = 0;
case ENOENT:
break;
default:
--
1.9.3




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



--
/ Alexander Bokovoy

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


Re: [Freeipa-devel] [PATCH 0052] Only specify the ipatokenuniqueid default in the add operation

2014-05-22 Thread Petr Viktorin

On 05/22/2014 04:12 PM, Nathaniel McCallum wrote:

On Tue, 2014-05-13 at 12:55 -0400, Nathaniel McCallum wrote:

On Tue, 2014-05-13 at 16:47 +0200, Jan Cholasta wrote:

On 12.5.2014 20:50, Nathaniel McCallum wrote:

On Mon, 2014-05-12 at 18:40 +0200, Misnyovszki Adam wrote:

On Tue, 06 May 2014 11:46:14 -0400
Nathaniel McCallum npmccal...@redhat.com wrote:


On Tue, 2014-05-06 at 11:38 -0400, Nathaniel McCallum wrote:

On Tue, 2014-05-06 at 17:34 +0200, Petr Vobornik wrote:

On 6.5.2014 17:13, Nathaniel McCallum wrote:

On Tue, 2014-05-06 at 17:04 +0200, Petr Vobornik wrote:

On 6.5.2014 16:51, Nathaniel McCallum wrote:

Specifying the default in the LDAP Object causes the
parameter to be specified for non-add operations. This is
especially problematic when performing the modify operation
as it causes the primary key to change for every modification.

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



shouldn't removal of `autofill=True,` be enough?


Removing autofill=True results in the default not being used
for the otptoken-add operation. That may be a different bug
(I'm not sure what the expectation of autofill is).

Nathaniel



Seems to work form me with:

diff --git a/ipalib/plugins/otptoken.py
b/ipalib/plugins/otptoken.py index f68ea7d..623f1f1 100644
--- a/ipalib/plugins/otptoken.py
+++ b/ipalib/plugins/otptoken.py
@@ -121,9 +121,7 @@ class otptoken(LDAPObject):
cli_name='id',
label=_('Unique ID'),
default_from=lambda: unicode(uuid.uuid4()),
-autofill=True,
primary_key=True,
-flags=('optional_create'),
),
StrEnum('type?',
label=_('Type'),


Doing this causes the ipa otptoken-add command to prompt for the
Unique ID. This may be the desired behavior, but it is not how it
worked previously (no prompt).


Here is an alternate patch for this second approach. I have no strong
opinion on the correct behavior here.

Nathaniel


IMO you should update API.txt with ./makeapi


Running ./makeapi results in no changes to API.txt.


This is not right, there *are* changes in the API and build fails for me
becase API.txt is not updated.


I think maybe I ran it from the wrong branch. Fixed.


I still need a review of this. It is pretty trivial.

Nathaniel


This still prompts for the unique ID on add:

$ ipa otptoken-add
Unique ID [25cb3aa9-db19-40f8-acf4-33ef65ca863c]:

I don't think that's the intended behavior.

--
Petr³

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


Re: [Freeipa-devel] [PATCH 0047] kdb: Don't provide password expiration when using only RADIUS

2014-05-22 Thread Petr Viktorin

On 05/22/2014 04:43 PM, Alexander Bokovoy wrote:

On Thu, 22 May 2014, Nathaniel McCallum wrote:

On Fri, 2014-05-02 at 17:49 -0400, Nathaniel McCallum wrote:

If the KDC doesn't use the FreeIPA password for authentication, then it
is futile to provide this information. Doing so will only confuse the
user. It also causes password change dialogues when the password is
irrelevant.

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


This new version fixes a small logic bug. This should be an easy review.

ACK.



Pushed to master: 58f8ebf49148172c6f3b1d22bcd7ea0fb3fb21c7


--
Petr³

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


Re: [Freeipa-devel] [PATCH 0052] Only specify the ipatokenuniqueid default in the add operation

2014-05-22 Thread Nathaniel McCallum
On Thu, 2014-05-22 at 16:45 +0200, Petr Viktorin wrote:
 On 05/22/2014 04:12 PM, Nathaniel McCallum wrote:
  On Tue, 2014-05-13 at 12:55 -0400, Nathaniel McCallum wrote:
  On Tue, 2014-05-13 at 16:47 +0200, Jan Cholasta wrote:
  On 12.5.2014 20:50, Nathaniel McCallum wrote:
  On Mon, 2014-05-12 at 18:40 +0200, Misnyovszki Adam wrote:
  On Tue, 06 May 2014 11:46:14 -0400
  Nathaniel McCallum npmccal...@redhat.com wrote:
 
  On Tue, 2014-05-06 at 11:38 -0400, Nathaniel McCallum wrote:
  On Tue, 2014-05-06 at 17:34 +0200, Petr Vobornik wrote:
  On 6.5.2014 17:13, Nathaniel McCallum wrote:
  On Tue, 2014-05-06 at 17:04 +0200, Petr Vobornik wrote:
  On 6.5.2014 16:51, Nathaniel McCallum wrote:
  Specifying the default in the LDAP Object causes the
  parameter to be specified for non-add operations. This is
  especially problematic when performing the modify operation
  as it causes the primary key to change for every modification.
 
  https://fedorahosted.org/freeipa/ticket/4227
 
 
  shouldn't removal of `autofill=True,` be enough?
 
  Removing autofill=True results in the default not being used
  for the otptoken-add operation. That may be a different bug
  (I'm not sure what the expectation of autofill is).
 
  Nathaniel
 
 
  Seems to work form me with:
 
  diff --git a/ipalib/plugins/otptoken.py
  b/ipalib/plugins/otptoken.py index f68ea7d..623f1f1 100644
  --- a/ipalib/plugins/otptoken.py
  +++ b/ipalib/plugins/otptoken.py
  @@ -121,9 +121,7 @@ class otptoken(LDAPObject):
  cli_name='id',
  label=_('Unique ID'),
  default_from=lambda: unicode(uuid.uuid4()),
  -autofill=True,
  primary_key=True,
  -flags=('optional_create'),
  ),
  StrEnum('type?',
  label=_('Type'),
 
  Doing this causes the ipa otptoken-add command to prompt for the
  Unique ID. This may be the desired behavior, but it is not how it
  worked previously (no prompt).
 
  Here is an alternate patch for this second approach. I have no strong
  opinion on the correct behavior here.
 
  Nathaniel
 
  IMO you should update API.txt with ./makeapi
 
  Running ./makeapi results in no changes to API.txt.
 
  This is not right, there *are* changes in the API and build fails for me
  becase API.txt is not updated.
 
  I think maybe I ran it from the wrong branch. Fixed.
 
  I still need a review of this. It is pretty trivial.
 
  Nathaniel
 
 This still prompts for the unique ID on add:
 
 $ ipa otptoken-add
 Unique ID [25cb3aa9-db19-40f8-acf4-33ef65ca863c]:
 
 I don't think that's the intended behavior.

Hence the alternate patch (0052a). If we don't want to prompt, we'll
need to use the first patch (0052). I have no strong opinion on the
correct behavior and I am fine with merging either patch.

Nathaniel

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


Re: [Freeipa-devel] [PATCH 0052] Only specify the ipatokenuniqueid default in the add operation

2014-05-22 Thread Nathaniel McCallum
On Thu, 2014-05-22 at 10:53 -0400, Nathaniel McCallum wrote:
 On Thu, 2014-05-22 at 16:45 +0200, Petr Viktorin wrote:
  On 05/22/2014 04:12 PM, Nathaniel McCallum wrote:
   On Tue, 2014-05-13 at 12:55 -0400, Nathaniel McCallum wrote:
   On Tue, 2014-05-13 at 16:47 +0200, Jan Cholasta wrote:
   On 12.5.2014 20:50, Nathaniel McCallum wrote:
   On Mon, 2014-05-12 at 18:40 +0200, Misnyovszki Adam wrote:
   On Tue, 06 May 2014 11:46:14 -0400
   Nathaniel McCallum npmccal...@redhat.com wrote:
  
   On Tue, 2014-05-06 at 11:38 -0400, Nathaniel McCallum wrote:
   On Tue, 2014-05-06 at 17:34 +0200, Petr Vobornik wrote:
   On 6.5.2014 17:13, Nathaniel McCallum wrote:
   On Tue, 2014-05-06 at 17:04 +0200, Petr Vobornik wrote:
   On 6.5.2014 16:51, Nathaniel McCallum wrote:
   Specifying the default in the LDAP Object causes the
   parameter to be specified for non-add operations. This is
   especially problematic when performing the modify operation
   as it causes the primary key to change for every modification.
  
   https://fedorahosted.org/freeipa/ticket/4227
  
  
   shouldn't removal of `autofill=True,` be enough?
  
   Removing autofill=True results in the default not being used
   for the otptoken-add operation. That may be a different bug
   (I'm not sure what the expectation of autofill is).
  
   Nathaniel
  
  
   Seems to work form me with:
  
   diff --git a/ipalib/plugins/otptoken.py
   b/ipalib/plugins/otptoken.py index f68ea7d..623f1f1 100644
   --- a/ipalib/plugins/otptoken.py
   +++ b/ipalib/plugins/otptoken.py
   @@ -121,9 +121,7 @@ class otptoken(LDAPObject):
   cli_name='id',
   label=_('Unique ID'),
   default_from=lambda: unicode(uuid.uuid4()),
   -autofill=True,
   primary_key=True,
   -flags=('optional_create'),
   ),
   StrEnum('type?',
   label=_('Type'),
  
   Doing this causes the ipa otptoken-add command to prompt for the
   Unique ID. This may be the desired behavior, but it is not how it
   worked previously (no prompt).
  
   Here is an alternate patch for this second approach. I have no strong
   opinion on the correct behavior here.
  
   Nathaniel
  
   IMO you should update API.txt with ./makeapi
  
   Running ./makeapi results in no changes to API.txt.
  
   This is not right, there *are* changes in the API and build fails for me
   becase API.txt is not updated.
  
   I think maybe I ran it from the wrong branch. Fixed.
  
   I still need a review of this. It is pretty trivial.
  
   Nathaniel
  
  This still prompts for the unique ID on add:
  
  $ ipa otptoken-add
  Unique ID [25cb3aa9-db19-40f8-acf4-33ef65ca863c]:
  
  I don't think that's the intended behavior.
 
 Hence the alternate patch (0052a). If we don't want to prompt, we'll
 need to use the first patch (0052). I have no strong opinion on the
 correct behavior and I am fine with merging either patch.

Attached is the non-alternate (0052) with the api updated.

Nathaniel
From 12316fb7cf6a72239ddb1ed64ba74b7b344b9206 Mon Sep 17 00:00:00 2001
From: Nathaniel McCallum npmccal...@redhat.com
Date: Thu, 1 May 2014 16:31:45 -0400
Subject: [PATCH] Only specify the ipatokenuniqueid default in the add
 operation

Specifying the default in the LDAP Object causes the parameter to be specified
for non-add operations. This is especially problematic when performing the
modify operation as it causes the primary key to change for every
modification.

https://fedorahosted.org/freeipa/ticket/4227
---
 API.txt| 10 +-
 VERSION|  4 ++--
 ipalib/plugins/otptoken.py |  7 +--
 3 files changed, 12 insertions(+), 9 deletions(-)

diff --git a/API.txt b/API.txt
index e674dfebeb924cb16e9b7fdb0520365774ac50d9..1ea93e9dd403c3fe1a8ca6b047d6fee72220a862 100644
--- a/API.txt
+++ b/API.txt
@@ -2224,7 +2224,7 @@ output: Output('summary', (type 'unicode', type 'NoneType'), None)
 output: PrimaryKey('value', None, None)
 command: otptoken_add
 args: 1,21,3
-arg: Str('ipatokenuniqueid', attribute=True, autofill=True, cli_name='id', multivalue=False, primary_key=True, required=False)
+arg: Str('ipatokenuniqueid', attribute=True, cli_name='id', multivalue=False, primary_key=True, required=False)
 option: Str('addattr*', cli_name='addattr', exclude='webui')
 option: Flag('all', autofill=True, cli_name='all', default=False, exclude='webui')
 option: Str('description', attribute=True, cli_name='desc', multivalue=False, required=False)
@@ -2251,7 +2251,7 @@ output: Output('summary', (type 'unicode', type 'NoneType'), None)
 output: PrimaryKey('value', None, None)
 command: otptoken_del
 args: 1,2,3
-arg: Str('ipatokenuniqueid', attribute=True, autofill=True, cli_name='id', multivalue=True, primary_key=True, query=True, required=True)
+arg: Str('ipatokenuniqueid', attribute=True, cli_name='id', multivalue=True, primary_key=True, query=True, required=True)
 option: Flag('continue', 

Re: [Freeipa-devel] [PATCH 0052] Only specify the ipatokenuniqueid default in the add operation

2014-05-22 Thread Petr Vobornik

On 22.5.2014 17:00, Nathaniel McCallum wrote:

On Thu, 2014-05-22 at 10:53 -0400, Nathaniel McCallum wrote:

On Thu, 2014-05-22 at 16:45 +0200, Petr Viktorin wrote:

On 05/22/2014 04:12 PM, Nathaniel McCallum wrote:

On Tue, 2014-05-13 at 12:55 -0400, Nathaniel McCallum wrote:

On Tue, 2014-05-13 at 16:47 +0200, Jan Cholasta wrote:

On 12.5.2014 20:50, Nathaniel McCallum wrote:

On Mon, 2014-05-12 at 18:40 +0200, Misnyovszki Adam wrote:

On Tue, 06 May 2014 11:46:14 -0400
Nathaniel McCallum npmccal...@redhat.com wrote:


On Tue, 2014-05-06 at 11:38 -0400, Nathaniel McCallum wrote:

On Tue, 2014-05-06 at 17:34 +0200, Petr Vobornik wrote:

On 6.5.2014 17:13, Nathaniel McCallum wrote:

On Tue, 2014-05-06 at 17:04 +0200, Petr Vobornik wrote:

On 6.5.2014 16:51, Nathaniel McCallum wrote:

Specifying the default in the LDAP Object causes the
parameter to be specified for non-add operations. This is
especially problematic when performing the modify operation
as it causes the primary key to change for every modification.

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



shouldn't removal of `autofill=True,` be enough?


Removing autofill=True results in the default not being used
for the otptoken-add operation. That may be a different bug
(I'm not sure what the expectation of autofill is).

Nathaniel



Seems to work form me with:

diff --git a/ipalib/plugins/otptoken.py
b/ipalib/plugins/otptoken.py index f68ea7d..623f1f1 100644
--- a/ipalib/plugins/otptoken.py
+++ b/ipalib/plugins/otptoken.py
@@ -121,9 +121,7 @@ class otptoken(LDAPObject):
 cli_name='id',
 label=_('Unique ID'),
 default_from=lambda: unicode(uuid.uuid4()),
-autofill=True,
 primary_key=True,
-flags=('optional_create'),
 ),
 StrEnum('type?',
 label=_('Type'),


Doing this causes the ipa otptoken-add command to prompt for the
Unique ID. This may be the desired behavior, but it is not how it
worked previously (no prompt).


Here is an alternate patch for this second approach. I have no strong
opinion on the correct behavior here.

Nathaniel


IMO you should update API.txt with ./makeapi


Running ./makeapi results in no changes to API.txt.


This is not right, there *are* changes in the API and build fails for me
becase API.txt is not updated.


I think maybe I ran it from the wrong branch. Fixed.


I still need a review of this. It is pretty trivial.

Nathaniel


This still prompts for the unique ID on add:

$ ipa otptoken-add
Unique ID [25cb3aa9-db19-40f8-acf4-33ef65ca863c]:

I don't think that's the intended behavior.


Hence the alternate patch (0052a). If we don't want to prompt, we'll
need to use the first patch (0052). I have no strong opinion on the
correct behavior and I am fine with merging either patch.


Attached is the non-alternate (0052) with the api updated.

Nathaniel


IMO 52a is better if used by hand and it keeps code cleaner. It might 
not be ideal though if used from a script because of nonexistent 
--unattended/-u option which would disable prompt (set 
env.interactive=False ?).

--
Petr Vobornik

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


Re: [Freeipa-devel] [PATCH 0052] Only specify the ipatokenuniqueid default in the add operation

2014-05-22 Thread Petr Viktorin

On 05/22/2014 05:13 PM, Petr Vobornik wrote:

On 22.5.2014 17:00, Nathaniel McCallum wrote:

On Thu, 2014-05-22 at 10:53 -0400, Nathaniel McCallum wrote:

On Thu, 2014-05-22 at 16:45 +0200, Petr Viktorin wrote:

On 05/22/2014 04:12 PM, Nathaniel McCallum wrote:

On Tue, 2014-05-13 at 12:55 -0400, Nathaniel McCallum wrote:

On Tue, 2014-05-13 at 16:47 +0200, Jan Cholasta wrote:

On 12.5.2014 20:50, Nathaniel McCallum wrote:

On Mon, 2014-05-12 at 18:40 +0200, Misnyovszki Adam wrote:

On Tue, 06 May 2014 11:46:14 -0400
Nathaniel McCallum npmccal...@redhat.com wrote:


On Tue, 2014-05-06 at 11:38 -0400, Nathaniel McCallum wrote:

On Tue, 2014-05-06 at 17:34 +0200, Petr Vobornik wrote:

On 6.5.2014 17:13, Nathaniel McCallum wrote:

On Tue, 2014-05-06 at 17:04 +0200, Petr Vobornik wrote:

On 6.5.2014 16:51, Nathaniel McCallum wrote:

Specifying the default in the LDAP Object causes the
parameter to be specified for non-add operations. This is
especially problematic when performing the modify operation
as it causes the primary key to change for every
modification.

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



shouldn't removal of `autofill=True,` be enough?


Removing autofill=True results in the default not being used
for the otptoken-add operation. That may be a different bug
(I'm not sure what the expectation of autofill is).

Nathaniel



Seems to work form me with:

diff --git a/ipalib/plugins/otptoken.py
b/ipalib/plugins/otptoken.py index f68ea7d..623f1f1 100644
--- a/ipalib/plugins/otptoken.py
+++ b/ipalib/plugins/otptoken.py
@@ -121,9 +121,7 @@ class otptoken(LDAPObject):
 cli_name='id',
 label=_('Unique ID'),
 default_from=lambda: unicode(uuid.uuid4()),
-autofill=True,
 primary_key=True,
-flags=('optional_create'),
 ),
 StrEnum('type?',
 label=_('Type'),


Doing this causes the ipa otptoken-add command to prompt for the
Unique ID. This may be the desired behavior, but it is not
how it
worked previously (no prompt).


Here is an alternate patch for this second approach. I have no
strong
opinion on the correct behavior here.

Nathaniel


IMO you should update API.txt with ./makeapi


Running ./makeapi results in no changes to API.txt.


This is not right, there *are* changes in the API and build fails
for me
becase API.txt is not updated.


I think maybe I ran it from the wrong branch. Fixed.


I still need a review of this. It is pretty trivial.

Nathaniel


This still prompts for the unique ID on add:

$ ipa otptoken-add
Unique ID [25cb3aa9-db19-40f8-acf4-33ef65ca863c]:

I don't think that's the intended behavior.


Hence the alternate patch (0052a). If we don't want to prompt, we'll
need to use the first patch (0052). I have no strong opinion on the
correct behavior and I am fine with merging either patch.


Attached is the non-alternate (0052) with the api updated.

Nathaniel


IMO 52a is better if used by hand and it keeps code cleaner.


I don't think that should influence the design of the CLI that much.


It might
not be ideal though if used from a script because of nonexistent
--unattended/-u option which would disable prompt (set
env.interactive=False ?).


Right.


ACK to the non-prompting version (0052.1).


--
Petr³

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


Re: [Freeipa-devel] Status/Question about User life cycle

2014-05-22 Thread thierry bordaz

On 05/22/2014 04:38 PM, Martin Kosek wrote:

On 05/22/2014 10:47 AM, Petr Viktorin wrote:

On 05/21/2014 10:00 PM, Dmitri Pal wrote:

On 05/19/2014 10:45 AM, thierry bordaz wrote:

On 05/19/2014 04:44 PM, Jan Cholasta wrote:

On 19.5.2014 16:34, thierry bordaz wrote:

On 05/19/2014 04:22 PM, Jan Cholasta wrote:

On 19.5.2014 16:03, thierry bordaz wrote:

On 05/19/2014 03:54 PM, Jan Cholasta wrote:

On 19.5.2014 15:19, Petr Viktorin wrote:

Hello list,
Here's a conversation that started internally. I'm making it
public.

On 05/19/2014 01:00 PM, Martin Kosek wrote:

On 05/19/2014 12:46 PM, Petr Viktorin wrote:

On 05/19/2014 08:25 AM, Martin Kosek wrote:

On 05/19/2014 08:24 AM, Martin Kosek wrote:

On 05/16/2014 04:48 PM, thierry bordaz wrote:

Hello Martin,

  I am getting familiar with the freeipa CLI code and
started
  implemented '--to-stage' and '--from-stage'. This
really an
  impressive set of code :-)

Great! :-)


  I completed 'to-stage' and testing '--from-stage'.

  I have a question regarding the '--from-stage' syntax.
'uid'
is a
  mandatory argument to 'user-add' subcommand. In the
design the
  '--from-stage' option is described with:

  ipa user-add --from-stage=tuser

Note, the design is here:
http://www.freeipa.org/page/V4/User_Life-Cycle_Management


  But as 'uid' is mandatory the command should rather be

  ipa user-add tuser --from-stage=tuser

  In that case the option value for '--from-stage' is not
required and
  the command should be

  ipa user-add tuser --from-stage

  Is that ok if I implement the command like above or did I
miss
  something ?

  regards
  thierry

Hmm, no, I think you are right.  We can change --from-stage to
just
Bool
parameter. When it is true, it'd mean that get_dn or
pre-callback
should
retrieve the record from stage and use all it's attributes (and
add
standard
default attributes values on top of that).

Also CC-ing Petr Viktorin for reference.

This operation can't change the user's attributes, can it?
I.e., we
don't
support something like:
  ipa user-add tuser --from-stage --phone=123456789
--email=newem...@example.com
If this is the case, what's the reason for using user-add for
this?
Wouldn't it
be better to make this a separate command, say:
  ipa user-activate tuser
  ipa user-activate tuser --from-deleted
  ipa user-activate tuser --from-deleted --to-staged

+1, I would even go as far as having separate commands for staged
and
deleted users, e.g.:

 ipa user-unstage tuser
 ipa user-undelete tuser
 ipa user-undelete tuser --to-staged

A deleted entry has already been active so it contains already set
attributes while the pure staged entries are almost empty boxes.
But
from an administrator point of view, both staged/deleted entries are
inactive. What would be the advantages of two separated commands ?

You just said it yourself: activating/unstaging a user is quite
different from undeleting a user. Cramming multiple different
operations in a single command is bad design IMHO.

Ok I understand.
I believe that deleted entries and staged entries will be in the same
container (provisioning).

The design page mentions cn=staged
users,cn=accounts,cn=provisioning,$SUFFIX and cn=deleted
users,cn=accounts,cn=provisioning,$SUFFIX, which are two different
containers.

Oppsss.. Sorry for the confusion :-[

So we may have at least those two possibilities:

   * ipa user-activate tuser [--from-staging|--from-delete]
   * ipa user-unstage tuser
 ipa user-undelete tuser


I like the idea of different verbs for different hives.
Something like:

Adding directly to stage via CLI: ipa user-stage
Removing from stage: user-unstage (user is gone)
Stage to Main - activate; - deactivate
Main to delete - del; -restore or undelete
Delete to stage - I think we can use ipa user-stage command with
--deleted=user or similar

To be honest, I don't like this idea.
Too many names are confusing, if we can find a consistent option to cut the
number of names down we should do it.
IMO This is the worst part of Git:
http://assets.osteele.com/images/2008/git-transport.png . We can do better.

Another good thing would be if options did not affect the applicability of
other options (too much). For example in your proposal there'd be something 
like:
 ipa user-stage tuser --first=abc --last=xyz --phone=123 ..
 ipa user-stage --deleted=tuser  # no attribute options allowed
We should avoid this, if only for the reason that it makes the help text
confusing.


My proposal would be that the move commands use the verb for the target and an
option for the source, and add/mod use an option for the container:

1) adding a new user
(to active)   ipa user-add tuser ...
(to stage)ipa user-add tuser --staged ...
(to deleted)  ipa user-add tuser --deleted ...  (*)

2) moving to main
(from stage)  ipa user-activate tuser  (**)
(from del)ipa user-activate tuser --deleted

3) 

Re: [Freeipa-devel] [PATCH 0052] Only specify the ipatokenuniqueid default in the add operation

2014-05-22 Thread Nathaniel McCallum
On Thu, 2014-05-22 at 17:13 +0200, Petr Vobornik wrote:
 On 22.5.2014 17:00, Nathaniel McCallum wrote:
  On Thu, 2014-05-22 at 10:53 -0400, Nathaniel McCallum wrote:
  On Thu, 2014-05-22 at 16:45 +0200, Petr Viktorin wrote:
  On 05/22/2014 04:12 PM, Nathaniel McCallum wrote:
  On Tue, 2014-05-13 at 12:55 -0400, Nathaniel McCallum wrote:
  On Tue, 2014-05-13 at 16:47 +0200, Jan Cholasta wrote:
  On 12.5.2014 20:50, Nathaniel McCallum wrote:
  On Mon, 2014-05-12 at 18:40 +0200, Misnyovszki Adam wrote:
  On Tue, 06 May 2014 11:46:14 -0400
  Nathaniel McCallum npmccal...@redhat.com wrote:
 
  On Tue, 2014-05-06 at 11:38 -0400, Nathaniel McCallum wrote:
  On Tue, 2014-05-06 at 17:34 +0200, Petr Vobornik wrote:
  On 6.5.2014 17:13, Nathaniel McCallum wrote:
  On Tue, 2014-05-06 at 17:04 +0200, Petr Vobornik wrote:
  On 6.5.2014 16:51, Nathaniel McCallum wrote:
  Specifying the default in the LDAP Object causes the
  parameter to be specified for non-add operations. This is
  especially problematic when performing the modify operation
  as it causes the primary key to change for every modification.
 
  https://fedorahosted.org/freeipa/ticket/4227
 
 
  shouldn't removal of `autofill=True,` be enough?
 
  Removing autofill=True results in the default not being used
  for the otptoken-add operation. That may be a different bug
  (I'm not sure what the expectation of autofill is).
 
  Nathaniel
 
 
  Seems to work form me with:
 
  diff --git a/ipalib/plugins/otptoken.py
  b/ipalib/plugins/otptoken.py index f68ea7d..623f1f1 100644
  --- a/ipalib/plugins/otptoken.py
  +++ b/ipalib/plugins/otptoken.py
  @@ -121,9 +121,7 @@ class otptoken(LDAPObject):
   cli_name='id',
   label=_('Unique ID'),
   default_from=lambda: unicode(uuid.uuid4()),
  -autofill=True,
   primary_key=True,
  -flags=('optional_create'),
   ),
   StrEnum('type?',
   label=_('Type'),
 
  Doing this causes the ipa otptoken-add command to prompt for the
  Unique ID. This may be the desired behavior, but it is not how it
  worked previously (no prompt).
 
  Here is an alternate patch for this second approach. I have no 
  strong
  opinion on the correct behavior here.
 
  Nathaniel
 
  IMO you should update API.txt with ./makeapi
 
  Running ./makeapi results in no changes to API.txt.
 
  This is not right, there *are* changes in the API and build fails for 
  me
  becase API.txt is not updated.
 
  I think maybe I ran it from the wrong branch. Fixed.
 
  I still need a review of this. It is pretty trivial.
 
  Nathaniel
 
  This still prompts for the unique ID on add:
 
  $ ipa otptoken-add
  Unique ID [25cb3aa9-db19-40f8-acf4-33ef65ca863c]:
 
  I don't think that's the intended behavior.
 
  Hence the alternate patch (0052a). If we don't want to prompt, we'll
  need to use the first patch (0052). I have no strong opinion on the
  correct behavior and I am fine with merging either patch.
 
  Attached is the non-alternate (0052) with the api updated.
 
  Nathaniel
 
 IMO 52a is better if used by hand and it keeps code cleaner. It might 
 not be ideal though if used from a script because of nonexistent 
 --unattended/-u option which would disable prompt (set 
 env.interactive=False ?).

It only prompts if the id is not specified. If you are using it in a
script you can just create your own random UUID and specify it. This is
exactly what I do in the import script patch.

Nathaniel

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


Re: [Freeipa-devel] Status/Question about User life cycle

2014-05-22 Thread Simo Sorce
On Thu, 2014-05-22 at 17:52 +0200, thierry bordaz wrote:
 On 05/22/2014 04:38 PM, Martin Kosek wrote:
  On 05/22/2014 10:47 AM, Petr Viktorin wrote:
  On 05/21/2014 10:00 PM, Dmitri Pal wrote:
  On 05/19/2014 10:45 AM, thierry bordaz wrote:
  On 05/19/2014 04:44 PM, Jan Cholasta wrote:
  On 19.5.2014 16:34, thierry bordaz wrote:
  On 05/19/2014 04:22 PM, Jan Cholasta wrote:
  On 19.5.2014 16:03, thierry bordaz wrote:
  On 05/19/2014 03:54 PM, Jan Cholasta wrote:
  On 19.5.2014 15:19, Petr Viktorin wrote:
  Hello list,
  Here's a conversation that started internally. I'm making it
  public.
 
  On 05/19/2014 01:00 PM, Martin Kosek wrote:
  On 05/19/2014 12:46 PM, Petr Viktorin wrote:
  On 05/19/2014 08:25 AM, Martin Kosek wrote:
  On 05/19/2014 08:24 AM, Martin Kosek wrote:
  On 05/16/2014 04:48 PM, thierry bordaz wrote:
  Hello Martin,
 
I am getting familiar with the freeipa CLI code and
  started
implemented '--to-stage' and '--from-stage'. This
  really an
impressive set of code :-)
  Great! :-)
 
I completed 'to-stage' and testing '--from-stage'.
 
I have a question regarding the '--from-stage' syntax.
  'uid'
  is a
mandatory argument to 'user-add' subcommand. In the
  design the
'--from-stage' option is described with:
 
ipa user-add --from-stage=tuser
  Note, the design is here:
  http://www.freeipa.org/page/V4/User_Life-Cycle_Management
 
But as 'uid' is mandatory the command should rather be
 
ipa user-add tuser --from-stage=tuser
 
In that case the option value for '--from-stage' is not
  required and
the command should be
 
ipa user-add tuser --from-stage
 
Is that ok if I implement the command like above or did 
  I
  miss
something ?
 
regards
thierry
  Hmm, no, I think you are right.  We can change --from-stage to
  just
  Bool
  parameter. When it is true, it'd mean that get_dn or
  pre-callback
  should
  retrieve the record from stage and use all it's attributes (and
  add
  standard
  default attributes values on top of that).
 
  Also CC-ing Petr Viktorin for reference.
  This operation can't change the user's attributes, can it?
  I.e., we
  don't
  support something like:
ipa user-add tuser --from-stage --phone=123456789
  --email=newem...@example.com
  If this is the case, what's the reason for using user-add for
  this?
  Wouldn't it
  be better to make this a separate command, say:
ipa user-activate tuser
ipa user-activate tuser --from-deleted
ipa user-activate tuser --from-deleted --to-staged
  +1, I would even go as far as having separate commands for staged
  and
  deleted users, e.g.:
 
   ipa user-unstage tuser
   ipa user-undelete tuser
   ipa user-undelete tuser --to-staged
  A deleted entry has already been active so it contains already set
  attributes while the pure staged entries are almost empty boxes.
  But
  from an administrator point of view, both staged/deleted entries are
  inactive. What would be the advantages of two separated commands ?
  You just said it yourself: activating/unstaging a user is quite
  different from undeleting a user. Cramming multiple different
  operations in a single command is bad design IMHO.
  Ok I understand.
  I believe that deleted entries and staged entries will be in the same
  container (provisioning).
  The design page mentions cn=staged
  users,cn=accounts,cn=provisioning,$SUFFIX and cn=deleted
  users,cn=accounts,cn=provisioning,$SUFFIX, which are two different
  containers.
  Oppsss.. Sorry for the confusion :-[
  So we may have at least those two possibilities:
 
 * ipa user-activate tuser [--from-staging|--from-delete]
 * ipa user-unstage tuser
   ipa user-undelete tuser
 
  I like the idea of different verbs for different hives.
  Something like:
 
  Adding directly to stage via CLI: ipa user-stage
  Removing from stage: user-unstage (user is gone)
  Stage to Main - activate; - deactivate
  Main to delete - del; -restore or undelete
  Delete to stage - I think we can use ipa user-stage command with
  --deleted=user or similar
  To be honest, I don't like this idea.
  Too many names are confusing, if we can find a consistent option to cut the
  number of names down we should do it.
  IMO This is the worst part of Git:
  http://assets.osteele.com/images/2008/git-transport.png . We can do better.
 
  Another good thing would be if options did not affect the applicability of
  other options (too much). For example in your proposal there'd be 
  something like:
   ipa user-stage tuser --first=abc --last=xyz --phone=123 ..
   ipa user-stage --deleted=tuser  # no attribute options allowed
  We should avoid this, if only for the reason that it makes the help text
  confusing.
 
 
  My proposal would be that the move commands use the verb for the target 
  and an
  option for the source, and add/mod use an option for the 

Re: [Freeipa-devel] [PATCH 0048] Default the token owner to the person adding the token

2014-05-22 Thread Simo Sorce
On Thu, 2014-05-22 at 10:21 -0400, Nathaniel McCallum wrote:
 I still need a review on this.

LGTM.

Simo.

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

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


Re: [Freeipa-devel] User life cycle: question regarding the design

2014-05-22 Thread Simo Sorce
On Thu, 2014-05-22 at 15:35 +0200, Martin Kosek wrote:
 On 05/21/2014 10:11 PM, Dmitri Pal wrote:
  On 05/21/2014 03:06 PM, Martin Kosek wrote:
  On 05/21/2014 08:14 PM, Simo Sorce wrote:
  On Wed, 2014-05-21 at 16:01 +0200, thierry bordaz wrote:
  Hello,
 
   Thanks for all these detailed descriptions.
   Just to be sure to be on the same page, here is my understanding of
   the provisioning templates and placeholder definitions. An
   administrator can provide a provisioning template. I suppose it
   would be a file containing a lines of placeholder definitions.
 
 * Where is located the template file ? Is there a standard
   repository where templates are put ? (somewhere under 
  /etc/ipa/* ?)
 
  FreeIPA is a multi-master system, a file stored in a file would be
  extremely cumbersome to use as it would require the admin to manually
  copy it for every new replica and then keep it in sync.
  It would also make it hard to change 'on-line'.
 
  Placeholders should be defined in an object similar to
  cn=ipaConfig,cn=etc,$suffix
 
 * Is there an already defined syntax for the provisionning
   template. ('$' is separator attr/value, %{attr} is substitute
   pattern...). If not, is it possible to user ':space ' as
   separator ?
 
  Using initial and final ? like in Martin's example doesn't work ?
 
 * What is the priority. The user can provide the 'homeDirectory'
   through different methods. Is it ok to use the following order:
 o the CLI option
 o the provisionning template
 o the default config value (in cn=ipaConfig,cn=etc,$SUFFIX)
 
   For example, if it exists the provisioning template:
   /etc/ipa/provisioning/shell-user.template
 
   roomnumber$-2
   homeDirectory$/home/net/shell-%{uid}
   loginShell$?shell-plugin-autogenerate?
 
  I do not understand this, we are not building a templating engine here,
  you only have 2 options:
  1) a required (MUST) attribute has an explicit value
  2) a require (MUST) attribute has a placeholder value
 
  the placeholder value is fixed per type, and what it is substituted with
  uses the same rules as the current code uses to autogenerate values.
 
   the command: ipa user-add tuser
   --homedir=/tmp/tuser--roomnumber=1234 --to-stage would create a
   staging entry:
 
   dn: uid=tuser,cn=staged users,cn=provisioning,$SUFFIX
   ...
   roomNumber: 1234
   homeDirectory: /tmp/tuser
   loginShell: shell-plugin-autogenerate
 
  loginShell is a MAY attribute, not a MUST attribute, so nothing should
  be stored at all in the staged entry unless explicitly provided for by
  the admin.
 
   Then a private DS plugin (catching shell-plugin-autogenerate)
   generate the loginShell value when the entry becomes active.
 
 
   the command: ipa user-add tuser --homedir=/tmp/tuser--to-stage would
   create a staging entry:
 
   dn: uid=tuser,cn=staged users,cn=provisioning,$SUFFIX
   ...
   roomNumber: -2
   homeDirectory: /tmp/tuser
   loginShell: shell-plugin-autogenerate
 
  roomNumber is also a MAY, so what would cause it to be set at -2, and
  why ?
 
   the command: ipa user-add tuser --to-stage would create a staging 
  entry:
 
   dn: uid=tuser,cn=staged users,cn=provisioning,$SUFFIX
   ...
   roomNumber: -2
   homeDirectory: /home/net/shell-tuser
   loginShell: shell-plugin-autogenerate
 
  homeDirectory should be something like: ?placeholder? IMO, we do not
  really want to play templating here.
 
   In case the provisioning template does not define 'homeDirectory',
   then the created entry would take the value from the ipa config
   definition:
 
  that value should be taken and applied at the time the user is unstaged
  and brought in the actual tree, not at the time a user is staged.
 
  HTH,
  Simo.
 
 
  Hello Thierry and Simo,
 
  I think Thierry was confused with this part of the design:
 
  
  This format of placeholders gives enough space for future enhancements. For
  example, Administrator could configure a new template
  myhomedirtemplate$/home/net/%{uid} and use it in the staged LDAP entry.
  Such value would be replaced by /home/net/tuser if user uid attribute is 
  set
  to tuser
  
 
  My intention when writing this design was to enable future use of
  configurable placeholders, i.e. a value ?someplaceholder? could be turn
  into /custom/path/%{uid}. But I meant that this can be considered as a
  future enhancement. For now, I think implementing a placeholder -1 for
  numerical values and ?autogenerate? for string ones a good start.
 
  Martin
 
  ___
  Freeipa-devel mailing list
  Freeipa-devel@redhat.com
  https://www.redhat.com/mailman/listinfo/freeipa-devel
  
  
  Please consider the flow: user added staged - activated/moved to main tree 
  -
  

[Freeipa-devel] Understanding FreeIPA replica internals

2014-05-22 Thread James
I'm trying to understand some of the FreeIPA replication internals so
that I can better know how to do this properly in Puppet without
storing any secret information in Puppet, and so that automating
FreeIPA is awesome.

Please point me to any docs, if there is reading I could be doing :)

Here are some open questions I have:

1) Is the GPG file created with ipa-replica-prepare using a symmetric
password and is that password equal to the dm_password ? If not, where
do the pub/priv key pairs come from and how do they get transferred to
the replica.

2) If I have root on the IPA server (actually all of them) how can I
run ipa-replica-prepare without needing interactive prompting for
entering the password. It's not possible with puppet. Is there another
(possibly less user friendly even) method to prepare the replica?
What is prepare actually doing?

3) With a multi master setup, what happens if I run the same action
(eg: user-mod or user-add or user-del) on more than one server. Can I
run it on any server? What if I run different user-mod commands of the
same user on different masters. Is there split brain? Are all the
transactions and writes synchronous across the whole cluster? Please
point me to a doc that explains this FAQ stuff if possible. Sorry for
the noise

Thanks again,
James

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


Re: [Freeipa-devel] [PATCH 0048] Default the token owner to the person adding the token

2014-05-22 Thread Jan Cholasta

On 22.5.2014 16:21, Nathaniel McCallum wrote:

I still need a review on this.

On Wed, 2014-05-07 at 10:06 -0400, Nathaniel McCallum wrote:

On Wed, 2014-05-07 at 15:54 +0200, Petr Vobornik wrote:

On 6.5.2014 17:07, Nathaniel McCallum wrote:

On Tue, 2014-05-06 at 16:11 +0200, Jan Cholasta wrote:

On 6.5.2014 15:16, Nathaniel McCallum wrote:

On Tue, 2014-05-06 at 13:46 +0200, Jan Cholasta wrote:

Hi,

On 5.5.2014 18:40, Nathaniel McCallum wrote:

Creating tokens for yourself is the most common operation. Making this
the default optimizes for the common case.


The user-find call should be inside the if statement.


This is actually for a reason. See my patch 0049 for further context.


IMO something like this would be better:

   if 'ipatokenowner' not in entry_attrs or 'ipatokenprotected' not in
entry_attrs:
   result = self.api.Command.user_find(whoami=True)['result']
   if result:
   cur_uid = result[0]['uid'][0]
   prev_uid = entry_attrs.setdefault('ipatokenowner', cur_uid)
   if cur_uid != prev_uid:
   entry_attrs.setdefault('ipatokenprotected', True)


Fixed (see also my new revision of patch 0049).

Nathaniel



I assume that this won't allow to create a token without an owner. Do we
want to have this restriction?

Usecase: import a batch of hw tokens


This case is currently very much on my radar (I'm finishing the import
script now). To set no owner, just use --owner=. We are testing for
key presence here, not the value of the key. So if the key is present
with an empty value, no owner will be set.

FYI, the import format (RFC 6030) also permits a mechanism for declaring
ownership in DN format.

Nathaniel

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



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



ACK.

--
Jan Cholasta

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