matomo-org / matomo

Empowering People Ethically with the leading open source alternative to Google Analytics that gives you full control over your data. Matomo lets you easily collect data from websites & apps and visualise this data and extract insights. Privacy is built-in. Liberating Web Analytics. Star us on Github? +1. And we love Pull Requests!
https://matomo.org/
GNU General Public License v3.0
19.66k stars 2.62k forks source link

Installer reports invalid email address when the email is valid #14475

Closed NightJar closed 5 years ago

NightJar commented 5 years ago

admin@localhost is a valid email address, however the installer will not let me continue with this as the address for the super user. The installer did however accept user@matomo.localhost

This is a test account on a local install in a development environment, however since TLDs were opened up it is no-longer infeasible that someone may have e.g. user@test as their email address (i.e. no .TLD at the end e.g. user@example.com).

On internal (private) networks (e.g. large company intranets) an address without a . in the domain may still be valid.

fdellwing commented 5 years ago

Just my 2 cents on this:

If you open this restriction up too much, the check will do nothing anymore. Maybe try using a method that I learned some years ago for this instead? This will make sure the machine running the code is indeed able to find a valid record for the given domain and is therefore able to send mails to it.

     /**
     * Gets a mail address and validates the domain against it DNS records.
     * <p>
     * See RFC2821 for more background information.
     *
     * @param mail String
     * @return MX, A or AAAA record exists - true<br>otherwise - false
     */
    public Boolean checkMailDomainRecord(String mail) {
        try {
            // According to RFC2821, the domain (A record) can be treated as an
            // MX if no MX records exist for the domain. Also, include a
            // full-stop trailing char so that the default domain of the server
            // is not added automatically
            mail = mail.split("@")[1] + ".";
            Record[] records = new Lookup(mail, Type.MX).run();
            if (records == null) {
                records = new Lookup(mail, Type.A).run();
                if (records == null) {
                    records = new Lookup(mail, Type.AAAA).run();
                    if (records == null) {
                        return false;
                    }
                }
            }
        } catch (Exception ex) {
            Logger.getLogger(UsersBean.class.getName()).log(Level.SEVERE, null, ex);
            return false;
        }
        return true;
    }

(This is obviously Java code and would need to be adapted to PHP)

Findus23 commented 5 years ago

@fdellwing This might add another complexity for people where mailserver and webserver are not configured the same.

I'd propose to stay with the simplest solution (#14476) and add #13533 one day to actually make sure the address is valid (and more importantly the reciever has actually opted into recieving emails).

fdellwing commented 5 years ago

This might add another complexity for people where mailserver and webserver are not configured the same.

I do not understand that point, what does the mailserver has to do with this?

14476 is fine for me, was just a thought as we are running very good with this solution :)

Findus23 commented 5 years ago

I do not understand that point, what does the mailserver has to do with this?

Okay, that example is really contrived, but what if you want to send an e-mail to test@domain.internal that is only resolved on the intranet? And only your SMTP server, but not the webserver is on the intranet?

Maybe a better reason would be that this depends on internet access that many people have disallowed on their webserver.

NightJar commented 5 years ago

Just my 2 cents on this:

If you open this restriction up too much, the check will do nothing anymore. Maybe try using a method that I learned some years ago for this instead? This will make sure the machine running the code is indeed able to find a valid record for the given domain and is therefore able to send mails to it.

An email address is either valid or invalid (according to specification), there is no reason to tie it to an actual registered domain with correctly configured MX records. That is conflating two concepts and would result in shifting the validation of email addresses to validating email address format and DNS.

This might add another complexity for people where mailserver and webserver are not configured the same.

I do not understand that point, what does the mailserver has to do with this?

Nothing in your example, but it does presume that one is set up, in that the administrator has also correctly configured their DNS for that purpose (receiving email).

On my development machine for example, there is no DNS record for the top level domain test, and I run a catcher as SMTP so mail is never even really sent. No DNS entries are looked up, the mail is simply passed to the MTA for processing.

I recommend https://archive.fosdem.org/2018/schedule/event/email_address_quiz/ for some good (and entertaining) viewing material on the topic of validating email addresses.

fdellwing commented 5 years ago

An email address is either valid or invalid (according to specification), there is no reason to tie it to an actual registered domain with correctly configured MX records.

This might be true if you do not rely on emails being correctly received. The better solution is always an email verification, but if that is not possible this is the second best.

It might not be the best here or even necessary, but I do not concur with your opinion :)

NightJar commented 5 years ago

This might be true if you do not rely on emails being correctly received. The better solution is always an email verification, but if that is not possible this is the second best.

It might not be the best here or even necessary, but I do not concur with your opinion :)

I feel I should clarify: I actually agree with you when it comes to verification as you've said. However as you've also said "It might not be the best here" - I'm trying also to clarify that the act of validation and the issue opened here are two separate concerns.

I will agree that if an email address is being verified, then yes, it is important that the address can receive emails. I would whole heartedly agree that a user account should be verified by an email being sent with e.g. a verification link contained within, in where the user can then click to inform the system that the email was received correctly. If that option is unavailable (for unknown reasons) then your method is a good option. I have no problems with this, in that context.

However the issue in context here is only validation, that the address is correctly formatted as to the standard for email addresses.

I feel it important to not conflate these two concepts in this issue.

One might think of this as akin to the difference between authentication, and authorisation. Subtle, but important. Authorisation cannot happen without authentication happening first, in the same way that verification of an email address cannot happen without that address first being valid (otherwise the MTA will error). The system (Matomo) should not care whether or not the address is set up correctly, as a dummy, or mocked via a catcher. It is not of its concern.

It seems from my limited experience that a local install of Matomo does not seem to perform any verification on email addresses supplied, at least not for the super administrator account that is set up during installation. And if this were to be added, then I would suggest that it be a separate issue :)