WordPress.org

Make WordPress Core

Opened 3 years ago

Closed 16 months ago

#20601 closed defect (bug) (fixed)

Author archives queries should return a 404 when the user isn't a member of the blog

Reported by: yoavf Owned by: markjaquith
Milestone: 3.9 Priority: high
Severity: normal Version:
Component: Bootstrap/Load Keywords: has-patch needs-unit-tests
Focuses: multisite Cc:

Description

On a multi site install, querying for the author page for a user that is not a member of that blog, still returns a valid page, instead of a 404.

Example:
User crazygeorge is a member of of blog1 but not a member of blog2

The url http://example.com/blog2/author/crazygeorge/ will return the author template with a no posts found message.

Insead, we should ignore users that are not member of a specific blog, and return a proper 404.

Attachments (9)

20601.diff (602 bytes) - added by yoavf 3 years ago.
20601.2.diff (860 bytes) - added by nacin 3 years ago.
20601.3.diff (879 bytes) - added by SergeyBiryukov 20 months ago.
20601.4.diff (923 bytes) - added by SergeyBiryukov 19 months ago.
20601.5.diff (938 bytes) - added by SergeyBiryukov 19 months ago.
20601.6.diff (942 bytes) - added by SergeyBiryukov 19 months ago.
20601.7.diff (2.3 KB) - added by wonderboymusic 17 months ago.
20601.8.diff (2.4 KB) - added by markjaquith 16 months ago.
refresh
20601.9.diff (2.4 KB) - added by markjaquith 16 months ago.
fix final unit test

Download all attachments as: .zip

Change History (33)

@yoavf3 years ago

comment:1 @SergeyBiryukov3 years ago

Closed #19612 as a duplicate.

comment:2 @yoavf3 years ago

Thanks @SergeyBiryukov ! (I did search for a similar ticket before but couldn't find any)

comment:3 @nacin3 years ago

  • Keywords needs-patch added; has-patch removed

If an author has posts on a blog, but is not assigned to the blog, this patch would break their author page.

Often, you will see "retired" author accounts being set to the Subscriber role. But in multisite or with a shared user table, the concept of Subscriber is analogous to simply being a user in the table. (Indeed, there has never been a Subscriber role on WP.com.)

I think we need to hook in and consider this a 404 only if no posts get returned. Sounds like something that could be handled in WP::handle_404() rather than in WP_Query.

@nacin3 years ago

comment:4 @nacin3 years ago

  • Keywords has-patch added; needs-patch removed

Possibly easier than I thought. Here's 20601.2.diff.

comment:5 @yoavf3 years ago

Yep, looks like a better solution.

comment:6 follow-up: @obenland3 years ago

Nacin, would this be something to consider for 3.5? It seems rather straight forward.

comment:7 in reply to: ↑ 6 ; follow-ups: @nacin3 years ago

Replying to obenland:

Nacin, would this be something to consider for 3.5? It seems rather straight forward.

I thought this was trunk for a long while. Guess not.

It seems straightforward, but it can fail if is_author() is true, but the queried object is something other than an author, which could occur for more complex queries. like tax + author, etc. We'd be querying is_user_member_of_blog() for the wrong ID. So I'm going to hold off on this for now.

I'd be surprised if a developer on WordPress.com hasn't run into this issue, if this is already deployed there in some form. Or, maybe they have a more resilient fix. (Not sure either way.)

comment:8 in reply to: ↑ 7 ; follow-up: @danielbachhuber3 years ago

Replying to nacin:

I'd be surprised if a developer on WordPress.com hasn't run into this issue, if this is already deployed there in some form. Or, maybe they have a more resilient fix. (Not sure either way.)

It's broken on WordPress.com too :)

comment:9 in reply to: ↑ 8 @nacin3 years ago

Replying to danielbachhuber:

Replying to nacin:

I'd be surprised if a developer on WordPress.com hasn't run into this issue, if this is already deployed there in some form. Or, maybe they have a more resilient fix. (Not sure either way.)

It's broken on WordPress.com too :)

Got it, thanks. I was thinking of #19612.

comment:10 @lancewillett20 months ago

  • Cc lancewillett added
  • Milestone changed from Future Release to 3.8

Could we fix this in 3.8 please?

@SergeyBiryukov20 months ago

comment:11 in reply to: ↑ 7 @SergeyBiryukov20 months ago

Replying to nacin:

It seems straightforward, but it can fail if is_author() is true, but the queried object is something other than an author, which could occur for more complex queries. like tax + author, etc. We'd be querying is_user_member_of_blog() for the wrong ID.

I guess we can use get_query_var( 'author' ) instead of get_queried_object_id().

Version 0, edited 20 months ago by SergeyBiryukov (next)

comment:12 follow-up: @nacin19 months ago

  • Priority changed from normal to high

The 'author' query variable could be a comma-separated list. is_author can also be true for the author_name QV. I would love to fix this but I'm not sure how best to do so.

@SergeyBiryukov19 months ago

comment:13 in reply to: ↑ 12 @SergeyBiryukov19 months ago

Replying to nacin:

The 'author' query variable could be a comma-separated list.

20601.4.diff adds is_int() check.

is_author can also be true for the author_name QV.

author QV is automatically filled in that case: tags/3.7.1/src/wp-includes/query.php#L2538.

comment:14 follow-up: @nacin19 months ago

OK. Well, 'author' can also be a negative integer. :-)

@SergeyBiryukov19 months ago

comment:15 in reply to: ↑ 14 @SergeyBiryukov19 months ago

Replying to nacin:

OK. Well, 'author' can also be a negative integer. :-)

20601.5.diff :)

comment:16 follow-up: @nacin19 months ago

Query variables are strings, so I guess this needs to be is_numeric().

At some point, I guess one of us has to bite the bullet and test this. :-)

It's been broken forever. I have no qualms with punting this, but it's an ugly bug, so would be nice to fix if we have the right fix.

comment:17 @jeremyfelt19 months ago

I haven't tried to break it too hard yet, but Sergey's 20601.5.diff is working for me.

Before:

  1. http://example.com/blog1/author/user2/ -> No posts from (non-member) user2 found on blog1, 200 ok
  2. http://example.com/blog2/author/user2/ -> Posts from (member) user2 on blog2, 200 ok
  3. http://example.com/blog1/author/user3/ -> Posts from (no longer a member) user3 on blog2, 200 ok.

After:

  1. http://example.com/blog1/author/user2/ -> 404 page as expected.
  2. http://example.com/blog2/author/user2/ -> Posts from (member) user2 on blog2, 200 ok
  3. http://example.com/blog1/author/user3/ -> Posts from (no longer a member) user3 on blog2, 200 ok.

@SergeyBiryukov19 months ago

comment:18 in reply to: ↑ 16 @SergeyBiryukov19 months ago

Replying to nacin:

Query variables are strings, so I guess this needs to be is_numeric().

Correct.

comment:19 @lancewillett19 months ago

Does this one need testing or is it ready for commit?

comment:20 @nacin19 months ago

  • Keywords needs-unit-tests 3.9-early added
  • Milestone changed from 3.8 to Future Release

I am not crazy about this. I would be OK if the fix was more straightforward, but it's painfully not. I hesitate to screw something up.

This is admittedly extremely annoying on multisite, but it's been this way forever. Let's try to fix it right in 3.9. I think this is unit-testable, too.

comment:21 @nacin17 months ago

  • Component changed from Template to Bootstrap/Load
  • Focuses multisite added
  • Keywords 3.9-early removed
  • Milestone changed from Future Release to 3.9

Needs unit tests still, if feasible.

@wonderboymusic17 months ago

comment:22 @wonderboymusic17 months ago

In .7.diff, I added a unit test that blows up at the end. Someone should take a look. My test is not a real world scenario, but it exposes some weirdness.

@markjaquith16 months ago

refresh

@markjaquith16 months ago

fix final unit test

comment:23 @markjaquith16 months ago

wonderboymusic — the problem with that last unit test was that you were switching to a blog with different rewrite rules. So the author posts query was coming through as a 404ing Page query. $wp_rewrite->init() after switch_to_blog() sorted that.

comment:24 @markjaquith16 months ago

  • Owner set to markjaquith
  • Resolution set to fixed
  • Status changed from new to closed

In 27290:

Return 404 when querying author's posts who is not a member and has no posts on the site

fixes #20601. props yoavf, nacin, SergeyBiryukov, wonderboymusic, markjaquith.

Note: See TracTickets for help on using tickets.