Re: [Freeipa-devel] [PATCH 0093] Enable service authentication indicator management
On 05/31/2016 03:57 PM, Nathaniel McCallum wrote:
> On Tue, 2016-05-31 at 15:25 +0200, Petr Vobornik wrote:
>> On 05/31/2016 02:49 PM, Nathaniel McCallum wrote:
>>> On Mon, 2016-05-30 at 19:08 +0300, Alexander Bokovoy wrote:
On Mon, 30 May 2016, Petr Vobornik wrote:
> On 05/27/2016 06:00 PM, Nathaniel McCallum wrote:
>> Pavel, since we made the change here from a StrEnum to a Str,
>> we
>> need
>> to update the UI patch accordingly.
>
> How should admin know what to write there intuitively?
>
> Shouldn't Web UI or CLI advertise the indicators supported by
> IPA?
> E.g.
> CLI in doc string. Web UI might even combine checkboxes (otp,
> radius)
> with textbox.
That would be better, I think. We still need to keep the API with
a
free
text field but Web UI, of course, should provide some pre-defined
labels.
>>>
>>> I *think* this means that this patch doesn't need any changes. Is
>>> that
>>> correct? If so, can I get a review? :)
>>>
>>
>> I meant that the param's 'doc' attribute can get the supported
>> values.
>> So that they would be shown in `ipa service-mod --help`
>>
>> Btw, the `required: false` and `multivalued: true` can be simplified
>> into Str('krbprincipalauthind*')
>
> I fixed the doc string as well as the verbosity. I also rebased against
> the current master.
>
ACK
master:
* 4ded2ffc161ec649ba1ccf8d0b528d24028080df Enable service authentication
indicator management
--
Petr Vobornik
--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] [PATCH 0093] Enable service authentication indicator management
On Tue, 2016-05-31 at 15:25 +0200, Petr Vobornik wrote:
> On 05/31/2016 02:49 PM, Nathaniel McCallum wrote:
> > On Mon, 2016-05-30 at 19:08 +0300, Alexander Bokovoy wrote:
> > > On Mon, 30 May 2016, Petr Vobornik wrote:
> > > > On 05/27/2016 06:00 PM, Nathaniel McCallum wrote:
> > > > > Pavel, since we made the change here from a StrEnum to a Str,
> > > > > we
> > > > > need
> > > > > to update the UI patch accordingly.
> > > >
> > > > How should admin know what to write there intuitively?
> > > >
> > > > Shouldn't Web UI or CLI advertise the indicators supported by
> > > > IPA?
> > > > E.g.
> > > > CLI in doc string. Web UI might even combine checkboxes (otp,
> > > > radius)
> > > > with textbox.
> > > That would be better, I think. We still need to keep the API with
> > > a
> > > free
> > > text field but Web UI, of course, should provide some pre-defined
> > > labels.
> >
> > I *think* this means that this patch doesn't need any changes. Is
> > that
> > correct? If so, can I get a review? :)
> >
>
> I meant that the param's 'doc' attribute can get the supported
> values.
> So that they would be shown in `ipa service-mod --help`
>
> Btw, the `required: false` and `multivalued: true` can be simplified
> into Str('krbprincipalauthind*')
I fixed the doc string as well as the verbosity. I also rebased against
the current master.From 786ca520566fa5ae7ffe7c309f33cea6096781ff Mon Sep 17 00:00:00 2001
From: Nathaniel McCallum
Date: Wed, 4 May 2016 17:08:45 -0400
Subject: [PATCH] Enable service authentication indicator management
https://fedorahosted.org/freeipa/ticket/433
---
API.txt | 9 ++---
VERSION | 4 ++--
ipalib/plugins/service.py | 10 +-
3 files changed, 17 insertions(+), 6 deletions(-)
diff --git a/API.txt b/API.txt
index 3ad250e74f48ef3c54494ba6bd2d398a7c5d1b69..94e2cb71bc5fe777b219b7174c3057bc222fbb78 100644
--- a/API.txt
+++ b/API.txt
@@ -3901,7 +3901,7 @@ output: Entry('result')
output: Output('summary', type=[, ])
output: PrimaryKey('value')
command: service_add
-args: 1,11,3
+args: 1,12,3
arg: Str('krbprincipalname', cli_name='principal')
option: Str('addattr*', cli_name='addattr')
option: Flag('all', autofill=True, cli_name='all', default=False)
@@ -3909,6 +3909,7 @@ option: Flag('force', autofill=True, default=False)
option: StrEnum('ipakrbauthzdata*', cli_name='pac_type', values=[u'MS-PAC', u'PAD', u'NONE'])
option: Bool('ipakrbokasdelegate?', cli_name='ok_as_delegate')
option: Bool('ipakrbrequirespreauth?', cli_name='requires_pre_auth')
+option: Str('krbprincipalauthind*', cli_name='auth_ind')
option: Flag('no_members', autofill=True, default=False)
option: Flag('raw', autofill=True, cli_name='raw', default=False)
option: Str('setattr*', cli_name='setattr')
@@ -4011,10 +4012,11 @@ output: Output('completed', type=[])
output: Output('failed', type=[])
output: Entry('result')
command: service_find
-args: 1,11,4
+args: 1,12,4
arg: Str('criteria?')
option: Flag('all', autofill=True, cli_name='all', default=False)
option: StrEnum('ipakrbauthzdata*', autofill=False, cli_name='pac_type', values=[u'MS-PAC', u'PAD', u'NONE'])
+option: Str('krbprincipalauthind*', autofill=False, cli_name='auth_ind')
option: Str('krbprincipalname?', autofill=False, cli_name='principal')
option: Str('man_by_host*', cli_name='man_by_hosts')
option: Flag('no_members', autofill=True, default=True)
@@ -4029,7 +4031,7 @@ output: ListOfEntries('result')
output: Output('summary', type=[, ])
output: Output('truncated', type=[])
command: service_mod
-args: 1,12,3
+args: 1,13,3
arg: Str('krbprincipalname', cli_name='principal')
option: Str('addattr*', cli_name='addattr')
option: Flag('all', autofill=True, cli_name='all', default=False)
@@ -4037,6 +4039,7 @@ option: Str('delattr*', cli_name='delattr')
option: StrEnum('ipakrbauthzdata*', autofill=False, cli_name='pac_type', values=[u'MS-PAC', u'PAD', u'NONE'])
option: Bool('ipakrbokasdelegate?', autofill=False, cli_name='ok_as_delegate')
option: Bool('ipakrbrequirespreauth?', autofill=False, cli_name='requires_pre_auth')
+option: Str('krbprincipalauthind*', autofill=False, cli_name='auth_ind')
option: Flag('no_members', autofill=True, default=False)
option: Flag('raw', autofill=True, cli_name='raw', default=False)
option: Flag('rights', autofill=True, default=False)
diff --git a/VERSION b/VERSION
index 45fdb09788dbc6496272da786bb6d6afa45bf118..29e67f3d7ed232de8d6e71ea6b58bd1f1cbbea5d 100644
--- a/VERSION
+++ b/VERSION
@@ -90,5 +90,5 @@ IPA_DATA_VERSION=2010061412
# #
IPA_API_VERSION_MAJOR=2
-IPA_API_VERSION_MINOR=170
-# Last change: mbasti - *-find: do not search for members by default
+IPA_API_VERSION_MINOR=171
+# Last change: npmccallum - enable setting authinds on services
diff --git a/ipalib/plugins/service.py b/ipalib/plugins/service.py
index 2d3476e832dc8694eae04a0ecd5c
Re: [Freeipa-devel] [PATCH 0093] Enable service authentication indicator management
On 05/31/2016 02:49 PM, Nathaniel McCallum wrote:
> On Mon, 2016-05-30 at 19:08 +0300, Alexander Bokovoy wrote:
>> On Mon, 30 May 2016, Petr Vobornik wrote:
>>> On 05/27/2016 06:00 PM, Nathaniel McCallum wrote:
Pavel, since we made the change here from a StrEnum to a Str, we
need
to update the UI patch accordingly.
>>>
>>> How should admin know what to write there intuitively?
>>>
>>> Shouldn't Web UI or CLI advertise the indicators supported by IPA?
>>> E.g.
>>> CLI in doc string. Web UI might even combine checkboxes (otp,
>>> radius)
>>> with textbox.
>> That would be better, I think. We still need to keep the API with a
>> free
>> text field but Web UI, of course, should provide some pre-defined
>> labels.
>
> I *think* this means that this patch doesn't need any changes. Is that
> correct? If so, can I get a review? :)
>
I meant that the param's 'doc' attribute can get the supported values.
So that they would be shown in `ipa service-mod --help`
Btw, the `required: false` and `multivalued: true` can be simplified
into Str('krbprincipalauthind*')
--
Petr Vobornik
--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] [PATCH 0093] Enable service authentication indicator management
On Mon, 2016-05-30 at 19:08 +0300, Alexander Bokovoy wrote: > On Mon, 30 May 2016, Petr Vobornik wrote: > > On 05/27/2016 06:00 PM, Nathaniel McCallum wrote: > > > Pavel, since we made the change here from a StrEnum to a Str, we > > > need > > > to update the UI patch accordingly. > > > > How should admin know what to write there intuitively? > > > > Shouldn't Web UI or CLI advertise the indicators supported by IPA? > > E.g. > > CLI in doc string. Web UI might even combine checkboxes (otp, > > radius) > > with textbox. > That would be better, I think. We still need to keep the API with a > free > text field but Web UI, of course, should provide some pre-defined > labels. I *think* this means that this patch doesn't need any changes. Is that correct? If so, can I get a review? :) -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] [PATCH 0093] Enable service authentication indicator management
On Mon, 2016-05-30 at 19:08 +0300, Alexander Bokovoy wrote: > On Mon, 30 May 2016, Petr Vobornik wrote: > > On 05/27/2016 06:00 PM, Nathaniel McCallum wrote: > > > Pavel, since we made the change here from a StrEnum to a Str, we > > > need > > > to update the UI patch accordingly. > > > > How should admin know what to write there intuitively? > > > > Shouldn't Web UI or CLI advertise the indicators supported by IPA? > > E.g. > > CLI in doc string. Web UI might even combine checkboxes (otp, > > radius) > > with textbox. > That would be better, I think. We still need to keep the API with a > free > text field but Web UI, of course, should provide some pre-defined > labels. +1 -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] [PATCH 0093] Enable service authentication indicator management
On Mon, 30 May 2016, Petr Vobornik wrote: On 05/27/2016 06:00 PM, Nathaniel McCallum wrote: Pavel, since we made the change here from a StrEnum to a Str, we need to update the UI patch accordingly. How should admin know what to write there intuitively? Shouldn't Web UI or CLI advertise the indicators supported by IPA? E.g. CLI in doc string. Web UI might even combine checkboxes (otp, radius) with textbox. That would be better, I think. We still need to keep the API with a free text field but Web UI, of course, should provide some pre-defined labels. -- / Alexander Bokovoy -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] [PATCH 0093] Enable service authentication indicator management
On 05/27/2016 06:00 PM, Nathaniel McCallum wrote: > Pavel, since we made the change here from a StrEnum to a Str, we need > to update the UI patch accordingly. How should admin know what to write there intuitively? Shouldn't Web UI or CLI advertise the indicators supported by IPA? E.g. CLI in doc string. Web UI might even combine checkboxes (otp, radius) with textbox. > > On Fri, 2016-05-27 at 11:55 -0400, Nathaniel McCallum wrote: >> On Fri, 2016-05-27 at 18:35 +0300, Alexander Bokovoy wrote: >>> On Fri, 27 May 2016, Nathaniel McCallum wrote: All core functionality for authentication indicators has already been merged. All that is left is the CLI and UI patches. Attached is the CLI patch. One outstanding question that I have is how to future-proof this patch. Right now, we want to only permit two possible values: otp and radius. So we are using an StrEnum. However, in the future (probably after krb5-spake) we may want to have per-token custom indicators. That means that this value will need to become a Str. >>> PKINIT has already support for AI, so it would be good to add >>> pkinit >>> indicator as well. The problem here is that pkinit indicator is not >>> fixed and can be defined in the krb5.conf. >> >> Okay. You've convinced me that we should just make it a string now >> and >> be done with it since administrators can already set custom AIs. New >> patch attached. I think this is ready for merge. >> How do I code this so that we can later do a StrEnum => Str transition without breaking API? >>> Maybe just go to Str* right now and make a validation function that >>> performs the actual check? Once you'd upgrade the validation code >>> would >>> change but method signature wouldn't. >> >> Since admins can already set custom AIs, there is no reason for a >> validator. Let's just accept everything. > -- Petr Vobornik -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] [PATCH 0093] Enable service authentication indicator management
Pavel, since we made the change here from a StrEnum to a Str, we need to update the UI patch accordingly. On Fri, 2016-05-27 at 11:55 -0400, Nathaniel McCallum wrote: > On Fri, 2016-05-27 at 18:35 +0300, Alexander Bokovoy wrote: > > On Fri, 27 May 2016, Nathaniel McCallum wrote: > > > All core functionality for authentication indicators has already > > > been > > > merged. All that is left is the CLI and UI patches. Attached is > > > the > > > CLI > > > patch. > > > > > > One outstanding question that I have is how to future-proof this > > > patch. > > > Right now, we want to only permit two possible values: otp and > > > radius. > > > So we are using an StrEnum. However, in the future (probably > > > after > > > krb5-spake) we may want to have per-token custom indicators. That > > > means > > > that this value will need to become a Str. > > PKINIT has already support for AI, so it would be good to add > > pkinit > > indicator as well. The problem here is that pkinit indicator is not > > fixed and can be defined in the krb5.conf. > > Okay. You've convinced me that we should just make it a string now > and > be done with it since administrators can already set custom AIs. New > patch attached. I think this is ready for merge. > > > > How do I code this so that we can later do a StrEnum => Str > > > transition > > > without breaking API? > > Maybe just go to Str* right now and make a validation function that > > performs the actual check? Once you'd upgrade the validation code > > would > > change but method signature wouldn't. > > Since admins can already set custom AIs, there is no reason for a > validator. Let's just accept everything. -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] [PATCH 0093] Enable service authentication indicator management
On Fri, 2016-05-27 at 18:35 +0300, Alexander Bokovoy wrote:
> On Fri, 27 May 2016, Nathaniel McCallum wrote:
> > All core functionality for authentication indicators has already
> > been
> > merged. All that is left is the CLI and UI patches. Attached is the
> > CLI
> > patch.
> >
> > One outstanding question that I have is how to future-proof this
> > patch.
> > Right now, we want to only permit two possible values: otp and
> > radius.
> > So we are using an StrEnum. However, in the future (probably after
> > krb5-spake) we may want to have per-token custom indicators. That
> > means
> > that this value will need to become a Str.
> PKINIT has already support for AI, so it would be good to add pkinit
> indicator as well. The problem here is that pkinit indicator is not
> fixed and can be defined in the krb5.conf.
Okay. You've convinced me that we should just make it a string now and
be done with it since administrators can already set custom AIs. New
patch attached. I think this is ready for merge.
> > How do I code this so that we can later do a StrEnum => Str
> > transition
> > without breaking API?
> Maybe just go to Str* right now and make a validation function that
> performs the actual check? Once you'd upgrade the validation code
> would
> change but method signature wouldn't.
Since admins can already set custom AIs, there is no reason for a
validator. Let's just accept everything.From 6edda3ea40a4ab1bc3a45c1415b8a2ee9150af34 Mon Sep 17 00:00:00 2001
From: Nathaniel McCallum
Date: Wed, 4 May 2016 17:08:45 -0400
Subject: [PATCH] Enable service authentication indicator management
https://fedorahosted.org/freeipa/ticket/433
---
API.txt | 9 ++---
VERSION | 4 ++--
ipalib/plugins/service.py | 9 -
3 files changed, 16 insertions(+), 6 deletions(-)
diff --git a/API.txt b/API.txt
index dbc6f1adc614607fab106ab0de7163961e7ecedc..16c286d85fb1c94b9c438bb908d735cc716ce728 100644
--- a/API.txt
+++ b/API.txt
@@ -3901,7 +3901,7 @@ output: Entry('result')
output: Output('summary', type=[, ])
output: PrimaryKey('value')
command: service_add
-args: 1,11,3
+args: 1,12,3
arg: Str('krbprincipalname', cli_name='principal')
option: Str('addattr*', cli_name='addattr')
option: Flag('all', autofill=True, cli_name='all', default=False)
@@ -3909,6 +3909,7 @@ option: Flag('force', autofill=True, default=False)
option: StrEnum('ipakrbauthzdata*', cli_name='pac_type', values=[u'MS-PAC', u'PAD', u'NONE'])
option: Bool('ipakrbokasdelegate?', cli_name='ok_as_delegate')
option: Bool('ipakrbrequirespreauth?', cli_name='requires_pre_auth')
+option: Str('krbprincipalauthind*', cli_name='auth_ind')
option: Flag('no_members', autofill=True, default=False)
option: Flag('raw', autofill=True, cli_name='raw', default=False)
option: Str('setattr*', cli_name='setattr')
@@ -4011,10 +4012,11 @@ output: Output('completed', type=[])
output: Output('failed', type=[])
output: Entry('result')
command: service_find
-args: 1,11,4
+args: 1,12,4
arg: Str('criteria?')
option: Flag('all', autofill=True, cli_name='all', default=False)
option: StrEnum('ipakrbauthzdata*', autofill=False, cli_name='pac_type', values=[u'MS-PAC', u'PAD', u'NONE'])
+option: Str('krbprincipalauthind*', autofill=False, cli_name='auth_ind')
option: Str('krbprincipalname?', autofill=False, cli_name='principal')
option: Str('man_by_host*', cli_name='man_by_hosts')
option: Flag('no_members', autofill=True, default=False)
@@ -4029,7 +4031,7 @@ output: ListOfEntries('result')
output: Output('summary', type=[, ])
output: Output('truncated', type=[])
command: service_mod
-args: 1,12,3
+args: 1,13,3
arg: Str('krbprincipalname', cli_name='principal')
option: Str('addattr*', cli_name='addattr')
option: Flag('all', autofill=True, cli_name='all', default=False)
@@ -4037,6 +4039,7 @@ option: Str('delattr*', cli_name='delattr')
option: StrEnum('ipakrbauthzdata*', autofill=False, cli_name='pac_type', values=[u'MS-PAC', u'PAD', u'NONE'])
option: Bool('ipakrbokasdelegate?', autofill=False, cli_name='ok_as_delegate')
option: Bool('ipakrbrequirespreauth?', autofill=False, cli_name='requires_pre_auth')
+option: Str('krbprincipalauthind*', autofill=False, cli_name='auth_ind')
option: Flag('no_members', autofill=True, default=False)
option: Flag('raw', autofill=True, cli_name='raw', default=False)
option: Flag('rights', autofill=True, default=False)
diff --git a/VERSION b/VERSION
index eb7957eb1c5ae2487975a2fae4485a43f613cb64..bdf408e2ed108dbf7503970559c39b998fa2689e 100644
--- a/VERSION
+++ b/VERSION
@@ -90,5 +90,5 @@ IPA_DATA_VERSION=2010061412
# #
IPA_API_VERSION_MAJOR=2
-IPA_API_VERSION_MINOR=169
-# Last change: vault: copy arguments of client commands from server counterparts
+IPA_API_VERSION_MINOR=170
+# Last change: npmccallum - enable setting authinds on services
diff --git a/ipalib/plugins/serv
Re: [Freeipa-devel] [PATCH 0093] Enable service authentication indicator management
On Fri, 27 May 2016, Nathaniel McCallum wrote: All core functionality for authentication indicators has already been merged. All that is left is the CLI and UI patches. Attached is the CLI patch. One outstanding question that I have is how to future-proof this patch. Right now, we want to only permit two possible values: otp and radius. So we are using an StrEnum. However, in the future (probably after krb5-spake) we may want to have per-token custom indicators. That means that this value will need to become a Str. PKINIT has already support for AI, so it would be good to add pkinit indicator as well. The problem here is that pkinit indicator is not fixed and can be defined in the krb5.conf. How do I code this so that we can later do a StrEnum => Str transition without breaking API? Maybe just go to Str* right now and make a validation function that performs the actual check? Once you'd upgrade the validation code would change but method signature wouldn't. -- / Alexander Bokovoy -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
