Require HCA step-up before account deletion

DEVELOPMENT.md

@@ -30,7 +30,7 @@ ENCRYPTION_DETERMINISTIC_KEY=32characterrandomstring12345678902
 ENCRYPTION_KEY_DERIVATION_SALT=16charssalt1234
 ```
 
-Visit <https://hca.dinosaurbbq.org>, log in with an email address, then enable Developer Mode in HCA settings. After that, navigate to the "Developers' Corner" and "app yourself up", specifying a callback URL of `http://localhost:3000/auth/hca/callback` and minimum scopes of `email`, `slack_id`, and `verification_status`. 
+Visit <https://hca.dinosaurbbq.org>, log in with an email address, then enable Developer Mode in HCA settings. After that, navigate to the "Developers' Corner" and "app yourself up", specifying callback URLs of `http://localhost:3000/auth/hca/callback` and `http://localhost:3000/deletion/hca/callback`, with minimum scopes of `email`, `slack_id`, and `verification_status`.
 
 Then, fill out the following fields in your `.env` file:
 

app/controllers/deletion_requests_controller.rb

@@ -12,11 +12,45 @@ def show
   end
 
   def create
-    @deletion_request = DeletionRequest.create_for_user!(current_user, **deletion_request_params)
-    redirect_to deletion_path
-  rescue ActiveRecord::RecordInvalid => e
-    report_error(e, message: "Deletion request creation failed")
-    redirect_to my_settings_path
+    return begin_hca_step_up if current_user.hca_id.present?
+
+    create_deletion_request(deletion_request_params)
+  end
+
+  def hca_callback
+    pending_request = session.delete(:pending_deletion_request)
+
+    unless valid_hca_state?(pending_request&.dig("state"))
+      report_message("HCA deletion step-up state was invalid")
+      return redirect_to(my_settings_path, alert: "Hack Club Auth verification failed. Please try again.")
+    end
+
+    if params[:error].present?
+      report_message("HCA deletion step-up error: #{params[:error]}") unless params[:error] == "access_denied"
+      return redirect_to(my_settings_path, alert: "Hack Club Auth verification was cancelled.")
+    end
+
+    unless pending_request["user_id"] == current_user.id &&
+        pending_request["hca_id"] == current_user.hca_id && params[:code].present?
+      report_message("HCA deletion step-up context was invalid")
+      return redirect_to(my_settings_path, alert: "Hack Club Auth verification failed. Please try again.")
+    end
+
+    redirect_uri = hca_deletion_callback_url
+    hca_id = User.hca_id_from_token(params[:code], redirect_uri)
+    unless hca_id.present? && hca_id == pending_request["hca_id"]
+      report_message("HCA deletion step-up identity did not match User ##{current_user.id}")
+      return redirect_to(my_settings_path, alert: "Please verify with the Hack Club Account linked to Hackatime.")
+    end
+
+    unless current_user.can_request_deletion?
+      return redirect_to(my_settings_path, alert: "You can't request deletion right now.")
+    end
+
+    create_deletion_request(pending_request.fetch("attributes").symbolize_keys)
+  rescue HTTP::Error, JSON::ParserError => e
+    report_error(e, message: "HCA deletion step-up failed")
+    redirect_to my_settings_path, alert: "Hack Club Auth verification failed. Please try again."
   end
 
   def cancel
@@ -45,6 +79,42 @@ def check_can_request
     end
   end
 
+  def begin_hca_step_up
+    attributes = deletion_request_params
+    if attributes[:reason].to_s.length > DeletionRequest::MAX_REASON_LENGTH ||
+        attributes[:reason_details].to_s.length > DeletionRequest::MAX_REASON_DETAILS_LENGTH
+      return redirect_to(my_settings_path, alert: "Deletion details are too long.")
+    end
+
+    state = SecureRandom.hex(24)
+    session[:pending_deletion_request] = {
+      "state" => state,
+      "user_id" => current_user.id,
+      "hca_id" => current_user.hca_id,
+      "attributes" => attributes.stringify_keys
+    }
+
+    redirect_to User.hca_authorize_url(
+      hca_deletion_callback_url,
+      state:,
+      prompt: "login",
+      scope: "openid email slack_id verification_status"
+    ), host: HCAService.host, allow_other_host: HCAService.host
+  end
+
+  def valid_hca_state?(expected_state)
+    expected_state.present? && params[:state].present? &&
+      ActiveSupport::SecurityUtils.secure_compare(params[:state].to_s, expected_state.to_s)
+  end
+
+  def create_deletion_request(attributes)
+    @deletion_request = DeletionRequest.create_for_user!(current_user, **attributes)
+    redirect_to deletion_path
+  rescue ActiveRecord::RecordInvalid => e
+    report_error(e, message: "Deletion request creation failed")
+    redirect_to my_settings_path
+  end
+
   def deletion_request_params
     params.fetch(:deletion_request, {}).permit(:reason, :reason_details).to_h.symbolize_keys
   end

app/controllers/settings/privacy_controller.rb

@@ -25,7 +25,8 @@ def render_privacy(status: :ok) = render_settings_page(active_section: "privacy"
 
   def section_props
     { user: user_props(keys: %i[allow_public_stats_lookup can_request_deletion]),
-      rotated_api_key: flash[:rotated_api_key] }
+      rotated_api_key: flash[:rotated_api_key],
+      deletion_reason_details_max_length: DeletionRequest::MAX_REASON_DETAILS_LENGTH }
   end
 
   def privacy_params = params.require(:user).permit(:allow_public_stats_lookup)

app/javascript/pages/Users/Settings/Privacy.svelte

@@ -17,6 +17,7 @@
     user,
     rotated_api_key = "",
     errors,
+    deletion_reason_details_max_length,
   }: PrivacyPageProps = $props();
 
   const deletionReasons = [
@@ -231,6 +232,7 @@
         name="deletion_request[reason_details]"
         rows="4"
         bind:value={deletionReasonDetails}
+        maxlength={deletion_reason_details_max_length}
         required
         class="block w-full rounded-lg border border-surface-300 bg-darker px-3 py-2 text-sm text-surface-content placeholder:text-muted focus:border-primary focus:outline-none focus:ring-2 focus:ring-primary/30"
         placeholder="Tell us anything else we should know."></textarea>

app/javascript/pages/Users/Settings/types.ts

@@ -270,6 +270,7 @@ export type NotificationsPageProps = SettingsCommonProps & {
 export type PrivacyPageProps = SettingsCommonProps & {
   user: Pick<UserProps, "allow_public_stats_lookup" | "can_request_deletion">;
   rotated_api_key?: string | null;
+  deletion_reason_details_max_length: number;
 };
 
 export type GoalsPageProps = SettingsCommonProps & {

app/models/concerns/oauth_authentication.rb

@@ -5,13 +5,28 @@ module OauthAuthentication
   class_methods do
     include ErrorReporting
 
-    def hca_authorize_url(redirect_uri)
+    def hca_authorize_url(redirect_uri, state: nil, prompt: nil, scope: "email slack_id verification_status")
       URI.parse("#{HCAService.host}/oauth/authorize?#{{
         redirect_uri:,
         client_id: ENV["HCA_CLIENT_ID"],
         response_type: "code",
-        scope: "email slack_id verification_status"
-      }.to_query}")
+        scope:,
+        state:,
+        prompt:
+      }.compact.to_query}")
+    end
+
+    def hca_id_from_token(code, redirect_uri)
+      response = HTTP.post("#{HCAService.host}/oauth/token", form: {
+        client_id: ENV["HCA_CLIENT_ID"], client_secret: ENV["HCA_CLIENT_SECRET"],
+        redirect_uri:, code:, grant_type: "authorization_code"
+      })
+      token_data = JSON.parse(response.body.to_s)
+      access_token = token_data["access_token"] if token_data.is_a?(Hash)
+      return if access_token.nil?
+
+      hca_data = HCAService.me(access_token)
+      hca_data.dig("identity", "id") if hca_data.is_a?(Hash)
     end
 
     def slack_authorize_url(redirect_uri, state: nil, close_window: false, continue_param: nil)

app/models/deletion_request.rb

@@ -1,12 +1,17 @@
 class DeletionRequest < ApplicationRecord
+  MAX_REASON_LENGTH = 100
+  MAX_REASON_DETAILS_LENGTH = 1_000
+
   belongs_to :user
   belongs_to :admin_approved_by, class_name: "User", optional: true
 
   enum :status, { pending: 0, approved: 1, cancelled: 2, completed: 3 }
 
   validates :requested_at, presence: true
   validates :reason, presence: true, if: -> { reason_details.present? }
+  validates :reason, length: { maximum: MAX_REASON_LENGTH }
   validates :reason_details, presence: true, if: -> { reason.present? }
+  validates :reason_details, length: { maximum: MAX_REASON_DETAILS_LENGTH }
   validate :user_not_banned_from_deletion, on: :create
 
   scope :active, -> { where(status: [ :pending, :approved ]) }

config/routes.rb

@@ -223,6 +223,7 @@ def matches?(request)
   get "deletion", to: "deletion_requests#show", as: :deletion
   post "deletion", to: "deletion_requests#create", as: :create_deletion
   delete "deletion", to: "deletion_requests#cancel", as: :cancel_deletion
+  get "deletion/hca/callback", to: "deletion_requests#hca_callback", as: :hca_deletion_callback
 
   get "setup", to: "users#setup", as: :setup
   get "my/wakatime_setup", to: redirect("/setup")

test/controllers/deletion_requests_controller_test.rb

@@ -0,0 +1,187 @@
+require "test_helper"
+require "webmock/minitest"
+
+class DeletionRequestsControllerTest < ActionDispatch::IntegrationTest
+  setup do
+    @user = User.create!(timezone: "UTC")
+    sign_in_as(@user)
+  end
+
+  test "create immediately creates a request when HCA is not linked" do
+    assert_difference "DeletionRequest.count", 1 do
+      post create_deletion_path, params: deletion_params
+    end
+
+    assert_redirected_to deletion_path
+  end
+
+  test "create requires HCA step-up when HCA is linked" do
+    @user.update!(hca_id: "hca-linked")
+
+    assert_no_difference "DeletionRequest.count" do
+      post create_deletion_path, params: deletion_params
+    end
+
+    authorize_uri = URI.parse(response.location)
+    authorize_query = Rack::Utils.parse_query(authorize_uri.query)
+    assert_equal "#{HCAService.host}/oauth/authorize", "#{authorize_uri.scheme}://#{authorize_uri.host}#{authorize_uri.path}"
+    assert_equal "login", authorize_query["prompt"]
+    assert_includes authorize_query["scope"].split, "openid"
+    assert_equal session.dig(:pending_deletion_request, "state"), authorize_query["state"]
+    assert_equal @user.id, session.dig(:pending_deletion_request, "user_id")
+    assert_equal "hca-linked", session.dig(:pending_deletion_request, "hca_id")
+    assert_equal deletion_params[:deletion_request][:reason], session.dig(:pending_deletion_request, "attributes", "reason")
+  end
+
+  test "create rejects deletion details that cannot fit in the session" do
+    @user.update!(hca_id: "hca-linked")
+
+    post create_deletion_path, params: {
+      deletion_request: deletion_params[:deletion_request].merge(reason_details: "a" * 10_000)
+    }
+
+    assert_redirected_to my_settings_path
+    assert_equal "Deletion details are too long.", flash[:alert]
+    assert_nil session[:pending_deletion_request]
+  end
+
+  test "HCA callback creates the pending request for the linked identity" do
+    @user.update!(hca_id: "hca-linked")
+    post create_deletion_path, params: deletion_params
+    state = session.dig(:pending_deletion_request, "state")
+    stub_hca_identity("hca-linked")
+
+    assert_difference "DeletionRequest.count", 1 do
+      get hca_deletion_callback_path, params: { code: "step-up-code", state: state }
+    end
+
+    assert_redirected_to deletion_path
+    assert_equal deletion_params[:deletion_request][:reason], DeletionRequest.last.reason
+    assert_nil session[:pending_deletion_request]
+  end
+
+  test "HCA callback rejects a different identity" do
+    @user.update!(hca_id: "hca-linked")
+    post create_deletion_path, params: deletion_params
+    state = session.dig(:pending_deletion_request, "state")
+    stub_hca_identity("hca-someone-else")
+
+    assert_no_difference "DeletionRequest.count" do
+      get hca_deletion_callback_path, params: { code: "step-up-code", state: state }
+    end
+
+    assert_redirected_to my_settings_path
+  end
+
+  test "HCA callback handles an invalid response" do
+    @user.update!(hca_id: "hca-linked")
+    post create_deletion_path, params: deletion_params
+    state = session.dig(:pending_deletion_request, "state")
+    stub_request(:post, "#{HCAService.host}/oauth/token").to_return(body: "not json")
+
+    assert_no_difference "DeletionRequest.count" do
+      get hca_deletion_callback_path, params: { code: "step-up-code", state: state }
+    end
+
+    assert_redirected_to my_settings_path
+    assert_equal "Hack Club Auth verification failed. Please try again.", flash[:alert]
+  end
+
+  test "HCA callback handles a connection failure" do
+    @user.update!(hca_id: "hca-linked")
+    post create_deletion_path, params: deletion_params
+    state = session.dig(:pending_deletion_request, "state")
+    stub_request(:post, "#{HCAService.host}/oauth/token")
+      .to_raise(HTTP::ConnectionError.new("HCA unavailable"))
+
+    assert_no_difference "DeletionRequest.count" do
+      get hca_deletion_callback_path, params: { code: "step-up-code", state: state }
+    end
+
+    assert_redirected_to my_settings_path
+    assert_equal "Hack Club Auth verification failed. Please try again.", flash[:alert]
+  end
+
+  test "HCA callback rejects an invalid state" do
+    @user.update!(hca_id: "hca-linked")
+    post create_deletion_path, params: deletion_params
+
+    assert_no_difference "DeletionRequest.count" do
+      get hca_deletion_callback_path, params: { code: "step-up-code", state: "wrong-state" }
+    end
+
+    assert_redirected_to my_settings_path
+    assert_nil session[:pending_deletion_request]
+  end
+
+  test "HCA callback cannot be replayed" do
+    @user.update!(hca_id: "hca-linked")
+    post create_deletion_path, params: deletion_params
+    state = session.dig(:pending_deletion_request, "state")
+    stub_hca_identity("hca-linked")
+    get hca_deletion_callback_path, params: { code: "step-up-code", state: state }
+
+    assert_no_difference "DeletionRequest.count" do
+      get hca_deletion_callback_path, params: { code: "step-up-code", state: state }
+    end
+
+    assert_redirected_to my_settings_path
+  end
+
+  test "HCA callback rejects a changed linked identity" do
+    @user.update!(hca_id: "hca-linked")
+    post create_deletion_path, params: deletion_params
+    state = session.dig(:pending_deletion_request, "state")
+    @user.update!(hca_id: "hca-changed")
+
+    assert_no_difference "DeletionRequest.count" do
+      get hca_deletion_callback_path, params: { code: "step-up-code", state: state }
+    end
+
+    assert_redirected_to my_settings_path
+  end
+
+  test "HCA callback rechecks deletion eligibility" do
+    @user.update!(hca_id: "hca-linked")
+    post create_deletion_path, params: deletion_params
+    state = session.dig(:pending_deletion_request, "state")
+    DeletionRequest.create_for_user!(@user)
+    stub_hca_identity("hca-linked")
+
+    assert_no_difference "DeletionRequest.count" do
+      get hca_deletion_callback_path, params: { code: "step-up-code", state: state }
+    end
+
+    assert_redirected_to my_settings_path
+  end
+
+  test "HCA error must include the pending state" do
+    @user.update!(hca_id: "hca-linked")
+    post create_deletion_path, params: deletion_params
+
+    get hca_deletion_callback_path, params: { error: "access_denied", state: "wrong-state" }
+
+    assert_redirected_to my_settings_path
+    assert_equal "Hack Club Auth verification failed. Please try again.", flash[:alert]
+  end
+
+  private
+
+  def deletion_params
+    {
+      deletion_request: {
+        reason: "Something else",
+        reason_details: "I no longer need my account"
+      }
+    }
+  end
+
+  def stub_hca_identity(hca_id)
+    stub_request(:post, "#{HCAService.host}/oauth/token")
+      .with(body: hash_including("code" => "step-up-code", "redirect_uri" => hca_deletion_callback_url))
+      .to_return(body: { access_token: "step-up-token" }.to_json)
+    stub_request(:get, "#{HCAService.host}/api/v1/me")
+      .with(headers: { "Authorization" => "Bearer step-up-token" })
+      .to_return(body: { identity: { id: hca_id } }.to_json)
+  end
+end