Re: [Freeipa-devel] [DESIGN] Lightweight CA renewal

[Fraser Tweedale]

On Tue, Jun 21, 2016 at 07:29:22AM +0200, Jan Cholasta wrote:
> On 18.6.2016 02:38, Fraser Tweedale wrote:
> > On Fri, Jun 17, 2016 at 03:21:07PM +0200, Jan Cholasta wrote:
> > > On 17.6.2016 09:34, Fraser Tweedale wrote:
> > > > On Mon, May 09, 2016 at 09:35:06AM +0200, Jan Cholasta wrote:
> > > > > Hi,
> > > > > 
> > > > > On 6.5.2016 08:01, Fraser Tweedale wrote:
> > > > > > Hullo all,
> > > > > > 
> > > > > > FreeIPA Lightweight CAs implementation is progressing well.  The
> > > > > > remaining big unknown in the design is how to do renewal.  I have
> > > > > > put my ideas into the design page[1] and would appreciate any and
> > > > > > all feedback!
> > > > > > 
> > > > > > [1] http://www.freeipa.org/page/V4/Sub-CAs#Renewal
> > > > > > 
> > > > > > Some brief commentary on the options:
> > > > > > 
> > > > > > I intend to implement approach (1) as a baseline.  Apart from
> > > > > > implementing machinery in Dogtag to actually perform the renewal -
> > > > > > which is required for all the approaches - it's not much work and
> > > > > > gets us over the "lightweight CAs can be renewed easily" line, even
> > > > > > if it is a manual process.
> > > > > > 
> > > > > > For automatic renewal, I am leaning towards approach (2).  Dogtag
> > > > > > owns the lightweight CAs so I think it makes sense to give Dogtag
> > > > > > the ability to renew them automatically (if configured to do so),
> > > > > > without relying on external tools i.e. Certmonger.  But as you will
> > > > > > see from the outlines, each approach has its upside and downside.
> > > > > 
> > > > > I would prefer (3), as I would very much like to avoid duplicating
> > > > > certmonger's functionality in Dogtag.
> > > > > 
> > > > > Some comments on the disadvantages:
> > > > > 
> > > > >   * "Proliferation of Certmonger tracking requests; one for each
> > > > > FreeIPA-managed lightweight CA."
> > > > > 
> > > > > I don't think this is an actual issue, as it's purely cosmetic.
> > > > > 
> > > > >   * "Either lightweight CA creation is restricted to the renewal 
> > > > > master, or
> > > > > the renewal master must observe the creation of new lightweight CAs 
> > > > > and
> > > > > start tracking their certificate."
> > > > > 
> > > > > IMO this doesn't have to be done automatically in the initial
> > > > > implementation. You could extend ipa-certupdate to set up certmonger 
> > > > > for
> > > > > lightweight CAs and have admins run it manually on masters after 
> > > > > adding a
> > > > > new lightweight CA. They will have to run it anyway to get the new
> > > > > lightweight CA certificate installed in the system, so it should be 
> > > > > fine to
> > > > > do it this way.
> > > > > 
> > > > I have updated the renew_ca_cert post-save script to perform the
> > > > database update necessary for CA replicas to pick up the new cert.
> > > > What remains is the command to tell certmonger to track the CA.
> > > > 
> > > > You mentioned ipa-certupdate but perhaps ipa-cacert-manage is a
> > > > better fit, e.g.:
> > > > 
> > > > ipa-cacert-manage track 
> > > > 
> > > > It would look up the necessary info (basically just the CA-ID) and
> > > > set up the certmonger tracking.
> > > 
> > > No. ipa-cacert-manage updates global configuration in LDAP, whereas
> > > ipa-certupdate applies the global configuration on the local system.
> > > Updating certmonger configuration is the latter, hence it should be done 
> > > in
> > > ipa-certupdate.
> > > 
> > > Also, I don't think we should expose (un)tracking certificates by CA ID to
> > > users, as all our CA certificates should always be tracked.
> > > 
> > OK, so ipa-certupdate just gets run without arguments on a CA
> > master, and it ensures that all CA certificates are tracked by
> > Certmonger.
> 
> Right.
> 
> > 
> > Makes sense to me.  Thanks for the clarifications.
> > 
> > > > 
> > > > It could be an error to run the command on other than the renewal
> > > > master.
> > > 
> > > Note that the main CA certificate is tracked on all CA servers, not just 
> > > the
> > > renewal master, otherwise it wouldn't get updated on the other CA servers
> > > after renewal. I would think the same needs to be done for lightweight CA
> > > certificates, unless there is a different mechanism for distributing the
> > > certificates to other CA masters, in which case I would prefer if the
> > > mechanism was also used for the main CA certificate.
> > > 
> > There is a different mechanism that causes other CA masters to
> > update their NSSDBs with the new certificate.  After the renewal is
> > performed, the authoritySerial attribute is updated in the
> > authority's entry in Dogtag's database; other CA replicas replicas
> > notice the update and install the new cert in their own NSSDBs
> > without requiring restart (thus, we only need to track LWCA certs on
> > the renewal master).
> > 
> > The mechanism is available on versions of Dogtag that support
> > lightweight CAs, with the 

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

[Jan Cholasta]

On 20.6.2016 19:56, Martin Basti wrote:



On 20.06.2016 18:48, Martin Basti wrote:



On 20.06.2016 16:42, Jan Cholasta wrote:

On 20.6.2016 16:13, David Kupka wrote:

On 28/04/16 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



Hello!

Patches:
replica install: fix thin client regression
schema: remove `no_cli` from command schema
schema: remove redundant information
schema: merge command args and options
schema: remove output_params
schema: add object class schema
permission: handle ipapermright deprecated CLI alias on the client
passwd: handle sort order of passwd argument on the client
misc: skip `count` and `total` output in env.output_for_cli
dns: do not rely on custom param fields in record attributes
automember: add object plugin for automember_rebuild
frontend: do not crash on missing output in output_for_cli
frontend: skip `value` output in output_for_cli
frontend: don't copy command arguments to output params
makeaci, makeapi: use in-server API

work for me, ACK.


Pushed to master: 8cc8b6fb1023fa4aeafac0df01cfacff4ebf537a

Note that I haven't pushed "replica install: fix thin client
regression" yet, I would like Martin to review it first.


ACK

pushed to master:
* 91d6d87ca76e3aa27d5f87fd4f0b70f1d4fe4e72 replica install: fix thin
client regression



Attaching the patches for reference.



Note: There is one one known issue in automember output, will be fixed
later.








Guys, I suspect that one of previously pushed patches breaks following
command on the client side

# ipa dns-update-system-records
ipa: ERROR: KeyError: 'ipa_records'
Traceback (most recent call last):
  File "/usr/lib/python2.7/site-packages/ipalib/cli.py", line 1346, in run
sys.exit(api.Backend.cli.run(argv))
  File "/usr/lib/python2.7/site-packages/ipalib/cli.py", line , in run
rv = cmd.output_for_cli(self.api.Backend.textui, result, *args,
**options)
  File "/usr/lib/python2.7/site-packages/ipaclient/plugins/dns.py", line
367, in output_for_cli
textui.print_indented(u'{}:'.format(labels[key]), indent=1)
KeyError: 'ipa_records'
ipa: ERROR: an internal error has occurred


Sorry. Fixed.

--
Jan Cholasta
From a1ae77ff2caf4daa87a0f1fdd4bc5e75bb8c3aae Mon Sep 17 00:00:00 2001
From: Jan Cholasta 
Date: Tue, 21 Jun 2016 07:47:52 +0200
Subject: [PATCH] dns: fix dns_update_system_records to work with thin client

https://fedorahosted.org/freeipa/ticket/2008
https://fedorahosted.org/freeipa/ticket/4739
---
 API.txt  |  6 --
 VERSION  |  4 ++--
 ipaclient/plugins/dns.py |  4 ++--
 ipaserver/plugins/dns.py | 20 
 4 files changed, 20 insertions(+), 14 deletions(-)

diff --git a/API.txt b/API.txt
index f2a0686..558be34 100644
--- a/API.txt
+++ b/API.txt
@@ -1068,10 +1068,12 @@ output: Output('result', type=[])
 output: Output('summary', type=[, ])
 output: Output('value', type=[])
 command: dns_update_system_records
-args: 0,2,2
+args: 0,4,2
+option: Flag('all', autofill=True, cli_name='all', default=False)
 option: Flag('dry_run', autofill=True, default=False)
+option: Flag('raw', autofill=True, cli_name='raw', default=False)
 option: Str('version?')
-output: Output('result', type=[])
+output: Entry('result')
 output: Output('value', type=[])
 command: dnsconfig_mod
 args: 0,11,3
diff --git a/VERSION b/VERSION
index faf10e3..76b83ee 100644
--- a/VERSION
+++ b/VERSION
@@ -90,5 +90,5 @@ IPA_DATA_VERSION=2010061412
 #  #
 
 IPA_API_VERSION_MAJOR=2
-IPA_API_VERSION_MINOR=193
-# Last change: schema: remove `no_cli` from command schema
+IPA_API_VERSION_MINOR=194
+# Last change: dns: fix dns_update_system_records to work with thin client
diff --git a/ipaclient/plugins/dns.py b/ipaclient/plugins/dns.py
index d04f686..bca5ad7 100644
--- a/ipaclient/plugins/dns.py
+++ b/ipaclient/plugins/dns.py
@@ -23,7 +23,7 @@ from __future__ import print_function
 import six
 import copy
 
-from ipaclient.frontend import MethodOverride, CommandOverride
+from ipaclient.frontend import MethodOverride
 from ipalib import errors
 from ipalib.dns import (get_part_rrtype,
 get_record_rrtype,
@@ -347,7 +347,7 @@ class dnsforwardzone_mod(MethodOverride):
 
 
 @register(override=True)
-class dns_update_system_records(CommandOverride):
+class dns_update_system_records(MethodOverride):
 def output_for_cli(self, textui, output, *args, **options):
 output_super = copy.deepcopy(output)
 super_res = output_super.get('result', {})
diff --git a/ipaserver/plugins/dns.py b/ipaserver/plugins/dns.py
index 14607b3..f350450 100644
--- 

Re: [Freeipa-devel] [DESIGN] Lightweight CA renewal

[Jan Cholasta]

On 18.6.2016 02:38, Fraser Tweedale wrote:

On Fri, Jun 17, 2016 at 03:21:07PM +0200, Jan Cholasta wrote:

On 17.6.2016 09:34, Fraser Tweedale wrote:

On Mon, May 09, 2016 at 09:35:06AM +0200, Jan Cholasta wrote:

Hi,

On 6.5.2016 08:01, Fraser Tweedale wrote:

Hullo all,

FreeIPA Lightweight CAs implementation is progressing well.  The
remaining big unknown in the design is how to do renewal.  I have
put my ideas into the design page[1] and would appreciate any and
all feedback!

[1] http://www.freeipa.org/page/V4/Sub-CAs#Renewal

Some brief commentary on the options:

I intend to implement approach (1) as a baseline.  Apart from
implementing machinery in Dogtag to actually perform the renewal -
which is required for all the approaches - it's not much work and
gets us over the "lightweight CAs can be renewed easily" line, even
if it is a manual process.

For automatic renewal, I am leaning towards approach (2).  Dogtag
owns the lightweight CAs so I think it makes sense to give Dogtag
the ability to renew them automatically (if configured to do so),
without relying on external tools i.e. Certmonger.  But as you will
see from the outlines, each approach has its upside and downside.


I would prefer (3), as I would very much like to avoid duplicating
certmonger's functionality in Dogtag.

Some comments on the disadvantages:

  * "Proliferation of Certmonger tracking requests; one for each
FreeIPA-managed lightweight CA."

I don't think this is an actual issue, as it's purely cosmetic.

  * "Either lightweight CA creation is restricted to the renewal master, or
the renewal master must observe the creation of new lightweight CAs and
start tracking their certificate."

IMO this doesn't have to be done automatically in the initial
implementation. You could extend ipa-certupdate to set up certmonger for
lightweight CAs and have admins run it manually on masters after adding a
new lightweight CA. They will have to run it anyway to get the new
lightweight CA certificate installed in the system, so it should be fine to
do it this way.


I have updated the renew_ca_cert post-save script to perform the
database update necessary for CA replicas to pick up the new cert.
What remains is the command to tell certmonger to track the CA.

You mentioned ipa-certupdate but perhaps ipa-cacert-manage is a
better fit, e.g.:

ipa-cacert-manage track 

It would look up the necessary info (basically just the CA-ID) and
set up the certmonger tracking.


No. ipa-cacert-manage updates global configuration in LDAP, whereas
ipa-certupdate applies the global configuration on the local system.
Updating certmonger configuration is the latter, hence it should be done in
ipa-certupdate.

Also, I don't think we should expose (un)tracking certificates by CA ID to
users, as all our CA certificates should always be tracked.


OK, so ipa-certupdate just gets run without arguments on a CA
master, and it ensures that all CA certificates are tracked by
Certmonger.


Right.



Makes sense to me.  Thanks for the clarifications.



It could be an error to run the command on other than the renewal
master.


Note that the main CA certificate is tracked on all CA servers, not just the
renewal master, otherwise it wouldn't get updated on the other CA servers
after renewal. I would think the same needs to be done for lightweight CA
certificates, unless there is a different mechanism for distributing the
certificates to other CA masters, in which case I would prefer if the
mechanism was also used for the main CA certificate.


There is a different mechanism that causes other CA masters to
update their NSSDBs with the new certificate.  After the renewal is
performed, the authoritySerial attribute is updated in the
authority's entry in Dogtag's database; other CA replicas replicas
notice the update and install the new cert in their own NSSDBs
without requiring restart (thus, we only need to track LWCA certs on
the renewal master).

The mechanism is available on versions of Dogtag that support
lightweight CAs, with the ou=authorities container existing in
Dogtag's database.  It should work for the main CA, but I have not
tested this yet.


Sounds great! I hope it works, I would very much like to get rid of our 
thing now that Dogtag handles it.






An untrack command could also be provided.

Thoughts?


  * "Development of new Certmonger renewal helpers solely for lightweight CA
renewal."

It would be easier to extend the existing helpers. I don't think there
is anything preventing them from being used for lighweight CAs, except not
conveying the CA name, which should be easy to implement.


I would also avoid starting with (1), I don't believe it adds any real
value. IMHO the first thing that should be done is implement lightweight CA
support in certmonger (add new 'request' / 'start-tracking' option for CA
name, store it in tracking requests, pass it to CA helpers in a new
environment variable).


Honza

--
Jan Cholasta



--
Jan Cholasta

Re: [Freeipa-devel] [PATCHES 551-552, 623-624] cert: add owner information, allow search by certificate

[Jan Cholasta]

On 20.6.2016 15:31, Jan Cholasta wrote:

On 20.6.2016 09:54, Jan Cholasta wrote:

On 15.6.2016 12:33, Jan Cholasta wrote:

On 14.6.2016 11:44, Jan Cholasta wrote:

On 21.4.2016 09:11, Jan Cholasta wrote:

On 6.4.2016 15:46, Pavel Vomacka wrote:



On 03/16/2016 01:50 PM, Jan Cholasta wrote:

Hi,

the attached patches implement the server-side part of
.

Honza


Hi,

thank you for the patches. I tested them and they work well. But I
would
like to ask you whether would be possible to extend the response of
'basecert_find' method and probably also 'basecert_show' response. I
think of these information:

1) information whether the certificate is issued by our CA or not.


You can check for that by comparing the issuer name of the certificate
to "CN=Certificate Authority,$SUBJECT_BASE". You can get subject base
from config-show.



2) this probably wouldn't be possible (as we discussed), but I rather
write it too - the information about revocation reason. The same as
the
'cert_show' provides.


Added --check-revocation flag to request this information.
Currently it
works only on certificates issued by our CA.



3) MD5 and SHA1 fingerprints as the 'cert_show' method returns


Added, also included SHA-256.



Thank you again.


Updated patches attached.


Updated and rebased patches attached. Requires Fraser's sub-CA patches.


Attaching updated patch 623, which fixes these issues found by David:
.


Updated and rebased patches attached.


Attaching updated patches 552 and 623, which fix the --sizelimit option.


Updated and rebased patches attached. The --revocation-reason option now 
works as expected.


--
Jan Cholasta
From c934c8b13d663177c0fa4344738052753e38ff1d Mon Sep 17 00:00:00 2001
From: Jan Cholasta 
Date: Wed, 16 Mar 2016 13:09:11 +0100
Subject: [PATCH 1/4] ldap: fix handling of binary data in search filters

This fixes a UnicodeDecodeError when passing non-UTF-8 binary data to
LDAPClient.make_filter() and friends.

https://fedorahosted.org/freeipa/ticket/5381
---
 ipapython/ipaldap.py | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/ipapython/ipaldap.py b/ipapython/ipaldap.py
index 410ddae..23405c6 100644
--- a/ipapython/ipaldap.py
+++ b/ipapython/ipaldap.py
@@ -1211,7 +1211,12 @@ class LDAPClient(object):
 ]
 return self.combine_filters(flts, rules)
 elif value is not None:
-value = ldap.filter.escape_filter_chars(value_to_utf8(value))
+if isinstance(value, bytes):
+if six.PY3:
+value = value.decode('raw_unicode_escape')
+else:
+value = value_to_utf8(value)
+value = ldap.filter.escape_filter_chars(value)
 if not exact:
 template = '%s'
 if leading_wildcard:
-- 
2.9.0

From 452893253f32c7be01d43ce9ed76fba84fd2bab6 Mon Sep 17 00:00:00 2001
From: Jan Cholasta 
Date: Tue, 14 Jun 2016 06:29:18 +0200
Subject: [PATCH 2/4] cert: add object plugin

Implement cert as an object with methods rather than a bunch of loosely
related commands.

https://fedorahosted.org/freeipa/ticket/5381
---
 API.txt   |  60 --
 VERSION   |   4 +-
 ipaclient/plugins/cert.py |   6 +-
 ipaserver/plugins/cert.py | 522 +-
 4 files changed, 327 insertions(+), 265 deletions(-)

diff --git a/API.txt b/API.txt
index f2a0686..4d16c50 100644
--- a/API.txt
+++ b/API.txt
@@ -723,25 +723,27 @@ output: Entry('result')
 output: Output('summary', type=[, ])
 output: PrimaryKey('value')
 command: cert_find
-args: 0,19,4
+args: 1,20,4
+arg: Str('criteria?')
 option: Flag('all', autofill=True, cli_name='all', default=False)
-option: Str('cacn?', autofill=False, cli_name='ca')
+option: Str('cacn?', cli_name='ca')
 option: Flag('exactly?', autofill=True, default=False)
-option: Str('issuedon_from?', autofill=False)
-option: Str('issuedon_to?', autofill=False)
-option: Str('issuer?', autofill=False)
+option: DateTime('issuedon_from?', autofill=False)
+option: DateTime('issuedon_to?', autofill=False)
+option: DNParam('issuer?', autofill=False)
 option: Int('max_serial_number?', autofill=False)
 option: Int('min_serial_number?', autofill=False)
+option: Flag('pkey_only?', autofill=True, default=False)
 option: Flag('raw', autofill=True, cli_name='raw', default=False)
 option: Int('revocation_reason?', autofill=False)
-option: Str('revokedon_from?', autofill=False)
-option: Str('revokedon_to?', autofill=False)
-option: Int('sizelimit?', default=100)
+option: DateTime('revokedon_from?', autofill=False)
+option: DateTime('revokedon_to?', autofill=False)
+option: Int('sizelimit?')
 option: Str('subject?', autofill=False)
-option: Str('validnotafter_from?', autofill=False)
-option: Str('validnotafter_to?', autofill=False)
-option: 

Re: [Freeipa-devel] [PATCH 0532] Fix possibly undefined variable

[Martin Basti]

On 20.06.2016 20:31, Alexander Bokovoy wrote:

On Mon, 20 Jun 2016, Martin Basti wrote:

Patch attached.




From a073b44587a5b34c4f1de5742d54e7c547cd5821 Mon Sep 17 00:00:00 2001
From: Martin Basti 
Date: Mon, 20 Jun 2016 12:48:38 +0200
Subject: [PATCH] Fix possibly undefined variable in 
ipa_smb_conf_exists()


There was missing else statement what may result in undefined conf_fd
variable.
---
ipaserver/install/adtrustinstance.py | 2 ++
1 file changed, 2 insertions(+)

diff --git a/ipaserver/install/adtrustinstance.py 
b/ipaserver/install/adtrustinstance.py
index 
94474122125a59d7da8b05a13dcd6c0f20568855..6ab15df27216580d440ce72386113d6872c046b2 
100644

--- a/ipaserver/install/adtrustinstance.py
+++ b/ipaserver/install/adtrustinstance.py
@@ -81,6 +81,8 @@ def ipa_smb_conf_exists():
except IOError as err:
if err.errno == errno.ENOENT:
return False
+else:
+raise

lines = conf_fd.readlines()
conf_fd.close()
--
2.5.5


ACK, thanks!


Thanks pushed to master
master:
* fe689e9938d6bcddd848f70b2f719253c55c09d7 Fix possibly undefined 
variable in ipa_smb_conf_exists()


--
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] 0059: webui: make 'Actions' strings translatable

[Pavel Vomacka]

Hello,

please review attached patch.

--

Pavel^3 Vomacka

From ff35b4ae33714783c42751f917e2c21fd390cbd7 Mon Sep 17 00:00:00 2001
From: Pavel Vomacka 
Date: Mon, 20 Jun 2016 20:43:26 +0200
Subject: [PATCH] Make Actions string translatable

Remove hardcoded strings 'Actions ' and substitute them by strings from
translatable strings.
---
 install/ui/src/freeipa/facet.js  | 2 +-
 install/ui/src/freeipa/facets/HeaderMixin.js | 5 +++--
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/install/ui/src/freeipa/facet.js b/install/ui/src/freeipa/facet.js
index cc51a1531d875882414c0392fabff2dbf4ce315a..4553c5c65e6c334ed451e9c2f1a89ddc455d71c3 100644
--- a/install/ui/src/freeipa/facet.js
+++ b/install/ui/src/freeipa/facet.js
@@ -1112,7 +1112,7 @@ exp.facet = IPA.facet = function(spec, no_init) {
 name: 'facet_actions',
 'class': 'dropdown facet-actions',
 right_aligned: true,
-toggle_text: 'Actions ',
+toggle_text: text.get('@i18n:actions.title') + ' ',
 toggle_class: 'btn btn-default',
 toggle_icon: 'fa fa-angle-down'
 });
diff --git a/install/ui/src/freeipa/facets/HeaderMixin.js b/install/ui/src/freeipa/facets/HeaderMixin.js
index 01273129106118fa7408b6ef217193424a678503..d5dbe924ad4117f3bd492abe3c15b4281b7c1594 100644
--- a/install/ui/src/freeipa/facets/HeaderMixin.js
+++ b/install/ui/src/freeipa/facets/HeaderMixin.js
@@ -9,10 +9,11 @@ define(['dojo/_base/declare',
 'dojo/dom-class',
 '../builder',
 '../facet',
+'../text',
 '../widgets/ActionDropdownWidget'
],
function(declare, lang, on, construct, dom_class,
-builder, mod_facet, ActionDropdownWidget) {
+builder, mod_facet, text, ActionDropdownWidget) {
 
 
 /**
@@ -247,7 +248,7 @@ var HeaderMixin = declare([], {
 name: 'facet_actions',
 'class': 'dropdown facet-actions',
 right_aligned: true,
-toggle_text: 'Actions ',
+toggle_text: text.get('@i18n:actions.title') + ' ',
 toggle_class: 'btn btn-default',
 toggle_icon: 'fa fa-angle-down'
 });
-- 
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 0532] Fix possibly undefined variable

[Alexander Bokovoy]

On Mon, 20 Jun 2016, Martin Basti wrote:

Patch attached.




From a073b44587a5b34c4f1de5742d54e7c547cd5821 Mon Sep 17 00:00:00 2001
From: Martin Basti 
Date: Mon, 20 Jun 2016 12:48:38 +0200
Subject: [PATCH] Fix possibly undefined variable in ipa_smb_conf_exists()

There was missing else statement what may result in undefined conf_fd
variable.
---
ipaserver/install/adtrustinstance.py | 2 ++
1 file changed, 2 insertions(+)

diff --git a/ipaserver/install/adtrustinstance.py 
b/ipaserver/install/adtrustinstance.py
index 
94474122125a59d7da8b05a13dcd6c0f20568855..6ab15df27216580d440ce72386113d6872c046b2
 100644
--- a/ipaserver/install/adtrustinstance.py
+++ b/ipaserver/install/adtrustinstance.py
@@ -81,6 +81,8 @@ def ipa_smb_conf_exists():
except IOError as err:
if err.errno == errno.ENOENT:
return False
+else:
+raise

lines = conf_fd.readlines()
conf_fd.close()
--
2.5.5


ACK, thanks!

--
/ 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] 0021 slapi-nis should allow password update on a virtual entry

[Alexander Bokovoy]

On Wed, 15 Jun 2016, thierry bordaz wrote:

Thanks Alexander for the review.
You are right I forgot to remove those lines during the cleanup.

ACK -- I've committed this patch to slapi-nis and released 0.56.0
version.

https://bodhi.fedoraproject.org/updates/slapi-nis-0.56.0-2.fc24


--
/ 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] 0020 Enable password change extop to apply on virtual entry like the entry in compat tree

[Alexander Bokovoy]

On Tue, 14 Jun 2016, thierry bordaz wrote:

From ac6c0617f618fc609df93dc18ec25255484b533d Mon Sep 17 00:00:00 2001
From: Thierry Bordaz 
Date: Fri, 10 Jun 2016 15:34:40 +0200
Subject: [PATCH] ipapwd_extop should use TARGET_DN defined by a pre-extop
plugin

ipapwd_extop allows to update the password on a specific entry, identified by 
its DN.
It can be usefull to support virtual DN in the extop so that update of a 
virtual entry
would land into the proper real entry.

If a pre-extop sets the TARGET_DN, ipapwd_extop sets ORIGINAL_DN with the value
of TARGET_DN, instead of using the original one (in the ber req)

https://fedorahosted.org/freeipa/ticket/5946
---
.../ipa-pwd-extop/ipa_pwd_extop.c  | 36 +-
1 file changed, 28 insertions(+), 8 deletions(-)

diff --git a/daemons/ipa-slapi-plugins/ipa-pwd-extop/ipa_pwd_extop.c 
b/daemons/ipa-slapi-plugins/ipa-pwd-extop/ipa_pwd_extop.c
index 440e221..3c2c44f 100644
--- a/daemons/ipa-slapi-plugins/ipa-pwd-extop/ipa_pwd_extop.c
+++ b/daemons/ipa-slapi-plugins/ipa-pwd-extop/ipa_pwd_extop.c
@@ -207,8 +207,10 @@ static int ipapwd_chpwop(Slapi_PBlock *pb, struct 
ipapwd_krbcfg *krbcfg)
char *attrlist[] = {"*", "passwordHistory", NULL };
struct ipapwd_data pwdata;
int is_krb, is_smb, is_ipant;
-char *principal = NULL;
+   char *principal = NULL;
Slapi_PBlock *chpwop_pb = NULL;
+   Slapi_DN *target_sdn = NULL;
+   char *target_dn = NULL;

/* Get the ber value of the extended operation */
slapi_pblock_get(pb, SLAPI_EXT_OP_REQ_VALUE, _value);
@@ -327,14 +329,32 @@ parse_req_done:
}
}

-/* Determine the target DN for this operation */
-/* Did they give us a DN ? */
-   if (dn == NULL || *dn == '\0') {
-   /* Get the DN from the bind identity on this connection */
-   dn = slapi_ch_strdup(bindDN);
-   LOG_TRACE("Missing userIdentity in request, "
-  "using the bind DN instead.\n");
+   /* Determine the target DN for this operation */
+   slapi_pblock_get(pb, SLAPI_TARGET_SDN, _sdn);
+   if (target_sdn != NULL) {
+   /* If there is a TARGET_DN we are consuming it */
+   slapi_pblock_set(pb, SLAPI_TARGET_SDN, NULL);
+   target_dn = slapi_sdn_get_ndn(target_sdn);
}
+   if (target_dn == NULL || *target_dn == '\0') {
+   /* Did they give us a DN ? */
+   if (dn == NULL || *dn == '\0') {
+   /* Get the DN from the bind identity on this connection 
*/
+   dn = slapi_ch_strdup(bindDN);
+   LOG_TRACE("Missing userIdentity in request, "
+   "using the bind DN instead.\n");
+   }
+   LOG_TRACE("extop dn %s (from ber)\n", dn ? dn : "");
+   } else {
+   /* At this point if SLAPI_TARGET_SDN was set that means
+* that a SLAPI_PLUGIN_PRE_EXTOP_FN plugin sets it
+* So take this one rather that the raw one that is in the ber
+*/
+   LOG_TRACE("extop dn %s was translated to %s\n", dn ? dn : 
"", target_dn);
+   slapi_ch_free_string();
+   dn = slapi_ch_strdup(target_dn);
+   }
+   slapi_sdn_free(_sdn);

 if (slapi_pblock_set( pb, SLAPI_ORIGINAL_TARGET, dn )) {
LOG_FATAL("slapi_pblock_set failed!\n");
--
2.5.0


ACK. A build with slapi-nis 0.56.0-2.fc24 that includes pre-extop
callback is available in Fedora --
https://bodhi.fedoraproject.org/updates/slapi-nis-0.56.0-2.fc24 so you
can test against it.

I think FreeIPA also needs to raise dependency to slapi-nis >= 0.56.0
for this.

--
/ 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] 0019 - 2 ipapwd_extop should take precedence over default DS plugin

[Alexander Bokovoy]

On Thu, 16 Jun 2016, thierry bordaz wrote:

From 81af4f17deca1814851429a054804b5bc9f63491 Mon Sep 17 00:00:00 2001
From: Thierry Bordaz 
Date: Thu, 16 Jun 2016 16:28:03 +0200
Subject: [PATCH] Make sure ipapwd_extop takes precedence over
passwd_modify_extop

DS core server provides a default plugin (passwd_modify_extop) to handle
1.3.6.1.4.1.4203.1.11.1 extended op (https://www.ietf.org/rfc/rfc3062.txt)

IPA delivers ipa_pwd_extop plugin that should take precedence over
the default DS plugin (passwd_modify_extop)

In addition make sure that slapi-nis has a low precedence
---
install/share/schema_compat.uldif   | 2 +-
install/updates/10-ipapwd.update| 9 +
install/updates/10-schema_compat.update | 2 +-
3 files changed, 11 insertions(+), 2 deletions(-)
create mode 100644 install/updates/10-ipapwd.update

diff --git a/install/share/schema_compat.uldif 
b/install/share/schema_compat.uldif
index a3d412f..66f8ea1 100644
--- a/install/share/schema_compat.uldif
+++ b/install/share/schema_compat.uldif
@@ -16,7 +16,7 @@ default:nsslapd-pluginid: schema-compat-plugin
# We need to run schema-compat pre-bind callback before
# other IPA pre-bind callbacks to make sure bind DN is
# rewritten to the original entry if needed
-default:nsslapd-pluginprecedence: 49
+default:nsslapd-pluginprecedence: 40
default:nsslapd-pluginversion: 0.8
default:nsslapd-pluginbetxn: on
default:nsslapd-pluginvendor: redhat.com
diff --git a/install/updates/10-ipapwd.update b/install/updates/10-ipapwd.update
new file mode 100644
index 000..d9bffa2
--- /dev/null
+++ b/install/updates/10-ipapwd.update
@@ -0,0 +1,9 @@
+dn: cn=ipa_pwd_extop,cn=plugins,cn=config
+# DS core server provides a default plugin (passwd_modify_extop) to handle
+# 1.3.6.1.4.1.4203.1.11.1 extended op (https://www.ietf.org/rfc/rfc3062.txt)
+# the pluginprecedence of the passwd_modify_extop is 50 (default value)
+#
+# IPA delivers ipa_pwd_extop plugin to handle that extended op
+# we need to make sure ipa_pwd_extop is called and so to set a lower
+# precedence value
+add:nsslapd-pluginprecedence: 49
diff --git a/install/updates/10-schema_compat.update 
b/install/updates/10-schema_compat.update
index 2d257a3..e4c257d 100644
--- a/install/updates/10-schema_compat.update
+++ b/install/updates/10-schema_compat.update
@@ -74,7 +74,7 @@ dn: cn=Schema Compatibility,cn=plugins,cn=config
# We need to run schema-compat pre-bind callback before
# other IPA pre-bind callbacks to make sure bind DN is
# rewritten to the original entry if needed
-add:nsslapd-pluginprecedence: 49
+add:nsslapd-pluginprecedence: 40

dn: cn=users,cn=Schema Compatibility,cn=plugins,cn=config
add:schema-compat-entry-attribute: 
%ifeq("ipauniqueid","%{ipauniqueid}","objectclass=ipaOverrideTarget","")
--
2.5.0


ACK. A build override with 389-ds-base 1.3.5.6-1.fc24 is also created.

--
/ 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 0137] DNS Locations: make ipa-ca record generation more robus

[Martin Basti]

On 20.06.2016 18:28, Petr Spacek wrote:

Hello,

DNS Locations: make ipa-ca record generation more robust

__add_ca_records_from_hostname() now skips over DNS exceptions and
retries resolution until timeout of 120 seconds is reached.

Luckily current logic fails safe: In cases where resolution failed for
all the CA servers, the resulting zone object will not contain ipa-ca
record at all and the update logic will skip update for this name.
I.e. the original values in ipa-ca record set will be left in place.

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


ACK

master:
* b6bab8d4e0d6f4715ef353b6944c85c5e88d44ab DNS Locations: make ipa-ca 
record generation more robust


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

[Martin Basti]

On 20.06.2016 18:48, Martin Basti wrote:



On 20.06.2016 16:42, Jan Cholasta wrote:

On 20.6.2016 16:13, David Kupka wrote:

On 28/04/16 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



Hello!

Patches:
replica install: fix thin client regression
schema: remove `no_cli` from command schema
schema: remove redundant information
schema: merge command args and options
schema: remove output_params
schema: add object class schema
permission: handle ipapermright deprecated CLI alias on the client
passwd: handle sort order of passwd argument on the client
misc: skip `count` and `total` output in env.output_for_cli
dns: do not rely on custom param fields in record attributes
automember: add object plugin for automember_rebuild
frontend: do not crash on missing output in output_for_cli
frontend: skip `value` output in output_for_cli
frontend: don't copy command arguments to output params
makeaci, makeapi: use in-server API

work for me, ACK.


Pushed to master: 8cc8b6fb1023fa4aeafac0df01cfacff4ebf537a

Note that I haven't pushed "replica install: fix thin client 
regression" yet, I would like Martin to review it first.


ACK

pushed to master:
* 91d6d87ca76e3aa27d5f87fd4f0b70f1d4fe4e72 replica install: fix thin 
client regression




Attaching the patches for reference.



Note: There is one one known issue in automember output, will be fixed
later.







Guys, I suspect that one of previously pushed patches breaks following 
command on the client side


# ipa dns-update-system-records
ipa: ERROR: KeyError: 'ipa_records'
Traceback (most recent call last):
  File "/usr/lib/python2.7/site-packages/ipalib/cli.py", line 1346, in run
sys.exit(api.Backend.cli.run(argv))
  File "/usr/lib/python2.7/site-packages/ipalib/cli.py", line , in run
rv = cmd.output_for_cli(self.api.Backend.textui, result, *args, 
**options)
  File "/usr/lib/python2.7/site-packages/ipaclient/plugins/dns.py", 
line 367, in output_for_cli

textui.print_indented(u'{}:'.format(labels[key]), indent=1)
KeyError: 'ipa_records'
ipa: ERROR: an internal error has occurred

-- 
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-0046] Increased certmonger timeout to address ticket N 5758

[Martin Basti]

On 16.06.2016 10:29, Oleg Fayans wrote:

With this change the certmonger timeout issue is no longer observed in
abcd lab.




ACK

Pushed to:
master: 0ba9e72057bd372a7cf8ee51d1521ec5d11069d5
ipa-4-3: 084340b1c513c874e259378c0e24008c8f0237ed

-- 
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 0533] Server-del: fix system records removal

[Martin Basti]

Patch attached.

Services must be removed before records are updated


From 7c0d3bd3ff3f507ddcd92d7b2f8e2363696c4dba Mon Sep 17 00:00:00 2001
From: Martin Basti 
Date: Mon, 20 Jun 2016 19:27:55 +0200
Subject: [PATCH] Server-del: fix system records removal

Services on replica to be removed  must be deleted first, otherwise
update of system records will not take this change into account

https://fedorahosted.org/freeipa/ticket/2008
---
 ipaserver/plugins/server.py | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/ipaserver/plugins/server.py b/ipaserver/plugins/server.py
index 42bcb393f21ca802c2a98a3674f9649e6ede446f..ad325130976bef1e0a1486ed73fc5a6a23879ebe 100644
--- a/ipaserver/plugins/server.py
+++ b/ipaserver/plugins/server.py
@@ -676,12 +676,12 @@ class server_del(LDAPDelete):
 # remove the references to master's ldap/http principals
 self._remove_server_principal_references(pkey)
 
-# try to clean up the leftover DNS entries
-self._cleanup_server_dns_records(pkey)
-
 # finally destroy all Kerberos principals
 self._remove_server_host_services(ldap, pkey)
 
+# try to clean up the leftover DNS entries
+self._cleanup_server_dns_records(pkey)
+
 return dn
 
 def exc_callback(self, keys, options, exc, call_func, *call_args,
-- 
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 0135-0136] DNS: Warn about restart when default TTL setting DNS is change DNS: Support default TTL setting for master DNS zone

[Martin Basti]

On 20.06.2016 18:32, Petr Spacek wrote:

On 20.6.2016 18:05, Martin Basti wrote:


On 20.06.2016 16:57, Petr Spacek wrote:

Hello,

DNS: Warn about restart when default TTL setting DNS is changed

bind-dyndb-ldap 10.0 has to be restarted after each change to default
TTL.

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

DNS: Support default TTL setting for master DNS zones

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




Thank you for patches, but I have a few comments

TTL patch:
1)
VERSION - please put short note why API was incremented

2)
60ipadns.ldif - please keep ordered attr definitions by OID

3)
You missed ACI for updating

Warning patch: LGTM

Thank you very much for review!

Here is revised version.



I cannot apply patches on current master, even with git am -3
Martin^2

--
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] 0019 - 2 ipapwd_extop should take precedence over default DS plugin

[Martin Basti]

On 16.06.2016 22:29, Alexander Bokovoy wrote:

On Thu, 16 Jun 2016, thierry bordaz wrote:
The version DS 1.3.5.6 is now available. Here is the second version 
of the patch taking into account lower precedence for Schema Compat




On 06/13/2016 06:01 PM, Alexander Bokovoy wrote:

On Mon, 13 Jun 2016, thierry bordaz wrote:



On 06/13/2016 04:57 PM, Alexander Bokovoy wrote:

On Mon, 13 Jun 2016, thierry bordaz wrote:

This is the fix for https://fedorahosted.org/freeipa/ticket/5944


From 2838fbfc7a22b9bc0c1c4dfaf3660d1ac7099461 Mon Sep 17 
00:00:00 2001

From: Thierry Bordaz 
Date: Wed, 8 Jun 2016 14:03:42 +0200
Subject: [PATCH] Make sure ipapwd_extop takes precedence over
passwd_modify_extop

DS core server provides a default plugin (passwd_modify_extop) to 
handle
1.3.6.1.4.1.4203.1.11.1 extended op 
(https://www.ietf.org/rfc/rfc3062.txt)


IPA delivers ipa_pwd_extop plugin that should take precedence over
the default DS plugin (passwd_modify_extop)
---
install/updates/10-ipapwd.update | 9 +
1 file changed, 9 insertions(+)
create mode 100644 install/updates/10-ipapwd.update

diff --git a/install/updates/10-ipapwd.update 
b/install/updates/10-ipapwd.update

new file mode 100644
index 000..d9bffa2
--- /dev/null
+++ b/install/updates/10-ipapwd.update
@@ -0,0 +1,9 @@
+dn: cn=ipa_pwd_extop,cn=plugins,cn=config
+# DS core server provides a default plugin (passwd_modify_extop) 
to handle
+# 1.3.6.1.4.1.4203.1.11.1 extended op 
(https://www.ietf.org/rfc/rfc3062.txt)
+# the pluginprecedence of the passwd_modify_extop is 50 (default 
value)

+#
+# IPA delivers ipa_pwd_extop plugin to handle that extended op
+# we need to make sure ipa_pwd_extop is called and so to set a 
lower

+# precedence value
+add:nsslapd-pluginprecedence: 49

Here is the problem: slapi-nis is 49 as well and it should be before
ipa_pwd_extop.

You need to update install/share/schema_compat.uldif and
install/updates/10-schema_compat.update to get slapi-nis before
ipa_pwd_extop.
ipapwd_plugin registers extendedop callback but slapi-nis does not. 
So I do not think they will "fight" for precedence.
Even if slapi-nis register perextendedop they will be on different 
lists and it should not create any issue.


Now I understand that slapi-nis must run with a precedence that 
should be lower than most of the others plugins. Currently it is 
49, are you ok with a value like 40 ?

I'm OK with 40, yes. The precedence applies to all callbacks, not just
to preextendedop, so a BIND callback would be affected too.

You also need to make sure we depend on the updated 389-ds-base 
package

version.


Good !
Now with this dependency we should wait for 389-ds 1.3.5.5 to be 
available, I will resend the review when it will be available.

Yep, thanks.






From 81af4f17deca1814851429a054804b5bc9f63491 Mon Sep 17 00:00:00 2001
From: Thierry Bordaz 
Date: Thu, 16 Jun 2016 16:28:03 +0200
Subject: [PATCH] Make sure ipapwd_extop takes precedence over
passwd_modify_extop

DS core server provides a default plugin (passwd_modify_extop) to handle
1.3.6.1.4.1.4203.1.11.1 extended op 
(https://www.ietf.org/rfc/rfc3062.txt)


IPA delivers ipa_pwd_extop plugin that should take precedence over
the default DS plugin (passwd_modify_extop)

In addition make sure that slapi-nis has a low precedence
---
install/share/schema_compat.uldif   | 2 +-
install/updates/10-ipapwd.update| 9 +
install/updates/10-schema_compat.update | 2 +-
3 files changed, 11 insertions(+), 2 deletions(-)
create mode 100644 install/updates/10-ipapwd.update

diff --git a/install/share/schema_compat.uldif 
b/install/share/schema_compat.uldif

index a3d412f..66f8ea1 100644
--- a/install/share/schema_compat.uldif
+++ b/install/share/schema_compat.uldif
@@ -16,7 +16,7 @@ default:nsslapd-pluginid: schema-compat-plugin
# We need to run schema-compat pre-bind callback before
# other IPA pre-bind callbacks to make sure bind DN is
# rewritten to the original entry if needed
-default:nsslapd-pluginprecedence: 49
+default:nsslapd-pluginprecedence: 40
default:nsslapd-pluginversion: 0.8
default:nsslapd-pluginbetxn: on
default:nsslapd-pluginvendor: redhat.com
diff --git a/install/updates/10-ipapwd.update 
b/install/updates/10-ipapwd.update

new file mode 100644
index 000..d9bffa2
--- /dev/null
+++ b/install/updates/10-ipapwd.update
@@ -0,0 +1,9 @@
+dn: cn=ipa_pwd_extop,cn=plugins,cn=config
+# DS core server provides a default plugin (passwd_modify_extop) to 
handle
+# 1.3.6.1.4.1.4203.1.11.1 extended op 
(https://www.ietf.org/rfc/rfc3062.txt)

+# the pluginprecedence of the passwd_modify_extop is 50 (default value)
+#
+# IPA delivers ipa_pwd_extop plugin to handle that extended op
+# we need to make sure ipa_pwd_extop is called and so to set a lower
+# precedence value
+add:nsslapd-pluginprecedence: 49
diff --git a/install/updates/10-schema_compat.update 
b/install/updates/10-schema_compat.update

index 2d257a3..e4c257d 100644
--- 

Re: [Freeipa-devel] [PATCH] pylint fixes

[Martin Basti]

On 20.06.2016 19:06, Martin Basti wrote:




On 20.06.2016 12:00, Florence Blanc-Renaud wrote:

On 06/09/2016 05:10 PM, Petr Spacek wrote:

Hello,

I've received a bunch of pylint fixes produced by upstream contributor who is
not subscribed to the list so I'm resending them here.

All credit goes to Bárta Jan<55042ba...@sstebrno.eu>.

Flo, if you have time for it I think that it could be a good exercise which
will lead you to various dark corners in IPA :-)

Petr^2 Spacek


 Forwarded Message 
Date: Fri, 3 Jun 2016 14:57:16 +0200
From: Bárta Jan<55042ba...@sstebrno.eu>
To:pspa...@redhat.com
___- In the patch 
0002-pylint-fix-simplifiable-if-statement-warnings.patch:_


diff --git a/ipatests/test_integration/tasks.py 
b/ipatests/test_integration/tasks.py

index aebd907..ca2e10f 100644
--- a/ipatests/test_integration/tasks.py
+++ b/ipatests/test_integration/tasks.py
@@ -149,11 +149,7 @@ def host_service_active(host, service):
 res = host.run_command(['systemctl', 'is-active', '--quiet', 
service],

raiseonerr=False)

-if res.returncode == 0:
-return True
-else:
-return False
-
+return res.returncode

should be instead: return res.returncode *== 0* (otherwise the return 
type is an int and not a boolean).


In the same file:
@@ -295,11 +291,7 @@ def 
master_authoritative_for_client_domain(master, client):

 zone = ".".join(client.hostname.split('.')[1:])
 result = master.run_command(["ipa", "dnszone-show", zone],
 raiseonerr=False)
-if result.returncode == 0:
-return True
-else:
-return False
-
+result.returncode == 0

should be instead: *return* result.returncode == 0 (otherwise there 
is no return statement)


diff --git a/ipaserver/plugins/dogtag.py b/ipaserver/plugins/dogtag.py
index 197814c..36b6ba5 100644
--- a/ipaserver/plugins/dogtag.py
+++ b/ipaserver/plugins/dogtag.py
@@ -1689,12 +1689,7 @@ class ra(rabase.rabase):
 # Return command result
 cmd_result = {}

-if parse_result.get('revoked') == 'yes':
-cmd_result['revoked'] = True
-else:
-cmd_result['revoked'] = False
-
-return cmd_result
+cmd_result['revoked'] = parse_result.get('revoked')

Should be instead: cmd_result['revoked'] = 
parse_result.get('revoked') *== 'yes'* (otherwise the type is a 
string and not a boolean)


_- in the patch 00__04-pylint-fix-unneeded-not.patch_

@@ -632,7 +632,7 @@ class host_add(LDAPCreate):
 options['ip_address'],
 check_forward=True,
 check_reverse=check_reverse)
-if not options.get('force', False) and not 'ip_address' in 
options:

+if options.get('force', False) and 'ip_address' not in options:

Should be instead: if *not* options.get('force', False) and 
'ip_address' not in options:

because of operators precedence

I will review patches 0005 to 0010 later today.
Flo.




How about patches 1, and 3? Because patches are independent, we can 
separately ACK them and push them.


Martin^2




Sorry, I just noticed that there is no patch 1 :)
-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [PATCH] pylint fixes

[Martin Basti]

On 20.06.2016 12:00, Florence Blanc-Renaud wrote:

On 06/09/2016 05:10 PM, Petr Spacek wrote:

Hello,

I've received a bunch of pylint fixes produced by upstream contributor who is
not subscribed to the list so I'm resending them here.

All credit goes to Bárta Jan<55042ba...@sstebrno.eu>.

Flo, if you have time for it I think that it could be a good exercise which
will lead you to various dark corners in IPA :-)

Petr^2 Spacek


 Forwarded Message 
Date: Fri, 3 Jun 2016 14:57:16 +0200
From: Bárta Jan<55042ba...@sstebrno.eu>
To:pspa...@redhat.com
___- In the patch 
0002-pylint-fix-simplifiable-if-statement-warnings.patch:_


diff --git a/ipatests/test_integration/tasks.py 
b/ipatests/test_integration/tasks.py

index aebd907..ca2e10f 100644
--- a/ipatests/test_integration/tasks.py
+++ b/ipatests/test_integration/tasks.py
@@ -149,11 +149,7 @@ def host_service_active(host, service):
 res = host.run_command(['systemctl', 'is-active', '--quiet', 
service],

raiseonerr=False)

-if res.returncode == 0:
-return True
-else:
-return False
-
+return res.returncode

should be instead: return res.returncode *== 0* (otherwise the return 
type is an int and not a boolean).


In the same file:
@@ -295,11 +291,7 @@ def 
master_authoritative_for_client_domain(master, client):

 zone = ".".join(client.hostname.split('.')[1:])
 result = master.run_command(["ipa", "dnszone-show", zone],
 raiseonerr=False)
-if result.returncode == 0:
-return True
-else:
-return False
-
+result.returncode == 0

should be instead: *return* result.returncode == 0 (otherwise there is 
no return statement)


diff --git a/ipaserver/plugins/dogtag.py b/ipaserver/plugins/dogtag.py
index 197814c..36b6ba5 100644
--- a/ipaserver/plugins/dogtag.py
+++ b/ipaserver/plugins/dogtag.py
@@ -1689,12 +1689,7 @@ class ra(rabase.rabase):
 # Return command result
 cmd_result = {}

-if parse_result.get('revoked') == 'yes':
-cmd_result['revoked'] = True
-else:
-cmd_result['revoked'] = False
-
-return cmd_result
+cmd_result['revoked'] = parse_result.get('revoked')

Should be instead: cmd_result['revoked'] = parse_result.get('revoked') 
*== 'yes'* (otherwise the type is a string and not a boolean)


_- in the patch 00__04-pylint-fix-unneeded-not.patch_

@@ -632,7 +632,7 @@ class host_add(LDAPCreate):
 options['ip_address'],
 check_forward=True,
 check_reverse=check_reverse)
-if not options.get('force', False) and not 'ip_address' in 
options:

+if options.get('force', False) and 'ip_address' not in options:

Should be instead: if *not* options.get('force', False) and 
'ip_address' not in options:

because of operators precedence

I will review patches 0005 to 0010 later today.
Flo.




How about patches 1, and 3? Because patches are independent, we can 
separately ACK them and push them.


Martin^2
-- 
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 0134] DNS: Fix realm domains integration with DNS zone add

[Martin Basti]

On 20.06.2016 14:35, Petr Spacek wrote:

Hello,

DNS: Fix realm domains integration with DNS zone add.

Realmdomains integration into DNS commands pre-dates split of DNS forward zones
and DNS master zones into two distinct commands.

There was an forgotten condition in dnszone_add command which caused omission
of DNS master zones with non-empty forwarders from realmdomain list.

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




ACK
-- 
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] 0058 WebUI: certificate widget on ID override user page

[Pavel Vomacka]

Hello,

please review attached patch.

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

--

Pavel^3 Vomacka

From aa6ef0a9b51d8c2d955399a044d1ee90cc6f936e Mon Sep 17 00:00:00 2001
From: Pavel Vomacka 
Date: Fri, 17 Jun 2016 10:05:52 +0200
Subject: [PATCH] Add certificate widget to ID override user details page.

Add possibility to add, remove, view, get and download custom certificates on ID override user page.

https://fedorahosted.org/freeipa/ticket/5926
---
 install/ui/src/freeipa/idviews.js | 152 +-
 1 file changed, 151 insertions(+), 1 deletion(-)

diff --git a/install/ui/src/freeipa/idviews.js b/install/ui/src/freeipa/idviews.js
index 823936401653096a0a649282e3b18f573de09804..bf097e08199c9fa8de313acd19c4c98fd1d3a0f0 100644
--- a/install/ui/src/freeipa/idviews.js
+++ b/install/ui/src/freeipa/idviews.js
@@ -22,6 +22,7 @@
 define([
 'dojo/on',
 './ipa',
+'./builder',
 './jquery',
 './menu',
 './phases',
@@ -32,7 +33,8 @@ define([
 './facet',
 './search',
 './entity'],
-function(on, IPA, $, menu, phases, reg, rpc, text, mod_details, mod_facet) {
+function(on, IPA, builder, $, menu, phases, reg, rpc, text,
+mod_details, mod_facet) {
 /**
  * ID Views module
  * @class
@@ -223,11 +225,18 @@ return {
 source_facet: 'details',
 dest_entity: 'idview',
 dest_facet: 'idoverrideuser'
+},
+{
+$factory: IPA.cert.cert_update_policy,
+source_facet: 'details',
+dest_entity: 'cert',
+dest_facet: 'search'
 }
 ],
 containing_entity: 'idview',
 facets: [
 {
+$factory: idviews.id_override_user_details_facet,
 $type: 'details',
 disable_breadcrumb: false,
 containing_facet: 'idoverrideuser',
@@ -261,6 +270,11 @@ return {
 $type: 'sshkeys',
 name: 'ipasshpubkey',
 label: '@i18n:objects.sshkeystore.keys'
+},
+{
+$type: 'idviews_certs',
+name: 'usercertificate',
+label: '@i18n:objects.cert.certificates'
 }
 ]
 }
@@ -294,6 +308,10 @@ return {
 'gecos',
 'uidnumber',
 'gidnumber',
+{
+$type: 'textarea',
+name: 'usercertificate'
+},
 'loginshell',
 'homedirectory',
 {
@@ -386,6 +404,134 @@ return {
 
 
 /**
+ * Facet for User ID override, uses batch command to fetch certificates.
+ *
+ * @class
+ * @extends IPA.details_facet
+ */
+idviews.id_override_user_details_facet = function(spec) {
+
+spec = spec || {};
+
+var that = IPA.details_facet(spec);
+
+that.certificate_updated = IPA.observer();
+
+that.create_refresh_command = function() {
+
+var user_command = that.details_facet_create_refresh_command();
+
+var batch = rpc.batch_command({
+name: that.entity.name + "_details_refresh"
+});
+
+batch.add_command(user_command);
+
+var pkey = that.get_pkey();
+
+var certs = rpc.command({
+entity: 'cert',
+method: 'find',
+retry: false,
+options: {
+idoverrideuser: [ pkey ],
+all: true
+}
+});
+
+batch.add_command(certs);
+
+return batch;
+};
+
+return that;
+};
+
+/**
+ * @extends IPA.cert.certs_widget
+ */
+idviews.idviews_certs_widget = function(spec) {
+
+spec = spec || {};
+spec.child_spec = {
+$factory: idviews.idviews_cert_widget,
+css_class: 'certificate-widget',
+facet: spec.facet
+};
+
+var that = IPA.cert.certs_widget(spec);
+
+/* Adds two args to add command - special nested entities. */
+that.create_add_args = function() {
+return that.facet.get_pkeys();
+};
+
+/* Adds two args to remove command - special nested entities. */
+that.create_remove_args = function() {
+return that.facet.get_pkeys();
+};
+
+return that;
+};
+
+/**
+ * This widget uses cert_find instead of cert_show, because cert_show does not
+ * support nested entities.
+ *
+ * @extends IPA.cert.cert_widget
+ */
+idviews.idviews_cert_widget = function(spec) {
+
+spec = spec || {};
+
+var that = IPA.cert.cert_widget(spec);
+
+that.adapter = builder.build('adapter', spec.adapter || 'object_adapter', {});
+
+that.fetch_certificate_data = function(cert) {
+var result = {};
+var adapter = that.adapter;
+
+if (!cert) return;
+
+var command = rpc.command({
+

[Freeipa-devel] [PATCH 0532] Fix possibly undefined variable

[Martin Basti]

Patch attached.

From a073b44587a5b34c4f1de5742d54e7c547cd5821 Mon Sep 17 00:00:00 2001
From: Martin Basti 
Date: Mon, 20 Jun 2016 12:48:38 +0200
Subject: [PATCH] Fix possibly undefined variable in ipa_smb_conf_exists()

There was missing else statement what may result in undefined conf_fd
variable.
---
 ipaserver/install/adtrustinstance.py | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/ipaserver/install/adtrustinstance.py b/ipaserver/install/adtrustinstance.py
index 94474122125a59d7da8b05a13dcd6c0f20568855..6ab15df27216580d440ce72386113d6872c046b2 100644
--- a/ipaserver/install/adtrustinstance.py
+++ b/ipaserver/install/adtrustinstance.py
@@ -81,6 +81,8 @@ def ipa_smb_conf_exists():
 except IOError as err:
 if err.errno == errno.ENOENT:
 return False
+else:
+raise
 
 lines = conf_fd.readlines()
 conf_fd.close()
-- 
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 0135-0136] DNS: Warn about restart when default TTL setting DNS is change DNS: Support default TTL setting for master DNS zone

[Petr Spacek]

On 20.6.2016 18:05, Martin Basti wrote:
> 
> 
> On 20.06.2016 16:57, Petr Spacek wrote:
>> Hello,
>>
>> DNS: Warn about restart when default TTL setting DNS is changed
>>
>> bind-dyndb-ldap 10.0 has to be restarted after each change to default
>> TTL.
>>
>> https://fedorahosted.org/freeipa/ticket/2956
>>
>> DNS: Support default TTL setting for master DNS zones
>>
>> https://fedorahosted.org/freeipa/ticket/2956
>>
>>
>>
> Thank you for patches, but I have a few comments
> 
> TTL patch:
> 1)
> VERSION - please put short note why API was incremented
> 
> 2)
> 60ipadns.ldif - please keep ordered attr definitions by OID
> 
> 3)
> You missed ACI for updating
> 
> Warning patch: LGTM

Thank you very much for review!

Here is revised version.

-- 
Petr^2 Spacek
From c05f83896a124004935f578bddbbb881fb892197 Mon Sep 17 00:00:00 2001
From: Petr Spacek 
Date: Mon, 20 Jun 2016 14:38:56 +0200
Subject: [PATCH] DNS: Support default TTL setting for master DNS zones

https://fedorahosted.org/freeipa/ticket/2956
---
 ACI.txt |  2 +-
 API.txt |  9 ++---
 VERSION |  4 ++--
 install/share/60ipadns.ldif |  3 ++-
 ipaserver/plugins/dns.py| 15 +++
 5 files changed, 22 insertions(+), 11 deletions(-)

diff --git a/ACI.txt b/ACI.txt
index 0646d0d24d0e8a427eabf5aca04566f269e96cd2..9dd7f8d5d9df09e11b740f247a4afe3f68328002 100644
--- a/ACI.txt
+++ b/ACI.txt
@@ -73,7 +73,7 @@ aci: (targetattr = "ipaprivatekey || ipapublickey || ipasecretkey || ipasecretke
 dn: dc=ipa,dc=example
 aci: (targetattr = "cn || idnssecalgorithm || idnsseckeyactivate || idnsseckeycreated || idnsseckeydelete || idnsseckeyinactive || idnsseckeypublish || idnsseckeyref || idnsseckeyrevoke || idnsseckeysep || idnsseckeyzone || objectclass")(target = "ldap:///cn=dns,dc=ipa,dc=example;)(targetfilter = "(objectclass=idnsSecKey)")(version 3.0;acl "permission:System: Manage DNSSEC metadata";allow (all) groupdn = "ldap:///cn=System: Manage DNSSEC metadata,cn=permissions,cn=pbac,dc=ipa,dc=example";)
 dn: dc=ipa,dc=example
-aci: (targetattr = "a6record || record || afsdbrecord || aplrecord || arecord || certrecord || cn || cnamerecord || createtimestamp || dhcidrecord || dlvrecord || dnamerecord || dnsclass || dnsttl || dsrecord || entryusn || hinforecord || hiprecord || idnsallowdynupdate || idnsallowquery || idnsallowsyncptr || idnsallowtransfer || idnsforwarders || idnsforwardpolicy || idnsname || idnssecinlinesigning || idnssoaexpire || idnssoaminimum || idnssoamname || idnssoarefresh || idnssoaretry || idnssoarname || idnssoaserial || idnstemplateattribute || idnsupdatepolicy || idnszoneactive || ipseckeyrecord || keyrecord || kxrecord || locrecord || managedby || mdrecord || minforecord || modifytimestamp || mxrecord || naptrrecord || nsec3paramrecord || nsecrecord || nsrecord || nxtrecord || objectclass || ptrrecord || rprecord || rrsigrecord || sigrecord || spfrecord || srvrecord || sshfprecord || tlsarecord || txtrecord || unknownrecord")(target = "ldap:///idnsname=*,cn=dns,dc=ipa,dc=example;)(version 3.0;acl "permission:System: Read DNS Entries";allow (compare,read,search) groupdn = "ldap:///cn=System: Read DNS Entries,cn=permissions,cn=pbac,dc=ipa,dc=example";)
+aci: (targetattr = "a6record || record || afsdbrecord || aplrecord || arecord || certrecord || cn || cnamerecord || createtimestamp || dhcidrecord || dlvrecord || dnamerecord || dnsclass || dnsdefaultttl || dnsttl || dsrecord || entryusn || hinforecord || hiprecord || idnsallowdynupdate || idnsallowquery || idnsallowsyncptr || idnsallowtransfer || idnsforwarders || idnsforwardpolicy || idnsname || idnssecinlinesigning || idnssoaexpire || idnssoaminimum || idnssoamname || idnssoarefresh || idnssoaretry || idnssoarname || idnssoaserial || idnstemplateattribute || idnsupdatepolicy || idnszoneactive || ipseckeyrecord || keyrecord || kxrecord || locrecord || managedby || mdrecord || minforecord || modifytimestamp || mxrecord || naptrrecord || nsec3paramrecord || nsecrecord || nsrecord || nxtrecord || objectclass || ptrrecord || rprecord || rrsigrecord || sigrecord || spfrecord || srvrecord || sshfprecord || tlsarecord || txtrecord || unknownrecord")(target = "ldap:///idnsname=*,cn=dns,dc=ipa,dc=example;)(version 3.0;acl "permission:System: Read DNS Entries";allow (compare,read,search) groupdn = "ldap:///cn=System: Read DNS Entries,cn=permissions,cn=pbac,dc=ipa,dc=example";)
 dn: dc=ipa,dc=example
 aci: (targetattr = "cn || createtimestamp || entryusn || idnssecalgorithm || idnsseckeyactivate || idnsseckeycreated || idnsseckeydelete || idnsseckeyinactive || idnsseckeypublish || idnsseckeyref || idnsseckeyrevoke || idnsseckeysep || idnsseckeyzone || modifytimestamp || objectclass")(target = "ldap:///cn=dns,dc=ipa,dc=example;)(targetfilter = "(objectclass=idnsSecKey)")(version 3.0;acl "permission:System: Read DNSSEC metadata";allow (compare,read,search) groupdn = "ldap:///cn=System: Read DNSSEC 

[Freeipa-devel] [PATCH 0137] DNS Locations: make ipa-ca record generation more robus

[Petr Spacek]

Hello,

DNS Locations: make ipa-ca record generation more robust

__add_ca_records_from_hostname() now skips over DNS exceptions and
retries resolution until timeout of 120 seconds is reached.

Luckily current logic fails safe: In cases where resolution failed for
all the CA servers, the resulting zone object will not contain ipa-ca
record at all and the update logic will skip update for this name.
I.e. the original values in ipa-ca record set will be left in place.

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

-- 
Petr^2 Spacek
From 63fdff793acef0232bf352042f952d47d575d1d1 Mon Sep 17 00:00:00 2001
From: Petr Spacek 
Date: Mon, 20 Jun 2016 18:23:51 +0200
Subject: [PATCH] DNS Locations: make ipa-ca record generation more robust

__add_ca_records_from_hostname() now skips over DNS exceptions and
retries resolution until timeout of 120 seconds is reached.

Luckily current logic fails safe: In cases where resolution failed for
all the CA servers, the resulting zone object will not contain ipa-ca
record at all and the update logic will skip update for this name.
I.e. the original values in ipa-ca record set will be left in place.

https://fedorahosted.org/freeipa/ticket/2008
---
 ipaserver/dns_data_management.py | 21 -
 1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/ipaserver/dns_data_management.py b/ipaserver/dns_data_management.py
index 3ca40c785681a56fd6e7583c6b4db88c58317305..a9e9c0a3856961b5494c8d3ca30ddb2e4aa5c523 100644
--- a/ipaserver/dns_data_management.py
+++ b/ipaserver/dns_data_management.py
@@ -12,12 +12,16 @@ from dns import (
 rdatatype,
 zone,
 )
+from dns.exception import DNSException
 from dns.rdtypes.IN.SRV import SRV
 from dns.rdtypes.ANY.TXT import TXT
 
+from time import sleep, time
+
 from ipalib import errors
 from ipalib.dns import record_name_format
 from ipapython.dnsutil import DNSName, resolve_rrsets
+from ipapython.ipa_log_manager import root_logger
 
 if six.PY3:
 unicode=str
@@ -134,7 +138,22 @@ class IPASystemRecords(object):
 def __add_ca_records_from_hostname(self, zone_obj, hostname):
 assert isinstance(hostname, DNSName) and hostname.is_absolute()
 r_name = DNSName('ipa-ca') + self.domain_abs
-rrsets = resolve_rrsets(hostname, (rdatatype.A, rdatatype.))
+rrsets = []
+end_time = time() + 120  # timeout in seconds
+while time() < end_time:
+try:
+rrsets = resolve_rrsets(hostname, (rdatatype.A, rdatatype.))
+except DNSException:  # logging is done inside resolve_rrsets
+pass
+if rrsets:
+break
+sleep(5)
+
+if not rrsets:
+root_logger.error('unable to resolve host name %s to IP address, '
+  'ipa-ca DNS record will be incomplete', hostname)
+return
+
 for rrset in rrsets:
 for rd in rrset:
 rdataset = zone_obj.get_rdataset(
-- 
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 0135-0136] DNS: Warn about restart when default TTL setting DNS is change DNS: Support default TTL setting for master DNS zone

[Martin Basti]

On 20.06.2016 16:57, Petr Spacek wrote:

Hello,

DNS: Warn about restart when default TTL setting DNS is changed

bind-dyndb-ldap 10.0 has to be restarted after each change to default
TTL.

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

DNS: Support default TTL setting for master DNS zones

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




Thank you for patches, but I have a few comments

TTL patch:
1)
VERSION - please put short note why API was incremented

2)
60ipadns.ldif - please keep ordered attr definitions by OID

3)
You missed ACI for updating

Warning patch: LGTM
-- 
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 0135-0136] DNS: Warn about restart when default TTL setting DNS is change DNS: Support default TTL setting for master DNS zone

[Petr Spacek]

Hello,

DNS: Warn about restart when default TTL setting DNS is changed

bind-dyndb-ldap 10.0 has to be restarted after each change to default
TTL.

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

DNS: Support default TTL setting for master DNS zones

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

-- 
Petr^2 Spacek
From 66af2a2f96fef7e4dff5ae8c35fc03c6f4701194 Mon Sep 17 00:00:00 2001
From: Petr Spacek 
Date: Mon, 20 Jun 2016 14:38:56 +0200
Subject: [PATCH] DNS: Support default TTL setting for master DNS zones

https://fedorahosted.org/freeipa/ticket/2956
---
 ACI.txt |  2 +-
 API.txt |  9 ++---
 VERSION |  2 +-
 install/share/60ipadns.ldif |  3 ++-
 ipaserver/plugins/dns.py| 11 +--
 5 files changed, 19 insertions(+), 8 deletions(-)

diff --git a/ACI.txt b/ACI.txt
index 0646d0d24d0e8a427eabf5aca04566f269e96cd2..9dd7f8d5d9df09e11b740f247a4afe3f68328002 100644
--- a/ACI.txt
+++ b/ACI.txt
@@ -73,7 +73,7 @@ aci: (targetattr = "ipaprivatekey || ipapublickey || ipasecretkey || ipasecretke
 dn: dc=ipa,dc=example
 aci: (targetattr = "cn || idnssecalgorithm || idnsseckeyactivate || idnsseckeycreated || idnsseckeydelete || idnsseckeyinactive || idnsseckeypublish || idnsseckeyref || idnsseckeyrevoke || idnsseckeysep || idnsseckeyzone || objectclass")(target = "ldap:///cn=dns,dc=ipa,dc=example;)(targetfilter = "(objectclass=idnsSecKey)")(version 3.0;acl "permission:System: Manage DNSSEC metadata";allow (all) groupdn = "ldap:///cn=System: Manage DNSSEC metadata,cn=permissions,cn=pbac,dc=ipa,dc=example";)
 dn: dc=ipa,dc=example
-aci: (targetattr = "a6record || record || afsdbrecord || aplrecord || arecord || certrecord || cn || cnamerecord || createtimestamp || dhcidrecord || dlvrecord || dnamerecord || dnsclass || dnsttl || dsrecord || entryusn || hinforecord || hiprecord || idnsallowdynupdate || idnsallowquery || idnsallowsyncptr || idnsallowtransfer || idnsforwarders || idnsforwardpolicy || idnsname || idnssecinlinesigning || idnssoaexpire || idnssoaminimum || idnssoamname || idnssoarefresh || idnssoaretry || idnssoarname || idnssoaserial || idnstemplateattribute || idnsupdatepolicy || idnszoneactive || ipseckeyrecord || keyrecord || kxrecord || locrecord || managedby || mdrecord || minforecord || modifytimestamp || mxrecord || naptrrecord || nsec3paramrecord || nsecrecord || nsrecord || nxtrecord || objectclass || ptrrecord || rprecord || rrsigrecord || sigrecord || spfrecord || srvrecord || sshfprecord || tlsarecord || txtrecord || unknownrecord")(target = "ldap:///idnsname=*,cn=dns,dc=ipa,dc=example;)(version 3.0;acl "permission:System: Read DNS Entries";allow (compare,read,search) groupdn = "ldap:///cn=System: Read DNS Entries,cn=permissions,cn=pbac,dc=ipa,dc=example";)
+aci: (targetattr = "a6record || record || afsdbrecord || aplrecord || arecord || certrecord || cn || cnamerecord || createtimestamp || dhcidrecord || dlvrecord || dnamerecord || dnsclass || dnsdefaultttl || dnsttl || dsrecord || entryusn || hinforecord || hiprecord || idnsallowdynupdate || idnsallowquery || idnsallowsyncptr || idnsallowtransfer || idnsforwarders || idnsforwardpolicy || idnsname || idnssecinlinesigning || idnssoaexpire || idnssoaminimum || idnssoamname || idnssoarefresh || idnssoaretry || idnssoarname || idnssoaserial || idnstemplateattribute || idnsupdatepolicy || idnszoneactive || ipseckeyrecord || keyrecord || kxrecord || locrecord || managedby || mdrecord || minforecord || modifytimestamp || mxrecord || naptrrecord || nsec3paramrecord || nsecrecord || nsrecord || nxtrecord || objectclass || ptrrecord || rprecord || rrsigrecord || sigrecord || spfrecord || srvrecord || sshfprecord || tlsarecord || txtrecord || unknownrecord")(target = "ldap:///idnsname=*,cn=dns,dc=ipa,dc=example;)(version 3.0;acl "permission:System: Read DNS Entries";allow (compare,read,search) groupdn = "ldap:///cn=System: Read DNS Entries,cn=permissions,cn=pbac,dc=ipa,dc=example";)
 dn: dc=ipa,dc=example
 aci: (targetattr = "cn || createtimestamp || entryusn || idnssecalgorithm || idnsseckeyactivate || idnsseckeycreated || idnsseckeydelete || idnsseckeyinactive || idnsseckeypublish || idnsseckeyref || idnsseckeyrevoke || idnsseckeysep || idnsseckeyzone || modifytimestamp || objectclass")(target = "ldap:///cn=dns,dc=ipa,dc=example;)(targetfilter = "(objectclass=idnsSecKey)")(version 3.0;acl "permission:System: Read DNSSEC metadata";allow (compare,read,search) groupdn = "ldap:///cn=System: Read DNSSEC metadata,cn=permissions,cn=pbac,dc=ipa,dc=example";)
 dn: dc=ipa,dc=example
diff --git a/API.txt b/API.txt
index eb14d44eedfdca44043e808d3ef31d6300281cd6..eb5ace6001f1fe8833341d0cd638a007b8187275 100644
--- a/API.txt
+++ b/API.txt
@@ -1565,11 +1565,12 @@ output: Entry('result')
 output: Output('summary', type=[, ])
 output: PrimaryKey('value')
 command: dnszone_add
-args: 1,28,3
+args: 1,29,3
 arg: DNSNameParam('idnsname', cli_name='name')
 option: Str('addattr*', 

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

[Jan Cholasta]

On 20.6.2016 16:13, David Kupka wrote:

On 28/04/16 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



Hello!

Patches:
replica install: fix thin client regression
schema: remove `no_cli` from command schema
schema: remove redundant information
schema: merge command args and options
schema: remove output_params
schema: add object class schema
permission: handle ipapermright deprecated CLI alias on the client
passwd: handle sort order of passwd argument on the client
misc: skip `count` and `total` output in env.output_for_cli
dns: do not rely on custom param fields in record attributes
automember: add object plugin for automember_rebuild
frontend: do not crash on missing output in output_for_cli
frontend: skip `value` output in output_for_cli
frontend: don't copy command arguments to output params
makeaci, makeapi: use in-server API

work for me, ACK.


Pushed to master: 8cc8b6fb1023fa4aeafac0df01cfacff4ebf537a

Note that I haven't pushed "replica install: fix thin client regression" 
yet, I would like Martin to review it first.


Attaching the patches for reference.



Note: There is one one known issue in automember output, will be fixed
later.



--
Jan Cholasta
From 82c9def5f01964a64e51ebf41b3bcfb4ab6a888c Mon Sep 17 00:00:00 2001
From: Jan Cholasta 
Date: Fri, 3 Jun 2016 09:38:11 +0200
Subject: [PATCH 01/14] makeaci, makeapi: use in-server API

Capture the server API rather than client API in API.txt. Client API may be
affected by client-side plugins and thus may not correspond to what is
transmitted over the wire.

https://fedorahosted.org/freeipa/ticket/4739
---
 API.txt | 131 +++-
 makeaci |   4 +-
 makeapi |   4 +-
 3 files changed, 10 insertions(+), 129 deletions(-)

diff --git a/API.txt b/API.txt
index eb14d44..35ae5e4 100644
--- a/API.txt
+++ b/API.txt
@@ -343,13 +343,6 @@ output: Output('count', type=[])
 output: ListOfEntries('result')
 output: Output('summary', type=[, ])
 output: Output('truncated', type=[])
-command: automountlocation_import
-args: 2,2,1
-arg: Str('cn', cli_name='location')
-arg: Str('masterfile')
-option: Flag('continue?', autofill=True, cli_name='continue', default=False)
-option: Str('version?')
-output: Output('result')
 command: automountlocation_show
 args: 1,4,3
 arg: Str('cn', cli_name='location')
@@ -761,7 +754,7 @@ option: Str('version?')
 output: Output('result')
 command: cert_request
 args: 1,6,1
-arg: File('csr', cli_name='csr_file')
+arg: Str('csr', cli_name='csr_file')
 option: Flag('add', autofill=True, default=False)
 option: Str('cacn?', cli_name='ca')
 option: Str('principal')
@@ -816,7 +809,7 @@ args: 1,6,3
 arg: Str('cn', cli_name='id')
 option: Flag('all', autofill=True, cli_name='all', default=False)
 option: Str('description', cli_name='desc')
-option: File('file', cli_name='file')
+option: Str('file', cli_name='file')
 option: Bool('ipacertprofilestoreissued', cli_name='store', default=True)
 option: Flag('raw', autofill=True, cli_name='raw', default=False)
 option: Str('version?')
@@ -830,7 +823,7 @@ option: Str('addattr*', cli_name='addattr')
 option: Flag('all', autofill=True, cli_name='all', default=False)
 option: Str('delattr*', cli_name='delattr')
 option: Str('description?', autofill=False, cli_name='desc')
-option: File('file?', cli_name='file')
+option: Str('file?', cli_name='file')
 option: Bool('ipacertprofilestoreissued?', autofill=False, cli_name='store', default=True)
 option: Flag('raw', autofill=True, cli_name='raw', default=False)
 option: Flag('rights', autofill=True, default=False)
@@ -3018,7 +3011,7 @@ arg: Str('ldapuri', cli_name='ldap_uri')
 arg: Password('bindpw', cli_name='password', confirm=False)
 option: DNParam('basedn?', cli_name='base_dn')
 option: DNParam('binddn?', autofill=True, cli_name='bind_dn', default=ipapython.dn.DN('cn=directory manager'))
-option: File('cacertfile?', cli_name='ca_cert_file')
+option: Str('cacertfile?', cli_name='ca_cert_file')
 option: Flag('compat?', autofill=True, cli_name='with_compat', default=False)
 option: Flag('continue?', autofill=True, default=False)
 option: Str('exclude_groups*', autofill=True, cli_name='exclude_groups', default=[])
@@ -3225,25 +3218,6 @@ option: Str('version?')
 output: Output('completed', type=[])
 output: Output('failed', type=[])
 output: Entry('result')
-command: otptoken_add_yubikey
-args: 1,13,3
-arg: Str('ipatokenuniqueid?', cli_name='id')
-option: Str('addattr*', cli_name='addattr')
-option: Flag('all', autofill=True, cli_name='all', default=False)
-option: Str('description?', cli_name='desc')
-option: Bool('ipatokendisabled?', cli_name='disabled')
-option: DateTime('ipatokennotafter?', 

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

[David Kupka]

On 28/04/16 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



Hello!

Patches:
replica install: fix thin client regression
schema: remove `no_cli` from command schema
schema: remove redundant information
schema: merge command args and options
schema: remove output_params
schema: add object class schema
permission: handle ipapermright deprecated CLI alias on the client
passwd: handle sort order of passwd argument on the client
misc: skip `count` and `total` output in env.output_for_cli
dns: do not rely on custom param fields in record attributes
automember: add object plugin for automember_rebuild
frontend: do not crash on missing output in output_for_cli
frontend: skip `value` output in output_for_cli
frontend: don't copy command arguments to output params
makeaci, makeapi: use in-server API

work for me, ACK.

Note: There is one one known issue in automember output, will be fixed 
later.


--
David Kupka

--
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 0162] Do not update result of *-config-show with empty server attributes

[Martin Babinsky]

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

--
Martin^3 Babinsky
From a366d731d276efd34e1f0924ddc4e51041c1814c Mon Sep 17 00:00:00 2001
From: Martin Babinsky 
Date: Mon, 20 Jun 2016 15:29:21 +0200
Subject: [PATCH] Do not update result of *-config-show with empty server
 attributes

If a server attribute such as DNSSec Key master is unset, None is passed as
the attribute value into the upper API layers and displayed in the output of
`dnsconfig-show` et al. We should not show this and leave the attribute empty
instead.

https://fedorahosted.org/freeipa/ticket/5960
---
 ipaserver/plugins/serverroles.py | 8 +---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/ipaserver/plugins/serverroles.py b/ipaserver/plugins/serverroles.py
index 4a44fca5a82038452a9e33d3df8919754b8bf2f3..e22eadd7b163469cc9fc4472640aa64d21c9d38f 100644
--- a/ipaserver/plugins/serverroles.py
+++ b/ipaserver/plugins/serverroles.py
@@ -134,9 +134,11 @@ class serverroles(Backend):
 except NotImplementedError:
 return result
 
-result.update(
-{name: attr.get(self.api) for name, attr in
- assoc_attributes.items()})
+for name, attr in assoc_attributes.items():
+attr_value = attr.get(self.api)
+
+if attr_value is not None:
+result.update({name: attr_value})
 
 return result
 
-- 
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] [PATCHES 551-552, 623-624] cert: add owner information, allow search by certificate

[Jan Cholasta]

On 20.6.2016 09:54, Jan Cholasta wrote:

On 15.6.2016 12:33, Jan Cholasta wrote:

On 14.6.2016 11:44, Jan Cholasta wrote:

On 21.4.2016 09:11, Jan Cholasta wrote:

On 6.4.2016 15:46, Pavel Vomacka wrote:



On 03/16/2016 01:50 PM, Jan Cholasta wrote:

Hi,

the attached patches implement the server-side part of
.

Honza


Hi,

thank you for the patches. I tested them and they work well. But I
would
like to ask you whether would be possible to extend the response of
'basecert_find' method and probably also 'basecert_show' response. I
think of these information:

1) information whether the certificate is issued by our CA or not.


You can check for that by comparing the issuer name of the certificate
to "CN=Certificate Authority,$SUBJECT_BASE". You can get subject base
from config-show.



2) this probably wouldn't be possible (as we discussed), but I rather
write it too - the information about revocation reason. The same as
the
'cert_show' provides.


Added --check-revocation flag to request this information. Currently it
works only on certificates issued by our CA.



3) MD5 and SHA1 fingerprints as the 'cert_show' method returns


Added, also included SHA-256.



Thank you again.


Updated patches attached.


Updated and rebased patches attached. Requires Fraser's sub-CA patches.


Attaching updated patch 623, which fixes these issues found by David:
.


Updated and rebased patches attached.


Attaching updated patches 552 and 623, which fix the --sizelimit option.

--
Jan Cholasta
From 3052e23aebd737ded4effd99329e23a24524757b Mon Sep 17 00:00:00 2001
From: Jan Cholasta 
Date: Tue, 14 Jun 2016 06:29:18 +0200
Subject: [PATCH 2/4] cert: add object plugin

Implement cert as an object with methods rather than a bunch of loosely
related commands.

https://fedorahosted.org/freeipa/ticket/5381
---
 API.txt   |  62 +++---
 VERSION   |   4 +-
 ipaclient/plugins/cert.py |   6 +-
 ipaserver/plugins/cert.py | 517 +-
 4 files changed, 327 insertions(+), 262 deletions(-)

diff --git a/API.txt b/API.txt
index eb14d44..168f536 100644
--- a/API.txt
+++ b/API.txt
@@ -730,25 +730,27 @@ output: Entry('result')
 output: Output('summary', type=[, ])
 output: PrimaryKey('value')
 command: cert_find
-args: 0,19,4
+args: 1,20,4
+arg: Str('criteria?')
 option: Flag('all', autofill=True, cli_name='all', default=False)
-option: Str('cacn?', autofill=False, cli_name='ca')
+option: Str('cacn?', cli_name='ca')
 option: Flag('exactly?', autofill=True, default=False)
-option: Str('issuedon_from?', autofill=False)
-option: Str('issuedon_to?', autofill=False)
-option: Str('issuer?', autofill=False)
+option: DateTime('issuedon_from?', autofill=False)
+option: DateTime('issuedon_to?', autofill=False)
+option: DNParam('issuer?', autofill=False)
 option: Int('max_serial_number?', autofill=False)
 option: Int('min_serial_number?', autofill=False)
+option: Flag('pkey_only?', autofill=True, default=False)
 option: Flag('raw', autofill=True, cli_name='raw', default=False)
-option: Int('revocation_reason?', autofill=False)
-option: Str('revokedon_from?', autofill=False)
-option: Str('revokedon_to?', autofill=False)
-option: Int('sizelimit?', default=100)
+option: Int('revocation_reason?', autofill=False, default=0)
+option: DateTime('revokedon_from?', autofill=False)
+option: DateTime('revokedon_to?', autofill=False)
+option: Int('sizelimit?')
 option: Str('subject?', autofill=False)
-option: Str('validnotafter_from?', autofill=False)
-option: Str('validnotafter_to?', autofill=False)
-option: Str('validnotbefore_from?', autofill=False)
-option: Str('validnotbefore_to?', autofill=False)
+option: DateTime('validnotafter_from?', autofill=False)
+option: DateTime('validnotafter_to?', autofill=False)
+option: DateTime('validnotbefore_from?', autofill=False)
+option: DateTime('validnotbefore_to?', autofill=False)
 option: Str('version?')
 output: Output('count', type=[])
 output: ListOfEntries('result')
@@ -756,37 +758,49 @@ output: Output('summary', type=[, ])
 output: Output('truncated', type=[])
 command: cert_remove_hold
 args: 1,1,1
-arg: Str('serial_number')
+arg: Int('serial_number')
 option: Str('version?')
 output: Output('result')
 command: cert_request
-args: 1,6,1
+args: 1,8,3
 arg: File('csr', cli_name='csr_file')
 option: Flag('add', autofill=True, default=False)
+option: Flag('all', autofill=True, cli_name='all', default=False)
 option: Str('cacn?', cli_name='ca')
 option: Str('principal')
 option: Str('profile_id?')
+option: Flag('raw', autofill=True, cli_name='raw', default=False)
 option: Str('request_type', autofill=True, default=u'pkcs10')
 option: Str('version?')
-output: Output('result', type=[])
+output: Entry('result')
+output: Output('summary', type=[, ])
+output: PrimaryKey('value')
 command: cert_revoke
 args: 1,2,1
-arg: 

[Freeipa-devel] [PATCH 0134] DNS: Fix realm domains integration with DNS zone add

[Petr Spacek]

Hello,

DNS: Fix realm domains integration with DNS zone add.

Realmdomains integration into DNS commands pre-dates split of DNS forward zones
and DNS master zones into two distinct commands.

There was an forgotten condition in dnszone_add command which caused omission
of DNS master zones with non-empty forwarders from realmdomain list.

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

-- 
Petr^2 Spacek
From 5e5c04ec3bd8b4c57afdcbe6dfefab25786b06b9 Mon Sep 17 00:00:00 2001
From: Petr Spacek 
Date: Mon, 20 Jun 2016 14:29:08 +0200
Subject: [PATCH] DNS: Fix realm domains integration with DNS zone add.

Realmdomains integration into DNS commands pre-dates split of DNS forward zones
and DNS master zones into two distinct commands.

There was an forgotten condition in dnszone_add command which caused omission
of DNS master zones with non-empty forwarders from realmdomain list.

https://fedorahosted.org/freeipa/ticket/5980
---
 ipaserver/plugins/dns.py | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/ipaserver/plugins/dns.py b/ipaserver/plugins/dns.py
index 094f8eb5703124a658a1becc63070b5f72fe82d8..06425affc0b9306b3f774877b713dce0c53c9b10 100644
--- a/ipaserver/plugins/dns.py
+++ b/ipaserver/plugins/dns.py
@@ -2743,11 +2743,10 @@ class dnszone_add(DNSZoneBase_add):
 assert isinstance(dn, DN)
 
 # Add entry to realmdomains
-# except for our own domain, forward zones, reverse zones and root zone
+# except for our own domain, reverse zones and root zone
 zone = keys[0]
 
 if (zone != DNSName(api.env.domain).make_absolute() and
-not options.get('idnsforwarders') and
 not zone.is_reverse() and
 zone != DNSName.root):
 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 0531] Fix undefined variable in replica install

[Martin Basti]

On 20.06.2016 13:44, Martin Basti wrote:

Patch attached.

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



Discard this, Honza set patch independently because it was related to 
thin client
-- 
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] Fix minor typos

[Martin Basti]

On 19.06.2016 10:08, Yuri Chornoivan wrote:

Hi,

Just a fix for two minor typos:

recors -> records
recieve -> receive

Thanks for fixing these typos.

Best regards,
Yuri



Thanks!
ACK

master:
* a95e0777ac64cc8edad152f189be5347117785ef Fix minor typos

-- 
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 0531] Fix undefined variable in replica install

[Martin Basti]

Patch attached.

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

From 0995aa1cabff511899d5de03972ed96818a1495f Mon Sep 17 00:00:00 2001
From: Martin Basti 
Date: Mon, 20 Jun 2016 12:35:45 +0200
Subject: [PATCH] Fix: undefined 'ipaconf' in replica promotion

ipaconf variable and target_fname variable must be extracted from try
block and used before

https://fedorahosted.org/freeipa/ticket/5975
---
 ipaserver/install/server/replicainstall.py | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/ipaserver/install/server/replicainstall.py b/ipaserver/install/server/replicainstall.py
index 3801f794998d904c0790a4bd5ad643792e13c1b9..38da85cd5fcd5ba3ad1555736bad5f2cacc00adf 100644
--- a/ipaserver/install/server/replicainstall.py
+++ b/ipaserver/install/server/replicainstall.py
@@ -1310,6 +1310,10 @@ def promote(installer):
 ccache = os.environ['KRB5CCNAME']
 remote_api = installer._remote_api
 conn = remote_api.Backend.ldap2
+ipaconf = ipaclient.ipachangeconf.IPAChangeConf("IPA Replica Promote")
+# Save client file and merge in server directives
+target_fname = paths.IPA_DEFAULT_CONF
+fstore.backup_file(target_fname)
 try:
 conn.connect(ccache=installer._ccache)
 
@@ -1319,10 +1323,6 @@ def promote(installer):
 host=[unicode(api.env.host)],
 )
 
-# Save client file and merge in server directives
-target_fname = paths.IPA_DEFAULT_CONF
-fstore.backup_file(target_fname)
-ipaconf = ipaclient.ipachangeconf.IPAChangeConf("IPA Replica Promote")
 ipaconf.setOptionAssignment(" = ")
 ipaconf.setSectionNameDelimiters(("[", "]"))
 
-- 
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] 0048-50: webui: extend topology graph functionality

[Pavel Vomacka]

On 06/13/2016 10:48 AM, Pavel Vomacka wrote:

Hello,

please review attached patches which extend topology graph 
functionality. First two add possibility to create agreement using 
mouse and the third one adds 'Autogenerated' placeholder.


0047,48: https://fedorahosted.org/freeipa/ticket/5648

0050: https://fedorahosted.org/freeipa/ticket/5867

--

Pavel^3 Vomacka



Updated patch 48 attached. Added FontAwesome to css style of plus icon 
and set lower stabilization timeout.


--
Pavel^3 Vomacka
From 31c22e520df96a357cec56d2d809889f18cefe6f Mon Sep 17 00:00:00 2001
From: Pavel Vomacka 
Date: Mon, 13 Jun 2016 10:21:25 +0200
Subject: [PATCH 1/3] Add creating a segment using mouse

Create new semicircles around the node after mouseover. These work as buttons
to create arrow and after clicking on another node the Add topology segment dialog
is opened. Also selecting segment works, if the segment already exists then
the segment is selected instead of opening the dialog.

https://fedorahosted.org/freeipa/ticket/5648
---
 install/ui/less/widgets.less |  59 --
 install/ui/src/freeipa/topology_graph.js | 347 ++-
 2 files changed, 382 insertions(+), 24 deletions(-)

diff --git a/install/ui/less/widgets.less b/install/ui/less/widgets.less
index 0f9bc8c171d5c35d955c6b55d2e95ee105c35159..686131d3222da556dffb91870f179f8270ff1419 100644
--- a/install/ui/less/widgets.less
+++ b/install/ui/less/widgets.less
@@ -150,41 +150,60 @@ tbody:empty { display: none; }
 
 .topology-view {
 svg {
-  background-color: #FFF;
-  cursor: default;
-  -webkit-user-select: none;
-  -moz-user-select: none;
-  -ms-user-select: none;
-  -o-user-select: none;
-  user-select: none;
+background-color: #FFF;
+cursor: default;
+-webkit-user-select: none;
+-moz-user-select: none;
+-ms-user-select: none;
+-o-user-select: none;
+user-select: none;
 }
 
+
 path.link {
-  fill: none;
-  stroke-width: 4px;
-  cursor: pointer;
+fill: none;
+stroke: #000;
+stroke-width: 4px;
+cursor: pointer;
+.dragline {
+pointer-events: none;
+}
+}
+
+.plus {
+font-size: .9em;
+font-family: FontAwesome;
+fill: #fff;
+}
+
+.adder_label {
+font-weight: bold;
+}
+
+path.adder {
+cursor: pointer;
+}
+
+.selected {
+stroke-dasharray: 10,2;
 }
 
 .marker {
 stroke: rgba(0, 0, 0);
 }
 
-path.link.selected {
-  stroke-dasharray: 10,2;
-}
-
 circle.node {
-  stroke-width: 1.5px;
-  cursor: pointer;
+stroke-width: 1.5px;
+cursor: pointer;
 }
 
 text {
-  font: 16px sans-serif;
-  pointer-events: none;
+font: 16px sans-serif;
+pointer-events: none;
 }
 
 text.id {
-  text-anchor: middle;
-  font-weight: bold;
+text-anchor: middle;
+font-weight: bold;
 }
 }
diff --git a/install/ui/src/freeipa/topology_graph.js b/install/ui/src/freeipa/topology_graph.js
index 332411f943770a8e1833b698ee28026e2382bb76..ce2ebeaff611987ae27f2655b5da80bdcd1b4f8a 100644
--- a/install/ui/src/freeipa/topology_graph.js
+++ b/install/ui/src/freeipa/topology_graph.js
@@ -29,13 +29,21 @@ var topology_graph = {
 topology_graph.TopoGraph = declare([Evented], {
 width: 960,
 height: 500,
+_adder_anim_duration: 200,
+_adder_inner_radius: 15,
+_adder_outer_radius: 30,
 _colors: d3.scale.category10(),
 _svg : null,
 _path: null,
 _circle: null,
 
+_create_agreement: null,
 _selected_link: null,
 _mousedown_link: null,
+_source_node: null,
+_source_node_html: null,
+_target_node: null,
+_drag_line: null,
 
 /**
  * Nodes - IPA servers
@@ -115,31 +123,89 @@ topology_graph.TopoGraph = declare([Evented], {
 node.x = Number(this._get_local_storage_attr(node.id, 'x'));
 node.y = Number(this._get_local_storage_attr(node.id, 'y'));
 }
+node.ca_adder = d3.svg.arc()
+.innerRadius(this._adder_inner_radius)
+.outerRadius(this._adder_inner_radius)
+.startAngle(2 * (Math.PI/180))
+.endAngle(178 * (Math.PI/180));
+
+node.domain_adder = d3.svg.arc()
+.innerRadius(this._adder_inner_radius)
+.outerRadius(this._adder_inner_radius)
+.startAngle(182 * (Math.PI/180))
+.endAngle(358 * (Math.PI/180));
+
+node.drag_mode = false;
 }
 
 this._init_layout();
 this._define_shapes();
 
 // handles to link and node element groups
+// the order of adding shapes is important because of order of showing
+// them
 this._path = this._svg.append('svg:g')
   

Re: [Freeipa-devel] [PATCH] pylint fixes

[Florence Blanc-Renaud]

On 06/09/2016 05:10 PM, Petr Spacek wrote:

Hello,

I've received a bunch of pylint fixes produced by upstream contributor who is
not subscribed to the list so I'm resending them here.

All credit goes to Bárta Jan<55042ba...@sstebrno.eu>.

Flo, if you have time for it I think that it could be a good exercise which
will lead you to various dark corners in IPA :-)

Petr^2 Spacek


 Forwarded Message 
Date: Fri, 3 Jun 2016 14:57:16 +0200
From: Bárta Jan<55042ba...@sstebrno.eu>
To:pspa...@redhat.com

___- In the patch 0002-pylint-fix-simplifiable-if-statement-warnings.patch:_

diff --git a/ipatests/test_integration/tasks.py 
b/ipatests/test_integration/tasks.py

index aebd907..ca2e10f 100644
--- a/ipatests/test_integration/tasks.py
+++ b/ipatests/test_integration/tasks.py
@@ -149,11 +149,7 @@ def host_service_active(host, service):
 res = host.run_command(['systemctl', 'is-active', '--quiet', service],
raiseonerr=False)

-if res.returncode == 0:
-return True
-else:
-return False
-
+return res.returncode

should be instead: return res.returncode *== 0* (otherwise the return 
type is an int and not a boolean).


In the same file:
@@ -295,11 +291,7 @@ def master_authoritative_for_client_domain(master, 
client):

 zone = ".".join(client.hostname.split('.')[1:])
 result = master.run_command(["ipa", "dnszone-show", zone],
 raiseonerr=False)
-if result.returncode == 0:
-return True
-else:
-return False
-
+result.returncode == 0

should be instead: *return* result.returncode == 0 (otherwise there is 
no return statement)


diff --git a/ipaserver/plugins/dogtag.py b/ipaserver/plugins/dogtag.py
index 197814c..36b6ba5 100644
--- a/ipaserver/plugins/dogtag.py
+++ b/ipaserver/plugins/dogtag.py
@@ -1689,12 +1689,7 @@ class ra(rabase.rabase):
 # Return command result
 cmd_result = {}

-if parse_result.get('revoked') == 'yes':
-cmd_result['revoked'] = True
-else:
-cmd_result['revoked'] = False
-
-return cmd_result
+cmd_result['revoked'] = parse_result.get('revoked')

Should be instead: cmd_result['revoked'] = parse_result.get('revoked') 
*== 'yes'* (otherwise the type is a string and not a boolean)


_- in the patch 00__04-pylint-fix-unneeded-not.patch_

@@ -632,7 +632,7 @@ class host_add(LDAPCreate):
 options['ip_address'],
 check_forward=True,
 check_reverse=check_reverse)
-if not options.get('force', False) and not 'ip_address' in options:
+if options.get('force', False) and 'ip_address' not in options:

Should be instead: if *not* options.get('force', False) and 'ip_address' 
not in options:

because of operators precedence

I will review patches 0005 to 0010 later today.
Flo.
-- 
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 0017][Tests] Fix failing ipatests/test_ipalib/test_errors.py

[Lenka Doudova]

Hi,

attaching patch to fix failing test in 
ipatests/test_ipalib/test_errors.py. Failures were caused by comparing 
unicode and non-unicode strings, hence I modified responsible 
non-unicode strings to unicode.



Lenka

From 28f4860fc22307e9e16937b0e748aa16c2e85937 Mon Sep 17 00:00:00 2001
From: Lenka Doudova 
Date: Mon, 20 Jun 2016 10:29:26 +0200
Subject: [PATCH] Tests: Fix failing ipatests/test_ipalib/test_errors.py

Some strings in the testsuite are unicode which wasn't reflected in the tests. This patch fixes the problem by changing concerned strings to unicode.
---
 ipatests/test_ipalib/test_errors.py | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/ipatests/test_ipalib/test_errors.py b/ipatests/test_ipalib/test_errors.py
index 7ad07b0418c8f1b1bb89397aa7bf6c8c2f87ab6d..8bf2cba450f26ba12d116725cca4f377029f05db 100644
--- a/ipatests/test_ipalib/test_errors.py
+++ b/ipatests/test_ipalib/test_errors.py
@@ -244,8 +244,8 @@ class test_PublicError(PublicExceptionTester):
 message = u'The translated, interpolated message'
 format = 'key=%(key1)r and key2=%(key2)r'
 uformat = u'Translated key=%(key1)r and key2=%(key2)r'
-val1 = 'Value 1'
-val2 = 'Value 2'
+val1 = u'Value 1'
+val2 = u'Value 2'
 kw = dict(key1=val1, key2=val2)
 
 # Test with format=str, message=None
@@ -304,7 +304,7 @@ class test_PublicError(PublicExceptionTester):
 format = '%(true)r %(text)r %(number)r'
 
 uformat = u'Translated %(true)r %(text)r %(number)r'
-kw = dict(true=True, text='Hello!', number=18)
+kw = dict(true=True, text=u'Hello!', number=18)
 
 # Test with format=str, message=None
 e = raises(ValueError, subclass, format, **kw)
@@ -342,7 +342,7 @@ class test_PublicError(PublicExceptionTester):
 '$')
 inst = subclass(instructions=instructions, **kw)
 assert inst.format is subclass.format
-assert_equal(inst.instructions, instructions)
+assert_equal(inst.instructions, unicode(instructions))
 inst_match = regexp.match(inst.strerror).groups()
 assert_equal(list(inst_match),list(instructions))
 
-- 
2.7.4

-- 
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] [PATCHES 551-552, 623-624] cert: add owner information, allow search by certificate

[Jan Cholasta]

On 15.6.2016 12:33, Jan Cholasta wrote:

On 14.6.2016 11:44, Jan Cholasta wrote:

On 21.4.2016 09:11, Jan Cholasta wrote:

On 6.4.2016 15:46, Pavel Vomacka wrote:



On 03/16/2016 01:50 PM, Jan Cholasta wrote:

Hi,

the attached patches implement the server-side part of
.

Honza


Hi,

thank you for the patches. I tested them and they work well. But I
would
like to ask you whether would be possible to extend the response of
'basecert_find' method and probably also 'basecert_show' response. I
think of these information:

1) information whether the certificate is issued by our CA or not.


You can check for that by comparing the issuer name of the certificate
to "CN=Certificate Authority,$SUBJECT_BASE". You can get subject base
from config-show.



2) this probably wouldn't be possible (as we discussed), but I rather
write it too - the information about revocation reason. The same as the
'cert_show' provides.


Added --check-revocation flag to request this information. Currently it
works only on certificates issued by our CA.



3) MD5 and SHA1 fingerprints as the 'cert_show' method returns


Added, also included SHA-256.



Thank you again.


Updated patches attached.


Updated and rebased patches attached. Requires Fraser's sub-CA patches.


Attaching updated patch 623, which fixes these issues found by David:
.


Updated and rebased patches attached.

--
Jan Cholasta
From ef0b52cd5f3943443cd43b64c712dc47a5ebbfd6 Mon Sep 17 00:00:00 2001
From: Jan Cholasta 
Date: Wed, 16 Mar 2016 13:09:11 +0100
Subject: [PATCH 1/4] ldap: fix handling of binary data in search filters

This fixes a UnicodeDecodeError when passing non-UTF-8 binary data to
LDAPClient.make_filter() and friends.

https://fedorahosted.org/freeipa/ticket/5381
---
 ipapython/ipaldap.py | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/ipapython/ipaldap.py b/ipapython/ipaldap.py
index 410ddae..23405c6 100644
--- a/ipapython/ipaldap.py
+++ b/ipapython/ipaldap.py
@@ -1211,7 +1211,12 @@ class LDAPClient(object):
 ]
 return self.combine_filters(flts, rules)
 elif value is not None:
-value = ldap.filter.escape_filter_chars(value_to_utf8(value))
+if isinstance(value, bytes):
+if six.PY3:
+value = value.decode('raw_unicode_escape')
+else:
+value = value_to_utf8(value)
+value = ldap.filter.escape_filter_chars(value)
 if not exact:
 template = '%s'
 if leading_wildcard:
-- 
2.7.4

From 8692fb3779ff5e6e9b394eaed11e4ad00f967912 Mon Sep 17 00:00:00 2001
From: Jan Cholasta 
Date: Tue, 14 Jun 2016 06:29:18 +0200
Subject: [PATCH 2/4] cert: add object plugin

Implement cert as an object with methods rather than a bunch of loosely
related commands.

https://fedorahosted.org/freeipa/ticket/5381
---
 API.txt   |  62 +++---
 VERSION   |   4 +-
 ipaclient/plugins/cert.py |   6 +-
 ipaserver/plugins/cert.py | 517 +-
 4 files changed, 327 insertions(+), 262 deletions(-)

diff --git a/API.txt b/API.txt
index eb14d44..168f536 100644
--- a/API.txt
+++ b/API.txt
@@ -730,25 +730,27 @@ output: Entry('result')
 output: Output('summary', type=[, ])
 output: PrimaryKey('value')
 command: cert_find
-args: 0,19,4
+args: 1,20,4
+arg: Str('criteria?')
 option: Flag('all', autofill=True, cli_name='all', default=False)
-option: Str('cacn?', autofill=False, cli_name='ca')
+option: Str('cacn?', cli_name='ca')
 option: Flag('exactly?', autofill=True, default=False)
-option: Str('issuedon_from?', autofill=False)
-option: Str('issuedon_to?', autofill=False)
-option: Str('issuer?', autofill=False)
+option: DateTime('issuedon_from?', autofill=False)
+option: DateTime('issuedon_to?', autofill=False)
+option: DNParam('issuer?', autofill=False)
 option: Int('max_serial_number?', autofill=False)
 option: Int('min_serial_number?', autofill=False)
+option: Flag('pkey_only?', autofill=True, default=False)
 option: Flag('raw', autofill=True, cli_name='raw', default=False)
-option: Int('revocation_reason?', autofill=False)
-option: Str('revokedon_from?', autofill=False)
-option: Str('revokedon_to?', autofill=False)
-option: Int('sizelimit?', default=100)
+option: Int('revocation_reason?', autofill=False, default=0)
+option: DateTime('revokedon_from?', autofill=False)
+option: DateTime('revokedon_to?', autofill=False)
+option: Int('sizelimit?')
 option: Str('subject?', autofill=False)
-option: Str('validnotafter_from?', autofill=False)
-option: Str('validnotafter_to?', autofill=False)
-option: Str('validnotbefore_from?', autofill=False)
-option: Str('validnotbefore_to?', autofill=False)
+option: DateTime('validnotafter_from?', autofill=False)
+option: DateTime('validnotafter_to?',