[Freeipa-devel] [PATCH] 0156 extdom: add certificate request

2016-05-20 Thread Sumit Bose 
Hi,

this patch allows the extom plugin to lookup users by certificate which
is needed in the case where a IPA client wants to lookup an AD user who
has the certificate stored in AD. To make this work the related patches
I just send to sssd-devel are needed as well.

Currently the patches miss the change in the required version of SSSD.
since the SSSD patches are not committed. But the patches are needed to
fully test the SSSD patches. I will send a new version with the needed
changes to the minimal SSSD version when the SSSD patches are committed.

bye,
Sumit
From b7b84fb4192af70e784c4cee18ff4be532d0f83f Mon Sep 17 00:00:00 2001
From: Sumit Bose 
Date: Tue, 26 Apr 2016 13:22:40 +0200
Subject: [PATCH] extdom: add certificate request

Related to https://fedorahosted.org/freeipa/ticket/4955
---
 .../ipa-extdom-extop/ipa_extdom.h  |  4 ++-
 .../ipa-extdom-extop/ipa_extdom_common.c   | 31 +-
 2 files changed, 27 insertions(+), 8 deletions(-)

diff --git a/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom.h 
b/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom.h
index 
a77711977186b702caafa2729dc13090c6031791..aa7855650789448ae4220b33cc2de858883fe302
 100644
--- a/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom.h
+++ b/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom.h
@@ -80,7 +80,8 @@ enum input_types {
 INP_SID = 1,
 INP_NAME,
 INP_POSIX_UID,
-INP_POSIX_GID
+INP_POSIX_GID,
+INP_CERT
 };
 
 enum request_types {
@@ -115,6 +116,7 @@ struct extdom_req {
 char *domain_name;
 gid_t gid;
 } posix_gid;
+char *cert;
 } data;
 char *err_msg;
 };
diff --git a/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_common.c 
b/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_common.c
index 
823c05c810361f121cb46831fb2d4e846729d792..e629247fd771e374d50486d836cd3b0d8d32a78a
 100644
--- a/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_common.c
+++ b/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_common.c
@@ -349,6 +349,9 @@ int parse_request_data(struct berval *req_val, struct 
extdom_req **_req)
 &id);
 req->data.posix_gid.gid = (gid_t) id;
 break;
+case INP_CERT:
+tag = ber_scanf(ber, "a}", &req->data.cert);
+break;
 default:
 ber_free(ber, 1);
 set_err_msg(req, "Unknown input type");
@@ -383,6 +386,9 @@ void free_req_data(struct extdom_req *req)
 case INP_POSIX_GID:
 ber_memfree(req->data.posix_gid.domain_name);
 break;
+case INP_CERT:
+ber_memfree(req->data.cert);
+break;
 }
 
 free(req->err_msg);
@@ -861,10 +867,12 @@ done:
 return ret;
 }
 
-static int handle_sid_request(struct ipa_extdom_ctx *ctx,
-  struct extdom_req *req,
-  enum request_types request_type, const char *sid,
-  struct berval **berval)
+static int handle_sid_or_cert_request(struct ipa_extdom_ctx *ctx,
+  struct extdom_req *req,
+  enum request_types request_type,
+  enum input_types input_type,
+  const char *input,
+  struct berval **berval)
 {
 int ret;
 struct passwd pwd;
@@ -878,7 +886,11 @@ static int handle_sid_request(struct ipa_extdom_ctx *ctx,
 enum sss_id_type id_type;
 struct sss_nss_kv *kv_list = NULL;
 
-ret = sss_nss_getnamebysid(sid, &fq_name, &id_type);
+if (input_type == INP_SID) {
+ret = sss_nss_getnamebysid(input, &fq_name, &id_type);
+} else {
+ret = sss_nss_getnamebycert(input, &fq_name, &id_type);
+}
 if (ret != 0) {
 if (ret == ENOENT) {
 ret = LDAP_NO_SUCH_OBJECT;
@@ -1135,8 +1147,13 @@ int handle_request(struct ipa_extdom_ctx *ctx, struct 
extdom_req *req,
 
 break;
 case INP_SID:
-ret = handle_sid_request(ctx, req, req->request_type, req->data.sid,
- berval);
+case INP_CERT:
+ret = handle_sid_or_cert_request(ctx, req, req->request_type,
+ req->input_type,
+ req->input_type == INP_SID ?
+ req->data.sid 
:
+ 
req->data.cert,
+ berval);
 break;
 case INP_NAME:
 ret = handle_name_request(ctx, req, req->request_type,
-- 
2.4.11

-- 
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] [PATCHES] 0789-0796 Python 3 fixes for the server part

2016-05-20 Thread Petr Viktorin 
Hello,
Here are some more Python3 patches. Most are pretty routine, but pay
special attention to the first and last patch.


With these patches, running the in-tree test suite gives me the same
errors in Python 2 and Python 3, except:
- test_install – failures in the updater that I haven't investigated yet
- test_ipaserver – test bug (relying on order of values in an LDAP
attribute) and a text/bytes issue in certificate parsing


In the next few months, I'll need to focus less on IPA and more on
Samba, which is a prerequisite for porting the IPA server. So I'll
quickly summarize the current state of the porting effort:

All of FreeIPA's dependencies except Samba are ported to Python 3 (and
packaged in Fedora).
A recent change switched the IPA client to running on Python 3. With the
patches I'm sending now, most of the "single machine" tests are passing.
The install scripts will still need some work, as will the server parts
that aren't shared with the client.


I'd like to ask the IPA team to sometimes take a look at the Python 3
tests, and try to avoid too many regressions.


-- 
Petr Viktorin
From 3a645b39f2ab80e60dd2dbcd5f6f4e39946d4450 Mon Sep 17 00:00:00 2001
From: Petr Viktorin 
Date: Fri, 29 Apr 2016 17:13:08 +0200
Subject: [PATCH] ipaldap: Keep attribute names as text, not bytes

Part of the work for: https://fedorahosted.org/freeipa/ticket/4985
---
 ipapython/ipaldap.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/ipapython/ipaldap.py b/ipapython/ipaldap.py
index 7e920e1003d84b729547d75d8e1324d2f73c120c..74add512adb6af092e9220c0ca2164c95caae262 100644
--- a/ipapython/ipaldap.py
+++ b/ipapython/ipaldap.py
@@ -813,7 +813,7 @@ class LDAPClient(object):
 If there is a problem loading the schema or the attribute is
 not in the schema return None
 """
-if isinstance(name_or_oid, unicode):
+if six.PY2 and isinstance(name_or_oid, unicode):
 name_or_oid = name_or_oid.encode('utf-8')
 
 if name_or_oid in self._SINGLE_VALUE_OVERRIDE:
@@ -1516,7 +1516,7 @@ class LDAPClient(object):
 
 # pass arguments to python-ldap
 with self.error_handler():
-modlist = [(a, self.encode(b), self.encode(c))
+modlist = [(a, str(b), self.encode(c))
for a, b, c in modlist]
 self.conn.modify_s(str(entry.dn), modlist)
 
-- 
2.5.5

From 85a4c73ca57e1f3ba64bdb76c386b6a9c32f9d36 Mon Sep 17 00:00:00 2001
From: Petr Viktorin 
Date: Fri, 6 May 2016 17:08:18 +0200
Subject: [PATCH] ipapython.secrets.kem: Use ConfigParser from six.moves

In Python 3, the module name changed from 'ConfigParser' to
'configparser'. Use the appropriate location from six.

Part of the work for: https://fedorahosted.org/freeipa/ticket/4985
---
 ipapython/secrets/kem.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/ipapython/secrets/kem.py b/ipapython/secrets/kem.py
index 1025ed7980f055c82c602634e8845fa490cf0514..0abf28ae4403a7b6225404df361d12cb07ccc70b 100644
--- a/ipapython/secrets/kem.py
+++ b/ipapython/secrets/kem.py
@@ -2,7 +2,7 @@
 
 from __future__ import print_function
 from ipaplatform.paths import paths
-import ConfigParser
+from six.moves.configparser import ConfigParser
 from cryptography.hazmat.backends import default_backend
 from cryptography.hazmat.primitives import serialization
 from cryptography.hazmat.primitives.asymmetric import rsa, ec
@@ -154,7 +154,7 @@ class IPAKEMKeys(KEMKeysStore):
 
 def __init__(self, config=None, ipaconf=paths.IPA_DEFAULT_CONF):
 super(IPAKEMKeys, self).__init__(config)
-conf = ConfigParser.ConfigParser()
+conf = ConfigParser()
 conf.read(ipaconf)
 self.host = conf.get('global', 'host')
 self.realm = conf.get('global', 'realm')
-- 
2.5.5

From f5d3da6a63a41b626bc9c1b4ac7f6603e220d6ff Mon Sep 17 00:00:00 2001
From: Petr Viktorin 
Date: Fri, 6 May 2016 17:35:26 +0200
Subject: [PATCH] test_topology_plugin: Don't rely on order of an attribute's
 values

Order of Python dicts/sets was always unreliable, but in Python 3
it's usually different every time. This affects the order in which
values of a LDAP attribute appear.
LDAP values are also specified to be unordered.

Part of the work for: https://fedorahosted.org/freeipa/ticket/4985
---
 ipatests/test_ipaserver/test_topology_plugin.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ipatests/test_ipaserver/test_topology_plugin.py b/ipatests/test_ipaserver/test_topology_plugin.py
index 6407f3e6f4ef4ff0fe12de721749dd41a667885c..eaa7b01cb977e885a731e83d0a31597cf7a05168 100644
--- a/ipatests/test_ipaserver/test_topology_plugin.py
+++ b/ipatests/test_ipaserver/test_topology_plugin.py
@@ -72,4 +72,4 @@ class TestTopologyPlugin(object):
 entry = self.conn.get_entry(topoplugindn)
 assert(set(entry.keys()) == set(pluginattrs.keys()))
 for i in checkvalues:
-assert(pluginattrs[i] == entry[i]

Re: [Freeipa-devel] [PATCH 0484] remove unused code from automount plugin

2016-05-20 Thread Martin Basti 



On 20.05.2016 15:03, Martin Basti wrote:

The removed code is unused for long time.

Patch attached.




https://fedorahosted.org/freeipa/attachment/ticket/5899/
-- 
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] 0002 Incorrect message when KRA already installed

2016-05-20 Thread Martin Basti 



On 20.05.2016 16:20, Martin Basti wrote:



On 20.05.2016 12:12, Petr Spacek wrote:

On 17.5.2016 10:54, Patrice Duc-Jacquet wrote:

Hi everyone

Please see attached candidate patch for ticket

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

ACK, please add link to the ticket to commit message before pushing.


Pushed to master: 65794fc71c6b76a8fe96423e3fac128dc5de2c7d


yes I forgot to add ticket there before push, sorry

--
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] 0002 Incorrect message when KRA already installed

2016-05-20 Thread Martin Basti 



On 20.05.2016 12:12, Petr Spacek wrote:

On 17.5.2016 10:54, Patrice Duc-Jacquet wrote:

Hi everyone

Please see attached candidate patch for ticket

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

ACK, please add link to the ticket to commit message before pushing.


Pushed to master: 65794fc71c6b76a8fe96423e3fac128dc5de2c7d

--
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 0473-0476]DNS Locations: Prologue

2016-05-20 Thread Martin Basti 



On 20.05.2016 12:30, Petr Spacek wrote:

On 18.5.2016 12:43, Martin Basti wrote:


On 12.05.2016 16:16, Martin Basti wrote:



On 12.05.2016 11:01, Martin Basti wrote:


On 11.05.2016 09:41, Martin Basti wrote:


On 10.05.2016 18:56, Petr Spacek wrote:

On 10.5.2016 15:38, Petr Spacek wrote:

On 10.5.2016 15:26, Martin Basti wrote:

On 10.05.2016 15:23, Petr Spacek wrote:

On 10.5.2016 14:44, Martin Basti wrote:

On 10.05.2016 14:33, Petr Spacek wrote:

On 6.5.2016 10:20, Martin Basti wrote:

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

Patches attached.


freeipa-mbasti-0473-DNS-Locations-Always-create-DNS-related-privileges.patch



From 9a936740da7cdacec150acc92a45041a98ce7cb3 Mon Sep 17
00:00:00 2001
From: Martin Basti 
Date: Wed, 4 May 2016 17:33:52 +0200
Subject: [PATCH 1/4] DNS Locations: Always create DNS related
privileges

DNS privileges are important for handling DNS locations which can be
created without DNS servers in IPA topology. We will also need this
privileges presented for future feature 'External DNS support'

Seems reasonable, ACK.



freeipa-mbasti-0474-DNS-Locations-add-new-attributes-and-objectclasses.patch



From a7766da5fd1a72884308d4206c9cde262f5c8d35 Mon Sep 17
00:00:00 2001
From: Martin Basti 
Date: Thu, 5 May 2016 11:12:00 +0200
Subject: [PATCH 2/4] DNS Locations: add new attributes and
objectclasses

http://www.freeipa.org/page/V4/DNS_Location_Mechanism

https://fedorahosted.org/freeipa/ticket/2008
---
 install/share/60ipadns.ldif | 4 
 1 file changed, 4 insertions(+)

diff --git a/install/share/60ipadns.ldif b/install/share/60ipadns.ldif
index
e0ed0ab869cea0478d9640bb509c6267abed1a01..31c2f71f8566d04a05709f1359b20e6fa51921ce


100644
--- a/install/share/60ipadns.ldif
+++ b/install/share/60ipadns.ldif
@@ -70,9 +70,13 @@ attributeTypes: ( 2.16.840.1.113730.3.8.5.25 NAME
'idnsSecKeyRevoke' DESC 'DNSKE
 attributeTypes: ( 2.16.840.1.113730.3.8.5.26 NAME
'idnsSecKeySep' DESC
'DNSKEY SEP flag (equivalent to bit 15): RFC 4035' EQUALITY
booleanMatch
SYNTAX 1.3.6.1.4.1.1466.115.121.1.7 SINGLE-VALUE X-ORIGIN 'IPA v4.1' )
 attributeTypes: ( 2.16.840.1.113730.3.8.5.27 NAME
'idnsSecAlgorithm' DESC
'DNSKEY algorithm: string used as mnemonic' EQUALITY
caseIgnoreIA5Match
SUBSTR caseIgnoreIA5SubstringsMatch SYNTAX
1.3.6.1.4.1.1466.115.121.1.26
SINGLE-VALUE X-ORIGIN 'IPA v4.1' )
 attributeTypes: ( 2.16.840.1.113730.3.8.5.28 NAME
'idnsSecKeyRef' DESC
'PKCS#11 URI of the key' EQUALITY caseExactMatch SINGLE-VALUE SYNTAX
1.3.6.1.4.1.1466.115.121.1.15 X-ORIGIN 'IPA v4.1' )
+attributeTypes: ( 2.16.840.1.113730.3.8.5.32 NAME 'ipaLocation' DESC
'Reference to IPA location' EQUALITY distinguishedNameMatch SYNTAX
1.3.6.1.4.1.1466.115.121.1.12 SINGLE-VALUE X-ORIGIN 'IPA v4.4' )
+attributeTypes: ( 2.16.840.1.113730.3.8.5.33 NAME
'ipaLocationWeight' DESC
'Weight for the server in IPA location' EQUALITY integerMatch SYNTAX
1.3.6.1.4.1.1466.115.121.1.27 SINGLE-VALUE X-ORIGIN 'IPA v4.4' )
 objectClasses: ( 2.16.840.1.113730.3.8.6.0 NAME 'idnsRecord'
DESC 'dns
Record, usually a host' SUP top STRUCTURAL MUST idnsName MAY ( cn $
idnsAllowDynUpdate $ dNSTTL $ dNSClass $ aRecord $ aAAARecord $
a6Record $
nSRecord $ cNAMERecord $ pTRRecord $ sRVRecord $ tXTRecord $
mXRecord $
mDRecord $ hInfoRecord $ mInfoRecord $ aFSDBRecord $ SigRecord $
KeyRecord
$ LocRecord $ nXTRecord $ nAPTRRecord $ kXRecord $ certRecord $
dNameRecord
$ dSRecord $ sSHFPRecord $ rRSIGRecord $ nSECRecord $ DLVRecord $
TLSARecord $ UnknownRecord $ RPRecord $ APLRecord $ IPSECKEYRecord $
DHCIDRecord $ HIPRecord $ SPFRecord ) )
 objectClasses: ( 2.16.840.1.113730.3.8.6.1 NAME 'idnsZone' DESC
'Zone
class' SUP idnsRecord STRUCTURAL MUST ( idnsZoneActive $
idnsSOAmName $
idnsSOArName $ idnsSOAserial $ idnsSOArefresh $ idnsSOAretry $
idnsSOAexpire $ idnsSOAminimum ) MAY ( idnsUpdatePolicy $
idnsAllowQuery $
idnsAllowTransfer $ idnsAllowSyncPTR $ idnsForwardPolicy $
idnsForwarders $
idnsSecInlineSigning $ nSEC3PARAMRecord ) )
 objectClasses: ( 2.16.840.1.113730.3.8.6.2 NAME
'idnsConfigObject' DESC
'DNS global config options' STRUCTURAL MAY ( idnsForwardPolicy $
idnsForwarders $ idnsAllowSyncPTR $ idnsZoneRefresh $
idnsPersistentSearch ) )
 objectClasses: ( 2.16.840.1.113730.3.8.12.18 NAME 'ipaDNSZone'
SUP top
AUXILIARY MUST idnsName MAY managedBy X-ORIGIN 'IPA v3' )
 objectClasses: ( 2.16.840.1.113730.3.8.6.3 NAME
'idnsForwardZone' DESC
'Forward Zone class' SUP top STRUCTURAL MUST ( idnsName $
idnsZoneActive )
MAY ( idnsForwarders $ idnsForwardPolicy ) )
 objectClasses: ( 2.16.840.1.113730.3.8.6.4 NAME 'idnsSecKey'
DESC 'DNSSEC
key metadata' STRUCTURAL MUST ( idnsSecKeyRef $ idnsSecKeyCreated $
idnsSecAlgorithm ) MAY ( idnsSecKeyPublish $ idnsSecKeyActivate $
idnsSecKeyInactive $ idnsSecKeyDelete $ idnsSecKeyZone $
idnsSecKeyRevoke $
idnsSecKeySep $ cn ) X-ORIGIN 'IPA v4.1' )
+objectClasses: ( 2.16.840.1.113730.3.8.6.7 NAME
'ipaLocationObject' DESC
'Object for storing IPA server location' AUXI

Re: [Freeipa-devel] [PATCH 0477] upgrade: always start CA

2016-05-20 Thread Martin Basti 



On 19.05.2016 13:34, Stanislav Laznicka wrote:


Also, I tried to upgrade from 4.2.4 to 4.3.1 and it seems that it 
might be necessary to start the service even earlier in the upgrade 
logic. Attached is the trace that occurred during the upgrade.


I sent the whole log earlier accidentally, hopefully it will not 
arrive here as well.


On 05/19/2016 11:10 AM, Stanislav Laznicka wrote:


NACK, see my comments below

+# following upgrade steps require running CA
This is a nitpicky nitpick but could you please change this comment 
for # the following ...

Took me a while to understand what you were trying to say here.
+if ca_running and not ca.is_running():
+ca.stop('pki-tomcat')
+elif not ca_running and ca.is_running():
+ca.start('pki-tomcat')
+
You should swap ca.stop and ca.start here, you're stopping the 
service when it's stopped and starting it when it's already running.

Shame, shame, shame on me.



On 05/12/2016 04:34 PM, Martin Basti wrote:

Patch attached.

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











I moved starting of CA to the earlier phase and swapped start/stop to 
correct order.


Patch attached.
From b93d3d7823cf17cb32b25ed8b374155c446743ba Mon Sep 17 00:00:00 2001
From: Martin Basti 
Date: Thu, 12 May 2016 15:39:30 +0200
Subject: [PATCH] Upgrade: always start CA

Some CA upgrade steps in upgrader requires running CA. We have to always
start CA and wait for running status using http, because systemd may
return false positive result that CA is running even if CA is just
starting and unable to serve.

https://fedorahosted.org/freeipa/ticket/5868
---
 ipaserver/install/server/upgrade.py | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/ipaserver/install/server/upgrade.py b/ipaserver/install/server/upgrade.py
index 38fe2c3e89da55faa30c624983cb8f9c630357b3..1d5ef0b23e4afc25b44dbce3012fbbac6d68c476 100644
--- a/ipaserver/install/server/upgrade.py
+++ b/ipaserver/install/server/upgrade.py
@@ -1448,6 +1448,7 @@ def upgrade_configuration():
 sub_dict['SUBJECT_BASE'] = subject_base
 
 ca = cainstance.CAInstance(api.env.realm, certs.NSS_DIR)
+ca_running = ca.is_running()
 
 with installutils.stopped_service('pki-tomcatd', 'pki-tomcat'):
 # Dogtag must be stopped to be able to backup CS.cfg config
@@ -1481,6 +1482,11 @@ def upgrade_configuration():
 )
 upgrade_pki(ca, fstore)
 
+# several upgrade steps require running CA
+# always run ca.start() because we need to wait until CA is really ready
+# by checking status using http
+ca.start('pki-tomcat')
+
 certmonger_service = services.knownservices.certmonger
 if ca.is_configured() and not certmonger_service.is_running():
 certmonger_service.start()
@@ -1650,6 +1656,11 @@ def upgrade_configuration():
 elif not ds_running and ds.is_running():
 ds.stop(ds_serverid)
 
+if ca_running and not ca.is_running():
+ca.start('pki-tomcat')
+elif not ca_running and ca.is_running():
+ca.stop('pki-tomcat')
+
 
 def upgrade_check(options):
 try:
-- 
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 0483] fix referenced before assignment error in baseldap

2016-05-20 Thread Martin Basti 



On 19.05.2016 10:09, Stanislav Laznicka wrote:


ACK


On 05/18/2016 07:24 PM, Martin Basti wrote:

Patch attached






Pushed to master: ad1cac12834615a424885ae4997286641548

-- 
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 0099] ipa-nis-manage: add status option

2016-05-20 Thread Martin Babinsky 

On 04/28/2016 05:15 PM, Petr Spacek wrote:

On 28.4.2016 14:52, Abhijeet Kasurde wrote:

Hi Petr,

On 04/25/2016 08:28 PM, Petr Spacek wrote:

Hello,

ipa-nis-manage: add status option

https://bugzilla.redhat.com/show_bug.cgi?id=1329275




Can you reword the error message here as well ?

 if len(args) != 1:
 sys.exit("You must specify one action, either enable or disable")

Thanks,
Abhijeet Kasurde


Good catch!





Hi Petr,

please use upstream ticket provided by Petr Vobornik[1] in the commit 
message.


Also I would rewrite

"""+elif args[0] != "enable" and args[0] != "disable" and args[0] != 
"status":


"""

in a more pythonic way:

"elif args[0] not in {"enable", "disable", "status"}:"

Otherwise the patch works as expected.

[1] https://fedorahosted.org/freeipa/ticket/5856

--
Martin^3 Babinsky

--
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 0484] remove unused code from automount plugin

2016-05-20 Thread Martin Basti 

The removed code is unused for long time.

Patch attached.

From 074ffe14fb271ca32421021fb8a40f3d9fb7bc83 Mon Sep 17 00:00:00 2001
From: Martin Basti 
Date: Thu, 19 May 2016 10:24:43 +0200
Subject: [PATCH] Remove unused variables in automount plugin

---
 ipalib/plugins/automount.py | 9 +
 1 file changed, 1 insertion(+), 8 deletions(-)

diff --git a/ipalib/plugins/automount.py b/ipalib/plugins/automount.py
index c0a55d5dabec72ace99ab178ff3dec7b15ede744..3eb904840d86027ca194db856fa21bf03451e99d 100644
--- a/ipalib/plugins/automount.py
+++ b/ipalib/plugins/automount.py
@@ -307,13 +307,9 @@ class automountlocation_tofiles(LDAPQuery):
 __doc__ = _('Generate automount files for a specific location.')
 
 def execute(self, *args, **options):
-ldap = self.obj.backend
+self.api.Command['automountlocation_show'](args[0])
 
-location = self.api.Command['automountlocation_show'](args[0])
-
-maps = []
 result = self.api.Command['automountkey_find'](args[0], u'auto.master')
-truncated = result['truncated']
 maps = result['result']
 
 # maps, truncated
@@ -327,7 +323,6 @@ class automountlocation_tofiles(LDAPQuery):
 mapnames.append(info)
 key = info.split(None)
 result = self.api.Command['automountkey_find'](args[0], key[0])
-truncated = result['truncated']
 keys[info] = result['result']
 # TODO: handle truncated results, same as above
 
@@ -342,7 +337,6 @@ class automountlocation_tofiles(LDAPQuery):
 for m in orphanmaps:
 key = m['automountmapname']
 result = self.api.Command['automountkey_find'](args[0], key[0])
-truncated = result['truncated']
 orphankeys.append(result['result'])
 
 return dict(result=dict(maps=maps, keys=keys,
@@ -456,7 +450,6 @@ class automountlocation_import(LDAPQuery):
 mapfile = am[1].replace('"','')
 am[1] = os.path.basename(am[1])
 maps[am[1]] = mapfile
-info = ' '.join(am[1:])
 
 # Add a new key to the auto.master map for the new map file
 try:
-- 
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 0095-0098] NTP: use augeas, configure chronyd, do not overwrite config

2016-05-20 Thread Martin Babinsky 

On 05/20/2016 02:29 PM, Martin Babinsky wrote:

On 05/16/2016 01:58 PM, David Kupka wrote:

On 26/04/16 10:09, David Kupka wrote:

On 14/03/16 14:01, Martin Basti wrote:



On 14.03.2016 13:46, Martin Babinsky wrote:

On 03/11/2016 09:16 AM, David Kupka wrote:

Current version (0.5.0) of python-augeas is missing copy() method.
Use
dkupka/python-augeas copr repo before new version it's build and
available in the official repos.




Hi David,

TLDR: NACK :D.

Here are high-level remarks to discuss:

Maybe it would be a good idea to move ipaaugeas/changeconf and ntp to
ipaplatform since it is dealing with (sorta) platform specific
behavior (ntp vs. chrony vs. whatever we will have for timesync in the
future). CC'ing Jan for thoughts.

Also regarding patches 0096-0097, we could have platform specific
TimeDateService object that could wrap NTP/chrony management. for
example, the task namespace functions in Pathc 0096 could be
reimplemented as a methods of the service (RedhatTimeDateService,
FedoraTimeDateService etc.) and then called in a platform-agnostic
manner.

Here are some comments regarding code:

Patch 0095:

1.)
+IPA_CUSTOM_AUGEAS_LENSES_DIR = '/usr/share/augeas/lenses/ipa/'

Do not forget to add this directory to %install and %files spection of
the spec file so that it is correctly added to RPM build.

2.)

please separate import of system-wide and IPA-specific modules by
blank line

+import collections
+from augeas import Augeas
+from ipaplatform.paths import paths
+from ipapython.ipa_log_manager import root_logger

3.) the call to parent's __new__ should have signature 'super(aug_obj,
cls).__new__(*args, **kwargs)'

+cls._instance = super(aug_obj, cls).__new__(cls, *args,
**kwargs)

4.)

+raise RuntimeError('Augeas lenses was loaded. Could not
add more'
+   'lenses.')

Should be 'Augeas lenses _were_ loaded'

5.)

+if lens in self.lenses:
+raise RuntimeError('Lens %s already added.' % lens)
+self.lenses.append(lens)
+load_path = '/augeas/load/{0}'.format(lens


Shouldn't the following code use os.path,join to construct the
load_path?

6.) I would prefer the following indentation style in the add_lens()
method

@@ -65,9 +65,9 @@ class aug_obj(object):
 for conf_file in conf_files:
 self._aug.set(os.path.join(load_path, 'incl[0]'),
conf_file)
 self.tree['old'] = self.tree.get(conf_file, None)
-self.tree[conf_file] = aug_node(self._aug,
- os.path.join('/files',
- conf_file[1:]))
+self.tree[conf_file] = aug_node(
+self._aug, os.path.join('/files', conf_file[1:])
+)

7.) I would also prefer if the hardcoded paths like '/augeas/load',
'files', and '//error' would be made into either module variables or
class members.

8.)

+def load(self):
+if self._loaded:
+raise RuntimeError('Augeas lenses was loaded. Could not
add more'
+   'lenses.')

Fix the excpetion text in the same way as in 4.)

9.)

+errors = self._aug.match(os.path.join('//error'))

is the os.path.join necessary here?

10.) I guess you can rewrite the error message in load() method using
list comprehension:

@@ -76,9 +76,9 @@ class aug_obj(object):
 self._aug.load()
 errors = self._aug.match(os.path.join('//error'))
 if errors:
-err_msg = ""
-for error in errors:
-err_msg += ("{}: {}".format(error,
self._aug.get(error)))
+err_msg = '\n'.join(
+["{}: {}".format(e, self._aug.get(e)) for e in
errors]
+)
 raise RuntimeError(err_msg)
 self._loaded = True

11.)

+class aug_node(collections.MutableMapping):
+""" Single augeas node.
+Can be handled as python dict().
+"""
+def __init__(self, aug, path):
+self._aug = aug
+if path and os.path.isabs(path):
+self._path = path
+else:
+self._tmp = _tmp_path(aug, path)
+self._path = self._tmp.path

Isn't it better to change signature of __init__ to:

def __init__(self, aug, path=None):

and then test whether path is None?

12.)

def __setitem__(self, key, node):
+target = os.path.join(self._path, key)
+end = '{0}[0]'.format(os.path.join(self._path, key))
+if self._aug.match(target):
+self._aug.remove(target)
+target_list = aug_list(self._aug, target)
+for src_node in aug_list(node._aug, node._path):
+target_list.append(src_node)

The 'end' variable is declared but not used.

13.)

+
+def __len__(self):
+return len(self._aug.match('{0}/*'.format(self._path)))
+
+def __iter__(self):
+for key in self._aug.match('{0}/*'.format(self._path)):
+yield self._aug.label(key)
+raise StopIteration()
+

Shouldn't we construct paths using os.path.join for the sake of
consistency?

1

Re: [Freeipa-devel] [PATCH 0112] pylint: replace Refactor category with individual check name

2016-05-20 Thread Martin Basti 



On 20.05.2016 10:26, Petr Spacek wrote:

On 19.5.2016 14:47, Martin Basti wrote:

On 19.05.2016 14:26, Petr Spacek wrote:

Hello,

pylint: replace Refactor category with individual check names

This eases enabling/disabling individual tests like cyclic-import.


I like this patch but, NACK

.
* Module ipalib.config
ipalib/config.py:260: [R0204(redefined-variable-type), Env.__setitem__]
Redefinition of value type from int to ipapython.dn.DN)
ipalib/config.py:458: [R0102(simplifiable-if-statement), Env._bootstrap] The
if statement can be replaced with 'var = bool(test)')
* Module ipalib.messages
ipalib/messages.py:90: [R0204(redefined-variable-type),
process_message_arguments] Redefinition of obj.strerror type from unicode to 
str)
* Module ipalib.plugable
ipalib/plugable.py:569: [R0204(redefined-variable-type), API.import_plugins]
Redefinition of modules type from generator to list)
* Module ipalib.rpc
ipalib/rpc.py:609: [R0101(too-many-nested-blocks),
KerbTransport.single_request] Too many nested blocks (6/5))
ipalib/rpc.py:753: [R0204(redefined-variable-type), RPCClient.get_url_list]
Redefinition of answers type from dns.resolver.Answer to list)



tested with pylint-1.5.5-1.fc24.noarch

Here it is.


ACK

Pushed to master: 6e4b749b59ebae82c613fe799dda7cb21dc080cd

I'm looking forward to enabling any of that checks :)

--
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 0095-0098] NTP: use augeas, configure chronyd, do not overwrite config

2016-05-20 Thread Martin Babinsky 

On 05/16/2016 01:58 PM, David Kupka wrote:

On 26/04/16 10:09, David Kupka wrote:

On 14/03/16 14:01, Martin Basti wrote:



On 14.03.2016 13:46, Martin Babinsky wrote:

On 03/11/2016 09:16 AM, David Kupka wrote:

Current version (0.5.0) of python-augeas is missing copy() method. Use
dkupka/python-augeas copr repo before new version it's build and
available in the official repos.




Hi David,

TLDR: NACK :D.

Here are high-level remarks to discuss:

Maybe it would be a good idea to move ipaaugeas/changeconf and ntp to
ipaplatform since it is dealing with (sorta) platform specific
behavior (ntp vs. chrony vs. whatever we will have for timesync in the
future). CC'ing Jan for thoughts.

Also regarding patches 0096-0097, we could have platform specific
TimeDateService object that could wrap NTP/chrony management. for
example, the task namespace functions in Pathc 0096 could be
reimplemented as a methods of the service (RedhatTimeDateService,
FedoraTimeDateService etc.) and then called in a platform-agnostic
manner.

Here are some comments regarding code:

Patch 0095:

1.)
+IPA_CUSTOM_AUGEAS_LENSES_DIR = '/usr/share/augeas/lenses/ipa/'

Do not forget to add this directory to %install and %files spection of
the spec file so that it is correctly added to RPM build.

2.)

please separate import of system-wide and IPA-specific modules by
blank line

+import collections
+from augeas import Augeas
+from ipaplatform.paths import paths
+from ipapython.ipa_log_manager import root_logger

3.) the call to parent's __new__ should have signature 'super(aug_obj,
cls).__new__(*args, **kwargs)'

+cls._instance = super(aug_obj, cls).__new__(cls, *args,
**kwargs)

4.)

+raise RuntimeError('Augeas lenses was loaded. Could not
add more'
+   'lenses.')

Should be 'Augeas lenses _were_ loaded'

5.)

+if lens in self.lenses:
+raise RuntimeError('Lens %s already added.' % lens)
+self.lenses.append(lens)
+load_path = '/augeas/load/{0}'.format(lens


Shouldn't the following code use os.path,join to construct the
load_path?

6.) I would prefer the following indentation style in the add_lens()
method

@@ -65,9 +65,9 @@ class aug_obj(object):
 for conf_file in conf_files:
 self._aug.set(os.path.join(load_path, 'incl[0]'),
conf_file)
 self.tree['old'] = self.tree.get(conf_file, None)
-self.tree[conf_file] = aug_node(self._aug,
- os.path.join('/files',
- conf_file[1:]))
+self.tree[conf_file] = aug_node(
+self._aug, os.path.join('/files', conf_file[1:])
+)

7.) I would also prefer if the hardcoded paths like '/augeas/load',
'files', and '//error' would be made into either module variables or
class members.

8.)

+def load(self):
+if self._loaded:
+raise RuntimeError('Augeas lenses was loaded. Could not
add more'
+   'lenses.')

Fix the excpetion text in the same way as in 4.)

9.)

+errors = self._aug.match(os.path.join('//error'))

is the os.path.join necessary here?

10.) I guess you can rewrite the error message in load() method using
list comprehension:

@@ -76,9 +76,9 @@ class aug_obj(object):
 self._aug.load()
 errors = self._aug.match(os.path.join('//error'))
 if errors:
-err_msg = ""
-for error in errors:
-err_msg += ("{}: {}".format(error,
self._aug.get(error)))
+err_msg = '\n'.join(
+["{}: {}".format(e, self._aug.get(e)) for e in errors]
+)
 raise RuntimeError(err_msg)
 self._loaded = True

11.)

+class aug_node(collections.MutableMapping):
+""" Single augeas node.
+Can be handled as python dict().
+"""
+def __init__(self, aug, path):
+self._aug = aug
+if path and os.path.isabs(path):
+self._path = path
+else:
+self._tmp = _tmp_path(aug, path)
+self._path = self._tmp.path

Isn't it better to change signature of __init__ to:

def __init__(self, aug, path=None):

and then test whether path is None?

12.)

def __setitem__(self, key, node):
+target = os.path.join(self._path, key)
+end = '{0}[0]'.format(os.path.join(self._path, key))
+if self._aug.match(target):
+self._aug.remove(target)
+target_list = aug_list(self._aug, target)
+for src_node in aug_list(node._aug, node._path):
+target_list.append(src_node)

The 'end' variable is declared but not used.

13.)

+
+def __len__(self):
+return len(self._aug.match('{0}/*'.format(self._path)))
+
+def __iter__(self):
+for key in self._aug.match('{0}/*'.format(self._path)):
+yield self._aug.label(key)
+raise StopIteration()
+

Shouldn't we construct paths using os.path.join for the sake of
consistency?

14.)

+def __bool__(self):
+return (b

Re: [Freeipa-devel] [PATCH 0013] Updated ipa-server-install man page for domain-level attribute

2016-05-20 Thread Petr Spacek 
On 20.5.2016 13:21, Abhijeet Kasurde wrote:
> Hi All,
> 
> Please find the patch for review.
> 
> On 05/09/2016 01:28 PM, Petr Spacek wrote:
>> On 6.5.2016 07:33, Abhijeet Kasurde wrote:
>>> Please review this patch.
>> Good catch!
>>
>> In general, I believe that man page should explain what domain level means
>> (probably with an example of levels 0 and 1) so the user can actually use the
>> man page to find out what value is needed for his purposes.
>>
>> Considering this, I have to NACK this patch. Please elaborate.
>>
>> Thank you!
>>
> 
> -- 
> Thanks,
> Abhijeet Kasurde
> 
> IRC: akasurde
> http://akasurde.github.io
> 
> 
> freeipa-akasurde-0013-1-Updated-ipa-server-install-man-page-for-domain-level-ipa-4-3.patch
> 
> 
> From e32e9a1b5b1d666d53c575a27beb8dadd09e26cc Mon Sep 17 00:00:00 2001
> From: Abhijeet Kasurde 
> Date: Fri, 20 May 2016 16:45:40 +0530
> Subject: [PATCH] Updated ipa-server-install man page for domain-level
>  attribute
> 
> Fixes: https://fedorahosted.org/freeipa/ticket/5708
> 
> Signed-off-by: Abhijeet Kasurde 
> ---
>  install/tools/man/ipa-server-install.1 | 7 +--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/install/tools/man/ipa-server-install.1 
> b/install/tools/man/ipa-server-install.1
> index 
> 55b49449e3c44aebfeefe5cb71d73e9abf07c5b2..7638726c306ed64706c33564f1ca175197e7a7bf
>  100644
> --- a/install/tools/man/ipa-server-install.1
> +++ b/install/tools/man/ipa-server-install.1
> @@ -1,5 +1,5 @@
>  .\" A man page for ipa-server-install
> -.\" Copyright (C) 2008 Red Hat, Inc.
> +.\" Copyright (C) 2008-2016 Red Hat, Inc.
>  .\"
>  .\" This program is free software; you can redistribute it and/or modify
>  .\" it under the terms of the GNU General Public License as published by
> @@ -16,7 +16,7 @@
>  .\"
>  .\" Author: Rob Crittenden 
>  .\"
> -.TH "ipa-server-install" "1" "Jun 28 2012" "FreeIPA" "FreeIPA Manual Pages"
> +.TH "ipa-server-install" "1" "May 20 2016" "FreeIPA" "FreeIPA Manual Pages"
>  .SH "NAME"
>  ipa\-server\-install \- Configure an IPA server
>  .SH "SYNOPSIS"
> @@ -84,6 +84,9 @@ An unattended installation that will never prompt for user 
> input
>  .TP
>  \fB\-\-dirsrv\-config\-file\fR
>  The path to LDIF file that will be used to modify configuration of dse.ldif 
> during installation of the directory server instance
> +.TP
> +\fB\-\-domain\-level\fR
> +Specifies IPA domain level value. Domain level indicates that server is 
> capable of doing certain operations. Domain level 1 means that it supports 
> replica promotion and topology management. Old IPA servers and IPA servers 
> upgraded to 4.3 in existing environments have domain level 0.
>  
>  .SS "CERTIFICATE SYSTEM OPTIONS"
>  .TP
> -- 2.4.11
> 

Thanks but NACK.

Domain level is not about one server. It defines that all servers in one IPA
topology behave in some particular way.

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

Re: [Freeipa-devel] [PATCH 0013] Updated ipa-server-install man page for domain-level attribute

2016-05-20 Thread Abhijeet Kasurde 

Hi All,

Please find the patch for review.

On 05/09/2016 01:28 PM, Petr Spacek wrote:

On 6.5.2016 07:33, Abhijeet Kasurde wrote:

Please review this patch.

Good catch!

In general, I believe that man page should explain what domain level means
(probably with an example of levels 0 and 1) so the user can actually use the
man page to find out what value is needed for his purposes.

Considering this, I have to NACK this patch. Please elaborate.

Thank you!



--
Thanks,
Abhijeet Kasurde

IRC: akasurde
http://akasurde.github.io

From e32e9a1b5b1d666d53c575a27beb8dadd09e26cc Mon Sep 17 00:00:00 2001
From: Abhijeet Kasurde 
Date: Fri, 20 May 2016 16:45:40 +0530
Subject: [PATCH] Updated ipa-server-install man page for domain-level
 attribute

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

Signed-off-by: Abhijeet Kasurde 
---
 install/tools/man/ipa-server-install.1 | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/install/tools/man/ipa-server-install.1 b/install/tools/man/ipa-server-install.1
index 55b49449e3c44aebfeefe5cb71d73e9abf07c5b2..7638726c306ed64706c33564f1ca175197e7a7bf 100644
--- a/install/tools/man/ipa-server-install.1
+++ b/install/tools/man/ipa-server-install.1
@@ -1,5 +1,5 @@
 .\" A man page for ipa-server-install
-.\" Copyright (C) 2008 Red Hat, Inc.
+.\" Copyright (C) 2008-2016 Red Hat, Inc.
 .\"
 .\" This program is free software; you can redistribute it and/or modify
 .\" it under the terms of the GNU General Public License as published by
@@ -16,7 +16,7 @@
 .\"
 .\" Author: Rob Crittenden 
 .\"
-.TH "ipa-server-install" "1" "Jun 28 2012" "FreeIPA" "FreeIPA Manual Pages"
+.TH "ipa-server-install" "1" "May 20 2016" "FreeIPA" "FreeIPA Manual Pages"
 .SH "NAME"
 ipa\-server\-install \- Configure an IPA server
 .SH "SYNOPSIS"
@@ -84,6 +84,9 @@ An unattended installation that will never prompt for user input
 .TP
 \fB\-\-dirsrv\-config\-file\fR
 The path to LDIF file that will be used to modify configuration of dse.ldif during installation of the directory server instance
+.TP
+\fB\-\-domain\-level\fR
+Specifies IPA domain level value. Domain level indicates that server is capable of doing certain operations. Domain level 1 means that it supports replica promotion and topology management. Old IPA servers and IPA servers upgraded to 4.3 in existing environments have domain level 0.
 
 .SS "CERTIFICATE SYSTEM OPTIONS"
 .TP
-- 
2.4.11

-- 
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] 0035: webui: change Restore cert to Remove cert hold

2016-05-20 Thread Pavel Vomacka 

Hi,

please review attached patch. It change Restore certificate strings to 
Remove certificate hold to be consistent with CLI.


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

--

Pavel^3 Vomacka


From bd0c90c5c52120a5cc7989fd2e6160a13a845ee4 Mon Sep 17 00:00:00 2001
From: Pavel Vomacka 
Date: Fri, 20 May 2016 12:46:53 +0200
Subject: [PATCH] Change 'Restore' to 'Remove Hold'

To be consistent with CLI the restoring certificate is renamed to
removing certificate hold in all WebUI components.

https://fedorahosted.org/freeipa/ticket/5878
---
 install/ui/src/freeipa/certificate.js | 24 +---
 install/ui/test/data/ipa_init.json| 10 ++
 ipalib/plugins/internal.py| 10 ++
 3 files changed, 25 insertions(+), 19 deletions(-)

diff --git a/install/ui/src/freeipa/certificate.js b/install/ui/src/freeipa/certificate.js
index ae05ebb3d45974cd1df50c16e19d0ab9fd27a19b..ec653a0a464adad80f7ad847909e13d0d025a7fc 100755
--- a/install/ui/src/freeipa/certificate.js
+++ b/install/ui/src/freeipa/certificate.js
@@ -810,17 +810,18 @@ IPA.cert.revoke_action = function(spec) {
 return that;
 };
 
-IPA.cert.restore_action = function(spec) {
+IPA.cert.remove_hold_action = function(spec) {
 
 spec = spec || {};
-spec.name = spec.name || 'restore_cert';
-spec.label = spec.label || '@i18n:objects.cert.restore_certificate_simple';
+spec.name = spec.name || 'remove_hold_cert';
+spec.label = spec.label || '@i18n:objects.cert.remove_hold';
 spec.enable_cond = spec.enable_cond || ['has_certificate', 'certificate_hold'];
 spec.hide_cond = spec.hide_cond || ['ra_disabled'];
-spec.confirm_msg = spec.confirm_msg || '@i18n:objects.cert.restore_confirmation';
+spec.confirm_msg = spec.confirm_msg ||
+'@i18n:objects.cert.remove_certificate_hold_confirmation';
 spec.confirm_dialog = spec.confirm_dialog || {
 $factory: IPA.confirm_dialog,
-ok_label: '@i18n:buttons.restore'
+ok_label: '@i18n:buttons.remove_hold'
 };
 spec.needs_confirm = spec.needs_confirm !== undefined ? spec.needs_confirm : true;
 
@@ -834,9 +835,10 @@ IPA.cert.restore_action = function(spec) {
 var entity_label = that.entity_label || facet.entity.metadata.label_singular;
 var entity_name = certificate.entity_info.name;
 
-var title = text.get('@i18n:objects.cert.restore_certificate_simple');
+var title = text.get('@i18n:objects.cert.remove_certificate_hold_simple');
+
 if (entity_name && entity_label) {
-title = text.get('@i18n:objects.cert.restore_certificate');
+title = text.get('@i18n:objects.cert.remove_certificate_hold');
 title = title.replace('${entity}', entity_label);
 title = title.replace('${primary_key}', entity_name);
 }
@@ -855,7 +857,7 @@ IPA.cert.restore_action = function(spec) {
 args: [certificate.serial_number],
 on_success: function(data, text_status, xhr) {
 facet.refresh();
-IPA.notify_success('@i18n:objects.cert.restored');
+IPA.notify_success('@i18n:objects.cert.hold_removed');
 facet.certificate_updated.notify([], that.facet);
 }
 }).execute();
@@ -1334,14 +1336,14 @@ return {
 disable_facet_tabs: true,
 actions: [
 'cert_revoke',
-'cert_restore'
+'cert_remove_hold'
 ],
 state: {
 evaluators: [
 IPA.cert.certificate_evaluator
 ]
 },
-header_actions: ['revoke_cert', 'restore_cert'],
+header_actions: ['revoke_cert', 'remove_hold_cert'],
 sections: [
 {
 name: 'details',
@@ -1529,7 +1531,7 @@ exp.register = function() {
 a.register('cert_get', IPA.cert.get_action);
 a.register('cert_request', IPA.cert.request_action);
 a.register('cert_revoke', IPA.cert.revoke_action);
-a.register('cert_restore', IPA.cert.restore_action);
+a.register('cert_remove_hold', IPA.cert.remove_hold_action);
 
 e.register({type: 'cert', spec: exp.entity_spec});
 };
diff --git a/install/ui/test/data/ipa_init.json b/install/ui/test/data/ipa_init.json
index 1b9b69ff909a9668c1e1867008459d25d5e062a9..a2f23db206651d52440c47d8b1d16164c330b1e9 100644
--- a/install/ui/test/data/ipa_init.json
+++ b/install/ui/test/data/ipa_init.json
@@ -79,6 +79,7 @@
 "refresh": "Refresh",
 "refresh_title": "Reload current settings from the server.",
 "remove": "Delete",
+"remove_hold": "Remove hold",
 "reset": "Reset",
 "reset_password_and_login": "Reset Password and Login",
 "restore": "Restore",
@@ -245,6 +246,7 @@
 "find_validnotbefo

[Freeipa-devel] [PATCH] 0001 Add missing CA options to the manpage for ipa-replica-install

2016-05-20 Thread Florence Blanc-Renaud 

Hi all,

this one will be my first patch submission, so apologies in advance if I 
make mistakes...


The man page for ipa-replica-install was missing some commands related 
to CA-less installation, as well as --allow-zone-overlap and 
--auto-reverse. I added them in the relevant sections (CERTIFICATE 
SYSTEM OPTIONS and DNS OPTIONS).


I also fixed a wrong short option for --realm (-r).

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

From e2ec0bb1570caec67df58d259be63ac7191e24b0 Mon Sep 17 00:00:00 2001
From: Florence Blanc-Renaud 
Date: Fri, 20 May 2016 12:09:06 +0200
Subject: [PATCH] Add missing CA options to the manpage for ipa-replica-install

CA-less options were missing, as well as --allow-zone-overlap
and --auto-reverse.
Fix short option for --realm which was displayed as -d instead of -r.

https://fedorahosted.org/freeipa/ticket/5835
---
 install/tools/man/ipa-replica-install.1 | 35 -
 1 file changed, 34 insertions(+), 1 deletion(-)

diff --git a/install/tools/man/ipa-replica-install.1 b/install/tools/man/ipa-replica-install.1
index 0e9f51a645afeac63b0c87418cd1468db951c4ef..55bae2cb77e1a1c520e0598983b8939a919a9ee9 100644
--- a/install/tools/man/ipa-replica-install.1
+++ b/install/tools/man/ipa-replica-install.1
@@ -69,7 +69,7 @@ The fully qualified domain name of the IPA server to enroll to.
 \fB\-n\fR, \fB\-\-domain\fR=\fIDOMAIN\fR
 Set the domain name to DOMAIN.
 .TP
-\fB\-d\fR, \fB\-\-realm\fR=\fIREALM_NAME\fR
+\fB\-r\fR, \fB\-\-realm\fR=\fIREALM_NAME\fR
 Set the IPA realm name to REALM_NAME.
 .TP
 \fB\-\-hostname\fR
@@ -128,6 +128,33 @@ certificate operations will be forwarded to a master with a CA installed.
 \fB\-\-no\-pkinit\fR
 Disables pkinit setup steps
 .TP
+\fB\-\-dirsrv\-cert\-file\fR=FILE
+File containing the Directory Server SSL certificate and private key
+.TP
+\fB\-\-http\-cert\-file\fR=FILE
+File containing the Apache Server SSL certificate and private key
+.TP
+\fB\-\-pkinit\-cert\-file\fR=FILE
+File containing the Kerberos KDC SSL certificate and private key
+.TP
+\fB\-\-dirsrv\-pin\fR=PIN
+The password to unlock the Directory Server private key
+.TP
+\fB\-\-http\-pin\fR=PIN
+The password to unlock the Apache Server private key
+.TP
+\fB\-\-pkinit\-pin\fR=PIN
+The password to unlock the Kerberos KDC private key
+.TP
+\fB\-\-dirsrv\-cert\-name\fR=NAME
+Name of the Directory Server SSL certificate to install
+.TP
+\fB\-\-http\-cert\-name\fR=NAME
+Name of the Apache Server SSL certificate to install
+.TP
+\fB\-\-pkinit\-cert\-name\fR=NAME
+Name of the Kerberos KDC SSL certificate to install
+.TP
 \fB\-\-skip\-schema\-check\fR
 Skip check for updated CA DS schema on the remote master
 
@@ -161,6 +188,12 @@ The reverse DNS zone to use. This option can be used multiple times to specify m
 \fB\-\-no\-reverse\fR
 Do not create new reverse DNS zone. If a reverse DNS zone already exists for the subnet, it will be used.
 .TP
+\fB\-\-auto-reverse\fR
+Create necessary reverse zones
+.TP
+\fB\-\-allow-zone-overlap\fR
+Create DNS zone even if it already exists
+.TP
 \fB\-\-no\-host\-dns\fR
 Do not use DNS for hostname lookup during installation
 .TP
-- 
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 0099] ipa-nis-manage: add status option

2016-05-20 Thread Abhijeet Kasurde 



On 05/20/2016 04:02 PM, Petr Spacek wrote:

On 12.5.2016 16:17, Petr Spacek wrote:

On 28.4.2016 17:15, Petr Spacek wrote:

On 28.4.2016 14:52, Abhijeet Kasurde wrote:

Hi Petr,

On 04/25/2016 08:28 PM, Petr Spacek wrote:

Hello,

ipa-nis-manage: add status option

https://bugzilla.redhat.com/show_bug.cgi?id=1329275




Can you reword the error message here as well ?

  if len(args) != 1:
  sys.exit("You must specify one action, either enable or disable")

Thanks,
Abhijeet Kasurde

Good catch!

Please review this, thanks.

Ping, please review it.


LGTM. But someone else should approve it.

--
Thanks,
Abhijeet Kasurde

IRC: akasurde
http://akasurde.github.io

--
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] [DESIGN] IPA client in AD DNS domain

2016-05-20 Thread Alexander Bokovoy 

On Fri, 20 May 2016, Petr Spacek wrote:

Theory I have seen looks good to me but Security considerations section is
missing. The design must spell out what are security implications of
 ignore_acceptor_hostname = true
 GSSAPIStrictAcceptorCheck no

I'll add security considerations, thanks for noticing that.


All of the implementation details are missing so this review cannot be
considered complete.

There is nothing to implement here on our side. We discussed it with
Martin K. and he suggested that we might add a link to documentation
when it would be written but that's as much as we can do.

Thing is, a proper implementation means changes to be done way above
ipa-client-install level, even way above FreeIPA deployment itself,
especially for SSO case where a CNAME would need to be created in a
separate DNS zone that is not under control of FreeIPA. So all we can do
is to suggest something rather than implement. We do that already and
Mark is going to turn the design into a section in the Windows
Integration Guide.


I'm very interested in implementation details & usability of it. Can we make
this setup easier to achieve by changing ipa-client-install?

Some ideas:
- populate krb.conf only with
[domain_realm]
canonical hostname = IPA.EXAMPLE.COM

and enable DNS auto-detection for everything else.

We already have auto-detection working. For non-SSO case
'ipa-client-install --domain=ipa.example.com' on ipa-client.example.com
will automatically configure everything. The only change is indeed by
setting 'ipa-client.example.com = IPA.EXAMPLE.COM'. For SSO case we
simply discover that AD DC is not IPA LDAP server so we refuse to
operate unless you provide manual options. However, it is impossible to
do anything here automatically because the actual behavior would depend
on external conditions which we cannot control.

This is really something that has to be written in the planning guide.
For example, if you have SSO case and want to put A/ record and
CNAME record, it is not a given fact that both of them have to be named
in the same way. In fact, they most likely will be different as CNAME
record is part of user-facing application namespace and A/ records
in IPA DNS zone are part of a backend naming. There is no
standardization here.



I think that:
a) For normal setups with disjoint domains this should just work as usual.

b) For setup without CNAME for IPA client it should work because example.com
will be detected as AD domain and scenario described in the section 'No single
sign-on required' will work.

c) For setup with CNAME the user will need to add ignore_acceptor_hostname but
krb5.conf will be configured properly.



BTW why is it needed to use ignore_acceptor_hostname if there is a CNAME? MIT
Kerberos should see the correct name as it detects CNAMEs. Does AD ignore the
CNAME when requesting a ticket? What else?

No, it does not ignore, it resolves CNAME to A/ name. However, there
are cases when people want to have both principals from IPA and AD
realms in /etc/krb5.keytab and want to support both ways to access it.


If it is needed, can we detect the CNAME and turn on ignore_acceptor_hostname
automatically? (This depends on security considerations section, of course.)

Exactly because it is part of the behavior defined by application
frontend considerations, we can only document it and not do automated
handling.


Speaking of certs, should we introduce a aliases for host entries to avoid the
need of fake hosts?

These 'fake hosts' are as good as aliases, even better, because they
allow us to have full control over who can manage them.

--
/ 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 0099] ipa-nis-manage: add status option

2016-05-20 Thread Petr Spacek 
On 12.5.2016 16:17, Petr Spacek wrote:
> On 28.4.2016 17:15, Petr Spacek wrote:
>> On 28.4.2016 14:52, Abhijeet Kasurde wrote:
>>> Hi Petr,
>>>
>>> On 04/25/2016 08:28 PM, Petr Spacek wrote:
 Hello,

 ipa-nis-manage: add status option

 https://bugzilla.redhat.com/show_bug.cgi?id=1329275



>>> Can you reword the error message here as well ?
>>>
>>>  if len(args) != 1:
>>>  sys.exit("You must specify one action, either enable or disable")
>>>
>>> Thanks,
>>> Abhijeet Kasurde
>>
>> Good catch!
> 
> Please review this, thanks.

Ping, please review it.

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

Re: [Freeipa-devel] [PATCH 0473-0476]DNS Locations: Prologue

2016-05-20 Thread Petr Spacek 
On 18.5.2016 12:43, Martin Basti wrote:
> 
> 
> On 12.05.2016 16:16, Martin Basti wrote:
>>
>>
>>
>> On 12.05.2016 11:01, Martin Basti wrote:
>>>
>>>
>>> On 11.05.2016 09:41, Martin Basti wrote:


 On 10.05.2016 18:56, Petr Spacek wrote:
> On 10.5.2016 15:38, Petr Spacek wrote:
>> On 10.5.2016 15:26, Martin Basti wrote:
>>>
>>> On 10.05.2016 15:23, Petr Spacek wrote:
 On 10.5.2016 14:44, Martin Basti wrote:
> On 10.05.2016 14:33, Petr Spacek wrote:
>> On 6.5.2016 10:20, Martin Basti wrote:
>>> https://fedorahosted.org/freeipa/ticket/2008
>>>
>>> Patches attached.
>>>
>>>
>>> freeipa-mbasti-0473-DNS-Locations-Always-create-DNS-related-privileges.patch
>>>
>>>
>>>
>>>From 9a936740da7cdacec150acc92a45041a98ce7cb3 Mon Sep 17
>>> 00:00:00 2001
>>> From: Martin Basti 
>>> Date: Wed, 4 May 2016 17:33:52 +0200
>>> Subject: [PATCH 1/4] DNS Locations: Always create DNS related
>>> privileges
>>>
>>> DNS privileges are important for handling DNS locations which can be
>>> created without DNS servers in IPA topology. We will also need this
>>> privileges presented for future feature 'External DNS support'
>> Seems reasonable, ACK.
>>
>>
>>> freeipa-mbasti-0474-DNS-Locations-add-new-attributes-and-objectclasses.patch
>>>
>>>
>>>
>>>From a7766da5fd1a72884308d4206c9cde262f5c8d35 Mon Sep 17
>>> 00:00:00 2001
>>> From: Martin Basti 
>>> Date: Thu, 5 May 2016 11:12:00 +0200
>>> Subject: [PATCH 2/4] DNS Locations: add new attributes and
>>> objectclasses
>>>
>>> http://www.freeipa.org/page/V4/DNS_Location_Mechanism
>>>
>>> https://fedorahosted.org/freeipa/ticket/2008
>>> ---
>>> install/share/60ipadns.ldif | 4 
>>> 1 file changed, 4 insertions(+)
>>>
>>> diff --git a/install/share/60ipadns.ldif 
>>> b/install/share/60ipadns.ldif
>>> index
>>> e0ed0ab869cea0478d9640bb509c6267abed1a01..31c2f71f8566d04a05709f1359b20e6fa51921ce
>>>
>>>
>>> 100644
>>> --- a/install/share/60ipadns.ldif
>>> +++ b/install/share/60ipadns.ldif
>>> @@ -70,9 +70,13 @@ attributeTypes: ( 2.16.840.1.113730.3.8.5.25 NAME
>>> 'idnsSecKeyRevoke' DESC 'DNSKE
>>> attributeTypes: ( 2.16.840.1.113730.3.8.5.26 NAME
>>> 'idnsSecKeySep' DESC
>>> 'DNSKEY SEP flag (equivalent to bit 15): RFC 4035' EQUALITY
>>> booleanMatch
>>> SYNTAX 1.3.6.1.4.1.1466.115.121.1.7 SINGLE-VALUE X-ORIGIN 'IPA 
>>> v4.1' )
>>> attributeTypes: ( 2.16.840.1.113730.3.8.5.27 NAME
>>> 'idnsSecAlgorithm' DESC
>>> 'DNSKEY algorithm: string used as mnemonic' EQUALITY
>>> caseIgnoreIA5Match
>>> SUBSTR caseIgnoreIA5SubstringsMatch SYNTAX
>>> 1.3.6.1.4.1.1466.115.121.1.26
>>> SINGLE-VALUE X-ORIGIN 'IPA v4.1' )
>>> attributeTypes: ( 2.16.840.1.113730.3.8.5.28 NAME
>>> 'idnsSecKeyRef' DESC
>>> 'PKCS#11 URI of the key' EQUALITY caseExactMatch SINGLE-VALUE SYNTAX
>>> 1.3.6.1.4.1.1466.115.121.1.15 X-ORIGIN 'IPA v4.1' )
>>> +attributeTypes: ( 2.16.840.1.113730.3.8.5.32 NAME 'ipaLocation' 
>>> DESC
>>> 'Reference to IPA location' EQUALITY distinguishedNameMatch SYNTAX
>>> 1.3.6.1.4.1.1466.115.121.1.12 SINGLE-VALUE X-ORIGIN 'IPA v4.4' )
>>> +attributeTypes: ( 2.16.840.1.113730.3.8.5.33 NAME
>>> 'ipaLocationWeight' DESC
>>> 'Weight for the server in IPA location' EQUALITY integerMatch SYNTAX
>>> 1.3.6.1.4.1.1466.115.121.1.27 SINGLE-VALUE X-ORIGIN 'IPA v4.4' )
>>> objectClasses: ( 2.16.840.1.113730.3.8.6.0 NAME 'idnsRecord'
>>> DESC 'dns
>>> Record, usually a host' SUP top STRUCTURAL MUST idnsName MAY ( cn $
>>> idnsAllowDynUpdate $ dNSTTL $ dNSClass $ aRecord $ aAAARecord $
>>> a6Record $
>>> nSRecord $ cNAMERecord $ pTRRecord $ sRVRecord $ tXTRecord $
>>> mXRecord $
>>> mDRecord $ hInfoRecord $ mInfoRecord $ aFSDBRecord $ SigRecord $
>>> KeyRecord
>>> $ LocRecord $ nXTRecord $ nAPTRRecord $ kXRecord $ certRecord $
>>> dNameRecord
>>> $ dSRecord $ sSHFPRecord $ rRSIGRecord $ nSECRecord $ DLVRecord $
>>> TLSARecord $ UnknownRecord $ RPRecord $ APLRecord $ IPSECKEYRecord $
>>> DHCIDRecord $ HIPRecord $ SPFRecord ) )
>>> objectClasses: ( 2.16.840.1.113730.3.8.6.1 NAME 'idnsZone' DESC
>>> 'Zone
>>> class' SUP idnsRecord STRUCTURAL MUST ( idnsZoneActive $
>>> idnsSOAmName $
>>> idnsSOArName $ idnsSOAserial $ idnsSOArefresh $ idnsSOAretry $
>>> idnsSOAexpire $ idnsSOAmini

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

2016-05-20 Thread Petr Spacek 
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
> 
> 10)
> -return
> +return False, []
> This should be fixed in the previous commit
> 
> PATCH * DNS upgrade: change global forwarding policy in named.conf to "only"
> if private IPs are used *
> 11)
> IMO this is an upgrade of configuration and this should be in
> ipaserver/install/server/upgrade.py, upgrade plugins are used only for
> upda

Re: [Freeipa-devel] [PATCH] 0002 Incorrect message when KRA already installed

2016-05-20 Thread Petr Spacek 
On 17.5.2016 10:54, Patrice Duc-Jacquet wrote:
> Hi everyone
> 
> Please see attached candidate patch for ticket
> 
> https://fedorahosted.org/freeipa/ticket/5315

ACK, please add link to the ticket to commit message before pushing.

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

Re: [Freeipa-devel] [DESIGN] IPA client in AD DNS domain

2016-05-20 Thread Petr Spacek 
On 19.5.2016 22:39, Alexander Bokovoy wrote:
> Hi,
> 
> A new design page is ready for review:
> http://www.freeipa.org/page/V4/IPA_Client_in_Active_Directory_DNS_domain
> 
> Below is the text for convenience. I did test both scenarios
> successfully.
> 
> Single sign-on scenario:
> - Client has A/ record in IDM DNS domain and CNAME record in AD DNS
>  domain
> - SSO with Kerberos is possible from Windows machines
> 
> Non-single sign-on scenario:
> - Client has A/ record in AD DNS domain
> - No SSO with Kerberos is possible from Windows machines
> 
> 
> {{Feature|version=4.4.0|ticket=5762|author=Ab}}
> 
> == Overview ==
> In the ideal world, FreeIPA clients should be deployed in DNS zones
> owned by FreeIPA. However, in many environments where FreeIPA is being
> deployed, Active Directory is the dominant identity management solution
> owning not only the identities, but also the DNS domains. Currently, the
> only solution how to migrate a Linux client in such AD owned DNS domain
> to FreeIPA was to move it to FreeIPA owned domain. While this procedure
> works well when migrating 10 client systems, it is less desirable when
> migrating 10k client systems.
> 
> == Use Cases ==
> === User Story ===
> As an Administrator with a big number of Linux machines in a DNS domain
> controlled by Active Directory, I want to join them to the IdM Server so
> that they can benefit from it’s Linux focused features.
> 
> === Details ===
> Allow FreeIPA client to respond a host name in a DNS domain belonging to
> a domain from a trusted Active Directory forest.
> 
> If Active Directory forest example.com uses DNS zone ''example.com'',
> and FreeIPA is deployed at ''ipa.example.com'', then it is desirable to
> have some FreeIPA machines accessible as machine-foo.example.com, not
> just ''machine-foo.ipa.example.com''.
> 
> In many cases FreeIPA client machines are used as servers for hosting
> applications in the same DNS name space as existing Active Directory
> environment. While Active Directory enforces ownership of resources (DNS
> domain is owned by corresponding Active Directory domain) and FreeIPA
> cannot be part of the Active Directory forest by itself, it should be
> possible to have a DNS host name for FreeIPA client as a part of a DNS
> domain of existing Active Directory domain and still allow single
> sign-on operations.
> 
> == Theory and practice of a forest trust interaction ==
> 
> There are several concepts needs to be understood for the setup when
> FreeIPA machine is a part of the DNS zone belonging to the domain of
> Active Directory forest:
> 
> * Active Directory has a concept of relationship between the domain and
>  DNS zones. An Active Directory domain ''owns'' DNS zone of the same
>  name and no  other Active Directory domain may claim the same DNS
>  zone.
> 
> * When forest trust is established between FreeIPA and an Active
>  Directory  forest, Active Directory Domain Controller enforces
>  non-conflict check of  the DNS name spaces claimed by FreeIPA. If
>  there is any conflict between  what FreeIPA claims to own and what the
>  Active Directory Domain Controller  knows to belong to any of the
>  Active Directory domain in the same forest, a  link between FreeIPA
>  and AD would be disabled and no authentication would be  possible
>  across the trust link.
> 
> * FreeIPA automatically adds DNS domains it manages to the list of DNS
>  domains  associated with FreeIPA realm. This list is then presented to
>  Active  Directory when trust is established to allow proper routing of
>  authentication requests when talking to servers in these DNS domains.
> 
> As a consequence, Active Directory will never refer authentication request to
> FreeIPA domain controller for a Kerberos service principal on a host within
> DNS zone owned by Active Directory. This means no Kerberos authentication is
> possible against such FreeIPA machines from Windows systems.
> 
> When FreeIPA client ''ipa-client.ipa.domain'' is enrolled into FreeIPA
> realm, following is done:
> 
> # Host object ''ipa-client.ipa.domain'' is created in FreeIPA to hold
> references to the new FreeIPA client
> # Kerberos principal is created based on the host object,
> ''host/ipa-client.ipa.domain@IPA.DOMAIN''
> # Keys for this principal retrieved and stored in ''/etc/krb5.keytab'' on the
> FreeIPA client
> # Kerberos configuration is added to ''/etc/krb5.conf'' to refer
> ''IPA.DOMAIN'' Kerberos realm to FreeIPA KDC and map ''ipa.domain'' DNS zone
> to ''IPA.DOMAIN'' Kerberos realm
> # SSSD daemon on FreeIPA will attempt to update DNS record for 
> ''ipa-client.ipa.domain'' using the host Kerberos principal created above.
> 
> Host object in FreeIPA is decoupled from the corresponding DNS record.
> Creating the host object with host name from non-FreeIPA DNS zone does
> not cause adding that DNS zone to the list of DNS zones associated with
> FreeIPA realm.
> 
> The way how Kerberos configuration for FreeIPA realm in ''/etc/krb5

[Freeipa-devel] Karma Request for JSS 4.2.6-39 on Fedora 24

2016-05-20 Thread Christina Fu 
The following candidate builds of JSS 4.2.6-39 for Fedora 24 (final) 
consist of the following:
jss-4.2.6-39.fc24 



Please provide Karma for these builds in Bodhi located at:
https://bodhi.fedoraproject.org/updates/FEDORA-2016-d3c8f022c5

Additionally, the following builds have been provided for Fedora 25 
(rawhide):
jss-4.2.6-39.fc25 



Thanks,
Christina
-- 
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] Karma Request for JSS 4.2.6-39 on Fedora 24

2016-05-20 Thread Christina Fu 
Had to respin due to small piece of incompatible code that failed to 
compile on rhel7.


The following candidate builds of JSS 4.2.6-39 for Fedora 24 (final) 
consist of the following:
jss-4.2.6-40.fc24 



Please provide Karma for these builds in Bodhi located at:

https://bodhi.fedoraproject.org/updates/FEDORA-2016-c036afbe30

Additionally, the following builds have been provided for Fedora 
25(rawhide):
jss-4.2.6-40.fc25 



Thanks,
Christina

On 05/19/2016 02:38 PM, Christina Fu wrote:
The following candidate builds of JSS 4.2.6-39 for Fedora 24 (final) 
consist of the following:
jss-4.2.6-39.fc24 



Please provide Karma for these builds in Bodhi located at:
https://bodhi.fedoraproject.org/updates/FEDORA-2016-d3c8f022c5

Additionally, the following builds have been provided for Fedora 25 
(rawhide):
jss-4.2.6-39.fc25 



Thanks,
Christina


-- 
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 0112] pylint: replace Refactor category with individual check name

2016-05-20 Thread Petr Spacek 
On 19.5.2016 14:47, Martin Basti wrote:
> On 19.05.2016 14:26, Petr Spacek wrote:
>> Hello,
>>
>> pylint: replace Refactor category with individual check names
>>
>> This eases enabling/disabling individual tests like cyclic-import.
>>
> 
> I like this patch but, NACK
> 
> .
> * Module ipalib.config
> ipalib/config.py:260: [R0204(redefined-variable-type), Env.__setitem__]
> Redefinition of value type from int to ipapython.dn.DN)
> ipalib/config.py:458: [R0102(simplifiable-if-statement), Env._bootstrap] The
> if statement can be replaced with 'var = bool(test)')
> * Module ipalib.messages
> ipalib/messages.py:90: [R0204(redefined-variable-type),
> process_message_arguments] Redefinition of obj.strerror type from unicode to 
> str)
> * Module ipalib.plugable
> ipalib/plugable.py:569: [R0204(redefined-variable-type), API.import_plugins]
> Redefinition of modules type from generator to list)
> * Module ipalib.rpc
> ipalib/rpc.py:609: [R0101(too-many-nested-blocks),
> KerbTransport.single_request] Too many nested blocks (6/5))
> ipalib/rpc.py:753: [R0204(redefined-variable-type), RPCClient.get_url_list]
> Redefinition of answers type from dns.resolver.Answer to list)
> 
> 
> 
> tested with pylint-1.5.5-1.fc24.noarch

Here it is.

-- 
Petr^2 Spacek
From a1679b9b7256db91dc47f41c1eed07e91224a836 Mon Sep 17 00:00:00 2001
From: Petr Spacek 
Date: Thu, 19 May 2016 14:25:31 +0200
Subject: [PATCH] pylint: replace Refactor category with individual check names

This eases enabling/disabling individual tests like cyclic-import.
---
 pylintrc | 19 ++-
 1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/pylintrc b/pylintrc
index 7ef441844ef0051ddbdec348236e6fdc308f79b6..bb9c636867e1a89afa689cb4bbb901411cd3cf7a 100644
--- a/pylintrc
+++ b/pylintrc
@@ -16,8 +16,25 @@ enable=
 python3
 
 disable=
-R,
 I,
+cyclic-import,
+duplicate-code,
+import-error,
+interface-not-implemented,
+no-self-use,
+redefined-variable-type,
+simplifiable-if-statement,
+too-few-public-methods,
+too-many-ancestors,
+too-many-arguments,
+too-many-boolean-expressions,
+too-many-branches,
+too-many-instance-attributes,
+too-many-locals,
+too-many-nested-blocks,
+too-many-public-methods,
+too-many-return-statements,
+too-many-statements,
 import-error,
 abstract-method,
 anomalous-backslash-in-string,
-- 
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