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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Inserting a Site Title block causes a 403 error (REST API) #33003

Closed
ockham opened this issue Jun 25, 2021 · 4 comments 路 Fixed by #45093
Closed

Inserting a Site Title block causes a 403 error (REST API) #33003

ockham opened this issue Jun 25, 2021 · 4 comments 路 Fixed by #45093
Assignees
Labels
[Block] Query Loop Affects the Query Loop Block [Block] Site Title Affects the Site Title Block [Status] In Progress Tracking issues with work in progress [Type] Bug An existing feature does not function as intended

Comments

@ockham
Copy link
Contributor

ockham commented Jun 25, 2021

Description

Found while working on #32868. This is currently blocking #32868, as it causes e2e tests to fail 馃槙

Repro'd on current GB trunk.

Step-by-step reproduction instructions

  1. Log into WP as a non-admin user (e.g. Editor role).
  2. Create a new post.
  3. Open your browser devtools bar (Network tab), and clear it.
  4. Insert a 'Site Title' block

Expected behaviour

No network request to fail with a 403.

Actual behaviour

The network request to GET http://localhost:8888/index.php?rest_route=%2Fwp%2Fv2%2Fsettings%2F&_locale=user comes back with a 403 error, with the following response:

{
	"code": "rest_forbidden",
	"message": "Sorry, you are not allowed to do that.",
	"data": { "status": 403 }
}

The error is also visible in the devtools console, including a stacktrace. While minimized, it seems to be caused by this line:

const [ title, setTitle ] = useEntityProp( 'root', 'site', 'title' );

Bonus

If you enter 'Site' into the block picker to locate the Site Title block, there's another network request that fails with a 403:

GET http://localhost:8888/index.php?rest_route=/wp/v2/users&context=edit&per_page=100&_locale=user fails with

{
	"code": "rest_forbidden_context",
	"message": "Sorry, you are not allowed to list users.",
	"data": { "status": 403 }
}

The stack trace points to the Query Loop block for this one:

authorList: getEntityRecords( 'root', 'user', {
per_page: -1,
} ),
.

This latter 403 also causes an Uncaught (in promise) error to be logged in the browser console.

cc/ @ntsekouras

@ockham ockham added [Type] Bug An existing feature does not function as intended [Block] Site Title Affects the Site Title Block [Block] Query Loop Affects the Query Loop Block labels Jun 25, 2021
@ntsekouras
Copy link
Contributor

This PR: #32961 might simplify things for such errors. We need to audit all the usages in combination with REST API.

We should probably also look to remove apiFetch usages that were there for bypassing the permissions check in data layer, because there was no context support.

@ockham
Copy link
Contributor Author

ockham commented Jun 28, 2021

Thanks for the pointer, Nik!

IIUC, we'll need to change the implementation of useEntityProp() and getEntityRecords() to use the right context. I'll work around that for now in #32868, since it seems like a rather substantial change, and I'd like to unblock #32868 馃槵

@ntsekouras
Copy link
Contributor

getEntityRecords

Context support is there by Riad in the above PR.

since it seems like a rather substantial change

Not so sure about that 馃槃 . Some controls I had seen need some data that are only in edit context, like labels of taxonomies. So the changes need to be made really carefully not to introduce regressions.

@ockham
Copy link
Contributor Author

ockham commented Jun 29, 2021

getEntityRecords

Context support is there by Riad in the above PR.

Ah, hadn't noticed that getEntityRecords was being changed by that PR as well! Thanks for clarifying 馃憤

since it seems like a rather substantial change

Not so sure about that 馃槃 .

Ah, indeed! Seems like we just need to provide a context key-value pair in the 4th arg of getEntityRecords 馃

Some controls I had seen need some data that are only in edit context, like labels of taxonomies. So the changes need to be made really carefully not to introduce regressions.

馃憤

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Query Loop Affects the Query Loop Block [Block] Site Title Affects the Site Title Block [Status] In Progress Tracking issues with work in progress [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants