Re: [Freeipa-devel] [PATCH 0048] Default the token owner to the person adding the token
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
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
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
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
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
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
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
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
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
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