<html>
<head>
<meta content="text/html; charset=windows-1252"
http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
<div class="moz-cite-prefix">On 03/24/2015 01:45 PM, Jan Cholasta
wrote:<br>
</div>
<blockquote cite="mid:55115C7E.1090306@redhat.com" type="cite">Dne
19.3.2015 v 13:07 thierry bordaz napsal(a):
<br>
<blockquote type="cite">On 03/19/2015 07:37 AM, Jan Cholasta
wrote:
<br>
<blockquote type="cite">Dne 18.3.2015 v 19:39 thierry bordaz
napsal(a):
<br>
<blockquote type="cite">On 03/17/2015 08:01 AM, Jan Cholasta
wrote:
<br>
<blockquote type="cite">Dne 16.3.2015 v 12:06 David Kupka
napsal(a):
<br>
<blockquote type="cite">On 03/06/2015 07:30 PM, thierry
bordaz wrote:
<br>
<blockquote type="cite">On 02/19/2015 04:19 PM, Martin
Basti wrote:
<br>
<blockquote type="cite">On 19/02/15 13:01, thierry
bordaz wrote:
<br>
<blockquote type="cite">On 02/04/2015 05:14 PM, Jan
Cholasta wrote:
<br>
<blockquote type="cite">Hi,
<br>
<br>
Dne 4.2.2015 v 15:25 David Kupka napsal(a):
<br>
<blockquote type="cite">On 02/03/2015 11:50 AM,
thierry bordaz wrote:
<br>
<blockquote type="cite">On 09/17/2014 12:32
PM, thierry bordaz wrote:
<br>
<blockquote type="cite">On 09/01/2014 01:08
PM, Petr Viktorin wrote:
<br>
<blockquote type="cite">On 08/08/2014
03:54 PM, thierry bordaz wrote:
<br>
<blockquote type="cite">Hi,
<br>
<br>
The attached patch is related to 'User
Life Cycle'
<br>
(<a class="moz-txt-link-freetext" href="https://fedorahosted.org/freeipa/ticket/3813">https://fedorahosted.org/freeipa/ticket/3813</a>)
<br>
<br>
It creates a stageuser plugin with a
first function
<br>
stageuser-add.
<br>
Stage
<br>
user entries are provisioned under
'cn=staged
<br>
users,cn=accounts,cn=provisioning,SUFFIX'.
<br>
<br>
Thanks
<br>
thierry
<br>
</blockquote>
<br>
Avoid `from ipalib.plugins.baseldap
import *` in new code;
<br>
instead
<br>
import the module itself and use e.g.
`baseldap.LDAPObject`.
<br>
<br>
The stageuser help (docstring) is copied
from the user
<br>
plugin, and
<br>
discusses things like account lockout
and disabling users. It
<br>
should
<br>
rather explain what stageuser itself
does. (And I don't very
<br>
much
<br>
like the Note about the interface being
badly designed...)
<br>
Also decide if the docs should call it
"staged user" or "stage
<br>
user"
<br>
or "stageuser".
<br>
<br>
A lot of the code is copied and pasted
over from the users
<br>
plugin.
<br>
Don't do that. Either import things
(e.g.
<br>
validate_nsaccountlock)
<br>
from the users plugin, or move the
reused code into a shared
<br>
module.
<br>
<br>
For the `user` object, since so much is
the same, it might be
<br>
best to
<br>
create a common base class for user and
stageuser; and
<br>
similarly
<br>
for
<br>
the Command plugins.
<br>
<br>
The default permissions need different
names, and you don't
<br>
need
<br>
another copy of the 'non_object' ones.
Also, run the makeaci
<br>
script.
<br>
<br>
</blockquote>
Hello,
<br>
<br>
This modified patch is mainly moving
common base class
<br>
into a
<br>
new
<br>
plugin: accounts.py. user/stageuser
plugin inherits from
<br>
accounts.
<br>
It also creates a better description
of what are stage
<br>
user,
<br>
how
<br>
to add a new stage user, updates
ACI.txt and separate
<br>
active/stage
<br>
user managed permissions.
<br>
<br>
thanks
<br>
thierry
<br>
<br>
<br>
<br>
<br>
<br>
<br>
_______________________________________________
<br>
Freeipa-devel mailing list
<br>
<a class="moz-txt-link-abbreviated" href="mailto:Freeipa-devel@redhat.com">Freeipa-devel@redhat.com</a>
<br>
<a class="moz-txt-link-freetext" href="https://www.redhat.com/mailman/listinfo/freeipa-devel">https://www.redhat.com/mailman/listinfo/freeipa-devel</a>
<br>
</blockquote>
<br>
<br>
Thanks David for the reviews. Here the last
patches
<br>
<br>
<br>
<br>
<br>
_______________________________________________
<br>
Freeipa-devel mailing list
<br>
<a class="moz-txt-link-abbreviated" href="mailto:Freeipa-devel@redhat.com">Freeipa-devel@redhat.com</a>
<br>
<a class="moz-txt-link-freetext" href="https://www.redhat.com/mailman/listinfo/freeipa-devel">https://www.redhat.com/mailman/listinfo/freeipa-devel</a>
<br>
<br>
</blockquote>
<br>
The freeipa-tbordaz-0002 patch had trailing
whitespaces on few
<br>
lines so
<br>
I'm attaching fixed version (and unchanged
patch
<br>
freeipa-tbordaz-0003-3
<br>
to keep them together).
<br>
<br>
The ULC feature is still WIP but these patches
look good to me
<br>
and
<br>
don't
<br>
break anything as far as I tested.
<br>
We should push them now to avoid further
rebases. Thierry can
<br>
then
<br>
prepare other patches delivering the rest of
ULC functionality.
<br>
</blockquote>
<br>
Few comments from just reading the patches:
<br>
<br>
1) I would name the base class "baseuser",
"account" does not
<br>
necessarily mean user account.
<br>
<br>
2) This is very wrong:
<br>
<br>
-class user_add(LDAPCreate):
<br>
+class user_add(user, LDAPCreate):
<br>
<br>
You are creating a plugin which is both an
object and an command.
<br>
<br>
3) This is purely subjective, but I don't like
the name
<br>
"deleteuser", as it has a verb in it. We usually
don't do that and
<br>
IMHO we shouldn't do that.
<br>
<br>
Honza
<br>
<br>
</blockquote>
<br>
Thank you for the review. I am attaching the
updates patches
<br>
<br>
<br>
<br>
<br>
<br>
<br>
_______________________________________________
<br>
Freeipa-devel mailing list
<br>
<a class="moz-txt-link-abbreviated" href="mailto:Freeipa-devel@redhat.com">Freeipa-devel@redhat.com</a>
<br>
<a class="moz-txt-link-freetext" href="https://www.redhat.com/mailman/listinfo/freeipa-devel">https://www.redhat.com/mailman/listinfo/freeipa-devel</a>
<br>
</blockquote>
Hello,
<br>
I'm getting errors during make rpms:
<br>
<br>
if [ "" != "yes" ]; then \
<br>
./makeapi --validate; \
<br>
./makeaci --validate; \
<br>
fi
<br>
<br>
/root/freeipa/ipalib/plugins/baseuser.py:641 command
"baseuser_add"
<br>
doc is not internationalized
<br>
/root/freeipa/ipalib/plugins/baseuser.py:653 command
"baseuser_find"
<br>
doc is not internationalized
<br>
/root/freeipa/ipalib/plugins/baseuser.py:647 command
"baseuser_mod"
<br>
doc is not internationalized
<br>
0 commands without doc, 3 commands whose doc is not
i18n
<br>
Command baseuser_add in ipalib, not in API
<br>
Command baseuser_find in ipalib, not in API
<br>
Command baseuser_mod in ipalib, not in API
<br>
<br>
There are one or more new commands defined.
<br>
Update API.txt and increment the minor version in
VERSION.
<br>
<br>
There are one or more documentation problems.
<br>
You must fix these before preceeding
<br>
<br>
Issues probably caused by this:
<br>
1)
<br>
You should not use the register decorator, if this
class is just for
<br>
inheritance
<br>
@register()
<br>
class baseuser_add(LDAPCreate):
<br>
<br>
@register()
<br>
class baseuser_mod(LDAPUpdate):
<br>
<br>
@register()
<br>
class baseuser_find(LDAPSearch):
<br>
<br>
see dns.py plugin and "DNSZoneBase" and "dnszone"
classes
<br>
<br>
2)
<br>
there might be an issue with
<br>
@register()
<br>
class baseuser(LDAPObject):
<br>
<br>
the register decorator should not be there, I was
warned by
<br>
Petr^3 to
<br>
not use permission in parent class. The same
permission should be
<br>
specified only in one place (for example user
class), (otherwise
<br>
they
<br>
will be generated twice??) I don't know more details
about it.
<br>
<br>
--
<br>
Martin Basti
<br>
</blockquote>
<br>
Hello Martin, Jan,
<br>
<br>
Thanks for your review.
<br>
I changed the patch so that it does not register
baseuser_*. Also
<br>
increase the minor version because of new command.
<br>
Finally I moved the managed_permission definition out
of the parent
<br>
baseuser class.
<br>
<br>
<br>
<br>
<br>
<br>
</blockquote>
<br>
Martin, could you please verify that the issues you
encountered are
<br>
fixed?
<br>
<br>
Thanks!
<br>
<br>
</blockquote>
<br>
You bumped wrong version variable:
<br>
<br>
-IPA_VERSION_MINOR=1
<br>
+IPA_VERSION_MINOR=2
<br>
<br>
It should have been IPA_API_VERSION_MINOR (at the bottom
of the file),
<br>
including the last change comment below it.
<br>
<br>
<br>
IMO baseuser should include superclasses for all the usual
commands
<br>
(add, mod, del, show, find) and stageuser/deleteuser
commands should
<br>
inherit from them.
<br>
<br>
<br>
You don't need to override class properties like
active_container_dn
<br>
and takes_params on baseuser subclasses when they have the
same value
<br>
as in baseuser.
<br>
<br>
<br>
Honza
<br>
<br>
</blockquote>
Hello Honza,
<br>
<br>
Thanks for the review. I did the modifications you
recommended
<br>
within that attached patches
<br>
<br>
* Change version
<br>
</blockquote>
<br>
Please also update the comment below (e.g. "# Last change:
tbordaz -
<br>
Add stageuser_add command")
<br>
<br>
<blockquote type="cite"> * create the baseuser_* plugins
commands and use them in the
<br>
user/stageuser plugin commands
<br>
* Do not redefine the class properties in the
subclasses.
<br>
</blockquote>
<br>
There are still some in baseuser command classes:
<br>
<br>
+class baseuser_add(LDAPCreate):
<br>
+ """
<br>
+ Prototype command plugin to be implemented by real plugin
<br>
+ """
<br>
+ active_container_dn = api.env.container_user
<br>
+ has_output_params = LDAPCreate.has_output_params
<br>
<br>
You don't need to set active_container_dn here, you only need
to set
<br>
it in baseuser. Then in stageuser_add and other subclasses you
use
<br>
"self.obj.active_container_dn" instead of
"self.active_container_dn".
<br>
<br>
You also don't need to override has_output_params if you are
not
<br>
changing its value - you are inheriting from LDAPCreate, so
<br>
baseuser_add.has_output_params implicitly has the same value
as
<br>
LDAPCreate.has_output_params.
<br>
<br>
<blockquote type="cite">
<br>
Thanks
<br>
thierry
<br>
<br>
</blockquote>
<br>
</blockquote>
<br>
Hello Honza,
<br>
<br>
Thanks for your patience .. :-)
<br>
I understand my mistake. Just a question, in a plugin
command
<br>
(user_add), is 'self.obj' referring to the plugin object
(like 'user') ?
<br>
</blockquote>
<br>
Yes, that's correct.
<br>
<br>
<blockquote type="cite">
<br>
updated patches (with the appropriate naming and patch
versioning).
<br>
<br>
thanks
<br>
theirry
<br>
<br>
</blockquote>
<br>
One more thing:
<br>
<br>
Instead of:
<br>
<br>
class stageuser(baseuser):
<br>
...
<br>
# take_params does not support 'nsaccountlock' option
<br>
stageuser_takes_params_list = []
<br>
for elt in baseuser.takes_params:
<br>
if isinstance(elt, Bool) and elt.param_spec ==
'nsaccountlock?':
<br>
pass
<br>
else:
<br>
stageuser_takes_params_list.append(elt)
<br>
takes_params = tuple(stageuser_takes_params_list)
<br>
<br>
I would remove nsaccountlock from baseuser.takes_params and add it
in user.takes_params:
<br>
<br>
class user(baseuser):
<br>
...
<br>
takes_params = baseuser.takes_params + (
<br>
Bool('nsaccountlock?',
<br>
label=_('Account disabled'),
<br>
flags=['no_option'],
<br>
),
<br>
)
<br>
<br>
<br>
</blockquote>
<font face="Times New Roman, Times, serif">Right, making this option
specific to active user makes sense.<br>
<br>
Thanks<br>
thierry<br>
</font>
</body>
</html>