date:20160818

Re: [Freeipa-devel] [PATCH] 0207, 0218-0219 Solving trust conflicts and external trust topology fixes

2016-08-18 Thread Martin Babinsky


On 08/18/2016 01:25 PM, Martin Babinsky wrote:

On 08/17/2016 01:20 PM, Alexander Bokovoy wrote:

On Wed, 17 Aug 2016, Martin Babinsky wrote:

Hi Alexander,

patch 207: LGTM, but I have a feeling that the patch should be linked
to both #6021 and #6076 so that it is not lost during backports.

patch 218:

ipalib/errors.py:

1.)
I'm not sure if TrustTopologyConflictError should inherit from
InvocationError. The semantics of InvocationError implies that
something was wrong when trying to invoke the command (a param failed
to validate/convert, incorrect number of args, etc.), while this is
more of an exception during command execution (no. and type of params
was correct, command started to execute but encountered an error
condition). Thus I think it should inherit from ExecutionError. CC'ing
Jan for more thoughts on this.

Using ExecutionError would work to me too, as long as we display the
error to a user.

Why is TrustTopologyConflictSolved listed amogn public errors? Since
it is used only in dcerpc.py to restart trust establishment after
resolving conflicts, it should be a private exception in dcerpc.py for
this purpose.

I originally wanted to make it a warning -- i.e. if we fixed the
conflict, return the result and show the warning that we did solve the
conflict. After all, the code is modifying another trusted forest's
topology on behalf of the user. I can move the error class to dcerpc.py



3.)
Also please split the exception format string like this so that the
line is not too long (there is not much we can do about doctest so
leave that line as it is):

@@ -882,7 +882,8 @@ class TrustTopologyConflictError(InvocationError):
"""

errno = 3017
-format = _("Forest '%(forest)s' has existing trust to forest(s)
%(domains)s which prevents a trust to '%(conflict)s'")
+format = _("Forest '%(forest)s' has existing trust to forest(s) "
+   "%(domains)s which prevents a trust to '%(conflict)s'")

Do not worry about gettext, it can handle it just fine, there are
plenty of examples in server plugins, for example.

Done.


ipaserver/dcerpc.py:

1.)

I think that instead of returning result and raising
TrustTopologyConflictError based on that, the 'clear_ftinfo_conflict'
can raise this exception directly. You can have an empty list defined
at the beginning instead of 'result list', append unresolvable
conflicts to it and then at the end of the method check if it is
non-empty and raise the exception.

Good suggestion, fixed.



2.)

+# In the code below:
+# self -- the forest we establish trust to
+# another_domain -- a forest that establishes trust to 'self'
+# cinfo -- lsa_ForestTrustCollisionInfo structure that contain
+#  set of of lsa_ForestTrustCollisionRecord structures
I would add this directly into the method docstring:

"""
...

:param self: the forest we establish trust to
:param another_domain: a forest that establishes trust to 'self'
:param cinfo: lsa_ForestTrustCollisionInfo structure that contain
  set of of lsa_ForestTrustCollisionRecord structures
"""

Added.


Additionally, the behavior specifed in previous comment can be added
using :raises: stanza:

"""
:raises: errors.TrustTopologyConflictError if there are unresolvable
   namespace conflicts between trusted domains
"""

Added.



3.)

rewriting 'clear_ftinfo_conflict' according to point 1.) will allow to
simplify code in 'update_ftinfo' like this:

"""
-res = self.clear_ftinfo_conflict(another_domain, cinfo)
-if len(res[1]) != 0:
-domains = [x.name.string for x in res[1]]
-raise errors.TrustTopologyConflictError(
-  target=self.info['dns_domain'],
-
conflict=another_domain.info['dns_domain'],
-  domains=domains)
-else:
-raise errors.TrustTopologyConflictSolved(
-  target=self.info['dns_domain'],
-
conflict=another_domain.info['dns_domain'])
+self.clear_ftinfo_conflict(another_domain, cinfo)
+raise errors.TrustTopologyConflictSolved(
+target=self.info['dns_domain'],
+conflict=another_domain.info['dns_domain'])
"""

done.



Patch 218:

1.)
typo in the commit message:

"""
...
suffixes are forest-wide, there *are could be* user accounts in the
...
"""

Fixed.

Updated patches attached.


PATCH 207: ACK, I am attaching rebased version for ipa-4-3. Please check
if the rebase is correct.

PATCH 218: I am attaching rebased version for control. Unfortunately, I
am unable to properly test conflict resolution due to reasons beyond my
control but it does not break any ordinary workflows and code looks OK,
so ACK.



I have noticed that raising of TrustTopologyConflictSolved is broken. I 
have changed the base class to Exception and it works. Attaching patches 
with the change.



PATCH 219: ACK






--
Martin^3

[Freeipa-devel] FreeIPA wiki - fighting the spammers

2016-08-18 Thread Martin Kosek

Hello everyone,

As some of you noticed, we had lately an increasing number of spam attacks
against FreeIPA.org wiki. Even though we did not accept user registration
through the standard Mediawiki User Creation form (which is often misused by
attacked) and only allow Fedora users logged in by OpenID, the spam producers
started to favor this authentication mode too.

This week and especially yesterday, the spam activity was high enough to
warrant a "drastic change" in how we allow wiki modifications. Me and Petr
Vobornik had to react quickly yesterday to hundreds of new spam-based pages
(thanks to Petr for deleting the spam pages, Stephen for altering us and
Patrick Uiterwijk for advising us!).

As a prevention for future attacks, I also needed to chose the most simple and
convenient method to stop spammers from editing wiki. This is the result:

- Anonymous and regular users are no longer allowed to create or edit wiki pages
- Anyone who wants to be able to edit wiki needs to be in a new "editor" group

The full description of new group rights is here:
http://www.freeipa.org/page/Special:ListGroupRights

I added the contributors active in the last 30 days to the editor group. If
more people are needed, wiki "Bureaucrats" can it through this form:
http://www.freeipa.org/page/Special:UserRights

If you do not know who is in the Bureaucrat group, this is the list:
http://www.freeipa.org/index.php?title=Special%3AListUsers==bureaucrat=50

The model I had in mind was that new wiki contributors would ask for access on
#freeipa IRC channel, when they have some content to be added to the pages. We
should probably add that suggestion to the wiki somewhere.

I hope this works for you. If you have comments on above or even better ideas
what is the easiest way to fight spam on our precious wiki, please let me know.

-- 
Martin Kosek 
Manager, Software Engineering - Identity Management Team
Red Hat, Inc.

-- 
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 0004] [Test] Test for caacl-add-service: incorrect error message when service does not exists

2016-08-18 Thread Martin Basti




On 18.08.2016 15:02, Tomas Krizek wrote:


Hi,

NACK.

The issue is not that the error message contains the "no such entry" 
string. That is actually a valid part of the error message if the 
service indeed does not exist.


The problem is that the error message contained only the first 
character of the service's name instead of the entire name of the 
service. For example, the command


ipa caacl-add-service test_caacl --services svc/master2.ipa.test --services 
svc/master1.ipa.test

should end with these error messages if those services do not exist:

 member service:svc/master2.ipa.t...@ipa.test: no such entry
 member service:svc/master1.ipa.t...@ipa.test: no such entry

There is also a typo in the file name of the test and I'm also not sure if 
there isn't a better place to put the test.


+1

This should not be integration_test but only test_xmlrpc

Martin^2


On 08/18/2016 01:48 PM, Ganna Kaihorodova wrote:

Hello!

Test for caacl-add-service: incorrect error message when service does not exists
https://fedorahosted.org/freeipa/ticket/6171

Best regards,
Ganna Kaihorodova
Associate Software Quality Engineer


:




--
Tomas Krizek




-- 
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 0004] [Test] Test for caacl-add-service: incorrect error message when service does not exists

2016-08-18 Thread Tomas Krizek


Hi,

NACK.

The issue is not that the error message contains the "no such entry" 
string. That is actually a valid part of the error message if the 
service indeed does not exist.


The problem is that the error message contained only the first character 
of the service's name instead of the entire name of the service. For 
example, the command


ipa caacl-add-service test_caacl --services svc/master2.ipa.test --services 
svc/master1.ipa.test

should end with these error messages if those services do not exist:

member service: svc/master2.ipa.t...@ipa.test: no such entry
member service: svc/master1.ipa.t...@ipa.test: no such entry

There is also a typo in the file name of the test and I'm also not sure if 
there isn't a better place to put the test.

On 08/18/2016 01:48 PM, Ganna Kaihorodova wrote:

Hello!

Test for caacl-add-service: incorrect error message when service does not exists
https://fedorahosted.org/freeipa/ticket/6171

Best regards,
Ganna Kaihorodova
Associate Software Quality Engineer


:




--
Tomas Krizek

-- 
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 0562] Fix: container owner should be able to add vault

2016-08-18 Thread Tomas Krizek


I forgot about the oneliner rule, but for what it's worth - ACK.


On 08/18/2016 01:04 PM, Martin Basti wrote:




On 18.08.2016 12:04, Martin Basti wrote:

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


Patch attached.




Pushed to master: 6b7d6417d403c983691c790c1e60cfe32bf1c420

Pushed under oneliner rule




--
Tomas Krizek

-- 
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 0004] [Test] Test for caacl-add-service: incorrect error message when service does not exists

2016-08-18 Thread Ganna Kaihorodova

Hello!

Test for caacl-add-service: incorrect error message when service does not exists
https://fedorahosted.org/freeipa/ticket/6171

Best regards,
Ganna Kaihorodova
Associate Software Quality Engineer


:From 837d85f0b3e8544a983e1ceeb770712807c0f0cf Mon Sep 17 00:00:00 2001
From: Ganna Kaihorodova 
Date: Thu, 18 Aug 2016 11:16:49 +0200
Subject: [PATCH] Test for caacl-add-service

Test for caacl-add-service: incorrect error message when service does not exists

https://fedorahosted.org/freeipa/ticket/6171
---
 ipatests/test_integration/test_caal_message.py | 20 
 1 file changed, 20 insertions(+)
 create mode 100644 ipatests/test_integration/test_caal_message.py

diff --git a/ipatests/test_integration/test_caal_message.py b/ipatests/test_integration/test_caal_message.py
new file mode 100644
index ..d9c5d4e5b74aeec87b6985ed94a69bcc9d9435ff
--- /dev/null
+++ b/ipatests/test_integration/test_caal_message.py
@@ -0,0 +1,20 @@
+from ipatests.test_integration.base import IntegrationTest
+
+
+class TestIncorrectErrorMessage(IntegrationTest):
+
+"https://fedorahosted.org/freeipa/ticket/6171;
+
+topology = 'star'
+num_replicas = 0
+caacl = 'test_caacl'
+
+def test_errormsg(self):
+master = self.master
+master.run_command("ipa caacl-add %s --desc \"test\"" % self.caacl)
+result = master.run_command(
+"ipa caacl-add-service %s --services"
+"svc/`hostname`" % self.caacl, raiseonerr=False
+)
+assert("no such entry" not in result.stderr_text), (
+"Wrong error message")
-- 
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] [PATCH] 0207, 0218-0219 Solving trust conflicts and external trust topology fixes

2016-08-18 Thread Martin Babinsky


On 08/17/2016 01:20 PM, Alexander Bokovoy wrote:

On Wed, 17 Aug 2016, Martin Babinsky wrote:

Hi Alexander,

patch 207: LGTM, but I have a feeling that the patch should be linked
to both #6021 and #6076 so that it is not lost during backports.

patch 218:

ipalib/errors.py:

1.)
I'm not sure if TrustTopologyConflictError should inherit from
InvocationError. The semantics of InvocationError implies that
something was wrong when trying to invoke the command (a param failed
to validate/convert, incorrect number of args, etc.), while this is
more of an exception during command execution (no. and type of params
was correct, command started to execute but encountered an error
condition). Thus I think it should inherit from ExecutionError. CC'ing
Jan for more thoughts on this.

Using ExecutionError would work to me too, as long as we display the
error to a user.

Why is TrustTopologyConflictSolved listed amogn public errors? Since
it is used only in dcerpc.py to restart trust establishment after
resolving conflicts, it should be a private exception in dcerpc.py for
this purpose.

I originally wanted to make it a warning -- i.e. if we fixed the
conflict, return the result and show the warning that we did solve the
conflict. After all, the code is modifying another trusted forest's
topology on behalf of the user. I can move the error class to dcerpc.py



3.)
Also please split the exception format string like this so that the
line is not too long (there is not much we can do about doctest so
leave that line as it is):

@@ -882,7 +882,8 @@ class TrustTopologyConflictError(InvocationError):
"""

errno = 3017
-format = _("Forest '%(forest)s' has existing trust to forest(s)
%(domains)s which prevents a trust to '%(conflict)s'")
+format = _("Forest '%(forest)s' has existing trust to forest(s) "
+   "%(domains)s which prevents a trust to '%(conflict)s'")

Do not worry about gettext, it can handle it just fine, there are
plenty of examples in server plugins, for example.

Done.


ipaserver/dcerpc.py:

1.)

I think that instead of returning result and raising
TrustTopologyConflictError based on that, the 'clear_ftinfo_conflict'
can raise this exception directly. You can have an empty list defined
at the beginning instead of 'result list', append unresolvable
conflicts to it and then at the end of the method check if it is
non-empty and raise the exception.

Good suggestion, fixed.



2.)

+# In the code below:
+# self -- the forest we establish trust to
+# another_domain -- a forest that establishes trust to 'self'
+# cinfo -- lsa_ForestTrustCollisionInfo structure that contain
+#  set of of lsa_ForestTrustCollisionRecord structures
I would add this directly into the method docstring:

"""
...

:param self: the forest we establish trust to
:param another_domain: a forest that establishes trust to 'self'
:param cinfo: lsa_ForestTrustCollisionInfo structure that contain
  set of of lsa_ForestTrustCollisionRecord structures
"""

Added.


Additionally, the behavior specifed in previous comment can be added
using :raises: stanza:

"""
:raises: errors.TrustTopologyConflictError if there are unresolvable
   namespace conflicts between trusted domains
"""

Added.



3.)

rewriting 'clear_ftinfo_conflict' according to point 1.) will allow to
simplify code in 'update_ftinfo' like this:

"""
-res = self.clear_ftinfo_conflict(another_domain, cinfo)
-if len(res[1]) != 0:
-domains = [x.name.string for x in res[1]]
-raise errors.TrustTopologyConflictError(
-  target=self.info['dns_domain'],
-
conflict=another_domain.info['dns_domain'],
-  domains=domains)
-else:
-raise errors.TrustTopologyConflictSolved(
-  target=self.info['dns_domain'],
-
conflict=another_domain.info['dns_domain'])
+self.clear_ftinfo_conflict(another_domain, cinfo)
+raise errors.TrustTopologyConflictSolved(
+target=self.info['dns_domain'],
+conflict=another_domain.info['dns_domain'])
"""

done.



Patch 218:

1.)
typo in the commit message:

"""
...
suffixes are forest-wide, there *are could be* user accounts in the
...
"""

Fixed.

Updated patches attached.


PATCH 207: ACK, I am attaching rebased version for ipa-4-3. Please check 
if the rebase is correct.


PATCH 218: I am attaching rebased version for control. Unfortunately, I 
am unable to properly test conflict resolution due to reasons beyond my 
control but it does not break any ordinary workflows and code looks OK, 
so ACK.


PATCH 219: ACK

--
Martin^3 Babinsky
From c1250c835e1b7e2860c6307a86c2d810405666ff Mon Sep 17 00:00:00 2001
From: Alexander Bokovoy 
Date: Tue, 7 Jun 2016 22:41:10 +0300
Subject: [PATCH 1/2] ipaserver/dcerpc: reformat to

Re: [Freeipa-devel] [PATCH 0562] Fix: container owner should be able to add vault

2016-08-18 Thread Martin Basti




On 18.08.2016 12:04, Martin Basti wrote:

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


Patch attached.




Pushed to master: 6b7d6417d403c983691c790c1e60cfe32bf1c420

Pushed under oneliner rule
-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [PATCH 0112-7] Speeding up cli help

2016-08-18 Thread Jan Cholasta


On 18.8.2016 11:06, David Kupka wrote:

On 17/08/16 14:17, Jan Cholasta wrote:

On 17.8.2016 13:21, David Kupka wrote:

On 08/08/16 13:26, Jan Cholasta wrote:

On 4.8.2016 16:32, David Kupka wrote:

On 03/08/16 16:33, Jan Cholasta wrote:

On 3.8.2016 16:23, David Kupka wrote:

On 21/07/16 10:12, Jan Cholasta wrote:

Hi,

On 20.7.2016 14:32, David Kupka wrote:

On 15/07/16 12:53, David Kupka wrote:

Hello!

After Honza introduced thin client that builds plugins and
commands
dynamically from schema client became much slower. This is only
logical,
instead of importing a module client now must fetch the schema
from
server, parse it and instantiate the commands using the data.

First step to speed it up was addition of schema cache to client.
That
removed the RTT and download time of fetching schema every time.

Now the most time consuming task became displaying help for
lists of
topics and command and displaying individual topics. This is
simply
because of the need to instantiate all the commands to find the
relations between topics and commands.

All the necessary bits for server commands and topics are
already in
the
schema cache so we can skip this part and generate help from it,
right?
Not so fast!

There are client plugins with commands and topics. So we can
generate
basic bits (list of all topics, list of all commands, list of
commands
for each topic) from schema and store it in cache. Then we need
to go
through all client plugins and get similar bits for client
plugins.
Then
we can merge and print.

Still the client response is not as fast as before and I this it
even
can't be. Also first time you display particular topic or list
takes
longer because it must be freshly generated and stored in cache
for
next
use. And this is what the attached patches do.

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


Reimplemented so there is no need to distinguish client plugins
and
remote plugins.
The main idea of this approach is to avoid creating instances of
the
commands just to get the information about topic, name and summary
needed for displaying help. Instead class properties are used to
access
the information directly in schema.


Patch 0112:

I think this would better be done in Schema.read_namespace_member,
because Schema is where all the state is.

(BTW does _SchemaNameSpace.__getitem__ raise KeyError for
non-existent
keys? It looks like it doesn't.)


Patch 0113:

How about setting _schema_cached to False in Schema.__init__()
rather
that getattr()-ing it in _ensure_cached()?


Patch 0116:

ClientCommand.doc should be a class property as well, otherwise
.summary
won't work on it correctly.

_SchemaCommand.doc should not be a property, as it's not needed for
.summary to work on it correctly.


Otherwise works fine for me.

Honza



Updated patches attached.


Thanks, ACK.

Pushed to master: 229e2a1ed9ea9877cb5e879fadd99f9040f77c96



I've made and reviewer overlooked some errors. Attached patches fix
them.


Shame on me.

Anyway,

1) When schema() raises SchemaUpToDate, the current code explodes:

ipa: ERROR: KeyError: 'fingerprint'
Traceback (most recent call last):
  File "/usr/lib/python2.7/site-packages/ipalib/cli.py", line 1348, in
run
api.finalize()
  File "/usr/lib/python2.7/site-packages/ipalib/plugable.py", line 707,
in finalize
self.__do_if_not_done('load_plugins')
  File "/usr/lib/python2.7/site-packages/ipalib/plugable.py", line 422,
in __do_if_not_done
getattr(self, name)()
  File "/usr/lib/python2.7/site-packages/ipalib/plugable.py", line 585,
in load_plugins
for package in self.packages:
  File "/usr/lib/python2.7/site-packages/ipalib/__init__.py", line 919,
in packages
ipaclient.remote_plugins.get_package(self),
  File
"/usr/lib/python2.7/site-packages/ipaclient/remote_plugins/__init__.py",

line 92, in get_package
plugins = schema.get_package(api, server_info, client)
  File
"/usr/lib/python2.7/site-packages/ipaclient/remote_plugins/schema.py",
line 558, in get_package
fingerprint = str(schema['fingerprint'])
  File
"/usr/lib/python2.7/site-packages/ipaclient/remote_plugins/schema.py",
line 483, in __getitem__
return self._dict[key]
KeyError: 'fingerprint'


2) We read server info every time get_package() is called. It should be
cache similarly to how the schema is cached in the api object.


3) Some of the commands are still fully initialized during "ipa help
commands".


Honza


Updated patches attached.


Thanks, ACK.

Pushed to master: 6e6cbda036559e741ead0ab5ba18b0be0b41621e



When locale is not available setlocale() explodes. Handling this
exception in the same way as in ipalib/rpc.py to behave consistently.


Works for me, ACK.

Pushed to master: b6d5ed139b261b5db078ab652d22ea1d3b8092d3

--
Jan Cholasta

--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

[Freeipa-devel] [PATCH 0562] Fix: container owner should be able to add vault

2016-08-18 Thread Martin Basti


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


Patch attached.

From 7277e6ac97bacb86ea8eb56125b8071165e2f777 Mon Sep 17 00:00:00 2001
From: Martin Basti 
Date: Thu, 18 Aug 2016 10:11:25 +0200
Subject: [PATCH] Fix: container owner should be able to add vault

With recent change in DS (CVE fix), ds is not returging DuplicatedEntry
error in case that user is not permitted by ACI to write, but ACIError instead.

Is safe to ignore ACI error in container, because it will be raised
again later if user has no access to container.

https://fedorahosted.org/freeipa/ticket/6159
---
 ipaserver/plugins/vault.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ipaserver/plugins/vault.py b/ipaserver/plugins/vault.py
index c9b7cb942cfbca74134bce4ba039619b4f5f2845..5c4c09685ceb95c6634306c4275008d602099e12 100644
--- a/ipaserver/plugins/vault.py
+++ b/ipaserver/plugins/vault.py
@@ -783,7 +783,7 @@ class vault_add_internal(LDAPCreate):
 
 try:
 self.obj.create_container(parent_dn, owner_dn)
-except errors.DuplicateEntry as e:
+except (errors.DuplicateEntry, errors.ACIError):
 pass
 
 # vault should be owned by the creator
-- 
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] [PATCH 0112-7] Speeding up cli help

2016-08-18 Thread David Kupka


On 17/08/16 14:17, Jan Cholasta wrote:

On 17.8.2016 13:21, David Kupka wrote:

On 08/08/16 13:26, Jan Cholasta wrote:

On 4.8.2016 16:32, David Kupka wrote:

On 03/08/16 16:33, Jan Cholasta wrote:

On 3.8.2016 16:23, David Kupka wrote:

On 21/07/16 10:12, Jan Cholasta wrote:

Hi,

On 20.7.2016 14:32, David Kupka wrote:

On 15/07/16 12:53, David Kupka wrote:

Hello!

After Honza introduced thin client that builds plugins and
commands
dynamically from schema client became much slower. This is only
logical,
instead of importing a module client now must fetch the schema
from
server, parse it and instantiate the commands using the data.

First step to speed it up was addition of schema cache to client.
That
removed the RTT and download time of fetching schema every time.

Now the most time consuming task became displaying help for
lists of
topics and command and displaying individual topics. This is
simply
because of the need to instantiate all the commands to find the
relations between topics and commands.

All the necessary bits for server commands and topics are
already in
the
schema cache so we can skip this part and generate help from it,
right?
Not so fast!

There are client plugins with commands and topics. So we can
generate
basic bits (list of all topics, list of all commands, list of
commands
for each topic) from schema and store it in cache. Then we need
to go
through all client plugins and get similar bits for client
plugins.
Then
we can merge and print.

Still the client response is not as fast as before and I this it
even
can't be. Also first time you display particular topic or list
takes
longer because it must be freshly generated and stored in cache
for
next
use. And this is what the attached patches do.

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


Reimplemented so there is no need to distinguish client plugins and
remote plugins.
The main idea of this approach is to avoid creating instances of
the
commands just to get the information about topic, name and summary
needed for displaying help. Instead class properties are used to
access
the information directly in schema.


Patch 0112:

I think this would better be done in Schema.read_namespace_member,
because Schema is where all the state is.

(BTW does _SchemaNameSpace.__getitem__ raise KeyError for
non-existent
keys? It looks like it doesn't.)


Patch 0113:

How about setting _schema_cached to False in Schema.__init__()
rather
that getattr()-ing it in _ensure_cached()?


Patch 0116:

ClientCommand.doc should be a class property as well, otherwise
.summary
won't work on it correctly.

_SchemaCommand.doc should not be a property, as it's not needed for
.summary to work on it correctly.


Otherwise works fine for me.

Honza



Updated patches attached.


Thanks, ACK.

Pushed to master: 229e2a1ed9ea9877cb5e879fadd99f9040f77c96



I've made and reviewer overlooked some errors. Attached patches fix
them.


Shame on me.

Anyway,

1) When schema() raises SchemaUpToDate, the current code explodes:

ipa: ERROR: KeyError: 'fingerprint'
Traceback (most recent call last):
  File "/usr/lib/python2.7/site-packages/ipalib/cli.py", line 1348, in
run
api.finalize()
  File "/usr/lib/python2.7/site-packages/ipalib/plugable.py", line 707,
in finalize
self.__do_if_not_done('load_plugins')
  File "/usr/lib/python2.7/site-packages/ipalib/plugable.py", line 422,
in __do_if_not_done
getattr(self, name)()
  File "/usr/lib/python2.7/site-packages/ipalib/plugable.py", line 585,
in load_plugins
for package in self.packages:
  File "/usr/lib/python2.7/site-packages/ipalib/__init__.py", line 919,
in packages
ipaclient.remote_plugins.get_package(self),
  File
"/usr/lib/python2.7/site-packages/ipaclient/remote_plugins/__init__.py",
line 92, in get_package
plugins = schema.get_package(api, server_info, client)
  File
"/usr/lib/python2.7/site-packages/ipaclient/remote_plugins/schema.py",
line 558, in get_package
fingerprint = str(schema['fingerprint'])
  File
"/usr/lib/python2.7/site-packages/ipaclient/remote_plugins/schema.py",
line 483, in __getitem__
return self._dict[key]
KeyError: 'fingerprint'


2) We read server info every time get_package() is called. It should be
cache similarly to how the schema is cached in the api object.


3) Some of the commands are still fully initialized during "ipa help
commands".


Honza


Updated patches attached.


Thanks, ACK.

Pushed to master: 6e6cbda036559e741ead0ab5ba18b0be0b41621e



When locale is not available setlocale() explodes. Handling this 
exception in the same way as in ipalib/rpc.py to behave consistently.


--
David Kupka
From 083ad6016999eb244dc9915ec5fe6988f9053b0d Mon Sep 17 00:00:00 2001
From: David Kupka 
Date: Thu, 18 Aug 2016 10:59:09 +0200
Subject: [PATCH] schema cache: Fallback to 'en_us' when locale is not
 available

https://fedorahosted.org/freeipa/ticket/6204
---
 ipaclient/remote_plugins/schema.py | 12 +---
 1 file changed, 9 insertions(+), 3

Re: [Freeipa-devel] [PATCH 689] tests: fix test_ipalib.test_frontend.test_Object

2016-08-18 Thread Martin Basti




On 18.08.2016 10:56, Petr Spacek wrote:

On 18.8.2016 10:08, Jan Cholasta wrote:

SSIA

Could you add one sentence or a link to a ticket which forced this change?

When reading the patch, I have no way to say why the change is necessary - so
it is impossible to verify correctness. (Sure, the test will pass, but I have
no way to distinguish incorrect test passing on incorrect implementation vs.
correct test passing on correct implementation.)


+1

and add there this link also https://fedorahosted.org/freeipa/ticket/6188

--
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 689] tests: fix test_ipalib.test_frontend.test_Object

2016-08-18 Thread Petr Spacek

On 18.8.2016 10:08, Jan Cholasta wrote:
> SSIA

Could you add one sentence or a link to a ticket which forced this change?

When reading the patch, I have no way to say why the change is necessary - so
it is impossible to verify correctness. (Sure, the test will pass, but I have
no way to distinguish incorrect test passing on incorrect implementation vs.
correct test passing on correct implementation.)

-- 
Petr^2 Spacek

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] pam_sss(sshd:auth): authentication failure; logname= uid=0 euid=0 tty=ssh ruser= rhost=ilt-gif-ipa01.ipa.preprod.local user=adu...@corp.addomain.com

2016-08-18 Thread Jakub Hrozek

On Thu, Aug 18, 2016 at 09:48:59AM +0200, rajat gupta wrote:
> Thanks.
> 
> When i am trying to accesses user with password i am getting below message
> in logs.
> 
> *Aug 18 09:38:17 ilt-gif-ipa02 [sssd[krb5_child[8505]]]: Cannot find KDC
> for realm "ADDOMAON.COM "*
> 
> when i connect through ssh, it tries to contact the KDC for the realm
> *ADDOMAON.COM
> *
> 
> which should be corp.addomain.com
> 
> 
> Do you have any further comments or suggestions that may help us.

Can we see the sssd logs as requested before please?

https://fedorahosted.org/sssd/wiki/Troubleshooting

-- 
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 689] tests: fix test_ipalib.test_frontend.test_Object

2016-08-18 Thread Jan Cholasta


SSIA

--
Jan Cholasta
From c3f3ffd235b39fbdc61d8ae0b3f55eca97613499 Mon Sep 17 00:00:00 2001
From: Jan Cholasta 
Date: Thu, 18 Aug 2016 10:04:59 +0200
Subject: [PATCH] tests: fix test_ipalib.test_frontend.test_Object

---
 ipatests/test_ipalib/test_frontend.py | 12 +---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/ipatests/test_ipalib/test_frontend.py b/ipatests/test_ipalib/test_frontend.py
index c3dd910..ac299e2 100644
--- a/ipatests/test_ipalib/test_frontend.py
+++ b/ipatests/test_ipalib/test_frontend.py
@@ -922,6 +922,9 @@ class test_Object(ClassChecker):
 self.name = '%s_%s' % (obj_name, attr_name)
 else:
 self.name = name
+self.bases = (DummyAttribute,)
+self.version = '1'
+self.full_name = '{}/{}'.format(self.name, self.version)
 self.param = frontend.create_param(attr_name)
 
 def __clone__(self, attr_name):
@@ -940,15 +943,18 @@ class test_Object(ClassChecker):
 methods_format = 'method_%d'
 
 class FakeAPI(object):
-Method = NameSpace(
-get_attributes(cnt, methods_format)
-)
+def __init__(self):
+self._API__plugins = get_attributes(cnt, methods_format)
+self._API__default_map = {}
+self.Method = plugable.APINameSpace(self, DummyAttribute)
 def __contains__(self, key):
 return hasattr(self, key)
 def __getitem__(self, key):
 return getattr(self, key)
 def is_production_mode(self):
 return False
+def _get(self, plugin):
+return plugin
 api = FakeAPI()
 assert len(api.Method) == cnt * 3
 
-- 
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] pam_sss(sshd:auth): authentication failure; logname= uid=0 euid=0 tty=ssh ruser= rhost=ilt-gif-ipa01.ipa.preprod.local user=adu...@corp.addomain.com

2016-08-18 Thread rajat gupta

Thanks.

When i am trying to accesses user with password i am getting below message
in logs.

*Aug 18 09:38:17 ilt-gif-ipa02 [sssd[krb5_child[8505]]]: Cannot find KDC
for realm "ADDOMAON.COM "*

when i connect through ssh, it tries to contact the KDC for the realm
*ADDOMAON.COM
*

which should be corp.addomain.com


Do you have any further comments or suggestions that may help us.


/Rajat



On Tue, Aug 16, 2016 at 2:46 PM, Alexander Bokovoy 
wrote:

> On Tue, 16 Aug 2016, rajat gupta wrote:
>
>> Hi,
>>
>>
>> I have done IPA AD trust between IPA and AD server. But trust is showing
>> offline always. But we are able to get the AD user information. And able
>> to
>> grant the  KRB ticket.
>>
>>
>>
>> # wbinfo --online-status
>> BUILTIN : online
>> IPA : online
>> *CORP : offline*
>>
> Don't use wbinfo. Its output is irrelevant starting from FreeIPA 3.3.
>
>
>>
>> #id adu...@corp.addomain.com
>> uid=1007656917(adu...@corp.addomain.com) gid=1007656917(
>> adu...@corp.addomain.com) groups=1007656917(adu...@corp.addomain.com
>> ),1007715891(prg-msoffice2013pro(kms)@corp.addomain.com),1007663829(
>> da-eeg-intra-r...@corp.addomain.com),1007600513(domain
>> us...@corp.addomain.com)
>>
>>
>> [root@ilt-gif-ipa01 ~]# kinit  adu...@corp.addomain.com
>> Password for adu...@corp.addomain.com:
>> [root@ilt-gif-ipa01 ~]#
>> [root@ilt-gif-ipa01 ~]#
>> [root@ilt-gif-ipa01 ~]# klist
>> Ticket cache: KEYRING:persistent:0:0
>> Default principal: adu...@corp.addomain.com
>>
>> Valid starting   Expires  Service principal
>> 08/11/2016 13:11:35  08/11/2016 23:11:35  krbtgt/
>> corp.addomain@corp.addomain.com
>>renew until 08/12/2016 13:11:29
>> [root@ilt-gif-ipa01 ~]#
>>
> This is irrelevant for the trust case because you are authenticating
> against AD DCs, not IPA KDCs.
>
>
>>
>>
>> Form IPA client server we are able to get the all thinks ( KRB ticket/
>> user/groups )
>>
>> [root@ilt-gif-ipa02 ~]# getent passwd adu...@corp.addomain.com
>> adu...@corp.addomain.com:*:1007656917:1007656917:USER  NAME:/home/
>> corp.addomain.com/aduser:
>> [root@ilt-gif-ipa02 ~]#
>>
>>
>> [root@ilt-gif-ipa02 ~]# getent group adu...@corp.addomain.com
>> adu...@corp.addomain.com:*:1007656917:
>> [root@ilt-gif-ipa02 ~]#
>>
>>
>> [root@ilt-gif-ipa02 ~]# id adu...@corp.addomain.com
>> uid=1007656917(adu...@corp.addomain.com) gid=1007656917(
>> adu...@corp.addomain.com) groups=1007656917(adu...@corp.addomain.com
>> ),1007715891(prg-msoffice2013pro(kms)@corp.addomain.com),1007663829(
>> da-eeg-intra-r...@corp.addomain.com),1007600513(domain
>> us...@corp.addomain.com),1007725088(tfs_us...@corp.addomain.com)
>>
>>
>> Also we are to ssh  to IPA client on same machine or from some other
>> machine with gss authentication. But using password authentication it’s
>> failed to login.
>>
>> *ERROR:- pam_sss(sshd:auth): authentication failure; logname*
>>
>>
>>
>> kinit adu...@corp.addomain.com
>> Password for adu...@corp.addomain.com:
>>
>>
>>
>> [root@ilt-gif-ipa02 ~]# ssh -vl adu...@corp.addomain.com
>> ilt-gif-ipa02.ipa.preprod.local
>> OpenSSH_6.6.1, OpenSSL 1.0.1e-fips 11 Feb 2013
>> debug1: Reading configuration data /etc/ssh/ssh_config
>> debug1: /etc/ssh/ssh_config line 60: Applying options for *
>> debug1: Executing proxy command: exec /usr/bin/sss_ssh_knownhostsproxy -p
>> 22 ilt-gif-ipa02.ipa.preprod.local
>> debug1: permanently_set_uid: 0/0
>> debug1: permanently_drop_suid: 0
>> debug1: identity file /root/.ssh/id_rsa type -1
>> debug1: identity file /root/.ssh/id_rsa-cert type -1
>> debug1: identity file /root/.ssh/id_dsa type -1
>> debug1: identity file /root/.ssh/id_dsa-cert type -1
>> debug1: identity file /root/.ssh/id_ecdsa type -1
>> debug1: identity file /root/.ssh/id_ecdsa-cert type -1
>> debug1: identity file /root/.ssh/id_ed25519 type -1
>> debug1: identity file /root/.ssh/id_ed25519-cert type -1
>> debug1: Enabling compatibility mode for protocol 2.0
>> debug1: Local version string SSH-2.0-OpenSSH_6.6.1
>> debug1: Remote protocol version 2.0, remote software version OpenSSH_6.6.1
>> debug1: match: OpenSSH_6.6.1 pat OpenSSH_6.6.1* compat 0x0400
>> debug1: SSH2_MSG_KEXINIT sent
>> debug1: SSH2_MSG_KEXINIT received
>> debug1: kex: server->client aes128-ctr hmac-md5-...@openssh.com none
>> debug1: kex: client->server aes128-ctr hmac-md5-...@openssh.com none
>> debug1: kex: curve25519-sha...@libssh.org need=16 dh_need=16
>> debug1: kex: curve25519-sha...@libssh.org need=16 dh_need=16
>> debug1: sending SSH2_MSG_KEX_ECDH_INIT
>> debug1: expecting SSH2_MSG_KEX_ECDH_REPLY
>> debug1: Server host key: ECDSA
>> f0:e6:b2:66:c8:41:06:4e:83:a4:a2:c5:5a:57:24:66
>> debug1: Host 'ilt-gif-ipa02.ipa.preprod.local' is known and matches the
>> ECDSA host key.
>> debug1: Found key in /root/.ssh/known_hosts:3
>> debug1: ssh_ecdsa_verify: signature correct
>> debug1: SSH2_MSG_NEWKEYS sent
>> debug1: expecting SSH2_MSG_NEWKEYS
>> debug1: SSH2_MSG_NEWKEYS received
>>

Re: [Freeipa-devel] [PATCH] 0207, 0218-0219 Solving trust conflicts and external trust topology fixes

[Freeipa-devel] FreeIPA wiki - fighting the spammers

Re: [Freeipa-devel] [PATCH 0004] [Test] Test for caacl-add-service: incorrect error message when service does not exists

Re: [Freeipa-devel] [PATCH 0004] [Test] Test for caacl-add-service: incorrect error message when service does not exists

Re: [Freeipa-devel] [PATCH 0562] Fix: container owner should be able to add vault

[Freeipa-devel] [PATCH 0004] [Test] Test for caacl-add-service: incorrect error message when service does not exists

Re: [Freeipa-devel] [PATCH] 0207, 0218-0219 Solving trust conflicts and external trust topology fixes

Re: [Freeipa-devel] [PATCH 0562] Fix: container owner should be able to add vault

Re: [Freeipa-devel] [PATCH 0112-7] Speeding up cli help

[Freeipa-devel] [PATCH 0562] Fix: container owner should be able to add vault

Re: [Freeipa-devel] [PATCH 0112-7] Speeding up cli help

Re: [Freeipa-devel] [PATCH 689] tests: fix test_ipalib.test_frontend.test_Object

Re: [Freeipa-devel] [PATCH 689] tests: fix test_ipalib.test_frontend.test_Object

Re: [Freeipa-devel] pam_sss(sshd:auth): authentication failure; logname= uid=0 euid=0 tty=ssh ruser= rhost=ilt-gif-ipa01.ipa.preprod.local user=adu...@corp.addomain.com

[Freeipa-devel] [PATCH 689] tests: fix test_ipalib.test_frontend.test_Object

Re: [Freeipa-devel] pam_sss(sshd:auth): authentication failure; logname= uid=0 euid=0 tty=ssh ruser= rhost=ilt-gif-ipa01.ipa.preprod.local user=adu...@corp.addomain.com

16 matches

Site Navigation

Mail list logo

Footer information