Fix subscribe endpoint when subscribers are disabled#4601
Fix subscribe endpoint when subscribers are disabled#4601Misrilal-Sah wants to merge 1 commit intocachethq:developfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Adds a defensive server-side guard to prevent subscription creation and subscribe page access when email subscribers are disabled in settings, addressing direct POSTs to the subscribe endpoint.
Changes:
- Added
subscribers_enabled()checks to redirect to the status page fromshowSubscribe()when subscribers are disabled. - Added the same guard to
postSubscribe()to block direct subscription creation when subscribers are disabled. - Removed an unused
$subscriptions = Binput::get('subscriptions');assignment inpostSubscribe().
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (!subscribers_enabled()) { | ||
| return Redirect::route('status-page'); | ||
| } |
There was a problem hiding this comment.
The subscribe routes are already protected by the subscribers route middleware, which performs the same !subscribers_enabled() redirect (see app/Http/Routes/SubscribeRoutes.php:32 and app/Http/Middleware/SubscribersConfigured.php:28-34). Duplicating the gate here in the controller adds a second source of truth that can drift; consider relying on the middleware (or, if the middleware isn’t reliably applied in some deployments, fix the routing/middleware configuration instead of duplicating the check).
| public function showSubscribe() | ||
| { | ||
| if (!subscribers_enabled()) { | ||
| return Redirect::route('status-page'); | ||
| } |
There was a problem hiding this comment.
showSubscribe() can now return a redirect when subscribers are disabled, but the PHPDoc still declares @return \\Illuminate\\View\\View. Please update the docblock return type to reflect that this method may return a redirect response as well.
| public function postSubscribe() | ||
| { | ||
| if (!subscribers_enabled()) { | ||
| return Redirect::route('status-page'); | ||
| } |
There was a problem hiding this comment.
postSubscribe() returns a redirect response in all paths (including the new disabled-subscribers guard), but its PHPDoc declares @return \\Illuminate\\View\\View. Update the docblock return type to match the actual response type(s).
Issue link id: #4515
Description:
This change prevents direct access to subscription creation when email notifications are disabled in settings.
Previously, the subscribe page button was hidden but posting directly to the subscribe endpoint could still create subscriptions.
The fix adds a defensive check in SubscribeController for both subscribe page rendering and subscribe submission, redirecting to the status page when subscribers are disabled.