Re: [Freeipa-devel] [PATCH 0068] Fix ipa.service restart
On 06/17/2014 04:22 PM, Martin Basti wrote: Patch attached. Ticket: https://fedorahosted.org/freeipa/ticket/4243 Works fine. ACK, pushed to master, ipa-3-3. Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0267-0268] Preparation for 4.4 release
On 24.6.2014 22:46, Lukas Slebodnik wrote: On (24/06/14 21:34), Martin Kosek wrote: On 06/24/2014 05:37 PM, Petr Spacek wrote: On 23.6.2014 17:12, Petr Spacek wrote: Bump NVR to 4.4. Update NEWS for upcoming 4.4 release. Pushed to master: 2cd574e90a9fdebdbeaab45ef335e7d63e85dfd7 3a705963ed575f01b792a7e89d825cf56ce99734 At this point v4 branches from master. Just for my education - what is the branching model in bind-dyndb-ldap? In FreeIPA, we branch when we release FreeIPA x.y so that bugfixing releases FreeIPA x.y.z can be done. When you now branched at 4.4, do you plan to release bugfixes 4.4.x or rather 4.5, 4.6...? Numbers are cheap. bind-dyndb-ldap is small project. It will be simpler to use only two numbers for version. To sum it up: https://fedorahosted.org/bind-dyndb-ldap/wiki/Versions -- Petr^2 Spacek ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES 202-222] Ipaplatform refactoring
On 06/19/2014 03:52 PM, Tomas Babej wrote: On 06/19/2014 12:52 PM, Tomas Babej wrote: On 06/18/2014 10:52 AM, Petr Viktorin wrote: On 06/17/2014 02:15 PM, Tomas Babej wrote: On 06/17/2014 12:03 PM, Timo Aaltonen wrote: On 17.06.2014 11:16, Martin Kosek wrote: Attached is a new version of patch 226, and a new patch 228, which moves the paths from installers to the paths module. In patch 226, there's another certificated typo in remove_ca_cert_from_systemwide_ca_store I greped the repository, and I do not see many paths lurking around any more, there are only some in the error messages (as these can't be reliably replaced automatically, and will need some manual love). If you see any forgotten paths, which should be added to the module, let me know. Well, since you asked... install/tools/ipa-upgradeconfig:236: ipautil.run([paths.PKI_SETUP_PROXY, '-pki_instance_root=/var/lib' ipaserver/install/cainstance.py:1330: -pki_instance_root=/var/lib, ipaserver/install/dsinstance.py:209:InstallLdifFile= /var/lib/dirsrv/boot.ldif ipaserver/install/dsinstance.py:210:inst_dir= /var/lib/dirsrv/scripts-$SERVERID ipaserver/install/ipa_backup.py:464: '--exclude=/var/lib/ipa/backup', ipatests/test_integration/tasks.py:451:host.run_command(find /var/lib/sss/db -name '*.ldb' | install/tools/ipa-replica-conncheck:403: /usr/sbin/ipa-replica-conncheck + install/tools/ipa-replica-conncheck:414: print_info(/usr/sbin/ipa-replica-conncheck + .join(remote_check_opts)) ipapython/ipautil.py:296:env[PATH] = /bin:/sbin:/usr/kerberos/bin:/usr/kerberos/sbin:/usr/bin:/usr/sbin ipaserver/install/cainstance.py:88:ConfigFile = /usr/share/pki/ca/conf/database.ldif ipaserver/install/bindinstance.py:829: ipautil.run(['/usr/libexec/generate-rndc-key.sh']) I guess it'll be a while before we catch them all, but now it's at least clear where these paths should be, so anyone porting to another distro can send patches (or tickets) upstream. I see another duplicate: SSS_KRB5_INCLUDE_D = /var/lib/sss/pubconf/krb5.include.d SSSD_PUBCONF_KRB5_INCLUDE_D_DIR = /var/lib/sss/pubconf/krb5.include.d/ Could you just pick one instead? Would ipa_backup.py break if it had a trailing slash here? In ipa-client-install, if you set: NSSWITCH_CONF = paths.NSSWITCH_CONF then you should only use one of those later. (Preferably paths.*, to get rid of the redundant constants.) Perhaps this is for another patch that would clean up all the cases where these trivial module variables are used. Fixed all mentioned issues. I also attached a patch 230, which removes the base Authconfig class. Attaching one additional patch, which removes unnecessary build warnings. 226, 230, 231 look good -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] User Life Cycle: scoping of referential integrity, memberof, IPA UUID plugins
On 06/24/2014 06:31 PM, thierry bordaz wrote: Hello, User life cycle assigns a status to user entries depending where they are in the DIT. 'Active' user will be under 'cn=accounts,SUFFIX' while 'Stage' and 'Delete' users are somewhere under 'cn=provisioning,SUFFIX'. Only 'Active' users have valid membership attributes: A Stage/Delete user does not belong to any 'Active' group. membership is managed by DS plugins, and particularly RI and memberof. To automatically update membership attributes RI and memberof implement a scoping, that update/add/remove membership attributes if the group/user are Active. The scoping is a single valued attribute. It create failures in IPA tests if I restrict RI/memberof to 'cn=accounts,SUFFIX'. For example adding a host (under 'cn=accounts,SUFFIX) adds it to a network group that is under 'cn=alt,SUFFIX'. A solution would be that the attribute that scopes the plugin is multivalued. But then it would require a long list of values: cn=pbac,SUFFIX cn=hbac,SUFFX cn=alt,SUFFIX cn=accounts, SUFFIX ... An other solution would be to exclude some parts of the DIT, here limited to 'cn=provisionning,SUFFIX'. (prefered solution). This is a similar issue with IPA UUID plugin that generates ipaUniqueID for entries under 'cn=accounts' but also 'cn=alt' or 'cn=hbac'. regards thierry Right. As discussed yesterday, I think the best approach would be to specify a SUFFIX + excluded tree. Specifying all containers where there may be an entry with member or RI'ed attribute would be very long and possibly error prone when we add a new one (all active IPA server plugin configuration would need to be updated?). Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] User Life Cycle: scoping of referential integrity, memberof, IPA UUID plugins
On 06/25/2014 10:52 AM, Martin Kosek wrote: On 06/24/2014 06:31 PM, thierry bordaz wrote: Hello, User life cycle assigns a status to user entries depending where they are in the DIT. 'Active' user will be under 'cn=accounts,SUFFIX' while 'Stage' and 'Delete' users are somewhere under 'cn=provisioning,SUFFIX'. Only 'Active' users have valid membership attributes: A Stage/Delete user does not belong to any 'Active' group. membership is managed by DS plugins, and particularly RI and memberof. To automatically update membership attributes RI and memberof implement a scoping, that update/add/remove membership attributes if the group/user are Active. The scoping is a single valued attribute. It create failures in IPA tests if I restrict RI/memberof to 'cn=accounts,SUFFIX'. For example adding a host (under 'cn=accounts,SUFFIX) adds it to a network group that is under 'cn=alt,SUFFIX'. A solution would be that the attribute that scopes the plugin is multivalued. But then it would require a long list of values: cn=pbac,SUFFIX cn=hbac,SUFFX cn=alt,SUFFIX cn=accounts, SUFFIX ... An other solution would be to exclude some parts of the DIT, here limited to 'cn=provisionning,SUFFIX'. (prefered solution). This is a similar issue with IPA UUID plugin that generates ipaUniqueID for entries under 'cn=accounts' but also 'cn=alt' or 'cn=hbac'. regards thierry Right. As discussed yesterday, I think the best approach would be to specify a SUFFIX + excluded tree. Specifying all containers where there may be an entry with member or RI'ed attribute would be very long and possibly error prone when we add a new one (all active IPA server plugin configuration would need to be updated?). Martin Thanks. I opened https://fedorahosted.org/389/ticket/47828 (DNA) and https://fedorahosted.org/389/ticket/47829 (memberof). For RI https://fedorahosted.org/389/ticket/47621 already implements it. For IPA Unique IDs I may use https://fedorahosted.org/freeipa/ticket/3813 or open a separated ticket. thierry ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0233] trusts: Add more read attributes
On 06/24/2014 08:15 PM, Tomas Babej wrote:
Attaching patch 234, which resolves another ACI issue related to trusts.
On 06/24/2014 02:50 PM, Tomas Babej wrote:
Hi,
this is a follow up patch for 232. Read access to additional attributes
is required for the trust objects.
First patch looks fine.
For the second: should the trust ACIs apply to other objects than
(objectclas=ipanttrusteddomain)?
If not, we can enable --type=trust permissions and use it to specify
location filter, see attached patch.
--
Petr³
From 075e6b669b7504cb0fbad15a6e2fec4f73c35851 Mon Sep 17 00:00:00 2001
From: Tomas Babej tba...@redhat.com
Date: Tue, 24 Jun 2014 18:24:32 +0200
Subject: [PATCH] trusts: Allow reading system trust accounts by adtrust agents
---
ACI.txt | 4 +++-
install/updates/60-trusts.update | 8
ipalib/plugins/trust.py | 12 ++--
3 files changed, 21 insertions(+), 3 deletions(-)
diff --git a/ACI.txt b/ACI.txt
index 687b1ef3017dd9b2388c1d413ed9f6ac211df406..9f1edce0ef293a2157015db79f9b1e4a36a83d64 100644
--- a/ACI.txt
+++ b/ACI.txt
@@ -213,7 +213,9 @@ aci: (targetattr = cmdcategory || cn || description || externalhost || external
dn: cn=System: Read Sudoers compat tree,cn=permissions,cn=pbac,dc=ipa,dc=example
aci: (targetattr = cn || description || objectclass || ou || sudocommand || sudohost || sudonotafter || sudonotbefore || sudooption || sudoorder || sudorunas || sudorunasgroup || sudorunasuser || sudouser)(target = ldap:///ou=sudoers,dc=ipa,dc=example;)(version 3.0;acl permission:System: Read Sudoers compat tree;allow (compare,read,search) userdn = ldap:///all;;)
dn: cn=System: Read Trust Information,cn=permissions,cn=pbac,dc=ipa,dc=example
-aci: (targetattr = cn || ipantflatname || ipantsecurityidentifier || ipantsidblacklistincoming || ipantsidblacklistoutgoing || ipanttrusteddomainsid || ipanttrustpartner || objectclass)(version 3.0;acl permission:System: Read Trust Information;allow (compare,read,search) userdn = ldap:///all;;)
+aci: (targetattr = cn || ipantflatname || ipantsecurityidentifier || ipantsidblacklistincoming || ipantsidblacklistoutgoing || ipanttrusteddomainsid || ipanttrustpartner || objectclass)(targetfilter = (objectclass=ipanttrusteddomain))(version 3.0;acl permission:System: Read Trust Information;allow (compare,read,search) userdn = ldap:///all;;)
+dn: cn=System: Read system trust accounts,cn=permissions,cn=pbac,dc=ipa,dc=example
+aci: (targetattr = gidnumber || uidnumber)(targetfilter = (objectclass=ipanttrusteddomain))(version 3.0;acl permission:System: Read system trust accounts;allow (compare,read,search) groupdn = ldap:///cn=System: Read system trust accounts,cn=permissions,cn=pbac,dc=ipa,dc=example;)
dn: cn=System: Add User to default group,cn=permissions,cn=pbac,dc=ipa,dc=example
aci: (targetattr = member)(target = ldap:///cn=ipausers,cn=groups,cn=accounts,dc=ipa,dc=example;)(version 3.0;acl permission:System: Add User to default group;allow (write) groupdn = ldap:///cn=System: Add User to default group,cn=permissions,cn=pbac,dc=ipa,dc=example;)
dn: cn=System: Add Users,cn=permissions,cn=pbac,dc=ipa,dc=example
diff --git a/install/updates/60-trusts.update b/install/updates/60-trusts.update
index 371bf656fcdea6b7ec54aeb42c5afd25ef1b90f9..d55bc94bbe917571999bcc7dfb6e6aaf641c4b49 100644
--- a/install/updates/60-trusts.update
+++ b/install/updates/60-trusts.update
@@ -15,6 +15,14 @@ dn: cn=adtrust agents,cn=sysaccounts,cn=etc,$SUFFIX
default: objectClass: top
default: cn: adtrust agents
+dn: cn=ADTrust Agents,cn=privileges,cn=pbac,$SUFFIX
+default: objectClass: top
+default: objectClass: groupofnames
+default: objectClass: nestedgroup
+default: cn: ADTrust Agents
+default: description: System accounts able to access trust information
+default: member: cn=adtrust agents,cn=sysaccounts,cn=etc,$SUFFIX
+
dn: cn=trusts,$SUFFIX
default: objectClass: top
default: objectClass: nsContainer
diff --git a/ipalib/plugins/trust.py b/ipalib/plugins/trust.py
index 25755d7a41e93b869f4d4afbf0ac094c42212451..f850a358b26ca2284d7e238770f6e67fde1dd9d2 100644
--- a/ipalib/plugins/trust.py
+++ b/ipalib/plugins/trust.py
@@ -309,6 +309,7 @@ class trust(LDAPObject):
object_name = _('trust')
object_name_plural = _('trusts')
object_class = ['ipaNTTrustedDomain']
+permission_filter_objectclasses = ['ipanttrusteddomain']
default_attributes = ['cn', 'ipantflatname', 'ipanttrusteddomainsid',
'ipanttrusttype', 'ipanttrustattributes', 'ipanttrustdirection',
'ipanttrustpartner', 'ipanttrustforesttrustinfo',
@@ -318,8 +319,6 @@ class trust(LDAPObject):
managed_permissions = {
'System: Read Trust Information': {
# Allow reading of attributes needed for SSSD subdomains support
-'non_object': True,
-'ipapermlocation': DN(container_dn, api.env.basedn),
'replaces_global_anonymous_aci': True,
'ipapermbindruletype': 'all',
Re: [Freeipa-devel] [PATCH 0077] Add dnssecinlinesigning attribute to ACI
On 06/20/2014 03:32 PM, Martin Basti wrote: Required patches: mbasti-0060, mbasti-0073 Patch attached. Hi, For the raw ACI in dns.ldif, there are some more hoops to jump through. Remove the ACI from /install/share/dns.ldif entirely (except for schema, we're slowly replacing the .ldif content by .update files). In install/updates/40-dns.update, you'll notice the Update DNS entries in a zone ACI is already being added. You'll need to replace it, using a line like: replace:aci:'old ACI::new ACI' This will remove the old value that IPA 3.x users still have. I see you already changed the ACI in 7cdc417, in dns.ldif only. Be sureto use the original value for old ACI. -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0056] Add otptoken-sync command
On Tue, 24 Jun 2014, Nathaniel McCallum wrote: On Tue, 2014-06-03 at 09:18 -0400, Nathaniel McCallum wrote: On Tue, 2014-06-03 at 10:27 +0200, Petr Vobornik wrote: On 3.6.2014 05:08, Nathaniel McCallum wrote: This command calls the token sync HTTP POST call in the server providing the CLI interface to synchronization. https://fedorahosted.org/freeipa/ticket/4260 This patch depends on my patch #0055. Build fails on validation. You forgot to update API.txt and also the command misses __doc__. (not a proper review) Thanks, fixed. Attached is a new revision which is rebased on master. In addition it: 1. Moves user to a parameter and moves token to an argument. Doing it this way both mirrors the existing otptoken APIs and sets us up for future Kerberos based syncing where the username/password will be optional. 2. Converts the token ID to a DN. ACK. Please do not commit this patch yet, we are not done with its dependencies. -- / Alexander Bokovoy ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0055] Add /session/token_sync POST support
On Tue, 24 Jun 2014, Nathaniel McCallum wrote:
On Mon, 2014-06-02 at 23:07 -0400, Nathaniel McCallum wrote:
This HTTP call takes the following parameters:
* user
* password
* first_code
* second_code
* token (optional)
Using this information, the server will perform token synchronization.
If the token is not specified, all tokens will be searched for
synchronization.
Otherwise, only the token specified will be searched.
This patch depends on my patch #0054.
Attached is a new revision. This version should force an update
to /etc/httpd/conf.d/ipa.conf on update. It is also rebased on master.
ACK with condition that you apply attached fixups.
Since token that is passed by 'ipa otptoken-sync' command is not a full
DN, we need to support both cases, when DN and just a name is passed.
Attached patch fixes this.
--
/ Alexander Bokovoy
From ac31c6c6ce0b7bc163696968e07e8dd3c75accef Mon Sep 17 00:00:00 2001
From: Alexander Bokovoy aboko...@redhat.com
Date: Wed, 25 Jun 2014 13:17:08 +0300
Subject: [PATCH 9/9] fixup! Add /session/token_sync POST support
---
ipaserver/rpcserver.py | 8 ++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/ipaserver/rpcserver.py b/ipaserver/rpcserver.py
index 39134af..33d03e2 100644
--- a/ipaserver/rpcserver.py
+++ b/ipaserver/rpcserver.py
@@ -1165,8 +1165,12 @@ class sync_token(Backend, HTTP_Status):
sr.setComponentByName('firstCode', data['first_code'])
sr.setComponentByName('secondCode', data['second_code'])
if 'token' in data:
-token_dn = DN((self.api.Object.otptoken.primary_key.name,
data['token']),
- self.api.env.container_otp, self.api.env.basedn)
+try:
+token_dn = DN(data['token'])
+except ValueError:
+token_dn = DN((self.api.Object.otptoken.primary_key.name,
data['token']),
+ self.api.env.container_otp, self.api.env.basedn)
+
sr.setComponentByName('tokenDN', str(token_dn))
rc = ldap.controls.RequestControl(sr.OID, True, encoder.encode(sr))
--
1.9.3
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0058] Add the otptoken-add-yubikey command
On Mon, 23 Jun 2014, Nathaniel McCallum wrote:
On Mon, 2014-06-23 at 10:29 +0300, Alexander Bokovoy wrote:
On Fri, 20 Jun 2014, Nathaniel McCallum wrote:
On Thu, 2014-06-19 at 16:30 -0400, Nathaniel McCallum wrote:
This command behaves almost exactly like otptoken-add except:
1. The new token data is written directly to a YubiKey
2. The vendor/model/serial fields are populated from the YubiKey
=== NOTE ===
1. This patch depends on the new Fedora package: python-yubico. If you
would like to help with the package review, please assign yourself here:
https://bugzilla.redhat.com/show_bug.cgi?id=334
New version of the patch. This one works (yay!).
1. Because of the dependency on python-yubico, is this feature something
we want in core FreeIPA? As a subpackage? Separate project altogether?
The only dependency for python-yubico is pyusb.
I'd prefer to have it integrated but have a separate dummy subpackage
that pulls in all required dependencies, like, freeipa-tools-yubico. Instead
of failing when 'ipa otptoken-add-yubikey' is called, please wrap the
python-yubico import into a code that allows reporting a message back to
the user advising to install the package.
Who is a supposed user for this command? IPA command line interface isn't
usually available on enrolled machines even though underlying Python
modules are all there. Are we talking about admins or just users?
As discussed on IRC, we are currently hard-coding lots of optional
dependencies. And breaking this apart into subpackages can be solved at
a later point. YubiKey is also a unique case: we don't expect to be
adding many more plugins like this.
For these reasons, I have kept this as a hard dependency. To ease this
transition, I have added python-yubico to F20 and EL6. You can help with
the update review here:
https://admin.fedoraproject.org/updates/python-yubico-1.2.1-3.fc20
https://admin.fedoraproject.org/updates/python-yubico-1.2.1-3.el6
3. This code currently emits a warning from the call to otptoken-add:
WARNING: API Version number was not sent, forward compatibility not
guaranteed. Assuming server's API version, 2.89
How do I fix this?
Do not filter 'version' field in options in execute().
I opted to not filter out version rather than hard-code it (pviktori's
suggestion). This is based on the fact that otptoken-add-yubikey is
tightly integrated with otptoken-add (even using some of the class
attributes from otptoken).
4. I am not sure why I have to delete the summary and value keys from
the return dictionary. It would be nice to display this summary message
just like otptoken-add.
I still need help on this.
5. Am I doing the ipatoken(vendor|model|serial) options correctly? These
aren't user settable, but we need to pass them from the yubikey
(client-side) to the server.
This is no longer needed since I am doing everything in forward().
However, listing these three as output params causes them to appear
before the token's ID. I don't think this is the right way to output
these. But this seems to me a framework issue.
6. I'm not sure my use of assert or ValueError are correct. What should
I do here?
Still need help here.
Fixed this part.
7. Considering that this is just a specialized invocation of
otptoken-add, can't we do this all on the client-side? This is why I had
originally used frontend.Local rather than frontend.Command.
You don't need to implement execute then, only forward, where you'll
forward your call to the server under otptoken_add name.
Typically in #forward we call super's forward but that is because we
in Command.forward() we simply forward the command to the remote backend,
using the self.name. In your case we shouldn't really have a separate
command on the server under the same name, so you'll need to avoid
calling
So, it should look like this:
def forward(self, *args, **kw):
perform yubikey initialization
filter out kw and args, if needed
return self.Backend.rpcclient.forward('otptoken_add', *args, **kw)
See service_show implementation for an example.
Fixed.
I'm attaching few fixups to the patch that make it proper reporting for
non-Yubikey case and also properly update VERSION.
Provisional ACK.
--
/ Alexander Bokovoy
From 1c83f1943a635a4b2541addd9fc21b6155fb12d6 Mon Sep 17 00:00:00 2001
From: Alexander Bokovoy aboko...@redhat.com
Date: Wed, 25 Jun 2014 13:32:16 +0300
Subject: [PATCH 10/10] fixup! Add the otptoken-add-yubikey command
---
ipalib/plugins/otptoken_yubikey.py | 9 +++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/ipalib/plugins/otptoken_yubikey.py
b/ipalib/plugins/otptoken_yubikey.py
index 30a81e5..b4f2188 100644
--- a/ipalib/plugins/otptoken_yubikey.py
+++ b/ipalib/plugins/otptoken_yubikey.py
@@ -18,6 +18,7 @@
# along with this program. If not, see http://www.gnu.org/licenses/.
from ipalib import _, Str, IntEnum
+from ipalib.errors import NotFound
from ipalib.plugable import Registry
from
Re: [Freeipa-devel] [PATCH 0053] Implement OTP token importing
On Wed, 18 Jun 2014, Nathaniel McCallum wrote:
On Wed, 2014-06-18 at 17:48 -0400, Simo Sorce wrote:
On Wed, 2014-06-18 at 17:34 -0400, Nathaniel McCallum wrote:
On Tue, 2014-05-13 at 12:38 -0400, Nathaniel McCallum wrote:
This patch adds support for importing tokens using RFC 6030 key
container files. This includes decryption support. For sysadmin sanity,
any tokens which fail to add will be written to the output file for
examination. The main use case here is where a small subset of a large
set of tokens fails to validate or add. Using the output file, the
sysadmin can attempt to recover these specific tokens.
This code is implemented as a server-side script. However, it doesn't
actually need to run on the server. This was done because importing is
an odd fit for the IPA command framework:
1. We need to write an output file.
2. The operation may be long-running (thousands of tokens).
3. Only admins need to perform this task and it only happens
infrequently.
Attached is revision 4. I believe this addresses all the points given
over the last few days in all emails. The ipa_otptoken_import.py has
been significantly reworked to make it simpler and easy to test, but
none of the logic has changed.
I have removed most of the inheritance and sorted out most of the style
issues (like map() vs comprehension). I did not change the XML parsing
because it appears that network access is disabled by default.
I have also included a test suite which should have 100% code coverage.
It even tests for features we don't support yet (like X.509). All tests
pass for me.
Nathaniel
+++ b/install/tools/man/ipa-otptoken-import.1
@@ -0,0 +1,36 @@
+.\ A man page for ipa-compat-manage
Bad Copypaste here ^^^
Thanks! Fixed.
There is whitespace warning in the man page, needs to be fixed.
Also, spec file changes are incomplete, man page is not there.
The patch itself works fine for me with the test suite.
Attached is the specfile fix, with that one and whitespace removal --
ACK.
Attached also is a small fix for ipaplatform changes as specfile now has
wrong scoping for the platform files.
--
/ Alexander Bokovoy
From c4c303fc471e8ad2561bd8e3180985485981472d Mon Sep 17 00:00:00 2001
From: Alexander Bokovoy aboko...@redhat.com
Date: Wed, 25 Jun 2014 09:46:39 +0300
Subject: [PATCH 08/10] fixup! Implement OTP token importing
---
freeipa.spec.in | 1 +
1 file changed, 1 insertion(+)
diff --git a/freeipa.spec.in b/freeipa.spec.in
index 63f7477..2ba9786 100644
--- a/freeipa.spec.in
+++ b/freeipa.spec.in
@@ -775,6 +775,7 @@ fi
%{_mandir}/man1/ipa-backup.1.gz
%{_mandir}/man1/ipa-restore.1.gz
%{_mandir}/man1/ipa-advise.1.gz
+%{_mandir}/man1/ipa-otptoken-import.1.gz
%files server-trust-ad
%{_sbindir}/ipa-adtrust-install
--
1.9.3
From 6e5aabecc290e170882f53de52987d675a9b78b6 Mon Sep 17 00:00:00 2001
From: Alexander Bokovoy aboko...@redhat.com
Date: Tue, 24 Jun 2014 15:51:45 +0300
Subject: [PATCH 06/10] Fix packaging issue with doubly specified directories
---
freeipa.spec.in | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/freeipa.spec.in b/freeipa.spec.in
index ed125e5..63f7477 100644
--- a/freeipa.spec.in
+++ b/freeipa.spec.in
@@ -837,7 +837,7 @@ fi
%dir %{python_sitelib}/ipaplatform
%dir %{python_sitelib}/ipaplatform/base
%dir %{python_sitelib}/ipaplatform/fedora
-%{python_sitelib}/ipaplatform/*
+%{python_sitelib}/ipaplatform/*.py*
%{python_sitelib}/ipaplatform/base/*.py*
%{python_sitelib}/ipaplatform/fedora/*.py*
%attr(0644,root,root) %{python_sitearch}/default_encoding_utf8.so
--
1.9.3
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0053] Implement OTP token importing
On 06/25/2014 12:40 PM, Alexander Bokovoy wrote: On Wed, 18 Jun 2014, Nathaniel McCallum wrote: On Wed, 2014-06-18 at 17:48 -0400, Simo Sorce wrote: On Wed, 2014-06-18 at 17:34 -0400, Nathaniel McCallum wrote: On Tue, 2014-05-13 at 12:38 -0400, Nathaniel McCallum wrote: This patch adds support for importing tokens using RFC 6030 key container files. This includes decryption support. For sysadmin sanity, any tokens which fail to add will be written to the output file for examination. The main use case here is where a small subset of a large set of tokens fails to validate or add. Using the output file, the sysadmin can attempt to recover these specific tokens. This code is implemented as a server-side script. However, it doesn't actually need to run on the server. This was done because importing is an odd fit for the IPA command framework: 1. We need to write an output file. 2. The operation may be long-running (thousands of tokens). 3. Only admins need to perform this task and it only happens infrequently. Attached is revision 4. I believe this addresses all the points given over the last few days in all emails. The ipa_otptoken_import.py has been significantly reworked to make it simpler and easy to test, but none of the logic has changed. I have removed most of the inheritance and sorted out most of the style issues (like map() vs comprehension). I did not change the XML parsing because it appears that network access is disabled by default. I have also included a test suite which should have 100% code coverage. It even tests for features we don't support yet (like X.509). All tests pass for me. Nathaniel +++ b/install/tools/man/ipa-otptoken-import.1 @@ -0,0 +1,36 @@ +.\ A man page for ipa-compat-manage Bad Copypaste here ^^^ Thanks! Fixed. There is whitespace warning in the man page, needs to be fixed. Also, spec file changes are incomplete, man page is not there. The patch itself works fine for me with the test suite. Attached is the specfile fix, with that one and whitespace removal -- ACK. Attached also is a small fix for ipaplatform changes as specfile now has wrong scoping for the platform files. I pushed all 3 patches to master. (I did not realize you want to just fixup your patch 0008 before I pushed the first, so I chosen a better name for it.) Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0054] Change OTPSyncRequest structure to use OctetString
On Tue, 27 May 2014, Nathaniel McCallum wrote: This change has two motivations: 1. Clients don't have to parse the string. 2. Future token types may have new formats. ACK, works for me. -- / Alexander Bokovoy ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES] 267-294 Support multiple CA certificates in LDAP
On 16.6.2014 22:36, Rob Crittenden wrote: Rob Crittenden wrote: Jan Cholasta wrote: Hi, the attached patches implement https://fedorahosted.org/freeipa/ticket/3259 and https://fedorahosted.org/freeipa/ticket/3520. This work depends on my patches 241-253 and 262-266 (http://www.redhat.com/archives/freeipa-devel/2014-June/msg00276.html). Note that automatic distribution of CA certificates to IPA systems is not implemented yet (it's planned for IPA 4.2, see https://fedorahosted.org/freeipa/ticket/4322), so /etc/ipa/ca.crt, /etc/pki/nssdb, /etc/dirsrv/slapd-REALM and /etc/httpd/alias are updated *only* during client/server install. 267 What is the purpose of this change? Won't this cause no certificates to be exported in the default case? diff --git a/ipaserver/install/certs.py b/ipaserver/install/certs.py index 61a954f..3cd7a53 100644 --- a/ipaserver/install/certs.py +++ b/ipaserver/install/certs.py @@ -463,7 +463,7 @@ class CertDB(object): do that step. # export the CA cert for use with other apps ipautil.backup_file(self.cacert_fname) -root_nicknames = self.find_root_cert(nickname) +root_nicknames = self.find_root_cert(nickname)[:-1] fd = open(self.cacert_fname, w) for root in root_nicknames: (cert, stderr, returncode) = self.run_certutil([-L, -n, root, -a]) Or are you separating out issuing CA from the rest of the chain? find_root_cert() returns the certificate chain *including* the end-entity certificate. We want only CA certificates here. This is an error introduced in the CA-less rewrite. 268 ACK 269 ACK 270 If a user has their own CA installed, such as the case where they used ipa-server-certinstall, this will break it. You can't install own CA with ipa-server-certinstall. I did not see anything break. What can be in the other set? Docs are needed. The trusted set is for trusted CA certs and the other set is for other CA certs. I will add a comment... You potentially add a bunch of certs with no trust, what is the purpose? No trust in this context means trust only for issuing CA certificates. The certs are there for the sole purpose of forming the CA chain, so they don't need to be trusted for anything else. 271 ipaSecretKey isn't mentioned on http://www.freeipa.org/page/V4/CA_certificate_renewal . What is it and does it need to be added now? This is the schema from http://www.freeipa.org/page/V4/PKCS11_in_LDAP/Schema#Encoded_key_data. It is shared with DNSSEC. I will add a comment. 272 ACK 273 ACK 274 ACK 275 ACK 276 There isn't any error handling around the ASN.1 decoder. Is that wise? Probably not, but it's consistent with the rest of the x509 module - none of its functions do error handling, it is up to the caller. BTW I have started refactoring x509 code, but didn't have time to finish. The new code will include error handling. There should be unit tests Yes. 277 There should be unit tests Yes. 278 When the certificate must be passed in as DER can you use dercert as the argument name so it's clear what is needed? OK. In update_ca_cert() and get_ca_certs() should the values for trust be case insensitive? It already is in update_ca_cert(), but get_ca_certs() does indeed need to be fixed. This breaks the convention where attribute names are always lower-case. I wasn't aware there is such a convention, especially so after seeing loads of mixed case attribute names all over IPA code. Anyway, I don't think it matters anymore, the new LDAP code has case-insensitive attribute names. I can't really grok add_ca_cert(). You are adding certs but removing values? Is this un-setting the list of trusted CA's maybe? It is unsetting ipaConfigString values from entries that should no longer have them. I will add a comment. There isn't a single comment in this file beyond the header. Sorry, will fix. 279 Looks ok 280 Why add the chain with no trust? See my comment for patch 270. 281 / 282 What is the difference between add_cert and import_cert other than passing in trust and not having the db password? Do we even need add_single_pem_cert anymore? add_cert adds certificate by value, import_cert loads it from a file. We don't need add_single_pem_cert anymore. 283 / 284 In __import_ca_certs() I assume the pass is there for NotFound for the case of creating a replica with an older master that doesn't have these? How is SSL trust setup then? It is for the case where there is no cn=certificates nor cn=CAcert. The trust is setup as usual, failed import does not affect it. This code looks awfully similar. Will fix. 285 ACK 286 It looks ok, just one wild thought. If writing the certificate fails and you go to clean up by removing ca_file, what if that removal fails, for the same reason the write failed, SELinux for example? Good point, will fix. 287 If this weren't addressed in a later patch
Re: [Freeipa-devel] [PATCH 0233] trusts: Add more read attributes
On 06/25/2014 11:45 AM, Petr Viktorin wrote:
On 06/24/2014 08:15 PM, Tomas Babej wrote:
Attaching patch 234, which resolves another ACI issue related to trusts.
On 06/24/2014 02:50 PM, Tomas Babej wrote:
Hi,
this is a follow up patch for 232. Read access to additional attributes
is required for the trust objects.
First patch looks fine.
For the second: should the trust ACIs apply to other objects than
(objectclas=ipanttrusteddomain)?
If not, we can enable --type=trust permissions and use it to specify
location filter, see attached patch.
Turns out there are also kerberos principals stored under cn=trust tree
and this filter would block the access to them.
Attached is a new version of 234, which allows reading krbPrincipalName
as well.
--
Tomas Babej
Associate Software Engineer | Red Hat | Identity Management
RHCE | Brno Site | IRC: tbabej | freeipa.org
From 536b386a9196957edccc8731c1c2d9ed204c69e3 Mon Sep 17 00:00:00 2001
From: Tomas Babej tba...@redhat.com
Date: Tue, 24 Jun 2014 18:24:32 +0200
Subject: [PATCH] trusts: Allow reading system trust accounts by adtrust agents
---
ACI.txt | 2 ++
install/updates/60-trusts.update | 8
ipalib/plugins/trust.py | 11 +++
3 files changed, 21 insertions(+)
diff --git a/ACI.txt b/ACI.txt
index 0398a52fcf2639f895f6bb7cd8bd91412affa6d3..43cf1e3b931c03762c0628b4cef8a515ef5e44f7 100644
--- a/ACI.txt
+++ b/ACI.txt
@@ -112,6 +112,8 @@ dn: cn=System: Read Sudoers compat tree,cn=permissions,cn=pbac,dc=ipa,dc=example
aci: (targetattr = cn || description || objectclass || ou || sudocommand || sudohost || sudonotafter || sudonotbefore || sudooption || sudoorder || sudorunas || sudorunasgroup || sudorunasuser || sudouser)(target = ldap:///ou=sudoers,dc=ipa,dc=example;)(version 3.0;acl permission:System: Read Sudoers compat tree;allow (compare,read,search) userdn = ldap:///all;;)
dn: cn=System: Read Trust Information,cn=permissions,cn=pbac,dc=ipa,dc=example
aci: (targetattr = cn || ipantflatname || ipantsecurityidentifier || ipantsidblacklistincoming || ipantsidblacklistoutgoing || ipanttrusteddomainsid || ipanttrustpartner || objectclass)(version 3.0;acl permission:System: Read Trust Information;allow (compare,read,search) userdn = ldap:///all;;)
+dn: cn=System: Read system trust accounts,cn=permissions,cn=pbac,dc=ipa,dc=example
+aci: (targetattr = gidnumber || krbprincipalname || uidnumber)(version 3.0;acl permission:System: Read system trust accounts;allow (compare,read,search) groupdn = ldap:///cn=System: Read system trust accounts,cn=permissions,cn=pbac,dc=ipa,dc=example;)
dn: cn=System: Add User to default group,cn=permissions,cn=pbac,dc=ipa,dc=example
aci: (targetattr = member)(target = ldap:///cn=ipausers,cn=groups,cn=accounts,dc=ipa,dc=example;)(version 3.0;acl permission:System: Add User to default group;allow (write) groupdn = ldap:///cn=System: Add User to default group,cn=permissions,cn=pbac,dc=ipa,dc=example;)
dn: cn=System: Add Users,cn=permissions,cn=pbac,dc=ipa,dc=example
diff --git a/install/updates/60-trusts.update b/install/updates/60-trusts.update
index 371bf656fcdea6b7ec54aeb42c5afd25ef1b90f9..d55bc94bbe917571999bcc7dfb6e6aaf641c4b49 100644
--- a/install/updates/60-trusts.update
+++ b/install/updates/60-trusts.update
@@ -15,6 +15,14 @@ default: objectClass: GroupOfNames
default: objectClass: top
default: cn: adtrust agents
+dn: cn=ADTrust Agents,cn=privileges,cn=pbac,$SUFFIX
+default: objectClass: top
+default: objectClass: groupofnames
+default: objectClass: nestedgroup
+default: cn: ADTrust Agents
+default: description: System accounts able to access trust information
+default: member: cn=adtrust agents,cn=sysaccounts,cn=etc,$SUFFIX
+
dn: cn=trusts,$SUFFIX
default: objectClass: top
default: objectClass: nsContainer
diff --git a/ipalib/plugins/trust.py b/ipalib/plugins/trust.py
index 25755d7a41e93b869f4d4afbf0ac094c42212451..99acfb8f8ce1532e4406087af3f9c158fc313159 100644
--- a/ipalib/plugins/trust.py
+++ b/ipalib/plugins/trust.py
@@ -330,6 +330,17 @@ class trust(LDAPObject):
'ipantsidblacklistincoming', 'ipantsidblacklistoutgoing'
},
},
+
+'System: Read system trust accounts': {
+'non_object': True,
+'ipapermlocation': DN(container_dn, api.env.basedn),
+'replaces_global_anonymous_aci': True,
+'ipapermright': {'read', 'search', 'compare'},
+'ipapermdefaultattr': {
+'uidnumber', 'gidnumber', 'krbprincipalname'
+},
+'default_privileges': {'ADTrust Agents'},
+},
}
label = _('Trusts')
--
1.9.3
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0233] trusts: Add more read attributes
On Wed, 25 Jun 2014, Tomas Babej wrote: On 06/25/2014 11:45 AM, Petr Viktorin wrote: On 06/24/2014 08:15 PM, Tomas Babej wrote: Attaching patch 234, which resolves another ACI issue related to trusts. On 06/24/2014 02:50 PM, Tomas Babej wrote: Hi, this is a follow up patch for 232. Read access to additional attributes is required for the trust objects. First patch looks fine. For the second: should the trust ACIs apply to other objects than (objectclas=ipanttrusteddomain)? If not, we can enable --type=trust permissions and use it to specify location filter, see attached patch. Turns out there are also kerberos principals stored under cn=trust tree and this filter would block the access to them. Attached is a new version of 234, which allows reading krbPrincipalName as well. ACK. -- / Alexander Bokovoy ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0270-0271] Add TLSA and DLV RR types to LDAP schema
On Tue, 2014-06-24 at 17:04 +0200, Petr Spacek wrote: Hello, Add TLSA and DLV RR types to LDAP schema. Those RR types will be handy for DNSSEC users. ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel Patch 270 LGTM Patch 271 NACK: You have to add the 'TLSARecord' attribute to idnsRecord objectclass -- Martin^2 Basti ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCH 0078-0079] DNSEC: Add TLSA record
Ticket https://fedorahosted.org/freeipa/ticket/4328#comment:12 Patches attached. Note: ACI will be updated in another patch which fix ACIs in DNS plugin -- Martin^2 Basti ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0078-0079] DNSEC: Add TLSA record
On Wed, 2014-06-25 at 14:31 +0200, Martin Basti wrote: Ticket https://fedorahosted.org/freeipa/ticket/4328#comment:12 Patches attached. Note: ACI will be updated in another patch which fix ACIs in DNS plugin Patches are here -- Martin^2 Basti From f429d90eadaa7da6719665dc1f9c5fcdf02dcee5 Mon Sep 17 00:00:00 2001 From: Martin Basti mba...@redhat.com Date: Wed, 25 Jun 2014 12:36:59 +0200 Subject: [PATCH 1/2] DNSSEC: add TLSA record type Ticket: https://fedorahosted.org/freeipa/ticket/4328 --- ACI.txt | 4 +-- API.txt | 20 --- VERSION | 4 +-- install/share/60ipadns.ldif | 3 ++- ipalib/plugins/dns.py | 59 + 5 files changed, 66 insertions(+), 24 deletions(-) diff --git a/ACI.txt b/ACI.txt index dd9e7c5ae2445fff53b7b3d3905d8b1d4b852aca..4faa6edc7639c341b7743bcb77541ff7be770384 100644 --- a/ACI.txt +++ b/ACI.txt @@ -39,11 +39,11 @@ aci: (targetattr = idnsallowsyncptr || idnsforwarders || idnsforwardpolicy || i dn: cn=System: Add DNS Entries,cn=permissions,cn=pbac,dc=ipa,dc=example aci: (target = ldap:///idnsname=*,cn=dns,dc=ipa,dc=example;)(version 3.0;acl permission:System: Add DNS Entries;allow (add) groupdn = ldap:///cn=System: Add DNS Entries,cn=permissions,cn=pbac,dc=ipa,dc=example;) dn: cn=System: Read DNS Entries,cn=permissions,cn=pbac,dc=ipa,dc=example -aci: (targetattr = a6record || record || afsdbrecord || arecord || certrecord || cn || cnamerecord || dlvrecord || dnamerecord || dnsclass || dnsttl || dsrecord || hinforecord || idnsallowdynupdate || idnsallowquery || idnsallowsyncptr || idnsallowtransfer || idnsforwarders || idnsforwardpolicy || idnsname || idnssoaexpire || idnssoaminimum || idnssoamname || idnssoarefresh || idnssoaretry || idnssoarname || idnssoaserial || idnsupdatepolicy || idnszoneactive || keyrecord || kxrecord || locrecord || managedby || mdrecord || minforecord || mxrecord || naptrrecord || nsec3paramrecord || nsecrecord || nsrecord || nxtrecord || objectclass || ptrrecord || rrsigrecord || sigrecord || srvrecord || sshfprecord || txtrecord)(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 || arecord || certrecord || cn || cnamerecord || dlvrecord || dnamerecord || dnsclass || dnsttl || dsrecord || hinforecord || idnsallowdynupdate || idnsallowquery || idnsallowsyncptr || idnsallowtransfer || idnsforwarders || idnsforwardpolicy || idnsname || idnssoaexpire || idnssoaminimum || idnssoamname || idnssoarefresh || idnssoaretry || idnssoarname || idnssoaserial || idnsupdatepolicy || idnszoneactive || keyrecord || kxrecord || locrecord || managedby || mdrecord || minforecord || mxrecord || naptrrecord || nsec3paramrecord || nsecrecord || nsrecord || nxtrecord || objectclass || ptrrecord || rrsigrecord || sigrecord || srvrecord || sshfprecord || tlsarecord || txtrecord)(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: cn=System: Remove DNS Entries,cn=permissions,cn=pbac,dc=ipa,dc=example aci: (target = ldap:///idnsname=*,cn=dns,dc=ipa,dc=example;)(version 3.0;acl permission:System: Remove DNS Entries;allow (delete) groupdn = ldap:///cn=System: Remove DNS Entries,cn=permissions,cn=pbac,dc=ipa,dc=example;) dn: cn=System: Update DNS Entries,cn=permissions,cn=pbac,dc=ipa,dc=example -aci: (targetattr = a6record || record || afsdbrecord || arecord || certrecord || cn || cnamerecord || dlvrecord || dnamerecord || dnsclass || dnsttl || dsrecord || hinforecord || idnsallowdynupdate || idnsallowquery || idnsallowsyncptr || idnsallowtransfer || idnsforwarders || idnsforwardpolicy || idnsname || idnssoaexpire || idnssoaminimum || idnssoamname || idnssoarefresh || idnssoaretry || idnssoarname || idnssoaserial || idnsupdatepolicy || idnszoneactive || keyrecord || kxrecord || locrecord || managedby || mdrecord || minforecord || mxrecord || naptrrecord || nsec3paramrecord || nsecrecord || nsrecord || nxtrecord || ptrrecord || rrsigrecord || sigrecord || srvrecord || sshfprecord || txtrecord)(target = ldap:///idnsname=*,cn=dns,dc=ipa,dc=example;)(version 3.0;acl permission:System: Update DNS Entries;allow (write) groupdn = ldap:///cn=System: Update DNS Entries,cn=permissions,cn=pbac,dc=ipa,dc=example;) +aci: (targetattr = a6record || record || afsdbrecord || arecord || certrecord || cn || cnamerecord || dlvrecord || dnamerecord || dnsclass || dnsttl || dsrecord || hinforecord || idnsallowdynupdate || idnsallowquery || idnsallowsyncptr || idnsallowtransfer || idnsforwarders || idnsforwardpolicy || idnsname || idnssoaexpire || idnssoaminimum ||
Re: [Freeipa-devel] [PATCHES 0066-0067] Upgrade procedure for forwardzones
On 06/24/2014 04:52 PM, Martin Basti wrote: On Tue, 2014-06-24 at 16:36 +0200, Martin Kosek wrote: On 06/18/2014 01:46 PM, Martin Basti wrote: On Wed, 2014-06-18 at 13:44 +0200, Martin Basti wrote: On Fri, 2014-06-13 at 10:28 +0200, Martin Basti wrote: Patches attached, require patches mbasti 0052-0055. ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel Rebased patches attached. PEP8 fixes. Sorry, patches are here 66.2: 1) Is it OK to just change constants in the update plugins? -PRE_UPDATE = 1 -POST_UPDATE = 2 +PRE_SCHEMA_UPDATE = 1 +PRE_UPDATE = 2 +POST_UPDATE = 3 When people refer to the types via names, it should be OK. It just seemed unnecessary to me. I checked where the constants are used, and it shouldn't broke anything. It looks weird to me to have something which happens first with last number. Should I set PRE_SCHEMA_UPDATE = 3 and leave other unchanged then? Ok, either update it or set it to 0. Up to you. 67.2: 1) update_check_forwardzones: I think we should set update_to_forward_zones to False when the objectclass is there and add a check at the beginning of the execute to simply bail out, if update_to_forward_zones is present in the sysupgrade file. This will prevent the objectclass check (which takes some time) to run again and again. Good point thanks. 2) I would use different backup name: +backup_filename = u'master-zones-transform-backup.ldif' Probably something based on time so that different installations' backup do not step on each other. You can inspire yourself in other backup files we create: # ll /var/lib/ipa/backup/ total 16 drwx--. 2 root root 4096 May 30 08:10 ipa-full-2014-05-30-08-10-13 drwx--. 2 root root 4096 May 30 08:11 ipa-full-2014-05-30-08-11-09 drwx--. 2 root root 4096 May 30 08:13 ipa-full-2014-05-30-08-13-21 -rw-r--r--. 1 root root 3441 Jun 24 16:25 master-zones-transform-backup.ldif Is this better: forward-zones-transform-%datetime%.ldif ? It is. I would do something like dns-forward-zones-backup--MM-DD.ldif to match existing files. Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0233] trusts: Add more read attributes
On 06/25/2014 01:54 PM, Alexander Bokovoy wrote: On Wed, 25 Jun 2014, Tomas Babej wrote: On 06/25/2014 11:45 AM, Petr Viktorin wrote: On 06/24/2014 08:15 PM, Tomas Babej wrote: Attaching patch 234, which resolves another ACI issue related to trusts. On 06/24/2014 02:50 PM, Tomas Babej wrote: Hi, this is a follow up patch for 232. Read access to additional attributes is required for the trust objects. First patch looks fine. For the second: should the trust ACIs apply to other objects than (objectclas=ipanttrusteddomain)? If not, we can enable --type=trust permissions and use it to specify location filter, see attached patch. Turns out there are also kerberos principals stored under cn=trust tree and this filter would block the access to them. Attached is a new version of 234, which allows reading krbPrincipalName as well. ACK. Pushed to master: c2e6b74029e08a4eadb7a14a4c711febfc83b5be -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 667 webui-ci: adjust tests to dns changes
On 6/18/2014 6:22 AM, Petr Vobornik wrote: All DNS Zone names must be fully qualified. Assuming test works, ACK. -- Endi S. Dewata ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 668 webui: fix field's default value
On 6/18/2014 6:22 AM, Petr Vobornik wrote: Fields with default value, such as DNS Zone's idnsforwardpolicy, were marked as dirty when no value was loaded and when default value of input control was other than empty. Fixes regression in DNS Zone details facet - facet is always dirty. ACK. -- Endi S. Dewata ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 669 webui: don't limit permission search in privileges
On 6/23/2014 11:09 AM, Petr Vobornik wrote: Search for privileges was limited to bindruletype==permission. There was no reason to do that. This patch removes the restriction. Related to: https://fedorahosted.org/freeipa/ticket/4079 ACK. -- Endi S. Dewata ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCH] 678-679 webui: send API version in RPC requests and adapt to new response format
ticket: https://fedorahosted.org/freeipa/ticket/4394
== [PATCH] 678 webui: send API version in RPC requests ==
Currently there is an incorrect behavior that server doesn't send datetime
and dnsname data in new format.
This patch adds the version to each RPC request making the UI look as the
latest client. Server then sends data in correct format. It also removes
the unknown version warning from each RPC response.
== [PATCH] 679 webui: extract rpc value from object envelope ==
adapt Web UI to a newer style of encapsulation object data
--
Petr Vobornik
From 457ed178131ae7520324b7fb2062658c185bed1b Mon Sep 17 00:00:00 2001
From: Petr Vobornik pvobo...@redhat.com
Date: Wed, 18 Jun 2014 16:25:56 +0200
Subject: [PATCH] webui: extract rpc value from object envelope
adapt Web UI to a newer style of encapsulation object data
https://fedorahosted.org/freeipa/ticket/4394
---
install/ui/src/freeipa/dns.js| 4 ++--
install/ui/src/freeipa/facet.js | 4 ++--
install/ui/src/freeipa/field.js | 1 +
install/ui/src/freeipa/ipa.js| 2 +-
install/ui/src/freeipa/rpc.js| 43
install/ui/src/freeipa/widget.js | 16 +++
6 files changed, 61 insertions(+), 9 deletions(-)
diff --git a/install/ui/src/freeipa/dns.js b/install/ui/src/freeipa/dns.js
index e59f6e5ad2f46febfa8d4935e549ab15892dada6..260b6f8720c8f725426be249c0a72bd72055d4e5 100644
--- a/install/ui/src/freeipa/dns.js
+++ b/install/ui/src/freeipa/dns.js
@@ -824,7 +824,7 @@ IPA.dns.record_search_facet = function(spec) {
var original = records[i];
var record = {
-idnsname: original.idnsname,
+idnsname: rpc.extract_objects(original.idnsname),
values: []
};
@@ -2280,7 +2280,7 @@ IPA.dns.ptr_redirection_dialog = function(spec) {
for (var i=0; izones.length; i++) {
-var zone_name = zones[i].idnsname[0];
+var zone_name = rpc.extract_objects(zones[i].idnsname)[0];
if (that.reverse_address.indexOf(zone_name) -1) {
var msg = text.get('@i18n:objects.dnsrecord.ptr_redir_zone');
msg = msg.replace('${zone}', zone_name);
diff --git a/install/ui/src/freeipa/facet.js b/install/ui/src/freeipa/facet.js
index 419011627d3f453e98837378f77f88eb6c622219..0bb697be0b606743279b661d2e901372d735f8c3 100644
--- a/install/ui/src/freeipa/facet.js
+++ b/install/ui/src/freeipa/facet.js
@@ -1710,11 +1710,11 @@ exp.table_facet = IPA.table_facet = function(spec, no_init) {
var result = data.result.result;
var pkey_name = that.managed_entity.metadata.primary_key;
+var adapter = builder.build('adapter', 'adapter', {context: that});
for (var i=0; iresult.length; i++) {
var record = result[i];
-var pkey = record[pkey_name];
-if (pkey instanceof Array) pkey = pkey[0];
+var pkey = adapter.load(record, pkey_name)[0];
records_map.put(pkey, record);
}
diff --git a/install/ui/src/freeipa/field.js b/install/ui/src/freeipa/field.js
index 0bc8c6f5eb633463ca829b0e46a0fbc0ffacefd7..c2e96b392bdba057828c3d5d465e7e17a52ee535 100644
--- a/install/ui/src/freeipa/field.js
+++ b/install/ui/src/freeipa/field.js
@@ -824,6 +824,7 @@ field.Adapter = declare(null, {
if (util.is_empty(value) !util.is_empty(def)) {
value = util.normalize_value(def);
}
+value = rpc.extract_objects(value);
return value;
},
diff --git a/install/ui/src/freeipa/ipa.js b/install/ui/src/freeipa/ipa.js
index 8a1ebaed76e1bd5bbecec7e80a35197191c55920..77ded3b8bbedd42472c74ca43d27e186f5c90466 100644
--- a/install/ui/src/freeipa/ipa.js
+++ b/install/ui/src/freeipa/ipa.js
@@ -583,7 +583,7 @@ IPA.update_password_expiration = function() {
var now, expires, notify_days, diff, message, container, notify;
-expires = IPA.whoami.krbpasswordexpiration;
+expires = rpc.extract_objects(IPA.whoami.krbpasswordexpiration);
expires = expires ? datetime.parse(expires[0]) : null;
notify_days = IPA.server_config.ipapwdexpadvnotify;
diff --git a/install/ui/src/freeipa/rpc.js b/install/ui/src/freeipa/rpc.js
index d49f60fee65711e0139d7dd70a92ca4168858017..756d4090fdb0d49d335191d4cf5020448dc8e207 100644
--- a/install/ui/src/freeipa/rpc.js
+++ b/install/ui/src/freeipa/rpc.js
@@ -923,5 +923,48 @@ rpc.create_4304_error_handler = function(adder_dialog) {
};
};
+/**
+ * Property names to identify objects and values to extract in
+ * `rpc.extract_objects(array)` method.
+ * @type {Array}
+ */
+rpc.extract_types = ['__base64__', '__datetime__', '__dns_name__'];
+
+/**
+ * Extract values from specially encoded objects
+ *
+ * '''
+ * // from
+ * [{__datetime__: 20140625103152Z}]
+ * // to
+ * [20140625103152Z]
+ * '''
+ *
+ * - in-place operations, modifies input array
+ * - object properties to extract are defined in `rpc.extract_types`
+ * -
Re: [Freeipa-devel] [PATCH 0058] Add the otptoken-add-yubikey command
On Wed, 2014-06-25 at 13:35 +0300, Alexander Bokovoy wrote:
On Mon, 23 Jun 2014, Nathaniel McCallum wrote:
On Mon, 2014-06-23 at 10:29 +0300, Alexander Bokovoy wrote:
On Fri, 20 Jun 2014, Nathaniel McCallum wrote:
On Thu, 2014-06-19 at 16:30 -0400, Nathaniel McCallum wrote:
This command behaves almost exactly like otptoken-add except:
1. The new token data is written directly to a YubiKey
2. The vendor/model/serial fields are populated from the YubiKey
=== NOTE ===
1. This patch depends on the new Fedora package: python-yubico. If you
would like to help with the package review, please assign yourself here:
https://bugzilla.redhat.com/show_bug.cgi?id=334
New version of the patch. This one works (yay!).
1. Because of the dependency on python-yubico, is this feature something
we want in core FreeIPA? As a subpackage? Separate project altogether?
The only dependency for python-yubico is pyusb.
I'd prefer to have it integrated but have a separate dummy subpackage
that pulls in all required dependencies, like, freeipa-tools-yubico.
Instead
of failing when 'ipa otptoken-add-yubikey' is called, please wrap the
python-yubico import into a code that allows reporting a message back to
the user advising to install the package.
Who is a supposed user for this command? IPA command line interface isn't
usually available on enrolled machines even though underlying Python
modules are all there. Are we talking about admins or just users?
As discussed on IRC, we are currently hard-coding lots of optional
dependencies. And breaking this apart into subpackages can be solved at
a later point. YubiKey is also a unique case: we don't expect to be
adding many more plugins like this.
For these reasons, I have kept this as a hard dependency. To ease this
transition, I have added python-yubico to F20 and EL6. You can help with
the update review here:
https://admin.fedoraproject.org/updates/python-yubico-1.2.1-3.fc20
https://admin.fedoraproject.org/updates/python-yubico-1.2.1-3.el6
3. This code currently emits a warning from the call to otptoken-add:
WARNING: API Version number was not sent, forward compatibility not
guaranteed. Assuming server's API version, 2.89
How do I fix this?
Do not filter 'version' field in options in execute().
I opted to not filter out version rather than hard-code it (pviktori's
suggestion). This is based on the fact that otptoken-add-yubikey is
tightly integrated with otptoken-add (even using some of the class
attributes from otptoken).
4. I am not sure why I have to delete the summary and value keys from
the return dictionary. It would be nice to display this summary message
just like otptoken-add.
I still need help on this.
5. Am I doing the ipatoken(vendor|model|serial) options correctly? These
aren't user settable, but we need to pass them from the yubikey
(client-side) to the server.
This is no longer needed since I am doing everything in forward().
However, listing these three as output params causes them to appear
before the token's ID. I don't think this is the right way to output
these. But this seems to me a framework issue.
6. I'm not sure my use of assert or ValueError are correct. What should
I do here?
Still need help here.
Fixed this part.
7. Considering that this is just a specialized invocation of
otptoken-add, can't we do this all on the client-side? This is why I had
originally used frontend.Local rather than frontend.Command.
You don't need to implement execute then, only forward, where you'll
forward your call to the server under otptoken_add name.
Typically in #forward we call super's forward but that is because we
in Command.forward() we simply forward the command to the remote backend,
using the self.name. In your case we shouldn't really have a separate
command on the server under the same name, so you'll need to avoid
calling
So, it should look like this:
def forward(self, *args, **kw):
perform yubikey initialization
filter out kw and args, if needed
return self.Backend.rpcclient.forward('otptoken_add', *args, **kw)
See service_show implementation for an example.
Fixed.
I'm attaching few fixups to the patch that make it proper reporting for
non-Yubikey case and also properly update VERSION.
Provisional ACK.
Merged.
Nathaniel
From 4a8f119c4e8de70e6f2f997167421947f18f907f Mon Sep 17 00:00:00 2001
From: Nathaniel McCallum npmccal...@redhat.com
Date: Thu, 19 Jun 2014 12:28:32 -0400
Subject: [PATCH] Add the otptoken-add-yubikey command
This command behaves almost exactly like otptoken-add except:
1. The new token data is written directly to a YubiKey
2. The vendor/model/serial fields are populated from the YubiKey
---
VERSION| 4 +-
freeipa.spec.in| 1 +
ipalib/plugins/otptoken.py | 2 +-
Re: [Freeipa-devel] [PATCH 0076] Fix incompatible DNS permission
On 06/20/2014 03:28 PM, Martin Basti wrote:
Patch attached.
Ticket:https://fedorahosted.org/freeipa/ticket/4383
This works, just two comments:
To check if an entry exists, instead of calling
api.Command['permission_show'](permission_name_rel)
you should call the more light-weight
api.Object[permission].get_dn_if_exists(permission_name_rel)
And for translated messages, use:
_('message about %(topic)s) % {...}
rather than:
_('message about %(topic)s % {...})
In other words, _() must be called on a literal string. Otherwise it
couldn't be looked up in the translation database (or even picked up by
gettext).
--
Petr³
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES 202-222] Ipaplatform refactoring
On 06/25/2014 04:01 PM, Tomas Babej wrote: On 06/25/2014 10:48 AM, Petr Viktorin wrote: On 06/19/2014 03:52 PM, Tomas Babej wrote: On 06/19/2014 12:52 PM, Tomas Babej wrote: On 06/18/2014 10:52 AM, Petr Viktorin wrote: On 06/17/2014 02:15 PM, Tomas Babej wrote: On 06/17/2014 12:03 PM, Timo Aaltonen wrote: On 17.06.2014 11:16, Martin Kosek wrote: Attached is a new version of patch 226, and a new patch 228, which moves the paths from installers to the paths module. In patch 226, there's another certificated typo in remove_ca_cert_from_systemwide_ca_store I greped the repository, and I do not see many paths lurking around any more, there are only some in the error messages (as these can't be reliably replaced automatically, and will need some manual love). If you see any forgotten paths, which should be added to the module, let me know. Well, since you asked... install/tools/ipa-upgradeconfig:236: ipautil.run([paths.PKI_SETUP_PROXY, '-pki_instance_root=/var/lib' ipaserver/install/cainstance.py:1330: -pki_instance_root=/var/lib, ipaserver/install/dsinstance.py:209:InstallLdifFile= /var/lib/dirsrv/boot.ldif ipaserver/install/dsinstance.py:210:inst_dir= /var/lib/dirsrv/scripts-$SERVERID ipaserver/install/ipa_backup.py:464: '--exclude=/var/lib/ipa/backup', ipatests/test_integration/tasks.py:451:host.run_command(find /var/lib/sss/db -name '*.ldb' | install/tools/ipa-replica-conncheck:403: /usr/sbin/ipa-replica-conncheck + install/tools/ipa-replica-conncheck:414: print_info(/usr/sbin/ipa-replica-conncheck + .join(remote_check_opts)) ipapython/ipautil.py:296:env[PATH] = /bin:/sbin:/usr/kerberos/bin:/usr/kerberos/sbin:/usr/bin:/usr/sbin ipaserver/install/cainstance.py:88:ConfigFile = /usr/share/pki/ca/conf/database.ldif ipaserver/install/bindinstance.py:829: ipautil.run(['/usr/libexec/generate-rndc-key.sh']) /me will think twice about teasing nex time. This are paths requiring manual changes in one way or the other and as such cannot be handled by my tool. Let's not stall the patcheset on this. We can fix these (and surely there are other) as we go along. I guess it'll be a while before we catch them all, but now it's at least clear where these paths should be, so anyone porting to another distro can send patches (or tickets) upstream. I see another duplicate: SSS_KRB5_INCLUDE_D = /var/lib/sss/pubconf/krb5.include.d SSSD_PUBCONF_KRB5_INCLUDE_D_DIR = /var/lib/sss/pubconf/krb5.include.d/ Could you just pick one instead? Would ipa_backup.py break if it had a trailing slash here? Yes. I verified it produces the same result with or without trailing slash, fixed. In ipa-client-install, if you set: NSSWITCH_CONF = paths.NSSWITCH_CONF then you should only use one of those later. (Preferably paths.*, to get rid of the redundant constants.) Perhaps this is for another patch that would clean up all the cases where these trivial module variables are used. I agree. Fixed this occurence. Fixed all mentioned issues. I also attached a patch 230, which removes the base Authconfig class. Attaching one additional patch, which removes unnecessary build warnings. 226, 230, 231 look good Attaching whole updated patchset. Attaching one more patch which should fix broken CI tests. -- Tomas Babej Associate Software Engineer | Red Hat | Identity Management RHCE | Brno Site | IRC: tbabej | freeipa.org ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel -- Tomas Babej Associate Software Engineer | Red Hat | Identity Management RHCE | Brno Site | IRC: tbabej | freeipa.org From 2fda5e386b9fdf75b6c02fbeedafaeb001d80a74 Mon Sep 17 00:00:00 2001 From: Tomas Babej tba...@redhat.com Date: Wed, 25 Jun 2014 16:12:19 +0200 Subject: [PATCH] ipaplatform: Fix misspelled path constant --- ipatests/test_integration/tasks.py | 2 +- ipatests/test_integration/test_caless.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/ipatests/test_integration/tasks.py b/ipatests/test_integration/tasks.py index ccb0d8693a1e89d95bbeb4c75fc263d0f689cb36..cd8f98306030f46c099a08ca1a558fd10807bfa9 100644 --- a/ipatests/test_integration/tasks.py +++ b/ipatests/test_integration/tasks.py @@ -219,7 +219,7 @@ def install_replica(master, replica, setup_ca=True): '--ip-address', replica.ip, replica.hostname]) replica_bundle = master.get_file_contents( -paths.REPLICA_INFO_TEMPLATE_GPG % replica.hostname) +paths.REPLICA_INFO_GPG_TEMPLATE % replica.hostname) replica_filename = os.path.join(replica.config.test_dir, 'replica-info.gpg') replica.put_file_contents(replica_filename, replica_bundle) diff --git a/ipatests/test_integration/test_caless.py b/ipatests/test_integration/test_caless.py index
Re: [Freeipa-devel] [PATCH] 667 webui-ci: adjust tests to dns changes
On 25.6.2014 15:30, Endi Sukma Dewata wrote: On 6/18/2014 6:22 AM, Petr Vobornik wrote: All DNS Zone names must be fully qualified. Assuming test works, ACK. pushed to master: 15374cf58fe26396be7bc70d7133b501b11dad6d webui-ci: adjust tests to dns changes -- Petr Vobornik ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 668 webui: fix field's default value
On 25.6.2014 15:30, Endi Sukma Dewata wrote: On 6/18/2014 6:22 AM, Petr Vobornik wrote: Fields with default value, such as DNS Zone's idnsforwardpolicy, were marked as dirty when no value was loaded and when default value of input control was other than empty. Fixes regression in DNS Zone details facet - facet is always dirty. ACK. pushed to master: bfdf9039ce6396e343f85d475f589efd57d13304 webui: fix field's default value -- Petr Vobornik ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 669 webui: don't limit permission search in privileges
On 25.6.2014 15:31, Endi Sukma Dewata wrote: On 6/23/2014 11:09 AM, Petr Vobornik wrote: Search for privileges was limited to bindruletype==permission. There was no reason to do that. This patch removes the restriction. Related to: https://fedorahosted.org/freeipa/ticket/4079 ACK. pushed to master: 6dab9123be1d4c2db8a194d00f05884738fb692a webui: don't limit permission search in privileges -- Petr Vobornik ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES 202-222] Ipaplatform refactoring
On 06/25/2014 04:13 PM, Tomas Babej wrote: On 06/25/2014 04:01 PM, Tomas Babej wrote: On 06/25/2014 10:48 AM, Petr Viktorin wrote: On 06/19/2014 03:52 PM, Tomas Babej wrote: On 06/19/2014 12:52 PM, Tomas Babej wrote: On 06/18/2014 10:52 AM, Petr Viktorin wrote: On 06/17/2014 02:15 PM, Tomas Babej wrote: On 06/17/2014 12:03 PM, Timo Aaltonen wrote: On 17.06.2014 11:16, Martin Kosek wrote: Attached is a new version of patch 226, and a new patch 228, which moves the paths from installers to the paths module. In patch 226, there's another certificated typo in remove_ca_cert_from_systemwide_ca_store I greped the repository, and I do not see many paths lurking around any more, there are only some in the error messages (as these can't be reliably replaced automatically, and will need some manual love). If you see any forgotten paths, which should be added to the module, let me know. Well, since you asked... install/tools/ipa-upgradeconfig:236: ipautil.run([paths.PKI_SETUP_PROXY, '-pki_instance_root=/var/lib' ipaserver/install/cainstance.py:1330: -pki_instance_root=/var/lib, ipaserver/install/dsinstance.py:209:InstallLdifFile= /var/lib/dirsrv/boot.ldif ipaserver/install/dsinstance.py:210:inst_dir= /var/lib/dirsrv/scripts-$SERVERID ipaserver/install/ipa_backup.py:464: '--exclude=/var/lib/ipa/backup', ipatests/test_integration/tasks.py:451:host.run_command(find /var/lib/sss/db -name '*.ldb' | install/tools/ipa-replica-conncheck:403: /usr/sbin/ipa-replica-conncheck + install/tools/ipa-replica-conncheck:414: print_info(/usr/sbin/ipa-replica-conncheck + .join(remote_check_opts)) ipapython/ipautil.py:296:env[PATH] = /bin:/sbin:/usr/kerberos/bin:/usr/kerberos/sbin:/usr/bin:/usr/sbin ipaserver/install/cainstance.py:88:ConfigFile = /usr/share/pki/ca/conf/database.ldif ipaserver/install/bindinstance.py:829: ipautil.run(['/usr/libexec/generate-rndc-key.sh']) /me will think twice about teasing nex time. This are paths requiring manual changes in one way or the other and as such cannot be handled by my tool. Let's not stall the patcheset on this. We can fix these (and surely there are other) as we go along. I guess it'll be a while before we catch them all, but now it's at least clear where these paths should be, so anyone porting to another distro can send patches (or tickets) upstream. I see another duplicate: SSS_KRB5_INCLUDE_D = /var/lib/sss/pubconf/krb5.include.d SSSD_PUBCONF_KRB5_INCLUDE_D_DIR = /var/lib/sss/pubconf/krb5.include.d/ Could you just pick one instead? Would ipa_backup.py break if it had a trailing slash here? Yes. I verified it produces the same result with or without trailing slash, fixed. In ipa-client-install, if you set: NSSWITCH_CONF = paths.NSSWITCH_CONF then you should only use one of those later. (Preferably paths.*, to get rid of the redundant constants.) Perhaps this is for another patch that would clean up all the cases where these trivial module variables are used. I agree. Fixed this occurence. Fixed all mentioned issues. I also attached a patch 230, which removes the base Authconfig class. Attaching one additional patch, which removes unnecessary build warnings. 226, 230, 231 look good Attaching whole updated patchset. Attaching one more patch which should fix broken CI tests. -- Tomas Babej Associate Software Engineer | Red Hat | Identity Management RHCE | Brno Site | IRC: tbabej | freeipa.org ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel -- Tomas Babej Associate Software Engineer | Red Hat | Identity Management RHCE | Brno Site | IRC: tbabej | freeipa.org ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel Self-NACK - It seems I omitted one occurence of NSSWITCH_CONF in ipa-client-install, fixed now. Attaching the whole patchset for your convenience. -- Tomas Babej Associate Software Engineer | Red Hat | Identity Management RHCE | Brno Site | IRC: tbabej | freeipa.org From 5c1cc30a4100ab11fa9a31d478ecb4677edf78dc Mon Sep 17 00:00:00 2001 From: Tomas Babej tba...@redhat.com Date: Wed, 25 Jun 2014 16:12:19 +0200 Subject: [PATCH] ipaplatform: Fix misspelled path constant --- ipatests/test_integration/tasks.py | 2 +- ipatests/test_integration/test_caless.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/ipatests/test_integration/tasks.py b/ipatests/test_integration/tasks.py index ccb0d8693a1e89d95bbeb4c75fc263d0f689cb36..cd8f98306030f46c099a08ca1a558fd10807bfa9 100644 --- a/ipatests/test_integration/tasks.py +++ b/ipatests/test_integration/tasks.py @@ -219,7 +219,7 @@ def install_replica(master, replica, setup_ca=True): '--ip-address', replica.ip,
Re: [Freeipa-devel] [PATCH 0076] Fix incompatible DNS permission
On Wed, 2014-06-25 at 15:54 +0200, Petr Viktorin wrote:
On 06/20/2014 03:28 PM, Martin Basti wrote:
Patch attached.
Ticket:https://fedorahosted.org/freeipa/ticket/4383
This works, just two comments:
To check if an entry exists, instead of calling
api.Command['permission_show'](permission_name_rel)
you should call the more light-weight
api.Object[permission].get_dn_if_exists(permission_name_rel)
And for translated messages, use:
_('message about %(topic)s) % {...}
rather than:
_('message about %(topic)s % {...})
In other words, _() must be called on a literal string. Otherwise it
couldn't be looked up in the translation database (or even picked up by
gettext).
Thank you for review.
Updated patch attached.
--
Martin^2 Basti
From bc11e3a533756714aca9dd44ef982d3284844dcd Mon Sep 17 00:00:00 2001
From: Martin Basti mba...@redhat.com
Date: Fri, 20 Jun 2014 13:52:12 +0200
Subject: [PATCH] Fix incompatible DNS permission
dns(forward)zone-add/remove-permission can work with permissions with
relative zone name
Ticket:https://fedorahosted.org/freeipa/ticket/4383
---
ipalib/plugins/dns.py | 31 ++-
1 file changed, 30 insertions(+), 1 deletion(-)
diff --git a/ipalib/plugins/dns.py b/ipalib/plugins/dns.py
index a81fb575b4af8f8a7df577c6a6bf230056f6c660..890d2cceb01faf0e8933a884d812aa2af9f08ab9 100644
--- a/ipalib/plugins/dns.py
+++ b/ipalib/plugins/dns.py
@@ -1876,6 +1876,23 @@ class DNSZoneBase_add_permission(LDAPQuery):
self.obj.handle_not_found(*keys)
permission_name = self.obj.permission_name(keys[-1])
+
+# compatibility with older IPA versions which allows relative zonenames
+permission_name_rel = self.obj.permission_name(
+keys[-1].relativize(DNSName.root)
+)
+try:
+api.Object['permission'].get_dn_if_exists(permission_name_rel)
+except errors.NotFound:
+pass
+else:
+# permission exists without absolute domain name
+raise errors.DuplicateEntry(
+message=_('permission %(value)s already exists') % {
+'value': permission_name
+}
+)
+
permission = api.Command['permission_add_noaci'](permission_name,
ipapermissiontype=u'SYSTEM'
)['result']
@@ -1922,7 +1939,19 @@ class DNSZoneBase_remove_permission(LDAPQuery):
pass
permission_name = self.obj.permission_name(keys[-1])
-api.Command['permission_del'](permission_name, force=True)
+try:
+api.Command['permission_del'](permission_name, force=True)
+except errors.NotFound, e:
+# compatibility, older IPA versions which allows to create zone
+# without absolute zone name
+permission_name_rel = self.obj.permission_name(
+keys[-1].relativize(DNSName.root)
+)
+try:
+api.Command['permission_del'](permission_name_rel, force=True)
+except errors.NotFound:
+raise e # re-raise original exception
+
return dict(
result=True,
--
1.8.3.1
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES 0066-0067] Upgrade procedure for forwardzones
On Wed, 2014-06-25 at 14:36 +0200, Martin Kosek wrote:
On 06/24/2014 04:52 PM, Martin Basti wrote:
On Tue, 2014-06-24 at 16:36 +0200, Martin Kosek wrote:
On 06/18/2014 01:46 PM, Martin Basti wrote:
On Wed, 2014-06-18 at 13:44 +0200, Martin Basti wrote:
On Fri, 2014-06-13 at 10:28 +0200, Martin Basti wrote:
Patches attached, require patches mbasti 0052-0055.
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Rebased patches attached.
PEP8 fixes.
Sorry, patches are here
66.2:
1) Is it OK to just change constants in the update plugins?
-PRE_UPDATE = 1
-POST_UPDATE = 2
+PRE_SCHEMA_UPDATE = 1
+PRE_UPDATE = 2
+POST_UPDATE = 3
When people refer to the types via names, it should be OK. It just seemed
unnecessary to me.
I checked where the constants are used, and it shouldn't broke anything.
It looks weird to me to have something which happens first with last
number.
Should I set PRE_SCHEMA_UPDATE = 3 and leave other unchanged then?
Ok, either update it or set it to 0. Up to you.
67.2:
1) update_check_forwardzones:
I think we should set update_to_forward_zones to False when the
objectclass is
there and add a check at the beginning of the execute to simply bail out,
if
update_to_forward_zones is present in the sysupgrade file.
This will prevent the objectclass check (which takes some time) to run
again
and again.
Good point thanks.
2) I would use different backup name:
+backup_filename = u'master-zones-transform-backup.ldif'
Probably something based on time so that different installations' backup
do not
step on each other. You can inspire yourself in other backup files we
create:
# ll /var/lib/ipa/backup/
total 16
drwx--. 2 root root 4096 May 30 08:10 ipa-full-2014-05-30-08-10-13
drwx--. 2 root root 4096 May 30 08:11 ipa-full-2014-05-30-08-11-09
drwx--. 2 root root 4096 May 30 08:13 ipa-full-2014-05-30-08-13-21
-rw-r--r--. 1 root root 3441 Jun 24 16:25
master-zones-transform-backup.ldif
Is this better: forward-zones-transform-%datetime%.ldif ?
It is. I would do something like dns-forward-zones-backup--MM-DD.ldif to
match existing files.
Martin
Updated patches attached
--
Martin^2 Basti
From b95439c2e593f949f44ef9e9bb1faedf4aa0f17e Mon Sep 17 00:00:00 2001
From: Martin Basti mba...@redhat.com
Date: Fri, 13 Jun 2014 10:15:23 +0200
Subject: [PATCH 1/2] Added upgrade step executed before schmema is upgraded
Class PreSchemaUpdate is executed before ldap schema update
This is required by ticket: https://fedorahosted.org/freeipa/ticket/3210
---
ipaserver/install/ipa_ldap_updater.py | 14 --
ipaserver/install/ldapupdate.py | 15 ++-
ipaserver/install/plugins/__init__.py | 1 +
ipaserver/install/plugins/baseupdate.py | 15 ++-
ipaserver/install/upgradeinstance.py| 17 +
5 files changed, 54 insertions(+), 8 deletions(-)
diff --git a/ipaserver/install/ipa_ldap_updater.py b/ipaserver/install/ipa_ldap_updater.py
index e8ef2b576a21f316941260cc00de7910859a9cec..fbbef142a387c6303d773e93be979d46d5f15a5b 100644
--- a/ipaserver/install/ipa_ldap_updater.py
+++ b/ipaserver/install/ipa_ldap_updater.py
@@ -191,12 +191,6 @@ class LDAPUpdater_NonUpgrade(LDAPUpdater):
modified = False
-if options.update_schema:
-modified = schemaupdate.update_schema(
-options.schema_files,
-dm_password=self.dirman_password,
-live_run=not options.test) or modified
-
ld = LDAPUpdate(
dm_password=self.dirman_password,
sub_dict={},
@@ -204,6 +198,14 @@ class LDAPUpdater_NonUpgrade(LDAPUpdater):
ldapi=options.ldapi,
plugins=options.plugins or self.run_plugins)
+modified = ld.pre_schema_update(ordered=True)
+
+if options.update_schema:
+modified = schemaupdate.update_schema(
+options.schema_files,
+dm_password=self.dirman_password,
+live_run=not options.test) or modified
+
if not self.files:
self.files = ld.get_all_files(UPDATES_DIR)
diff --git a/ipaserver/install/ldapupdate.py b/ipaserver/install/ldapupdate.py
index ecdf8e6e12751dbf472411ec08a3be2bf315c31c..b6c6d2b90b1f86f5d45985d4e1476fd03f7d112d 100644
--- a/ipaserver/install/ldapupdate.py
+++ b/ipaserver/install/ldapupdate.py
@@ -42,7 +42,8 @@ from ipalib import api
from ipaplatform.paths import paths
from ipapython.dn import DN
from ipapython.ipa_log_manager import *
-from ipaserver.install.plugins import PRE_UPDATE, POST_UPDATE
+from ipaserver.install.plugins import (PRE_UPDATE, POST_UPDATE,
+ PRE_SCHEMA_UPDATE)
from ipaserver.plugins import ldap2
UPDATES_DIR=paths.UPDATES_DIR
@@
[Freeipa-devel] [PATCH 0236] ipaldap: Fallback to string if datetime conversion went wrong
Hi,
Our datetime conversion does not support full LDAP Generalized
time syntax. In the unsupported cases, we should fall back
to string representation of the attribute.
In particular, '0' is used to denote no value of LDAP generalized
time attribute.
https://fedorahosted.org/freeipa/ticket/4350
--
Tomas Babej
Associate Software Engineer | Red Hat | Identity Management
RHCE | Brno Site | IRC: tbabej | freeipa.org
From f1ec7165b433056aafed8c14babf5033c896fde0 Mon Sep 17 00:00:00 2001
From: Tomas Babej tba...@redhat.com
Date: Tue, 17 Jun 2014 17:17:08 +0200
Subject: [PATCH] ipaldap: Fallback to string if datetime conversion went wrong
Our datetime conversion does not support full LDAP Generalized
time syntax. In the unsupported cases, we should fall back
to string representation of the attribute.
In particular, '0' is used to denote no value of LDAP generalized
time attribute.
https://fedorahosted.org/freeipa/ticket/4350
---
ipapython/ipaldap.py | 8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/ipapython/ipaldap.py b/ipapython/ipaldap.py
index 21706cff08a0d8be07db8a1b5fdb0367c10ad53d..ed31057bd0631ad91dd22bbce8a7d7592cd2cf90 100644
--- a/ipapython/ipaldap.py
+++ b/ipapython/ipaldap.py
@@ -439,7 +439,13 @@ class IPASimpleLDAPObject(object):
elif target_type is unicode:
return val.decode('utf-8')
elif target_type is datetime.datetime:
-return datetime.datetime.strptime(val, LDAP_GENERALIZED_TIME_FORMAT)
+try:
+return datetime.datetime.strptime(
+ val, LDAP_GENERALIZED_TIME_FORMAT)
+except Exception, e:
+# If the datetime conversion went wrong, use string
+# instead
+return val
else:
return target_type(val)
except Exception, e:
--
1.9.3
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0236] ipaldap: Fallback to string if datetime conversion went wrong
Hi,
On 25.6.2014 17:17, Tomas Babej wrote:
Hi,
Our datetime conversion does not support full LDAP Generalized
time syntax. In the unsupported cases, we should fall back
to string representation of the attribute.
In particular, '0' is used to denote no value of LDAP generalized
time attribute.
https://fedorahosted.org/freeipa/ticket/4350
NACK, this beats the purpose of decoding of the values, because it
requires you to check the type of the value before using it.
Instead, you should either fix the code that uses the
nsds5ReplicaLastUpdate{Start,End} attributes to access their raw value
directly, or exclude the attributes from decoding to datetime by
overriding their type in IPASimpleLDAPObject._SYNTAX_OVERRIDE.
Honza
--
Jan Cholasta
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 676 rpcserver: fix local vs utc time comparison
Hi, On 24.6.2014 16:02, Petr Vobornik wrote: login_password did not work properly in timezones other than +0h because local time was compared with utc time. ACK. Bug introduced in: https://fedorahosted.org/freeipa/ticket/4339 We should review other code for invalid usage of datetime.now() All other uses of datetime.now() predate LDAP datetime decoding, so I think we are fine. Honza -- Jan Cholasta ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES 187-201] Improvements and coverage for sudorule plugin
On 06/25/2014 04:28 PM, Tomas Babej wrote:
On 06/18/2014 09:54 AM, Petr Viktorin wrote:
On 06/17/2014 12:25 PM, Tomas Babej wrote:
On 05/26/2014 06:20 PM, Petr Viktorin wrote:
On 05/20/2014 06:15 PM, Tomas Babej wrote:
Hi,
the following set of patches fixes:
https://fedorahosted.org/freeipa/ticket/4274
https://fedorahosted.org/freeipa/ticket/4263
https://fedorahosted.org/freeipa/ticket/4324
https://fedorahosted.org/freeipa/ticket/4340
https://fedorahosted.org/freeipa/ticket/4341
and additional minor issues.
The improvemed CI test coverage for the sudorule plugin is added as a
bonus.
You've dropped most of the long commit messages and ticket URLs. Why?
Sorry about that.. fixed!
0187: OK
0188 - sudorule: Allow using hostmasks for setting allowed hosts
If I run sudorule-add-host / sudorule-remove-host with a hostmask, but
not host/hostgroup, I get prompted for host and hostgroup. I don't
think that's the intended behavior.
This problem is beyond this patchset. Observe that same thing happens
with ipa group-add-member --external.
I was afraid it wouldn't bee that easy.
I'm not sure if there's a ticket for this though.
There is now. https://fedorahosted.org/freeipa/ticket/4400
0189: OK
0190: OK
0191: OK
0192: OK
0193 sudorule: Make sure all the relevant attributes are checked when
setting category to ALL
You're missing the `_` for the hostcategory error message.
Did you think about using something like _(%s cannot be set to 'all'
while there are %s)?
Fixed. Initially, I changed the message as you suggested, but then I
realized, that this might pose a problem for translations that do not
follow the word order in the sentence as it is defined in English
language.
Right, sorry for the incorrect example. You can use named
substitutions for that:
_(can't %(action)s while %(state)s) % {'action': 'move',
'state': 'asleep'}
The placeholder syntax is %(xyz)s, not %{xyz}.
(Have you been using format() too much?)
One more thing - the function is only called once, could you move it
to the for loop?
Well, I meant put the function's body in the for loop, getting rid of
the function. But that's just a nitpick.
0194: OK
0195: OK
0196-0198: OK
0199-0201: OK
0225:
Looks good. Could you also document the arguments return value in
*_external_post_callback docstrings?
I did. The updated patchset attached.
Thanks!
--
Petr³
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0236] ipaldap: Fallback to string if datetime conversion went wrong
On 06/25/2014 05:29 PM, Jan Cholasta wrote:
Hi,
On 25.6.2014 17:17, Tomas Babej wrote:
Hi,
Our datetime conversion does not support full LDAP Generalized
time syntax. In the unsupported cases, we should fall back
to string representation of the attribute.
In particular, '0' is used to denote no value of LDAP generalized
time attribute.
https://fedorahosted.org/freeipa/ticket/4350
NACK, this beats the purpose of decoding of the values, because it
requires you to check the type of the value before using it.
Instead, you should either fix the code that uses the
nsds5ReplicaLastUpdate{Start,End} attributes to access their raw value
directly, or exclude the attributes from decoding to datetime by
overriding their type in IPASimpleLDAPObject._SYNTAX_OVERRIDE.
Honza
I agree that just returning a string when conversion fails is the wrong
thing to do.
I think that if LDAP generalized can contain the empty/invalid value, we
should convert the '0' to what Python uses for that -- None.
--
Petr³
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0076] Fix incompatible DNS permission
On 06/25/2014 05:03 PM, Martin Basti wrote:
On Wed, 2014-06-25 at 15:54 +0200, Petr Viktorin wrote:
On 06/20/2014 03:28 PM, Martin Basti wrote:
Patch attached.
Ticket:https://fedorahosted.org/freeipa/ticket/4383
This works, just two comments:
To check if an entry exists, instead of calling
api.Command['permission_show'](permission_name_rel)
you should call the more light-weight
api.Object[permission].get_dn_if_exists(permission_name_rel)
And for translated messages, use:
_('message about %(topic)s) % {...}
rather than:
_('message about %(topic)s % {...})
In other words, _() must be called on a literal string. Otherwise it
couldn't be looked up in the translation database (or even picked up by
gettext).
Thank you for review.
Updated patch attached.
Thanks!
ACK, pushed to master: 816007bdd911065b42170a06aea3cf750a5198fe
--
Petr³
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0077] Fix ACI in DNS (was Add dnssecinlinesigning attribute to ACI)
On Wed, 2014-06-25 at 12:13 +0200, Petr Viktorin wrote: On 06/20/2014 03:32 PM, Martin Basti wrote: Required patches: mbasti-0060, mbasti-0073 Patch attached. Hi, For the raw ACI in dns.ldif, there are some more hoops to jump through. Remove the ACI from /install/share/dns.ldif entirely (except for schema, we're slowly replacing the .ldif content by .update files). In install/updates/40-dns.update, you'll notice the Update DNS entries in a zone ACI is already being added. You'll need to replace it, using a line like: replace:aci:'old ACI::new ACI' This will remove the old value that IPA 3.x users still have. I see you already changed the ACI in 7cdc417, in dns.ldif only. Be sureto use the original value for old ACI. As we discuss personally, ACI requires more changes than add idnssecinlinesingning only. Updated patch attached. -- Martin^2 Basti From f4e65b54a7c99e27e900787222eb2043848104d8 Mon Sep 17 00:00:00 2001 From: Martin Basti mba...@redhat.com Date: Wed, 25 Jun 2014 17:24:45 +0200 Subject: [PATCH] Fix ACI in DNS Added ACI for idnssecinlinesigning, dlvrecord, nsec3paramrecord, tlsarecord --- ACI.txt | 4 ++-- install/share/dns.ldif| 1 - install/updates/40-dns.update | 1 + ipalib/plugins/dns.py | 4 ++-- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/ACI.txt b/ACI.txt index 23b0c6608cda06eda2273789fd4ba1f7f8a3e5f5..fe55e37215e59a1d2bc4295eb691f7c2e9398376 100644 --- a/ACI.txt +++ b/ACI.txt @@ -39,11 +39,11 @@ aci: (targetattr = idnsallowsyncptr || idnsforwarders || idnsforwardpolicy || i dn: cn=System: Add DNS Entries,cn=permissions,cn=pbac,dc=ipa,dc=example aci: (target = ldap:///idnsname=*,cn=dns,dc=ipa,dc=example;)(version 3.0;acl permission:System: Add DNS Entries;allow (add) groupdn = ldap:///cn=System: Add DNS Entries,cn=permissions,cn=pbac,dc=ipa,dc=example;) dn: cn=System: Read DNS Entries,cn=permissions,cn=pbac,dc=ipa,dc=example -aci: (targetattr = a6record || record || afsdbrecord || arecord || certrecord || cn || cnamerecord || dlvrecord || dnamerecord || dnsclass || dnsttl || dsrecord || hinforecord || idnsallowdynupdate || idnsallowquery || idnsallowsyncptr || idnsallowtransfer || idnsforwarders || idnsforwardpolicy || idnsname || idnssoaexpire || idnssoaminimum || idnssoamname || idnssoarefresh || idnssoaretry || idnssoarname || idnssoaserial || idnsupdatepolicy || idnszoneactive || keyrecord || kxrecord || locrecord || managedby || mdrecord || minforecord || mxrecord || naptrrecord || nsec3paramrecord || nsecrecord || nsrecord || nxtrecord || objectclass || ptrrecord || rrsigrecord || sigrecord || srvrecord || sshfprecord || tlsarecord || txtrecord)(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 || arecord || certrecord || cn || cnamerecord || dlvrecord || dnamerecord || dnsclass || dnsttl || dsrecord || hinforecord || idnsallowdynupdate || idnsallowquery || idnsallowsyncptr || idnsallowtransfer || idnsforwarders || idnsforwardpolicy || idnsname || idnssecinlinesigning || idnssoaexpire || idnssoaminimum || idnssoamname || idnssoarefresh || idnssoaretry || idnssoarname || idnssoaserial || idnsupdatepolicy || idnszoneactive || keyrecord || kxrecord || locrecord || managedby || mdrecord || minforecord || mxrecord || naptrrecord || nsec3paramrecord || nsecrecord || nsrecord || nxtrecord || objectclass || ptrrecord || rrsigrecord || sigrecord || srvrecord || sshfprecord || tlsarecord || txtrecord)(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: cn=System: Remove DNS Entries,cn=permissions,cn=pbac,dc=ipa,dc=example aci: (target = ldap:///idnsname=*,cn=dns,dc=ipa,dc=example;)(version 3.0;acl permission:System: Remove DNS Entries;allow (delete) groupdn = ldap:///cn=System: Remove DNS Entries,cn=permissions,cn=pbac,dc=ipa,dc=example;) dn: cn=System: Update DNS Entries,cn=permissions,cn=pbac,dc=ipa,dc=example -aci: (targetattr = a6record || record || afsdbrecord || arecord || certrecord || cn || cnamerecord || dlvrecord || dnamerecord || dnsclass || dnsttl || dsrecord || hinforecord || idnsallowdynupdate || idnsallowquery || idnsallowsyncptr || idnsallowtransfer || idnsforwarders || idnsforwardpolicy || idnsname || idnssoaexpire || idnssoaminimum || idnssoamname || idnssoarefresh || idnssoaretry || idnssoarname || idnssoaserial || idnsupdatepolicy || idnszoneactive || keyrecord || kxrecord || locrecord || managedby || mdrecord || minforecord || mxrecord || naptrrecord || nsec3paramrecord || nsecrecord || nsrecord || nxtrecord || ptrrecord || rrsigrecord || sigrecord ||
Re: [Freeipa-devel] [PATCH 0077] Fix ACI in DNS (was Add dnssecinlinesigning attribute to ACI)
On Wed, 2014-06-25 at 18:47 +0200, Martin Basti wrote: On Wed, 2014-06-25 at 12:13 +0200, Petr Viktorin wrote: On 06/20/2014 03:32 PM, Martin Basti wrote: Required patches: mbasti-0060, mbasti-0073 Patch attached. Hi, For the raw ACI in dns.ldif, there are some more hoops to jump through. Remove the ACI from /install/share/dns.ldif entirely (except for schema, we're slowly replacing the .ldif content by .update files). In install/updates/40-dns.update, you'll notice the Update DNS entries in a zone ACI is already being added. You'll need to replace it, using a line like: replace:aci:'old ACI::new ACI' This will remove the old value that IPA 3.x users still have. I see you already changed the ACI in 7cdc417, in dns.ldif only. Be sureto use the original value for old ACI. As we discuss personally, ACI requires more changes than add idnssecinlinesingning only. Updated patch attached. Patch freeipa-mbasti-0078-DNSSEC-add-TLSA-record-type.patch is required. -- Martin^2 Basti ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCH] 680-682 webui: validation reporting improvements
Patch 618 fixes a bug.
Patches 680 and 681 were implemented along with it. They address
pspacek's usability rant :).
[PATCH] 680 webui: show notification instead of modal dialog on
validation error
[PATCH] 681 webui: fix required error notification in multivalued widget
[PATCH] 682 webui: focus invalid widget on validation error
--
Petr Vobornik
From 5c3fcd240d61f378a8bb7e8cd4c2129cd11930df Mon Sep 17 00:00:00 2001
From: Petr Vobornik pvobo...@redhat.com
Date: Wed, 25 Jun 2014 15:17:26 +0200
Subject: [PATCH] webui: focus invalid widget on validation error
---
install/ui/src/freeipa/add.js | 7 +--
install/ui/src/freeipa/details.js | 4 +++-
install/ui/src/freeipa/widget.js | 22 ++
3 files changed, 30 insertions(+), 3 deletions(-)
diff --git a/install/ui/src/freeipa/add.js b/install/ui/src/freeipa/add.js
index a4b5d3649d2df2933cb7eb7c8a24a18b7b789f50..78f3890ad2320cbc3afd5cb9ae1e4ae2359d8023 100644
--- a/install/ui/src/freeipa/add.js
+++ b/install/ui/src/freeipa/add.js
@@ -20,7 +20,7 @@
*/
define(['./ipa', './jquery', './navigation', './rpc', './text', './field', './widget', './dialog'],
- function(IPA, $, navigation, rpc, text) {
+ function(IPA, $, navigation, rpc, text, field_mod, widget_mod) {
/**
* Entity adder dialog
@@ -219,7 +219,10 @@ IPA.entity_adder_dialog = function(spec) {
*/
that.add = function(on_success, on_error) {
-if (!that.validate()) return;
+if (!that.validate()) {
+widget_mod.focus_invalid(that);
+return;
+}
var record = {};
that.save(record);
diff --git a/install/ui/src/freeipa/details.js b/install/ui/src/freeipa/details.js
index ed057e98c14e8ee72e5f1596ed24599eb27abfa5..7aa4c0ef6541900d6fa5b14b16ec964b50349015 100644
--- a/install/ui/src/freeipa/details.js
+++ b/install/ui/src/freeipa/details.js
@@ -31,9 +31,10 @@ define([
'./rpc',
'./spec_util',
'./text',
+'./widget',
'./facet',
'./add'],
-function(lang, builder, IPA, $, phases, reg, rpc, su, text) {
+function(lang, builder, IPA, $, phases, reg, rpc, su, text, widget_mod) {
/**
* Details module
@@ -1436,6 +1437,7 @@ exp.update_action = IPA.update_action = function(spec) {
if (!facet.validate()) {
facet.show_validation_error();
+widget_mod.focus_invalid(facet);
return;
}
diff --git a/install/ui/src/freeipa/widget.js b/install/ui/src/freeipa/widget.js
index 3d3f427af43e429124f5e19a2604bd443a6e39c8..ab86c72d1abfae87984fa2212040a3e6c63f2f31 100644
--- a/install/ui/src/freeipa/widget.js
+++ b/install/ui/src/freeipa/widget.js
@@ -5747,6 +5747,28 @@ exp.activity_widget = IPA.activity_widget = function(spec) {
};
/**
+ * Find and focus first focusable invalid widget
+ * @member widget
+ * @param {IPA.widget|facet.facet} widget Widget container
+ * @return {boolean} A widget was focused
+ */
+exp.focus_invalid = function(widget) {
+
+var widgets = widget.widgets.widgets;
+var focused = false;
+for (var i=0, l=widgets.length; il; i++) {
+var w = widgets.values[i];
+if (w.valid === false w.focus_input) {
+w.focus_input();
+focused = true;
+}
+else if (w.widgets) focused = exp.focus_invalid(w);
+if (focused) break;
+}
+return focused;
+};
+
+/**
* pre_op operations for widgets
* - sets facet and entity if present in context
* @member widget
--
1.9.0
From 431dad35cbcb02351786fd75bd34aa86781d42fb Mon Sep 17 00:00:00 2001
From: Petr Vobornik pvobo...@redhat.com
Date: Wed, 25 Jun 2014 14:50:16 +0200
Subject: [PATCH] webui: fix required error notification in multivalued widget
---
install/ui/src/freeipa/widget.js | 7 +++
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/install/ui/src/freeipa/widget.js b/install/ui/src/freeipa/widget.js
index a49c6282b1977008e80a9c02754df44de31c8b02..3d3f427af43e429124f5e19a2604bd443a6e39c8 100644
--- a/install/ui/src/freeipa/widget.js
+++ b/install/ui/src/freeipa/widget.js
@@ -909,7 +909,7 @@ IPA.multivalued_widget = function(spec) {
var old = that.valid;
that.valid = result.valid;
-if (!result.valid result.errors) {
+if (!result.valid result.results) {
var offset = 0;
for (var i=0; ithat.rows.length; i++) {
@@ -928,10 +928,9 @@ IPA.multivalued_widget = function(spec) {
var error_link = that.get_error_link();
error_link.css('display', 'none');
error_link.html('');
-} else {
-that.show_error(result.message);
}
-
+} else if (!result.valid) {
+that.show_error(result.message);
} else {
that.hide_error();
}
--
1.9.0
From 65ce6fcd7578e0dbe03adf1ba8b9ae3fc67c09bb Mon Sep 17 00:00:00 2001
From: Petr Vobornik pvobo...@redhat.com
Date:
Re: [Freeipa-devel] [PATCHES 202-222] Ipaplatform refactoring
On 06/25/2014 04:59 PM, Tomas Babej wrote: On 06/25/2014 04:13 PM, Tomas Babej wrote: On 06/25/2014 04:01 PM, Tomas Babej wrote: On 06/25/2014 10:48 AM, Petr Viktorin wrote: On 06/19/2014 03:52 PM, Tomas Babej wrote: On 06/19/2014 12:52 PM, Tomas Babej wrote: On 06/18/2014 10:52 AM, Petr Viktorin wrote: On 06/17/2014 02:15 PM, Tomas Babej wrote: On 06/17/2014 12:03 PM, Timo Aaltonen wrote: On 17.06.2014 11:16, Martin Kosek wrote: Attached is a new version of patch 226, and a new patch 228, which moves the paths from installers to the paths module. In patch 226, there's another certificated typo in remove_ca_cert_from_systemwide_ca_store I greped the repository, and I do not see many paths lurking around any more, there are only some in the error messages (as these can't be reliably replaced automatically, and will need some manual love). If you see any forgotten paths, which should be added to the module, let me know. Well, since you asked... install/tools/ipa-upgradeconfig:236: ipautil.run([paths.PKI_SETUP_PROXY, '-pki_instance_root=/var/lib' ipaserver/install/cainstance.py:1330: -pki_instance_root=/var/lib, ipaserver/install/dsinstance.py:209:InstallLdifFile= /var/lib/dirsrv/boot.ldif ipaserver/install/dsinstance.py:210:inst_dir= /var/lib/dirsrv/scripts-$SERVERID ipaserver/install/ipa_backup.py:464: '--exclude=/var/lib/ipa/backup', ipatests/test_integration/tasks.py:451:host.run_command(find /var/lib/sss/db -name '*.ldb' | install/tools/ipa-replica-conncheck:403: /usr/sbin/ipa-replica-conncheck + install/tools/ipa-replica-conncheck:414: print_info(/usr/sbin/ipa-replica-conncheck + .join(remote_check_opts)) ipapython/ipautil.py:296:env[PATH] = /bin:/sbin:/usr/kerberos/bin:/usr/kerberos/sbin:/usr/bin:/usr/sbin ipaserver/install/cainstance.py:88:ConfigFile = /usr/share/pki/ca/conf/database.ldif ipaserver/install/bindinstance.py:829: ipautil.run(['/usr/libexec/generate-rndc-key.sh']) /me will think twice about teasing nex time. This are paths requiring manual changes in one way or the other and as such cannot be handled by my tool. Let's not stall the patcheset on this. We can fix these (and surely there are other) as we go along. I guess it'll be a while before we catch them all, but now it's at least clear where these paths should be, so anyone porting to another distro can send patches (or tickets) upstream. I see another duplicate: SSS_KRB5_INCLUDE_D = /var/lib/sss/pubconf/krb5.include.d SSSD_PUBCONF_KRB5_INCLUDE_D_DIR = /var/lib/sss/pubconf/krb5.include.d/ Could you just pick one instead? Would ipa_backup.py break if it had a trailing slash here? Yes. I verified it produces the same result with or without trailing slash, fixed. In ipa-client-install, if you set: NSSWITCH_CONF = paths.NSSWITCH_CONF then you should only use one of those later. (Preferably paths.*, to get rid of the redundant constants.) Perhaps this is for another patch that would clean up all the cases where these trivial module variables are used. I agree. Fixed this occurence. Fixed all mentioned issues. I also attached a patch 230, which removes the base Authconfig class. Attaching one additional patch, which removes unnecessary build warnings. 226, 230, 231 look good Attaching whole updated patchset. Attaching one more patch which should fix broken CI tests. -- Tomas Babej Associate Software Engineer | Red Hat | Identity Management RHCE | Brno Site | IRC: tbabej | freeipa.org ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel -- Tomas Babej Associate Software Engineer | Red Hat | Identity Management RHCE | Brno Site | IRC: tbabej | freeipa.org ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel Self-NACK - It seems I omitted one occurence of NSSWITCH_CONF in ipa-client-install, fixed now. Attaching the whole patchset for your convenience. -- Tomas Babej Associate Software Engineer | Red Hat | Identity Management RHCE | Brno Site | IRC: tbabej | freeipa.org Attaching a correct patchset this time. -- Tomas Babej Associate Software Engineer | Red Hat | Identity Management RHCE | Brno Site | IRC: tbabej | freeipa.org From 9c402cb6d6e8abf59284e6a524114253024059b3 Mon Sep 17 00:00:00 2001 From: Tomas Babej tba...@redhat.com Date: Thu, 19 Jun 2014 15:09:37 +0200 Subject: [PATCH] ipaplatform: Fix build warnings The newly created ipaplatform subdirectories base and fedora were mentioned multiple times in the specfile, which produced build warnings. Part of: https://fedorahosted.org/freeipa/ticket/4052 --- freeipa.spec.in | 6 +- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/freeipa.spec.in b/freeipa.spec.in index
Re: [Freeipa-devel] [PATCH] 659-666 Support of password reset with OTP
On 6/20/2014 10:18 AM, Petr Vobornik wrote: On 11.6.2014 15:19, Petr Vobornik wrote: Patch set contains both API/server and Web UI parts. [PATCH] 659 ldap2: add otp support to modify_password [PATCH] 660 rpcserver: add otp support to change_password handler [PATCH] 661 ipa-passwd: add OTP support [PATCH] 662 webui: support password change with OTP in login screen [PATCH] 663 webui: placeholder attribute support in textbox and textarea [PATCH] 664 webui: add placeholders to login screen [PATCH] 665 webui: rebase user password dialog on password dialog and add otp support [PATCH] 666 webui: support otp in reset_password.html https://fedorahosted.org/freeipa/ticket/4262 attaching rebased patches (mainly because of VERSION conflict) ACK. Possible improvements (some of which are already discussed on IRC): 1. The clock interval field in the Add OTP Token dialog could be disabled for HOTP. 2. The clock interval and counter fields (and probably some other fields too) in the OTP Token details page could be hidden depending on the token type. 3. The Add OTP Token dialog could provide more descriptive token types: time-based or counter-based token instead of just TOTP or HOTP. 4. The OTP Token details page could show the token type (I suppose the model may not be descriptive enough). 5. It would be nice to have a link/button to add OTP Token from the user details page with the owner already set to the user. 6. The clock interval should have a unit of measurements (i.e. seconds). 7. When logging in with an expired password, the user will be asked to reset a password and enter an OTP. Although OTP means one-time password, some users could be confusing it with the OTP he/she just entered in the previous page. It would be nicer to say New OTP or add an explanation Wait for a new OTP to make sure the user enters a new OTP. 8. In the User authentication types field it might be better to say password + OTP instead of just otp. The checkbox value can remain otp. 9. The User authentication types is a bit confusing because if none are selected it doesn't mean that no authentication is allowed, but it means it's unset and it will use the global setting. The UI probably should provide a separate radio button to select Use global setting or show the effective setting next to it. 10. The Default user authentication types in the global setting is a bit confusing because by default nothing is selected but the actual default is supposedly not empty. 11. Ideally the password reset page/dialog should indicate whether the old password and the OTP are required based on the actual authentication type available to the user. 12. Ideally there should be a way to display the QR code of an existing OTP token. 13. The UI could also provide a link to download the OTP app or a list of supported apps. -- Endi S. Dewata ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES 187-201] Improvements and coverage for sudorule plugin
On 06/25/2014 06:23 PM, Tomas Babej wrote:
On 06/25/2014 05:46 PM, Petr Viktorin wrote:
On 06/25/2014 04:28 PM, Tomas Babej wrote:
On 06/18/2014 09:54 AM, Petr Viktorin wrote:
On 06/17/2014 12:25 PM, Tomas Babej wrote:
On 05/26/2014 06:20 PM, Petr Viktorin wrote:
On 05/20/2014 06:15 PM, Tomas Babej wrote:
Hi,
the following set of patches fixes:
https://fedorahosted.org/freeipa/ticket/4274
https://fedorahosted.org/freeipa/ticket/4263
https://fedorahosted.org/freeipa/ticket/4324
https://fedorahosted.org/freeipa/ticket/4340
https://fedorahosted.org/freeipa/ticket/4341
and additional minor issues.
The improvemed CI test coverage for the sudorule plugin is added
as a
bonus.
You've dropped most of the long commit messages and ticket URLs. Why?
Sorry about that.. fixed!
0187: OK
0188 - sudorule: Allow using hostmasks for setting allowed hosts
If I run sudorule-add-host / sudorule-remove-host with a hostmask, but
not host/hostgroup, I get prompted for host and hostgroup. I don't
think that's the intended behavior.
This problem is beyond this patchset. Observe that same thing happens
with ipa group-add-member --external.
I was afraid it wouldn't bee that easy.
I'm not sure if there's a ticket for this though.
There is now. https://fedorahosted.org/freeipa/ticket/4400
0189: OK
0190: OK
0191: OK
0192: OK
0193 sudorule: Make sure all the relevant attributes are checked when
setting category to ALL
You're missing the `_` for the hostcategory error message.
Did you think about using something like _(%s cannot be set to 'all'
while there are %s)?
Fixed. Initially, I changed the message as you suggested, but then I
realized, that this might pose a problem for translations that do not
follow the word order in the sentence as it is defined in English
language.
Right, sorry for the incorrect example. You can use named
substitutions for that:
_(can't %(action)s while %(state)s) % {'action': 'move',
'state': 'asleep'}
The placeholder syntax is %(xyz)s, not %{xyz}.
(Have you been using format() too much?)
That's some detective insight! Thanks for catching that.
One more thing - the function is only called once, could you move it
to the for loop?
Well, I meant put the function's body in the for loop, getting rid of
the function. But that's just a nitpick.
No, you're right, the function's redundant after all the refactoring
that happened. Fixed.
0194: OK
0195: OK
0196-0198: OK
0199-0201: OK
0225:
Looks good. Could you also document the arguments return value in
*_external_post_callback docstrings?
I did. The updated patchset attached.
Thanks!
ACK, pushed to master: af4518b72882f88a01de0e5c23d423898ba894b4
--
Petr³
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES 202-222] Ipaplatform refactoring
On 06/25/2014 07:16 PM, Tomas Babej wrote: On 06/25/2014 04:59 PM, Tomas Babej wrote: On 06/25/2014 04:13 PM, Tomas Babej wrote: On 06/25/2014 04:01 PM, Tomas Babej wrote: On 06/25/2014 10:48 AM, Petr Viktorin wrote: On 06/19/2014 03:52 PM, Tomas Babej wrote: On 06/19/2014 12:52 PM, Tomas Babej wrote: On 06/18/2014 10:52 AM, Petr Viktorin wrote: On 06/17/2014 02:15 PM, Tomas Babej wrote: On 06/17/2014 12:03 PM, Timo Aaltonen wrote: On 17.06.2014 11:16, Martin Kosek wrote: Attached is a new version of patch 226, and a new patch 228, which moves the paths from installers to the paths module. In patch 226, there's another certificated typo in remove_ca_cert_from_systemwide_ca_store I greped the repository, and I do not see many paths lurking around any more, there are only some in the error messages (as these can't be reliably replaced automatically, and will need some manual love). If you see any forgotten paths, which should be added to the module, let me know. Well, since you asked... install/tools/ipa-upgradeconfig:236: ipautil.run([paths.PKI_SETUP_PROXY, '-pki_instance_root=/var/lib' ipaserver/install/cainstance.py:1330: -pki_instance_root=/var/lib, ipaserver/install/dsinstance.py:209:InstallLdifFile= /var/lib/dirsrv/boot.ldif ipaserver/install/dsinstance.py:210:inst_dir= /var/lib/dirsrv/scripts-$SERVERID ipaserver/install/ipa_backup.py:464: '--exclude=/var/lib/ipa/backup', ipatests/test_integration/tasks.py:451: host.run_command(find /var/lib/sss/db -name '*.ldb' | install/tools/ipa-replica-conncheck:403: /usr/sbin/ipa-replica-conncheck + install/tools/ipa-replica-conncheck:414: print_info(/usr/sbin/ipa-replica-conncheck + .join(remote_check_opts)) ipapython/ipautil.py:296:env[PATH] = /bin:/sbin:/usr/kerberos/bin:/usr/kerberos/sbin:/usr/bin:/usr/sbin ipaserver/install/cainstance.py:88:ConfigFile = /usr/share/pki/ca/conf/database.ldif ipaserver/install/bindinstance.py:829: ipautil.run(['/usr/libexec/generate-rndc-key.sh']) /me will think twice about teasing nex time. This are paths requiring manual changes in one way or the other and as such cannot be handled by my tool. Let's not stall the patcheset on this. We can fix these (and surely there are other) as we go along. OK, not a reason to hold the patch back. But, the fact that the tool can't handle them doesn't make them less important. Let's keep the ticket open, or open a new one. I guess it'll be a while before we catch them all, but now it's at least clear where these paths should be, so anyone porting to another distro can send patches (or tickets) upstream. I see another duplicate: SSS_KRB5_INCLUDE_D = /var/lib/sss/pubconf/krb5.include.d SSSD_PUBCONF_KRB5_INCLUDE_D_DIR = /var/lib/sss/pubconf/krb5.include.d/ Could you just pick one instead? Would ipa_backup.py break if it had a trailing slash here? Yes. I verified it produces the same result with or without trailing slash, fixed. In ipa-client-install, if you set: NSSWITCH_CONF = paths.NSSWITCH_CONF then you should only use one of those later. (Preferably paths.*, to get rid of the redundant constants.) Perhaps this is for another patch that would clean up all the cases where these trivial module variables are used. I agree. Fixed this occurence. I've opened an easyfix ticket for the rest: https://fedorahosted.org/freeipa/ticket/4399 Fixed all mentioned issues. I also attached a patch 230, which removes the base Authconfig class. Attaching one additional patch, which removes unnecessary build warnings. 226, 230, 231 look good Attaching whole updated patchset. Attaching one more patch which should fix broken CI tests. Self-NACK - It seems I omitted one occurence of NSSWITCH_CONF in ipa-client-install, fixed now. Attaching the whole patchset for your convenience. -- Attaching a correct patchset this time. I hate to break it to you, but... you sent the wrong patch 228 :( They're pretty independent, and there are fixes for failing tests, so I went ahead with the other ones. 226, 230, 231, 235: ACK, pushed to master: c8511d3b3baa389069156bf9991a9f4c7d64cf4a -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0058] Add the otptoken-add-yubikey command
On Wed, 2014-06-25 at 09:53 -0400, Nathaniel McCallum wrote:
On Wed, 2014-06-25 at 13:35 +0300, Alexander Bokovoy wrote:
On Mon, 23 Jun 2014, Nathaniel McCallum wrote:
On Mon, 2014-06-23 at 10:29 +0300, Alexander Bokovoy wrote:
On Fri, 20 Jun 2014, Nathaniel McCallum wrote:
On Thu, 2014-06-19 at 16:30 -0400, Nathaniel McCallum wrote:
This command behaves almost exactly like otptoken-add except:
1. The new token data is written directly to a YubiKey
2. The vendor/model/serial fields are populated from the YubiKey
=== NOTE ===
1. This patch depends on the new Fedora package: python-yubico. If you
would like to help with the package review, please assign yourself
here:
https://bugzilla.redhat.com/show_bug.cgi?id=334
New version of the patch. This one works (yay!).
1. Because of the dependency on python-yubico, is this feature something
we want in core FreeIPA? As a subpackage? Separate project altogether?
The only dependency for python-yubico is pyusb.
I'd prefer to have it integrated but have a separate dummy subpackage
that pulls in all required dependencies, like, freeipa-tools-yubico.
Instead
of failing when 'ipa otptoken-add-yubikey' is called, please wrap the
python-yubico import into a code that allows reporting a message back to
the user advising to install the package.
Who is a supposed user for this command? IPA command line interface isn't
usually available on enrolled machines even though underlying Python
modules are all there. Are we talking about admins or just users?
As discussed on IRC, we are currently hard-coding lots of optional
dependencies. And breaking this apart into subpackages can be solved at
a later point. YubiKey is also a unique case: we don't expect to be
adding many more plugins like this.
For these reasons, I have kept this as a hard dependency. To ease this
transition, I have added python-yubico to F20 and EL6. You can help with
the update review here:
https://admin.fedoraproject.org/updates/python-yubico-1.2.1-3.fc20
https://admin.fedoraproject.org/updates/python-yubico-1.2.1-3.el6
3. This code currently emits a warning from the call to otptoken-add:
WARNING: API Version number was not sent, forward compatibility not
guaranteed. Assuming server's API version, 2.89
How do I fix this?
Do not filter 'version' field in options in execute().
I opted to not filter out version rather than hard-code it (pviktori's
suggestion). This is based on the fact that otptoken-add-yubikey is
tightly integrated with otptoken-add (even using some of the class
attributes from otptoken).
4. I am not sure why I have to delete the summary and value keys from
the return dictionary. It would be nice to display this summary message
just like otptoken-add.
I still need help on this.
5. Am I doing the ipatoken(vendor|model|serial) options correctly? These
aren't user settable, but we need to pass them from the yubikey
(client-side) to the server.
This is no longer needed since I am doing everything in forward().
However, listing these three as output params causes them to appear
before the token's ID. I don't think this is the right way to output
these. But this seems to me a framework issue.
6. I'm not sure my use of assert or ValueError are correct. What should
I do here?
Still need help here.
Fixed this part.
7. Considering that this is just a specialized invocation of
otptoken-add, can't we do this all on the client-side? This is why I had
originally used frontend.Local rather than frontend.Command.
You don't need to implement execute then, only forward, where you'll
forward your call to the server under otptoken_add name.
Typically in #forward we call super's forward but that is because we
in Command.forward() we simply forward the command to the remote
backend,
using the self.name. In your case we shouldn't really have a separate
command on the server under the same name, so you'll need to avoid
calling
So, it should look like this:
def forward(self, *args, **kw):
perform yubikey initialization
filter out kw and args, if needed
return self.Backend.rpcclient.forward('otptoken_add', *args,
**kw)
See service_show implementation for an example.
Fixed.
I'm attaching few fixups to the patch that make it proper reporting for
non-Yubikey case and also properly update VERSION.
Provisional ACK.
Merged.
This patch includes everything above and fixes the missing ./makeapi
run.
Nathaniel
From 3f877142e99c12050def9f56c650eb474320bfff Mon Sep 17 00:00:00 2001
From: Nathaniel McCallum npmccal...@redhat.com
Date: Thu, 19 Jun 2014 12:28:32 -0400
Subject: [PATCH] Add the otptoken-add-yubikey command
This command behaves almost exactly like otptoken-add except:
1. The new token data
Re: [Freeipa-devel] [PATCH 0055] Add /session/token_sync POST support
On Wed, 2014-06-25 at 13:21 +0300, Alexander Bokovoy wrote: On Tue, 24 Jun 2014, Nathaniel McCallum wrote: On Mon, 2014-06-02 at 23:07 -0400, Nathaniel McCallum wrote: This HTTP call takes the following parameters: * user * password * first_code * second_code * token (optional) Using this information, the server will perform token synchronization. If the token is not specified, all tokens will be searched for synchronization. Otherwise, only the token specified will be searched. This patch depends on my patch #0054. Attached is a new revision. This version should force an update to /etc/httpd/conf.d/ipa.conf on update. It is also rebased on master. ACK with condition that you apply attached fixups. Since token that is passed by 'ipa otptoken-sync' command is not a full DN, we need to support both cases, when DN and just a name is passed. Attached patch fixes this. Applied. Nathaniel From a044f461b5233bd93417f7fd7acdd8e158a67fb8 Mon Sep 17 00:00:00 2001 From: Nathaniel McCallum npmccal...@redhat.com Date: Wed, 28 May 2014 11:38:40 -0400 Subject: [PATCH] Add /session/token_sync POST support This HTTP call takes the following parameters: * user * password * first_code * second_code * token (optional) Using this information, the server will perform token synchronization. If the token is not specified, all tokens will be searched for synchronization. Otherwise, only the token specified will be searched. --- install/conf/ipa.conf | 8 ++- ipaserver/plugins/ldap2.py | 14 -- ipaserver/plugins/xmlserver.py | 3 +- ipaserver/rpcserver.py | 110 +++-- 4 files changed, 126 insertions(+), 9 deletions(-) diff --git a/install/conf/ipa.conf b/install/conf/ipa.conf index f4dac9827bd0251463aade5854fd522e4306e468..7eede73efc559967925d2bbfeee54e1e2efd3e21 100644 --- a/install/conf/ipa.conf +++ b/install/conf/ipa.conf @@ -1,5 +1,5 @@ # -# VERSION 15 - DO NOT REMOVE THIS LINE +# VERSION 16 - DO NOT REMOVE THIS LINE # # This file may be overwritten on upgrades. # @@ -103,6 +103,12 @@ KrbConstrainedDelegationLock ipa Allow from all /Location +Location /ipa/session/sync_token + Satisfy Any + Order Deny,Allow + Allow from all +/Location + # This is where we redirect on failed auth Alias /ipa/errors /usr/share/ipa/html diff --git a/ipaserver/plugins/ldap2.py b/ipaserver/plugins/ldap2.py index cfcec7c803215459cad2c08adae26a44099f2982..888f085b9f251bc933bc15c24a14b4107d4d4784 100644 --- a/ipaserver/plugins/ldap2.py +++ b/ipaserver/plugins/ldap2.py @@ -93,7 +93,7 @@ class ldap2(LDAPClient, CrudBackend): def create_connection(self, ccache=None, bind_dn=None, bind_pw='', tls_cacertfile=None, tls_certfile=None, tls_keyfile=None, -debug_level=0, autobind=False): +debug_level=0, autobind=False, serverctrls=None, clientctrls=None): Connect to LDAP server. @@ -151,16 +151,22 @@ class ldap2(LDAPClient, CrudBackend): context=krbV.default_context()).principal().name os.environ['KRB5CCNAME'] = ccache -conn.sasl_interactive_bind_s(None, SASL_GSSAPI) +conn.sasl_interactive_bind_s(None, SASL_GSSAPI, + serverctrls=serverctrls, + clientctrls=clientctrls) setattr(context, 'principal', principal) else: # no kerberos ccache, use simple bind or external sasl if autobind: pent = pwd.getpwuid(os.geteuid()) auth_tokens = _ldap.sasl.external(pent.pw_name) -conn.sasl_interactive_bind_s(None, auth_tokens) +conn.sasl_interactive_bind_s(None, auth_tokens, + serverctrls=serverctrls, + clientctrls=clientctrls) else: -conn.simple_bind_s(bind_dn, bind_pw) +conn.simple_bind_s(bind_dn, bind_pw, + serverctrls=serverctrls, + clientctrls=clientctrls) return conn diff --git a/ipaserver/plugins/xmlserver.py b/ipaserver/plugins/xmlserver.py index 8d96262cf52f04620aeb5223002f4794e18cc0de..7460ead69a12fbe1b4613908f62787f3d26a1cde 100644 --- a/ipaserver/plugins/xmlserver.py +++ b/ipaserver/plugins/xmlserver.py @@ -25,7 +25,7 @@ Loads WSGI server plugins. from ipalib import api if 'in_server' in api.env and api.env.in_server is True: -from ipaserver.rpcserver import wsgi_dispatch, xmlserver, jsonserver_kerb, jsonserver_session, login_kerberos, login_password, change_password, xmlserver_session +from ipaserver.rpcserver import wsgi_dispatch, xmlserver, jsonserver_kerb, jsonserver_session, login_kerberos, login_password,
Re: [Freeipa-devel] [PATCH 0056] Add otptoken-sync command
On Wed, 2014-06-25 at 13:18 +0300, Alexander Bokovoy wrote:
On Tue, 24 Jun 2014, Nathaniel McCallum wrote:
On Tue, 2014-06-03 at 09:18 -0400, Nathaniel McCallum wrote:
On Tue, 2014-06-03 at 10:27 +0200, Petr Vobornik wrote:
On 3.6.2014 05:08, Nathaniel McCallum wrote:
This command calls the token sync HTTP POST call in the server
providing
the CLI interface to synchronization.
https://fedorahosted.org/freeipa/ticket/4260
This patch depends on my patch #0055.
Build fails on validation. You forgot to update API.txt and also the
command misses __doc__.
(not a proper review)
Thanks, fixed.
Attached is a new revision which is rebased on master.
In addition it:
1. Moves user to a parameter and moves token to an argument. Doing it
this way both mirrors the existing otptoken APIs and sets us up for
future Kerberos based syncing where the username/password will be
optional.
2. Converts the token ID to a DN.
ACK.
Please do not commit this patch yet, we are not done with its
dependencies.
As discussed off list, we also needed to verify the certificate so that
passwords were not sent in the clear to a MITM. This has now been
implemented. VERSION is bumped and ./makeapi was run. This patch is also
rebased on top of my patch 0058 (which is already ACK'd), so 0058 needs
to be merged before this patch (0056).
Nathaniel
From c06a4146d10759937116eb6ffe7201636febb1ab Mon Sep 17 00:00:00 2001
From: Nathaniel McCallum npmccal...@redhat.com
Date: Mon, 2 Jun 2014 23:00:52 -0400
Subject: [PATCH] Add otptoken-sync command
This command calls the token sync HTTP POST call in the server providing
the CLI interface to synchronization.
https://fedorahosted.org/freeipa/ticket/4260
---
API.txt| 9
VERSION| 4 +-
ipalib/plugins/otptoken.py | 102 -
3 files changed, 112 insertions(+), 3 deletions(-)
diff --git a/API.txt b/API.txt
index 3c3b6447fec3c313c3038390ac7317533c530d8b..0924402764ca29711d8a9094c1e4f7dec461ab3c 100644
--- a/API.txt
+++ b/API.txt
@@ -2420,6 +2420,15 @@ option: Str('version?', exclude='webui')
output: Entry('result', type 'dict', Gettext('A dictionary representing an LDAP entry', domain='ipa', localedir=None))
output: Output('summary', (type 'unicode', type 'NoneType'), None)
output: PrimaryKey('value', None, None)
+command: otptoken_sync
+args: 1,5,1
+arg: Str('token?')
+option: Password('first_code', confirm=False)
+option: Password('password', confirm=False)
+option: Password('second_code', confirm=False)
+option: Str('user')
+option: Str('version?', exclude='webui')
+output: Output('result', None, None)
command: passwd
args: 3,1,3
arg: Str('principal', autofill=True, cli_name='user', primary_key=True)
diff --git a/VERSION b/VERSION
index f0c2db55658f3ab4cfafffc5735aa77b52bc9cc8..1d2e81688b9934baf1790c390452d733b9bed2e9 100644
--- a/VERSION
+++ b/VERSION
@@ -89,5 +89,5 @@ IPA_DATA_VERSION=2010061412
# #
IPA_API_VERSION_MAJOR=2
-IPA_API_VERSION_MINOR=94
-# Last change: npmaccallum - otptoken-add-yubikey
+IPA_API_VERSION_MINOR=95
+# Last change: npmaccallum - otptoken-sync
diff --git a/ipalib/plugins/otptoken.py b/ipalib/plugins/otptoken.py
index 7962af0035fb9ac00e68c4b642bb62aa82d498c2..46ad77a2c81842cad9085651b794fd7959d783f0 100644
--- a/ipalib/plugins/otptoken.py
+++ b/ipalib/plugins/otptoken.py
@@ -19,15 +19,22 @@
from ipalib.plugins.baseldap import DN, LDAPObject, LDAPAddMember, LDAPRemoveMember
from ipalib.plugins.baseldap import LDAPCreate, LDAPDelete, LDAPUpdate, LDAPSearch, LDAPRetrieve
-from ipalib import api, Int, Str, Bool, Flag, Bytes, IntEnum, StrEnum, _, ngettext
+from ipalib import api, Int, Str, Bool, Flag, Bytes, IntEnum, StrEnum, Password, _, ngettext
from ipalib.plugable import Registry
from ipalib.errors import PasswordMismatch, ConversionError, LastMemberError, NotFound
from ipalib.request import context
+from ipalib.frontend import Local
+
+from backports.ssl_match_hostname import match_hostname
import base64
import uuid
import urllib
+import urllib2
+import httplib
+import urlparse
import qrcode
import os
+import ssl
__doc__ = _(
OTP Tokens
@@ -383,3 +390,96 @@ class otptoken_remove_managedby(LDAPRemoveMember):
__doc__ = _('Remove hosts that can manage this host.')
member_attributes = ['managedby']
+
+class HTTPSConnection(httplib.HTTPConnection):
+Generates an SSL HTTP connection that performs hostname validation.
+
+ssl_kwargs = ssl.wrap_socket.func_code.co_varnames[1:ssl.wrap_socket.func_code.co_argcount]
+default_port = httplib.HTTPS_PORT
+
+def __init__(self, host, **kwargs):
+# Strip out arguments we want to pass to ssl.wrap_socket()
+self.__kwargs = {k: v for k, v in kwargs.items() if k in self.ssl_kwargs}
+for k in
Re: [Freeipa-devel] Design Review Keytab Retrieval
On Mon, 2014-06-23 at 17:24 -0400, Nathaniel McCallum wrote: On Mon, 2014-06-23 at 14:35 -0400, Simo Sorce wrote: - Original Message - - Original Message - Can you check if ipaProtectedOperation is in the aci attribute in the base tree object ? It should be there as excluded, and that should cause admin to not be able to retrieve keytabs. It was not. While running ipa-ldap-updater I got the following: InvalidSyntax: ACL Syntax Error(-5):(targetattr= \22ipaProtectedOperation;write_keys\22)(version 3.0; acl \22Admins are allowed to rekey any entity\22; allow(write) groupdn = \22ldap:///cn=admins: Invalid syntax. Uhmm I do not see anything obviously wrong with ACI instruction, it looks just like the one I replace, Ideas ? Do you have ipaProtectedOperation in the schema ? (I rebased patch 3 but will wait to send a patchset until we understand (and fix) why this is failing to update. Ok, apparently it was a quoting issue in the .update files, hopefully that's the only issue (I am at a conference today and do not have my test env. handy). The attached patches are rebased on the latest master. 0001: Line 555 has very wrong indentation. I don't see anything else wrong in the other patches. I've tested everything and it works as designed. I have CC'd everyone who was involved with review at any point on these patches. This serves as my public notice that I'd like to ACK the next round of patches. If anyone has anything else to add, please do it before tomorrow evening. Thanks! Nathaniel ACK Nathaniel ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] FYI: Cert for https://www.freeipa.org/ is invalid
I think it's kind of funny that the cert for: https://www.freeipa.org/ is invalid, particularly since this is a security product. In any case, feel free to forward to whoever maintains this in case someone thinks it matters. Cheers, James ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
