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.

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

Fix author information leakage by author blocks for Custom Post Types without author support & display notice to user #67136

Merged
merged 7 commits into from
Dec 18, 2024

Conversation

sarthaknagoshe2002
Copy link
Contributor

@sarthaknagoshe2002 sarthaknagoshe2002 commented Nov 19, 2024

fixes: #65816

What?

This PR updates the behavior of author blocks to ensure they do not render when the post belongs to a Custom Post Type (CPT) that does not explicitly declare support for the author feature.

Why?

Author blocks displaying on CPTs without declared author support is inconsistent with WordPress behavior.
This can lead to unintentional data exposure, as WordPress still saves author information to the database even when it is not intended to be visible.
Ensures better security, consistency, and adherence to user expectations for CPTs.

How?

  • Added a conditional check in the rendering logic of the author blocks to verify if the CPT supports the author feature.
  • If the feature is unsupported, a message is displayed in the editor notifying users that the block will not render on the frontend.
  • Updated the frontend output to prevent leaking author information when the CPT does not support authors.

Testing Instructions

Reproduction:-

  1. Create a Custom Post Type (CPT) without declaring support for the author feature:
add_action( 'init', function() {
    register_post_type( 'custom-post', array(
        'public' => true,
        'supports' => array( 'title', 'editor', 'custom-fields' ),
    ) );
} );
  1. Add a post to the newly created CPT.
  2. Add an author-related block (e.g., Post Author) in the block editor for that CPT post.
  3. View the post on the frontend.

Expected Outcome:-

  1. In the Editor:

    • A message is displayed informing users that the author block will not render because the CPT does not support the author feature.
  2. On the Frontend:

    • The author block does not render, and no author information is displayed.

Screenshots or screencast

author.blocks.mov

The suggestion given by @dhruvang21 is also used in this PR from the comment

…pported Custom Post Types & show notice to user
Copy link

github-actions bot commented Nov 19, 2024

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: sarthaknagoshe2002 <sarthaknagoshe2002@git.wordpress.org>
Co-authored-by: Mamaduka <mamaduka@git.wordpress.org>
Co-authored-by: fabiankaegy <fabiankaegy@git.wordpress.org>
Co-authored-by: jameskoster <jameskoster@git.wordpress.org>
Co-authored-by: dhruvang21 <dhruvang21@git.wordpress.org>
Co-authored-by: groenroos <groenroos@git.wordpress.org>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

@Mamaduka Mamaduka added [Block] Post Author Affects the Post Author Block [Block] Author Name Affects the Author Name Block [Type] Bug An existing feature does not function as intended labels Nov 20, 2024
@Mamaduka
Copy link
Member

Thanks for contributing, @sarthaknagoshe2002!

I'm unsure if we need to modify the block's edit components. People with access to the editors can also access details about post type author.

If it's decided to display a notice in edit components, it should probably be a part of rendered content and not an editor notice.

cc @WordPress/gutenberg-design

@sarthaknagoshe2002
Copy link
Contributor Author

If it's decided to display a notice in edit components, it should probably be a part of rendered content and not an editor notice.

@Mamaduka Can you elaborate more on this?

@Mamaduka
Copy link
Member

The blocks could render static text/warning in the editor when the post type doesn't support authors. But as I mentioned, I'm unsure if that warning makes sense in the editor context.

Rought Example

CleanShot 2024-11-20 at 19 21 46

@sarthaknagoshe2002
Copy link
Contributor Author

@Mamaduka I have implemented your suggestion now.
In future if we conclude that the warning makes sense in the editor context then we can proceed with the merge.

Screenshot

Screenshot 2024-11-21 at 12 34 10 AM

@sarthaknagoshe2002
Copy link
Contributor Author

I believe that the warning definitely makes sense in the editor context since displaying warnings/errors on the frontend doesn't seem appropriate.

Also, as you suggested static warning makes sense too.

@sarthaknagoshe2002
Copy link
Contributor Author

@Mamaduka Any updates on how we should proceed with this PR?

Copy link
Member

@Mamaduka Mamaduka left a comment

Choose a reason for hiding this comment

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

@sarthaknagoshe2002, I left a couple of suggestions. Still looking for design feedback before final approval.

P.S. There's also a proposal to deprecate Post Author block - #53427, but the information leaking issue also applies to deprecated blocks.

packages/block-library/src/post-author-name/edit.js Outdated Show resolved Hide resolved
packages/block-library/src/post-author-name/edit.js Outdated Show resolved Hide resolved
packages/block-library/src/post-author/edit.js Outdated Show resolved Hide resolved
packages/block-library/src/post-author/edit.js Outdated Show resolved Hide resolved
@jameskoster
Copy link
Contributor

If it's decided to display a notice in edit components, it should probably be a part of rendered content and not an editor notice.

I agree with this, assuming you mean these inline warning notices (obviously with a different message):

Screenshot 2024-12-17 at 14 22 56

Something like:

This post type ($post_type) does not support Authors. [Remove block]

Additionally, should those blocks be hidden in the Inserter when editing a template associated with a post type that doesn't support them?

@Mamaduka
Copy link
Member

Thanks, @jameskoster!

Yes, the block editor warning component can work in this case.

Additionally, should those blocks be hidden in the Inserter when editing a template associated with a post type that doesn't support them?

Generally, a template-aware inserter would be cool, but hiding blocks without context as to why they're hidden can be confusing.

@sarthaknagoshe2002
Copy link
Contributor Author

@fabiankaegy @Mamaduka I’ve addressed the feedback.

@jameskoster I’ve updated the message to This post type ($post_type) does not support Authors. However, I’m unsure how to implement the [Remove block] functionality. Could you please guide me on this?

@jameskoster
Copy link
Contributor

I'm not actually sure myself, sorry. Maybe @Mamaduka knows 🤞

@fabiankaegy
Copy link
Member

@sarthaknagoshe2002 you can remove a block like so:

const BlockEdit = (props) => {
    const { clientId } = props;
    const { removeBlock } = useDispatch( blockEditorStore );
    
    return (
        <Button onClick={() => removeBlock(clientId)} isDestructive>Remove</Button>
    );
}

Copy link
Member

@Mamaduka Mamaduka left a comment

Choose a reason for hiding this comment

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

I think there's no need to use the warning component and separate the removal action. Why?

  • The severity of missing author support is negligible. It won't break or affect anything on the front end. You don't need the Waning component to grab the user's attention.
  • Users can use toolbar options to remove the block.

@sarthaknagoshe2002 left minor suggestions regarding translation comments and string. After that, I think we're good to merge.

Screenshot

CleanShot 2024-12-18 at 18 21 54

packages/block-library/src/post-author/edit.js Outdated Show resolved Hide resolved
packages/block-library/src/post-author-name/edit.js Outdated Show resolved Hide resolved
@Mamaduka
Copy link
Member

@fabiankaegy, the block editor component will receive the onRemove callback via props. But as I mentioned in my previous comment, I think there's no need to implement it.

onRemove={ canRemove ? onRemove : undefined }

@fabiankaegy
Copy link
Member

@fabiankaegy, the block editor component will receive the onRemove callback via props. But as I mentioned in my previous comment, I think there's no need to implement it.

onRemove={ canRemove ? onRemove : undefined }

🤯 very good to know thank you :)

@sarthaknagoshe2002
Copy link
Contributor Author

@Mamaduka I’ve addressed the changes, and I believe this is now ready to be merged.

@Mamaduka Mamaduka merged commit e9bd750 into WordPress:trunk Dec 18, 2024
62 checks passed
@github-actions github-actions bot added this to the Gutenberg 20.0 milestone Dec 18, 2024
@sarthaknagoshe2002 sarthaknagoshe2002 deleted the fix/issue-65816 branch December 18, 2024 19:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Author Name Affects the Author Name Block [Block] Post Author Affects the Post Author Block [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Author blocks display author information for CPT that does not support authors
4 participants