Require HCA step-up before account deletion - #1532
Conversation
Greptile SummaryThis PR adds Hack Club Auth step-up verification before linked users can create deletion requests, while preserving direct creation for unlinked users.
Confidence Score: 3/5The PR should not merge until oversized deletion details and HCA transport or parsing failures are handled without crashing the deletion flow. Linked users can currently trigger a cookie overflow through unrestricted deletion details, and ordinary external HCA failures escape the callback as unhandled server errors after pending state has been consumed. Files Needing Attention: app/controllers/deletion_requests_controller.rb and app/models/concerns/oauth_authentication.rb Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant App as Hackatime
participant HCA as Hack Club Auth
User->>App: POST /deletion
alt HCA identity linked
App->>App: Store pending request and state
App-->>User: "Redirect to HCA with prompt=login"
User->>HCA: Authenticate
HCA-->>App: GET callback with code and state
App->>App: Validate state, user, and linked HCA ID
App->>HCA: Exchange authorization code
HCA-->>App: Access token
App->>HCA: Fetch identity
HCA-->>App: identity.id
App->>App: Match identity and recheck eligibility
App->>App: Create deletion request
else No HCA identity linked
App->>App: Create deletion request immediately
end
Prompt To Fix All With AI### Issue 1
app/controllers/deletion_requests_controller.rb:85
**Session cookie overflows on details**
When a linked-HCA user submits sufficiently long `reason_details`, `begin_hca_step_up` copies the unrestricted value into the cookie-backed session, exceeding its size limit and causing the redirect to fail with `ActionDispatch::Cookies::CookieOverflow` before step-up can begin.
### Issue 2
app/controllers/deletion_requests_controller.rb:40
**HCA exchange failures escape callback**
If HCA times out, is unreachable, or returns a non-JSON error response, `hca_id_from_token` lets the transport or parsing exception escape, causing a 500 response after `pending_deletion_request` has already been consumed and forcing the user to restart deletion.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Reviews (1): Last reviewed commit: "Require HCA step-up for account deletion" | Re-trigger Greptile |
| "state" => state, | ||
| "user_id" => current_user.id, | ||
| "hca_id" => current_user.hca_id, | ||
| "attributes" => deletion_request_params.stringify_keys |
There was a problem hiding this comment.
Session cookie overflows on details
When a linked-HCA user submits sufficiently long reason_details, begin_hca_step_up copies the unrestricted value into the cookie-backed session, exceeding its size limit and causing the redirect to fail with ActionDispatch::Cookies::CookieOverflow before step-up can begin.
Knowledge Base Used: Authentication: sessions, API keys, and OAuth
Prompt To Fix With AI
This is a comment left during a code review.
Path: app/controllers/deletion_requests_controller.rb
Line: 85
Comment:
**Session cookie overflows on details**
When a linked-HCA user submits sufficiently long `reason_details`, `begin_hca_step_up` copies the unrestricted value into the cookie-backed session, exceeding its size limit and causing the redirect to fail with `ActionDispatch::Cookies::CookieOverflow` before step-up can begin.
**Knowledge Base Used:** [Authentication: sessions, API keys, and OAuth](https://app.greptile.com/mahadk/-/custom-context/knowledge-base/hackclub/hackatime/-/docs/api-authentication.md)
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.| end | ||
|
|
||
| redirect_uri = hca_deletion_callback_url | ||
| hca_id = User.hca_id_from_token(params[:code], redirect_uri) |
There was a problem hiding this comment.
HCA exchange failures escape callback
If HCA times out, is unreachable, or returns a non-JSON error response, hca_id_from_token lets the transport or parsing exception escape, causing a 500 response after pending_deletion_request has already been consumed and forcing the user to restart deletion.
Knowledge Base Used: Authentication: sessions, API keys, and OAuth
Prompt To Fix With AI
This is a comment left during a code review.
Path: app/controllers/deletion_requests_controller.rb
Line: 40
Comment:
**HCA exchange failures escape callback**
If HCA times out, is unreachable, or returns a non-JSON error response, `hca_id_from_token` lets the transport or parsing exception escape, causing a 500 response after `pending_deletion_request` has already been consumed and forcing the user to restart deletion.
**Knowledge Base Used:** [Authentication: sessions, API keys, and OAuth](https://app.greptile.com/mahadk/-/custom-context/knowledge-base/hackclub/hackatime/-/docs/api-authentication.md)
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
Summary of the problem
Accounts linked to Hack Club Auth could submit an account deletion request without confirming control of the linked HCA identity. Account deletion is a sensitive operation and should require HCA step-up authentication when that link exists.
Describe your changes
Linked accounts now enter an HCA OpenID Connect flow with
prompt=loginbefore a deletion request is created. The callback uses single-use state bound to the initiating Hackatime user and linked HCA ID, requires the returned identity to match exactly and rechecks deletion eligibility before continuing.Accounts without HCA retain the existing deletion flow. Development setup now documents the additional callback URL.
Before deployment,
https://hackatime.hackclub.com/deletion/hca/callbackmust be registered on the production HCA OAuth client.Screenshots / Media
No visual changes.