On 14.12.2015 10:27, Martin Basti wrote:



On 14.12.2015 07:23, Jan Cholasta wrote:
On 11.12.2015 18:49, Tomas Babej wrote:


On 12/11/2015 05:37 PM, Martin Basti wrote:


On 11.12.2015 15:40, Jan Cholasta wrote:
On 11.12.2015 08:03, Jan Cholasta wrote:
On 11.12.2015 07:08, Jan Cholasta wrote:
On 10.12.2015 15:56, Martin Babinsky wrote:
On 12/10/2015 09:48 AM, Jan Cholasta wrote:
On 9.12.2015 16:38, Jan Cholasta wrote:
On 9.12.2015 14:52, Jan Cholasta wrote:
On 9.12.2015 10:02, Jan Cholasta wrote:
Hi,

the attached patches fix
<https://fedorahosted.org/freeipa/ticket/5497>.

Note that this needs selinux-policy fix to work, so put SELinux
into
permissive mode for testing:
<https://bugzilla.redhat.com/show_bug.cgi?id=1289930>.

Updated patches attached.

I screwed up a change in patch 524 and accidentally included a
chunk of
code in patch 525 that doesn't belong in it.

Updated patches attached.




Patches work as expected and I was not able to find any functional
problem.

I have a question about the naming of the oddjob helper script: the
one
related to trusts is named 'com.redhat.idm.trust-fetch-domains',
and the
conncheck runner is named 'org.freeipa.server.conncheck'. I
don't want
to start another bikeshedding conversation but shouldn't we
named them
in a consistent fashion (either rename the first one in separate
patch
or rename the new helper to com.redhat.idm.server.conncheck)?

I understand that as an upstream, we should go with the
'org.freeipa.*'
convention, but having two helpers with different prefixes makes me
sad.

If you look at the larger picture, org.freeipa is the consistent
name.
It makes me sad as well, but mistakes should be corrected. This is
similar to how we use PEP8 in new code, but do not fix it in old
code
just for the sake of fixing it.


That is a nitpick though, it does not affect the overall
functionality
of the patches so ACK.

Thanks for the review. The current patch 523 breaks the trusts
oddjob
with SELinux in enforcing mode, I will send an update which corrects
that, until bug 1289930 is fixed.

Updated patches attached.

Rebased on top of current master.



Just question, should be any kinited user allowed to run conncheck
via rpc?

Martin^2

I guess there's is little harm, any kinited user that was allowed to
access the machine could perform the conncheck even without these
patches:

In the RPC check, the user must have the Replication Administrators
privilege, which by default only admins have.

I tried to install replica with a regular user and conncheck passed.
Martin^2

See patch 525.

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

Reply via email to