date:20140526


On 05/26/2014 12:15 PM, Petr Viktorin wrote:

On 05/23/2014 02:26 PM, Martin Kosek wrote:

On 05/22/2014 04:03 PM, Petr Viktorin wrote:

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

[...]

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.


Would it make sense to publish your FreeIPA git hooks somewhere on
http://www.freeipa.org/page/Contribute/Code or your github and link
it? I think
it already contains couple gems that may help other people prevent
basic errors
like this one.


Sure, I'll document it a bit and publish.


I've put my post-commit hook here: 
https://github.com/encukou/ipa-tools/blob/master/hooks/post-commit.example


It's still a bunch of examples however, and sometimes the output is not 
too understandable by itself, so I can't in good conscience recommend it 
to someone just starting to hack on FreeIPA.


Feel free to use & improve it though.

--
Petr³

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

Re: [Freeipa-devel] [PATCH 0004] freeipa-ci: Add sudo integration job


On 05/14/2014 04:56 PM, Tomas Babej wrote:

Hi,

the sudo integration job is already in master, so it's time for the job
to be pushed to the upstream test job repository.

Tomas



Thanks, ACK, pushed to CI master: c691941610f2d431867938e6438f36d7ec3cddc1


--
Petr³

 git pull https://github.com/encukou/freeipa-ci.git master

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

Re: [Freeipa-devel] OTP Sync Client Design

2014-05-26 Thread Nathaniel McCallum

On Mon, 2014-05-26 at 09:56 +0200, Jan Cholasta wrote:
> On 23.5.2014 23:19, Nathaniel McCallum wrote:
> > On Wed, 2014-05-14 at 14:08 -0400, Nathaniel McCallum wrote:
> >> Occasionally OTP tokens get out of sync with the server. When this
> >> happens, the user or an admin need to synchronize the token. To this
> >> end, we landed server-side synchronization support, which is a simple
> >> bind with a custom control. This all works with my sample test script.
> >>
> >> Client support is proving a bit more difficult. In the ideal world, the
> >> client would contact LDAP directly and perform the operation. This would
> >> make a man in the middle attack difficult and we can ensure encryption
> >> over the entire operation.
> >>
> >> However, browsers, without server-side assistance, cannot perform this
> >> operation from javascript. This means that, in this case, the first
> >> factor and two second factors must be transmitted to the FreeIPA server
> >> and then proxied to 389. Is this an acceptable compromise?
> >>
> >> This command also needs to be accessible *without* an existing user
> >> login since, if a user's token is out of sync, the user can't login. Is
> >> it possible to expose such an API? If so, how? Both "ipa env" and "ipa
> >> ping" seem to require kinit, so I don't see any obvious examples.
> >
> > Thanks everyone for your feedback. This particular feature is proving
> > difficult to implement, even with our agreed upon design. To reiterate
> > this design: there will be an HTTP method by which to synchronize
> > tokens.
> >
> > There are two assumptions in the code which are making this difficult:
> > 1. All cli commands are Command subclasses.
> > 2. All Command subclasses create authenticated server methods.
> >
> > There are thus two ways to tackle this problem.
> >
> > First, I can create a standard POST method in rpcserver.py. This is not
> > very modular. But the biggest problem is that there is no way to create
> > the cli-side command to call it (assumption #1).
> 
> Well, you could derive the command from ipalib.frontend.Local and 
> manually call the POST method from it.

This still creates a (NoOp) server-side RPC method, right? We can
probably just accept this as a drawback for now and move on. After a
future refactoring of rpcserver.py, we can move the manual POST method
into this server-side RPC method.

> > Second, I can create a Command subclass, similar to the passwd plugin.
> > This will create both the client- and server-side components. However,
> > there is no way to disable the server-side authentication.
> >
> > I think that solving the second of these problems is the most reusable.
> > Just as an example, the ping command currently requires authentication
> > but does not need to do so. The passwd Command too shouldn't need to
> > authenticate before executing the command because the command
> > authenticates itself.
> >
> > I think it very likely that we are going to have need for other Command
> > subclasses in the future which do not require authentication.
> >
> > However, implementing this approach is rather difficult as it will
> > require a refactoring of rpcserver.py. The code in rpcserver.py contains
> > many layering violations and the class structure is rather unclear
> > (look, for instance, at the different orders in which xmlrpc and jsonrpc
> > classes inherit from their parent classes).
> >
> > The current problem forcing this refactoring is that authentication
> > appears to happen across several different layers, but always before the
> > command to be executed is unmarshalled. We need to invert this order:
> > the command needs to be unmarshalled first in order to determine whether
> > or not authentication is necessary. I don't think that switching this
> > order is practical without constraining authentication to a single layer
> > (or two: session and krb5) late in the request process.
> >
> > Git tells me that lots of people have touched this code, so I'm hoping
> > for good feedback! ;)
> >
> > Alternatively, we could create a way to inject cli commands without
> > having Command subclasses. Isolating these concerns is itself probably a
> > good design choice. Ideally we'd have a structure where the Command
> > class itself inherits from a CLICommand class and a ServerMethod class.
> > But this too will be a massive refactoring, perhaps even bigger than the
> > rpcserver.py refactoring.
> >
> > So, which assumption should we break: #1 or #2? And who wants to help me
> > do it? Also, I am all ears for easier solutions for this feature.
> 
> I would go for the refactoring, the rpcserver code does indeed need some 
> love.
> 
> >
> > Nathaniel
> 


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

Re: [Freeipa-devel] [PATCHES 187-201] Improvements and coverage for sudorule plugin


On 05/20/2014 06:15 PM, Tomas Babej wrote:

Hi,

the following set of patches fixes:

https://fedorahosted.org/freeipa/ticket/4274
https://fedorahosted.org/freeipa/ticket/4263
https://fedorahosted.org/freeipa/ticket/4324
https://fedorahosted.org/freeipa/ticket/4340
https://fedorahosted.org/freeipa/ticket/4341

and additional minor issues.

The improvemed CI test coverage for the sudorule plugin is added as a bonus.


Thanks!
I haven't tested yet; here are my comments on the code.


0187 - sudorule: PEP8 fixes in sudorule.py

This change:


@@ -527,11 +551,15 @@ def pre_callback(self, ldap, dn, found, not_found, *keys, 
**options):
 _entry_attrs = ldap.get_entry(dn, self.obj.default_attributes)
 except errors.NotFound:
 self.obj.handle_not_found(*keys)
-if is_all(_entry_attrs, 'hostcategory'):
-raise errors.MutuallyExclusiveError(reason=_("hosts cannot be added 
when host category='all'"))
+
+if is_all(self._entry_attrs, 'hostcategory'):
+raise errors.MutuallyExclusiveError(
+reason=_("hosts cannot be added when host category='all'"))
+
 return add_external_pre_callback('host', ldap, dn, keys, options)


adds `self` to `self._entry_attrs`. That should be a part of the next patch.
Git tip: `git log --word-diff` helps a lot when you play with whitespace

(Speaking of PEP8, if you could remove the baseldap star import from 
sudorule.py, it would be great...)




General thoughts:

Would it be possible to merge schema_compat.uldif and 
install/updates/10-schema_compat.update into one file? Is the uldif 
special somehow? I guess this is a question for Rob.
It would be nice to add a link to some schema-compat-entry-attribute 
documentation to these files.




0188 - sudorule: Allow using hostmasks for setting allowed hosts

Here:


--- a/install/updates/40-delegation.update
+++ b/install/updates/40-delegation.update
@@ -174,7 +174,7 @@ default:member: cn=Sudo 
Administrator,cn=privileges,cn=pbac,$SUFFIX
 dn: $SUFFIX
 add:aci: '(target = "ldap:///ipauniqueid=*,cn=sudorules,cn=sudo,$SUFFIX";)(version 3.0;acl 
"permission:Add Sudo rule";allow (add) groupdn = "ldap:///cn=Add Sudo 
rule,cn=permissions,cn=pbac,$SUFFIX";)'
 add:aci: '(target = "ldap:///ipauniqueid=*,cn=sudorules,cn=sudo,$SUFFIX";)(version 3.0;acl 
"permission:Delete Sudo rule";allow (delete) groupdn = "ldap:///cn=Delete Sudo 
rule,cn=permissions,cn=pbac,$SUFFIX";)'
-add:aci: '(targetattr = "description || ipaenabledflag || usercategory || hostcategory || cmdcategory || 
ipasudorunasusercategory || ipasudorunasgroupcategory || externaluser || ipasudorunasextuser || ipasudorunasextgroup || 
memberdenycmd || memberallowcmd || memberuser")(target = 
"ldap:///ipauniqueid=*,cn=sudorules,cn=sudo,$SUFFIX";)(version 3.0;acl "permission:Modify Sudo 
rule";allow (write) groupdn = "ldap:///cn=Modify Sudo rule,cn=permissions,cn=pbac,$SUFFIX";)'
+add:aci: '(targetattr = "description || ipaenabledflag || usercategory || hostcategory || cmdcategory || 
ipasudorunasusercategory || ipasudorunasgroupcategory || externaluser || ipasudorunasextuser  || ipasudorunasextgroup 
|| memberdenycmd || memberallowcmd || memberuser || hostmask")(target = 
"ldap:///ipauniqueid=*,cn=sudorules,cn=sudo,$SUFFIX";)(version 3.0;acl "permission:Modify Sudo 
rule";allow (write) groupdn = "ldap:///cn=Modify Sudo rule,cn=permissions,cn=pbac,$SUFFIX";)'

 remove:aci: '(targetattr = "description")(target = 
"ldap:///sudocmd=*,cn=sudocmds,cn=sudo,$SUFFIX";)(version 3.0;acl "permission:Modify Sudo 
command";allow (write) groupdn = "ldap:///cn=Modify Sudo command,cn=permissions,cn=pbac,$SUFFIX";)'
 remove:aci: '(target = "ldap:///sudocmd=*,cn=sudocmds,cn=sudo,$SUFFIX";)(version 3.0;acl 
"permission:Delete Sudo command";allow (delete) groupdn = "ldap:///cn=Delete Sudo 
command,cn=permissions,cn=pbac,$SUFFIX";)'


you'll want to remove the old ACI before adding the new version.

In get_options of sudorule_add_host and sudorule_remove_host, it would 
be DRY-er to put the hostmask in a shared variable.


You write:

-_entry_attrs = ldap.get_entry(dn, self.obj.default_attributes)
+self._entry_attrs = ldap.get_entry(dn, self.obj.default_attributes)


Never store anything on the Command object. They're currently 
singletons. Another thread could change _entry_attrs to a different value.

This affects all the other uses of self._entry_attrs, of course.

In:

+num_added = len(new_masks.difference(old_masks))

and:

+entry_attrs['hostmask'] = list(old_masks.union(new_masks))

and:

+num_added = len(removed_masks.intersection(old_masks))

etc.: with two sets, you can use the `-`, `|`, and `&` operators.


Since you're touching this:

+return add_external_post_callback('memberhost', 'host', 'externalhost',
+  ldap, completed, failed, dn,
+  entry_attrs, keys, options

Re: [Freeipa-devel] [PATCH] 592-628 Update to PatternFly


On 19.5.2014 14:58, Petr Vobornik wrote:

On 12.5.2014 17:46, Misnyovszki Adam wrote:

Hi,
see my review notes below:

On Mon, 05 May 2014 18:41:13 +0200
Petr Vobornik  wrote:


This patchset updates Bootstrap 2 based RCUE to Bootstrap 3 based
PatternFly (v0.2.4) according to plan described at:

http://www.redhat.com/archives/freeipa-devel/2014-April/msg00045.html

The rest of the patches are mostly response to new CSS styles + some
new functionality and simplification of UI:
- css cleanup, images cleanup
- adjustment of stand-alone pages to PF
- adjustment of DOM structure to Bootstap 3 structure
- BS 3 enabled to change absolute positioned layout to responsive
fluid layout
- new activity indicators (since the old didn't fit into PF
navigation)
- new pager styles + additional behavior
- action select transform into dropdown and moved to control-button
section, making the header responsive
- fluid layout requested removal of computation of columns widths
- removal of login.html and logout.html
- new login background (the old one did not work with PF styles)
- new dialog styles
- + additional adjustments to use PF

The result is that UI uses most of PatternFly styles and is
responsive.

Fixes:
https://fedorahosted.org/freeipa/ticket/4177 - Better indication of
ongoing activity if dialog is opened

  - working progress could have a border. if it is over a dialog,
sometimes it looks messy over text



Fixed


https://fedorahosted.org/freeipa/ticket/4136 - WebUI unusable on
Cellphone screen

  - when I open the menu in 320x480, and select and navigate to an item,
the menu stays open - needs more investigation, if it is freeipa ui
issue


Fixed


  - qr code is fixed size in otp tokens, doesn't look nice on small
screens
not a problem, user just clicks on qr code link


Fixed


  - when a table header is longer, than the actual screen size, overflow
hidden occurs, unable to use buttons at the end of the header eg DNS
Resource Records, 320x480px, sometimes delete and add button
overflows the table, you can only scroll that table with tap
not a problem, responsive table works this way



I did not encounter overflow hidden issue - scrollbars were present and
I could scroll to the icons.


  - in 320x480, login page configuration text overflows on a white
background, especially if there is a login error, which makes the
white text unreadable


Behavior was improved.




https://fedorahosted.org/freeipa/ticket/4255 - Web UI: Display
"Loading" message when a list of entries is being loaded

see working progress comment above

https://fedorahosted.org/freeipa/ticket/3435 - [RFE] Remove width
limit in UI

ACK - PatternFly 3 handles this very neatly

https://fedorahosted.org/freeipa/ticket/3050 - WebUI: it is not clear
which row a value belongs to

ACK - row color alternation hopefully solves the problem

https://fedorahosted.org/freeipa/ticket/4278 - Use Patternfly theme
in config and migration pages

FreeIPA logo doesn't lead anywhere, no way to navigate to the
login page, only by altering the url, or clicking the back button. IMO
logo should always lead to login page if not logged in.


Logo now points to UI


https://fedorahosted.org/freeipa/ticket/4281 - Remove login.html and
logout.html

ACK

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


Other issues:
  - unit tests have several fails, possibly because of dom changes


Fixed


  - integration tests ran without errors

Also, according to the UX meeting with Kyle, this patchset should
include the following changes:

  - placeholder for search, box should be on the left
  - actions in one place, on the right in search page
  - actions in one place, on the left in details page
  - action dropdown list to the right near update button in details page
  - left align form fields in details page, two columns arrangement
if the screen is wide
  - hbac details pages - leave it as it is, no form modification required
  - association adder dialog - placeholder for textbox(Filter available),
change button text "Filter"
  - search page title should be changed - use dark variant text
  - multi value list - "add" to button, with "undo all" button group
  - multi value list - "delete" should be also a button
  - left align firefox configuration page steps - ie. every static
page
  - migration should look like login, (~reset_password), text
should go to right
  - error page "return back" should be a button


All fixed



Thanks
Adam



The suggestions found by UX review resulted in additional 10 new commits
(patch numbers 633-642):

I think, that we should switch from patch files to my git branch to
avoid sending 1-2MB of patches in each review cycle.

http://fedorapeople.org/cgit/pvoborni/public_git/freeipa.git/log/?h=patternfly


To be exact:

git log --pretty=format:%s -47
642 webui: use normal buttons instead of link buttons in multivalued widget
641 webui: move service action panel actions to action dropdown
640 web

Re: [Freeipa-devel] [PATCH 0049] Add support for protected tokens

2014-05-26 Thread Jan Cholasta


On 13.5.2014 19:12, Nathaniel McCallum wrote:

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

On 12.5.2014 21:02, Nathaniel McCallum wrote:

On Thu, 2014-05-08 at 13:51 -0400, Simo Sorce wrote:

On Thu, 2014-05-08 at 12:26 -0400, Nathaniel McCallum wrote:

On Wed, 2014-05-07 at 11:17 -0400, Simo Sorce wrote:

On Wed, 2014-05-07 at 09:54 -0400, Dmitri Pal wrote:

On 05/07/2014 09:05 AM, Nathaniel McCallum wrote:

On Wed, 2014-05-07 at 11:42 +0200, Jan Cholasta wrote:

Hi,

On 6.5.2014 17:08, Nathaniel McCallum wrote:

On Tue, 2014-05-06 at 09:49 -0400, Nathaniel McCallum wrote:

On Mon, 2014-05-05 at 12:42 -0400, Nathaniel McCallum wrote:

This also constitutes a rethinking of the token ACIs after the
introduction of SELFDN support.

Admins, as before, have full access to all token permissions.

Normal users have read/search/compare access to all of the non-secret
data for tokens assigned to them, whether protected or non-protected.
Users can add or delete non-protected tokens and modify most of their
metadata. However they cannot create, delete or modify protected tokens.
Regardless of whether the token is protected or not, users cannot change
a token's ownership or unique identity.

In contrast, admins can create protected tokens. This protects the token
from deletion or modification when assigned to users. Additionally, when
a user account is deleted, the assigned non-protected tokens are deleted
but the protected tokens are merely orphaned. This permits the token to
be reassigned without having to recreate it. This last point is
particularly useful in the case of hardware tokens.

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

NOTE: This patch depends on my patch 0048.

This new version makes ipatokenDisabled visible for token owners. It is
also writable if the token is non-protected. This additionally fixes:

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

This new version changes the way the default value of protected is setup
in accordance with the changes made for the review of my patch 0048.2.

Nathaniel

Is using the ipatokenprotected attribute the final design?

No. Alternate designs are welcome. The code is easy enough to modify.


I did not dig too deep into this, but I think it might be better to
instead use the managedby attribute on a token to limit who can delete
(or administer in other way) the token. On otptoken-add, managedby would
be set to the "whoami" user DN, unless run with --protected, in which
case managedby would be left empty. Then, when deleting a user, the
token would be deleted only if the user manages the token.

It seems to me that the mechanics of this are roughly the same as
protected, just with a different syntax. The cost of this is more
complex ACIs. In particular, we'd have to use two userdn clauses (is
this possible?) instead of a simple filter. If there is a clear benefit,
we can justify the more obtuse syntax.


We usually try not to create new attributes until it is fully justified.
I would like Simo to chime in on this.


I would also prefer to reuse existing attributes and mechanism if
possible and if it will not preclude future work.

In this case, it "sounds" like managed-by has the appropriate meaning:
"who manages the token ?" -> myself, admin, other, none ?

Nathaniel can you send 2 lines showing the difference in ACIs between
using managed-by vs a new attribute ?


These are the ACIs using the protected mechanism:

aci: (targetfilter = "(objectClass=ipaToken)")(targetattrs =
"objectclass || description || ipatokenUniqueID || ipatokenDisabled ||
ipatokenNotBefore || ipatokenNotAfter || ipatokenVendor || ipatokenModel
|| ipatokenSerial || ipatokenOwner || ipatokenProtected")(version 3.0;
acl "Users can read basic token info"; allow (read, search, compare)
userattr = "ipatokenOwner#USERDN";)

aci: (targetfilter = "(objectClass=ipatokenTOTP)")(targetattrs =
"ipatokenOTPalgorithm || ipatokenOTPdigits ||
ipatokenTOTPtimeStep")(version 3.0; acl "Users can see TOTP details";
allow (read, search, compare) userattr = "ipatokenOwner#USERDN";)

aci: (targetfilter = "(objectClass=ipatokenHOTP)")(targetattrs =
"ipatokenOTPalgorithm || ipatokenOTPdigits")(version 3.0; acl "Users can
see HOTP details"; allow (read, search, compare) userattr =
"ipatokenOwner#USERDN";)

aci: (targetfilter =
"(&(objectClass=ipaToken)(!(ipatokenProtected=TRUE)))")(targetattrs =
"description || ipatokenDisabled || ipatokenNotBefore ||
ipatokenNotAfter || ipatokenVendor || ipatokenModel ||
ipatokenSerial")(version 3.0; acl "Users can write basic token info";
allow (write) userattr = "ipatokenOwner#USERDN";)

aci: (target = "ldap:///ipatokenuniqueid=*,cn=otp,$SUFFIX";)(targetfilter
= "(&(objectClass=ipaToken)(!(ipatokenProtected=TRUE")(version 3.0;
acl "Users can create and delete tokens"; allow (add, delete) userattr =
"ipatokenOwner#SELFDN";)

This is what they look like using managedBy (I have not tested this):

aci: (targetfilter = "(objectClass=ipaToken)")(targetattrs =
"objectcl

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


On 05/22/2014 03:07 PM, Petr Viktorin wrote:

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


The user read permissions patches had a conflict with these; attaching 
rebased version.



--
Petr³
From 5daa5638fad0161966bb2437a73aba6cec286236 Mon Sep 17 00:00:00 2001
From: Petr Viktorin 
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 c9994c77d390a85bfa954231dc8114aeb19709d6..33c892895ef840ce4daafc7e6f4384aef7654577 100644
--- a/ipaserver/install/plugins/update_managed_permissions.py
+++ b/ipaserver/install/plugins/update_managed_permissions.py
@@ -64,6 +64,9 @@
 * 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.
 * fixup_function
   - A callable that may modify the template in-place before it is applied.
   - Called with the permission name, template dict, and keyword arguments:
@@ -77,11 +80,13 @@
 """
 
 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 ipalib.aci import ACI
+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
@@ -228,6 +233,17 @@
 }
 
 
+def get_aci_names(acistrs):
+

[Freeipa-devel] ACI "Midair collision" bug (Was: [PATCHES] 0552-0554 Upgrading write permissions)


On 05/22/2014 03:07 PM, Petr Viktorin wrote:

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.




This bug is indeed in 389 and there's a fix. I'll test with the current 
build to verify.




I'm re-sending some of our private comunication to the list, in case 
anyone wants to try reproducing the issue.


On 05/26/2014 11:27 AM, Ludwig Krispenz wrote:


Hi,

I now consitently reproduced the issue and debugged it. It is in fact a case, 
where in sorting the values of an attribute in some cases another comparison 
function was used. The current state of the 1.3.2 branch partially fixes and/or 
prevents the problem by using another defaultcomparison function, with a 
current build the test scenario passed.
Maybe you can try the rpms at

http://copr-be.cloud.fedoraproject.org/results/lkrispen/132test/fedora-20-x86_64/389-ds-base-1.3.2.16-20140526081843.fc17/

We will need to provide an official 1.3.217 (and should fix a few more 
locations which could lead to the problem.

Regards,
Ludwig



On 05/21/2014 01:19 PM, Petr Viktorin wrote:


Steps to reproduce:
- Install "master" & "replica" on FreeIPA from the f20 repos
(freeipa-server-3.3.5-1)
- Upgrade "master" to my #4344 WIP branch
  - RPMs: http://fedorapeople.org/~pviktori/rpms/freeipa-2f9399d/
  - source: git pull http://github.com/encukou/freeipa ticket-4344-wip
- Run ipa-ldap-updater on the "replica"
- The problem appears on master.

I can confirm that things work after a restart.



Commands used to reproduce:

$ ldapsearch -x -h localhost -D 'cn=Directory Manager' -w 12345678 -o
ldif-wrap=no -b dc=idm,dc=lab,dc=eng,dc=brq,dc=redhat,dc=com -s base
aci | grep -i 'Modify Sudo rule'
aci: (targetattr = "description || ipaenabledflag || usercategory ||
hostcategory || cmdcategory || ipasudorunasusercategory ||
ipasudorunasgroupcategory || externaluser || ipasudorunasextuser ||
ipasudorunasextgroup || memberdenycmd || memberallowcmd ||
memberuser")(target =
"ldap:///ipauniqueid=*,cn=sudorules,cn=sudo,dc=idm,dc=lab,dc=eng,dc=brq,dc=redhat,dc=com";)(version
3.0;acl "permission:Modify Sudo rule";allow (write) groupdn =
"ldap:///cn=Modify Sudo
rule,cn=permissions,cn=pbac,dc=idm,dc=lab,dc=eng,dc=brq,dc=redhat,dc=com";)

aci: (targetattr = "cmdcategory || description || externalhost ||
externaluser || hostcategory || hostmask || ipaenabledflag ||
ipasudoopt || ipasudorunas || ipasudorunasextgroup ||
ipasudorunasextuser || ipasudorunasgroup || ipasudorunasgroupcategory
|| ipasudorunasusercategory || memberallowcmd || memberdenycmd ||
memberhost || memberuser || sudonotafter || sudonotbefore || sudoorder
|| usercategory")(targetfilter = "(objectclass=ipasudorule)")(version
3.0;acl "permission:System: Modify Sudo rule";allow (add) groupdn =
"ldap:///cn=System: Modify Sudo
rule,cn=permissions,cn=pbac,dc=idm,dc=lab,dc=eng,dc=brq,dc=redhat,dc=com";)

# System: Modify Sudo rule, permissions, pbac, idm.lab.eng.brq.redhat.com
dn: cn=System: Modify Sudo
rule,cn=permissions,cn=pbac,dc=idm,dc=lab,dc=eng,dc=brq,dc=redhat,dc=com
# Modify Sudo rule, permissions, pbac, idm.lab.eng.brq.redhat.com
dn: cn=Modify Sudo
rule,cn=permissions,cn=pbac,dc=idm,dc=lab,dc=eng,dc=brq,dc=redhat,dc=com


$ ldapmodify -x -h localhost -D 'cn=Directory Manager' -w 12345678
dn: dc=idm,dc=lab,dc=eng,dc=brq,dc=redhat,dc=com
changetype: modify
delete: aci
aci: (targetattr = "description || ipaenabledflag || usercategory ||
hostcategory || cmdcategory || ipasudorunasusercategory ||
ipasudorunasgroupcategory || externaluser || ipasudorunasextuser ||
ipasudorunasextgroup || memberdenycmd || memberallowcmd ||
memberuser")(target =
"ldap:///ipauniqueid=*,cn=sudorules,cn=sudo,dc=idm,dc=lab,dc=eng,dc=brq,dc=redhat,dc=com";)(version
3.0;acl "permission:Modify Sudo rule";allow (write) groupdn =
"ldap:///cn=Modify Sudo
rule,cn=permissions,cn=pbac,dc=idm,dc=lab,dc=eng,dc=brq,dc=redhat,dc=com";)


modifying entry "dc=idm,dc=lab,dc=eng,dc=brq,dc=redhat,dc=com"
ldap_modify: No such attribute (16)



--
Petr³

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

Re: [Freeipa-devel] [PATCH] 643 Increase Java stack size for Web UI build on aarch64


On 21.5.2014 10:30, Alexander Bokovoy wrote:

On Wed, 21 May 2014, Petr Vobornik wrote:

Fixes build failure on aarch64:
http://arm.koji.fedoraproject.org/koji/taskinfo?taskID=2328356

Successful scratch build:
http://arm.koji.fedoraproject.org/koji/taskinfo?taskID=233

One patch is for upstream, second for fedora package.

ACK.


pushed to master:
* 8cde6f0d6e6b43c83f192ff8c1ff6107fa0736aa Increase Java stack size for 
Web UI build on aarch64

--
Petr Vobornik

___
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


On 22.5.2014 10:37, Tomas Babej wrote:


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.



Pushed to master: 1e96475a77280bbdc883c66e0dc451ef0559c5fb

--
Petr Vobornik

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

Re: [Freeipa-devel] [PATCH] 0543 - dns: Add idnsSecInlineSigning attribute, add --dnssec option to zone


On 05/14/2014 12:50 PM, Petr Viktorin wrote:

On 04/30/2014 10:00 AM, thierry bordaz wrote:

On 04/29/2014 10:07 PM, Martin Kosek wrote:

On 04/29/2014 08:17 PM, Simo Sorce wrote:

On Tue, 2014-04-29 at 20:00 +0200, Petr Viktorin wrote:

This adds the "idnsSecInlineSigning" attribute and related option.

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

Simo, is adding a MAY attribute to an existing objectClass okay?



Not unheard of, however in the past we discovered some schema
replication issues that may have an impact, let's make sure DS team
also
agrees this is not going to cause issue.


From a purely functional pov a MAY attribute will not break any stored

object, so it is fine.

Simo.


Adding Thierry to the CC list to evaluate the risks, he was already
involved in fixing related issue in the DS for a similar Dogtag schema
extension.

Hello,

When an instance in the topology contains schema extensions like new
MAY attribute, this extension would be propagated to all instances
by replication (no need to copy/merge schema files). This was the
purpose of https://fedorahosted.org/389/ticket/47721. So it is fine
to add new MAY/MUST attribute or new attribute/objectclasses.

During a replication session, a master will check what schema
definitions (objectclasses/attributes) of the replica extends its
own schema. If such definitions exist the supplier add/replace them
in its schema and its user99.ldif file. In your case if a replica
contains a new allowed attribute (e.g. 'idnsSecInlineSigning') but
not the supplier then the supplier 'learns it' (during a replication
session it initiated) and so an entry containing that new attribute
can be updated on the supplier as well.
There is a similar mechanism, when a replica receives a schema that
contains new definitions, it 'learns' them.

The fix for 47721 is introduced in 389-ds 1.3.2.17 and after.
It was tested with mixing 1.3.2.17 instance with legacy versions
(1.3.1, 1,3.0..), and the schema on both side converged to a common
one. This, whatever if the extensions are present on both side.
A limitation is that a legacy instance (not having the fix), must
have a replica agreements to/from an instance with the fix.

regards
thierry



Thanks. This means the patch is good for review.
I've just rebased it slightly.


Another rebase in VERSION was necessary.
Still looking for a review.



--
Petr³
From 3370cb86e874c0898d3160ac98ddc4da739ea65b Mon Sep 17 00:00:00 2001
From: Petr Viktorin 
Date: Tue, 29 Apr 2014 19:42:41 +0200
Subject: [PATCH] dns: Add idnsSecInlineSigning attribute, add --dnssec option
 to zone

Part of the work for: https://fedorahosted.org/freeipa/ticket/3801
---
 API.txt | 9 ++---
 VERSION | 4 ++--
 install/share/60ipadns.ldif | 3 ++-
 ipalib/plugins/dns.py   | 8 +++-
 4 files changed, 17 insertions(+), 7 deletions(-)

diff --git a/API.txt b/API.txt
index 1ea93e9dd403c3fe1a8ca6b047d6fee72220a862..caee61a22fcbf1395fcec55e9d5f5b23c4269523 100644
--- a/API.txt
+++ b/API.txt
@@ -1070,7 +1070,7 @@ command: dnsrecord_show
 output: Output('summary', (, ), None)
 output: PrimaryKey('value', None, None)
 command: dnszone_add
-args: 1,24,3
+args: 1,25,3
 arg: Str('idnsname', attribute=True, cli_name='name', multivalue=False, primary_key=True, required=True)
 option: Str('addattr*', cli_name='addattr', exclude='webui')
 option: Flag('all', autofill=True, cli_name='all', default=False, exclude='webui')
@@ -1083,6 +1083,7 @@ command: dnszone_add
 option: Str('idnsallowtransfer', attribute=True, autofill=True, cli_name='allow_transfer', default=u'none;', multivalue=False, required=False)
 option: Str('idnsforwarders', attribute=True, cli_name='forwarder', csv=True, multivalue=True, required=False)
 option: StrEnum('idnsforwardpolicy', attribute=True, cli_name='forward_policy', multivalue=False, required=False, values=(u'only', u'first', u'none'))
+option: Bool('idnssecinlinesigning', attribute=True, cli_name='dnssec', default=False, multivalue=False, required=False)
 option: Int('idnssoaexpire', attribute=True, autofill=True, cli_name='expire', default=1209600, maxvalue=2147483647, minvalue=0, multivalue=False, required=True)
 option: Int('idnssoaminimum', attribute=True, autofill=True, cli_name='minimum', default=3600, maxvalue=2147483647, minvalue=0, multivalue=False, required=True)
 option: Str('idnssoamname', attribute=True, cli_name='name_server', multivalue=False, required=True)
@@ -1129,7 +1130,7 @@ command: dnszone_enable
 output: Output('summary', (, ), None)
 output: PrimaryKey('value', None, None)
 command: dnszone_find
-args: 1,26,4
+args: 1,27,4
 arg: Str('criteria?', noextrawhitespace=False)
 option: Flag('all', autofill=True, cli_name='all', default=False, exclude='webui')
 option: StrEnum('dnsclass', attribute=True, autofill=False, cli_name='class', multivalue=False, query=True, required=False, values=

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


On 05/22/2014 03:36 PM, Jan Cholasta wrote:

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.



Thanks, pushed to master: 988b2cebf4bf6657eb50f5ecc57bd39425739b8b


--
Petr³

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

Re: [Freeipa-devel] OTP Sync Client Design


On 26.5.2014 09:41, Martin Kosek wrote:

On 05/23/2014 11:19 PM, Nathaniel McCallum wrote:

On Wed, 2014-05-14 at 14:08 -0400, Nathaniel McCallum wrote:

Occasionally OTP tokens get out of sync with the server. When this
happens, the user or an admin need to synchronize the token. To this
end, we landed server-side synchronization support, which is a simple
bind with a custom control. This all works with my sample test script.

Client support is proving a bit more difficult. In the ideal world, the
client would contact LDAP directly and perform the operation. This would
make a man in the middle attack difficult and we can ensure encryption
over the entire operation.

However, browsers, without server-side assistance, cannot perform this
operation from javascript. This means that, in this case, the first
factor and two second factors must be transmitted to the FreeIPA server
and then proxied to 389. Is this an acceptable compromise?

This command also needs to be accessible *without* an existing user
login since, if a user's token is out of sync, the user can't login. Is
it possible to expose such an API? If so, how? Both "ipa env" and "ipa
ping" seem to require kinit, so I don't see any obvious examples.


Thanks everyone for your feedback. This particular feature is proving
difficult to implement, even with our agreed upon design. To reiterate
this design: there will be an HTTP method by which to synchronize
tokens.

There are two assumptions in the code which are making this difficult:
1. All cli commands are Command subclasses.
2. All Command subclasses create authenticated server methods.

There are thus two ways to tackle this problem.

First, I can create a standard POST method in rpcserver.py. This is not
very modular. But the biggest problem is that there is no way to create
the cli-side command to call it (assumption #1).

Second, I can create a Command subclass, similar to the passwd plugin.
This will create both the client- and server-side components. However,
there is no way to disable the server-side authentication.

I think that solving the second of these problems is the most reusable.
Just as an example, the ping command currently requires authentication
but does not need to do so. The passwd Command too shouldn't need to
authenticate before executing the command because the command
authenticates itself.

I think it very likely that we are going to have need for other Command
subclasses in the future which do not require authentication.


It would be useful for `json_metadata` and `i18n` commands. Also I would 
like to see a global web ui configuration which could be read prior to 
authentication, e.g., to get allowed authentication methods( we have a 
ticket to disable forms-based auth).




However, implementing this approach is rather difficult as it will
require a refactoring of rpcserver.py. The code in rpcserver.py contains
many layering violations and the class structure is rather unclear
(look, for instance, at the different orders in which xmlrpc and jsonrpc
classes inherit from their parent classes).

The current problem forcing this refactoring is that authentication
appears to happen across several different layers, but always before the
command to be executed is unmarshalled. We need to invert this order:
the command needs to be unmarshalled first in order to determine whether
or not authentication is necessary. I don't think that switching this
order is practical without constraining authentication to a single layer
(or two: session and krb5) late in the request process.

Git tells me that lots of people have touched this code, so I'm hoping
for good feedback! ;)

Alternatively, we could create a way to inject cli commands without
having Command subclasses. Isolating these concerns is itself probably a
good design choice. Ideally we'd have a structure where the Command
class itself inherits from a CLICommand class and a ServerMethod class.
But this too will be a massive refactoring, perhaps even bigger than the
rpcserver.py refactoring.

So, which assumption should we break: #1 or #2? And who wants to help me
do it? Also, I am all ears for easier solutions for this feature.

Nathaniel


Hi Nathaniel,

These are all good ideas, it is true that we are sometimes hitting framework
limitations which will force to rewrite some parts, Petr Viktorin already have
couple of wished refactorings in mind.

What you are suggesting sounds as a pretty massive refactorings touching basic
framework parts that have not been touched for long years. Refactorings of
this scale would take months of planning and execution.

Question is, does this use case warrants such big change? AFAIK, we need to
solve following use cases:

1) Admin wants to manipulate user's token via Web UI/CLI - easy to do, admin is
authenticated and can run any commands

2) User wants to re-synchronize token via Web UI: easy to do, create POST
callback and a Web UI page. IMO, Web UI will be the most used synchronization
interf

[Freeipa-devel] [HEADS UP] Read ACI rework is now in master


All FreeIPA developers, hang on to your hats (be they red or otherwise)!

In master, the global ACI granting read/search/compare rights to anyyone 
has been and removed in favor of granular managed permissions.

Please help test the change.


Emergency override:
If you find an issue, first report it and then give the following ldif 
to ldapmodify to restore the global anonymous ACI.

Replace both instances of $SUFFIX by the DN that `ipa env basedn` gives you.

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";;)





Relevant ticket: https://fedorahosted.org/freeipa/ticket/3566

--
Petr³

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

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


On 05/23/2014 02:26 PM, Martin Kosek wrote:

On 05/22/2014 04:03 PM, Petr Viktorin wrote:

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.


Would it make sense to publish your FreeIPA git hooks somewhere on
http://www.freeipa.org/page/Contribute/Code or your github and link it? I think
it already contains couple gems that may help other people prevent basic errors
like this one.


Sure, I'll document it a bit and publish.


Otherwise, the patch worked fine - ACK!

I would like it to be pushed as soon as user ACI patch is pushed so that we
have some time to find issues.


Thanks!
Pushed to master: 193ced0bd7a9a26e7b25f08b023ee21302acaac7


--
Petr³

___
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


On 05/26/2014 12:09 PM, Martin Kosek wrote:

On 05/26/2014 12:04 PM, Petr Viktorin wrote:

On 05/25/2014 09:29 PM, Martin Kosek wrote:

On 05/23/2014 04:50 PM, Simo Sorce wrote:

On Fri, 2014-05-23 at 10:59 +0200, Martin Kosek wrote:

On 05/22/2014 04:20 PM, Petr Viktorin wrote:

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.



We are very close to push this one - I have just one last concern about
userpkcs12 attribute. On upgrade, we previously hidden userpkcs12
from user,
now we added it to be read by default. This results in this warning
during upgrade:

Excluded attributes for System: Read User Addressbook Attributes:
userpkcs12

Simo (or others), is this OK or do we want to keep hiding userpkcs12
by default?


Is there any client that needs access to that information that we are
aware of ?

Simo.


I do not think so. Rob, do you know?


This was my mistake. We never allowed non-admins to see that attribute by
default, so we shouldn't start now.


ack, we probably had a good reason and it is much safer to keep this decision.


I'm glad the updater caught it, sorry that I didn't.


Actually, that means that you made the security checks in the updater right :-)

I diffed the change in the patch and it removed the last obstacle I saw with
this patch set. Thus, ACK for all 3.


Thanks for the thorough review!
Pushed to master: 63becae88c6c270b98f0432dc474b661b82f3119


--
Petr³

___
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

On 05/26/2014 12:04 PM, Petr Viktorin wrote:
> On 05/25/2014 09:29 PM, Martin Kosek wrote:
>> On 05/23/2014 04:50 PM, Simo Sorce wrote:
>>> On Fri, 2014-05-23 at 10:59 +0200, Martin Kosek wrote:
 On 05/22/2014 04:20 PM, Petr Viktorin wrote:
> 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.
>

 We are very close to push this one - I have just one last concern about
 userpkcs12 attribute. On upgrade, we previously hidden userpkcs12
 from user,
 now we added it to be read by default. This results in this warning
 during upgrade:

 Excluded attributes for System: Read User Addressbook Attributes:
 userpkcs12

 Simo (or others), is this OK or do we want to keep hiding userpkcs12
 by default?
>>>
>>> Is there any client that needs access to that information that we are
>>> aware of ?
>>>
>>> Simo.
>>
>> I do not think so. Rob, do you know?
> 
> This was my mistake. We never allowed non-admins to see that attribute by
> default, so we shouldn't start now.

ack, we probably had a good reason and it is much safer to keep this decision.

> I'm glad the updater caught it, sorry that I didn't.

Actually, that means that you made the security checks in the updater right :-)

I diffed the change in the patch and it removed the last obstacle I saw with
this patch set. Thus, ACK for all 3.

Martin

___
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


On 05/25/2014 09:29 PM, Martin Kosek wrote:

On 05/23/2014 04:50 PM, Simo Sorce wrote:

On Fri, 2014-05-23 at 10:59 +0200, Martin Kosek wrote:

On 05/22/2014 04:20 PM, Petr Viktorin wrote:

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.



We are very close to push this one - I have just one last concern about
userpkcs12 attribute. On upgrade, we previously hidden userpkcs12
from user,
now we added it to be read by default. This results in this warning
during upgrade:

Excluded attributes for System: Read User Addressbook Attributes:
userpkcs12

Simo (or others), is this OK or do we want to keep hiding userpkcs12
by default?


Is there any client that needs access to that information that we are
aware of ?

Simo.


I do not think so. Rob, do you know?


This was my mistake. We never allowed non-admins to see that attribute 
by default, so we shouldn't start now.

I'm glad the updater caught it, sorry that I didn't.

--
Petr³

From a7db3134a81c4496a41407e7da617fcf7b47904a Mon Sep 17 00:00:00 2001
From: Petr Viktorin 
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..56e2fe69719f3d0133c3b0e745c5a37ec76e12ca 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', 'departmentnu

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

2014-05-26 Thread Alexander Bokovoy

On Mon, 26 May 2014, Martin Kosek wrote:

On 05/26/2014 09:33 AM, Jan Cholasta wrote:

On 26.5.2014 07:49, Martin Kosek wrote:

...

 > 5) modifying
 > (in active)   ipa user-mod tuser ...

Ok.

 > (in stage)ipa user-mod tuser --staged ...

Simo did not like this command, I would personally add it. As long as we
have "ipa user-add --staged", we should also have an option to delete
and modify user in staged area.

+1

 > (in del)  ipa user-mod tuser --deleted ...

Not needed.

Is this acceptable for everyone? If yes, the next step would be for
Thierry to update the design page with new proposals.

Martin

Are users in different containers using the same uid allowed?

Say you had a John Doe (uid jdoe) working in a company couple years ago. jdoe
left and is now in deleted accounts tree. Jane Doe joins the company now and
question is - do we want to allow Jane taking the same uid as John had? I am
thinking we should not allow that. Maybe we should allow override with --force
or having a global option.

This is pretty much a company policy thing. Not everyone will even want
to have this workflow implemented and even if they would, a policy to
keep the same uid (as opposed to uidNumber) is a separate one.

Thus, I'd rather have it optional with --force or get uid transformed to
uid=deleted+jdoe,cn=users... and given a way to handle conflicts when
getting deleted uids resurrected.

--
/ Alexander Bokovoy

___
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-26 Thread thierry bordaz

On 05/26/2014 10:18 AM, Martin Kosek wrote:

On 05/26/2014 09:33 AM, Jan Cholasta wrote:

On 26.5.2014 07:49, Martin Kosek wrote:

...

> 5) modifying
> (in active) ipa user-mod tuser ...

Ok.

> (in stage)ipa user-mod tuser --staged ...

Simo did not like this command, I would personally add it. As long as we
have "ipa user-add --staged", we should also have an option to delete
and modify user in staged area.

> (in del) ipa user-mod tuser --deleted ...

Not needed.

Is this acceptable for everyone? If yes, the next step would be for
Thierry to update the design page with new proposals.

Martin

Are users in different containers using the same uid allowed?

Say you had a John Doe (uid jdoe) working in a company couple years ago. jdoe
left and is now in deleted accounts tree. Jane Doe joins the company now and
question is - do we want to allow Jane taking the same uid as John had? I am
thinking we should not allow that. Maybe we should allow override with --force
or having a global option.

I agree, 'John Doe' should keep its uid and 'Jane Doe' should pickup a
different one.

So that means attribute uniqueness scope should covers the differents
containers (stage, delete, active) and likely all the DIT.
But then for generated attributes (like 'ipaUniqueid') it is a problem
because in 'stage' most/all entries will have 'ipaUniqueId: generate'.

So for such attributes, 'stage' container should be excluded from the scope.
If 'ipa user-mod --stage' is allowed to modify ipaUniqueiD, uniqueness
will not be enforced.

Another related topic is - do we want to enforce staged user to always have UID
RDN? Isn't that limiting? When writing

http://www.freeipa.org/page/V4/User_Life-Cycle_Management#Create_a_User_-_by_provisioning_system

I proposed that we should also be able to unstage a minimal record like this:

dn: cn=Test User,cn=staged users,cn=accounts,cn=provisioning,dc=example,dc=com
objectClass: top
objectClass: organizationalperson
cn: Test User
sn: User
nsAccountLock: True

If not, do we need the --staged/--deleted flags on anything but
user-add/user-find?

I see your point, but I think we should make admins to be very explicit when
manipulating users any area other than the active users area. As Simo noted,
these are not real users, just incomplete user records.

Martin

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

Re: [Freeipa-devel] [PATCH 0029-0046, 0047] Internationalized domain names in DNS plugin

On 05/21/2014 06:23 PM, Martin Basti wrote:
> Updated patches attached
> 
> Patch 33 removed.
> 
> Patches should be applied in order: 0029-0032, 0034-0040, 0047,
> 0041-0042, 0045-0046

FYI, I just did an upgrade from 3.3.5 to 4.0 with these patches applied and
this is what I got:

# rpm -Uvh --force ~/freeipa-master/dist/rpms/freeipa-*Preparing...
  # [100%]
Updating / installing...
   1:freeipa-python-3.3.90GITd4dd8c4-0# [  7%]
   2:freeipa-client-3.3.90GITd4dd8c4-0# [ 13%]
Could not load host key: /etc/ssh/ssh_host_dsa_key
   3:freeipa-admintools-3.3.90GITd4dd8# [ 20%]
   4:freeipa-server-3.3.90GITd4dd8c4-0# [ 27%]
   5:freeipa-server-trust-ad-3.3.90GIT# [ 33%]
   6:freeipa-server-foreman-smartproxy# [ 40%]
   7:freeipa-tests-3.3.90GITd4dd8c4-0.# [ 47%]
   8:freeipa-debuginfo-3.3.90GITd4dd8c# [ 53%]
Cleaning up / removing...
   9:freeipa-tests-3.3.5-1.fc20   # [ 60%]
  10:freeipa-debuginfo-3.3.5-1.fc20   # [ 67%]
  11:freeipa-server-trust-ad-3.3.5-1.f# [ 73%]
  12:freeipa-server-3.3.5-1.fc20  # [ 80%]
  13:freeipa-admintools-3.3.5-1.fc20  # [ 87%]
  14:freeipa-client-3.3.5-1.fc20  # [ 93%]
  15:freeipa-python-3.3.5-1.fc20  # [100%]
Upgrade failed with list.remove(x): x not in list
IPA upgrade failed.
Unexpected error
AttributeError: 'DNSName' object has no attribute 'endswith'


ipaupgrade.log:

2014-05-26T08:32:28Z DEBUG Created connection context.ldap2
2014-05-26T08:32:28Z DEBUG raw: dns_is_enabled()
2014-05-26T08:32:28Z DEBUG dns_is_enabled()
2014-05-26T08:32:28Z DEBUG retrieving schema for SchemaCache
url=ldapi://%2fvar%2frun%2fslapd-MKOSEK-FEDORA20-TEST.socket
conn=
2014-05-26T08:32:28Z DEBUG Loading StateFile from
'/var/lib/ipa/sysrestore/sysrestore.state'
2014-05-26T08:32:28Z DEBUG Loading Index file from
'/var/lib/ipa/sysrestore/sysrestore.index'
2014-05-26T08:32:28Z DEBUG raw: dnsrecord_find(u'mkosek-fedora20.test', 
u'ipa-ca')
2014-05-26T08:32:28Z DEBUG dnsrecord_find(,
u'ipa-ca', structured=False, all=False, raw=False, pkey_only=False)
2014-05-26T08:32:28Z DEBUG   File
"/usr/lib/python2.7/site-packages/ipaserver/install/installutils.py", line 638,
in run_script
return_value = main_function()

  File "/usr/sbin/ipa-upgradeconfig", line 1113, in main
add_ca_dns_records()

  File "/usr/sbin/ipa-upgradeconfig", line 809, in add_ca_dns_records
bind.convert_ipa_ca_cnames(api.env.domain)

  File "/usr/lib/python2.7/site-packages/ipaserver/install/bindinstance.py",
line 853, in convert_ipa_ca_cnames
cnames = get_rr(domain_name, IPA_CA_RECORD, "CNAME")

  File "/usr/lib/python2.7/site-packages/ipaserver/install/bindinstance.py",
line 369, in get_rr
ret = api.Command.dnsrecord_find(unicode(zone), unicode(name))

  File "/usr/lib/python2.7/site-packages/ipalib/frontend.py", line 436, in 
__call__
ret = self.run(*args, **options)

  File "/usr/lib/python2.7/site-packages/ipalib/frontend.py", line 752, in run
result = self.execute(*args, **options)

  File "/usr/lib/python2.7/site-packages/ipalib/plugins/baseldap.py", line
1869, in execute
base_dn = self.api.Object[self.obj.parent_object].get_dn(*args[:-1])

  File "/usr/lib/python2.7/site-packages/ipalib/plugins/dns.py", line 1714, in
get_dn
if zone.endswith(u'.'):

2014-05-26T08:32:28Z DEBUG The ipa-upgradeconfig command failed, exception:
AttributeError: 'DNSName' object has no attribute 'endswith'

Martin

___
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

On 05/26/2014 09:33 AM, Jan Cholasta wrote:
> On 26.5.2014 07:49, Martin Kosek wrote:
...
>>  > 5) modifying
>>  > (in active)   ipa user-mod tuser ...
>>
>> Ok.
>>
>>  > (in stage)ipa user-mod tuser --staged ...
>>
>> Simo did not like this command, I would personally add it. As long as we
>> have "ipa user-add --staged", we should also have an option to delete
>> and modify user in staged area.
> 
> +1
> 
>>
>>  > (in del)  ipa user-mod tuser --deleted ...
>>
>> Not needed.
>>
>> Is this acceptable for everyone? If yes, the next step would be for
>> Thierry to update the design page with new proposals.
>>
>> Martin
> 
> Are users in different containers using the same uid allowed?

Say you had a John Doe (uid jdoe) working in a company couple years ago. jdoe
left and is now in deleted accounts tree. Jane Doe joins the company now and
question is - do we want to allow Jane taking the same uid as John had? I am
thinking we should not allow that. Maybe we should allow override with --force
or having a global option.

Another related topic is - do we want to enforce staged user to always have UID
RDN? Isn't that limiting? When writing

http://www.freeipa.org/page/V4/User_Life-Cycle_Management#Create_a_User_-_by_provisioning_system

I proposed that we should also be able to unstage a minimal record like this:

dn: cn=Test User,cn=staged users,cn=accounts,cn=provisioning,dc=example,dc=com
objectClass: top
objectClass: organizationalperson
cn: Test User
sn: User
nsAccountLock: True

> If not, do we need the --staged/--deleted flags on anything but
> user-add/user-find?

I see your point, but I think we should make admins to be very explicit when
manipulating users any area other than the active users area. As Simo noted,
these are not real users, just incomplete user records.

Martin

___
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-26 Thread thierry bordaz

On 05/26/2014 07:49 AM, Martin Kosek wrote:

On 05/23/2014 04:55 PM, Simo Sorce wrote:

On Fri, 2014-05-23 at 10:13 -0400, Rob Crittenden wrote:

This, I believe, has already been covered, but I'm concerned with the
(over)use of active/inactive in this discussion.

I think use of "inactive" and "active" to describe users might be
confusing since there is already an account enable/disable command.
This
on top of unlock, are there now 3 possible boolean states a user can
be
in? locked/unlocked, enabled/disabled, active/inactive, plus
deleted/active and staged/active?

Agree, we should only have "ipa user-unstage " and not call
this operations with words like active/inactive.

User's in the staging area are not inactive, they are *not* users yet in
the first place.

Simo.

Ok. Let us consolidate the decisions, I think we are now running in 
circles. Let me start from Petr3's API proposal which was a 
functionally complete proposal and start from there:

On 05/22/2014 10:47 AM, Petr Viktorin wrote:
> ...
> 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 ...

Ok.

> (to deleted)  ipa user-add tuser --deleted ...  (*)

Not needed.

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

We need both, alternative is Simo's proposal:

ipa user-unstage
ipa user-undelete

I personally like unstage and undelete commands, I would go with those.

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

Ok.

> (from stage)  ipa user-del tuser --staged

IMO staged deleted users should not be moved to deleted container, but 
simply permanently deleted. As Simo noted, staged user are not real 
users, just incomplete users.

+1

> 4) moving to stage
> (from active) ipa user-stage tuser
> (from del)ipa user-stage tuser --deleted

None of the commands are needed for the basic workflow.

> 5) modifying
> (in active)   ipa user-mod tuser ...

Ok.

> (in stage)ipa user-mod tuser --staged ...

Simo did not like this command, I would personally add it. As long as 
we have "ipa user-add --staged", we should also have an option to 
delete and modify user in staged area.

> (in del)  ipa user-mod tuser --deleted ...

Not needed.

When a user that left the organisation is coming back we will move his 
entry 'delete' to 'active'. Now if the administrator wants the user to 
come back but with some changes, he may need this option. For example if 
by the time he left, the default homeDirectory changed the administrator 
may want to update its value.

Is this acceptable for everyone? If yes, the next step would be for 
Thierry to update the design page with new proposals.

Martin

___
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] OTP Sync Client Design

2014-05-26 Thread Jan Cholasta


On 23.5.2014 23:19, Nathaniel McCallum wrote:

On Wed, 2014-05-14 at 14:08 -0400, Nathaniel McCallum wrote:

Occasionally OTP tokens get out of sync with the server. When this
happens, the user or an admin need to synchronize the token. To this
end, we landed server-side synchronization support, which is a simple
bind with a custom control. This all works with my sample test script.

Client support is proving a bit more difficult. In the ideal world, the
client would contact LDAP directly and perform the operation. This would
make a man in the middle attack difficult and we can ensure encryption
over the entire operation.

However, browsers, without server-side assistance, cannot perform this
operation from javascript. This means that, in this case, the first
factor and two second factors must be transmitted to the FreeIPA server
and then proxied to 389. Is this an acceptable compromise?

This command also needs to be accessible *without* an existing user
login since, if a user's token is out of sync, the user can't login. Is
it possible to expose such an API? If so, how? Both "ipa env" and "ipa
ping" seem to require kinit, so I don't see any obvious examples.


Thanks everyone for your feedback. This particular feature is proving
difficult to implement, even with our agreed upon design. To reiterate
this design: there will be an HTTP method by which to synchronize
tokens.

There are two assumptions in the code which are making this difficult:
1. All cli commands are Command subclasses.
2. All Command subclasses create authenticated server methods.

There are thus two ways to tackle this problem.

First, I can create a standard POST method in rpcserver.py. This is not
very modular. But the biggest problem is that there is no way to create
the cli-side command to call it (assumption #1).


Well, you could derive the command from ipalib.frontend.Local and 
manually call the POST method from it.




Second, I can create a Command subclass, similar to the passwd plugin.
This will create both the client- and server-side components. However,
there is no way to disable the server-side authentication.

I think that solving the second of these problems is the most reusable.
Just as an example, the ping command currently requires authentication
but does not need to do so. The passwd Command too shouldn't need to
authenticate before executing the command because the command
authenticates itself.

I think it very likely that we are going to have need for other Command
subclasses in the future which do not require authentication.

However, implementing this approach is rather difficult as it will
require a refactoring of rpcserver.py. The code in rpcserver.py contains
many layering violations and the class structure is rather unclear
(look, for instance, at the different orders in which xmlrpc and jsonrpc
classes inherit from their parent classes).

The current problem forcing this refactoring is that authentication
appears to happen across several different layers, but always before the
command to be executed is unmarshalled. We need to invert this order:
the command needs to be unmarshalled first in order to determine whether
or not authentication is necessary. I don't think that switching this
order is practical without constraining authentication to a single layer
(or two: session and krb5) late in the request process.

Git tells me that lots of people have touched this code, so I'm hoping
for good feedback! ;)

Alternatively, we could create a way to inject cli commands without
having Command subclasses. Isolating these concerns is itself probably a
good design choice. Ideally we'd have a structure where the Command
class itself inherits from a CLICommand class and a ServerMethod class.
But this too will be a massive refactoring, perhaps even bigger than the
rpcserver.py refactoring.

So, which assumption should we break: #1 or #2? And who wants to help me
do it? Also, I am all ears for easier solutions for this feature.


I would go for the refactoring, the rpcserver code does indeed need some 
love.




Nathaniel


--
Jan Cholasta

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

Re: [Freeipa-devel] OTP Sync Client Design