Display warning in security & setup warnings if php version is EOL #17919

rullzer · 2015-07-28T08:20:14Z

When 8.2 comes out php 5.4 will be End of Life and no longer supported (https://secure.php.net/supported-versions.php). This PR makes sure that we warn admins about this, so they can either upgrade or start complaining to their distro to provider newer packages for PHP. Then at least we did what we could if all of a sudden a huge security exploit in 5.4 is discovered.

CC: @oparoz @DeepDiver1975 @karlitschek @LukasReschke

rullzer · 2015-07-28T08:21:09Z

core/js/setupchecks.js

@@ -69,6 +69,11 @@
 							t('core', '/dev/urandom is not readable by PHP which is highly discouraged for security reasons. Further information can be found in our <a href="{docLink}">documentation</a>.', {docLink: data.securityDocs})
 						);
 					}
+					if(data.phpSupported && data.phpSupported.eol) {
+						messages.push(
+							t('core', 'Your php version ({version}) is no longer supported. See <a href="{phpLink}">php.net</a>. Please upgrade.', {version: data.phpSupported.version, phpLink: 'https://secure.php.net/supported-versions.php'})


Maybe we need to formulate this differently?

Yes, that sentence is a little brash 😆 Also, make sure the capitalisation is correct

Perhaps "Your PHP version {{version}} is end-of-life, and may pose a security risk. We strongly recommend updating to a newer version." No link to php.net necessary IMO.

karlitschek · 2015-07-28T12:05:10Z

Thanks a lot for the effort @rullzer :-)
Unfortunately the decission what is supported by ownCloud is independent from what is supported by the PHP community. Or to use different words: We still have to support 5.4 because it is shipped and supported by key distributions even it support by PHP.net itself is dropped.
So unfortunately 👎 for now
@MTRichards

oparoz · 2015-07-28T12:20:19Z

Hmmm, but all these LTS version have a way to upgrade PHP to something more recent.
@karlitschek, Maybe don't show the warning in the EE edition, since ops will know what to do anyway?

karlitschek · 2015-07-28T12:29:34Z

@oparoz Hmm. Actually no. RHEL lately announced a way to get new php version with support. But others not.
@MTRichards What do you think about PHP 5.4 support ?

oparoz · 2015-07-28T12:32:48Z

Ah. If you need a version supported by the OS vendor itself, then probably not.

RobinMcCorkell · 2015-07-28T12:39:04Z

@karlitschek I see this as a subtle reminder to admins that they should upgrade from a (potentially insecure) PHP version. ownCloud won't stop working with 5.4, it'll just show this warning. And as we've seen on the forums and elsewhere, people tend to look at the Setup Warnings section a lot more than documentation or release notes 😄

rullzer · 2015-07-28T12:56:25Z

@karlitschek Indeed I was not planning on dropping PHP 5.4 support (as I know some distro's only ship 5.4). But we should tell people that the version they are using is not maintained and upstream does not support it any longer.

I see this as a way to get the people that can upgrade to upgrade. So that if a security bugs pops up (and lets face it that happens in PHP) less people will be affected.

Admins running owncloud on a distro that only ship 5.4 probably know this (and know the risks). But then again it might give some people more incentive to complain to their distro.

MTRichards · 2015-07-28T13:13:45Z

Thanks @rullzer - we knew this was coming, good to see the issue here.

My opinion is that an informed admin is a better admin. Assuming an admin knows that PHP 5.x is no longer supported, they can take the proper action for them - stick with overall server OS vendor supported stack, and keep the older version of PHP, or upgrade. At least then they will know.

As this is not about supporting PHP 5.4 with ownCloud for now, since we still support it - rather this is about the challenge of keeping an environment properly updated - I like having a message.

That said, we have to be careful with wording. Current wording makes it sounds like ownCloud don't support your PHP version, that is not quite true. We should consider something like:
"Your PHP version is no longer supported by PHP. We encourage you to upgrade your PHP version to take advantage of performance and security updates provided by PHP."
Or something like that, with the link to PHP.net. Clearly this just shows for admins in the admin panel error message section, I think that is how it is designed above.

MTRichards · 2015-07-28T13:15:05Z

Is it possible to have a new yellow error message, a caution, rather than a red warning for the message? Then we can adopt the standard error message approach: red=showstopper, yellow=informational but still works, black=nothing to worry about?

RobinMcCorkell · 2015-07-28T13:59:41Z

@MTRichards +1 for that, but I fear that's a whole other PR... Perhaps leave this one on standby until we have colours available?

rullzer · 2015-07-28T19:24:58Z

@MTRichards +1 for that, but I fear that's a whole other PR... Perhaps leave this one on standby until we have colours available?

Agreed. I would make the different colors a separate PR. Should not be hard but best to split it.

MTRichards · 2015-07-28T19:29:32Z

Agreed. Just getting the updated warning is all good for me.

MorrisJobke · 2015-07-29T07:04:21Z

settings/controller/checksetupcontroller.php

+			$eol = true;
+		}
+
+		return ['eol' => $eol, 'version' => phpversion()];


This feels so wrong. Use the same way of retrieving the PHP version - either PHP_VERSION or phpversion() but please not mixed.

jikes you are right.. that is dirty. Will fix.

scrutinizer-notifier · 2015-07-29T08:52:40Z

A new inspection was created.

ghost · 2015-07-29T11:34:29Z

🚀 Test PASSed.🚀

MorrisJobke · 2015-07-29T12:44:29Z

Tested and works 👍

DeepDiver1975 · 2015-08-10T21:03:33Z

👍

Display warning in security & setup warnings if php version is EOL

rullzer added enhancement 3 - To Review settings:admin security labels Jul 28, 2015

rullzer added this to the 8.2-current milestone Jul 28, 2015

rullzer reviewed Jul 28, 2015
View reviewed changes

rullzer mentioned this pull request Jul 28, 2015

Drop support for PHP 5.4 with ownCloud 8.2? #16428

Closed

MorrisJobke reviewed Jul 29, 2015
View reviewed changes

Display warning in security & setup warnings if php version is EOL

72ba678

rullzer mentioned this pull request Jul 30, 2015

Settings admin warning levels #17975

Merged

DeepDiver1975 added a commit that referenced this pull request Aug 10, 2015

Merge pull request #17919 from rullzer/php_supported_check

9650f3e

Display warning in security & setup warnings if php version is EOL

DeepDiver1975 merged commit 9650f3e into owncloud:master Aug 10, 2015

rullzer deleted the php_supported_check branch August 11, 2015 06:46

lock bot locked as resolved and limited conversation to collaborators Aug 11, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Display warning in security & setup warnings if php version is EOL #17919

Display warning in security & setup warnings if php version is EOL #17919

rullzer commented Jul 28, 2015

rullzer Jul 28, 2015

RobinMcCorkell Jul 28, 2015

karlitschek commented Jul 28, 2015

oparoz commented Jul 28, 2015

karlitschek commented Jul 28, 2015

oparoz commented Jul 28, 2015

RobinMcCorkell commented Jul 28, 2015

rullzer commented Jul 28, 2015

MTRichards commented Jul 28, 2015

MTRichards commented Jul 28, 2015

RobinMcCorkell commented Jul 28, 2015

rullzer commented Jul 28, 2015

MTRichards commented Jul 28, 2015

MorrisJobke Jul 29, 2015

rullzer Jul 29, 2015

scrutinizer-notifier commented Jul 29, 2015

ghost commented Jul 29, 2015

MorrisJobke commented Jul 29, 2015

DeepDiver1975 commented Aug 10, 2015

Display warning in security & setup warnings if php version is EOL #17919

Display warning in security & setup warnings if php version is EOL #17919

Conversation

rullzer commented Jul 28, 2015

rullzer Jul 28, 2015

Choose a reason for hiding this comment

RobinMcCorkell Jul 28, 2015

Choose a reason for hiding this comment

karlitschek commented Jul 28, 2015

oparoz commented Jul 28, 2015

karlitschek commented Jul 28, 2015

oparoz commented Jul 28, 2015

RobinMcCorkell commented Jul 28, 2015

rullzer commented Jul 28, 2015

MTRichards commented Jul 28, 2015

MTRichards commented Jul 28, 2015

RobinMcCorkell commented Jul 28, 2015

rullzer commented Jul 28, 2015

MTRichards commented Jul 28, 2015

MorrisJobke Jul 29, 2015

Choose a reason for hiding this comment

rullzer Jul 29, 2015

Choose a reason for hiding this comment

scrutinizer-notifier commented Jul 29, 2015

ghost commented Jul 29, 2015

MorrisJobke commented Jul 29, 2015

DeepDiver1975 commented Aug 10, 2015