Re: [Freeipa-devel] [WIP] Thin client

2016-05-27 Thread Martin Basti



On 27.05.2016 07:49, Jan Cholasta wrote:

On 26.5.2016 18:43, Martin Basti wrote:



On 26.05.2016 11:21, Martin Basti wrote:



On 26.05.2016 07:15, Jan Cholasta wrote:

On 25.5.2016 18:17, Martin Basti wrote:



On 25.05.2016 16:07, Jan Cholasta wrote:

On 25.5.2016 15:03, David Kupka wrote:

On 18/05/16 08:01, Jan Cholasta wrote:

On 16.5.2016 16:35, Martin Basti wrote:



On 09.05.2016 14:07, Jan Cholasta wrote:

On 6.5.2016 14:32, Martin Basti wrote:



On 28.04.2016 14:45, Jan Cholasta wrote:

Hi,

I have pushed my thin client WIP branch to GitHub:
.

All commits up to "ipalib: use relative imports for 
cross-plugin

imports" should be good for review. The rest is subject to
change
(WARNING: I will force push into this branch).

Honza


I did not test it yet, I just checked the code

* automount: do not inherit automountlocation_tofiles from
LDAPQuery *
LGTM

* dns: move all dnsrecord code called on client to a single
class *
LGTM

* dns: do not rely on server data structures in code called on
client *
1)
you forgot to increment VERSION


This was deliberate, as it will no longer be necessary to bump
VERSION
for backward compatible changes after this whole patchset is
merged.
But we're not there yet, so fixed.


How we should handle VERSION after your patches?


Otherwise LGTM

* otptoken: fix import of DN *
ACK

* otptoken_yubikey: fix otptoken_add_yubikey arguments *
1)
you forgot to increment VERSION


Fixed.



2)
Did you find out why this was issue?
-del answer['value']# Why does this cause an 
error if

omitted?
 -del answer['summary']  # Why does this cause an
error if
omitted?


The command definition was not complete, it was missing
has_output.



Otherwise LGTM

* vault: move client-side code to the module level *
LGTM

* vault: copy arguments of client commands from server
counterparts *
1)
you forgot to increment Version


Fixed.



* ipalib: use relative imports for cross-plugin imports *
1) Missing explanation for future generations why this 
change is

needed
in commit message


Fixed.



The other commits I will check later.
Martin^2



Okay. Thanks.



* frontend: remove the unused Command.soft_validate method
LGTM

* frontend: perform argument value validation only on server
LGTM

* frontend: do not forward argument defaults to server
I'm not a fan of returning  None in promt_param function, but I
havent
found anything better to use.


It always returned None for unset params.


LGTM

* ipalib: optimize API.txt
this contains a lot of black magic, but because this is mainly
copy of
current to proper places, LGTM


It's actually mostly cut-n-paste.



* makeaci: load additional plugins using API.add_module
Looks good, but I miss explanation why is this change needed


The next change would be impossible without this.



* plugable: replace API.import_plugins with new API.add_package
LGTM


* ipalib, ipaserver: migrate all plugins to Registry-based
registration
ACK

* ipalib, ipaserver: fix incorrect API.register calls in 
docstrings

LGTM


* plugable: remove the unused deprecated API.register method
LGTM


* plugable: switch API to Registry-based plugin discovery
LGTM

* frontend: merge baseldap.CallbackRegistry into Command
LGTM

*frontend: move the interactive_prompt callback type to Command
 LGTM

Martin^2







Hello,
first batch of 30 patches from Honza's trac-4739 branch
(https://github.com/jcholast/freeipa/commits/trac-4739) is ready
to be
pushed into master.
All upto "frontend: allow commands to have an argument named
`name`" got
over numerous test&fix cycles in last week+ and is working as
expected
now, ACK.


Thanks for the review.



Honzo, please rebase and push them, thanks!



Attaching the patches for reference.

Pushed to master: 2b50fc617024f18a81e97f30f75ed1b9221c4055



Guys, you forgot to test it with newer pylint.


I don't see any "BuildRequires: newer pylint" in the spec file.



Patch attached.


LGTM, although the commit message is wrong - this is not related to
thin client at all, PublicError and PublicMessage had .kw since 
forever.



updated commit message






Can I push that fix for pylint?


Sure, ACK.

Pushed to master: aa4123d852d81b908cd18208577bb509fff08c5b






I found something suspicious in tests, did anything in IPA messages
change? I suspect that this is related to client_patches.


Yes, 'data' is a dict which contains structured message data. I did 
not see these specific failures before, though. Just add it to 
expected results where missing.




E   AssertionError: assert_deepequal: dict keys mismatch.
E 0026: permission_find: Search for u'testperm' with a
limit of 1 (truncated) with members
E missing keys = []
E extra keys = [u'data']
E expected = {'message': u'Search result has been
truncated: Configured size limit exceeded', 'code': 13017, 'type':
u'warning', 'name': u'SearchResultTrunc

[Freeipa-devel] [Testplan] Support of UPN for trusted domains

2016-05-27 Thread Lenka Doudova

Hi all,


here [1] is a draft of test plan for V4 RFE Support of UPN for trusted 
domains.


Please review this and let me know if there's something missing or wrong.


Thanks,

Lenka


[1] 
http://www.freeipa.org/page/V4/Support_of_UPN_for_trusted_domains/Test_Plan


--
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] [Testplan] Support of UPN for trusted domains

2016-05-27 Thread Sumit Bose
On Fri, May 27, 2016 at 09:57:37AM +0200, Lenka Doudova wrote:
> Hi all,
> 
> 
> here [1] is a draft of test plan for V4 RFE Support of UPN for trusted
> domains.
> 
> Please review this and let me know if there's something missing or wrong.

Hi Lenka,

thank you for the test plan.

About the TBD, Alexander and I agreed to store the alternative domain
suffixes read from AD in a new attribute in the LDAP object of the
forest root of the trusted domain.

About the kinit tests. Please note that it is expected that the -E
option of kinit must be used when alternative suffixes are used.

I'm not sure if SSSD tests are in the scope here as well. If they are I
would suggest to add authentication tests with SSSD where e.g. the name
with an alternative domain suffix is used as login name. This in general
already works with SSSD but is disabled by default for IPA because of
the missing server-side support so far. Since SSSD must be able to work
with old and new IPA server https://fedorahosted.org/sssd/ticket/3018
was created so that SSSD can detect at runtime if the server supports
this or not.

bye,
Sumit


> 
> 
> Thanks,
> 
> Lenka
> 
> 
> [1]
> http://www.freeipa.org/page/V4/Support_of_UPN_for_trusted_domains/Test_Plan
> 
> -- 
> 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

-- 
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] [Testplan] Support of UPN for trusted domains

2016-05-27 Thread Alexander Bokovoy

On Fri, 27 May 2016, Sumit Bose wrote:

On Fri, May 27, 2016 at 09:57:37AM +0200, Lenka Doudova wrote:

Hi all,


here [1] is a draft of test plan for V4 RFE Support of UPN for trusted
domains.

Please review this and let me know if there's something missing or wrong.


Hi Lenka,

thank you for the test plan.

About the TBD, Alexander and I agreed to store the alternative domain
suffixes read from AD in a new attribute in the LDAP object of the
forest root of the trusted domain.

About the kinit tests. Please note that it is expected that the -E
option of kinit must be used when alternative suffixes are used.

I'm not sure if SSSD tests are in the scope here as well. If they are I
would suggest to add authentication tests with SSSD where e.g. the name
with an alternative domain suffix is used as login name. This in general
already works with SSSD but is disabled by default for IPA because of
the missing server-side support so far. Since SSSD must be able to work
with old and new IPA server https://fedorahosted.org/sssd/ticket/3018
was created so that SSSD can detect at runtime if the server supports
this or not.

Right, I think we should make sure SSSD is tested against IPA UPN
support because otherwise we might get regressions.


--
/ 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] [WIP] Thin client

2016-05-27 Thread Petr Spacek
On 27.5.2016 09:26, Martin Basti wrote:
> 
> 
> On 27.05.2016 07:49, Jan Cholasta wrote:
>> On 26.5.2016 18:43, Martin Basti wrote:
>>>
>>>
>>> On 26.05.2016 11:21, Martin Basti wrote:


 On 26.05.2016 07:15, Jan Cholasta wrote:
> On 25.5.2016 18:17, Martin Basti wrote:
>>
>>
>> On 25.05.2016 16:07, Jan Cholasta wrote:
>>> On 25.5.2016 15:03, David Kupka wrote:
 On 18/05/16 08:01, Jan Cholasta wrote:
> On 16.5.2016 16:35, Martin Basti wrote:
>>
>>
>> On 09.05.2016 14:07, Jan Cholasta wrote:
>>> On 6.5.2016 14:32, Martin Basti wrote:


 On 28.04.2016 14:45, Jan Cholasta wrote:
> Hi,
>
> I have pushed my thin client WIP branch to GitHub:
> .
>
> All commits up to "ipalib: use relative imports for cross-plugin
> imports" should be good for review. The rest is subject to
> change
> (WARNING: I will force push into this branch).
>
> Honza
>
 I did not test it yet, I just checked the code

 * automount: do not inherit automountlocation_tofiles from
 LDAPQuery *
 LGTM

 * dns: move all dnsrecord code called on client to a single
 class *
 LGTM

 * dns: do not rely on server data structures in code called on
 client *
 1)
 you forgot to increment VERSION
>>>
>>> This was deliberate, as it will no longer be necessary to bump
>>> VERSION
>>> for backward compatible changes after this whole patchset is
>>> merged.
>>> But we're not there yet, so fixed.
>>>
>> How we should handle VERSION after your patches?

 Otherwise LGTM

 * otptoken: fix import of DN *
 ACK

 * otptoken_yubikey: fix otptoken_add_yubikey arguments *
 1)
 you forgot to increment VERSION
>>>
>>> Fixed.
>>>

 2)
 Did you find out why this was issue?
 -del answer['value']# Why does this cause an error if
 omitted?
  -del answer['summary']  # Why does this cause an
 error if
 omitted?
>>>
>>> The command definition was not complete, it was missing
>>> has_output.
>>>

 Otherwise LGTM

 * vault: move client-side code to the module level *
 LGTM

 * vault: copy arguments of client commands from server
 counterparts *
 1)
 you forgot to increment Version
>>>
>>> Fixed.
>>>

 * ipalib: use relative imports for cross-plugin imports *
 1) Missing explanation for future generations why this change is
 needed
 in commit message
>>>
>>> Fixed.
>>>

 The other commits I will check later.
 Martin^2

>>>
>>> Okay. Thanks.
>>>
>>
>> * frontend: remove the unused Command.soft_validate method
>> LGTM
>>
>> * frontend: perform argument value validation only on server
>> LGTM
>>
>> * frontend: do not forward argument defaults to server
>> I'm not a fan of returning  None in promt_param function, but I
>> havent
>> found anything better to use.
>
> It always returned None for unset params.
>
>> LGTM
>>
>> * ipalib: optimize API.txt
>> this contains a lot of black magic, but because this is mainly
>> copy of
>> current to proper places, LGTM
>
> It's actually mostly cut-n-paste.
>
>>
>> * makeaci: load additional plugins using API.add_module
>> Looks good, but I miss explanation why is this change needed
>
> The next change would be impossible without this.
>
>>
>> * plugable: replace API.import_plugins with new API.add_package
>> LGTM
>>
>>
>> * ipalib, ipaserver: migrate all plugins to Registry-based
>> registration
>> ACK
>>
>> * ipalib, ipaserver: fix incorrect API.register calls in docstrings
>> LGTM
>>
>>
>> * plugable: remove the unused deprecated API.register method
>> LGTM
>>
>>
>> * plugable: switch API to Registry-based plugin discovery
>> LGTM
>>
>> * frontend: mer

[Freeipa-devel] [PATCH] bind-dyndb-ldap: hashsize() return type changed in libdns v164

2016-05-27 Thread Tomas Hozza
Hello.

Since bind version 9.10.4b1 (libdns version 164), the return type of
hashsize() has changed from unsigned int to size_t. Without this change
the plugin does not compile against bind 9.10.4b1 or newer on 64bit
architecture.

I tested building of the package on Fedora 24 and 25 and also RHEL-7.3.

Regards,
-- 
Tomas Hozza
Senior Software Engineer - EMEA ENG Developer Experience

PGP: 1D9F3C2D
UTC+1 (CET)
Red Hat Inc. http://cz.redhat.com
>From 91b4fdefc5836c259b783e56d77ff3e27ad62236 Mon Sep 17 00:00:00 2001
From: Tomas Hozza 
Date: Fri, 27 May 2016 10:21:15 +0200
Subject: [PATCH] hashsize() return type changed in libdns v164

Since bind version 9.10.4b1 (libdns version 164), the return type of
hashsize() has changed from unsigned int to size_t. Without this change
the plugin does not compile against bind 9.10.4b1 or newer on 64bit
architecture.

Signed-off-by: Tomas Hozza 
---
 src/ldap_driver.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/src/ldap_driver.c b/src/ldap_driver.c
index 5727641..83ec00a 100644
--- a/src/ldap_driver.c
+++ b/src/ldap_driver.c
@@ -871,7 +871,11 @@ setcachestats(dns_db_t *db, isc_stats_t *stats)
 	return dns_db_setcachestats(ldapdb->rbtdb, stats);
 }
 
+#if LIBDNS_VERSION_MAJOR >= 164
+size_t
+#else
 unsigned int
+#endif /* LIBDNS_VERSION_MAJOR >= 164 */
 hashsize(dns_db_t *db)
 {
 	ldapdb_t *ldapdb = (ldapdb_t *) db;
-- 
2.5.5

-- 
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] bind-dyndb-ldap: hashsize() return type changed in libdns v164

2016-05-27 Thread Petr Spacek
On 27.5.2016 10:41, Tomas Hozza wrote:
> Hello.
> 
> Since bind version 9.10.4b1 (libdns version 164), the return type of
> hashsize() has changed from unsigned int to size_t. Without this change
> the plugin does not compile against bind 9.10.4b1 or newer on 64bit
> architecture.
> 
> I tested building of the package on Fedora 24 and 25 and also RHEL-7.3.
> 
> Regards,
> 
Thanks!

ACK, pushed to master: a6853a73a52fa2f4f329ef52fe05b108a5b4e8e4

-- 
Petr^2 Spacek

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


[Freeipa-devel] [PATCH] 0002 Add the culprit line when a configuration file has an incorrect format

2016-05-27 Thread Florence Blanc-Renaud

Hi all,

this patch adds information to the output of ipa-client-install when it 
fails due to invalid format in a configuration file:
ipa-client-install failing with SyntaxError: Syntax Error: Unknown line 
format


Fixes: https://fedorahosted.org/freeipa/ticket/5811

--
Florence Blanc-Renaud
Identity Management Team, Red Hat

From 7b2f957f057d2397c91bee96af6dad577032b89d Mon Sep 17 00:00:00 2001
From: Florence Blanc-Renaud 
Date: Mon, 23 May 2016 17:18:15 +0200
Subject: [PATCH] Add the culprit line when a configuration file has an
 incorrect format

For instance if /etc/nsswitch.conf contains an incorrect line
sudoers		file sss
(Note the missing : after sudoers)
ipa-client-install exits with a SyntaxError traceback but does not state
which line caused the issue.
With the fix, the filename and the line are displayed in the SyntaxError message.

https://fedorahosted.org/freeipa/ticket/5811
---
 ipaclient/ipachangeconf.py | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/ipaclient/ipachangeconf.py b/ipaclient/ipachangeconf.py
index e73f2978cf512fdfe055f349dba80f4d3275c433..f8d2a5fda896f61c47713142ec547bf5e6e7d0ec 100644
--- a/ipaclient/ipachangeconf.py
+++ b/ipaclient/ipachangeconf.py
@@ -460,7 +460,11 @@ class IPAChangeConf:
 continue
 
 # Copy anything else as is.
-curopts.append(self.parseLine(line))
+try:
+curopts.append(self.parseLine(line))
+except SyntaxError:
+raise SyntaxError('SyntaxError: Unknown line format in file '
+  '%s: [%s]' % (f.name, line))
 
 #Add last section if any
 if len(sectopts) is not 0:
-- 
2.5.5

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

[Freeipa-devel] [PATCH 0034] Added some attributes to the Modify Users permission

2016-05-27 Thread Stanislav Laznicka

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

From f10e9b782bd92b86eb05ebd947d9799093a14091 Mon Sep 17 00:00:00 2001
From: Stanislav Laznicka 
Date: Fri, 27 May 2016 13:27:03 +0200
Subject: [PATCH] Added some attributes to Modify Users permission

Added 'employeenumber', 'departmentnumber' and 'mail' to Modify Users
permission

https://fedorahosted.org/freeipa/ticket/5911#comment:2
---
 ACI.txt| 2 +-
 ipalib/plugins/user.py | 7 ---
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/ACI.txt b/ACI.txt
index ae00cf7a1b8e2ea0e33798993bb24dc5f06127e3..01234e434a7dc2f2abe731c7d9cc5bc58030908b 100644
--- a/ACI.txt
+++ b/ACI.txt
@@ -313,7 +313,7 @@ aci: (targetattr = "usercertificate")(targetfilter = "(objectclass=posixaccount)
 dn: cn=users,cn=accounts,dc=ipa,dc=example
 aci: (targetattr = "ipasshpubkey")(targetfilter = "(objectclass=posixaccount)")(version 3.0;acl "permission:System: Manage User SSH Public Keys";allow (write) groupdn = "ldap:///cn=System: Manage User SSH Public Keys,cn=permissions,cn=pbac,dc=ipa,dc=example";)
 dn: cn=users,cn=accounts,dc=ipa,dc=example
-aci: (targetattr = "businesscategory || carlicense || cn || description || displayname || employeetype || facsimiletelephonenumber || gecos || givenname || homephone || inetuserhttpurl || initials || l || labeleduri || loginshell || manager || mepmanagedentry || mobile || objectclass || ou || pager || postalcode || preferredlanguage || roomnumber || secretary || seealso || sn || st || street || telephonenumber || title || userclass")(targetfilter = "(objectclass=posixaccount)")(version 3.0;acl "permission:System: Modify Users";allow (write) groupdn = "ldap:///cn=System: Modify Users,cn=permissions,cn=pbac,dc=ipa,dc=example";)
+aci: (targetattr = "businesscategory || carlicense || cn || departmentnumber || description || displayname || employeenumber || employeetype || facsimiletelephonenumber || gecos || givenname || homephone || inetuserhttpurl || initials || l || labeleduri || loginshell || mail || manager || mepmanagedentry || mobile || objectclass || ou || pager || postalcode || preferredlanguage || roomnumber || secretary || seealso || sn || st || street || telephonenumber || title || userclass")(targetfilter = "(objectclass=posixaccount)")(version 3.0;acl "permission:System: Modify Users";allow (write) groupdn = "ldap:///cn=System: Modify Users,cn=permissions,cn=pbac,dc=ipa,dc=example";)
 dn: cn=UPG Definition,cn=Definitions,cn=Managed Entries,cn=etc,dc=ipa,dc=example
 aci: (targetattr = "*")(target = "ldap:///cn=UPG Definition,cn=Definitions,cn=Managed Entries,cn=etc,dc=ipa,dc=example")(version 3.0;acl "permission:System: Read UPG Definition";allow (compare,read,search) groupdn = "ldap:///cn=System: Read UPG Definition,cn=permissions,cn=pbac,dc=ipa,dc=example";)
 dn: cn=users,cn=accounts,dc=ipa,dc=example
diff --git a/ipalib/plugins/user.py b/ipalib/plugins/user.py
index f92d26b2e8e5096f68c5c7fe576a120b93aad7b5..d71305d082e1d1f0c7463d12b15569259f2e2edd 100644
--- a/ipalib/plugins/user.py
+++ b/ipalib/plugins/user.py
@@ -294,10 +294,11 @@ class user(baseuser):
 'System: Modify Users': {
 'ipapermright': {'write'},
 'ipapermdefaultattr': {
-'businesscategory', 'carlicense', 'cn', 'description',
-'displayname', 'employeetype', 'facsimiletelephonenumber',
+'businesscategory', 'carlicense', 'cn', 'departmentnumber',
+'description', 'displayname', 'employeetype',
+'employeenumber', 'facsimiletelephonenumber',
 'gecos', 'givenname', 'homephone', 'inetuserhttpurl',
-'initials', 'l', 'labeleduri', 'loginshell', 'manager',
+'initials', 'l', 'labeleduri', 'loginshell', 'manager', 'mail',
 'mepmanagedentry', 'mobile', 'objectclass', 'ou', 'pager',
 'postalcode', 'roomnumber', 'secretary', 'seealso', 'sn', 'st',
 'street', 'telephonenumber', 'title', 'userclass',
-- 
2.5.5

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

[Freeipa-devel] [PATCH 35] Added pyusb dependency

2016-05-27 Thread Stanislav Laznicka

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

From 38eda0f2a08b6bff65217d6c4517fffc1e1b0f86 Mon Sep 17 00:00:00 2001
From: Stanislav Laznicka 
Date: Fri, 27 May 2016 13:45:57 +0200
Subject: [PATCH] Added pyusb as a dependency

https://fedorahosted.org/freeipa/ticket/5886
---
 freeipa.spec.in | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/freeipa.spec.in b/freeipa.spec.in
index 94bb1abee5782cc9f00fd83b916294f34deaccd6..c6499a5c6073ac4c2814d7ad14964927b88b1981 100644
--- a/freeipa.spec.in
+++ b/freeipa.spec.in
@@ -505,6 +505,7 @@ Requires: python-requests
 Requires: python-custodia
 Requires: python-dns >= 1.11.1
 Requires: python-netifaces >= 0.10.4
+Requires: pyusb
 
 Conflicts: %{alt_name}-python < %{version}
 
@@ -553,6 +554,7 @@ Requires: python3-custodia
 Requires: python3-requests
 Requires: python3-dns >= 1.11.1
 Requires: python3-netifaces >= 0.10.4
+Requires: python3-pyusb
 
 %description -n python3-ipalib
 IPA is an integrated solution to provide centrally managed Identity (users,
-- 
2.5.5

-- 
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 0110] DNS: Warn if forwarding policy conflicts with automatic empty zone

2016-05-27 Thread Petr Spacek
On 25.5.2016 12:30, Martin Basti wrote:
> 
> 
> On 04.05.2016 10:43, Petr Spacek wrote:
>> Hello,
>>
>> DNS: Warn if forwarding policy conflicts with automatic empty zones
>>
>> Forwarding policy "first" or "none" may conflicts with some automatic empty
>> zones. Queries for zones specified by RFC 6303 will ignore
>> forwarding and recursion and always result in NXDOMAIN answers.
>>
>> This is not detected and warned about. Global forwarding is equivalent
>> to forward zone ".".
>>
>> Example:
>> Forward zone 1.10.in-addr.arpa with policy "first"
>> will not forward anything because BIND will automatically prefer
>> automatic empty zone "10.in-addr.arpa." which is authoritative.
>>
>> https://fedorahosted.org/freeipa/ticket/5710
>>
>>
>> This is last patch in the series so the ticket can be closed when all 
>> relevant
>> patches are pushed.
>>
>>
>>
> 
> 
> You forgot to update tests
> 
> _
> test_dns.test_command[0087: dnsconfig_mod: Update global DNS settings]
> __
> 
> self =  0x7fcef3ef2510>, index = 87
> declarative_test_definition = {'command': ('dnsconfig_mod', [],
> {'idnsforwarders': ['172.16.31.80'], 'version': '2.166'}), 'desc': 'Update
> global DN...arders': ['172.16.31.80']}, 'summary': None, 'value': None},
> 'nice': '0087: dnsconfig_mod: Update global DNS settings'}
> 
> def test_command(self, index, declarative_test_definition):
> """Run an individual test
> 
> The arguments are provided by the pytest plugin.
> """
> if callable(declarative_test_definition):
> declarative_test_definition(self)
> else:
>>   self.check(**declarative_test_definition)
> 
> test_xmlrpc/xmlrpc_test.py:313:
> _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
> _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
> _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
> test_xmlrpc/xmlrpc_test.py:325: in check
> self.check_output(nice, cmd, args, options, expected, extra_check)
> test_xmlrpc/xmlrpc_test.py:368: in check_output
> assert_deepequal(expected, got, nice)
> util.py:361: in assert_deepequal
> assert_deepequal(e_sub, g_sub, doc, stack + (key,))
> _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
> _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
> _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
> 
> expected = [{'code': 13006, 'message':  at 0x7fcef426c758>,
> 'name': 'DNSServerValidationWarning', 'type': 'warning'}]
> got = [{'code': 13021, 'message': "Forwarding policy conflicts with some
> automatic empty zones. Queries for zones specified ...': The DNS operation
> timed out after 10.0008428097 seconds.", 'name': 'DNSServerValidationWarning',
> 'type': 'warning'}]
> doc = '0087: dnsconfig_mod: Update global DNS settings', stack = ('messages',)
> 
> def assert_deepequal(expected, got, doc='', stack=tuple()):
> """
> Recursively check for type and equality.
> 
> If a value in expected is callable then it will used as a callback to
> test for equality on the got value. The callback is passed the got
> value and returns True if equal, False otherwise.
> 
> If the tests fails, it will raise an ``AssertionError`` with detailed
> information, including the path to the offending value.  For example:
> 
> >>> expected = [u'Hello', dict(world=u'how are you?')]
> >>> got = [u'Hello', dict(world='how are you?')]
> >>> expected == got
> True
> >>> assert_deepequal(expected, got, doc='Testing my nested data')
> Traceback (most recent call last):
>   ...
> AssertionError: assert_deepequal: type(expected) is not type(got).
>   Testing my nested data
>   type(expected) = 
>   type(got) = 
>   expected = u'how are you?'
>   got = 'how are you?'
>   path = (0, 'world')
> 
> Note that lists and tuples are considered equivalent, and the order of
> their elements does not matter.
> """
> if isinstance(expected, tuple):
> expected = list(expected)
> if isinstance(got, tuple):
> got = list(got)
> if isinstance(expected, DN):
> if isinstance(got, six.string_types):
> got = DN(got)
> if not (isinstance(expected, Fuzzy) or callable(expected) or
> type(expected) is type(got)):
> raise AssertionError(
> TYPE % (doc, type(expected), type(got), expected, got, stack)
> )
> if isinstance(expected, (list, tuple)):
> if len(expected) != len(got):
> raise AssertionError(
>>   LEN % (doc, len(expected), len(got), expected, got, stack)
>

Re: [Freeipa-devel] [PATCH 0123-132] DNS upgrade: change forwarding policy to "only" if private IPs are used

2016-05-27 Thread Petr Spacek
On 25.5.2016 12:50, Martin Basti wrote:
> 
> 
> On 20.05.2016 12:19, Petr Spacek wrote:
>> On 11.5.2016 12:08, Martin Basti wrote:
>>>
>>> On 03.05.2016 14:59, Petr Spacek wrote:
 Hello,

 DNS upgrade: change forwarding policy to "only" if private IPs are used.

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

 This is the upgrade part. I will add one more patch to print a warning in
 dnsforwardzone* commands to avoid surprises. Please do not close the ticket
 yet.



>>> 1)
>>> Upgrade failed with 'BindInstance' object has no attribute
>>> 'named_conf_get_directive'
>>> IPA server upgrade failed: Inspect /var/log/ipaupgrade.log and run command
>>> ipa-server-upgrade manually.
>>> ('IPA upgrade failed.', 1)
>>> The ipa-server-upgrade command failed. See /var/log/ipaupgrade.log for more
>>> information
>>>
>>> 2016-05-11T08:26:20Z ERROR Upgrade failed with 'BindInstance' object has no
>>> attribute 'named_conf_get_directive'
>>> 2016-05-11T08:26:20Z DEBUG Traceback (most recent call last):
>>>File
>>> "/usr/lib/python2.7/site-packages/ipaserver/install/upgradeinstance.py", 
>>> line
>>> 213, in __upgrade
>>>  self.modified = (ld.update(self.files) or self.modified)
>>>File "/usr/lib/python2.7/site-packages/ipaserver/install/ldapupdate.py",
>>> line 917, in update
>>>  self._run_updates(all_updates)
>>>File "/usr/lib/python2.7/site-packages/ipaserver/install/ldapupdate.py",
>>> line 889, in _run_updates
>>>  self._run_update_plugin(update['plugin'])
>>>File "/usr/lib/python2.7/site-packages/ipaserver/install/ldapupdate.py",
>>> line 862, in _run_update_plugin
>>>  restart_ds, updates = self.api.Updater[plugin_name]()
>>>File "/usr/lib/python2.7/site-packages/ipalib/frontend.py", line 1418, in
>>> __call__
>>>  return self.execute(**options)
>>>File "/usr/lib/python2.7/site-packages/ipaserver/install/plugins/dns.py",
>>> line 547, in execute
>>>  self.update_global_named_conf_forwarder(bind)
>>>File "/usr/lib/python2.7/site-packages/ipaserver/install/plugins/dns.py",
>>> line 508, in update_global_named_conf_forwarder
>>>  if bind.named_conf_get_directive(
>>> AttributeError: 'BindInstance' object has no attribute
>>> 'named_conf_get_directive'
>>>
>>> 2016-05-11T08:26:20Z DEBUG Traceback (most recent call last):
>>>File "/usr/lib/python2.7/site-packages/ipaserver/install/service.py", 
>>> line
>>> 447, in start_creation
>>>  run_step(full_msg, method)
>>>File "/usr/lib/python2.7/site-packages/ipaserver/install/service.py", 
>>> line
>>> 437, in run_step
>>>  method()
>>>File
>>> "/usr/lib/python2.7/site-packages/ipaserver/install/upgradeinstance.py", 
>>> line
>>> 221, in __upgrade
>>>  raise RuntimeError(e)
>>> RuntimeError: 'BindInstance' object has no attribute
>>> 'named_conf_get_directive'
>>>
>>> PATCH * Add ipaDNSVersion option to dnsconfig* commands and use new
>>> attribute *
>>> 2)
>>> +Int('ipadnsversion?',
>>> +label=_('IPA DNS version'),
>>> +),
>>>
>>> Shouldn't be this part of System: Read DNS Configuration permission?
>>>
>>> 3)
>>> -def postprocess_result(self, result):
>>> +def postprocess_result(self, result, show_version):
>>>   if not any(param in result['result'] for param in self.params):
>>>   result['summary'] = unicode(_('Global DNS configuration is
>>> empty'))
>>>
>>> show_version param was added but I don't see it used in this patch.
>>>
>>> 4)
>>> +Int('ipadnsversion?',
>>> +label=_('IPA DNS version'),
>>> +),
>>>
>>> Could we add comment here that this option is accessible only from 
>>> installers
>>> and upgrade?
>>>
>>> 5)
>>> +for config_option in container_entry.get("ipaConfigString", []):
>>> +matched = re.match("^DNSVersion\s+(?P\d+)$",
>>> +   config_option, flags=re.I)
>>> +if matched:
>>> +version = int(matched.group("version"))
>>>
>>> Shouldn't we print error if version cannot be parsed?
>>>
>>> PATCH  * DNS upgrade: separate backup logic to make it reusable *
>>>
>>> LGTM
>>>
>>> PATCH * Add function ipapython.dnsutil.related_to_auto_empty_zone() *
>>>
>>> 7)
>>> I'm curious why do you need to check superdomains?
>>>
>>> PATCH * DNS upgrade: change forwarding policy to = only for conflicting
>>> forward zones*
>>>
>>> 8)
>>> +self.log.debug('Zone %s was sucessfully modified to use '
>>> +   'forward policy "only"', zone['idnsname'][0])
>>> <---missing empty line>
>>> +def execute(self, **options):
>>>
>>> PATCH * DNS upgrade: change global forwarding policy in LDAP to "only" if
>>> private IPs are used *
>>> 9)
>>> - dnsutil.related_to_auto_empty_zone(zone.get('idnsname')[0])
>>> +dnsutil.related_to_auto_empty_zone(
>>> +dnsutil.DNSName(zone.get('idnsname')[0]))
>>>
>>> Should be in previous commit
>>>
>>>

[Freeipa-devel] [PATCH 0036] Increased mod_wsgi socket-timeout

2016-05-27 Thread Stanislav Laznicka

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

From e69514ade7bae97bb2bb0e541c080c727ff7056c Mon Sep 17 00:00:00 2001
From: Stanislav Laznicka 
Date: Fri, 27 May 2016 14:44:30 +0200
Subject: [PATCH] Increased mod_wsgi socket-timeout

Longer-running CLI commands sometimes fail with "gateway time out" although
the task still runs and finishes on server, not notifying the CLI back.
Increasing socket-timeout should solve this.

https://fedorahosted.org/freeipa/ticket/5833
---
 install/conf/ipa.conf | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/install/conf/ipa.conf b/install/conf/ipa.conf
index cf10fc815640bcfc56152d342ee70d7d363ba4e5..7686999406bc2be57c121006931289c17bee67ad 100644
--- a/install/conf/ipa.conf
+++ b/install/conf/ipa.conf
@@ -41,7 +41,8 @@ WSGISocketPrefix /run/httpd/wsgi
 
 
 # Configure mod_wsgi handler for /ipa
-WSGIDaemonProcess ipa processes=2 threads=1 maximum-requests=500 display-name=%{GROUP}
+WSGIDaemonProcess ipa processes=2 threads=1 maximum-requests=500 \
+ display-name=%{GROUP} socket-timeout=300
 WSGIImportScript /usr/share/ipa/wsgi.py process-group=ipa application-group=ipa
 WSGIScriptAlias /ipa /usr/share/ipa/wsgi.py
 WSGIScriptReloading Off
-- 
2.5.5

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

[Freeipa-devel] [PATCH] 0003 batch command can be used to trigger internal errors on server

2016-05-27 Thread Florence Blanc-Renaud

Hi all,

the following patch checks the format of parameters passed to a method 
called through the batch command. I picked the ConversionError for 
invalid parameters format but this choice can be discussed if you have 
better suggestions...


Fixes: https://fedorahosted.org/freeipa/ticket/5810

--
Florence Blanc-Renaud
Identity Management Team, Red Hat

From 3695130deb27584a3f154505d905f4cec91913c8 Mon Sep 17 00:00:00 2001
From: Florence Blanc-Renaud 
Date: Fri, 27 May 2016 08:19:39 +0200
Subject: [PATCH] batch command can be used to trigger internal errors on
 server

In ipalib, the batch command expects the following format for arguments:
api.Command['batch']([
{'method':'method1', 'params': ([],{})}
{'method':'method2', 'params': ([],{})}
])

ie a list containing dict, each dict corresponding to a method to call.
The parameters passed to the methods must be stored inside a tuple (list, dict)
with the list containing the positional arguments and the dict the
keyword arguments.

The code did not check the format of the parameters, which could trigger
internal errors on the server.
With this fix:
- a ConversionError is raised if the arg passed to batch() is not a list of dict
- the result appended to the batch results is a ConversionError if the 'params'
does not contain a tuple(list,dict)

https://fedorahosted.org/freeipa/ticket/5810
---
 ipalib/plugins/batch.py | 26 +++---
 1 file changed, 23 insertions(+), 3 deletions(-)

diff --git a/ipalib/plugins/batch.py b/ipalib/plugins/batch.py
index 84a65057588e01e18a937c6e79f0974b25c529ed..41e98821b1fa715e4619099539b42ba8b5ff8d7b 100644
--- a/ipalib/plugins/batch.py
+++ b/ipalib/plugins/batch.py
@@ -90,6 +90,13 @@ class batch(Command):
 def execute(self, methods=None, **options):
 results = []
 for arg in (methods or []):
+# As take_args = Any, no check is done before
+# Need to make sure that methods contain dict objects
+# https://fedorahosted.org/freeipa/ticket/5810
+if not isinstance(arg, dict):
+raise errors.ConversionError(
+name='methods',
+error=_(u'must contain dict objects'))
 params = dict()
 name = None
 try:
@@ -100,9 +107,22 @@ class batch(Command):
 name = arg['method']
 if name not in self.Command:
 raise errors.CommandError(name=name)
-a, kw = arg['params']
-newkw = dict((str(k), v) for k, v in kw.items())
-params = api.Command[name].args_options_2_params(*a, **newkw)
+
+# If params are not formated as a tuple(list, dict)
+# the following lines will raise an exception
+# that triggers an internal server error
+# Raise a ConversionError instead to report the issue
+# to the client
+# https://fedorahosted.org/freeipa/ticket/5810
+try:
+a, kw = arg['params']
+newkw = dict((str(k), v) for k, v in kw.items())
+params = api.Command[name].args_options_2_params(
+*a, **newkw)
+except (AttributeError, ValueError, TypeError):
+raise errors.ConversionError(
+name='params',
+error=_(u'must contain a tuple (list, dict)'))
 newkw.setdefault('version', options['version'])
 
 result = api.Command[name](*a, **newkw)
-- 
2.5.5

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

[Freeipa-devel] [PATCH 0037] Added /etc/krb5.conf.d/ to krb5.conf

2016-05-27 Thread Stanislav Laznicka

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

From 7a55f169181ab8647cd2d919f35c004b14d5bc7f Mon Sep 17 00:00:00 2001
From: Stanislav Laznicka 
Date: Fri, 27 May 2016 16:12:31 +0200
Subject: [PATCH] Added krb5.conf.d/ to included dirs in krb5.conf

The include of /etc/krb5.conf.d/ is required for crypto-policies to work properly

https://fedorahosted.org/freeipa/ticket/5912
---
 client/ipa-client-install| 3 ++-
 install/share/krb5.conf.template | 1 +
 ipaplatform/base/paths.py| 1 +
 3 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/client/ipa-client-install b/client/ipa-client-install
index cff3fbfcdee8690c9466ea339a362edfb151a11a..ddefdbc385b5ac4619debf96610e8a7cdb18fc2e 100755
--- a/client/ipa-client-install
+++ b/client/ipa-client-install
@@ -1058,7 +1058,8 @@ def configure_krb5_conf(cli_realm, cli_domain, cli_server, cli_kdc, dnsok,
 krbconf.setIndent(("","  ",""))
 
 opts = [{'name':'comment', 'type':'comment', 'value':'File modified by ipa-client-install'},
-{'name':'empty', 'type':'empty'}]
+{'name':'empty', 'type':'empty'},
+{'name':'includedir', 'type':'option', 'value':paths.COMMON_KRB5_CONF_DIR, 'delim':' '}]
 
 # SSSD include dir
 if options.sssd:
diff --git a/install/share/krb5.conf.template b/install/share/krb5.conf.template
index 92431d3fde6afecd0e74803e18724379e8746f9b..f8b256aee690def6c415004df948a34d485578b1 100644
--- a/install/share/krb5.conf.template
+++ b/install/share/krb5.conf.template
@@ -1,3 +1,4 @@
+includedir /etc/krb5.conf.d/
 includedir /var/lib/sss/pubconf/krb5.include.d/
 
 [logging]
diff --git a/ipaplatform/base/paths.py b/ipaplatform/base/paths.py
index ca7eb6cf47b4442fa538a47c74846e13c25e02e8..336839b71e446bfc459d3bd5065b4c029b312832 100644
--- a/ipaplatform/base/paths.py
+++ b/ipaplatform/base/paths.py
@@ -68,6 +68,7 @@ class BasePathNamespace(object):
 DNSSEC_SOFTHSM_PIN_SO = "/etc/ipa/dnssec/softhsm_pin_so"
 IPA_NSSDB_DIR = "/etc/ipa/nssdb"
 IPA_NSSDB_PWDFILE_TXT = "/etc/ipa/nssdb/pwdfile.txt"
+COMMON_KRB5_CONF_DIR = "/etc/krb5.conf.d/"
 KRB5_CONF = "/etc/krb5.conf"
 KRB5_KEYTAB = "/etc/krb5.keytab"
 LDAP_CONF = "/etc/ldap.conf"
-- 
2.5.5

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

[Freeipa-devel] [PATCH 0093] Enable service authentication indicator management

2016-05-27 Thread Nathaniel McCallum
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.

How do I code this so that we can later do a StrEnum => Str transition
without breaking API?From e5507c8c49cb50be247f23627bf58b6953d7b8a9 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   |  3 +--
 ipalib/plugins/service.py | 10 +-
 3 files changed, 16 insertions(+), 6 deletions(-)

diff --git a/API.txt b/API.txt
index dbc6f1adc614607fab106ab0de7163961e7ecedc..3a938bad0f7e19744e97ded92490818c1b8beb73 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: StrEnum('krbprincipalauthind*', cli_name='auth_ind', values=[u'otp', u'radius'])
 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: StrEnum('krbprincipalauthind*', autofill=False, cli_name='auth_ind', values=[u'otp', u'radius'])
 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: StrEnum('krbprincipalauthind*', autofill=False, cli_name='auth_ind', values=[u'otp', u'radius'])
 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..fb720c6a729399cf90b2a072a344f313cf42bbfd 100644
--- a/VERSION
+++ b/VERSION
@@ -90,5 +90,4 @@ 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
diff --git a/ipalib/plugins/service.py b/ipalib/plugins/service.py
index 4f03bd35f80805c7f821cac8a3f0e85c547a6219..6a1b73eec3d1983283cc84842e7797d4af9ebbc6 100644
--- a/ipalib/plugins/service.py
+++ b/ipalib/plugins/service.py
@@ -411,7 +411,7 @@ class service(LDAPObject):
 permission_filter_objectclasses = ['ipaservice']
 search_attributes = ['krbprincipalname', 'managedby', 'ipakrbauthzdata']
 default_attributes = ['krbprincipalname', 'usercertificate', 'managedby',
-'ipakrbauthzdata', 'memberof', 'ipaallowedtoperform']
+'ipakrbauthzdata', 'memberof', 'ipaallowedtoperform', 'krbprincipalauthind']
 uuid_attribute = 'ipauniqueid'
 attribute_members = {
 'managedby': ['host'],
@@ -505,6 +505,14 @@ class service(LDAPObject):
   " e.g. this might be necessary for NFS services."

Re: [Freeipa-devel] [PATCH 0146-0147] Server Roles: basic infrastructure

2016-05-27 Thread Martin Babinsky

On 05/19/2016 04:59 PM, Martin Babinsky wrote:

Patch 0146 implements lower-lever infrastructure for querying server
roles/attributes

Patch 0147 are some basic tests slapped together for the `serverroles`
backend to ensure that it works as expected.

The new/modified CLI commands specified in the design page [1] will be
coming soon.

Also coming soon will be an interface to construct DNS records for each
role requested by Petr^2 and Martin^2 which I will start implement when
we agree on the backend implementation.

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

[1] https://www.freeipa.org/page/V4/Server_Roles#CLI




Thanks Martin and Honza for review.

I have reworked the patches based on your comments. I have split patch 
146 into two (role/attribute definitions and backend code) so patches 
146-147 are for code and 148 hosts test suite.


I hope that I will send API patches on monday after I resolve some 
framework-related questions with the local guru.


--
Martin^3 Babinsky
From 6994428e527760fdd6d5cb442af0a1f321a4e542 Mon Sep 17 00:00:00 2001
From: Martin Babinsky 
Date: Thu, 26 May 2016 19:24:22 +0200
Subject: [PATCH 1/3] Server roles: definitions of server roles and attributes

This patch introduces classes which define the properties of server roles and
attributes and their relationship to LDAP attributes representing the
role/attribute.

A brief documentation about defining and using roles is given at the beginning
of the module.

http://www.freeipa.org/page/V4/Server_Roles
https://fedorahosted.org/freeipa/ticket/5181
---
 ipaserver/servroles.py | 574 +
 1 file changed, 574 insertions(+)
 create mode 100644 ipaserver/servroles.py

diff --git a/ipaserver/servroles.py b/ipaserver/servroles.py
new file mode 100644
index ..0d5c997c9022fa256a5488772b15514570bc0dc2
--- /dev/null
+++ b/ipaserver/servroles.py
@@ -0,0 +1,574 @@
+#
+# Copyright (C) 2016 FreeIPA Contributors see COPYING for license
+#
+
+
+"""
+This module contains the set of classes which abstract various bits and pieces
+of information present in the LDAP tree about functionalities such as DNS
+server, Active Directory trust controller etc. These properties come in two
+distinct groups:
+
+server roles
+this group represents a genral functionality provided by one or more
+IPA servers, such as DNS server, certificate authority and such. In
+this case there is a many-to-many mapping between the roles and the
+masters which provide them.
+
+server attributes
+these represent a functionality associated with the whole topology,
+such as CA renewal master or DNSSec key master.
+
+See the corresponding design page (http://www.freeipa.org/page/V4/Server_Roles)
+for more info.
+
+Both of these groups use `LDAPBasedProperty` class as a base.
+
+Server Roles
+
+
+Server role objects are usually consuming information from the master's service
+container (cn=FQDN,cn=masters,cn=ipa,cn=etc,$SUFFIX) are represented by
+`ServiceBasedRole`class. To create an instance of such role, you only need to
+specify role name and individual services comprising the role (more systemd
+services may be enabled to provide some function):
+
+>>> example_role = ServiceBasedRole(
+... "Example Role",
+... component_services = ['SERVICE1', 'SERVICE2'])
+>>> example_role.name
+'Example Role'
+
+Each role object has an `attr_name` property which returns its name transformed
+into an LDAP-like attribute name useful in higher-level commands implemented in
+API framework
+
+>>> example_role.attr_name
+'ipaexamplerole'
+
+The role object can then be queried for the status of the role in the whole
+topology or on a single master by using its `status` method. This method
+returns a list of dictionaries akin to LDAP entries comprised from server name,
+role name and role status (enabled if role is enabled, configured if the
+service entries are present but not marked as enabled by 'enabledService'
+config string, absent if the service entries are not present).
+
+Note that 'AD trust agent' role is based on membership of the master in the
+'adtrust agents' sysaccount group and is thus an instance of different class
+(`ADTrustBasedRole`). This role also does not have 'configured' status, since
+the master is either member of the group ('enabled') or not ('absent')
+
+Server Attributes
+=
+
+Server attributes are implemented as instances of `ServerAttribute` class. The
+attribute is defined by some flag set ornf 'ipaConfigString' attribute of some
+service entry. To create your own server attribute, see the following example:
+
+>>> example_attribute = ServerAttribute("Example Attribute", example_role,
+... "SERVICE1", "roleMaster")
+>>> example_attribute.name
+'Example Attribute'
+
+As in the case of serverroles, each instance has the `attr_name` property
+giving LDAP-lik

Re: [Freeipa-devel] [PATCH 0094] Migrate from #ifndef guards to #pragma once

2016-05-27 Thread Nathaniel McCallum
On Tue, 2016-05-24 at 12:25 -0400, Nathaniel McCallum wrote:
> On Tue, 2016-05-24 at 11:01 -0400, Nathaniel McCallum wrote:
> > On Tue, 2016-05-24 at 16:55 +0200, Martin Kosek wrote:
> > > On 05/24/2016 04:29 PM, Nathaniel McCallum wrote:
> > > > Using a pragma instead of guards is easier to write, less error
> > > > prone
> > > > and avoids name clashes (a source of very subtle bugs). This
> > > > pragma
> > > > is supported on almost all compilers, including all the
> > > > compilers
> > > > we
> > > > care about: https://en.wikipedia.org/wiki/Pragma_once#Portabili
> > > > ty
> > > > .
> > > > 
> > > > 
> > > > 
> > > 
> > > Makes sense to me. I did not test, just saw a potential
> > > typo/omission:
> > > 
> > > --- a/daemons/ipa-otpd/internal.h
> > > +++ b/daemons/ipa-otpd/internal.h
> > > @@ -20,9 +20,6 @@
> > >   * along with this program.  If not, see  > > en
> > > se
> > > s/>.
> > >   */
> > > 
> > > -#ifndef INTERNAL_H_
> > > -#define INTERNAL_H_
> > > -
> > > 
> > > 
> > > ... no pragma there.
> > 
> > Fixed.
> 
> Here is a new version with a slightly edited commit message (just to
> confirm that we don't edit the autogenerated files).

Attached is a new version that is simply rebased on to master.From d7d93f817ca4cbc26041fce3603f2609a76d2654 Mon Sep 17 00:00:00 2001
From: Nathaniel McCallum 
Date: Tue, 24 May 2016 10:18:43 -0400
Subject: [PATCH] Migrate from #ifndef guards to #pragma once

Using a pragma instead of guards is easier to write, less error prone
and avoids name clashes (a source of very subtle bugs). This pragma
is supported on almost all compilers, including all the compilers we
care about: https://en.wikipedia.org/wiki/Pragma_once#Portability.

This patch does not change the autogenerated files: asn1/asn1c/*.h.
---
 asn1/ipa_asn1.h | 5 +
 client/ipa-client-common.h  | 5 +
 daemons/ipa-kdb/ipa_kdb_mspac_private.h | 6 +-
 daemons/ipa-otpd/internal.h | 5 +
 daemons/ipa-sam/ipa_sam.h   | 5 +
 daemons/ipa-slapi-plugins/common/util.h | 5 +
 daemons/ipa-slapi-plugins/ipa-cldap/ipa_cldap.h | 4 +---
 daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom.h | 4 +---
 daemons/ipa-slapi-plugins/ipa-pwd-extop/otpctrl.h   | 6 +-
 daemons/ipa-slapi-plugins/ipa-sidgen/ipa_sidgen.h   | 4 +---
 daemons/ipa-slapi-plugins/ipa-winsync/ipa-winsync.h | 5 +
 util/ipa_krb5.h | 4 +---
 util/ipa_mspac.h| 5 +
 util/ipa_pwd.h  | 5 +
 14 files changed, 14 insertions(+), 54 deletions(-)

diff --git a/asn1/ipa_asn1.h b/asn1/ipa_asn1.h
index 6ffcc5cc81992966654c21e923a1f8883b32833b..089a5e5202082eacab370c97420b74dc236828f7 100644
--- a/asn1/ipa_asn1.h
+++ b/asn1/ipa_asn1.h
@@ -1,5 +1,4 @@
-#ifndef __IPA_ASN1_H_
-#define __IPA_ASN1_H_
+#pragma once
 
 #include "ipa_krb5.h"
 
@@ -72,5 +71,3 @@ bool ipaasn1_dec_getkt(void *buf, size_t len, bool *newkt,
  */
 bool ipaasn1_dec_getktreply(void *buf, size_t len,
 int *kvno, struct keys_container *keys);
-
-#endif /* __IPA_ASN1_H_ */
diff --git a/client/ipa-client-common.h b/client/ipa-client-common.h
index e831c596c9d12c6205532442990fb5c89fbbfefc..d0db0637e3e9d09519fd43ef6851a6259a98142c 100644
--- a/client/ipa-client-common.h
+++ b/client/ipa-client-common.h
@@ -17,8 +17,7 @@
  * along with this program.  If not, see .
  */
 
-#ifndef __IPA_CLIENT_COMMON_H
-#define __IPA_CLIENT_COMMON_H
+#pragma once
 
 #include 
 #define _(STRING) gettext(STRING)
@@ -29,5 +28,3 @@
 #endif
 
 int init_gettext(void);
-
-#endif /* __IPA_CLIENT_COMMON_H */
diff --git a/daemons/ipa-kdb/ipa_kdb_mspac_private.h b/daemons/ipa-kdb/ipa_kdb_mspac_private.h
index be04071762316643d687e80986db0d7510e53ded..1052bb882e2a8a4dbbc1a6d3b166fca6e3a5e585 100644
--- a/daemons/ipa-kdb/ipa_kdb_mspac_private.h
+++ b/daemons/ipa-kdb/ipa_kdb_mspac_private.h
@@ -21,9 +21,7 @@
  * along with this program.  If not, see .
  */
 
-
-#ifndef _IPA_KDB_MSPAC_PRIVATE_H_
-#define _IPA_KDB_MSPAC_PRIVATE_H_
+#pragma once
 
 struct ipadb_mspac {
 char *flat_domain_name;
@@ -53,5 +51,3 @@ struct ipadb_adtrusts {
 
 int string_to_sid(const char *str, struct dom_sid *sid);
 char *dom_sid_string(TALLOC_CTX *memctx, const struct dom_sid *dom_sid);
-
-#endif /* _IPA_KDB_MSPAC_PRIVATE_H_ */
diff --git a/daemons/ipa-otpd/internal.h b/daemons/ipa-otpd/internal.h
index 5ab4a7776cafe037c55e7610c39a7be263f4e3d9..43096edf06360a4a4789f6b3dc128072c7118d81 100644
--- a/daemons/ipa-otpd/internal.h
+++ b/daemons/ipa-otpd/internal.h
@@ -20,8 +20,7 @@
  * along with this program.  If not, see .
  */
 
-#ifndef INTERNAL_H_
-#define INTERNAL_H_
+#pragma once
 
 #include "k

Re: [Freeipa-devel] [PATCH 0093] Enable service authentication indicator management

2016-05-27 Thread Alexander Bokovoy

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


Re: [Freeipa-devel] [PATCH] 0034: webui: Authentication indicators

2016-05-27 Thread Pavel Vomacka



On 05/12/2016 11:13 PM, Nathaniel McCallum wrote:

On Wed, 2016-05-11 at 13:08 +0200, Pavel Vomacka wrote:

Hi,

the patch adds webui part for authentication indicators.

Ticket: https://fedorahosted.org/freeipa/ticket/5872

The otp option displays as: OTP.
The radius option displays as: Radius.

However, both are acronyms. The capitalization should be consistent.
I'm sorry for late answer. They are displayed this way: 'OTP' and 
'Radius'. So it should not require any change.


--
Pavel^3

--
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] 0034: webui: Authentication indicators

2016-05-27 Thread Nathaniel McCallum
On Fri, 2016-05-27 at 17:43 +0200, Pavel Vomacka wrote:
> 
> On 05/12/2016 11:13 PM, Nathaniel McCallum wrote:
> > On Wed, 2016-05-11 at 13:08 +0200, Pavel Vomacka wrote:
> > > Hi,
> > > 
> > > the patch adds webui part for authentication indicators.
> > > 
> > > Ticket: https://fedorahosted.org/freeipa/ticket/5872
> > The otp option displays as: OTP.
> > The radius option displays as: Radius.
> > 
> > However, both are acronyms. The capitalization should be
> > consistent.
> I'm sorry for late answer. They are displayed this way: 'OTP' and 
> 'Radius'. So it should not require any change.

That is incorrect. It should be: OTP and RADIUS.

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

2016-05-27 Thread Nathaniel McCallum
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] 0034: webui: Authentication indicators

2016-05-27 Thread Pavel Vomacka



On 05/27/2016 05:44 PM, Nathaniel McCallum wrote:

On Fri, 2016-05-27 at 17:43 +0200, Pavel Vomacka wrote:

On 05/12/2016 11:13 PM, Nathaniel McCallum wrote:

On Wed, 2016-05-11 at 13:08 +0200, Pavel Vomacka wrote:

Hi,

the patch adds webui part for authentication indicators.

Ticket: https://fedorahosted.org/freeipa/ticket/5872

The otp option displays as: OTP.
The radius option displays as: Radius.

However, both are acronyms. The capitalization should be
consistent.

I'm sorry for late answer. They are displayed this way: 'OTP' and
'Radius'. So it should not require any change.

That is incorrect. It should be: OTP and RADIUS.

I'm sorry, I didn't understand correctly. Fixed patch attached.

From f488d3e109fc40aff616a846c0857b85b6d50f61 Mon Sep 17 00:00:00 2001
From: Pavel Vomacka 
Date: Wed, 11 May 2016 12:54:43 +0200
Subject: [PATCH] Add authentication indicators to webui

Add checkboxes for choosing authentication indicators to the service details page.

https://fedorahosted.org/freeipa/ticket/5872
---
 install/ui/src/freeipa/service.js  | 18 ++
 install/ui/test/data/ipa_init.json |  3 +++
 ipalib/plugins/internal.py |  3 +++
 3 files changed, 24 insertions(+)

diff --git a/install/ui/src/freeipa/service.js b/install/ui/src/freeipa/service.js
index f1f8d951e415e9768aab433e28da852a732bc8ba..5bae56abbd5414e8aa771d8c4c3092d67a077486 100644
--- a/install/ui/src/freeipa/service.js
+++ b/install/ui/src/freeipa/service.js
@@ -109,6 +109,24 @@ return {
 ]
 },
 {
+$type: 'checkboxes',
+label: '@i18n:objects.service.auth_indicators',
+name: 'krbprincipalauthind',
+options: [
+{
+label: '@i18n:authtype.otp',
+value: 'otp'
+},
+{
+label: '@i18n:authtype.radius',
+value: 'radius'
+}
+],
+tooltip: {
+title: '@mc-opt:service_add:krbprincipalauthind:doc'
+}
+},
+{
 name: 'ipakrbokasdelegate',
 $type: 'checkbox',
 acl_param: 'krbticketflags'
diff --git a/install/ui/test/data/ipa_init.json b/install/ui/test/data/ipa_init.json
index 1b9b69ff909a9668c1e1867008459d25d5e062a9..b058240371f1deab5517f37656f6062e2fa92b11 100644
--- a/install/ui/test/data/ipa_init.json
+++ b/install/ui/test/data/ipa_init.json
@@ -50,6 +50,8 @@
 },
 "authtype": {
 "config_tooltip": "Implicit method (password) will be used if no method is chosen.Password + Two-factor: LDAP and Kerberos allow authentication with either one of the authentication types but Kerberos uses pre-authentication method which requires to use armor ccache.RADIUS with another type: Kerberos always use RADIUS, but LDAP never does. LDAP only recognize the password and two-factor authentication options.",
+"otp": "OTP",
+"radius": "RADIUS",
 "type_otp": "Two factor authentication (password + OTP)",
 "type_password": "Password",
 "type_radius": "Radius",
@@ -512,6 +514,7 @@
 "user": "User"
 },
 "service": {
+"auth_indicators": "Authentication indicators",
 "certificate": "Service Certificate",
 "delete_key_unprovision": "Delete Key, Unprovision",
 "details": "Service Settings",
diff --git a/ipalib/plugins/internal.py b/ipalib/plugins/internal.py
index 54871f76de99d92f0f23129b4d636cc4fccfbb8b..714bcb7b2cd76b62f6b57d58101a5299405fef10 100644
--- a/ipalib/plugins/internal.py
+++ b/ipalib/plugins/internal.py
@@ -192,6 +192,8 @@ class i18n_messages(Command):
 },
 "authtype": {
 "config_tooltip": _("Implicit method (password) will be used if no method is chosen.Password + Two-factor: LDAP and Kerberos allow authentication with either one of the authentication types but Kerberos uses pre-authentication method which requires to use armor ccache.RADIUS with another type: Kerberos always use RADIUS, but LDAP never does. LDAP only recognize the password and two-factor authentication options."),
+"otp": _("OTP"),
+"radius": _("RADIUS"),
 "type_otp": _("Two factor authentication (password + OTP)"),
 "type_password": _("Password"),

Re: [Freeipa-devel] [PATCH 0093] Enable service authentication indicator management

2016-05-27 Thread Nathaniel McCallum
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


[Freeipa-devel] [PATCH 0095] Fix RADIUS capitalization

2016-05-27 Thread Nathaniel McCallum
RADIUS is an acryonym. This patch fixes its usage to match our
capitalization of other acronyms, like OTP.From 33f10766a9793531984d3be3fb7ec12c8ab1cde0 Mon Sep 17 00:00:00 2001
From: Nathaniel McCallum 
Date: Fri, 27 May 2016 12:10:00 -0400
Subject: [PATCH] Fix RADIUS capitalization

RADIUS is an acryonym. This patch fixes its usage to match our
capitalization of other acronyms, like OTP.
---
 daemons/ipa-otpd/main.c   | 4 ++--
 install/po/de.po  | 4 ++--
 install/po/fr.po  | 4 ++--
 install/po/ipa.pot| 2 +-
 install/po/uk.po  | 4 ++--
 install/ui/src/freeipa/radiusproxy.js | 4 ++--
 install/ui/test/data/ipa_init.json| 2 +-
 ipalib/plugins/internal.py| 2 +-
 8 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/daemons/ipa-otpd/main.c b/daemons/ipa-otpd/main.c
index aebc039bc05e5ddd04de2c0647a1cdb851a1b697..25906d0e36c7211ecaadcb4c8c9ed9b73ed7aa2b 100644
--- a/daemons/ipa-otpd/main.c
+++ b/daemons/ipa-otpd/main.c
@@ -270,10 +270,10 @@ int main(int argc, char **argv)
 goto error;
 }
 
-/* Radius Client */
+/* RADIUS Client */
 retval = krad_client_new(ctx.kctx, ctx.vctx, &ctx.client);
 if (retval != 0) {
-otpd_log_err(retval, "Unable to initialize radius client");
+otpd_log_err(retval, "Unable to initialize RADIUS client");
 goto error;
 }
 
diff --git a/install/po/de.po b/install/po/de.po
index 35d20932060ea8865aa8cb7b19a57e4e0e3d8469..d98cc9676e6a49c6b9f860cc706bb02cb42143e3 100644
--- a/install/po/de.po
+++ b/install/po/de.po
@@ -2359,8 +2359,8 @@ msgstr ""
 msgid "Two factor authentication (password + OTP)"
 msgstr "Zwei-Faktor-Authentifizierung (Passwort + Einmalpasswort)"
 
-msgid "Radius"
-msgstr "Radius"
+msgid "RADIUS"
+msgstr "RADIUS"
 
 msgid "About"
 msgstr "Über"
diff --git a/install/po/fr.po b/install/po/fr.po
index cefe28797ba0d89e7361980e3f851577738d8b63..6153641e903a3221bd8eae7b0bafe9244478f5c1 100644
--- a/install/po/fr.po
+++ b/install/po/fr.po
@@ -7643,8 +7643,8 @@ msgstr ""
 msgid "Two factor authentication (password + OTP)"
 msgstr "Authentification à deux-facteurs"
 
-msgid "Radius"
-msgstr "Radius"
+msgid "RADIUS"
+msgstr "RADIUS"
 
 msgid "Disable per-user override"
 msgstr "Désactiver la surcharge par utilisateur"
diff --git a/install/po/ipa.pot b/install/po/ipa.pot
index 8256bb77da282d6c327a761ffd07c31b8fc7bf28..ea1fe9412b261c4f2520d05f4d888e9645957d97 100644
--- a/install/po/ipa.pot
+++ b/install/po/ipa.pot
@@ -7527,7 +7527,7 @@ msgid "Two factor authentication (password + OTP)"
 msgstr ""
 
 #: ipalib/plugins/internal.py:198
-msgid "Radius"
+msgid "RADIUS"
 msgstr ""
 
 #: ipalib/plugins/internal.py:199
diff --git a/install/po/uk.po b/install/po/uk.po
index cb7f8705ccc0e51469c111eb7485eaa6ed580195..04e9034ae8b5971ab4b91fa75fd8058321b3357d 100644
--- a/install/po/uk.po
+++ b/install/po/uk.po
@@ -7648,8 +7648,8 @@ msgstr ""
 msgid "Two factor authentication (password + OTP)"
 msgstr "Двофакторне розпізнавання (пароль + OTP)"
 
-msgid "Radius"
-msgstr "Radius"
+msgid "RADIUS"
+msgstr "RADIUS"
 
 msgid "Disable per-user override"
 msgstr "Вимкнути перевизначення на рівні користувача"
diff --git a/install/ui/src/freeipa/radiusproxy.js b/install/ui/src/freeipa/radiusproxy.js
index 056d9504c198f6d54ad3d75aa490cb458dab15b6..3b523e5313434b3da2e198f139b1f1e96ca902a8 100644
--- a/install/ui/src/freeipa/radiusproxy.js
+++ b/install/ui/src/freeipa/radiusproxy.js
@@ -32,7 +32,7 @@ define([
 function(IPA, $, menu, phases, reg) {
 
 /**
- * Radius module
+ * RADIUS module
  * @class
  * @singleton
  */
@@ -115,7 +115,7 @@ return {
 };};
 
 /**
- * Radius specification object
+ * RADIUS specification object
  */
 radiusproxy.spec = make_spec();
 
diff --git a/install/ui/test/data/ipa_init.json b/install/ui/test/data/ipa_init.json
index 1b9b69ff909a9668c1e1867008459d25d5e062a9..d2f18a3d7ff60cf55df27f9d0f42874e9de1d008 100644
--- a/install/ui/test/data/ipa_init.json
+++ b/install/ui/test/data/ipa_init.json
@@ -52,7 +52,7 @@
 "config_tooltip": "Implicit method (password) will be used if no method is chosen.Password + Two-factor: LDAP and Kerberos allow authentication with either one of the authentication types but Kerberos uses pre-authentication method which requires to use armor ccache.RADIUS with another type: Kerberos always use RADIUS, but LDAP never does. LDAP only recognize the password and two-factor authentication options.",
 "type_otp": "Two factor authentication (password + OTP)",
 "type_password": "Password",
-"type_radius": "Radius",
+"type_radius": "RADIUS",
 "type_disabled": "Disable per-user override",
 "user_tooltip": "Per-user setting, overwrites the global setting if any option is checked.Password + Two-factor: LDAP and Kerberos al

Re: [Freeipa-devel] [PATCH 0037] Added /etc/krb5.conf.d/ to krb5.conf

2016-05-27 Thread Robbie Harwood
Stanislav Laznicka  writes:

> From 7a55f169181ab8647cd2d919f35c004b14d5bc7f Mon Sep 17 00:00:00 2001
> From: Stanislav Laznicka 
> Date: Fri, 27 May 2016 16:12:31 +0200
> Subject: [PATCH] Added krb5.conf.d/ to included dirs in krb5.conf
>
> The include of /etc/krb5.conf.d/ is required for crypto-policies to work 
> properly
>
> https://fedorahosted.org/freeipa/ticket/5912

Thank you for working on this.  Is the intent on the part of FreeIPA to
keep a separate, freeipa-speicifc directory?  And if so, can I suggest
that we not do that?


signature.asc
Description: PGP signature
-- 
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 0037] Added /etc/krb5.conf.d/ to krb5.conf

2016-05-27 Thread Alexander Bokovoy

On Fri, 27 May 2016, Robbie Harwood wrote:

Stanislav Laznicka  writes:


From 7a55f169181ab8647cd2d919f35c004b14d5bc7f Mon Sep 17 00:00:00 2001
From: Stanislav Laznicka 
Date: Fri, 27 May 2016 16:12:31 +0200
Subject: [PATCH] Added krb5.conf.d/ to included dirs in krb5.conf

The include of /etc/krb5.conf.d/ is required for crypto-policies to work 
properly

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


Thank you for working on this.  Is the intent on the part of FreeIPA to
keep a separate, freeipa-speicifc directory?  And if so, can I suggest
that we not do that?

Which directory are you talking about? /var/lib/sss/pubconf/krb5.include.d/?

SSSD directory is used already by all FreeIPA clients for very long time
because SSSD puts several important snippets there:
 - CA paths and domain_realm information based on the trust topology of FreeIPA
 - localauth plugin definition for SSSD plugin

SSSD cannot write to /etc and I don't think we have to change it.



--
/ 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