Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding email verification support #2469

Closed
10 tasks done
marienfressinaud opened this issue Jul 30, 2019 · 8 comments · Fixed by #2481
Closed
10 tasks done

Adding email verification support #2469

marienfressinaud opened this issue Jul 30, 2019 · 8 comments · Fixed by #2481
Assignees
Milestone

Comments

@marienfressinaud
Copy link
Member

marienfressinaud commented Jul 30, 2019

I'd like to add features to help administrators to manage multi-users instances. I'm currently working on an "email verification" feature but I'm facing an annoying problem.

Context: I need to send emails via SMTP and not using localhost. The only built-in function in PHP to send emails is mail, which needs to be configured via the php.ini file and needs sendmail to be configured. I'm not even sure we can pass credentials (some say no, some others say yes but not documented).

Almost everyone recommends to use PHPMailer and I think we should go with it. Problem: PHPMailer only supports PHP 5.5+, while FreshRSS works with PHP 5.3+ (according to our travis.yml file). My questions are:

  • should we re-evaluate our minimal requirements? Even if some distributions still support these old versions, maybe it's time to move on? (it doesn't mean we will break everything without proper warning to the administrators)
  • if the answer to the previous question is "no", is a solution with "no email validation feature" for the older PHP versions would be ok for you?
  • or maybe we can use "PHPMailer" for PHP 5.5+ and mail for older versions? (but it means more work for me)

Follow-up of my progress:

  • add an email field on the profile page and the CLI (reuse the existing mail_login config element)
  • add a force_email_validation element to system config (false by default) and add a field to the system config page
  • ask for email during registration if force_email_validation is true
  • create a page to validate the user email address (params: username and token)
  • send an email with the URL of the validation page when email is changed (or user create its account)
  • block access to FreshRSS if the email_validation_token is not null
  • allow user to resend the validation email
  • allow user to change its email while she's blocked
  • document the feature
  • (nice to have) improve the look of the "validation feedback page" (maybe using Allow to change the view layout #2467)

(more elements to come while I'm working on it)

@Frenzie
Copy link
Member

Frenzie commented Jul 30, 2019

Fwiw, at the time of writing https://wordpress.org/about/stats/ claims:

  • 5.2 1.7%
  • 5.3 4.1%
  • 5.4 6.9%

And also:

https://wordpress.org/about/requirements/

Note: If you are in a legacy environment where you only have older PHP or MySQL versions, WordPress also works with PHP 5.6.20+ and MySQL 5.0+, but these versions have reached official End Of Life and as such may expose your site to security vulnerabilities.

Since 5.3 was EOL some 6 (?) years ago I think it's fair to bump the requirements for a good reason (like PHPMailer).

@marienfressinaud
Copy link
Member Author

Thanks for the links, that could be very useful! What we might need is a clear rule of how we support the older PHP versions: I suggest that we could drop support if the versions are used by less than 5% of users, but we would stuck with PHP 5.4, which is not supported by PHPMailer… and 10% seems a lot of users for me. So finally my question remains the same :)

@Alkarex
Copy link
Member

Alkarex commented Jul 31, 2019

PHP 5.5 is not yet available even in the newest CentOS https://distrowatch.com/table.php?distribution=CentOS (supported until 2024 😛 - but I do not think we should wait that long!) and has just appeared in May in Red Hat (8).
I think it is fine to have optional features that require newer versions of PHP / database / ... while making more efforts to maintain back-compatibility in the core (that is your option number 2).

@Alkarex Alkarex added this to the 1.15.0 milestone Jul 31, 2019
@Frenzie
Copy link
Member

Frenzie commented Jul 31, 2019

I thought of mentioning CentOS but decided not to. :-)

I agree with @Alkarex that if you put in some extra effort it makes more sense to make the presumably lesser effort of making it say that this mailing business won't work on <5.5 instead of making it actually work. Especially for this use case, which implies greater technical expertise and more available options.

By the way, part of my reason in mentioning Wordpress is that it's fairly conservative, and has been accused of holding back migration to newer versions of PHP. But even so current WP requires at least 5.6 and current Drupal requires at least 5.5.

@marienfressinaud
Copy link
Member Author

Ok, thanks for your insight! I'll go with solution 2 (no email validation if PHP < 5.5). I think I'll have something working pretty soon :)

I think if we want to continue the discussion about the support of PHP, it can go in another ticket if it doesn't exist yet!

@jkinable
Copy link
Contributor

@marienfressinaud very related to this: would it be possible to add a 'forgot password' feature which, virtually identical to this new email address verification feature, sends a url to the user to reset his/her password. Or simply email a new randomly generated password. Given the work you already put into #2481 and #2476 , this should be rather straightforward :).

@marienfressinaud
Copy link
Member Author

Yes, that's in my next plans! :)

@Alkarex
Copy link
Member

Alkarex commented Aug 17, 2019

I am looking again at PHP versions, distributions, etc. and I think it would be reasonable to require PHP 5.5+ for the core of FreshRSS after all.

As one of you Frenzie said, WordPress currently requires PHP 5.6.20+, and it is the most popular PHP application.

We would loose about 20% of the PHP servers according to https://w3techs.com/technologies/details/pl-php/5/all but I expect this number to drop fast after the release of CentOS 8 (CentOS accounts for 17% of Linux servers https://w3techs.com/technologies/details/os-linux/all/all ).

Distributions:

  • no impact on Ubuntu, Fedora, Alpine, OpenWRT, FreeBSD, OpenSuze, Mageia, as all active versions have PHP > 7
  • no impact on Synology, as all active versions have PHP > 5.6
  • we drop Debian 8 Jessie (-2020) - we keep supporting Debian 9 Stretch (2017-06) - current is Debian 10 Buster
  • we drop Red Hat 7 (-2024) - we keep supporting RHEL 8 (2019-05)
  • we drop CentOS 7 (-2024) - we will support CentOS 8 (to be released soonish)

When dropping older versions, I can better like when it is for a good reason, and there is actually one with PHP 5.5, namely generators (yield) https://php.net/language.generators.overview which I consider using. More about that tomorrow - I hope :-)

Alkarex added a commit to Alkarex/FreshRSS that referenced this issue Aug 17, 2019
FreshRSS#2469 (comment)
I think it would be reasonable to require PHP 5.5+ for the core of
FreshRSS after all.

As Frenzie said, WordPress currently requires PHP 5.6.20+, and it is the
most popular PHP application.

We would loose about 20% of the PHP servers according to
https://w3techs.com/technologies/details/pl-php/5/all but I expect this
number to drop fast after the release of CentOS 8 (CentOS accounts for
17% of Linux servers
https://w3techs.com/technologies/details/os-linux/all/all ).

Distributions:
* no impact on Ubuntu, Fedora, Alpine, OpenWRT, FreeBSD, OpenSuze,
Mageia, as all active versions have PHP > 7
* no impact on OpenSuze, Synology, as all active versions have PHP > 5.5
* we drop Debian 8 Jessie (-2020) - we keep supporting Debian 9 Stretch
(2017-06) - current is Debian 10 Buster
* we drop Red Hat 7 (-2024) - we keep supporting RHEL 8 (2019-05)
* we drop CentOS 7 (-2024) - we will support CentOS 8 (to be released
soonish)

When dropping older versions, I can better like when it is for a good
reason, and there is actually one with PHP 5.5, namely generators
(yield) https://php.net/language.generators.overview which I consider
using.
Alkarex added a commit that referenced this issue Aug 20, 2019
* Require PHP 5.5+

#2469 (comment)
I think it would be reasonable to require PHP 5.5+ for the core of
FreshRSS after all.

As Frenzie said, WordPress currently requires PHP 5.6.20+, and it is the
most popular PHP application.

We would loose about 20% of the PHP servers according to
https://w3techs.com/technologies/details/pl-php/5/all but I expect this
number to drop fast after the release of CentOS 8 (CentOS accounts for
17% of Linux servers
https://w3techs.com/technologies/details/os-linux/all/all ).

Distributions:
* no impact on Ubuntu, Fedora, Alpine, OpenWRT, FreeBSD, OpenSuze,
Mageia, as all active versions have PHP > 7
* no impact on OpenSuze, Synology, as all active versions have PHP > 5.5
* we drop Debian 8 Jessie (-2020) - we keep supporting Debian 9 Stretch
(2017-06) - current is Debian 10 Buster
* we drop Red Hat 7 (-2024) - we keep supporting RHEL 8 (2019-05)
* we drop CentOS 7 (-2024) - we will support CentOS 8 (to be released
soonish)

When dropping older versions, I can better like when it is for a good
reason, and there is actually one with PHP 5.5, namely generators
(yield) https://php.net/language.generators.overview which I consider
using.

* Version note for JSON.php

* hex2bin

* Update .travis.yml

Co-Authored-By: Frans de Jonge <fransdejonge@gmail.com>
javerous pushed a commit to javerous/FreshRSS that referenced this issue Jan 20, 2020
* Require PHP 5.5+

FreshRSS#2469 (comment)
I think it would be reasonable to require PHP 5.5+ for the core of
FreshRSS after all.

As Frenzie said, WordPress currently requires PHP 5.6.20+, and it is the
most popular PHP application.

We would loose about 20% of the PHP servers according to
https://w3techs.com/technologies/details/pl-php/5/all but I expect this
number to drop fast after the release of CentOS 8 (CentOS accounts for
17% of Linux servers
https://w3techs.com/technologies/details/os-linux/all/all ).

Distributions:
* no impact on Ubuntu, Fedora, Alpine, OpenWRT, FreeBSD, OpenSuze,
Mageia, as all active versions have PHP > 7
* no impact on OpenSuze, Synology, as all active versions have PHP > 5.5
* we drop Debian 8 Jessie (-2020) - we keep supporting Debian 9 Stretch
(2017-06) - current is Debian 10 Buster
* we drop Red Hat 7 (-2024) - we keep supporting RHEL 8 (2019-05)
* we drop CentOS 7 (-2024) - we will support CentOS 8 (to be released
soonish)

When dropping older versions, I can better like when it is for a good
reason, and there is actually one with PHP 5.5, namely generators
(yield) https://php.net/language.generators.overview which I consider
using.

* Version note for JSON.php

* hex2bin

* Update .travis.yml

Co-Authored-By: Frans de Jonge <fransdejonge@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants