On Tue, 2011-10-25 at 15:57 -0400, Rob Crittenden wrote: > Martin Kosek wrote: > > On Mon, 2011-10-24 at 17:08 +0200, Martin Kosek wrote: > >> On Mon, 2011-10-24 at 09:02 -0400, Rob Crittenden wrote: > >>> Martin Kosek wrote: > >>>> On Fri, 2011-10-21 at 11:31 -0400, Rob Crittenden wrote: > >>>>> Martin Kosek wrote: > >>>>>> On Fri, 2011-10-14 at 14:11 -0400, Rob Crittenden wrote: > >>>>>>> Martin Kosek wrote: > >>>>>>>> Do at least a basic validation of DNS zone manager mail address. > >>>>>>>> > >>>>>>>> Do not require '@' to be in the mail address as it is not used > >>>>>>>> in common DNS zone configuration (in bind for example) and people > >>>>>>>> may be used to configure it that way. '@' is always removed by the > >>>>>>>> installer before the DNS zone is created. > >>>>>>>> > >>>>>>>> https://fedorahosted.org/freeipa/ticket/1966 > >>>>>>> > >>>>>>> There is already a zonemgr_callback defined for this option, can the > >>>>>>> verify_zonemgr call be either integrated or called from that? > >>>>>>> > >>>>>>> rob > >>>>>>> > >>>>>> > >>>>>> Right. Please, try this one. I also added a parser error when more than > >>>>>> one '@' is in the checked value. > >>>>>> > >>>>>> Martin > >>>>> > >>>>> A couple of things: > >>>>> > >>>>> In the block where you are counting @ why not add an : > >>>>> > >>>>> else: > >>>>> raise ValueError('address is not fully qualified') > >>>>> > >>>>> rather than looking for '.' in the result? I think it will be clearer > >>>>> that way. I wonder if the error should contain an example as well, are > >>>>> people going to know what a fully-qualified means? > >>>>> > >>>>> The regex is very strict for e-mail addresses, perhaps too much so. It > >>>>> doesn't allow upper-case characters, periods or _, both of which are > >>>>> allowed in login names. A common e-mail format is first.last@domain. > >>>>> > >>>>> rob > >>>> > >>>> I reorganized the validator a little and let people enter _ in the > >>>> domain name. I also added a small explanation of what we mean by > >>>> fully-qualified. > >>>> > >>>> Since we have the zonemgr validator available, why not use it for the > >>>> DNS plugin too? I enhanced the plugin to use this validator too. Please, > >>>> see attached patch. > >>>> > >>>> Martin > >>> > >>> NACK, a client might not have the server sub-package installed so the > >>> import of bindinstance will fail. > >>> > >>> I think that moving the validator into dns.py as a central location > >>> should work though. > >>> > >>> Otherwise looks good. > >>> > >>> rob > >> > >> Thanks. I didn't realize that user can have freeipa-admintools without > >> freeipa-server installed. > >> > >> I placed the validator to ipalib/util.py as we cannot place it to the > >> dns plugin directly as all our plugins assume that we have initialized > >> api. > >> > >> Updated patch attached. > >> > >> Martin > > > > And now also with API.txt change. This patch must be cursed or > > something. > > > > No need to bump API version, just the validator has been added. > > > > Martin > > ACK
Pushed to master. Martin _______________________________________________ Freeipa-devel mailing list [email protected] https://www.redhat.com/mailman/listinfo/freeipa-devel
