Re: [Freeipa-devel] Required descriptions
On 09/15/2014 05:37 PM, Petr Vobornik wrote: On 15.9.2014 17:21, Tomas Babej wrote: Hi folks, while developing parts of the upcoming views feature (http://www.freeipa.org/page/V4/Migrating_existing_environments_to_Trust), we stumbled upon the question of having descriptions required by the framework. There are arguments for the description being required, at least for overriding attributes of IPA users. However, nothing stops irresponsible admins from entering descriptions like 'foo'. There is related ticket in the Trac: https://fedorahosted.org/freeipa/ticket/4387 I'd like to avoid having this argument over again. Can we establish a guideline we wish to follow? Having tickets like #4387 and requiring descriptions in new features is too inconsistent for my taste :) we should either: 1.) Define a clear line - when it makes sense to require description and when not. 2.) Decide never to require description, since it is a non-enforcible requirement (nothing stops you from entering meaningless description). Description is a helper tool for users and it's not required for any functionality. Ideally user (company policy) should choose whether it should be required. We should only give recommendations, e.g., in documentation. Making it configurable seems like a lot of effort with little added value. I'm for #2. Btw, idview plugin is inconsistent by itself atm - overrides have it required but idview doesn't. my 2c +1, please don't require description unless it is really required for the actual functionality. As already discussed wrt #4387, we cannot force admins to enter something sensible in the description, so let us not punish those who do not want to document it and make adding views, SUDO rules, ... easier. Adding description to MUST part of objectclass is rather unprecedented move. Just for fun I listed all objectclasses in my DS that use description field and neither of them had it in MUST part. I could partially live with description being in MAY part of objectclass + the framework requiring it, but I do not think it is a good idea either. Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES 0114-0115, 0120-0121] DNS: allow to add root zone '.'
On 15/09/14 20:31, Martin Kosek wrote: On 09/15/2014 05:16 PM, Martin Basti wrote: On 15/09/14 17:10, Petr Spacek wrote: On 12.9.2014 15:19, Martin Basti wrote: On 03/09/14 12:45, Martin Basti wrote: On 03/09/14 12:27, Martin Kosek wrote: On 09/02/2014 05:46 PM, Petr Spacek wrote: On 25.8.2014 14:52, Martin Basti wrote: Patches attached. Ticket: https://fedorahosted.org/freeipa/ticket/4149 There is a bug in bind-dyndb-ldap (or worse in dirsrv), which cause the named service is stopped after deleting zone. Bug ticket: https://fedorahosted.org/bind-dyndb-ldap/ticket/138 Functional ACK, it works for me. It can be pushed if Python gurus are okay with the code. Is it safe to commit the change given that bind-dyndb-ldap still crash when . is removed? Wouldn't it break our CI tests? Maybe we should wait until fixed bind-dydnb-ldap is released. Hopefully it would be soon. Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel It will broke tests, don't push it until bind-dyndb-ldap is fixed. Currently I'm testing bind-dyndb-ldap related patch. Added patches 120 and 121, which are required by DNS to work correctly. Patches 120 and 121 add all DNS replicas to zone apex as NS, --name-server option doesn't add NS record, only changes the SOA MNAME attribute Original and new patches attached. NACK, unfortunately it doesn't work for me: # ipa dnszone-add tri.test. --name-server=ns.test. Administrator e-mail address [hostmaster.tri.test.]: ipa: WARNING: '--name-server' is used only for setting up the SOA MNAME record. To edit NS record(s) in zone apex, use command 'dnsrecord-mod [zone] @ --ns-rec=nameserver'. Zone name: tri.test. Active zone: TRUE Authoritative nameserver: ns.test. Administrator e-mail address: hostmaster.tri.test. SOA serial: 1410793406 SOA refresh: 3600 SOA retry: 900 SOA expire: 1209600 SOA minimum: 3600 BIND update policy: grant IPA.EXAMPLE krb5-self * A; grant IPA.EXAMPLE krb5-self * ; grant IPA.EXAMPLE krb5-self * SSHFP; Dynamic update: FALSE Allow query: any; Allow transfer: none; [root@vm-035 rpms]# ipa dnszone-show tri.test. --all --raw dn: idnsname=tri.test.,cn=dns,dc=ipa,dc=example idnsname: tri.test. idnszoneactive: TRUE idnssoamname: ns.test. idnssoarname: hostmaster.tri.test. idnssoaserial: 1410793408 idnssoarefresh: 3600 idnssoaretry: 900 idnssoaexpire: 1209600 idnssoaminimum: 3600 idnsallowquery: any; idnsallowtransfer: none; idnsAllowDynUpdate: FALSE idnsUpdatePolicy: grant IPA.EXAMPLE krb5-self * A; grant IPA.EXAMPLE krb5-self * ; grant IPA.EXAMPLE krb5-self * SSHFP; nsrecord: vm-035.idm.lab.eng.brq.redhat.com. objectClass: idnszone objectClass: top objectClass: idnsrecord [root@vm-035 rpms]# ipa dnsrecord-mod @ tri.test. --ns-rec=$(hostname). ipa: ERROR: tri.test.: DNS resource record not found NACKing NACK ipa dnsrecord-mod @ tri.test. --ns-rec=$(hostname). you switched order zone and record, it should be ipa dnsrecord-mod tri.test. @ --ns-rec=$(hostname). BTW, since we are so nicely breaking the dnszone-add interface, can we also get rid of always asking for Administrator e-mail address? # ipa dnszone-add tri.test. --name-server=ns.test. Administrator e-mail address [hostmaster.tri.test.]: ... Is there any risk in filling that with default as any other attribute? IMO it would simplify adding zones for one more redundant step. CCing Rob in case he knows some historical reasons why this is requested every time. Martin There is no risk, because ipa-replica-prepare do that with default values -- Martin Basti ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES 0114-0115, 0120-0121] DNS: allow to add root zone '.'
On 16/09/14 09:32, Martin Basti wrote: On 15/09/14 20:31, Martin Kosek wrote: On 09/15/2014 05:16 PM, Martin Basti wrote: On 15/09/14 17:10, Petr Spacek wrote: On 12.9.2014 15:19, Martin Basti wrote: On 03/09/14 12:45, Martin Basti wrote: On 03/09/14 12:27, Martin Kosek wrote: On 09/02/2014 05:46 PM, Petr Spacek wrote: On 25.8.2014 14:52, Martin Basti wrote: Patches attached. Ticket: https://fedorahosted.org/freeipa/ticket/4149 There is a bug in bind-dyndb-ldap (or worse in dirsrv), which cause the named service is stopped after deleting zone. Bug ticket: https://fedorahosted.org/bind-dyndb-ldap/ticket/138 Functional ACK, it works for me. It can be pushed if Python gurus are okay with the code. Is it safe to commit the change given that bind-dyndb-ldap still crash when . is removed? Wouldn't it break our CI tests? Maybe we should wait until fixed bind-dydnb-ldap is released. Hopefully it would be soon. Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel It will broke tests, don't push it until bind-dyndb-ldap is fixed. Currently I'm testing bind-dyndb-ldap related patch. Added patches 120 and 121, which are required by DNS to work correctly. Patches 120 and 121 add all DNS replicas to zone apex as NS, --name-server option doesn't add NS record, only changes the SOA MNAME attribute Original and new patches attached. NACK, unfortunately it doesn't work for me: # ipa dnszone-add tri.test. --name-server=ns.test. Administrator e-mail address [hostmaster.tri.test.]: ipa: WARNING: '--name-server' is used only for setting up the SOA MNAME record. To edit NS record(s) in zone apex, use command 'dnsrecord-mod [zone] @ --ns-rec=nameserver'. Zone name: tri.test. Active zone: TRUE Authoritative nameserver: ns.test. Administrator e-mail address: hostmaster.tri.test. SOA serial: 1410793406 SOA refresh: 3600 SOA retry: 900 SOA expire: 1209600 SOA minimum: 3600 BIND update policy: grant IPA.EXAMPLE krb5-self * A; grant IPA.EXAMPLE krb5-self * ; grant IPA.EXAMPLE krb5-self * SSHFP; Dynamic update: FALSE Allow query: any; Allow transfer: none; [root@vm-035 rpms]# ipa dnszone-show tri.test. --all --raw dn: idnsname=tri.test.,cn=dns,dc=ipa,dc=example idnsname: tri.test. idnszoneactive: TRUE idnssoamname: ns.test. idnssoarname: hostmaster.tri.test. idnssoaserial: 1410793408 idnssoarefresh: 3600 idnssoaretry: 900 idnssoaexpire: 1209600 idnssoaminimum: 3600 idnsallowquery: any; idnsallowtransfer: none; idnsAllowDynUpdate: FALSE idnsUpdatePolicy: grant IPA.EXAMPLE krb5-self * A; grant IPA.EXAMPLE krb5-self * ; grant IPA.EXAMPLE krb5-self * SSHFP; nsrecord: vm-035.idm.lab.eng.brq.redhat.com. objectClass: idnszone objectClass: top objectClass: idnsrecord [root@vm-035 rpms]# ipa dnsrecord-mod @ tri.test. --ns-rec=$(hostname). ipa: ERROR: tri.test.: DNS resource record not found NACKing NACK ipa dnsrecord-mod @ tri.test. --ns-rec=$(hostname). you switched order zone and record, it should be ipa dnsrecord-mod tri.test. @ --ns-rec=$(hostname). BTW, since we are so nicely breaking the dnszone-add interface, can we also get rid of always asking for Administrator e-mail address? # ipa dnszone-add tri.test. --name-server=ns.test. Administrator e-mail address [hostmaster.tri.test.]: ... Is there any risk in filling that with default as any other attribute? IMO it would simplify adding zones for one more redundant step. CCing Rob in case he knows some historical reasons why this is requested every time. Martin There is no risk, because ipa-replica-prepare do that with default values However, this will not work with root zone ., and I'm not sure how often an admin email is used. I think whois is better utility to get contact email. Also RIPE-203 [1] recommends to use 'hostmaster' alias. [1] http://www.ripe.net/ripe/docs/ripe-203 -- Martin Basti ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES 0114-0115, 0120-0121] DNS: allow to add root zone '.'
On 09/16/2014 09:57 AM, Martin Basti wrote: On 16/09/14 09:32, Martin Basti wrote: On 15/09/14 20:31, Martin Kosek wrote: On 09/15/2014 05:16 PM, Martin Basti wrote: On 15/09/14 17:10, Petr Spacek wrote: On 12.9.2014 15:19, Martin Basti wrote: On 03/09/14 12:45, Martin Basti wrote: On 03/09/14 12:27, Martin Kosek wrote: On 09/02/2014 05:46 PM, Petr Spacek wrote: On 25.8.2014 14:52, Martin Basti wrote: Patches attached. Ticket: https://fedorahosted.org/freeipa/ticket/4149 There is a bug in bind-dyndb-ldap (or worse in dirsrv), which cause the named service is stopped after deleting zone. Bug ticket: https://fedorahosted.org/bind-dyndb-ldap/ticket/138 Functional ACK, it works for me. It can be pushed if Python gurus are okay with the code. Is it safe to commit the change given that bind-dyndb-ldap still crash when . is removed? Wouldn't it break our CI tests? Maybe we should wait until fixed bind-dydnb-ldap is released. Hopefully it would be soon. Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel It will broke tests, don't push it until bind-dyndb-ldap is fixed. Currently I'm testing bind-dyndb-ldap related patch. Added patches 120 and 121, which are required by DNS to work correctly. Patches 120 and 121 add all DNS replicas to zone apex as NS, --name-server option doesn't add NS record, only changes the SOA MNAME attribute Original and new patches attached. NACK, unfortunately it doesn't work for me: # ipa dnszone-add tri.test. --name-server=ns.test. Administrator e-mail address [hostmaster.tri.test.]: ipa: WARNING: '--name-server' is used only for setting up the SOA MNAME record. To edit NS record(s) in zone apex, use command 'dnsrecord-mod [zone] @ --ns-rec=nameserver'. Zone name: tri.test. Active zone: TRUE Authoritative nameserver: ns.test. Administrator e-mail address: hostmaster.tri.test. SOA serial: 1410793406 SOA refresh: 3600 SOA retry: 900 SOA expire: 1209600 SOA minimum: 3600 BIND update policy: grant IPA.EXAMPLE krb5-self * A; grant IPA.EXAMPLE krb5-self * ; grant IPA.EXAMPLE krb5-self * SSHFP; Dynamic update: FALSE Allow query: any; Allow transfer: none; [root@vm-035 rpms]# ipa dnszone-show tri.test. --all --raw dn: idnsname=tri.test.,cn=dns,dc=ipa,dc=example idnsname: tri.test. idnszoneactive: TRUE idnssoamname: ns.test. idnssoarname: hostmaster.tri.test. idnssoaserial: 1410793408 idnssoarefresh: 3600 idnssoaretry: 900 idnssoaexpire: 1209600 idnssoaminimum: 3600 idnsallowquery: any; idnsallowtransfer: none; idnsAllowDynUpdate: FALSE idnsUpdatePolicy: grant IPA.EXAMPLE krb5-self * A; grant IPA.EXAMPLE krb5-self * ; grant IPA.EXAMPLE krb5-self * SSHFP; nsrecord: vm-035.idm.lab.eng.brq.redhat.com. objectClass: idnszone objectClass: top objectClass: idnsrecord [root@vm-035 rpms]# ipa dnsrecord-mod @ tri.test. --ns-rec=$(hostname). ipa: ERROR: tri.test.: DNS resource record not found NACKing NACK ipa dnsrecord-mod @ tri.test. --ns-rec=$(hostname). you switched order zone and record, it should be ipa dnsrecord-mod tri.test. @ --ns-rec=$(hostname). BTW, since we are so nicely breaking the dnszone-add interface, can we also get rid of always asking for Administrator e-mail address? # ipa dnszone-add tri.test. --name-server=ns.test. Administrator e-mail address [hostmaster.tri.test.]: ... Is there any risk in filling that with default as any other attribute? IMO it would simplify adding zones for one more redundant step. CCing Rob in case he knows some historical reasons why this is requested every time. Martin There is no risk, because ipa-replica-prepare do that with default values Then let us do this, as we are already simplifying the dnszone-add command. However, this will not work with root zone ., and I'm not sure how often an admin email is used. I think whois is better utility to get contact email. Also RIPE-203 [1] recommends to use 'hostmaster' alias. [1] http://www.ripe.net/ripe/docs/ripe-203 DNS zone . is quite an exception, you are not adding that zone every day. So I would not keep asking for admin mail just for this one. You can add a interactive prompt callback to ask in this case and otherwise just use the default - up to you. As for the mail alias, this can be an RFE. Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES 0114-0115, 0120-0121] DNS: allow to add root zone '.'
On 16.9.2014 10:09, Martin Kosek wrote: On 09/16/2014 09:57 AM, Martin Basti wrote: On 16/09/14 09:32, Martin Basti wrote: On 15/09/14 20:31, Martin Kosek wrote: On 09/15/2014 05:16 PM, Martin Basti wrote: On 15/09/14 17:10, Petr Spacek wrote: On 12.9.2014 15:19, Martin Basti wrote: On 03/09/14 12:45, Martin Basti wrote: On 03/09/14 12:27, Martin Kosek wrote: On 09/02/2014 05:46 PM, Petr Spacek wrote: On 25.8.2014 14:52, Martin Basti wrote: Patches attached. Ticket: https://fedorahosted.org/freeipa/ticket/4149 There is a bug in bind-dyndb-ldap (or worse in dirsrv), which cause the named service is stopped after deleting zone. Bug ticket: https://fedorahosted.org/bind-dyndb-ldap/ticket/138 Functional ACK, it works for me. It can be pushed if Python gurus are okay with the code. Is it safe to commit the change given that bind-dyndb-ldap still crash when . is removed? Wouldn't it break our CI tests? Maybe we should wait until fixed bind-dydnb-ldap is released. Hopefully it would be soon. Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel It will broke tests, don't push it until bind-dyndb-ldap is fixed. Currently I'm testing bind-dyndb-ldap related patch. Added patches 120 and 121, which are required by DNS to work correctly. Patches 120 and 121 add all DNS replicas to zone apex as NS, --name-server option doesn't add NS record, only changes the SOA MNAME attribute Original and new patches attached. NACK, unfortunately it doesn't work for me: # ipa dnszone-add tri.test. --name-server=ns.test. Administrator e-mail address [hostmaster.tri.test.]: ipa: WARNING: '--name-server' is used only for setting up the SOA MNAME record. To edit NS record(s) in zone apex, use command 'dnsrecord-mod [zone] @ --ns-rec=nameserver'. Zone name: tri.test. Active zone: TRUE Authoritative nameserver: ns.test. Administrator e-mail address: hostmaster.tri.test. SOA serial: 1410793406 SOA refresh: 3600 SOA retry: 900 SOA expire: 1209600 SOA minimum: 3600 BIND update policy: grant IPA.EXAMPLE krb5-self * A; grant IPA.EXAMPLE krb5-self * ; grant IPA.EXAMPLE krb5-self * SSHFP; Dynamic update: FALSE Allow query: any; Allow transfer: none; [root@vm-035 rpms]# ipa dnszone-show tri.test. --all --raw dn: idnsname=tri.test.,cn=dns,dc=ipa,dc=example idnsname: tri.test. idnszoneactive: TRUE idnssoamname: ns.test. idnssoarname: hostmaster.tri.test. idnssoaserial: 1410793408 idnssoarefresh: 3600 idnssoaretry: 900 idnssoaexpire: 1209600 idnssoaminimum: 3600 idnsallowquery: any; idnsallowtransfer: none; idnsAllowDynUpdate: FALSE idnsUpdatePolicy: grant IPA.EXAMPLE krb5-self * A; grant IPA.EXAMPLE krb5-self * ; grant IPA.EXAMPLE krb5-self * SSHFP; nsrecord: vm-035.idm.lab.eng.brq.redhat.com. objectClass: idnszone objectClass: top objectClass: idnsrecord [root@vm-035 rpms]# ipa dnsrecord-mod @ tri.test. --ns-rec=$(hostname). ipa: ERROR: tri.test.: DNS resource record not found NACKing NACK ipa dnsrecord-mod @ tri.test. --ns-rec=$(hostname). you switched order zone and record, it should be ipa dnsrecord-mod tri.test. @ --ns-rec=$(hostname). BTW, since we are so nicely breaking the dnszone-add interface, can we also get rid of always asking for Administrator e-mail address? # ipa dnszone-add tri.test. --name-server=ns.test. Administrator e-mail address [hostmaster.tri.test.]: ... Is there any risk in filling that with default as any other attribute? IMO it would simplify adding zones for one more redundant step. CCing Rob in case he knows some historical reasons why this is requested every time. Martin There is no risk, because ipa-replica-prepare do that with default values Then let us do this, as we are already simplifying the dnszone-add command. However, this will not work with root zone ., and I'm not sure how often an admin email is used. I think whois is better utility to get contact email. Also RIPE-203 [1] recommends to use 'hostmaster' alias. [1] http://www.ripe.net/ripe/docs/ripe-203 This will likely generate tons of invalid e-mail addresses which is somehow unfortunate. Please keep in mind that: 1) E-mail hostmaster@ipa.domain.example. will be useful only if ipa.domain.example. has MX record or at least A/ record (which is usually not the case for domains). 2) WHOIS is not useful for internal domains which is the main deployment scenario for IPA, right? DNS zone . is quite an exception, you are not adding that zone every day. So I would not keep asking for admin mail just for this one. You can add a interactive prompt callback to ask in this case and otherwise just use the default - up to you. As for the mail alias, this can be an RFE. It would be nice to have some IPA-global default like 'DNS administrator e-mail address' and to use this value for all DNS zones by
Re: [Freeipa-devel] [PATCHES 0114-0115, 0120-0121] DNS: allow to add root zone '.'
On 09/16/2014 10:30 AM, Martin Basti wrote: On 16/09/14 10:29, Petr Spacek wrote: On 16.9.2014 10:09, Martin Kosek wrote: On 09/16/2014 09:57 AM, Martin Basti wrote: On 16/09/14 09:32, Martin Basti wrote: On 15/09/14 20:31, Martin Kosek wrote: On 09/15/2014 05:16 PM, Martin Basti wrote: On 15/09/14 17:10, Petr Spacek wrote: On 12.9.2014 15:19, Martin Basti wrote: On 03/09/14 12:45, Martin Basti wrote: On 03/09/14 12:27, Martin Kosek wrote: On 09/02/2014 05:46 PM, Petr Spacek wrote: On 25.8.2014 14:52, Martin Basti wrote: Patches attached. Ticket: https://fedorahosted.org/freeipa/ticket/4149 There is a bug in bind-dyndb-ldap (or worse in dirsrv), which cause the named service is stopped after deleting zone. Bug ticket: https://fedorahosted.org/bind-dyndb-ldap/ticket/138 Functional ACK, it works for me. It can be pushed if Python gurus are okay with the code. Is it safe to commit the change given that bind-dyndb-ldap still crash when . is removed? Wouldn't it break our CI tests? Maybe we should wait until fixed bind-dydnb-ldap is released. Hopefully it would be soon. Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel It will broke tests, don't push it until bind-dyndb-ldap is fixed. Currently I'm testing bind-dyndb-ldap related patch. Added patches 120 and 121, which are required by DNS to work correctly. Patches 120 and 121 add all DNS replicas to zone apex as NS, --name-server option doesn't add NS record, only changes the SOA MNAME attribute Original and new patches attached. NACK, unfortunately it doesn't work for me: # ipa dnszone-add tri.test. --name-server=ns.test. Administrator e-mail address [hostmaster.tri.test.]: ipa: WARNING: '--name-server' is used only for setting up the SOA MNAME record. To edit NS record(s) in zone apex, use command 'dnsrecord-mod [zone] @ --ns-rec=nameserver'. Zone name: tri.test. Active zone: TRUE Authoritative nameserver: ns.test. Administrator e-mail address: hostmaster.tri.test. SOA serial: 1410793406 SOA refresh: 3600 SOA retry: 900 SOA expire: 1209600 SOA minimum: 3600 BIND update policy: grant IPA.EXAMPLE krb5-self * A; grant IPA.EXAMPLE krb5-self * ; grant IPA.EXAMPLE krb5-self * SSHFP; Dynamic update: FALSE Allow query: any; Allow transfer: none; [root@vm-035 rpms]# ipa dnszone-show tri.test. --all --raw dn: idnsname=tri.test.,cn=dns,dc=ipa,dc=example idnsname: tri.test. idnszoneactive: TRUE idnssoamname: ns.test. idnssoarname: hostmaster.tri.test. idnssoaserial: 1410793408 idnssoarefresh: 3600 idnssoaretry: 900 idnssoaexpire: 1209600 idnssoaminimum: 3600 idnsallowquery: any; idnsallowtransfer: none; idnsAllowDynUpdate: FALSE idnsUpdatePolicy: grant IPA.EXAMPLE krb5-self * A; grant IPA.EXAMPLE krb5-self * ; grant IPA.EXAMPLE krb5-self * SSHFP; nsrecord: vm-035.idm.lab.eng.brq.redhat.com. objectClass: idnszone objectClass: top objectClass: idnsrecord [root@vm-035 rpms]# ipa dnsrecord-mod @ tri.test. --ns-rec=$(hostname). ipa: ERROR: tri.test.: DNS resource record not found NACKing NACK ipa dnsrecord-mod @ tri.test. --ns-rec=$(hostname). you switched order zone and record, it should be ipa dnsrecord-mod tri.test. @ --ns-rec=$(hostname). BTW, since we are so nicely breaking the dnszone-add interface, can we also get rid of always asking for Administrator e-mail address? # ipa dnszone-add tri.test. --name-server=ns.test. Administrator e-mail address [hostmaster.tri.test.]: ... Is there any risk in filling that with default as any other attribute? IMO it would simplify adding zones for one more redundant step. CCing Rob in case he knows some historical reasons why this is requested every time. Martin There is no risk, because ipa-replica-prepare do that with default values Then let us do this, as we are already simplifying the dnszone-add command. However, this will not work with root zone ., and I'm not sure how often an admin email is used. I think whois is better utility to get contact email. Also RIPE-203 [1] recommends to use 'hostmaster' alias. [1] http://www.ripe.net/ripe/docs/ripe-203 This will likely generate tons of invalid e-mail addresses which is somehow unfortunate. Please keep in mind that: 1) E-mail hostmaster@ipa.domain.example. will be useful only if ipa.domain.example. has MX record or at least A/ record (which is usually not the case for domains). 2) WHOIS is not useful for internal domains which is the main deployment scenario for IPA, right? DNS zone . is quite an exception, you are not adding that zone every day. So I would not keep asking for admin mail just for this one. You can add a interactive prompt callback to ask in this case and otherwise just use the default - up to you.
[Freeipa-devel] [PATCH 0269] ipalib: host_del: Extend LDAPDelete's takes_options instead
Hi, The host-del command did not accept --continue option, since the takes_options was overriden and did not take the options from LDAPDelete. Fix the behaviour. https://fedorahosted.org/freeipa/ticket/4473 -- Tomas Babej Associate Software Engineer | Red Hat | Identity Management RHCE | Brno Site | IRC: tbabej | freeipa.org From b38301078116b2fb9bbefcb2875ce1a600e37de8 Mon Sep 17 00:00:00 2001 From: Tomas Babej tba...@redhat.com Date: Tue, 5 Aug 2014 09:12:24 +0200 Subject: [PATCH] ipalib: host_del: Extend LDAPDelete's takes_options instead of overriding The host-del command did not accept --continue option, since the takes_options was overriden and did not take the options from LDAPDelete. Fix the behaviour. https://fedorahosted.org/freeipa/ticket/4473 --- API.txt | 3 ++- ipalib/plugins/host.py | 2 +- ipatests/test_xmlrpc/test_host_plugin.py | 5 + 3 files changed, 4 insertions(+), 6 deletions(-) diff --git a/API.txt b/API.txt index 1243fefa7d37b34ac3dd0fac3df8820142c9f2c7..09464dbe2bc9fb7fa22a91879dc957da92aadc8f 100644 --- a/API.txt +++ b/API.txt @@ -1820,8 +1820,9 @@ output: Output('completed', type 'int', None) output: Output('failed', type 'dict', None) output: Entry('result', type 'dict', Gettext('A dictionary representing an LDAP entry', domain='ipa', localedir=None)) command: host_del -args: 1,2,3 +args: 1,3,3 arg: Str('fqdn', attribute=True, cli_name='hostname', multivalue=True, primary_key=True, query=True, required=True) +option: Flag('continue', autofill=True, cli_name='continue', default=False) option: Flag('updatedns?', autofill=True, default=False) option: Str('version?', exclude='webui') output: Output('result', type 'dict', None) diff --git a/ipalib/plugins/host.py b/ipalib/plugins/host.py index 4ec788d38ead1b987e94d5a68c14163d4dec2ea7..bbee09395dc55730d400944729a5bed48670f785 100644 --- a/ipalib/plugins/host.py +++ b/ipalib/plugins/host.py @@ -650,7 +650,7 @@ class host_del(LDAPDelete): msg_summary = _('Deleted host %(value)s') member_attributes = ['managedby'] -takes_options = ( +takes_options = LDAPDelete.takes_options + ( Flag('updatedns?', doc=_('Remove entries from DNS'), default=False, diff --git a/ipatests/test_xmlrpc/test_host_plugin.py b/ipatests/test_xmlrpc/test_host_plugin.py index a98fed1b64853e548d489f1bc12d012d18f986a7..7c0312bae5e9e652085aac8f1970610044504281 100644 --- a/ipatests/test_xmlrpc/test_host_plugin.py +++ b/ipatests/test_xmlrpc/test_host_plugin.py @@ -139,10 +139,7 @@ sshpubkeyfp = u'13:67:6B:BF:4E:A2:05:8E:AE:25:8B:A1:31:DE:6F:1B public key test class test_host(Declarative): cleanup_commands = [ -('host_del', [fqdn1], {}), -('host_del', [fqdn2], {}), -('host_del', [fqdn3], {}), -('host_del', [fqdn4], {}), +('host_del', [fqdn1, fqdn2, fqdn3, fqdn4], {'continue': True}), ('service_del', [service1], {}), ] -- 1.9.3 ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0269] ipalib: host_del: Extend LDAPDelete's takes_options instead
Dne 16.9.2014 v 13:21 Tomas Babej napsal(a): Hi, The host-del command did not accept --continue option, since the takes_options was overriden and did not take the options from LDAPDelete. Fix the behaviour. https://fedorahosted.org/freeipa/ticket/4473 ACK. -- Jan Cholasta ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 0009 Detect and configure all usable IP addresses.
On 09/12/2014 07:24 PM, Martin Basti wrote: snip / Be careful, reviewed on friday! :-) 1) whitespace error + pep8 error patch:76: trailing whitespace. # there is reverse zone for every ip address warning: 1 line adds whitespace errors. ./ipaserver/install/bindinstance.py:640:9: E265 block comment should start with '# ' Stupid mistake, sorry. 2) (server install) +for ip in ip_addresses: +for rev_zone in reverse_zones: +if bindinstance.verify_reverse_zone(rev_zone, str(ip)): +break +else: +sys.exit(1) Please add there error message instead 1 to exit function You are right, it's better to say what's wrong. 3) (server install) Code above, bindinstance.verify_reverse_zone(rev_zone, str(ip)): IMHO this will cause errors (not tested) try: rev-zones: [10.10.10.in-addr.arpa., 16.172.in-addr.arpa.] ip_addreses: [10.10.10.1, 172.16.0.1] it should be any() of zone can handle ip I indented the else branch more that I wanted. 4) (replica-install) I'm not sure about behavior of combination ip addresses, and reverse zones, In original code, if user specify reverse zone, the ip address must match. In patched version, you just add new zones for ip-addresses which doesn't math the user zones. IMO we should check if all addresses belongs to user specified zones, otherwise raise an error. But I'm open to suggestions. The behavior now basically is: Check if IP address matches any specified zone a. if yes we are good b. else search if there is existing zone that can be used ba. if yes we are good bb. else ask user for zone or create default if --unattended 5) Code for ipa-replica-install, and ipa-server-install, differs in parts where we expecting same behavior - ip addresses and reverse zones The behavoir now should be almost the same. The only difference is that when installing replica we need to search for existing reverse zones as they could be added during on another server. 6) https://engineering.redhat.com/irc-services.txt+# there is at least one ip address in every zone +if options.reverse_zones: +for reverse_zone in options.reverse_zones: +for ip in config.ips: -- A +if bindinstance.verify_reverse_zone(reverse_zone, ip): + reverse_zones.append(bindinstance.normalize_zone(reverse_zone)) +break +else: +sys.exit(1) * +# there is reverse zone for every ip address +for ip in config.ips: +for reverse_zone in options.reverse_zones: --- B +if bindinstance.verify_reverse_zone(reverse_zone, ip): +if reverse_zone not in reverse_zones: + reverse_zones.append(bindinstance.normalize_zone(reverse_zone)) +break +else: C +reverse_zone = bindinstance.find_reverse_zone(ip) +if reverse_zone is None and not options.no_reverse: +reverse_zone = util.get_reverse_zone_default(ip) - D1 +if not options.unattended and bindinstance.create_reverse(): - D +reverse_zone = bindinstance.read_reverse_zone(reverse_zone, ip) + reverse_zones.append(bindinstance.normalize_zone(reverse_zone)) - D2 You added all possible zones in step A, B step is not needed. If user doesn't specify reverse_zones you can just jump to C step *add error message C: elif not options.no_reverse D: you asked user if wants to create zone, but you don't care about answer, and in step D2 append zone from D1 note: --no-reverse and --reverse-zones cant be used together, you can use it in new code, test it before cycle Rewritten. 7) # Always use force=True as named is not set up yet add_zone(self.domain, self.zonemgr, dns_backup=self.dns_backup, -ns_hostname=api.env.host, ns_ip_address=nameserver_ip_address, -force=True) + ns_hostname=api.env.host, ns_ip_address=None, force=True) +#for ns_ip_address in nameserver_ip_address: +#add_zone(self.domain, self.zonemgr, dns_backup=self.dns_backup, +#ns_hostname=api.env.host, ns_ip_address=ns_ip_address, +#force=True) Please remove commented section I forget to clean the trash, thanks. You can remove 'ns_ip_address=None' from function 8) +if set(options.ip_addresses) = set(ips): +ips = options.ip_addresses +else: +print sys.stderr, Error: the hostname resolves to an IP address that is
Re: [Freeipa-devel] Proposal for #4456 -- Regular users should not be able to add OTP tokens with custom name
On Tue, 2014-08-19 at 17:10 -0400, Nathaniel McCallum wrote: Admins need the ability to specify the token ID in the case of imports. However, generally, this ability is not needed. Is it possible to offload the ID generation to the ipa-uuid plugin? I'm not quite sure how to enable this (I think it involves passing a magic value?). But I'm not quite sure how this fits in with the IPA framework as the generated value is the DN. However, assuming this can be used, I propose the following. The token ID is removed from the UI for regular users (but retained for admins). We change the ACIs for token addition/modification to prevent regular users from specifying the ID in an add or mod operation. The CLI would retain the option to set it, but this option would only be usable by admins. Make sense? Nobody has responded to this. :) However, since investigating it a bit more, this approach won't really work without further effort. Here is the problem. First, the UUID plugin doesn't currently support this kind of operation. Either it needs to be modified or a new plugin needs to be created. Second, the client needs to know the ID in order to generate the token URI. If we generate the UUID inside the DS, the UUID is unknown to the client and the URI can't be generated. This would mean a new control. As I see it we have three options: 1. Remove the option to specify the ipatokenUniqueID from the GUI. Don't make any change in the CLI. ENFORCEMENT: none EFFORT: low 2. Perform a server-side check for admin membership. Raise an exception if the ipatokenUniqueID is specified and the user is not an admin. ENFORCEMENT: API-level EFFORT: medium 3. Modify otptoken-add to create tokens with a magical ipatokenUniqueID value by default. An ACI would prevent normal users from adding tokens without this magic value. Create/modify a plugin to generate UUIDs when the magic value is found. Send a control back to the client indicating the real ipatokenUniqueID value. Modify otptoken-add to read this control. ENFORCEMENT: DS-level EFFORT: high I think my preference for now is #1. Thoughts? Nathaniel ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] Proposal for #4456 -- Regular users should not be able to add OTP tokens with custom name
On 16.9.2014 17:26, Nathaniel McCallum wrote: On Tue, 2014-08-19 at 17:10 -0400, Nathaniel McCallum wrote: Admins need the ability to specify the token ID in the case of imports. However, generally, this ability is not needed. Is it possible to offload the ID generation to the ipa-uuid plugin? I'm not quite sure how to enable this (I think it involves passing a magic value?). But I'm not quite sure how this fits in with the IPA framework as the generated value is the DN. However, assuming this can be used, I propose the following. The token ID is removed from the UI for regular users (but retained for admins). We change the ACIs for token addition/modification to prevent regular users from specifying the ID in an add or mod operation. The CLI would retain the option to set it, but this option would only be usable by admins. Make sense? Nobody has responded to this. :) However, since investigating it a bit more, this approach won't really work without further effort. Here is the problem. First, the UUID plugin doesn't currently support this kind of operation. Either it needs to be modified or a new plugin needs to be created. Second, the client needs to know the ID in order to generate the token URI. If we generate the UUID inside the DS, the UUID is unknown to the client and the URI can't be generated. This would mean a new control. As I see it we have three options: 1. Remove the option to specify the ipatokenUniqueID from the GUI. Don't make any change in the CLI. ENFORCEMENT: none EFFORT: low 2. Perform a server-side check for admin membership. Raise an exception if the ipatokenUniqueID is specified and the user is not an admin. ENFORCEMENT: API-level EFFORT: medium 3. Modify otptoken-add to create tokens with a magical ipatokenUniqueID value by default. An ACI would prevent normal users from adding tokens without this magic value. Create/modify a plugin to generate UUIDs when the magic value is found. Send a control back to the client indicating the real ipatokenUniqueID value. Modify otptoken-add to read this control. ENFORCEMENT: DS-level EFFORT: high I think my preference for now is #1. Thoughts? Nathaniel I want to raise a question if we really want to disable this feature for normal users. Wouldn't it be better to improve error message or check if the name is taken? UUID is the value which is displayed in the Soft. Token application as name. Sure, one can rename it there to match the description in IPA but that seems quite unpleasant to me. Also user has to know that he can rename the token in FreeOTP, Atm the existence check might be little problematic - disclose of information which is not readable to user by default. But current state is basically it, just unfriendly. -- Petr Vobornik ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] Proposal for #4456 -- Regular users should not be able to add OTP tokens with custom name
On Tue, 2014-09-16 at 17:51 +0200, Petr Vobornik wrote: On 16.9.2014 17:26, Nathaniel McCallum wrote: On Tue, 2014-08-19 at 17:10 -0400, Nathaniel McCallum wrote: Admins need the ability to specify the token ID in the case of imports. However, generally, this ability is not needed. Is it possible to offload the ID generation to the ipa-uuid plugin? I'm not quite sure how to enable this (I think it involves passing a magic value?). But I'm not quite sure how this fits in with the IPA framework as the generated value is the DN. However, assuming this can be used, I propose the following. The token ID is removed from the UI for regular users (but retained for admins). We change the ACIs for token addition/modification to prevent regular users from specifying the ID in an add or mod operation. The CLI would retain the option to set it, but this option would only be usable by admins. Make sense? Nobody has responded to this. :) However, since investigating it a bit more, this approach won't really work without further effort. Here is the problem. First, the UUID plugin doesn't currently support this kind of operation. Either it needs to be modified or a new plugin needs to be created. Second, the client needs to know the ID in order to generate the token URI. If we generate the UUID inside the DS, the UUID is unknown to the client and the URI can't be generated. This would mean a new control. As I see it we have three options: 1. Remove the option to specify the ipatokenUniqueID from the GUI. Don't make any change in the CLI. ENFORCEMENT: none EFFORT: low 2. Perform a server-side check for admin membership. Raise an exception if the ipatokenUniqueID is specified and the user is not an admin. ENFORCEMENT: API-level EFFORT: medium 3. Modify otptoken-add to create tokens with a magical ipatokenUniqueID value by default. An ACI would prevent normal users from adding tokens without this magic value. Create/modify a plugin to generate UUIDs when the magic value is found. Send a control back to the client indicating the real ipatokenUniqueID value. Modify otptoken-add to read this control. ENFORCEMENT: DS-level EFFORT: high I think my preference for now is #1. Thoughts? Nathaniel I want to raise a question if we really want to disable this feature for normal users. Wouldn't it be better to improve error message or check if the name is taken? UUID is the value which is displayed in the Soft. Token application as name. Sure, one can rename it there to match the description in IPA but that seems quite unpleasant to me. Also user has to know that he can rename the token in FreeOTP, Atm the existence check might be little problematic - disclose of information which is not readable to user by default. But current state is basically it, just unfriendly. The existence check is impossible: users can't see tokens they don't own. ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 0009 Detect and configure all usable IP addresses.
On 16/09/14 15:59, David Kupka wrote: On 09/12/2014 07:24 PM, Martin Basti wrote: snip / Be careful, reviewed on friday! :-) 1) whitespace error + pep8 error patch:76: trailing whitespace. # there is reverse zone for every ip address warning: 1 line adds whitespace errors. ./ipaserver/install/bindinstance.py:640:9: E265 block comment should start with '# ' Stupid mistake, sorry. 2) (server install) +for ip in ip_addresses: +for rev_zone in reverse_zones: +if bindinstance.verify_reverse_zone(rev_zone, str(ip)): +break +else: +sys.exit(1) Please add there error message instead 1 to exit function You are right, it's better to say what's wrong. 3) (server install) Code above, bindinstance.verify_reverse_zone(rev_zone, str(ip)): IMHO this will cause errors (not tested) try: rev-zones: [10.10.10.in-addr.arpa., 16.172.in-addr.arpa.] ip_addreses: [10.10.10.1, 172.16.0.1] it should be any() of zone can handle ip I indented the else branch more that I wanted. 4) (replica-install) I'm not sure about behavior of combination ip addresses, and reverse zones, In original code, if user specify reverse zone, the ip address must match. In patched version, you just add new zones for ip-addresses which doesn't math the user zones. IMO we should check if all addresses belongs to user specified zones, otherwise raise an error. But I'm open to suggestions. The behavior now basically is: Check if IP address matches any specified zone a. if yes we are good b. else search if there is existing zone that can be used ba. if yes we are good bb. else ask user for zone or create default if --unattended 5) Code for ipa-replica-install, and ipa-server-install, differs in parts where we expecting same behavior - ip addresses and reverse zones The behavoir now should be almost the same. The only difference is that when installing replica we need to search for existing reverse zones as they could be added during on another server. 6) https://engineering.redhat.com/irc-services.txt+# there is at least one ip address in every zone +if options.reverse_zones: +for reverse_zone in options.reverse_zones: +for ip in config.ips: -- A +if bindinstance.verify_reverse_zone(reverse_zone, ip): + reverse_zones.append(bindinstance.normalize_zone(reverse_zone)) +break +else: +sys.exit(1) * +# there is reverse zone for every ip address +for ip in config.ips: +for reverse_zone in options.reverse_zones: --- B +if bindinstance.verify_reverse_zone(reverse_zone, ip): +if reverse_zone not in reverse_zones: + reverse_zones.append(bindinstance.normalize_zone(reverse_zone)) +break +else: C +reverse_zone = bindinstance.find_reverse_zone(ip) +if reverse_zone is None and not options.no_reverse: +reverse_zone = util.get_reverse_zone_default(ip) - D1 +if not options.unattended and bindinstance.create_reverse(): - D +reverse_zone = bindinstance.read_reverse_zone(reverse_zone, ip) + reverse_zones.append(bindinstance.normalize_zone(reverse_zone)) - D2 You added all possible zones in step A, B step is not needed. If user doesn't specify reverse_zones you can just jump to C step *add error message C: elif not options.no_reverse D: you asked user if wants to create zone, but you don't care about answer, and in step D2 append zone from D1 note: --no-reverse and --reverse-zones cant be used together, you can use it in new code, test it before cycle Rewritten. 7) # Always use force=True as named is not set up yet add_zone(self.domain, self.zonemgr, dns_backup=self.dns_backup, -ns_hostname=api.env.host, ns_ip_address=nameserver_ip_address, -force=True) + ns_hostname=api.env.host, ns_ip_address=None, force=True) +#for ns_ip_address in nameserver_ip_address: +#add_zone(self.domain, self.zonemgr, dns_backup=self.dns_backup, +#ns_hostname=api.env.host, ns_ip_address=ns_ip_address, +#force=True) Please remove commented section I forget to clean the trash, thanks. You can remove 'ns_ip_address=None' from function 8) +if set(options.ip_addresses) = set(ips): +ips = options.ip_addresses +else: +print sys.stderr,
Re: [Freeipa-devel] Proposal for #4456 -- Regular users should not be able to add OTP tokens with custom name
On Tue, 2014-09-16 at 12:22 -0400, Simo Sorce wrote: On Tue, 16 Sep 2014 11:52:43 -0400 Nathaniel McCallum npmccal...@redhat.com wrote: On Tue, 2014-09-16 at 17:51 +0200, Petr Vobornik wrote: On 16.9.2014 17:26, Nathaniel McCallum wrote: On Tue, 2014-08-19 at 17:10 -0400, Nathaniel McCallum wrote: Admins need the ability to specify the token ID in the case of imports. However, generally, this ability is not needed. Is it possible to offload the ID generation to the ipa-uuid plugin? I'm not quite sure how to enable this (I think it involves passing a magic value?). But I'm not quite sure how this fits in with the IPA framework as the generated value is the DN. However, assuming this can be used, I propose the following. The token ID is removed from the UI for regular users (but retained for admins). We change the ACIs for token addition/modification to prevent regular users from specifying the ID in an add or mod operation. The CLI would retain the option to set it, but this option would only be usable by admins. Make sense? Nobody has responded to this. :) However, since investigating it a bit more, this approach won't really work without further effort. Here is the problem. First, the UUID plugin doesn't currently support this kind of operation. Either it needs to be modified or a new plugin needs to be created. Second, the client needs to know the ID in order to generate the token URI. If we generate the UUID inside the DS, the UUID is unknown to the client and the URI can't be generated. This would mean a new control. As I see it we have three options: 1. Remove the option to specify the ipatokenUniqueID from the GUI. Don't make any change in the CLI. ENFORCEMENT: none EFFORT: low 2. Perform a server-side check for admin membership. Raise an exception if the ipatokenUniqueID is specified and the user is not an admin. ENFORCEMENT: API-level EFFORT: medium 3. Modify otptoken-add to create tokens with a magical ipatokenUniqueID value by default. An ACI would prevent normal users from adding tokens without this magic value. Create/modify a plugin to generate UUIDs when the magic value is found. Send a control back to the client indicating the real ipatokenUniqueID value. Modify otptoken-add to read this control. ENFORCEMENT: DS-level EFFORT: high I think my preference for now is #1. Thoughts? Nathaniel I want to raise a question if we really want to disable this feature for normal users. Wouldn't it be better to improve error message or check if the name is taken? UUID is the value which is displayed in the Soft. Token application as name. Sure, one can rename it there to match the description in IPA but that seems quite unpleasant to me. Also user has to know that he can rename the token in FreeOTP, Atm the existence check might be little problematic - disclose of information which is not readable to user by default. But current state is basically it, just unfriendly. The existence check is impossible: users can't see tokens they don't own. Why don't we simply allow user to use an arbitrary name of their choosing (or a random string if not specified) and then append their uid string to it in the UI ? I wonder if we have ACIs where the value must match a pattern defined by the bound user ... This would present a strange scenario where a token is created and then reassigned to another user. Nathaniel ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] Proposal for #4456 -- Regular users should not be able to add OTP tokens with custom name
On Tue, 16 Sep 2014 11:52:43 -0400 Nathaniel McCallum npmccal...@redhat.com wrote: On Tue, 2014-09-16 at 17:51 +0200, Petr Vobornik wrote: On 16.9.2014 17:26, Nathaniel McCallum wrote: On Tue, 2014-08-19 at 17:10 -0400, Nathaniel McCallum wrote: Admins need the ability to specify the token ID in the case of imports. However, generally, this ability is not needed. Is it possible to offload the ID generation to the ipa-uuid plugin? I'm not quite sure how to enable this (I think it involves passing a magic value?). But I'm not quite sure how this fits in with the IPA framework as the generated value is the DN. However, assuming this can be used, I propose the following. The token ID is removed from the UI for regular users (but retained for admins). We change the ACIs for token addition/modification to prevent regular users from specifying the ID in an add or mod operation. The CLI would retain the option to set it, but this option would only be usable by admins. Make sense? Nobody has responded to this. :) However, since investigating it a bit more, this approach won't really work without further effort. Here is the problem. First, the UUID plugin doesn't currently support this kind of operation. Either it needs to be modified or a new plugin needs to be created. Second, the client needs to know the ID in order to generate the token URI. If we generate the UUID inside the DS, the UUID is unknown to the client and the URI can't be generated. This would mean a new control. As I see it we have three options: 1. Remove the option to specify the ipatokenUniqueID from the GUI. Don't make any change in the CLI. ENFORCEMENT: none EFFORT: low 2. Perform a server-side check for admin membership. Raise an exception if the ipatokenUniqueID is specified and the user is not an admin. ENFORCEMENT: API-level EFFORT: medium 3. Modify otptoken-add to create tokens with a magical ipatokenUniqueID value by default. An ACI would prevent normal users from adding tokens without this magic value. Create/modify a plugin to generate UUIDs when the magic value is found. Send a control back to the client indicating the real ipatokenUniqueID value. Modify otptoken-add to read this control. ENFORCEMENT: DS-level EFFORT: high I think my preference for now is #1. Thoughts? Nathaniel I want to raise a question if we really want to disable this feature for normal users. Wouldn't it be better to improve error message or check if the name is taken? UUID is the value which is displayed in the Soft. Token application as name. Sure, one can rename it there to match the description in IPA but that seems quite unpleasant to me. Also user has to know that he can rename the token in FreeOTP, Atm the existence check might be little problematic - disclose of information which is not readable to user by default. But current state is basically it, just unfriendly. The existence check is impossible: users can't see tokens they don't own. Why don't we simply allow user to use an arbitrary name of their choosing (or a random string if not specified) and then append their uid string to it in the UI ? I wonder if we have ACIs where the value must match a pattern defined by the bound user ... Simo. -- Simo Sorce * Red Hat, Inc * New York ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] Proposal for #4456 -- Regular users should not be able to add OTP tokens with custom name
On Tue, 16 Sep 2014 12:23:51 -0400 Nathaniel McCallum npmccal...@redhat.com wrote: On Tue, 2014-09-16 at 12:22 -0400, Simo Sorce wrote: On Tue, 16 Sep 2014 11:52:43 -0400 Nathaniel McCallum npmccal...@redhat.com wrote: On Tue, 2014-09-16 at 17:51 +0200, Petr Vobornik wrote: On 16.9.2014 17:26, Nathaniel McCallum wrote: On Tue, 2014-08-19 at 17:10 -0400, Nathaniel McCallum wrote: Admins need the ability to specify the token ID in the case of imports. However, generally, this ability is not needed. Is it possible to offload the ID generation to the ipa-uuid plugin? I'm not quite sure how to enable this (I think it involves passing a magic value?). But I'm not quite sure how this fits in with the IPA framework as the generated value is the DN. However, assuming this can be used, I propose the following. The token ID is removed from the UI for regular users (but retained for admins). We change the ACIs for token addition/modification to prevent regular users from specifying the ID in an add or mod operation. The CLI would retain the option to set it, but this option would only be usable by admins. Make sense? Nobody has responded to this. :) However, since investigating it a bit more, this approach won't really work without further effort. Here is the problem. First, the UUID plugin doesn't currently support this kind of operation. Either it needs to be modified or a new plugin needs to be created. Second, the client needs to know the ID in order to generate the token URI. If we generate the UUID inside the DS, the UUID is unknown to the client and the URI can't be generated. This would mean a new control. As I see it we have three options: 1. Remove the option to specify the ipatokenUniqueID from the GUI. Don't make any change in the CLI. ENFORCEMENT: none EFFORT: low 2. Perform a server-side check for admin membership. Raise an exception if the ipatokenUniqueID is specified and the user is not an admin. ENFORCEMENT: API-level EFFORT: medium 3. Modify otptoken-add to create tokens with a magical ipatokenUniqueID value by default. An ACI would prevent normal users from adding tokens without this magic value. Create/modify a plugin to generate UUIDs when the magic value is found. Send a control back to the client indicating the real ipatokenUniqueID value. Modify otptoken-add to read this control. ENFORCEMENT: DS-level EFFORT: high I think my preference for now is #1. Thoughts? Nathaniel I want to raise a question if we really want to disable this feature for normal users. Wouldn't it be better to improve error message or check if the name is taken? UUID is the value which is displayed in the Soft. Token application as name. Sure, one can rename it there to match the description in IPA but that seems quite unpleasant to me. Also user has to know that he can rename the token in FreeOTP, Atm the existence check might be little problematic - disclose of information which is not readable to user by default. But current state is basically it, just unfriendly. The existence check is impossible: users can't see tokens they don't own. Why don't we simply allow user to use an arbitrary name of their choosing (or a random string if not specified) and then append their uid string to it in the UI ? I wonder if we have ACIs where the value must match a pattern defined by the bound user ... This would present a strange scenario where a token is created and then reassigned to another user. The idea is to strip away the tail when visualizing to the user, so it wouldn't really bee seen ? Or perhaps the token can simply be renamed when assigned. Simo. -- Simo Sorce * Red Hat, Inc * New York ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] Proposal for #4456 -- Regular users should not be able to add OTP tokens with custom name
On Tue, 2014-09-16 at 12:28 -0400, Simo Sorce wrote: On Tue, 16 Sep 2014 12:23:51 -0400 Nathaniel McCallum npmccal...@redhat.com wrote: On Tue, 2014-09-16 at 12:22 -0400, Simo Sorce wrote: On Tue, 16 Sep 2014 11:52:43 -0400 Nathaniel McCallum npmccal...@redhat.com wrote: On Tue, 2014-09-16 at 17:51 +0200, Petr Vobornik wrote: On 16.9.2014 17:26, Nathaniel McCallum wrote: On Tue, 2014-08-19 at 17:10 -0400, Nathaniel McCallum wrote: Admins need the ability to specify the token ID in the case of imports. However, generally, this ability is not needed. Is it possible to offload the ID generation to the ipa-uuid plugin? I'm not quite sure how to enable this (I think it involves passing a magic value?). But I'm not quite sure how this fits in with the IPA framework as the generated value is the DN. However, assuming this can be used, I propose the following. The token ID is removed from the UI for regular users (but retained for admins). We change the ACIs for token addition/modification to prevent regular users from specifying the ID in an add or mod operation. The CLI would retain the option to set it, but this option would only be usable by admins. Make sense? Nobody has responded to this. :) However, since investigating it a bit more, this approach won't really work without further effort. Here is the problem. First, the UUID plugin doesn't currently support this kind of operation. Either it needs to be modified or a new plugin needs to be created. Second, the client needs to know the ID in order to generate the token URI. If we generate the UUID inside the DS, the UUID is unknown to the client and the URI can't be generated. This would mean a new control. As I see it we have three options: 1. Remove the option to specify the ipatokenUniqueID from the GUI. Don't make any change in the CLI. ENFORCEMENT: none EFFORT: low 2. Perform a server-side check for admin membership. Raise an exception if the ipatokenUniqueID is specified and the user is not an admin. ENFORCEMENT: API-level EFFORT: medium 3. Modify otptoken-add to create tokens with a magical ipatokenUniqueID value by default. An ACI would prevent normal users from adding tokens without this magic value. Create/modify a plugin to generate UUIDs when the magic value is found. Send a control back to the client indicating the real ipatokenUniqueID value. Modify otptoken-add to read this control. ENFORCEMENT: DS-level EFFORT: high I think my preference for now is #1. Thoughts? Nathaniel I want to raise a question if we really want to disable this feature for normal users. Wouldn't it be better to improve error message or check if the name is taken? UUID is the value which is displayed in the Soft. Token application as name. Sure, one can rename it there to match the description in IPA but that seems quite unpleasant to me. Also user has to know that he can rename the token in FreeOTP, Atm the existence check might be little problematic - disclose of information which is not readable to user by default. But current state is basically it, just unfriendly. The existence check is impossible: users can't see tokens they don't own. Why don't we simply allow user to use an arbitrary name of their choosing (or a random string if not specified) and then append their uid string to it in the UI ? I wonder if we have ACIs where the value must match a pattern defined by the bound user ... This would present a strange scenario where a token is created and then reassigned to another user. The idea is to strip away the tail when visualizing to the user, so it wouldn't really bee seen ? Or perhaps the token can simply be renamed when assigned. It just feels like a kludge to me. Also, automated stripping may run afoul of names assigned by the imports. What is wrong with the three options I provided above? We can do #1 now and #2 or #3 later. ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] Proposal for #4456 -- Regular users should not be able to add OTP tokens with custom name
On Tue, 16 Sep 2014 12:33:16 -0400 Nathaniel McCallum npmccal...@redhat.com wrote: On Tue, 2014-09-16 at 12:28 -0400, Simo Sorce wrote: On Tue, 16 Sep 2014 12:23:51 -0400 Nathaniel McCallum npmccal...@redhat.com wrote: On Tue, 2014-09-16 at 12:22 -0400, Simo Sorce wrote: On Tue, 16 Sep 2014 11:52:43 -0400 Nathaniel McCallum npmccal...@redhat.com wrote: On Tue, 2014-09-16 at 17:51 +0200, Petr Vobornik wrote: On 16.9.2014 17:26, Nathaniel McCallum wrote: On Tue, 2014-08-19 at 17:10 -0400, Nathaniel McCallum wrote: Admins need the ability to specify the token ID in the case of imports. However, generally, this ability is not needed. Is it possible to offload the ID generation to the ipa-uuid plugin? I'm not quite sure how to enable this (I think it involves passing a magic value?). But I'm not quite sure how this fits in with the IPA framework as the generated value is the DN. However, assuming this can be used, I propose the following. The token ID is removed from the UI for regular users (but retained for admins). We change the ACIs for token addition/modification to prevent regular users from specifying the ID in an add or mod operation. The CLI would retain the option to set it, but this option would only be usable by admins. Make sense? Nobody has responded to this. :) However, since investigating it a bit more, this approach won't really work without further effort. Here is the problem. First, the UUID plugin doesn't currently support this kind of operation. Either it needs to be modified or a new plugin needs to be created. Second, the client needs to know the ID in order to generate the token URI. If we generate the UUID inside the DS, the UUID is unknown to the client and the URI can't be generated. This would mean a new control. As I see it we have three options: 1. Remove the option to specify the ipatokenUniqueID from the GUI. Don't make any change in the CLI. ENFORCEMENT: none EFFORT: low 2. Perform a server-side check for admin membership. Raise an exception if the ipatokenUniqueID is specified and the user is not an admin. ENFORCEMENT: API-level EFFORT: medium 3. Modify otptoken-add to create tokens with a magical ipatokenUniqueID value by default. An ACI would prevent normal users from adding tokens without this magic value. Create/modify a plugin to generate UUIDs when the magic value is found. Send a control back to the client indicating the real ipatokenUniqueID value. Modify otptoken-add to read this control. ENFORCEMENT: DS-level EFFORT: high I think my preference for now is #1. Thoughts? Nathaniel I want to raise a question if we really want to disable this feature for normal users. Wouldn't it be better to improve error message or check if the name is taken? UUID is the value which is displayed in the Soft. Token application as name. Sure, one can rename it there to match the description in IPA but that seems quite unpleasant to me. Also user has to know that he can rename the token in FreeOTP, Atm the existence check might be little problematic - disclose of information which is not readable to user by default. But current state is basically it, just unfriendly. The existence check is impossible: users can't see tokens they don't own. Why don't we simply allow user to use an arbitrary name of their choosing (or a random string if not specified) and then append their uid string to it in the UI ? I wonder if we have ACIs where the value must match a pattern defined by the bound user ... This would present a strange scenario where a token is created and then reassigned to another user. The idea is to strip away the tail when visualizing to the user, so it wouldn't really bee seen ? Or perhaps the token can simply be renamed when assigned. It just feels like a kludge to me. Also, automated stripping may run afoul of names assigned by the imports. What is wrong with the three options I provided above? We can do #1 now and #2 or #3 later. Nothing wrong, really, #1 is fine by me. Simo. -- Simo Sorce * Red Hat, Inc * New York ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0064] Create ipa-otp-decrement 389DS plugin
On 09/15/2014 09:05 PM, Nathaniel McCallum wrote: This plugin ensures that all counter/watermark operations are atomic and never decrement. Also, deletion is not permitted. https://fedorahosted.org/freeipa/ticket/4494 ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel Hello Nathaniel, Starting looking at it, I have just a question about sanitize_input. If the modification (replace) is related to counter/watermark, it triggers an internal search on the target entry itself. The original/modified entry is also present in the pblock. The internal search will check the filter but except that what is the benefit vs taking the entry directly in the pblock. thanks thierry ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0064] Create ipa-otp-decrement 389DS plugin
On Tue, 2014-09-16 at 19:24 +0200, thierry bordaz wrote: On 09/15/2014 09:05 PM, Nathaniel McCallum wrote: This plugin ensures that all counter/watermark operations are atomic and never decrement. Also, deletion is not permitted. https://fedorahosted.org/freeipa/ticket/4494 ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel Hello Nathaniel, Starting looking at it, I have just a question about sanitize_input. If the modification (replace) is related to counter/watermark, it triggers an internal search on the target entry itself. The original/modified entry is also present in the pblock. The internal search will check the filter but except that what is the benefit vs taking the entry directly in the pblock. I didn't know the entry was already in the pblock. What loads it? And when? How do I access it? Nathaniel ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCH] [dyndb] Fix error handling in configure_view() to prevent deadlocks
Hello, attached patches fix https://bugzilla.redhat.com/show_bug.cgi?id=1142150 https://bugzilla.redhat.com/show_bug.cgi?id=1142152 ... and improve related error messages. I will push it to https://github.com/spacekpe/bind-dynamic_db if you are okay with it. -- Petr^2 Spacek From d2d0b25e7586c306fc5e6ac62a82f680e70a91e8 Mon Sep 17 00:00:00 2001 From: Petr Spacek pspa...@redhat.com Date: Tue, 16 Sep 2014 19:25:34 +0200 Subject: [PATCH] Fix error handling in configure_view() to prevent deadlocks. Signed-off-by: Petr Spacek pspa...@redhat.com --- bin/named/server.c | 8 +++- lib/dns/dynamic_db.c | 10 +- lib/dns/include/dns/dynamic_db.h | 2 +- 3 files changed, 13 insertions(+), 7 deletions(-) diff --git a/bin/named/server.c b/bin/named/server.c index 6236c1014f07bdb826308290d98a0c5112732b31..0ea512b4b7673a1bf3a88506c6922e5d74d6b909 100644 --- a/bin/named/server.c +++ b/bin/named/server.c @@ -2178,6 +2178,7 @@ configure_view(dns_view_t *view, cfg_obj_t *config, cfg_obj_t *vconfig, unsigned int query_timeout, ndisp; struct cfg_context *nzctx; dns_rpz_zone_t *rpz; + dns_dyndb_arguments_t *args = NULL; REQUIRE(DNS_VIEW_VALID(view)); @@ -3341,8 +3342,6 @@ configure_view(dns_view_t *view, cfg_obj_t *config, cfg_obj_t *vconfig, (void)cfg_map_get(config, dynamic-db, dynamic_db_list); element = cfg_list_first(dynamic_db_list); if (element != NULL) { - dns_dyndb_arguments_t *args; - args = dns_dyndb_arguments_create(mctx); if (args == NULL) { result = ISC_R_NOMEMORY; @@ -3355,11 +3354,8 @@ configure_view(dns_view_t *view, cfg_obj_t *config, cfg_obj_t *vconfig, while (element != NULL) { obj = cfg_listelt_value(element); CHECK(configure_dynamic_db(obj, mctx, args)); - element = cfg_list_next(element); } - - dns_dyndb_arguments_destroy(mctx, args); } /* @@ -3547,6 +3543,8 @@ configure_view(dns_view_t *view, cfg_obj_t *config, cfg_obj_t *vconfig, if (cache != NULL) dns_cache_detach(cache); + if (args != NULL) + dns_dyndb_arguments_destroy(mctx, args); return (result); } diff --git a/lib/dns/dynamic_db.c b/lib/dns/dynamic_db.c index bf831617b391778ec540b2a5ca0df341937f2427..30c56a65c7227497c3e772c3e1b58ff49eacbd35 100644 --- a/lib/dns/dynamic_db.c +++ b/lib/dns/dynamic_db.c @@ -280,16 +280,24 @@ dns_dyndb_arguments_create(isc_mem_t *mctx) } void -dns_dyndb_arguments_destroy(isc_mem_t *mctx, dns_dyndb_arguments_t *args) +dns_dyndb_arguments_destroy(isc_mem_t *mctx, dns_dyndb_arguments_t **argsp) { + dns_dyndb_arguments_t *args; + REQUIRE(args != NULL); + args = *argsp; + if (args == NULL) + return; + dns_dyndb_set_view(args, NULL); dns_dyndb_set_zonemgr(args, NULL); dns_dyndb_set_task(args, NULL); dns_dyndb_set_timermgr(args, NULL); isc_mem_put(mctx, args, sizeof(*args)); + + *argsp = NULL; } void diff --git a/lib/dns/include/dns/dynamic_db.h b/lib/dns/include/dns/dynamic_db.h index 673ad4bc12bfe0875d7bac69a219574e11674bcf..e50491d33c4cd4a9e600110926349af32622c24a 100644 --- a/lib/dns/include/dns/dynamic_db.h +++ b/lib/dns/include/dns/dynamic_db.h @@ -35,7 +35,7 @@ isc_result_t dns_dynamic_db_load(const char *libname, const char *name, void dns_dynamic_db_cleanup(isc_boolean_t exiting); dns_dyndb_arguments_t *dns_dyndb_arguments_create(isc_mem_t *mctx); -void dns_dyndb_arguments_destroy(isc_mem_t *mctx, dns_dyndb_arguments_t *args); +void dns_dyndb_arguments_destroy(isc_mem_t *mctx, dns_dyndb_arguments_t **args); void dns_dyndb_set_view(dns_dyndb_arguments_t *args, dns_view_t *view); dns_view_t *dns_dyndb_get_view(dns_dyndb_arguments_t *args); -- 1.9.3 From 378bdc0646e1738b39f5da2e26da7753d7b562a6 Mon Sep 17 00:00:00 2001 From: Petr Spacek pspa...@redhat.com Date: Tue, 16 Sep 2014 19:26:03 +0200 Subject: [PATCH] Log error if dynamic database configuration failed. Signed-off-by: Petr Spacek pspa...@redhat.com --- bin/named/server.c | 5 + 1 file changed, 5 insertions(+) diff --git a/bin/named/server.c b/bin/named/server.c index 0ea512b4b7673a1bf3a88506c6922e5d74d6b909..590b32923b8fdfd68caabfc57a1dbd127a234ce3 100644 --- a/bin/named/server.c +++ b/bin/named/server.c @@ -1308,6 +1308,11 @@ configure_dynamic_db(const cfg_obj_t *dynamic_db, isc_mem_t *mctx, CHECK(dns_dynamic_db_load(libname, name, mctx, argv, dyndb_args)); cleanup: + if (result != ISC_R_SUCCESS) + isc_log_write(ns_g_lctx, NS_LOGCATEGORY_GENERAL, + NS_LOGMODULE_SERVER, ISC_LOG_ERROR, + dynamic database '%s' configuration failed: %s, + name, isc_result_totext(result)); if (argv != NULL) isc_mem_free(mctx, argv); -- 1.9.3 ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCH 0065] Don't allow users to create tokens with a specified ID
We perform this enforcement at the API level since: * DS level enforcement would be difficult * ipatokenUniqueID generation already happens at the API level It may be nice in the future to perform enforcement in the DS itself. However, the question of the location of enforcement is largely an aesthetic issue. https://fedorahosted.org/freeipa/ticket/4456 From d52d7b5ed8514057a0d55ac2009576dadcec1cef Mon Sep 17 00:00:00 2001 From: Nathaniel McCallum npmccal...@redhat.com Date: Tue, 16 Sep 2014 13:21:05 -0400 Subject: [PATCH] Don't allow users to create tokens with a specified ID We perform this enforcement at the API level since: * DS level enforcement would be difficult * ipatokenUniqueID generation already happens at the API level It may be nice in the future to perform enforcement in the DS itself. However, the question of the location of enforcement is largely an aesthetic issue. https://fedorahosted.org/freeipa/ticket/4456 --- ipalib/plugins/otptoken.py | 8 1 file changed, 8 insertions(+) diff --git a/ipalib/plugins/otptoken.py b/ipalib/plugins/otptoken.py index 1bd85d4b952dc51ea800ed37c49b3c50aeb31492..231e383e3a3bc8506bed1c72f2859c2832c240a1 100644 --- a/ipalib/plugins/otptoken.py +++ b/ipalib/plugins/otptoken.py @@ -259,6 +259,14 @@ class otptoken_add(LDAPCreate): entry_attrs['ipatokenuniqueid'] = str(uuid.uuid4()) dn = DN(ipatokenuniqueid=%s % entry_attrs['ipatokenuniqueid'], dn) +# Ensure that no token id is specified if the user is not an admin. +else: +gresult = self.api.Command.group_show(cn=u'admins')['result'] +uresult = self.api.Command.user_find(whoami=True)['result'] +if uresult[0]['uid'][0] not in gresult['member_user']: +raise ValidationError(name='ipatokenuniqueid', + error='can only be specified by admins') + if not _check_interval(options.get('ipatokennotbefore', None), options.get('ipatokennotafter', None)): raise ValidationError(name='not_after', -- 2.1.0 ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] Proposal for #4456 -- Regular users should not be able to add OTP tokens with custom name
On Tue, 2014-09-16 at 12:52 -0400, Simo Sorce wrote: On Tue, 16 Sep 2014 12:33:16 -0400 Nathaniel McCallum npmccal...@redhat.com wrote: On Tue, 2014-09-16 at 12:28 -0400, Simo Sorce wrote: On Tue, 16 Sep 2014 12:23:51 -0400 Nathaniel McCallum npmccal...@redhat.com wrote: On Tue, 2014-09-16 at 12:22 -0400, Simo Sorce wrote: On Tue, 16 Sep 2014 11:52:43 -0400 Nathaniel McCallum npmccal...@redhat.com wrote: On Tue, 2014-09-16 at 17:51 +0200, Petr Vobornik wrote: On 16.9.2014 17:26, Nathaniel McCallum wrote: On Tue, 2014-08-19 at 17:10 -0400, Nathaniel McCallum wrote: Admins need the ability to specify the token ID in the case of imports. However, generally, this ability is not needed. Is it possible to offload the ID generation to the ipa-uuid plugin? I'm not quite sure how to enable this (I think it involves passing a magic value?). But I'm not quite sure how this fits in with the IPA framework as the generated value is the DN. However, assuming this can be used, I propose the following. The token ID is removed from the UI for regular users (but retained for admins). We change the ACIs for token addition/modification to prevent regular users from specifying the ID in an add or mod operation. The CLI would retain the option to set it, but this option would only be usable by admins. Make sense? Nobody has responded to this. :) However, since investigating it a bit more, this approach won't really work without further effort. Here is the problem. First, the UUID plugin doesn't currently support this kind of operation. Either it needs to be modified or a new plugin needs to be created. Second, the client needs to know the ID in order to generate the token URI. If we generate the UUID inside the DS, the UUID is unknown to the client and the URI can't be generated. This would mean a new control. As I see it we have three options: 1. Remove the option to specify the ipatokenUniqueID from the GUI. Don't make any change in the CLI. ENFORCEMENT: none EFFORT: low 2. Perform a server-side check for admin membership. Raise an exception if the ipatokenUniqueID is specified and the user is not an admin. ENFORCEMENT: API-level EFFORT: medium 3. Modify otptoken-add to create tokens with a magical ipatokenUniqueID value by default. An ACI would prevent normal users from adding tokens without this magic value. Create/modify a plugin to generate UUIDs when the magic value is found. Send a control back to the client indicating the real ipatokenUniqueID value. Modify otptoken-add to read this control. ENFORCEMENT: DS-level EFFORT: high I think my preference for now is #1. Thoughts? Nathaniel I want to raise a question if we really want to disable this feature for normal users. Wouldn't it be better to improve error message or check if the name is taken? UUID is the value which is displayed in the Soft. Token application as name. Sure, one can rename it there to match the description in IPA but that seems quite unpleasant to me. Also user has to know that he can rename the token in FreeOTP, Atm the existence check might be little problematic - disclose of information which is not readable to user by default. But current state is basically it, just unfriendly. The existence check is impossible: users can't see tokens they don't own. Why don't we simply allow user to use an arbitrary name of their choosing (or a random string if not specified) and then append their uid string to it in the UI ? I wonder if we have ACIs where the value must match a pattern defined by the bound user ... This would present a strange scenario where a token is created and then reassigned to another user. The idea is to strip away the tail when visualizing to the user, so it wouldn't really bee seen ? Or perhaps the token can simply be renamed when assigned. It just feels like a kludge to me. Also, automated stripping may run afoul of names assigned by the imports. What is wrong with the three options I provided above? We can do #1 now and #2 or #3 later. Nothing wrong, really, #1 is fine by me. I settled on #2 since it turned out to be somewhat easy. I think we also need an update to the UI (aka #1). This should be an easy review:
Re: [Freeipa-devel] [PATCH] 0009 Detect and configure all usable IP addresses.
On 09/16/2014 06:09 PM, Martin Basti wrote: On 16/09/14 15:59, David Kupka wrote: On 09/12/2014 07:24 PM, Martin Basti wrote: snip / Be careful, reviewed on friday! :-) 1) whitespace error + pep8 error patch:76: trailing whitespace. # there is reverse zone for every ip address warning: 1 line adds whitespace errors. ./ipaserver/install/bindinstance.py:640:9: E265 block comment should start with '# ' Stupid mistake, sorry. 2) (server install) +for ip in ip_addresses: +for rev_zone in reverse_zones: +if bindinstance.verify_reverse_zone(rev_zone, str(ip)): +break +else: +sys.exit(1) Please add there error message instead 1 to exit function You are right, it's better to say what's wrong. 3) (server install) Code above, bindinstance.verify_reverse_zone(rev_zone, str(ip)): IMHO this will cause errors (not tested) try: rev-zones: [10.10.10.in-addr.arpa., 16.172.in-addr.arpa.] ip_addreses: [10.10.10.1, 172.16.0.1] it should be any() of zone can handle ip I indented the else branch more that I wanted. 4) (replica-install) I'm not sure about behavior of combination ip addresses, and reverse zones, In original code, if user specify reverse zone, the ip address must match. In patched version, you just add new zones for ip-addresses which doesn't math the user zones. IMO we should check if all addresses belongs to user specified zones, otherwise raise an error. But I'm open to suggestions. The behavior now basically is: Check if IP address matches any specified zone a. if yes we are good b. else search if there is existing zone that can be used ba. if yes we are good bb. else ask user for zone or create default if --unattended 5) Code for ipa-replica-install, and ipa-server-install, differs in parts where we expecting same behavior - ip addresses and reverse zones The behavoir now should be almost the same. The only difference is that when installing replica we need to search for existing reverse zones as they could be added during on another server. 6) https://engineering.redhat.com/irc-services.txt+# there is at least one ip address in every zone +if options.reverse_zones: +for reverse_zone in options.reverse_zones: +for ip in config.ips: -- A +if bindinstance.verify_reverse_zone(reverse_zone, ip): + reverse_zones.append(bindinstance.normalize_zone(reverse_zone)) +break +else: +sys.exit(1) * +# there is reverse zone for every ip address +for ip in config.ips: +for reverse_zone in options.reverse_zones: --- B +if bindinstance.verify_reverse_zone(reverse_zone, ip): +if reverse_zone not in reverse_zones: + reverse_zones.append(bindinstance.normalize_zone(reverse_zone)) +break +else: C +reverse_zone = bindinstance.find_reverse_zone(ip) +if reverse_zone is None and not options.no_reverse: +reverse_zone = util.get_reverse_zone_default(ip) - D1 +if not options.unattended and bindinstance.create_reverse(): - D +reverse_zone = bindinstance.read_reverse_zone(reverse_zone, ip) + reverse_zones.append(bindinstance.normalize_zone(reverse_zone)) - D2 You added all possible zones in step A, B step is not needed. If user doesn't specify reverse_zones you can just jump to C step *add error message C: elif not options.no_reverse D: you asked user if wants to create zone, but you don't care about answer, and in step D2 append zone from D1 note: --no-reverse and --reverse-zones cant be used together, you can use it in new code, test it before cycle Rewritten. 7) # Always use force=True as named is not set up yet add_zone(self.domain, self.zonemgr, dns_backup=self.dns_backup, -ns_hostname=api.env.host, ns_ip_address=nameserver_ip_address, -force=True) + ns_hostname=api.env.host, ns_ip_address=None, force=True) +#for ns_ip_address in nameserver_ip_address: +#add_zone(self.domain, self.zonemgr, dns_backup=self.dns_backup, +#ns_hostname=api.env.host, ns_ip_address=ns_ip_address, +#force=True) Please remove commented section I forget to clean the trash, thanks. You can remove 'ns_ip_address=None' from function 8) +if set(options.ip_addresses) = set(ips): +ips = options.ip_addresses +else: +