Allow unauthenticated user to view a customize-draft post via REST API #117

miina · 2017-02-16T16:22:03Z

Fixes #32

Allows unauthenticated user read a customize-draft post in case it's with status publish within the Snapshot/Changeset.

coveralls · 2017-02-16T16:28:55Z

Coverage decreased (-26.8%) to 50.152% when pulling e9576ba on feature/grant-unauth-users-api-read-permissions into cd5a18f on develop.

coveralls · 2017-02-16T16:28:56Z

Coverage decreased (-26.8%) to 50.152% when pulling e9576ba on feature/grant-unauth-users-api-read-permissions into cd5a18f on develop.

coveralls · 2017-02-16T16:28:56Z

Coverage decreased (-26.8%) to 50.152% when pulling e9576ba on feature/grant-unauth-users-api-read-permissions into cd5a18f on develop.

coveralls · 2017-02-16T16:39:41Z

Coverage increased (+0.2%) to 77.103% when pulling 12051c7 on feature/grant-unauth-users-api-read-permissions into cd5a18f on develop.

westonruter · 2017-02-18T23:07:24Z

php/class-customize-snapshot-manager.php

+			return $allcaps;
+		}
+
+		$post = get_post( absint( $args[2] ) );


Question: Why absint? Should be guaranteed to already be a post ID. If it isn't, then the previous if should check for it.

Yes, it seems to be guaranteed, good point. Added that automatically, will remove.

westonruter · 2017-02-18T23:08:45Z

php/class-customize-snapshot-manager.php

+		if ( ! $chageset_id ) {
+			return $allcaps;
+		}
+		$data = $this->post_type->get_post_content( get_post( absint( $chageset_id ) ) );


Mispelling in chageset_id.

westonruter · 2017-02-18T23:10:40Z

php/class-customize-snapshot-manager.php

+		if ( isset( $data[ $key ] ) ) {
+			$changeset_post_values = $data[ $key ]['value'];
+			if ( isset( $changeset_post_values['post_status'] ) ) {
+				$is_published = 'publish' === $changeset_post_values['post_status'];


What if the post is private and the current user can read private posts?

Unauthenticated user shouldn't have the ability to read private posts by default. Are you thinking of a case when reading private posts permission is given somewhere else to the unauthenticated user via a filter? Maybe it should be handled in the place where that's added then? Or in which cases would unauthenticated user have the permission to read private posts?

I'm thinking of when a less-privileged authenticated user is making an API request. Consider an editor user making an authenticated request to the API. They should be able to see a post which is in the DB as an draft but which is private in the customized state.

westonruter · 2017-02-18T23:12:21Z

tests/php/test-class-customize-snapshot-manager.php

+			'0' => 'read_post',
+			'2' => $post_id,
+		);
+		$allcaps = $this->manager->filter_user_has_cap( array(), $caps, $args );


Instead of using the filter_user_has_cap method directly, should this instead use current_user_can to ensure that the filter is applying?

coveralls · 2017-02-20T19:41:28Z

Coverage increased (+0.2%) to 77.103% when pulling 2331edd on feature/grant-unauth-users-api-read-permissions into cd5a18f on develop.

westonruter · 2017-02-20T21:41:31Z

php/class-customize-snapshot-manager.php

+
+		if (
+			! $this->current_snapshot_uuid
+			|| 0 !== $current_user->ID


I think the check for current user being 0 should be removed because it should also work for under-privileged users making authenticated requests to read posts that normally they wouldn't be able to read, except that the post is made readable due to the status in the customized state.

Consider an author user making an authenticated API request. They should be able to access a post that is draft in the DB but publish in the customized state.

westonruter · 2017-02-20T21:41:47Z

php/class-customize-snapshot-manager.php

+			! $this->current_snapshot_uuid
+			|| 0 !== $current_user->ID
+			|| ! isset( $args[2] )
+			|| 'read_post' !== $args['0']


Why is 0 a string here?

westonruter · 2017-02-20T21:45:03Z

php/class-customize-snapshot-manager.php

+
+		if ( true === $is_published ) {
+			$allcaps[ $caps[0] ] = true;
+		}


You might want to change the condition here to be:

$allcaps[ $caps[0] ] = $is_published;

This could potentially handle the reverse condition where access to a post could be revoked if if its status is changed from publish to private and the user is not authorized to view private posts. There's probably more to it than that, but this is what came to mind.

coveralls · 2017-02-21T15:23:21Z

Coverage increased (+0.1%) to 77.058% when pulling 1e9b8e7 on feature/grant-unauth-users-api-read-permissions into cd5a18f on develop.

westonruter · 2017-02-23T07:08:35Z

tests/php/test-class-customize-snapshot-manager.php

+		$args = array(
+			'0' => 'read_post',
+			'2' => $post_id,
+		);


These variables, $args and $caps aren't being used?

westonruter · 2017-02-23T07:12:12Z

tests/php/test-class-customize-snapshot-manager.php

+		$post_id = $this->factory()->post->create( array(
+			'post_type' => 'post',
+			'post_status' => 'auto-draft',
+		) );


Can there be other posts here including ones that have the draft, private, and publish statuses? Then below in the changeset data these posts can have the post_status overridden to for example reverse the statuses. Then at the end it can set the current user to anonymous, to an author, and to an administrator and for each then the read_post cap check can be verified to return the proper result for the customized state?

On it. For some reason checking for read_post cap causes failure (fatal error) in unit tests, haven't figured out why yet, that's why the delay in fixing.

…th-users-api-read-permissions

coveralls · 2017-03-02T20:23:41Z

Changes Unknown when pulling abefb00 on feature/grant-unauth-users-api-read-permissions into ** on develop**.

coveralls · 2017-03-16T21:01:24Z

Coverage increased (+0.2%) to 76.756% when pulling 9ad389a on feature/grant-unauth-users-api-read-permissions into c7b9124 on develop.

valendesigns · 2017-08-23T01:03:37Z

@miina Please fix merge conflicts so I can review. Thanks!

coveralls · 2017-08-24T17:29:09Z

Coverage increased (+0.2%) to 75.075% when pulling 7d0da3e on feature/grant-unauth-users-api-read-permissions into 1784a74 on develop.

coveralls · 2017-08-24T17:29:09Z

Coverage increased (+0.2%) to 75.075% when pulling 7d0da3e on feature/grant-unauth-users-api-read-permissions into 1784a74 on develop.

Add unit tests. Grant read permission to unauthenticated user.

e9576ba

Mark unit test incompatible.

12051c7

westonruter suggested changes Feb 18, 2017

View reviewed changes

Fix typos. Change test logic.

2331edd

westonruter reviewed Feb 20, 2017

View reviewed changes

Allow anyone view published posts. Add private post check.

1e9b8e7

westonruter reviewed Feb 23, 2017

View reviewed changes

miina changed the title ~~Allow unauthenticated user to view a customize-draft post via REST API~~ [WIP] Allow unauthenticated user to view a customize-draft post via REST API Feb 24, 2017

miina added 3 commits February 24, 2017 11:30

Improve tests. Fix some errors.

0e1c1f8

Merge remote-tracking branch 'origin/develop' into feature/grant-unau…

bc38620

…th-users-api-read-permissions

Add todo notes.

abefb00

westonruter modified the milestone: Next Major Release Mar 10, 2017

miina added 3 commits March 16, 2017 14:06

Resolve merge conflicts.

4251725

Fix phpcs.

c153ada

Fix tests.

9ad389a

miina changed the title ~~[WIP] Allow unauthenticated user to view a customize-draft post via REST API~~ Allow unauthenticated user to view a customize-draft post via REST API Mar 16, 2017

miina added 2 commits July 27, 2017 17:20

Merge develop & resolve conflicts

6a74e38

Fix indent.

dd25749

miina added 2 commits August 24, 2017 20:09

Merge develop & resolve conflicts.

b0f9bcb

Fix indent.

7d0da3e

mehigh closed this Dec 27, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow unauthenticated user to view a customize-draft post via REST API #117

Allow unauthenticated user to view a customize-draft post via REST API #117

miina commented Feb 16, 2017 •

edited

coveralls commented Feb 16, 2017

coveralls commented Feb 16, 2017

coveralls commented Feb 16, 2017

coveralls commented Feb 16, 2017 •

edited

westonruter Feb 18, 2017

miina Feb 20, 2017

westonruter Feb 18, 2017

westonruter Feb 18, 2017

miina Feb 20, 2017

westonruter Feb 20, 2017

westonruter Feb 18, 2017

coveralls commented Feb 20, 2017 •

edited

westonruter Feb 20, 2017 •

edited

westonruter Feb 20, 2017

westonruter Feb 20, 2017

coveralls commented Feb 21, 2017 •

edited

westonruter Feb 23, 2017

westonruter Feb 23, 2017

miina Mar 2, 2017 •

edited

coveralls commented Mar 2, 2017 •

edited

coveralls commented Mar 16, 2017 •

edited

valendesigns commented Aug 23, 2017

coveralls commented Aug 24, 2017 •

edited

coveralls commented Aug 24, 2017

Allow unauthenticated user to view a customize-draft post via REST API #117

Allow unauthenticated user to view a customize-draft post via REST API #117

Conversation

miina commented Feb 16, 2017 • edited

coveralls commented Feb 16, 2017

coveralls commented Feb 16, 2017

coveralls commented Feb 16, 2017

coveralls commented Feb 16, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

coveralls commented Feb 20, 2017 • edited

westonruter Feb 20, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

coveralls commented Feb 21, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

miina Mar 2, 2017 • edited

Choose a reason for hiding this comment

coveralls commented Mar 2, 2017 • edited

coveralls commented Mar 16, 2017 • edited

valendesigns commented Aug 23, 2017

coveralls commented Aug 24, 2017 • edited

coveralls commented Aug 24, 2017

miina commented Feb 16, 2017 •

edited

coveralls commented Feb 16, 2017 •

edited

coveralls commented Feb 20, 2017 •

edited

westonruter Feb 20, 2017 •

edited

coveralls commented Feb 21, 2017 •

edited

miina Mar 2, 2017 •

edited

coveralls commented Mar 2, 2017 •

edited

coveralls commented Mar 16, 2017 •

edited

coveralls commented Aug 24, 2017 •

edited