search

silverstripe / silverstripe-installer

The installer for Silverstripe CMS and Framework. Check out this repository to start working with Silverstripe!
http://silverstripe.org/
BSD 3-Clause "New" or "Revised" License
165 stars 163 forks source link

Version indicator in CMS should point to recipe version, not silverstripe/cms module version #276

Closed prbt2016 closed 4 years ago

prbt2016 commented 4 years ago

Overivew

Hello,

I was in the process of installation of Silverstripe CMS 4.5.2 via composer. When i run composer install via this command i.e : php composer.phar create-project silverstripe/installer {{/path/to/stripe/}} 4.5.2. After that I enter db details and admin details in .env file i.e : 


SS_DATABASE_CLASS="MySQLDatabase"
SS_DATABASE_SERVER="localhost"
SS_DATABASE_USERNAME="root"
SS_DATABASE_PASSWORD="testpass"
SS_DATABASE_NAME="test"
SS_DEFAULT_ADMIN_USERNAME="test@test.com"
SS_DEFAULT_ADMIN_PASSWORD="somepass"

and populate the database via this command i.e vendor/bin/sake dev/build. and visit

http://{{URL}}/public/admin and log inside the admin panel.

The version inside is shown as 4.5.1, which is the older version.

silver

What could be the issue ?. Could you please fix this?

ACs/ Implementation notes

PRs

robbieaverill commented 4 years ago

Hi @prbt2016, thanks for the report.

That version is extracted from the CMS module, which is 4.5.1 in the SilverStripe installer 4.5.2 package. It also contains SilverStripe framework 4.5.3.

Perhaps we need to extract this information from recipes (silverstripe/recipe-core or recipe-cms) instead of from the modules, given that we no longer release core modules in lock step with each other.

I agree that this indicator is a little confusing and could be improved.

brynwhyman commented 4 years ago

Agree with you @robbieaverill, silverstripe/recipe-core or recipe-cms version would make much more sense here.

I've updated the title to reflect the work required - thanks for raising. I also feel like the impact is high here. If we're telling CMS users to be on a new security patch release, they need a clear indication of what version their site is currently running. The current way this is set up provides no real workaround to find this information. (I'll change the label, but still open for discussion...)

robbieaverill commented 4 years ago

I've bumped the effort rating too because I think the "module manifest" we use to provide this information may not contain recipes. I haven't verified this theory though.

Using this indicator as part of security release satisfaction is inherently risky, because it only shows the CMS or the framework (or CWP, if you're using that). In this case if you say "there is a security issue in CMS 1.2.3, please upgrade to 1.2.4" then this indicator is correct. If you refer to SilverStripe as a package, you should not refer to security vulnerabilities in it. Advisories target the specific packages that they affect - this indicator also does not include version information for >80% of the SilverStripe packages you install. If you want in-depth version information and security advisories you should use the "silverstripe-maintenance" module with associated addon packages.

I would argue with the above that a "high" impact rating is generous". This label is designed to be informative but not definitive.

brynwhyman commented 4 years ago

I agree that that this shouldn't be treated as a label to fully understand if your site contains a vulnerable version of any module or dependancy. There are better places to look (like the "silverstripe-maintenance" module).

However, I still see this being a very useful tool for CMS users to understand what CMS version they are on and then review release announcements to understand whether their site is NOT on a new version that contains security fixes to core dependancies. I'm assuming that CMS users would see the 'CMS version' as the bundle of dependancies (satisfied by silverstripe/recipe-core or recipe-cms or silverstripe/installer), not the specific silverstripe/cms module.

For example, there's just been a changelog announcement for 4.5.2 which urges people to upgrade to this version to make use of recent security fixes. This label is perfect for alerting the CMS user whether they're on this version. Probably hence this open issue!

maxime-rainville commented 4 years ago

I had a glance at the logic. It doesn't look to complex to fix. We just need to add a bit of extra logic to say we give higher priority to recipe-cms.

One thing to note is that it's totally possible to create a CMS project by requiring each module individually without relying on the recipes. In that scenario, we'll default back to showing the CMS version. If we're OK with that approach, I can make a quick patch.

brynwhyman commented 4 years ago

Sounds good @maxime-rainville!

Cheddam commented 4 years ago

I'm wondering if we should also adjust the logic to also be clearer about which package it's pulling from - so if it manages to pull from recipe-core or recipe-cms, it says Silverstripe CMS Recipe x.x.x and if it falls back to the CMS/Framework module it says something like Silverstripe CMS/Framework x.x.x.

We need to be clearer about the difference between Recipe versions and Module versions in general, so being more specific here seems like a good idea.

maxime-rainville commented 4 years ago

When you over your cursor over the number it will tell you what module the version is based on as a title.

brynwhyman commented 4 years ago

So it looks like the priority would look like this?

Prioritise in the following order:

maxime-rainville commented 4 years ago

FYI the way this is controlled is by the module config flag on SilverStripe\Core\Manifest\VersionProvider. e.g.: https://github.com/silverstripe/cwp-core/blob/2/_config/version.yml

---
Name: cwpversion
After:
  - '#cmsversion'
  - '#coreconfig'
---
SilverStripe\Core\Manifest\VersionProvider:
  modules:
    cwp/cwp-core: 'CWP'

You can have multiple module listed there. The first one is what's displayed at the bottom of the sidebar. The other ones get displayed in the title attribute.

Framework defaults to showing the framework version. The CMS module overrides that to display the CMS version. CWP-core keeps CMS as the default and add itself to the list.

On thing to note is that the current logic doesn't quite work if the module is not installed. For example, you couldn't tell it to look at recipe-cms first and then fallback to CMS. Shouldn't be too hard to fix, but it's an edge case to keep in mind.

emteknetnz commented 4 years ago

Enhanced in https://github.com/silverstripe/silverstripe-framework/pull/9628

Cheddam commented 4 years ago

silverstripe/silverstripe-framework#9628 has been merged, but until additional config is added to silverstripe/recipe-core and silverstripe/recipe-cms, we'll continue to see the silverstripe/cms module version.

emteknetnz commented 4 years ago

PR to add config here https://github.com/silverstripe/silverstripe-cms/pull/2578