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

2014-05-23 Thread Alexander Bokovoy

On Fri, 23 May 2014, Jan Cholasta wrote:

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.


Pushed to master
* db7d0219bac72daa270ee28d5db5c18ea41fb8b1 Default the token owner to the 
person adding the token


--
/ Alexander Bokovoy

___
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


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] [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 0048] Default the token owner to the person adding the token

2014-05-07 Thread Nathaniel McCallum
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


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

2014-05-07 Thread Petr Vobornik

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

___
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-06 Thread Nathaniel McCallum
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
>From 773901e0c31e5eb520a882ce44027117d80a7b79 Mon Sep 17 00:00:00 2001
From: Nathaniel McCallum 
Date: Mon, 5 May 2014 10:41:20 -0400
Subject: [PATCH] Default the token owner to the person adding the token

Creating tokens for yourself is the most common operation. Making this the
default optimizes for the common case.
---
 ipalib/plugins/otptoken.py | 9 -
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/ipalib/plugins/otptoken.py b/ipalib/plugins/otptoken.py
index f68ea7df596c8d7e837d98874f4fd630a6d7524a..280e552811630bf01f86528fdd06c2cc9b724790 100644
--- a/ipalib/plugins/otptoken.py
+++ b/ipalib/plugins/otptoken.py
@@ -241,7 +241,14 @@ class otptoken_add(LDAPCreate):
 if tattr in entry_attrs:
 del entry_attrs[tattr]
 
-# Resolve the user's dn
+# If owner was not specified, default to the person adding this token.
+if 'ipatokenowner' not in entry_attrs:
+result = self.api.Command.user_find(whoami=True)['result']
+if result:
+cur_uid = result[0]['uid'][0]
+entry_attrs.setdefault('ipatokenowner', cur_uid)
+
+# Resolve the owner's dn
 _normalize_owner(self.api.Object.user, entry_attrs)
 
 # Get the issuer for the URI
-- 
1.9.0

___
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-06 Thread Jan Cholasta

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)




Also please check
if there actually is a result, if you run user-find --whoami when
authenticated as non-user, the result will be empty.


Fixed.

Nathaniel




--
Jan Cholasta

___
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-06 Thread Nathaniel McCallum
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.

> Also please check 
> if there actually is a result, if you run user-find --whoami when 
> authenticated as non-user, the result will be empty.

Fixed.

Nathaniel
>From 37b4bc35c5108cca06b4c83d3de2719aa14a467b Mon Sep 17 00:00:00 2001
From: Nathaniel McCallum 
Date: Mon, 5 May 2014 10:41:20 -0400
Subject: [PATCH] Default the token owner to the person adding the token

Creating tokens for yourself is the most common operation. Making this the
default optimizes for the common case.
---
 ipalib/plugins/otptoken.py | 12 +++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/ipalib/plugins/otptoken.py b/ipalib/plugins/otptoken.py
index f68ea7df596c8d7e837d98874f4fd630a6d7524a..42cc16d1686cb411b3170d8ee59ad37986c13772 100644
--- a/ipalib/plugins/otptoken.py
+++ b/ipalib/plugins/otptoken.py
@@ -241,7 +241,17 @@ class otptoken_add(LDAPCreate):
 if tattr in entry_attrs:
 del entry_attrs[tattr]
 
-# Resolve the user's dn
+# Get the UID of the person adding this token.
+try:
+cur_uid = self.api.Command.user_find(whoami=True)['result'][0]['uid'][0]
+except (KeyError, IndexError):
+cur_uid = None
+
+# If no owner was specified, default to the person adding this token.
+if "ipatokenowner" not in entry_attrs and cur_id is not None:
+entry_attrs["ipatokenowner"] = cur_uid
+
+# Resolve the owner's dn
 _normalize_owner(self.api.Object.user, entry_attrs)
 
 # Get the issuer for the URI
-- 
1.9.0

___
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-06 Thread Jan Cholasta

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. Also please check 
if there actually is a result, if you run user-find --whoami when 
authenticated as non-user, the result will be empty.


Honza

--
Jan Cholasta

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