fix: block entering a room for not active accounts#256
Conversation
✅ Deploy Preview for hoppdocs ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
📝 WalkthroughWalkthroughAdds a centralized access check for subscriptions/trials used by backend token endpoints and websocket handlers, surfaces a specific "trial-ended" error to frontend UIs (including deep link handler and room UI), and bumps the golangci-lint pre-commit hook version. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Backend as Backend Handler
participant DB as Database
participant Telegram
participant Frontend
Client->>Backend: Request token / join
activate Backend
Backend->>DB: Load user + subscription (checkUserHasAccess)
activate DB
DB-->>Backend: subscription data / error
deactivate DB
alt Subscription fetch error
Backend-->>Client: 500 {"error":"Failed to check subscription status"}
Client->>Frontend: Show generic failure
else User has access (Pro or active trial)
Backend-->>Client: 200 {token...}
Client->>Frontend: Proceed to session
else Access denied (trial-ended)
Backend->>Telegram: Alert unsubscribed attempt
Backend-->>Client: 402/403 {"error":"trial-ended"}
Client->>Frontend: Show "Trial has expired..." message
end
deactivate Backend
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
No actionable comments were generated in the recent review. 🎉 Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
| // Check if user has access (paid or active trial) | ||
| userWithSub, err2 := models.GetUserWithSubscription(h.DB, user) | ||
| if err2 != nil { | ||
| c.Logger().Error("Error getting user subscription: ", err2) | ||
| return echo.NewHTTPError(http.StatusInternalServerError, "Failed to check subscription status") | ||
| } | ||
|
|
||
| hasAccess := userWithSub.IsPro | ||
| if !hasAccess && userWithSub.IsTrial && userWithSub.TrialEndsAt != nil { | ||
| hasAccess = userWithSub.TrialEndsAt.After(time.Now()) | ||
| } | ||
|
|
||
| if !hasAccess { | ||
| return c.JSON(http.StatusForbidden, map[string]string{"error": "trial-ended"}) | ||
| } | ||
|
|
There was a problem hiding this comment.
nit: We have a flavour of this in 3 places, at some point it would be worth extracting this as a util
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@backend/internal/handlers/handlers.go`:
- Around line 1027-1037: The endpoint currently returns http.StatusForbidden
(403) when checkUserHasAccess returns false, which conflicts with the frontend
expecting 402 for trial-ended; change the response code in the failure branch
(where checkUserHasAccess(...) returns !hasAccess) from http.StatusForbidden to
http.StatusPaymentRequired and keep the same JSON body, and also update any
corresponding logic in slackHandlers.go that emits a 403 for trial-ended users
so both backend handlers consistently return 402 for expired trials (reference
symbols: checkUserHasAccess, notifications.SendTelegramNotification).
In `@backend/internal/handlers/utils.go`:
- Around line 204-208: checkUserHasAccess calls models.GetUserWithSubscription
which assumes user.TeamID is non-nil; add a nil guard for user.TeamID before
calling GetUserWithSubscription in checkUserHasAccess so we don't dereference a
nil *uint. Specifically, in checkUserHasAccess check if user.TeamID == nil and
handle that case (e.g., treat as no team / return appropriate access result or
skip subscription lookup) instead of calling GetUserWithSubscription, or
alternatively adapt the call site to pass a safe value; reference the functions
checkUserHasAccess and GetUserWithSubscription and the field user.TeamID when
making this change.
| // Check if caller has access (paid or active trial) | ||
| hasAccess, err := checkUserHasAccess(h.DB, user) | ||
| if err != nil { | ||
| c.Logger().Error("Error getting user subscription: ", err) | ||
| return echo.NewHTTPError(http.StatusInternalServerError, "Failed to check subscription status") | ||
| } | ||
|
|
||
| if !hasAccess { | ||
| _ = notifications.SendTelegramNotification(fmt.Sprintf("Unsubscribed user %s tried to join room %s", user.ID, room.Name), h.Config) | ||
| return c.JSON(http.StatusForbidden, map[string]string{"error": "trial-ended"}) | ||
| } |
There was a problem hiding this comment.
HTTP 403 for trial-ended conflicts with the frontend's expected 402 status code.
deepLinkUtils.ts (Line 94) checks for response.status === 402 to handle trial expiration, but this endpoint returns http.StatusForbidden (403). The frontend's 403 handler (Line 101-102) would catch this instead and show "You don't have access to this session. It belongs to a different team." — a misleading message for expired trials.
Align on a single status code. 402 (Payment Required) is semantically appropriate for subscription/trial access gating. This would also require updating slackHandlers.go.
Proposed fix (if choosing 402)
if !hasAccess {
_ = notifications.SendTelegramNotification(fmt.Sprintf("Unsubscribed user %s tried to join room %s", user.ID, room.Name), h.Config)
- return c.JSON(http.StatusForbidden, map[string]string{"error": "trial-ended"})
+ return c.JSON(http.StatusPaymentRequired, map[string]string{"error": "trial-ended"})
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Check if caller has access (paid or active trial) | |
| hasAccess, err := checkUserHasAccess(h.DB, user) | |
| if err != nil { | |
| c.Logger().Error("Error getting user subscription: ", err) | |
| return echo.NewHTTPError(http.StatusInternalServerError, "Failed to check subscription status") | |
| } | |
| if !hasAccess { | |
| _ = notifications.SendTelegramNotification(fmt.Sprintf("Unsubscribed user %s tried to join room %s", user.ID, room.Name), h.Config) | |
| return c.JSON(http.StatusForbidden, map[string]string{"error": "trial-ended"}) | |
| } | |
| // Check if caller has access (paid or active trial) | |
| hasAccess, err := checkUserHasAccess(h.DB, user) | |
| if err != nil { | |
| c.Logger().Error("Error getting user subscription: ", err) | |
| return echo.NewHTTPError(http.StatusInternalServerError, "Failed to check subscription status") | |
| } | |
| if !hasAccess { | |
| _ = notifications.SendTelegramNotification(fmt.Sprintf("Unsubscribed user %s tried to join room %s", user.ID, room.Name), h.Config) | |
| return c.JSON(http.StatusPaymentRequired, map[string]string{"error": "trial-ended"}) | |
| } |
🤖 Prompt for AI Agents
In `@backend/internal/handlers/handlers.go` around lines 1027 - 1037, The endpoint
currently returns http.StatusForbidden (403) when checkUserHasAccess returns
false, which conflicts with the frontend expecting 402 for trial-ended; change
the response code in the failure branch (where checkUserHasAccess(...) returns
!hasAccess) from http.StatusForbidden to http.StatusPaymentRequired and keep the
same JSON body, and also update any corresponding logic in slackHandlers.go that
emits a 403 for trial-ended users so both backend handlers consistently return
402 for expired trials (reference symbols: checkUserHasAccess,
notifications.SendTelegramNotification).
Summary by CodeRabbit
New Features
Chores