Font upload: check file mods permission and fall back to remote font urls #59104
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
creativecoder
added
[Type] Bug
|
This pull request has changed or added PHP files. Please confirm whether these changes need to be synced to WordPress Core, and therefore featured in the next release of WordPress. If so, it is recommended to create a new Trac ticket and submit a pull request to the WordPress Core Github repository soon after this pull request is merged. If you're unsure, you can always ask for help in the #core-editor channel in WordPress Slack. Thank you! ❤️ View changed files❔ lib/compat/wordpress-6.5/fonts/class-wp-rest-font-faces-controller.php ❔ lib/compat/wordpress-6.5/fonts/fonts.php |
|
Size Change: +54 B (0%) Total Size: 1.7 MB
ℹ️ View Unchanged
|
|
Flaky tests detected in 2e1d6c6. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/8003031365
|
|
As mentioned on the ticket, the existing filter can be used to manage this via add_filter(
'file_mod_allowed',
function ( $allowed, $context ) {
if ( 'can_modify_font_faces' === $context ) {
return true;
}
return $allowed;
},
10,
2
);The above code will allow uploads and deletions without any changes to #58957. |
There was a problem hiding this comment.
Added a couple of quick notes.
For the use of capabilities for uploading, reviewing map_meta_cap's handling of language files will be a good start. There's a filter of the same name you can use for fonts.php
When working on the other PR for caps, I needed to test against the font-families resource as there isn't an equivalent of current_user_can that handles post types and the font-faces post type is a sub-resource.
Comment on lines
+921
to
+924
| protected function can_upload_fonts() { | ||
| $fonts_dir = wp_get_font_dir()['path']; | ||
| return wp_is_file_mod_allowed( 'can_upload_fonts' ) && wp_is_writable( $fonts_dir ); | ||
| } |
There was a problem hiding this comment.
This should use a current_user_can() check rather than calling wp_is_file_mod_allowed() directly. This will allow plugin developers to avoid having to reproduce this code each time.
The read_post cap is inconsistent and can lead to some messy code as a result.
| @@ -44,9 +47,15 @@ function FontLibraryModal( { | |||
| initialTabId = 'installed-fonts', | |||
| } ) { | |||
| const { collections, setNotice } = useContext( FontLibraryContext ); | |||
| const fontUploadEnabled = useSelect( | |||
There was a problem hiding this comment.
This will still need a canUser( 'create', 'font-families' ); check to account for developers disabling the creating of the post type via roles and caps. As will delete post, edit post, etc.
add_filter( 'register_post_type_args', function( $args, $name ) {
// if post type == font-families, font-faces
$args['capabilities']['create_posts'] = 'do_not_allow';
return $args;
}, 10, 2 ); 
getdave
force-pushed
the
try/font-face-remote-url-when-upload-not-allowed
branch
from
2fecbf1
to
2e1d6c6
Compare
This was referenced Feb 22, 2024
Closed
|
Closing in favor of #59294 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Related to #55280
What?
DISALLOW_FILE_MODSis set to true or the fonts dir is not writableWhy?
Makes it possible to use font collections when the site does not allow file modification
How?
fontUploadEnabledsetting to the site editor that is filtered to check the value ofDISALLOW_FILE_MODSand whether the font dir is writablefontUploadEnabledis truefontUploadEnabledis true, otherwise use the remote font urlTesting Instructions
Important: Test with WP 6.4.3 to ensure all PHP files are loaded from the Gutenberg plugin
define( 'DISALLOW_FILE_MODS', true );in wp-config.php or remove write permissions fromwp-content/fontsTest the API permissions
fontUploadEnabledsetting (see below)