WordPress.org

Make WordPress Core

Opened 6 years ago

Last modified 11 months ago

#25219 new enhancement

DISALLOW_FILE_MODS shouldn't remove update notifications

Reported by: iandunn Owned by:
Milestone: Future Release Priority: normal
Severity: normal Version:
Component: Upgrade/Install Keywords: ui-feedback needs-patch needs-refresh
Focuses: ui Cc:

Description

I think there are valid use cases where an admin would want to set DISALLOW_FILE_MODS, but still want to get the notifications when core/plugins/theme updates are available.

Instead of using the built-in updater, some installs are setup to use svn:externals (or Git submodules) for updates, and others prefer to use wp-cli. In those cases, it's still very useful to get the notifications, because without them an admin has to remember to manually check for updated. That inevitably leads to situations where important security updates are missed for weeks or months, which makes the site vulnerable.

I understand the logic behind removing the notifications -- because the admin can't actually take action on them through WordPress -- but I think that incorrectly assumes that the notices have no purpose if they can't be acted on from inside WordPress. The notifications are still very useful, even if the admin chooses a different method of actually installing the updates.

My proposed solution is to introduce new meta capabilities for view_core_updates, view_plugin_updates, and view_theme_updates. This would add some granularity to the current approach, so we could distinguish between being able to know that updates are available, and being able to install them.

The new meta caps would default to manage_options, so that all administrators could see them, regardless of DISALLOW_FILE_MODS. If that's undesirable, though, then they could map to their corresponding meta caps (update_core, etc) instead (and then be overridden via the map_meta_cap filter in order to enable the notifications).

Attachments (9)

25219.diff (1.6 KB) - added by iandunn 6 years ago.
25219.2.diff (7.0 KB) - added by iandunn 6 years ago.
25219.3.diff (22.0 KB) - added by iandunn 6 years ago.
plugin-theme-update-tables.png (5.0 KB) - added by iandunn 6 years ago.
25219.4.diff (22.6 KB) - added by iandunn 5 years ago.
Refresh
25219.5.diff (23.4 KB) - added by kraftner 5 years ago.
updates-20180129.png (394.8 KB) - added by boemedia 16 months ago.
file-mods-enabled.png (22.4 KB) - added by sfussenegger 11 months ago.
plugins.php with DISALLOW_FILE_MODS=false
file-mods-disabled.png (12.8 KB) - added by sfussenegger 11 months ago.
plugins.php with DISALLOW_FILE_MODS=true

Download all attachments as: .zip

Change History (32)

@iandunn
6 years ago

#1 @iandunn
6 years ago

  • Keywords has-patch added

I've attached a proof of concept that enables an admin to view plugin notifications in the Admin Bar and on wp-admin/plugins.php. If this looks good to everyone, I can expand it to include the rest of the notifications.

#2 @SergeyBiryukov
6 years ago

  • Component changed from Warnings/Notices to Upgrade/Install

#3 @johnbillion
6 years ago

  • Cc johnbillion added

#4 @dd32
6 years ago

I don't think adding a new meta_cap is really needed honestly.

I'd just remove the cap check in wp_plugin_update_rows() myself, the wp_plugin_update_row() function includes a cap check which either notifies about the update, or notifies and links to a update depending on the users auth.
This check in particular was added to replace a if ( is_multisite() && !is_super_admin() ) check in [12753], it seems that the previous check was more appropriate IMHO.

The Core update nag check also tests if ( is_multisite() && !current_user_can('update_core') ) which I also feel should switch to nagging super admins..

The Dashboard -> Updates page is a bit newer though, and allowing access to that would be a little more interesting.

#5 @SergeyBiryukov
6 years ago

  • Milestone changed from Awaiting Review to 3.7

#6 @nacin
6 years ago

I've been meaning to do this for years. I don't hate the idea of a meta capability for this. It would not be terrible to allow for some sort of fine-grained control. On one hand, I hate the idea of being able to specifically hide update nags; on the other hand, there are some decent use cases for doing so.

#7 @nacin
6 years ago

  • Type changed from enhancement to task (blessed)

@iandunn
6 years ago

#8 @iandunn
6 years ago

Ok, I think I got them all in 25219.2.diff.

There are are few places where we might want to change the text based on if the user has view_[core|plugin|theme]_updates but not update_[core|plugins|themes], though. For example, the footer text says "Get Version 3.6.1" and links to wp-admin/update-core.php. Someone with view_core_updates can open that page, but can't run the updater, so maybe something like "View Available Updates" would be more appropriate. Thoughts on that?

#9 @nacin
6 years ago

  • Keywords needs-patch added; has-patch removed
  • Milestone changed from 3.7 to Future Release
  • Type changed from task (blessed) to enhancement

I like the patch, but there's more to do here. Because being able to see a plugin update is currently tied to being able to execute a plugin update, the UI needs to change. We still need to do things like suppress buttons and other actions.

Automatic background updates have their own controls, so this isn't a super priority at the moment.

#10 @iandunn
6 years ago

I've attached another pass on it to improve the UI when updates are available but can't be installed through WP. Some of it is just changing the text, but there were a few visual changes too:

  • Dashboard > Home > Right Now - replaced 'Download' button with '...please update now' text.
  • Dashboard > Updates - removed header/footer from plugin/theme tables, since it has no purpose.

Any design feedback/suggestions to improve those? I've attached a screenshot of the plugin/theme tables on the Updates screen, which can probably be made a lot better. Maybe splitting the version/compatibility/etc info into columns so that the header/footer can return?

@iandunn
6 years ago

@iandunn
5 years ago

Refresh

#11 @iandunn
5 years ago

  • Focuses ui added
  • Keywords has-patch ui-feedback added; needs-patch removed

I refreshed the patch and would love to get some feedback on the UI.

#12 @kraftner
5 years ago

I tried to fix an UI issue where the red background/line on the edge of plugins to be updated was missing.

@kraftner
5 years ago

#13 @johnbillion
5 years ago

I like this. 25219.5.diff looks pretty comprehensive. Couple of points:

  • The change to the cap check and message at the top of wp-admin/plugin-install.php looks incorrect. I don't think this needs to change.
  • Is there a more appropriate cap that we can map the meta caps to, instead of manage_options?

#14 @kraftner
5 years ago

The reason for the change in wp-admin/plugin-install.php seems to be that the iframe showing the changelog is served through this file. Without the change the link is there, but if you want to have a look at the changelog you get a "You do not have sufficient permissions to install plugins on this site." error in the iframe.

But this might hint to needing another check to see if this might have unwanted side effects.

Another thing I just noticed is that the Plugins-submenu shows the "Add New" item with the patch applied. This probably isn't wanted either.

#15 @chriscct7
3 years ago

  • Keywords needs-patch added; has-patch removed

#16 @archon810
3 years ago

I was stumped by not seeing the plugin updates anymore after a migration, and finally another thread led me here. I wouldn't have even suspected DISALLOW_FILE_MODS as the culprit in a million years. Is there resolution in sight?

#17 follow-up: @ryanhellyer
3 years ago

This plugin solves the primary issue here. Just dump the file in your mu-plugins folder (to avoid rogue admins deactivating it).

https://gist.github.com/jaydansand/2e41490a8b040a199db4

Last edited 3 years ago by ryanhellyer (previous) (diff)

#18 in reply to: ↑ 17 @iandunn
3 years ago

Replying to ryanhellyer:

This plugin solves the primary issue here

I think that's a good workaround, but I'm not sure I'd consider it a complete solution. It reintroduces all of the UI for installing updates, but then if someone tries, they'd get a cannot remove old plugin error.

Some of that can probably be mitigated by extending the snippet to hide the UI with CSS, or deny other caps that are necessary for that screen to appear in menus, etc, but it still feels a bit clunky.

There's also the question of whether or not admins should be required to setup a workaround or anything in the first place. Given how important it is to install updates, it might be worth adding the more granular capabilities, just for the extra nudge that would give to admins.

#19 @lukecavanagh
2 years ago

  • Keywords needs-refresh added

Did not even know about this issue until reading a related post about it.

#20 @johnbillion
2 years ago

#40824 was marked as a duplicate.

This ticket was mentioned in Slack in #design by melchoyce. View the logs.


16 months ago

#22 @boemedia
16 months ago

Hey @iandunn ,

We discussed this ticket in the design ticket triage tonight. But before we can give you some feedback, we would like to see a screenshot before and after activating DISALLOW_FILE_MODS. If you could annotate what information goes missing, we can discuss this ticket again. For clarification, I added a screenshot of the place in WP-admin we think we're talking about.

updates-20180129.png

Last edited 16 months ago by boemedia (previous) (diff)

@sfussenegger
11 months ago

plugins.php with DISALLOW_FILE_MODS=false

@sfussenegger
11 months ago

plugins.php with DISALLOW_FILE_MODS=true

#23 @sfussenegger
11 months ago

I've added two screenshots with DISALLOW_FILE_MODS=false and DISALLOW_FILE_MODS=true respectively.

I think the idea is to leave the "There is a new version of ... available" message and only remove the "or update now" link at the end.

Note: See TracTickets for help on using tickets.