[PR #4320] [MERGED] Improve ApiAuthException control flow #6342

New Issue

OVERLORD · 2026-02-05T10:29:48+03:00

OVERLORD commented

2026-02-05 10:29:48 +03:00

📋 Pull Request Information

Original PR: https://github.com/BookStackApp/BookStack/pull/4320
Author: @devdot
Created: 6/16/2023
Status: ✅ Merged
Merged: 6/26/2023
Merged by: @ssddanbrown

Base: development ← Head: improve-api-auth-exception

📝 Commits (1)

74097bd Simplify ApiAuthException control flow

📊 Changes

3 files changed (+24 additions, -37 deletions)

View changed files

📝 app/Exceptions/ApiAuthException.php (+20 -1)
➖ app/Exceptions/UnauthorizedException.php (+0 -16)
📝 app/Http/Middleware/ApiAuthenticate.php (+4 -20)

📄 Description

First of all, the UnauthorizedException seems to be a strange base class for ApiAuthException, as it implies HTTP 401 but the exceptions are sometimes thrown with 403 instead:

ec775aec02/app/Api/ApiTokenGuard.php (L134-L152)

Further, the ApiAuthenticate middleware catches all exceptions that are thrown by the ApiTokenGuard, only to return a JSON response that is exactly the same as the application exception handler:

ec775aec02/app/Http/Middleware/ApiAuthenticate.php (L15-L25)

unauthorizedResponse will return the same JSON response as the exception handler.

My changes would further simplify the exception return messages for API errors into one place, rather than multiple places that create the standardized JSON response pattern.

The only downside I see is that this change would make the exceptions get logged through the exception logger, which may not be desired (altough that might be an improvement as well, right now there is no logging of failed API access attempts). However, if this exception should not be logged, it could be added here:

ec775aec02/app/Exceptions/Handler.php (L22-L25)

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/BookStackApp/BookStack/pull/4320 **Author:** [@devdot](https://github.com/devdot) **Created:** 6/16/2023 **Status:** ✅ Merged **Merged:** 6/26/2023 **Merged by:** [@ssddanbrown](https://github.com/ssddanbrown) **Base:** `development` ← **Head:** `improve-api-auth-exception` --- ### 📝 Commits (1) - [`74097bd`](https://github.com/BookStackApp/BookStack/commit/74097bd47c050cd3ddd1c3ccf7eef3a20bc463a3) Simplify ApiAuthException control flow ### 📊 Changes **3 files changed** (+24 additions, -37 deletions) <details> <summary>View changed files</summary> 📝 `app/Exceptions/ApiAuthException.php` (+20 -1) ➖ `app/Exceptions/UnauthorizedException.php` (+0 -16) 📝 `app/Http/Middleware/ApiAuthenticate.php` (+4 -20) </details> ### 📄 Description First of all, the `UnauthorizedException` seems to be a strange base class for `ApiAuthException`, as it implies HTTP 401 but the exceptions are sometimes thrown with 403 instead: https://github.com/BookStackApp/BookStack/blob/ec775aec02c0887d5cf2dc23c938a75b7eaf67d2/app/Api/ApiTokenGuard.php#L134-L152 Further, the `ApiAuthenticate` middleware catches all exceptions that are thrown by the `ApiTokenGuard`, only to return a JSON response that is exactly the same as the application exception handler: https://github.com/BookStackApp/BookStack/blob/ec775aec02c0887d5cf2dc23c938a75b7eaf67d2/app/Http/Middleware/ApiAuthenticate.php#L15-L25 [unauthorizedResponse](https://github.com/BookStackApp/BookStack/blob/ec775aec02c0887d5cf2dc23c938a75b7eaf67d2/app/Http/Middleware/ApiAuthenticate.php#L65-L73) will return the same JSON response as [the exception handler](https://github.com/BookStackApp/BookStack/blob/ec775aec02c0887d5cf2dc23c938a75b7eaf67d2/app/Exceptions/Handler.php#L80-L109). My changes would further simplify the exception return messages for API errors into one place, rather than multiple places that create the standardized JSON response pattern. The only downside I see is that this change would make the exceptions get logged through the exception logger, which may not be desired (altough that might be an improvement as well, right now there is no logging of failed API access attempts). However, if this exception should not be logged, it could be added here: https://github.com/BookStackApp/BookStack/blob/ec775aec02c0887d5cf2dc23c938a75b7eaf67d2/app/Exceptions/Handler.php#L22-L25 --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

OVERLORD added the pull-request label 2026-02-05 10:29:48 +03:00

OVERLORD closed this issue

2026-02-05 10:29:51 +03:00

Sign in to join this conversation.

Branches Tags

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: starred/BookStack#6342