Make WordPress Core

Opened 12 years ago

Closed 10 years ago

Last modified 8 years 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's profile yoavf Owned by: markjaquith's profile 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 12 years ago.
20601.2.diff (860 bytes) - added by nacin 12 years ago.
20601.3.diff (879 bytes) - added by SergeyBiryukov 10 years ago.
20601.4.diff (923 bytes) - added by SergeyBiryukov 10 years ago.
20601.5.diff (938 bytes) - added by SergeyBiryukov 10 years ago.
20601.6.diff (942 bytes) - added by SergeyBiryukov 10 years ago.
20601.7.diff (2.3 KB) - added by wonderboymusic 10 years ago.
20601.8.diff (2.4 KB) - added by markjaquith 10 years ago.
refresh
20601.9.diff (2.4 KB) - added by markjaquith 10 years ago.
fix final unit test

Download all attachments as: .zip

Change History (34)

@yoavf
12 years ago

#1 @SergeyBiryukov
12 years ago

Closed #19612 as a duplicate.

#2 @yoavf
12 years ago

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

#3 @nacin
12 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.

@nacin
12 years ago

#4 @nacin
12 years ago

  • Keywords has-patch added; needs-patch removed

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

#5 @yoavf
12 years ago

Yep, looks like a better solution.

#6 follow-up: @obenland
11 years ago

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

#7 in reply to: ↑ 6 ; follow-ups: @nacin
11 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.)

#8 in reply to: ↑ 7 ; follow-up: @danielbachhuber
11 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 :)

#9 in reply to: ↑ 8 @nacin
11 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.

#10 @lancewillett
10 years ago

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

Could we fix this in 3.8 please?

#11 in reply to: ↑ 7 @SergeyBiryukov
10 years 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(). Refreshed the patch.

Last edited 10 years ago by SergeyBiryukov (previous) (diff)

#12 follow-up: @nacin
10 years 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.

#13 in reply to: ↑ 12 @SergeyBiryukov
10 years 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.

#14 follow-up: @nacin
10 years ago

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

#15 in reply to: ↑ 14 @SergeyBiryukov
10 years ago

Replying to nacin:

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

20601.5.diff :)

#16 follow-up: @nacin
10 years 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.

#17 @jeremyfelt
10 years 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.

#18 in reply to: ↑ 16 @SergeyBiryukov
10 years ago

Replying to nacin:

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

Correct.

#19 @lancewillett
10 years ago

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

#20 @nacin
10 years 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.

#21 @nacin
10 years 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.

#22 @wonderboymusic
10 years 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.

@markjaquith
10 years ago

refresh

@markjaquith
10 years ago

fix final unit test

#23 @markjaquith
10 years 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.

#24 @markjaquith
10 years 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.

This ticket was mentioned in Slack in #core-restapi by danielbachhuber. View the logs.


8 years ago

Note: See TracTickets for help on using tickets.