Skip to content

Check authorization before input validation#173

Open
otazniksk wants to merge 1 commit intotomaj:masterfrom
otazniksk:fix/auth-before-input-validation
Open

Check authorization before input validation#173
otazniksk wants to merge 1 commit intotomaj:masterfrom
otazniksk:fix/auth-before-input-validation

Conversation

@otazniksk
Copy link
Copy Markdown

Problem

In ApiPresenter::run(), input parameter validation is executed before authorization check:

https://github.com/tomaj/nette-api/blob/3.4.0/src/Presenters/ApiPresenter.php#L87-L99

$paramsProcessor = new ParamsProcessor($handler->params());
if ($paramsProcessor->isError()) {
    return $this->errorHandler->handleInputParams(...);  // 400 wrong input
}

$params = $paramsProcessor->getValues();
$authResponse = $this->checkAuth($authorization, $params);  // auth runs later

This is a regression from v2.x where auth was checked first:
https://github.com/tomaj/nette-api/blob/2.13.0/src/Presenters/ApiPresenter.php#L73-L83

Security impact

An unauthenticated client can probe API validation rules without providing a valid token. Requests with missing/invalid auth + invalid body return 400 wrong input instead of 401, leaking information about:

  • which fields are required
  • which validation rules apply
  • the shape of the expected payload

Per OWASP ASVS 4.0.3 (V4.1.1 Access Control), authorization must be enforced before request processing.

Suggested fix

Move checkAuth() before ParamsProcessor in ApiPresenter::run().

Version

tomaj/nette-api 3.4.0

Moves checkAuth() call before ParamsProcessor in ApiPresenter::run()
to prevent unauthenticated clients from probing API validation rules.

Previously, requests with missing/invalid auth would return
"400 wrong input" when the body was also invalid, leaking information
about required fields and validation rules to unauthenticated callers.

Per OWASP ASVS V4.1.1, authorization must be enforced before request
processing.

Trade-off: error handlers (handleAuthorization, handleAuthorizationException)
now receive an empty params array on auth failure. Debugging context is
lost on failed auth, but authorization itself does not use params
(ApiAuthorizationInterface::authorized() takes no arguments), so the
security behavior is unchanged.
@otazniksk
Copy link
Copy Markdown
Author

The failing PHPStan checks are pre-existing errors in Handlers/OpenApiHandler.php (lines 345, 354) — Cannot assign offset 'application/json;…' to array<string, array<string, mixed>>|string.

They reproduce on clean master and are unrelated to this PR.

Happy to open a separate PR to fix them if you'd like.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant