wp-graphql / wp-graphql/wp-graphql

Return an error when a request uses an input the user isn't allowed to use

Open
#4,338 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

compat: breaking change component: connections component: input effort: med impact: med object type: comment object type: post scope: api type: enhancement v3.0
Dominant language
PHP
Stars
3.8k
Forks
472
Avg merge
1d 7h
Merged PRs (30d)
85

Description

In 3.0, using an input you aren't allowed to use should return an error instead of quietly returning empty results.

Background

Some connection filters (and some input fields) should only be usable by users with the right permissions. Today, when someone without those permissions uses one, we don't error. We narrow or empty the results instead. For example, a public request for posts( where: { stati: [ DRAFT ] } ) or comments( where: { statusIn: [ HOLD ] } ) just gets no nodes back, and docs/comments.md documents that behavior.

It doesn't break anyone, but it's not a great contract. A client can't tell "you aren't allowed to ask for this" apart from "nothing matched", which makes bugs hard to track down. Other GraphQL APIs and headless CMSs reject these queries. Payload and Directus, for example, both return an error when you filter on a field you can't read.

The contract I want for 3.0 (more context in #3930):

  • The schema is the same for every user. We don't hide or change args, fields or types based on who's asking.
  • Inputs you aren't allowed to use return an error. Output fields you aren't allowed to read keep resolving to null.
  • The decision only depends on the request (who's asking and what they asked for), never on what the stored data contains, so the error can't reveal anything about data you can't see.
  • The error is scoped to the field. The rest of the response still resolves.

Switching existing inputs from "empty results" to "error" will break clients that rely on today's behavior, so it lands on next as part of 3.0 (#3902).

Until 3.0

On main, restricted filters keep returning empty results rather than errors:

  • A list filter drops the values you can't use. If none are left, the query doesn't run.
  • A filter that can't be narrowed matches nothing.
  • A filter is never ignored in a way that returns unfiltered results.

New inputs that don't exist yet can use the error contract right away, since nothing depends on older behavior for them.

Scope for 3.0

  • Comments connection: statusIn (and the deprecated status arg) error for statuses the user can't see, instead of dropping them.
  • Comments connection: contentStatus errors for content statuses the user can't see.
  • Posts connections: stati / status error for statuses the user can't query, instead of dropping them. Decide what happens to the graphql_allowed_post_stati filter (keep, reinterpret, or deprecate).
  • Any other inputs that get an "empty results" guard on main before 3.0 get added to this list as they land.
  • Update argument descriptions to say what's required, in plain domain language ("requires permission to moderate comments"), not capability slugs.
  • Update docs/comments.md and the authentication and authorization docs for the new behavior.
  • Upgrade guide entry: what changed, and how to spot affected queries (a request that used to return empty results now returns an error on that field).

Related

  • #3930: arguments declaring their own behavior. Ideally these rules are declared on the argument (a capability shorthand or a validate callback) and enforced in one place, instead of hand-written checks in each connection resolver. If that API is ready in time, 3.0 uses it. If not, the hand-written checks switch to errors and get migrated later.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with docs/comments.md and the authentication, authorization, and upgrade-guide documentation named in the issue, then inspect the comments and posts connection inputs and their existing empty-results guards. Review #3930 for the proposed shared enforcement approach. Done means the listed restricted inputs return field-scoped errors, output fields still resolve to null, and the affected documentation is updated.

Written by the indexing model from the issue text.

Assessment

Tech stack
php
Domain
api, authorization, backend, documentation
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.