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.

Sign up for GitHub

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

Jump to bottom

Font Library: font family and font uploads capability checks #59294

Closed
wants to merge 20 commits into from
Closed

Font Library: font family and font uploads capability checks #59294

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
9e11bd9
Adds meta cap for upload_fonts and gates REST API font uploads
creativecoder Feb 22, 2024
7846623
Shows/hides font upload and install tabs according to user capabilities
creativecoder Feb 22, 2024
314cad6
Prevents deleting fonts if user lacks capability
creativecoder Feb 22, 2024
efa01ec
Fixes comment block
creativecoder Feb 22, 2024
dc57cd4
Display the font collection tabs when user can create font families. …
matiasbenedetto Feb 23, 2024
3da8387
Merge branch 'trunk' into update/font-upload-ui-permissions
creativecoder Feb 23, 2024
84b54ed
Check for font file upload capability before deleting fonts with asso…
creativecoder Feb 23, 2024
5fe3776
Checks wp_font_face post type capability before adding upload_fonts c…
creativecoder Feb 23, 2024
9867c3e
Moves font REST API registration to correct hook
creativecoder Feb 23, 2024
0800644
Commit WIP
anton-vlasenko Feb 24, 2024
ed93188
Fix the test.
anton-vlasenko Feb 24, 2024
047662a
Fix formatting.
anton-vlasenko Feb 24, 2024
7d44a12
Fix code style.
anton-vlasenko Feb 24, 2024
090c7a0
Draft test.
anton-vlasenko Feb 24, 2024
b1fb5db
Commit WIP.
anton-vlasenko Feb 24, 2024
1dc3153
Implement additional test cases for Tests_Fonts_GutenbergDeleteFontPo…
anton-vlasenko Feb 24, 2024
8b1663f
Remove the unused methods.
anton-vlasenko Feb 24, 2024
9925ddd
Fix the unit tests.
anton-vlasenko Feb 27, 2024
1edf1ac
Rename test classes.
anton-vlasenko Feb 27, 2024
742d091
Fix the covers tag.
anton-vlasenko Feb 27, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions lib/compat/wordpress-6.5/fonts/class-wp-rest-font-faces-controller.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -326,6 +326,14 @@ public function create_item( $request ) {
$settings = $request->get_param( 'font_face_settings' );
$file_params = $request->get_file_params();

if ( ! empty( $file_params ) && ! current_user_can( 'upload_fonts' ) ) {
return new WP_Error(
'rest_cannot_upload_fonts',
__( 'You are not allowed to upload font files.', 'gutenberg' ),
array( 'status' => 403 )
);
}

// Check that the necessary font face properties are unique.
$query = new WP_Query(
array(
Expand Down
44 changes: 44 additions & 0 deletions lib/compat/wordpress-6.5/fonts/fonts.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,35 @@ function gutenberg_create_initial_post_types() {
);
}

/**
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The delete_post meta capability will need to account for deleting fonts with files attached. This is to account for the file deletion hooks in wp_delete_post().

I'm not in love with the performance aspect of this code when checking the font family post type but can't think of more performant approach.

Suggested change
/**
/**
* Modify the `delete_post` meta capability for font post types.
*
* For font families and font faces containing attached font files, file
* system access is required by the user in order to delete posts.
*
* @param string[] $caps The primitive capabilities required for the given capability.
* @param string $cap The capability being checked.
* @param int $user_id The user ID.
* @param array $args Context for the capability check.
* @return string[] The modified primitive capabilities required for the given capability.
*/
function gutenberg_delete_font_post_meta_caps( $caps, $cap, $user_id, $args ) {
if ( in_array( 'do_not_allow', $caps, true ) ) {
// It's already known that the user is not allowed to perform the requested capability.
return $caps;
}
if ( 'delete_post' !== $cap ) {
// This filter is only concerned with the 'delete_post' meta capability.
return $caps;
}
$post = get_post( $args[0] );
if ( ! $post ) {
$caps[] = 'do_not_allow';
return $caps;
}
$post_type = get_post_type( $post );
if ( 'wp_font_face' === $post_type ) {
// Are there any font files associated with this font face?
$font_files = get_post_meta( $post->ID, '_wp_font_face_file', false );
if ( empty( $font_files ) ) {
/*
* No font files.
*
* The user can delete the post based on the 'delete_post' meta capability.
*/
return $caps;
}
// The user can only delete the post if they can modify the file system.
$caps[] = 'upload_fonts';
return $caps;
}
if ( 'wp_font_family' === $post_type ) {
// Are there any font faces associated with this font family?
$font_faces = get_children(
array(
'post_parent' => $post->ID,
'post_type' => 'wp_font_face',
)
);
if ( empty( $font_faces ) ) {
/*
* No font faces.
*
* The user can delete the post based on the 'delete_post' meta capability.
*/
return $caps;
}
// If any of the font faces contain files, the user needs to be able to modify the file system.
foreach ( $font_faces as $font_face ) {
$font_files = get_post_meta( $font_face->ID, '_wp_font_face_file', false );
if ( ! empty( $font_files ) ) {
$caps[] = 'upload_fonts';
// File system caps are required, so no need to check further.
break;
}
}
return $caps;
}
// Return existing caps if the post type is not a font family or font face.
return $caps;
}
/**

* Filters the user capabilities to grant the 'upload_fonts' capability as necessary.
*
* To grant the 'upload_fonts' capability, file modifications must be allowed, the fonts directory must be
* writable, and the user must have the 'edit_theme_options' capability.
*
* @since 6.5.0
*
* @param bool[] $allcaps An array of all the user's capabilities.
peterwilsoncc marked this conversation as resolved.
Show resolved Hide resolved
* @return bool[] Filtered array of the user's capabilities.
*/
function gutenberg_maybe_grant_upload_font_cap( $allcaps, $caps ) {
peterwilsoncc marked this conversation as resolved.
Show resolved Hide resolved
if ( ! in_array( 'upload_fonts', $caps, true ) ) {
return $allcaps;
}

$fonts_dir = wp_get_font_dir()['path'];
if (
wp_is_file_mod_allowed( 'can_upload_fonts' ) &&
wp_is_writable( $fonts_dir ) &&
! empty( $allcaps['edit_theme_options'] )
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Developers can modify the cap required for editing the font face/families (similar to the example in #59104 (comment) ) so I am not sure this is required.

canUser( 'create' ... ) should catch this.

However, I'm not entirely sure it is not required, either, so will require a logic check on this.

) {
$allcaps['upload_fonts'] = true;
}

return $allcaps;
}
add_filter( 'user_has_cap', 'gutenberg_maybe_grant_upload_font_cap', 10, 2 );

/**
* Initializes REST routes.
*
Expand Down Expand Up @@ -303,6 +332,21 @@ function _wp_before_delete_font_face( $post_id, $post ) {
add_action( 'before_delete_post', '_wp_before_delete_font_face', 10, 2 );
}

/**
* Filters the block editor settings to enable or disable font uploads according to user capability.
*
* @since 6.5.0
*
* @param array $settings Block editor settings.
* @return array Block editor settings.
*/
function gutenberg_font_uploads_settings( $settings ) {
$settings['fontUploadsEnabled'] = current_user_can( 'upload_fonts' );

return $settings;
}
add_filter( 'block_editor_settings_all', 'gutenberg_font_uploads_settings' );

// @core-merge: Do not merge this back compat function, it is for supporting a legacy font family format only in Gutenberg.
/**
* Convert legacy font family posts to the new format.
Expand Down
17 changes: 15 additions & 2 deletions packages/edit-site/src/components/global-styles/font-library-modal/font-collection.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ import {
import { debounce } from '@wordpress/compose';
import { sprintf, __, _x } from '@wordpress/i18n';
import { search, closeSmall } from '@wordpress/icons';
import { useSelect } from '@wordpress/data';
import { store as editorStore } from '@wordpress/editor';

/**
* Internal dependencies
Expand Down Expand Up @@ -215,6 +217,7 @@ function FontCollection( { slug } ) {
<InstallFooter
handleInstall={ handleInstall }
isDisabled={ fontsToInstall.length === 0 }
selectedFont={ selectedFont }
/>
);
} else if ( ! renderConfirmDialog && totalPages > 1 ) {
Expand Down Expand Up @@ -400,16 +403,26 @@ function PaginationFooter( { page, totalPages, setPage } ) {
);
}

function InstallFooter( { handleInstall, isDisabled } ) {
function InstallFooter( { handleInstall, isDisabled, selectedFont } ) {
const { isInstalling } = useContext( FontLibraryContext );

const fontUploadsEnabled = useSelect( ( select ) => {
const { getEditorSettings } = select( editorStore );
return getEditorSettings().fontUploadsEnabled;
}, [] );

const isInstallButtonDisabled =
isDisabled ||
isInstalling ||
( ! fontUploadsEnabled && selectedFont?.fontFace?.length );

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@matiasbenedetto Since InstallFooter already has the isDisabled prop, would it be cleaner to move this logic into FontCollection and modify the prop, rather than override it here within the component?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, sounds good

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like this is a non-issue after merging from trunk, since the InstallFooter component no longer exists.

return (
<Flex justify="flex-end">
<Button
variant="primary"
onClick={ handleInstall }
isBusy={ isInstalling }
disabled={ isDisabled || isInstalling }
disabled={ isInstallButtonDisabled }
__experimentalIsFocusable
>
{ __( 'Install' ) }
Expand Down
43 changes: 29 additions & 14 deletions packages/edit-site/src/components/global-styles/font-library-modal/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@ import {
Modal,
privateApis as componentsPrivateApis,
} from '@wordpress/components';
import { store as coreStore } from '@wordpress/core-data';
import { useSelect } from '@wordpress/data';
import { store as editorStore } from '@wordpress/editor';
import { useContext } from '@wordpress/element';

/**
Expand All @@ -19,16 +22,15 @@ import { unlock } from '../../../lock-unlock';

const { Tabs } = unlock( componentsPrivateApis );

const DEFAULT_TABS = [
{
id: 'installed-fonts',
title: __( 'Library' ),
},
{
id: 'upload-fonts',
title: __( 'Upload' ),
},
];
const DEFAULT_TAB = {
id: 'installed-fonts',
title: __( 'Library' ),
};

const UPLOAD_TAB = {
id: 'upload-fonts',
title: __( 'Upload' ),
};

const tabsFromCollections = ( collections ) =>
collections.map( ( { slug, name } ) => ( {
Expand All @@ -44,11 +46,24 @@ function FontLibraryModal( {
defaultTabId = 'installed-fonts',
} ) {
const { collections, setNotice } = useContext( FontLibraryContext );
const canUserCreate = useSelect( ( select ) => {
const { canUser } = select( coreStore );
return canUser( 'create', 'font-families' );
}, [] );
const fontUploadsEnabled = useSelect( ( select ) => {
const { getEditorSettings } = select( editorStore );
return getEditorSettings().fontUploadsEnabled;
}, [] );

const tabs = [ DEFAULT_TAB ];

if ( canUserCreate ) {
if ( fontUploadsEnabled ) {
tabs.push( UPLOAD_TAB );
}

const tabs = [
...DEFAULT_TABS,
...tabsFromCollections( collections || [] ),
];
tabs.push( ...tabsFromCollections( collections || [] ) );
}

// Reset notice when new tab is selected.
const onSelect = () => {
Expand Down
20 changes: 19 additions & 1 deletion packages/edit-site/src/components/global-styles/font-library-modal/installed-fonts.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ import {
Spinner,
FlexItem,
} from '@wordpress/components';
import { store as coreStore } from '@wordpress/core-data';
import { useSelect } from '@wordpress/data';

/**
* Internal dependencies
Expand All @@ -38,6 +40,20 @@ function InstalledFonts() {
} = useContext( FontLibraryContext );
const [ isConfirmDeleteOpen, setIsConfirmDeleteOpen ] = useState( false );

const customFontFamilyId =
peterwilsoncc marked this conversation as resolved.
Show resolved Hide resolved
libraryFontSelected?.source === 'custom' && libraryFontSelected?.id;

const canUserDelete = useSelect(
( select ) => {
const { canUser } = select( coreStore );
return (
customFontFamilyId &&
canUser( 'delete', 'font-families', customFontFamilyId )
);
},
[ customFontFamilyId ]
);

const handleUnselectFont = () => {
handleSetLibraryFontSelected( null );
};
Expand Down Expand Up @@ -84,7 +100,9 @@ function InstalledFonts() {
: null;

const shouldDisplayDeleteButton =
!! libraryFontSelected && libraryFontSelected?.source !== 'theme';
!! libraryFontSelected &&
libraryFontSelected?.source !== 'theme' &&
canUserDelete;

useEffect( () => {
handleSelectFont( libraryFontSelected );
Expand Down
2 changes: 2 additions & 0 deletions packages/editor/src/store/defaults.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import { SETTINGS_DEFAULTS } from '@wordpress/block-editor';
* @property {boolean} richEditingEnabled Whether rich editing is enabled or not
* @property {boolean} codeEditingEnabled Whether code editing is enabled or not
* @property {boolean} fontLibraryEnabled Whether the font library is enabled or not.
* @property {boolean} fontUploadsEnabled Whether uploading fonts in the font library is enabled or not.
* @property {boolean} enableCustomFields Whether the WordPress custom fields are enabled or not.
* true = the user has opted to show the Custom Fields panel at the bottom of the editor.
* false = the user has opted to hide the Custom Fields panel at the bottom of the editor.
Expand All @@ -28,6 +29,7 @@ export const EDITOR_SETTINGS_DEFAULTS = {
richEditingEnabled: true,
codeEditingEnabled: true,
fontLibraryEnabled: true,
fontUploadsEnabled: true,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's probably safer to assume false as the default if the filter is removed. That will avoid the UI showing only to display an error if the caps check fails.

enableCustomFields: undefined,
defaultRenderingMode: 'post-only',
};
Loading