Re: [Freeipa-devel] [PATCH] 1112 Add service constraint delegation plugin
Dne 3.6.2015 v 11:34 Martin Basti napsal(a):
On 02/06/15 22:51, Rob Crittenden wrote:
Martin Basti wrote:
On 31/05/15 04:07, Rob Crittenden wrote:
Petr Vobornik wrote:
On 05/27/2015 08:17 PM, Martin Basti wrote:
On 27/05/15 19:27, Rob Crittenden wrote:
Martin Basti wrote:
Thank you.
I haven't finished review yet, but I have few notes in case you
will
modify the patch.
Please fix following issues:
3)
There are many PEP8 errors, can you fix some of them,?
Is PEP8 a concern? What kinds of errors do we fix? For example, the
current model for defining options generates a slew of indention
errors.
In old modules it's preferred to keep the old indentation style for
options(not to mix 2 styles). New modules should use following pep8
compliant style:
Str(
'cn',
cli_name='name',
primary_key=True,
label=_('Server name'),
doc=_('IPA server hostname'),
),
We try to keep PEP8 in new code, mainly indentation, blank lines, too
long lines.
Yes in test definitions and option definitions, is better to keep the
same style, but other parts of code should be PEP8.
For example these should be fixed
./ipatests/test_xmlrpc/test_serviceconstraint_plugin.py:37:13: E225
missing whitespace around operator
./ipatests/test_xmlrpc/test_serviceconstraint_plugin.py:39:1: E302
expected 2 blank lines, found 1
./ipatests/test_xmlrpc/test_serviceconstraint_plugin.py:42:1: E302
expected 2 blank lines, found 1
I'll wait and see what falls out of the API review before making any
real changes.
rob
Updated API and addressed Martin's concerns. The regex must have been
a bad copy/paste, it is fixed now.
The design page has been updated as well.
rob
Hello,
comments below, in the right thread:
1)
+Str(
+'memberprincipal',
+label=_('Failed principals'),
+),
+Str(
+'ipaallowedtarget',
+label=_('Failed targets'),
+),
+Str(
+'servicedelegationrule',
+label=_('principal member'),
+),
Are these names correct?
# ipa servicedelegationrule-find
--
1 service delegation rule matched
--
Delegation name: ipa-http-delegation
Allowed Target: ipa-ldap-delegation-targets,
ipa-cifs-delegation-targets
Failed principals: HTTP/vm-093.example@example.com
Fixed.
2)
+ pattern='^[a-zA-Z0-9_.][a-zA-Z0-9_.-]{0,252}[a-zA-Z0-9_.$-]?$',
+pattern_errmsg='may only include letters, numbers, _, -,
., '
+ 'and a space inside',
This regex does not allow space inside
In [6]: print re.match(pattern, 'lalalala lalala')
None
Fixed. I'm tempted to just drop this regex entirely. Other plugins
have no such restrictions, but this should work better now.
3)
+yield Str('%s*' % name, cli_name='%ss' % name, doc=doc,
+ label=_('member %s') % name,
+ csv=True, alwaysask=True)
IMHO CSV values should not be supported.
Honza told me, the option doesn't work anyway.
Yeah, a copy and paste issue.
Patch with minor fixes attached.
I removed unused code and PEP8 complains
Incorporated and fixed a number of other things, including some typos
in the doc examples.
rob
Thank you, ACK!
Pushed to master: a92328452dced34d6d6df7ad6fe585563bb909f6
--
Jan Cholasta
--
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] 1112 Add service constraint delegation plugin
Martin Basti wrote:
On 31/05/15 04:07, Rob Crittenden wrote:
Petr Vobornik wrote:
On 05/27/2015 08:17 PM, Martin Basti wrote:
On 27/05/15 19:27, Rob Crittenden wrote:
Martin Basti wrote:
Thank you.
I haven't finished review yet, but I have few notes in case you will
modify the patch.
Please fix following issues:
3)
There are many PEP8 errors, can you fix some of them,?
Is PEP8 a concern? What kinds of errors do we fix? For example, the
current model for defining options generates a slew of indention
errors.
In old modules it's preferred to keep the old indentation style for
options(not to mix 2 styles). New modules should use following pep8
compliant style:
Str(
'cn',
cli_name='name',
primary_key=True,
label=_('Server name'),
doc=_('IPA server hostname'),
),
We try to keep PEP8 in new code, mainly indentation, blank lines, too
long lines.
Yes in test definitions and option definitions, is better to keep the
same style, but other parts of code should be PEP8.
For example these should be fixed
./ipatests/test_xmlrpc/test_serviceconstraint_plugin.py:37:13: E225
missing whitespace around operator
./ipatests/test_xmlrpc/test_serviceconstraint_plugin.py:39:1: E302
expected 2 blank lines, found 1
./ipatests/test_xmlrpc/test_serviceconstraint_plugin.py:42:1: E302
expected 2 blank lines, found 1
I'll wait and see what falls out of the API review before making any
real changes.
rob
Updated API and addressed Martin's concerns. The regex must have been
a bad copy/paste, it is fixed now.
The design page has been updated as well.
rob
Hello,
comments below, in the right thread:
1)
+Str(
+'memberprincipal',
+label=_('Failed principals'),
+),
+Str(
+'ipaallowedtarget',
+label=_('Failed targets'),
+),
+Str(
+'servicedelegationrule',
+label=_('principal member'),
+),
Are these names correct?
# ipa servicedelegationrule-find
--
1 service delegation rule matched
--
Delegation name: ipa-http-delegation
Allowed Target: ipa-ldap-delegation-targets,
ipa-cifs-delegation-targets
Failed principals: HTTP/vm-093.example@example.com
Fixed.
2)
+ pattern='^[a-zA-Z0-9_.][a-zA-Z0-9_.-]{0,252}[a-zA-Z0-9_.$-]?$',
+pattern_errmsg='may only include letters, numbers, _, -, ., '
+ 'and a space inside',
This regex does not allow space inside
In [6]: print re.match(pattern, 'lalalala lalala')
None
Fixed. I'm tempted to just drop this regex entirely. Other plugins have
no such restrictions, but this should work better now.
3)
+yield Str('%s*' % name, cli_name='%ss' % name, doc=doc,
+ label=_('member %s') % name,
+ csv=True, alwaysask=True)
IMHO CSV values should not be supported.
Honza told me, the option doesn't work anyway.
Yeah, a copy and paste issue.
Patch with minor fixes attached.
I removed unused code and PEP8 complains
Incorporated and fixed a number of other things, including some typos in
the doc examples.
rob
From 8458d605dcbe30191dda561d60d0145b44a65cfb Mon Sep 17 00:00:00 2001
From: Rob Crittenden rcrit...@redhat.com
Date: Thu, 14 May 2015 13:08:58 +
Subject: [PATCH] Add plugin to manage service constraint delegations
Service Constraints are the delegation model used by
ipa-kdb to grant service A to obtain a TGT for a user
against service B.
https://fedorahosted.org/freeipa/ticket/3644
---
ACI.txt| 16 +
API.txt| 153 ++
VERSION| 4 +-
install/updates/20-indices.update | 9 +
install/updates/25-referint.update | 1 +
ipalib/plugins/servicedelegation.py| 537 +++
ipatests/test_xmlrpc/objectclasses.py | 11 +
.../test_xmlrpc/test_servicedelegation_plugin.py | 591 +
8 files changed, 1320 insertions(+), 2 deletions(-)
create mode 100644 ipalib/plugins/servicedelegation.py
create mode 100644 ipatests/test_xmlrpc/test_servicedelegation_plugin.py
diff --git a/ACI.txt b/ACI.txt
index 3c4ebde..1821696 100644
--- a/ACI.txt
+++ b/ACI.txt
@@ -212,6 +212,22 @@ dn: cn=services,cn=accounts,dc=ipa,dc=example
aci: (targetattr = createtimestamp || entryusn || ipakrbauthzdata || ipakrbprincipalalias || ipauniqueid || krbcanonicalname || krblastpwdchange || krbobjectreferences || krbpasswordexpiration || krbprincipalaliases || krbprincipalexpiration || krbprincipalname || managedby || memberof || modifytimestamp || objectclass || usercertificate)(targetfilter = (objectclass=ipaservice))(version 3.0;acl permission:System: Read Services;allow (compare,read,search) userdn = ldap:///all;;)
dn:
Re: [Freeipa-devel] [PATCH] 1112 Add service constraint delegation plugin
On 31/05/15 04:07, Rob Crittenden wrote:
Petr Vobornik wrote:
On 05/27/2015 08:17 PM, Martin Basti wrote:
On 27/05/15 19:27, Rob Crittenden wrote:
Martin Basti wrote:
Thank you.
I haven't finished review yet, but I have few notes in case you will
modify the patch.
Please fix following issues:
3)
There are many PEP8 errors, can you fix some of them,?
Is PEP8 a concern? What kinds of errors do we fix? For example, the
current model for defining options generates a slew of indention
errors.
In old modules it's preferred to keep the old indentation style for
options(not to mix 2 styles). New modules should use following pep8
compliant style:
Str(
'cn',
cli_name='name',
primary_key=True,
label=_('Server name'),
doc=_('IPA server hostname'),
),
We try to keep PEP8 in new code, mainly indentation, blank lines, too
long lines.
Yes in test definitions and option definitions, is better to keep the
same style, but other parts of code should be PEP8.
For example these should be fixed
./ipatests/test_xmlrpc/test_serviceconstraint_plugin.py:37:13: E225
missing whitespace around operator
./ipatests/test_xmlrpc/test_serviceconstraint_plugin.py:39:1: E302
expected 2 blank lines, found 1
./ipatests/test_xmlrpc/test_serviceconstraint_plugin.py:42:1: E302
expected 2 blank lines, found 1
I'll wait and see what falls out of the API review before making any
real changes.
rob
Updated API and addressed Martin's concerns. The regex must have been
a bad copy/paste, it is fixed now.
The design page has been updated as well.
rob
Hello,
comments below, in the right thread:
1)
+Str(
+'memberprincipal',
+label=_('Failed principals'),
+),
+Str(
+'ipaallowedtarget',
+label=_('Failed targets'),
+),
+Str(
+'servicedelegationrule',
+label=_('principal member'),
+),
Are these names correct?
# ipa servicedelegationrule-find
--
1 service delegation rule matched
--
Delegation name: ipa-http-delegation
Allowed Target: ipa-ldap-delegation-targets,
ipa-cifs-delegation-targets
Failed principals: HTTP/vm-093.example@example.com
2)
+ pattern='^[a-zA-Z0-9_.][a-zA-Z0-9_.-]{0,252}[a-zA-Z0-9_.$-]?$',
+pattern_errmsg='may only include letters, numbers, _, -, ., '
+ 'and a space inside',
This regex does not allow space inside
In [6]: print re.match(pattern, 'lalalala lalala')
None
3)
+yield Str('%s*' % name, cli_name='%ss' % name, doc=doc,
+ label=_('member %s') % name,
+ csv=True, alwaysask=True)
IMHO CSV values should not be supported.
Honza told me, the option doesn't work anyway.
Patch with minor fixes attached.
I removed unused code and PEP8 complains
--
Martin Basti
From 3d051d3ec13fe19e2e5246b28e848460538d81bf Mon Sep 17 00:00:00 2001
From: Martin Basti mba...@redhat.com
Date: Tue, 2 Jun 2015 15:49:21 +0200
Subject: [PATCH] review service delegation
---
ipalib/plugins/servicedelegation.py | 40 ++---
1 file changed, 15 insertions(+), 25 deletions(-)
diff --git a/ipalib/plugins/servicedelegation.py b/ipalib/plugins/servicedelegation.py
index 434c446de590299b8a1c26344cbecd17f6a8ef3c..50b07dc8ad3112c0ee636b0d0ea7e0cd59365d9c 100644
--- a/ipalib/plugins/servicedelegation.py
+++ b/ipalib/plugins/servicedelegation.py
@@ -178,8 +178,7 @@ class servicedelegation_add_member(LDAPAddMember):
name = self.member_names[attr]
doc = self.member_param_doc % name
yield Str('%s*' % name, cli_name='%ss' % name, doc=doc,
- label=_('member %s') % name,
- csv=True, alwaysask=True)
+ label=_('member %s') % name, alwaysask=True)
def get_member_dns(self, **options):
@@ -187,7 +186,7 @@ class servicedelegation_add_member(LDAPAddMember):
special handling since it is just a principal, not a
full dn.
-return (dict(), dict())
+return dict(), dict()
def post_callback(self, ldap, completed, failed, dn, entry_attrs,
*keys, **options):
@@ -210,8 +209,8 @@ class servicedelegation_add_member(LDAPAddMember):
name = normalize_principal(name)
obj_dn = ldap_obj.get_dn(name)
try:
-s_attrs = ldap.get_entry(obj_dn, ['krbprincipalname'])
-except errors.NotFound, e:
+ldap.get_entry(obj_dn, ['krbprincipalname'])
+except errors.NotFound as e:
failed[attr][ldap_obj_name].append((name, unicode(e)))
continue
try:
@@ -219,22 +218,21 @@ class servicedelegation_add_member(LDAPAddMember):
members.append(name)
Re: [Freeipa-devel] [PATCH] 1112 Add service constraint delegation plugin
Petr Vobornik wrote:
On 05/27/2015 08:17 PM, Martin Basti wrote:
On 27/05/15 19:27, Rob Crittenden wrote:
Martin Basti wrote:
Thank you.
I haven't finished review yet, but I have few notes in case you will
modify the patch.
Please fix following issues:
3)
There are many PEP8 errors, can you fix some of them,?
Is PEP8 a concern? What kinds of errors do we fix? For example, the
current model for defining options generates a slew of indention errors.
In old modules it's preferred to keep the old indentation style for
options(not to mix 2 styles). New modules should use following pep8
compliant style:
Str(
'cn',
cli_name='name',
primary_key=True,
label=_('Server name'),
doc=_('IPA server hostname'),
),
We try to keep PEP8 in new code, mainly indentation, blank lines, too
long lines.
Yes in test definitions and option definitions, is better to keep the
same style, but other parts of code should be PEP8.
For example these should be fixed
./ipatests/test_xmlrpc/test_serviceconstraint_plugin.py:37:13: E225
missing whitespace around operator
./ipatests/test_xmlrpc/test_serviceconstraint_plugin.py:39:1: E302
expected 2 blank lines, found 1
./ipatests/test_xmlrpc/test_serviceconstraint_plugin.py:42:1: E302
expected 2 blank lines, found 1
I'll wait and see what falls out of the API review before making any
real changes.
rob
Updated API and addressed Martin's concerns. The regex must have been a
bad copy/paste, it is fixed now.
The design page has been updated as well.
rob
From ea1fc1589227bcbe3ab91e34b76d3bae46883cfd Mon Sep 17 00:00:00 2001
From: Rob Crittenden rcrit...@redhat.com
Date: Thu, 14 May 2015 13:08:58 +
Subject: [PATCH] Add plugin to manage service constraint delegations
Service Constraints are the delegation model used by
ipa-kdb to grant service A to obtain a TGT for a user
against service B.
https://fedorahosted.org/freeipa/ticket/3644
---
ACI.txt| 16 +
API.txt| 153 ++
VERSION| 4 +-
install/updates/20-indices.update | 9 +
install/updates/25-referint.update | 1 +
ipalib/plugins/servicedelegation.py| 531 ++
ipatests/test_xmlrpc/objectclasses.py | 11 +
.../test_xmlrpc/test_servicedelegation_plugin.py | 591 +
8 files changed, 1314 insertions(+), 2 deletions(-)
create mode 100644 ipalib/plugins/servicedelegation.py
create mode 100644 ipatests/test_xmlrpc/test_servicedelegation_plugin.py
diff --git a/ACI.txt b/ACI.txt
index 3c4ebde5b3ac2eb0b8e9465c5f2bd74f5bdbfb01..1821696fda912fdd11149062f9feaf4edcf0adfd 100644
--- a/ACI.txt
+++ b/ACI.txt
@@ -212,6 +212,22 @@ dn: cn=services,cn=accounts,dc=ipa,dc=example
aci: (targetattr = createtimestamp || entryusn || ipakrbauthzdata || ipakrbprincipalalias || ipauniqueid || krbcanonicalname || krblastpwdchange || krbobjectreferences || krbpasswordexpiration || krbprincipalaliases || krbprincipalexpiration || krbprincipalname || managedby || memberof || modifytimestamp || objectclass || usercertificate)(targetfilter = (objectclass=ipaservice))(version 3.0;acl permission:System: Read Services;allow (compare,read,search) userdn = ldap:///all;;)
dn: cn=services,cn=accounts,dc=ipa,dc=example
aci: (targetfilter = (objectclass=ipaservice))(version 3.0;acl permission:System: Remove Services;allow (delete) groupdn = ldap:///cn=System: Remove Services,cn=permissions,cn=pbac,dc=ipa,dc=example;)
+dn: cn=s4u2proxy,cn=etc,dc=ipa,dc=example
+aci: (targetfilter = (objectclass=groupofprincipals))(version 3.0;acl permission:System: Add Service Delegations;allow (add) groupdn = ldap:///cn=System: Add Service Delegations,cn=permissions,cn=pbac,dc=ipa,dc=example;)
+dn: cn=s4u2proxy,cn=etc,dc=ipa,dc=example
+aci: (targetattr = ipaallowedtarget || memberprincipal)(targetfilter = (objectclass=groupofprincipals))(version 3.0;acl permission:System: Modify Service Delegation Membership;allow (write) groupdn = ldap:///cn=System: Modify Service Delegation Membership,cn=permissions,cn=pbac,dc=ipa,dc=example;)
+dn: cn=s4u2proxy,cn=etc,dc=ipa,dc=example
+aci: (targetattr = cn || createtimestamp || entryusn || ipaallowedtarget || memberprincipal || modifytimestamp || objectclass)(targetfilter = (objectclass=groupofprincipals))(version 3.0;acl permission:System: Read Service Delegations;allow (compare,read,search) groupdn = ldap:///cn=System: Read Service Delegations,cn=permissions,cn=pbac,dc=ipa,dc=example;)
+dn: cn=s4u2proxy,cn=etc,dc=ipa,dc=example
+aci: (targetfilter = (objectclass=groupofprincipals))(version 3.0;acl permission:System: Remove Service Delegations;allow (delete) groupdn = ldap:///cn=System: Remove Service Delegations,cn=permissions,cn=pbac,dc=ipa,dc=example;)
+dn: cn=s4u2proxy,cn=etc,dc=ipa,dc=example
+aci:
Re: [Freeipa-devel] [PATCH] 1112 Add service constraint delegation plugin
On 05/27/2015 08:17 PM, Martin Basti wrote:
On 27/05/15 19:27, Rob Crittenden wrote:
Martin Basti wrote:
Thank you.
I haven't finished review yet, but I have few notes in case you will
modify the patch.
Please fix following issues:
3)
There are many PEP8 errors, can you fix some of them,?
Is PEP8 a concern? What kinds of errors do we fix? For example, the
current model for defining options generates a slew of indention errors.
In old modules it's preferred to keep the old indentation style for
options(not to mix 2 styles). New modules should use following pep8
compliant style:
Str(
'cn',
cli_name='name',
primary_key=True,
label=_('Server name'),
doc=_('IPA server hostname'),
),
We try to keep PEP8 in new code, mainly indentation, blank lines, too
long lines.
Yes in test definitions and option definitions, is better to keep the
same style, but other parts of code should be PEP8.
For example these should be fixed
./ipatests/test_xmlrpc/test_serviceconstraint_plugin.py:37:13: E225
missing whitespace around operator
./ipatests/test_xmlrpc/test_serviceconstraint_plugin.py:39:1: E302
expected 2 blank lines, found 1
./ipatests/test_xmlrpc/test_serviceconstraint_plugin.py:42:1: E302
expected 2 blank lines, found 1
I'll wait and see what falls out of the API review before making any
real changes.
rob
Martin^2
--
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] 1112 Add service constraint delegation plugin
Dne 27.5.2015 v 19:38 Rob Crittenden napsal(a):
Petr Vobornik wrote:
On 05/27/2015 05:46 PM, Alexander Bokovoy wrote:
On Wed, 27 May 2015, Rob Crittenden wrote:
Petr Vobornik wrote:
On 05/20/2015 06:02 PM, Rob Crittenden wrote:
Rob Crittenden wrote:
Rob Crittenden wrote:
Add a plugin to manage service delegations, like the one allowing
the
HTTP service to obtain an ldap service ticket on behalf of the
user.
This does not include impersonation targets, so one cannot yet
limit by
user what tickets can be obtained.
There is also no referential integrity for the memberPrincipal
attribute
since it is a string and not a DN. I don't see a way around this
that
isn't either clunky or requires a 389-ds plugin, both of which are
overkill in this case IMHO.
If you wonder why all the overrides it's because all of this is
stored
in the same container, and membership-like functions are used for a
non-DN attribute (memberPrincipal).
I used Alexander's patch in the ticket as a jumping off point.
Removed a couple of hardcoded domain/realm elements in the tests.
I must be getting rustly. Forgot to include ACIs. Added now.
rob
Thanks for the design[1] and patches.
First some high level questions before any unnecessary changes are
made.
From API point of view wouldn't it be better to distinguish rules and
targets by object name so we could avoid usage of the --type option.
I.e., why expose internal schema limitations in the API?
Type could be implied by the object name and commands can do what they
do now. They could even have a common base class if needed.
E.g.:
servicedelegationrule-add MYRULE
servicedelegationrule-find
servicedelegationrule-del MYRULE
servicedelegationrule-add_member MYRULE --targets={MYTARGET,MYTARGET2}
--principals={..,..}
servicedelegationrule-remove_member MYRULE
--targets={MYTARGET,MYTARGET2} --principals={..,..}
servicedelegationtarget-add MYTARGET
servicedelegationtarget-find
servicedelegationtarget-del MYTARGET
servicedelegationtarget-add_member MYTARGET --principals={..,..}
servicedelegationtarget-remove_member MYTARGET --principals={..,..}
+1, but I would split servicedelegationrule-{add,remove}-member into
separate commands:
servicedelegationrule-add-member --principals=
servicedelegationrule-remove-member --principals=
servicedelegationrule-add-target --targets=
servicedelegationrule-remove-target --targets=
because one means services which can delegate and the other services
to which can be delegated.
Yes, I used delegation instead of constraint. What is the rationale
behind 'constraint'? To me 'constraint' specifies what kind of
delegation we want to set but a goal of S4U2 proxy is to allow
something
which is not allowed by default not to create a new constraint.
[1] http://www.freeipa.org/page/V4/Service_Constraint_Delegation
I considered that but we already have this precedent in automember so
I went with it. This is complex enough with the fake memberPrincipal
stuff, I don't want to explode it with a baseclass and two subclasses
as well, plus doubling the number of new API commands.
It could tolerated in automember given that hostgroup and group rules
have exactly the same schema and purpose. The only difference is a
different target.
On the other hand, here, the purpose of both types is different. One is
a rule, second a target. Separation of the names would tell it to the
users.
+1
Number of API commands is a topic for different discussion. In short, it
could be handled better in CLI and future doc.
I don't want to discuss implementation details(complexity) yet. Issue
with API is that we are stuck with it and can't change it(much).
I very much agree.
Technically this is called constrained delegation. I was trying to
keep the name short and still descriptive. There is already aci
delegation which is a completely separate thing.
I understand that. The existing delegation might be misleading and
should be distinguished from s4u2. But imagine that somebody told you
that he created a new service constraint rule of service A to service
B. Since there is no mention of word delegation nor S4U I wouldn't know
that it's related to ticket delegation. I would think the *opposite*.
That he constrained service A to do something with service B. But a
service delegation rule, constrained delegation rule or ticket
delegation rule says what it is actually about.
Rather than avoiding descriptive commands we should improve a speed bash
completion.
Perhaps, but it still possible to be excessive.
A feeble attempt to remove service from the object name:
A question: Even thought the kerberos feature is called S4U (service for
user), is it limited to service principals? Services are of course the
primary target but in theory they don't have to be, right?
What is the user case for non-services? Sure, you could probably use
this to allow paul to get an ldap ticket for dave, but why would you? It
would be nice for delegating calendar entries for a
Re: [Freeipa-devel] [PATCH] 1112 Add service constraint delegation plugin
On 27/05/15 19:27, Rob Crittenden wrote:
Martin Basti wrote:
On 20/05/15 18:02, Rob Crittenden wrote:
Rob Crittenden wrote:
Rob Crittenden wrote:
Add a plugin to manage service delegations, like the one allowing the
HTTP service to obtain an ldap service ticket on behalf of the user.
This does not include impersonation targets, so one cannot yet
limit by
user what tickets can be obtained.
There is also no referential integrity for the memberPrincipal
attribute
since it is a string and not a DN. I don't see a way around this that
isn't either clunky or requires a 389-ds plugin, both of which are
overkill in this case IMHO.
If you wonder why all the overrides it's because all of this is
stored
in the same container, and membership-like functions are used for a
non-DN attribute (memberPrincipal).
I used Alexander's patch in the ticket as a jumping off point.
Removed a couple of hardcoded domain/realm elements in the tests.
I must be getting rustly. Forgot to include ACIs. Added now.
rob
Thank you.
I haven't finished review yet, but I have few notes in case you will
modify the patch.
Please fix following issues:
1) Patch needs rebase, VERSION conflict
2)
+pattern='^[a-zA-Z0-9_.][
a-zA-Z0-9_.-]{0,252}[a-zA-Z0-9_.-]?$',
+pattern_errmsg='may only include letters, numbers, _, -, .,
and a space inside',
+maxlength=255,
If I count correctly, regexp allows only 254 characters, not 255, and
this regexp also allows the space at the end of the string.
IMHO '^[a-zA-Z0-9_.]([ a-zA-Z0-9_.-]*[a-zA-Z0-9_.-])?$' would work.
This is a direct copy from many places. I'd file a bug to fix all of
them I guess.
I cannot find the same regexp in current code, there are only patterns
without space, so the space issue is only in this patch.
Otherwise I agree to file a ticket to fix the length issue.
3)
There are many PEP8 errors, can you fix some of them,?
Is PEP8 a concern? What kinds of errors do we fix? For example, the
current model for defining options generates a slew of indention errors.
We try to keep PEP8 in new code, mainly indentation, blank lines, too
long lines.
Yes in test definitions and option definitions, is better to keep the
same style, but other parts of code should be PEP8.
For example these should be fixed
./ipatests/test_xmlrpc/test_serviceconstraint_plugin.py:37:13: E225
missing whitespace around operator
./ipatests/test_xmlrpc/test_serviceconstraint_plugin.py:39:1: E302
expected 2 blank lines, found 1
./ipatests/test_xmlrpc/test_serviceconstraint_plugin.py:42:1: E302
expected 2 blank lines, found 1
4)
Please use
except Exception as e: to be compatible with python 3
ok
5)
For new files we stared using shorter license header.
#
# Copyright (C) 2015 FreeIPA Contributors see COPYING for license
#
ok
I'll wait and see what falls out of the API review before making any
real changes.
rob
Martin^2
--
Martin Basti
--
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] 1112 Add service constraint delegation plugin
On Wed, 27 May 2015, Rob Crittenden wrote:
Petr Vobornik wrote:
On 05/20/2015 06:02 PM, Rob Crittenden wrote:
Rob Crittenden wrote:
Rob Crittenden wrote:
Add a plugin to manage service delegations, like the one allowing the
HTTP service to obtain an ldap service ticket on behalf of the user.
This does not include impersonation targets, so one cannot yet limit by
user what tickets can be obtained.
There is also no referential integrity for the memberPrincipal
attribute
since it is a string and not a DN. I don't see a way around this that
isn't either clunky or requires a 389-ds plugin, both of which are
overkill in this case IMHO.
If you wonder why all the overrides it's because all of this is stored
in the same container, and membership-like functions are used for a
non-DN attribute (memberPrincipal).
I used Alexander's patch in the ticket as a jumping off point.
Removed a couple of hardcoded domain/realm elements in the tests.
I must be getting rustly. Forgot to include ACIs. Added now.
rob
Thanks for the design[1] and patches.
First some high level questions before any unnecessary changes are made.
From API point of view wouldn't it be better to distinguish rules and
targets by object name so we could avoid usage of the --type option.
I.e., why expose internal schema limitations in the API?
Type could be implied by the object name and commands can do what they
do now. They could even have a common base class if needed.
E.g.:
servicedelegationrule-add MYRULE
servicedelegationrule-find
servicedelegationrule-del MYRULE
servicedelegationrule-add_member MYRULE --targets={MYTARGET,MYTARGET2}
--principals={..,..}
servicedelegationrule-remove_member MYRULE
--targets={MYTARGET,MYTARGET2} --principals={..,..}
servicedelegationtarget-add MYTARGET
servicedelegationtarget-find
servicedelegationtarget-del MYTARGET
servicedelegationtarget-add_member MYTARGET --principals={..,..}
servicedelegationtarget-remove_member MYTARGET --principals={..,..}
Yes, I used delegation instead of constraint. What is the rationale
behind 'constraint'? To me 'constraint' specifies what kind of
delegation we want to set but a goal of S4U2 proxy is to allow something
which is not allowed by default not to create a new constraint.
[1] http://www.freeipa.org/page/V4/Service_Constraint_Delegation
I considered that but we already have this precedent in automember so
I went with it. This is complex enough with the fake memberPrincipal
stuff, I don't want to explode it with a baseclass and two subclasses
as well, plus doubling the number of new API commands.
Technically this is called constrained delegation. I was trying to
keep the name short and still descriptive. There is already aci
delegation which is a completely separate thing.
I agree with Rob. There is also a need to address principals which
aren't directly accessible as IPA objects in the framework (think about
trusts, for example).
--
/ 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] 1112 Add service constraint delegation plugin
Petr Vobornik wrote:
On 05/20/2015 06:02 PM, Rob Crittenden wrote:
Rob Crittenden wrote:
Rob Crittenden wrote:
Add a plugin to manage service delegations, like the one allowing the
HTTP service to obtain an ldap service ticket on behalf of the user.
This does not include impersonation targets, so one cannot yet limit by
user what tickets can be obtained.
There is also no referential integrity for the memberPrincipal
attribute
since it is a string and not a DN. I don't see a way around this that
isn't either clunky or requires a 389-ds plugin, both of which are
overkill in this case IMHO.
If you wonder why all the overrides it's because all of this is stored
in the same container, and membership-like functions are used for a
non-DN attribute (memberPrincipal).
I used Alexander's patch in the ticket as a jumping off point.
Removed a couple of hardcoded domain/realm elements in the tests.
I must be getting rustly. Forgot to include ACIs. Added now.
rob
Thanks for the design[1] and patches.
First some high level questions before any unnecessary changes are made.
From API point of view wouldn't it be better to distinguish rules and
targets by object name so we could avoid usage of the --type option.
I.e., why expose internal schema limitations in the API?
Type could be implied by the object name and commands can do what they
do now. They could even have a common base class if needed.
E.g.:
servicedelegationrule-add MYRULE
servicedelegationrule-find
servicedelegationrule-del MYRULE
servicedelegationrule-add_member MYRULE --targets={MYTARGET,MYTARGET2}
--principals={..,..}
servicedelegationrule-remove_member MYRULE
--targets={MYTARGET,MYTARGET2} --principals={..,..}
servicedelegationtarget-add MYTARGET
servicedelegationtarget-find
servicedelegationtarget-del MYTARGET
servicedelegationtarget-add_member MYTARGET --principals={..,..}
servicedelegationtarget-remove_member MYTARGET --principals={..,..}
Yes, I used delegation instead of constraint. What is the rationale
behind 'constraint'? To me 'constraint' specifies what kind of
delegation we want to set but a goal of S4U2 proxy is to allow something
which is not allowed by default not to create a new constraint.
[1] http://www.freeipa.org/page/V4/Service_Constraint_Delegation
I considered that but we already have this precedent in automember so I
went with it. This is complex enough with the fake memberPrincipal
stuff, I don't want to explode it with a baseclass and two subclasses as
well, plus doubling the number of new API commands.
Technically this is called constrained delegation. I was trying to keep
the name short and still descriptive. There is already aci delegation
which is a completely separate thing.
rob
--
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] 1112 Add service constraint delegation plugin
On 05/20/2015 06:02 PM, Rob Crittenden wrote:
Rob Crittenden wrote:
Rob Crittenden wrote:
Add a plugin to manage service delegations, like the one allowing the
HTTP service to obtain an ldap service ticket on behalf of the user.
This does not include impersonation targets, so one cannot yet limit by
user what tickets can be obtained.
There is also no referential integrity for the memberPrincipal attribute
since it is a string and not a DN. I don't see a way around this that
isn't either clunky or requires a 389-ds plugin, both of which are
overkill in this case IMHO.
If you wonder why all the overrides it's because all of this is stored
in the same container, and membership-like functions are used for a
non-DN attribute (memberPrincipal).
I used Alexander's patch in the ticket as a jumping off point.
Removed a couple of hardcoded domain/realm elements in the tests.
I must be getting rustly. Forgot to include ACIs. Added now.
rob
Thanks for the design[1] and patches.
First some high level questions before any unnecessary changes are made.
From API point of view wouldn't it be better to distinguish rules and
targets by object name so we could avoid usage of the --type option.
I.e., why expose internal schema limitations in the API?
Type could be implied by the object name and commands can do what they
do now. They could even have a common base class if needed.
E.g.:
servicedelegationrule-add MYRULE
servicedelegationrule-find
servicedelegationrule-del MYRULE
servicedelegationrule-add_member MYRULE --targets={MYTARGET,MYTARGET2}
--principals={..,..}
servicedelegationrule-remove_member MYRULE
--targets={MYTARGET,MYTARGET2} --principals={..,..}
servicedelegationtarget-add MYTARGET
servicedelegationtarget-find
servicedelegationtarget-del MYTARGET
servicedelegationtarget-add_member MYTARGET --principals={..,..}
servicedelegationtarget-remove_member MYTARGET --principals={..,..}
Yes, I used delegation instead of constraint. What is the rationale
behind 'constraint'? To me 'constraint' specifies what kind of
delegation we want to set but a goal of S4U2 proxy is to allow something
which is not allowed by default not to create a new constraint.
[1] http://www.freeipa.org/page/V4/Service_Constraint_Delegation
--
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] 1112 Add service constraint delegation plugin
On 05/27/2015 05:46 PM, Alexander Bokovoy wrote:
On Wed, 27 May 2015, Rob Crittenden wrote:
Petr Vobornik wrote:
On 05/20/2015 06:02 PM, Rob Crittenden wrote:
Rob Crittenden wrote:
Rob Crittenden wrote:
Add a plugin to manage service delegations, like the one allowing the
HTTP service to obtain an ldap service ticket on behalf of the user.
This does not include impersonation targets, so one cannot yet
limit by
user what tickets can be obtained.
There is also no referential integrity for the memberPrincipal
attribute
since it is a string and not a DN. I don't see a way around this that
isn't either clunky or requires a 389-ds plugin, both of which are
overkill in this case IMHO.
If you wonder why all the overrides it's because all of this is
stored
in the same container, and membership-like functions are used for a
non-DN attribute (memberPrincipal).
I used Alexander's patch in the ticket as a jumping off point.
Removed a couple of hardcoded domain/realm elements in the tests.
I must be getting rustly. Forgot to include ACIs. Added now.
rob
Thanks for the design[1] and patches.
First some high level questions before any unnecessary changes are made.
From API point of view wouldn't it be better to distinguish rules and
targets by object name so we could avoid usage of the --type option.
I.e., why expose internal schema limitations in the API?
Type could be implied by the object name and commands can do what they
do now. They could even have a common base class if needed.
E.g.:
servicedelegationrule-add MYRULE
servicedelegationrule-find
servicedelegationrule-del MYRULE
servicedelegationrule-add_member MYRULE --targets={MYTARGET,MYTARGET2}
--principals={..,..}
servicedelegationrule-remove_member MYRULE
--targets={MYTARGET,MYTARGET2} --principals={..,..}
servicedelegationtarget-add MYTARGET
servicedelegationtarget-find
servicedelegationtarget-del MYTARGET
servicedelegationtarget-add_member MYTARGET --principals={..,..}
servicedelegationtarget-remove_member MYTARGET --principals={..,..}
Yes, I used delegation instead of constraint. What is the rationale
behind 'constraint'? To me 'constraint' specifies what kind of
delegation we want to set but a goal of S4U2 proxy is to allow something
which is not allowed by default not to create a new constraint.
[1] http://www.freeipa.org/page/V4/Service_Constraint_Delegation
I considered that but we already have this precedent in automember so
I went with it. This is complex enough with the fake memberPrincipal
stuff, I don't want to explode it with a baseclass and two subclasses
as well, plus doubling the number of new API commands.
It could tolerated in automember given that hostgroup and group rules
have exactly the same schema and purpose. The only difference is a
different target.
On the other hand, here, the purpose of both types is different. One is
a rule, second a target. Separation of the names would tell it to the users.
Number of API commands is a topic for different discussion. In short, it
could be handled better in CLI and future doc.
I don't want to discuss implementation details(complexity) yet. Issue
with API is that we are stuck with it and can't change it(much).
Technically this is called constrained delegation. I was trying to
keep the name short and still descriptive. There is already aci
delegation which is a completely separate thing.
I understand that. The existing delegation might be misleading and
should be distinguished from s4u2. But imagine that somebody told you
that he created a new service constraint rule of service A to service
B. Since there is no mention of word delegation nor S4U I wouldn't know
that it's related to ticket delegation. I would think the *opposite*.
That he constrained service A to do something with service B. But a
service delegation rule, constrained delegation rule or ticket
delegation rule says what it is actually about.
Rather than avoiding descriptive commands we should improve a speed bash
completion.
A feeble attempt to remove service from the object name:
A question: Even thought the kerberos feature is called S4U (service for
user), is it limited to service principals? Services are of course the
primary target but in theory they don't have to be, right?
I agree with Rob. There is also a need to address principals which
aren't directly accessible as IPA objects in the framework (think about
trusts, for example).
I don't think this part is affected at all. The API's have the same
capabilities.
--
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] 1112 Add service constraint delegation plugin
Petr Vobornik wrote:
On 05/27/2015 05:46 PM, Alexander Bokovoy wrote:
On Wed, 27 May 2015, Rob Crittenden wrote:
Petr Vobornik wrote:
On 05/20/2015 06:02 PM, Rob Crittenden wrote:
Rob Crittenden wrote:
Rob Crittenden wrote:
Add a plugin to manage service delegations, like the one allowing
the
HTTP service to obtain an ldap service ticket on behalf of the user.
This does not include impersonation targets, so one cannot yet
limit by
user what tickets can be obtained.
There is also no referential integrity for the memberPrincipal
attribute
since it is a string and not a DN. I don't see a way around this
that
isn't either clunky or requires a 389-ds plugin, both of which are
overkill in this case IMHO.
If you wonder why all the overrides it's because all of this is
stored
in the same container, and membership-like functions are used for a
non-DN attribute (memberPrincipal).
I used Alexander's patch in the ticket as a jumping off point.
Removed a couple of hardcoded domain/realm elements in the tests.
I must be getting rustly. Forgot to include ACIs. Added now.
rob
Thanks for the design[1] and patches.
First some high level questions before any unnecessary changes are
made.
From API point of view wouldn't it be better to distinguish rules and
targets by object name so we could avoid usage of the --type option.
I.e., why expose internal schema limitations in the API?
Type could be implied by the object name and commands can do what they
do now. They could even have a common base class if needed.
E.g.:
servicedelegationrule-add MYRULE
servicedelegationrule-find
servicedelegationrule-del MYRULE
servicedelegationrule-add_member MYRULE --targets={MYTARGET,MYTARGET2}
--principals={..,..}
servicedelegationrule-remove_member MYRULE
--targets={MYTARGET,MYTARGET2} --principals={..,..}
servicedelegationtarget-add MYTARGET
servicedelegationtarget-find
servicedelegationtarget-del MYTARGET
servicedelegationtarget-add_member MYTARGET --principals={..,..}
servicedelegationtarget-remove_member MYTARGET --principals={..,..}
Yes, I used delegation instead of constraint. What is the rationale
behind 'constraint'? To me 'constraint' specifies what kind of
delegation we want to set but a goal of S4U2 proxy is to allow
something
which is not allowed by default not to create a new constraint.
[1] http://www.freeipa.org/page/V4/Service_Constraint_Delegation
I considered that but we already have this precedent in automember so
I went with it. This is complex enough with the fake memberPrincipal
stuff, I don't want to explode it with a baseclass and two subclasses
as well, plus doubling the number of new API commands.
It could tolerated in automember given that hostgroup and group rules
have exactly the same schema and purpose. The only difference is a
different target.
On the other hand, here, the purpose of both types is different. One is
a rule, second a target. Separation of the names would tell it to the
users.
Number of API commands is a topic for different discussion. In short, it
could be handled better in CLI and future doc.
I don't want to discuss implementation details(complexity) yet. Issue
with API is that we are stuck with it and can't change it(much).
Technically this is called constrained delegation. I was trying to
keep the name short and still descriptive. There is already aci
delegation which is a completely separate thing.
I understand that. The existing delegation might be misleading and
should be distinguished from s4u2. But imagine that somebody told you
that he created a new service constraint rule of service A to service
B. Since there is no mention of word delegation nor S4U I wouldn't know
that it's related to ticket delegation. I would think the *opposite*.
That he constrained service A to do something with service B. But a
service delegation rule, constrained delegation rule or ticket
delegation rule says what it is actually about.
Rather than avoiding descriptive commands we should improve a speed bash
completion.
Perhaps, but it still possible to be excessive.
A feeble attempt to remove service from the object name:
A question: Even thought the kerberos feature is called S4U (service for
user), is it limited to service principals? Services are of course the
primary target but in theory they don't have to be, right?
What is the user case for non-services? Sure, you could probably use
this to allow paul to get an ldap ticket for dave, but why would you? It
would be nice for delegating calendar entries for a personal assistant,
for example, but would be an audit nightmare.
I agree with Rob. There is also a need to address principals which
aren't directly accessible as IPA objects in the framework (think about
trusts, for example).
I don't think this part is affected at all. The API's have the same
capabilities.
I'm not going to argue too much about this. I imagine that a vast
majority of users will never use this at all, and even then probably not
Re: [Freeipa-devel] [PATCH] 1112 Add service constraint delegation plugin
On 20/05/15 18:02, Rob Crittenden wrote:
Rob Crittenden wrote:
Rob Crittenden wrote:
Add a plugin to manage service delegations, like the one allowing the
HTTP service to obtain an ldap service ticket on behalf of the user.
This does not include impersonation targets, so one cannot yet limit by
user what tickets can be obtained.
There is also no referential integrity for the memberPrincipal
attribute
since it is a string and not a DN. I don't see a way around this that
isn't either clunky or requires a 389-ds plugin, both of which are
overkill in this case IMHO.
If you wonder why all the overrides it's because all of this is stored
in the same container, and membership-like functions are used for a
non-DN attribute (memberPrincipal).
I used Alexander's patch in the ticket as a jumping off point.
Removed a couple of hardcoded domain/realm elements in the tests.
I must be getting rustly. Forgot to include ACIs. Added now.
rob
Thank you.
I haven't finished review yet, but I have few notes in case you will
modify the patch.
Please fix following issues:
1) Patch needs rebase, VERSION conflict
2)
+pattern='^[a-zA-Z0-9_.][ a-zA-Z0-9_.-]{0,252}[a-zA-Z0-9_.-]?$',
+pattern_errmsg='may only include letters, numbers, _, -, .,
and a space inside',
+maxlength=255,
If I count correctly, regexp allows only 254 characters, not 255, and
this regexp also allows the space at the end of the string.
IMHO '^[a-zA-Z0-9_.]([ a-zA-Z0-9_.-]*[a-zA-Z0-9_.-])?$' would work.
3)
There are many PEP8 errors, can you fix some of them,?
4)
Please use
except Exception as e: to be compatible with python 3
5)
For new files we stared using shorter license header.
#
# Copyright (C) 2015 FreeIPA Contributors see COPYING for license
#
--
Martin Basti
--
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] 1112 Add service constraint delegation plugin
Martin Basti wrote:
On 20/05/15 18:02, Rob Crittenden wrote:
Rob Crittenden wrote:
Rob Crittenden wrote:
Add a plugin to manage service delegations, like the one allowing the
HTTP service to obtain an ldap service ticket on behalf of the user.
This does not include impersonation targets, so one cannot yet limit by
user what tickets can be obtained.
There is also no referential integrity for the memberPrincipal
attribute
since it is a string and not a DN. I don't see a way around this that
isn't either clunky or requires a 389-ds plugin, both of which are
overkill in this case IMHO.
If you wonder why all the overrides it's because all of this is stored
in the same container, and membership-like functions are used for a
non-DN attribute (memberPrincipal).
I used Alexander's patch in the ticket as a jumping off point.
Removed a couple of hardcoded domain/realm elements in the tests.
I must be getting rustly. Forgot to include ACIs. Added now.
rob
Thank you.
I haven't finished review yet, but I have few notes in case you will
modify the patch.
Please fix following issues:
1) Patch needs rebase, VERSION conflict
2)
+pattern='^[a-zA-Z0-9_.][ a-zA-Z0-9_.-]{0,252}[a-zA-Z0-9_.-]?$',
+pattern_errmsg='may only include letters, numbers, _, -, .,
and a space inside',
+maxlength=255,
If I count correctly, regexp allows only 254 characters, not 255, and
this regexp also allows the space at the end of the string.
IMHO '^[a-zA-Z0-9_.]([ a-zA-Z0-9_.-]*[a-zA-Z0-9_.-])?$' would work.
This is a direct copy from many places. I'd file a bug to fix all of
them I guess.
3)
There are many PEP8 errors, can you fix some of them,?
Is PEP8 a concern? What kinds of errors do we fix? For example, the
current model for defining options generates a slew of indention errors.
4)
Please use
except Exception as e: to be compatible with python 3
ok
5)
For new files we stared using shorter license header.
#
# Copyright (C) 2015 FreeIPA Contributors see COPYING for license
#
ok
I'll wait and see what falls out of the API review before making any
real changes.
rob
--
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] 1112 Add service constraint delegation plugin
On Wed, 2015-05-27 at 18:46 +0300, Alexander Bokovoy wrote:
On Wed, 27 May 2015, Rob Crittenden wrote:
Petr Vobornik wrote:
On 05/20/2015 06:02 PM, Rob Crittenden wrote:
Rob Crittenden wrote:
Rob Crittenden wrote:
Add a plugin to manage service delegations, like the one allowing the
HTTP service to obtain an ldap service ticket on behalf of the user.
This does not include impersonation targets, so one cannot yet limit by
user what tickets can be obtained.
There is also no referential integrity for the memberPrincipal
attribute
since it is a string and not a DN. I don't see a way around this that
isn't either clunky or requires a 389-ds plugin, both of which are
overkill in this case IMHO.
If you wonder why all the overrides it's because all of this is stored
in the same container, and membership-like functions are used for a
non-DN attribute (memberPrincipal).
I used Alexander's patch in the ticket as a jumping off point.
Removed a couple of hardcoded domain/realm elements in the tests.
I must be getting rustly. Forgot to include ACIs. Added now.
rob
Thanks for the design[1] and patches.
First some high level questions before any unnecessary changes are made.
From API point of view wouldn't it be better to distinguish rules and
targets by object name so we could avoid usage of the --type option.
I.e., why expose internal schema limitations in the API?
Type could be implied by the object name and commands can do what they
do now. They could even have a common base class if needed.
E.g.:
servicedelegationrule-add MYRULE
servicedelegationrule-find
servicedelegationrule-del MYRULE
servicedelegationrule-add_member MYRULE --targets={MYTARGET,MYTARGET2}
--principals={..,..}
servicedelegationrule-remove_member MYRULE
--targets={MYTARGET,MYTARGET2} --principals={..,..}
servicedelegationtarget-add MYTARGET
servicedelegationtarget-find
servicedelegationtarget-del MYTARGET
servicedelegationtarget-add_member MYTARGET --principals={..,..}
servicedelegationtarget-remove_member MYTARGET --principals={..,..}
Yes, I used delegation instead of constraint. What is the rationale
behind 'constraint'? To me 'constraint' specifies what kind of
delegation we want to set but a goal of S4U2 proxy is to allow something
which is not allowed by default not to create a new constraint.
[1] http://www.freeipa.org/page/V4/Service_Constraint_Delegation
I considered that but we already have this precedent in automember so
I went with it. This is complex enough with the fake memberPrincipal
stuff, I don't want to explode it with a baseclass and two subclasses
as well, plus doubling the number of new API commands.
Technically this is called constrained delegation. I was trying to
keep the name short and still descriptive. There is already aci
delegation which is a completely separate thing.
I agree with Rob. There is also a need to address principals which
aren't directly accessible as IPA objects in the framework (think about
trusts, for example).
Shouldn't the commands then be: constraineddelegation-add etc...
serviceconstraint doesn't really help understanding what this is all
about. If service is in the name I agree with Petr that
servicedelegation makes much more sense.
On the API I also do not like to split rules into 2 apis, but I see no
other option really.
Something like the following makes it clear what you are operating on:
ipa servicedelegationtarget-add target [principals]
ipa servicedelegation-add rule --members [principals] --targets [targets]
Example:
ipa servicedelegationtarget-add ipa-ldap ldap/ipa1.example.com
ldap/ipa2.example.com
This would add ldap/ipa1.example@example.com and
ldap/ipa2.example@example.com to cn=ipa-ldap-delegation-targets
(note that rule name is shorter than DN and principals are automatically
augmented with the REALM part if not provided on the command line)
ipa servicedelegation-add ipa-http --targets ipa-ldap
This would create the cn=ipa-http-delegation rule and add
cn=ipa-ldap-delegation-targets,... to it (note again that the rule name
and target name are shortcuts wrt the actual DNs used)
ipa servicedelegation-mod ipa-http --add-members HTTP/ipa1.example.com
HTTP/ipa2.example.com
This would add member principals to cn=ipa-http-delegation
The full list of APIs would be:
ipa servicedelegation-add rule [--members principals] [--targets targets]
ipa servicedelegation-del rule
ipa servicedelegation-mod rule [--add-members Ps] [--del-members Ps]
[--add-targets Ts] [--del-targets Ts]
ipa servicedelegation-show rule
ipa servicedelegation-find [rule]
ipa servicedelegtarget-add target [--members principals]
ipa servicedelegtarget-del target
ipa servicedelegtarget-mod target [--add-members Ps] [--del-members Ps]
ipa servicedelegtarget-show target
ipa servicedelegtarget-find [target]
HTH,
Simo.
--
Simo Sorce * Red Hat, Inc * New York
--
Manage your subscription for the Freeipa-devel
Re: [Freeipa-devel] [PATCH] 1112 Add service constraint delegation plugin
On 05/20/2015 06:02 PM, Rob Crittenden wrote: Rob Crittenden wrote: Rob Crittenden wrote: Add a plugin to manage service delegations, like the one allowing the HTTP service to obtain an ldap service ticket on behalf of the user. This does not include impersonation targets, so one cannot yet limit by user what tickets can be obtained. There is also no referential integrity for the memberPrincipal attribute since it is a string and not a DN. I don't see a way around this that isn't either clunky or requires a 389-ds plugin, both of which are overkill in this case IMHO. If you wonder why all the overrides it's because all of this is stored in the same container, and membership-like functions are used for a non-DN attribute (memberPrincipal). I used Alexander's patch in the ticket as a jumping off point. Removed a couple of hardcoded domain/realm elements in the tests. I must be getting rustly. Forgot to include ACIs. Added now. Thanks Rob! Martin Basti planned to look at this patch set. BTW, I did not see any design. Would it be fine with you to prepare some? This is a new feature, far from straightforward one, so it would be very helpful to have some metadata and docs on FreeIPA.org design. Thanks, Martin -- 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] 1112 Add service constraint delegation plugin
Rob Crittenden wrote:
Rob Crittenden wrote:
Add a plugin to manage service delegations, like the one allowing the
HTTP service to obtain an ldap service ticket on behalf of the user.
This does not include impersonation targets, so one cannot yet limit by
user what tickets can be obtained.
There is also no referential integrity for the memberPrincipal attribute
since it is a string and not a DN. I don't see a way around this that
isn't either clunky or requires a 389-ds plugin, both of which are
overkill in this case IMHO.
If you wonder why all the overrides it's because all of this is stored
in the same container, and membership-like functions are used for a
non-DN attribute (memberPrincipal).
I used Alexander's patch in the ticket as a jumping off point.
Removed a couple of hardcoded domain/realm elements in the tests.
I must be getting rustly. Forgot to include ACIs. Added now.
rob
From f68fcf59b9792795d405bc35e5a8c3372f0fea7d Mon Sep 17 00:00:00 2001
From: Rob Crittenden rcrit...@redhat.com
Date: Thu, 14 May 2015 13:08:58 +
Subject: [PATCH] Add plugin to manage service constraints
Service Constraints are the delegation model used by
ipa-kdb to grant service A to obtain a TGT for a user
against service B.
https://fedorahosted.org/freeipa/ticket/3644
---
ACI.txt| 8 +
API.txt| 72
VERSION| 4 +-
install/updates/20-indices.update | 9 +
install/updates/25-referint.update | 1 +
ipalib/plugins/serviceconstraint.py| 473
ipatests/test_xmlrpc/objectclasses.py | 11 +
.../test_xmlrpc/test_serviceconstraint_plugin.py | 479 +
8 files changed, 1055 insertions(+), 2 deletions(-)
create mode 100644 ipalib/plugins/serviceconstraint.py
create mode 100644 ipatests/test_xmlrpc/test_serviceconstraint_plugin.py
diff --git a/ACI.txt b/ACI.txt
index bf539892910f14ebc3fbee88a72d2b57c0d1327b..c9dc92fe63c46992f0a99fca84b2a12d3ce03ce0 100644
--- a/ACI.txt
+++ b/ACI.txt
@@ -212,6 +212,14 @@ dn: cn=services,cn=accounts,dc=ipa,dc=example
aci: (targetattr = createtimestamp || entryusn || ipakrbauthzdata || ipakrbprincipalalias || ipauniqueid || krbcanonicalname || krblastpwdchange || krbobjectreferences || krbpasswordexpiration || krbprincipalaliases || krbprincipalexpiration || krbprincipalname || managedby || memberof || modifytimestamp || objectclass || usercertificate)(targetfilter = (objectclass=ipaservice))(version 3.0;acl permission:System: Read Services;allow (compare,read,search) userdn = ldap:///all;;)
dn: cn=services,cn=accounts,dc=ipa,dc=example
aci: (targetfilter = (objectclass=ipaservice))(version 3.0;acl permission:System: Remove Services;allow (delete) groupdn = ldap:///cn=System: Remove Services,cn=permissions,cn=pbac,dc=ipa,dc=example;)
+dn: cn=s4u2proxy,cn=etc,dc=ipa,dc=example
+aci: (targetfilter = (objectclass=groupofprincipals))(version 3.0;acl permission:System: Add Service Constraints;allow (add) groupdn = ldap:///cn=System: Add Service Constraints,cn=permissions,cn=pbac,dc=ipa,dc=example;)
+dn: cn=s4u2proxy,cn=etc,dc=ipa,dc=example
+aci: (targetattr = ipaallowedtarget || memberprincipal)(targetfilter = (objectclass=groupofprincipals))(version 3.0;acl permission:System: Modify Service Constraint Membership;allow (write) groupdn = ldap:///cn=System: Modify Service Constraint Membership,cn=permissions,cn=pbac,dc=ipa,dc=example;)
+dn: cn=s4u2proxy,cn=etc,dc=ipa,dc=example
+aci: (targetattr = cn || createtimestamp || entryusn || ipaallowedtarget || memberprincipal || modifytimestamp || objectclass)(targetfilter = (objectclass=groupofprincipals))(version 3.0;acl permission:System: Read Service Constraints;allow (compare,read,search) groupdn = ldap:///cn=System: Read Service Constraints,cn=permissions,cn=pbac,dc=ipa,dc=example;)
+dn: cn=s4u2proxy,cn=etc,dc=ipa,dc=example
+aci: (targetfilter = (objectclass=groupofprincipals))(version 3.0;acl permission:System: Remove Service Constraints;allow (delete) groupdn = ldap:///cn=System: Remove Service Constraints,cn=permissions,cn=pbac,dc=ipa,dc=example;)
dn: cn=staged users,cn=accounts,cn=provisioning,dc=ipa,dc=example
aci: (targetattr = *)(target = ldap:///uid=*,cn=staged users,cn=accounts,cn=provisioning,dc=ipa,dc=example)(targetfilter = (objectclass=*))(version 3.0;acl permission:System: Add Stage Users by Provisioning and Administrators;allow (add) groupdn = ldap:///cn=System: Add Stage Users by Provisioning and Administrators,cn=permissions,cn=pbac,dc=ipa,dc=example;)
dn: cn=staged users,cn=accounts,cn=provisioning,dc=ipa,dc=example
diff --git a/API.txt b/API.txt
index 0808f3c64595495c8a9e60da5cbd689d5cdc6224..b548132a1e119204cd8452c4b8db80fa00263ccc 100644
--- a/API.txt
+++ b/API.txt
@@ -3694,6 +3694,78 @@ option: Str('version?', exclude='webui')
output: Entry('result', type 'dict',
Re: [Freeipa-devel] [PATCH] 1112 Add service constraint delegation plugin
Rob Crittenden wrote:
Add a plugin to manage service delegations, like the one allowing the
HTTP service to obtain an ldap service ticket on behalf of the user.
This does not include impersonation targets, so one cannot yet limit by
user what tickets can be obtained.
There is also no referential integrity for the memberPrincipal attribute
since it is a string and not a DN. I don't see a way around this that
isn't either clunky or requires a 389-ds plugin, both of which are
overkill in this case IMHO.
If you wonder why all the overrides it's because all of this is stored
in the same container, and membership-like functions are used for a
non-DN attribute (memberPrincipal).
I used Alexander's patch in the ticket as a jumping off point.
Removed a couple of hardcoded domain/realm elements in the tests.
rob
From b1366069b4494cac38d486d9dda02a03226fd0d3 Mon Sep 17 00:00:00 2001
From: Rob Crittenden rcrit...@redhat.com
Date: Thu, 14 May 2015 13:08:58 +
Subject: [PATCH] Add plugin to manage service constraints
Service Constraints are the delegation model used by
ipa-kdb to grant service A to obtain a TGT for a user
against service B.
https://fedorahosted.org/freeipa/ticket/3644
---
API.txt| 72
VERSION| 4 +-
install/updates/20-indices.update | 9 +
install/updates/25-referint.update | 1 +
ipalib/plugins/serviceconstraint.py| 444 +++
ipatests/test_xmlrpc/objectclasses.py | 11 +
.../test_xmlrpc/test_serviceconstraint_plugin.py | 479 +
7 files changed, 1018 insertions(+), 2 deletions(-)
create mode 100644 ipalib/plugins/serviceconstraint.py
create mode 100644 ipatests/test_xmlrpc/test_serviceconstraint_plugin.py
diff --git a/API.txt b/API.txt
index 0808f3c64595495c8a9e60da5cbd689d5cdc6224..b548132a1e119204cd8452c4b8db80fa00263ccc 100644
--- a/API.txt
+++ b/API.txt
@@ -3694,6 +3694,78 @@ option: Str('version?', exclude='webui')
output: Entry('result', type 'dict', Gettext('A dictionary representing an LDAP entry', domain='ipa', localedir=None))
output: Output('summary', (type 'unicode', type 'NoneType'), None)
output: PrimaryKey('value', None, None)
+command: serviceconstraint_add
+args: 1,7,3
+arg: Str('cn', attribute=True, cli_name='constraint_name', maxlength=255, multivalue=False, pattern='^[a-zA-Z0-9_.][ a-zA-Z0-9_.-]{0,252}[a-zA-Z0-9_.-]?$', 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')
+option: Flag('no_members', autofill=True, default=False, exclude='webui')
+option: Flag('raw', autofill=True, cli_name='raw', default=False, exclude='webui')
+option: Str('setattr*', cli_name='setattr', exclude='webui')
+option: StrEnum('type', values=(u'rule', u'target'))
+option: Str('version?', exclude='webui')
+output: Entry('result', type 'dict', Gettext('A dictionary representing an LDAP entry', domain='ipa', localedir=None))
+output: Output('summary', (type 'unicode', type 'NoneType'), None)
+output: PrimaryKey('value', None, None)
+command: serviceconstraint_add_member
+args: 1,6,3
+arg: Str('cn', attribute=True, cli_name='constraint_name', maxlength=255, multivalue=False, pattern='^[a-zA-Z0-9_.][ a-zA-Z0-9_.-]{0,252}[a-zA-Z0-9_.-]?$', primary_key=True, query=True, required=True)
+option: Flag('all', autofill=True, cli_name='all', default=False, exclude='webui')
+option: Flag('no_members', autofill=True, default=False, exclude='webui')
+option: Str('principal*', alwaysask=True, cli_name='principals', csv=True)
+option: Flag('raw', autofill=True, cli_name='raw', default=False, exclude='webui')
+option: Str('target*', alwaysask=True, cli_name='targets', csv=True)
+option: Str('version?', exclude='webui')
+output: Output('completed', type 'int', None)
+output: Output('failed', type 'dict', None)
+output: Entry('result', type 'dict', Gettext('A dictionary representing an LDAP entry', domain='ipa', localedir=None))
+command: serviceconstraint_del
+args: 1,2,3
+arg: Str('cn', attribute=True, cli_name='constraint_name', maxlength=255, multivalue=True, pattern='^[a-zA-Z0-9_.][ a-zA-Z0-9_.-]{0,252}[a-zA-Z0-9_.-]?$', primary_key=True, query=True, required=True)
+option: Flag('continue', autofill=True, cli_name='continue', default=False)
+option: Str('version?', exclude='webui')
+output: Output('result', type 'dict', None)
+output: Output('summary', (type 'unicode', type 'NoneType'), None)
+output: ListOfPrimaryKeys('value', None, None)
+command: serviceconstraint_find
+args: 1,9,4
+arg: Str('criteria?', noextrawhitespace=False)
+option: Flag('all', autofill=True, cli_name='all', default=False, exclude='webui')
+option: Str('cn', attribute=True, autofill=False, cli_name='constraint_name', maxlength=255, multivalue=False, pattern='^[a-zA-Z0-9_.][
