Re: [Freeipa-devel] [PATCH 0031] provide a dedicated ccache file to httpd

2015-04-30 Thread Alexander Bokovoy

On Thu, 30 Apr 2015, Jan Cholasta wrote:

Hi,

Dne 29.4.2015 v 19:42 Martin Babinsky napsal(a):

The attached patch is a merge of PATCHES 0031-0032 incorporating Simo's
and Martin's suggestions (see e.g.
https://www.redhat.com/archives/freeipa-devel/2015-April/msg00327.html
for reference).

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


IMHO we should set the environment variable in 
/etc/systemd/system/httpd.service, instead of providing a new service 
file, because we are just changing configuration, not creating a new 
concurrent httpd instance, as is the case with ipa-memcached, and also 
not using alternative httpd implementation which masks the current 
one, as is the case with bind-pkcs11. It would simplify the whole 
thing significantly and it's even recommended in httpd.service to do 

I agree.


so:

   # For example, to pass additional options (for instance, -D 
definitions) to the

   # httpd binary at startup, you need to create a file named
   # /etc/systemd/system/httpd.service containing:
   #.include /lib/systemd/system/httpd.service
   #[Service]
   #Environment=OPTIONS=-DMY_DEFINE

(BTW I wonder why /etc/sysconfig/httpd support was removed from httpd 
in Fedora (http://pkgs.fedoraproject.org/cgit/httpd.git/commit/?id=0b19f7b6e1a47c6167a8ab43b4a9d1e759b54721), 
it seems like a better place to customize environment variables, 
rather than having to create a modified service file...)

We had discussion with Joe Orton (httpd maintainer) a while ago and his
arguments were following:

Hi guys, we made that change to adopt what is considered best practice
for systemd.  The change is not in RHEL7, only Fedora = 20.

I would not say we are strongly wedded to that change, but the use case
you provide seems very weak.  /etc/sysconfig/httpd is intended to be
user-configurable and if users do rm -f /etc/sysconfig/httpd then
Fedora packages should keep working correctly.  Can we find a more
robust way to achieve the same results?  Why is it required that the
environment variable is set globally within /usr/sbin/httpd?

... [and later in dicussion]

I'd argue that in this case you should not be using httpd.service as-is;
instead it would be correct to create an httpd-ipa.service unit file
or similar, which can .include the system httpd.service, and sets up
the appropriate Environment= (or EnvironmentFile=) directly.

Also, if the intent is to purely to change mod_auth_kerb's interaction
with libkrb5 is there no way to do this via the libkrb API - or
mod_auth_kerb's existing use thereof?

The use of /etc/sysconfig/httpd has historically been a mild PITA and
I'm not seeing a compelling reason to revert the decision to kill it
here.


Anyway, I would prefer if we set it in a way that works on non-systemd 
distros as well. Can't we just set GssapiCredStore 
ccache:FILE:/var/run/httpd/krbcache/krb5ccache in 
/etc/httpd/conf.d/ipa.conf?

It is not just mod_auth_gssapi, it is needed for users of the
credentials obtained by mod_auth_gssapi. mod_auth_gssapi only sets
KRB5CCNAME value when there is delegation of credentials in use and
there is something to delegate.


--
/ 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] 821 webui: add pwpolicy link to group details page if group has associated pwpolicy

2015-04-30 Thread Martin Babinsky

On 04/17/2015 05:58 PM, Petr Vobornik wrote:

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



ACK

--
Martin^3 Babinsky

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


Re: [Freeipa-devel] [PATCHES 0233-0234] DNSSEC: forwarders validation

2015-04-30 Thread David Kupka

On 04/24/2015 02:56 PM, Martin Basti wrote:

Patches attached.





Hi,
thanks for patches.

1. You changed message in DNSServerNotRespondingWarning class but not 
the test in ipatest/test_xmlrpc/test_dns_plugin.py


nitpick. Please spell 'edns' correctly. I've seen several instances of 
'ends'.


--
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] [WARNING] Trusts are broken in Fedora 22

2015-04-30 Thread Alexander Bokovoy

Hi,

If you are eager to try Fedora 22 beta and overall try FreeIPA in Fedora
22, be aware that trusts to Active Directory are currently broken due to
Samba 4.2.1 update in Fedora 22.

I've pushed build [1] of Samba today that at least allows Samba
processes to start properly but establishing trust will fail due to
changes in Samba client libraries.

I'm investigating the reason for the issues and hope to get them fixed
before Fedora 22 final freeze comes. 


[1] https://admin.fedoraproject.org/updates/samba-4.2.1-7.fc22

--
/ 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] 814-818 migrate-ds: optimize adding users to default group

2015-04-30 Thread Martin Basti

On 10/04/15 12:55, Petr Vobornik wrote:

The essential patch is 814.

815 a proposal for new option.

816 and 818 are cleanup patches.

817 little optimization.

== [PATCH] 814 migrate-ds: optimize adding users to default group ==
Migrate-ds searches for user without a group and adds them to default 
group. There is no point in checking if the user's selected by 
previous query are not member of default group because they are not 
member of any group.


The operation is also speeded up by not fetching the default group. 
Users are added right away.


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


NACK

Users haven't been added into ipa default group after migration.

Just nitpick

1) too many parentheses
api.log.error(('Adding new members to default group failed: 
%s \n'

   'members: %s') % (e, (','.join(member_dns
You can use this instead:
api.log.error('Adding new members to default group failed: 
%s \n'

  'members: %s', e, ','.join(member_dns))

== [PATCH] 815 migrate-ds: skip default group options ==

New option --use-default-group=False could be used to disable adding of
migrated users into default group.

By default, the default group is no longer POSIX therefore it doesn't
fulfill the original idea of providing GID and therefore it could be
skipped during migration.

LGTM


== [PATCH] 816 migrate-ds: remove unused def_group_gid context 
property ==

it's no longer used anywhere


1)
You can remove the unused variable 'g_attrs'
== [PATCH] migrate-ds: optimize gid checks by utilizing dictionary 
nature of set ==



LGTM

== [PATCH] migrate-ds: log migrated group members only on debug level ==
It pollutes error_log.


1)
you do not need % formatting in logger
api.log.debug('migrating %s group %s' , member_attr, m)





Martin^2

--
Martin Basti

-- 
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 0047] Unsaved changes dialog inconsistent

2015-04-30 Thread Gabe Alford
Thanks Kyle and Petr.

Update patch attached.

On Wed, Apr 29, 2015 at 7:59 AM, Kyle Baker kyba...@redhat.com wrote:


 - Original Message -
  On 04/27/2015 03:03 PM, Gabe Alford wrote:
   Hello,
  
   Fix for https://fedorahosted.org/freeipa/ticket/4926
  
   Thanks,
  
   Gabe
  
 
  PatternFly has new recommendations for terminology and wording [1]. I'm
  not entirely sure if the usage of 'save' here is good. PF defines 'edit'
  as the recommended term. The page doesn't say if 'save' is not
  recommended, though. Save seems to me as a confirmation of editing.

 Yes I think save would be best here based on the message given.

 Thanks for checking out the Terminology screen!

 
  Kyle, could you advise what is the best term for reflecting user changes
  and for confirmation of this action?
 
  Technical notes:
  1. it would be better to add a new string and then use it in the button
  instead of having 'Save' text for '@i18n:buttons.update' definition.
 
  2. String changes in internal.py should be also reflected in
  install/ui/test/data/ipa_init.json (for static web ui demo).
 
  3. optional: in addition to text change, buttons and related actions
  could also be renamed (same reasons as in 1). It's more proper but much
  more complicated.
 
 
  [1]
 https://www.patternfly.org/styles/terminology-and-wording/#action-labels
  --
  Petr Vobornik
 

From 45ea1a7804b76f73a3a83b1452f83b5895614986 Mon Sep 17 00:00:00 2001
From: Gabe redhatri...@gmail.com
Date: Thu, 30 Apr 2015 11:39:34 -0600
Subject: [PATCH] Unsaved changes dialog internally inconsistent

https://fedorahosted.org/freeipa/ticket/4926
---
 install/ui/src/freeipa/details.js  | 30 +++---
 install/ui/src/freeipa/dns.js  |  2 +-
 install/ui/src/freeipa/ipa.js  |  8 
 install/ui/test/data/ipa_init.json |  2 ++
 ipalib/plugins/internal.py |  2 ++
 5 files changed, 24 insertions(+), 20 deletions(-)

diff --git a/install/ui/src/freeipa/details.js b/install/ui/src/freeipa/details.js
index 7aa4c0ef6541900d6fa5b14b16ec964b50349015..e428dc90875a1ad567a13f379aa5ca079e47b672 100644
--- a/install/ui/src/freeipa/details.js
+++ b/install/ui/src/freeipa/details.js
@@ -453,8 +453,8 @@ exp.facet_policies = IPA.facet_policies = function(spec) {
  * - sets name, title, label if not present
  * - adds default actions and related buttons
  *   - refresh
- *   - reset
- *   - update
+ *   - revert
+ *   - save
  * - adds dirty state evaluator
  *
  * @member details
@@ -472,21 +472,21 @@ exp.details_facet_pre_op = function(spec, context) {
 spec.actions = spec.actions || [];
 spec.actions.unshift(
 'refresh',
-'reset',
-'update');
+'revert',
+'save');
 
 spec.control_buttons = spec.control_buttons || [];
 
 if (!spec.no_update) {
 spec.control_buttons.unshift(
 {
-name: 'reset',
-label: '@i18n:buttons.reset',
+name: 'revert',
+label: '@i18n:buttons.revert',
 icon: 'fa-undo'
 },
 {
-name: 'update',
-label: '@i18n:buttons.update',
+name: 'save',
+label: '@i18n:buttons.save',
 icon: 'fa-upload'
 });
 }
@@ -1404,8 +1404,8 @@ exp.refresh_action = IPA.refresh_action = function(spec) {
 exp.reset_action = IPA.reset_action = function(spec) {
 
 spec = spec || {};
-spec.name = spec.name || 'reset';
-spec.label = spec.label || '@i18n:buttons.reset';
+spec.name = spec.name || 'revert';
+spec.label = spec.label || '@i18n:buttons.revert';
 spec.enable_cond = spec.enable_cond || ['dirty'];
 
 var that = IPA.action(spec);
@@ -1426,8 +1426,8 @@ exp.reset_action = IPA.reset_action = function(spec) {
 exp.update_action = IPA.update_action = function(spec) {
 
 spec = spec || {};
-spec.name = spec.name || 'update';
-spec.label = spec.label || '@i18n:buttons.update';
+spec.name = spec.name || 'save';
+spec.label = spec.label || '@i18n:buttons.save';
 spec.needs_confirm = spec.needs_confirm !== undefined ? spec.needs_confirm : false;
 spec.enable_cond = spec.enable_cond || ['dirty'];
 
@@ -2007,8 +2007,8 @@ exp.register = function() {
 var f = reg.facet;
 
 a.register('refresh', exp.refresh_action);
-a.register('reset', exp.reset_action);
-a.register('update', exp.update_action);
+a.register('revert', exp.reset_action);
+a.register('save', exp.update_action);
 a.register('object', exp.object_action);
 a.register('enable', exp.enable_action);
 a.register('disable', exp.disable_action);
@@ -2026,4 +2026,4 @@ exp.register = function() {
 phases.on('registration', exp.register);
 
 return exp;
-});
\ No newline at end of file
+});
diff --git a/install/ui/src/freeipa/dns.js b/install/ui/src/freeipa/dns.js
index 

Re: [Freeipa-devel] [PATCH 0083] Fix a signedness bug in OTP code

2015-04-30 Thread Martin Babinsky

On 04/27/2015 04:30 PM, Nathaniel McCallum wrote:

This bug caused negative token windows to wrap-around, causing issues
with TOTP authentication and (especially) synchronization.

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




ACK

--
Martin^3 Babinsky

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