Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion app/controllers/api/v1/accounts_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ def index

render json: {
error: "internal_server_error",
message: "Error: #{e.message}"
message: "An unexpected error occurred"
}, status: :internal_server_error
end

Expand Down
52 changes: 36 additions & 16 deletions app/controllers/api/v1/auth_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -45,23 +45,28 @@ def signup
user.family = family
user.role = User.role_for_new_family_creator

if user.save
# Claim invite code if provided
InviteCode.claim!(params[:invite_code]) if params[:invite_code].present?

# Create device and OAuth token
begin
# Atomic: user creation, invite-code claim, and device/token issuance
# either all commit or none do. Without this, a post-commit device
# failure (e.g., racing uniqueness) would leave the user/invite/family
# committed while the client got a 422 "Failed to register device".
token_response = nil
begin
ActiveRecord::Base.transaction do
unless user.save
render json: { errors: user.errors.full_messages }, status: :unprocessable_entity
raise ActiveRecord::Rollback
end
InviteCode.claim!(params[:invite_code]) if params[:invite_code].present?
device = MobileDevice.upsert_device!(user, device_params)
token_response = device.issue_token!
rescue ActiveRecord::RecordInvalid => e
render json: { error: "Failed to register device: #{e.message}" }, status: :unprocessable_entity
return
end

render json: token_response.merge(user: mobile_user_payload(user)), status: :created
else
render json: { errors: user.errors.full_messages }, status: :unprocessable_entity
rescue ActiveRecord::RecordInvalid => e
Rails.logger.error("[Auth] Device registration failed: #{e.class} - #{e.message}")
render json: { error: "Failed to register device" }, status: :unprocessable_entity
return
end

render json: token_response.merge(user: mobile_user_payload(user)), status: :created if token_response
end

def login
Expand Down Expand Up @@ -90,7 +95,8 @@ def login
device = MobileDevice.upsert_device!(user, device_params)
token_response = device.issue_token!
rescue ActiveRecord::RecordInvalid => e
render json: { error: "Failed to register device: #{e.message}" }, status: :unprocessable_entity
Rails.logger.error("[Auth] Device registration failed: #{e.message}")
render json: { error: "Failed to register device" }, status: :unprocessable_entity
return
end

Expand Down Expand Up @@ -312,7 +318,20 @@ def valid_device_info?
return false if device.nil?

required_fields = %w[device_id device_name device_type os_version app_version]
required_fields.all? { |field| device[field].present? }
return false unless required_fields.all? { |field| device[field].present? }

# Run MobileDevice's attribute-level validations up front (e.g.,
# device_type must be ios/android/web) so a misconfigured client
# is rejected BEFORE signup commits user/family/invite. Skip
# errors we can't evaluate without a user: the :user belongs_to
# presence check, and device_id uniqueness scoped to user_id
# (upsert_device! treats collisions as updates anyway).
preview = MobileDevice.new(device_params)
preview.valid?
relevant_errors = preview.errors.errors.reject do |err|
err.type == :taken || err.attribute == :user
end
relevant_errors.empty?
end

def device_params
Expand Down Expand Up @@ -373,7 +392,8 @@ def issue_mobile_tokens(user, device_info)

render json: token_response.merge(user: mobile_user_payload(user))
rescue ActiveRecord::RecordInvalid => e
render json: { error: "Failed to register device: #{e.message}" }, status: :unprocessable_entity
Rails.logger.error("[Auth] Device registration failed: #{e.message}")
render json: { error: "Failed to register device" }, status: :unprocessable_entity
end

def ensure_write_scope
Expand Down
4 changes: 2 additions & 2 deletions app/controllers/api/v1/categories_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ def index

render json: {
error: "internal_server_error",
message: "Error: #{e.message}"
message: "An unexpected error occurred"
}, status: :internal_server_error
end

Expand All @@ -41,7 +41,7 @@ def show

render json: {
error: "internal_server_error",
message: "Error: #{e.message}"
message: "An unexpected error occurred"
}, status: :internal_server_error
end

Expand Down
2 changes: 1 addition & 1 deletion app/controllers/api/v1/holdings_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ def log_and_render_error(action, exception)
Rails.logger.error exception.backtrace.join("\n")
render json: {
error: "internal_server_error",
message: "Error: #{exception.message}"
message: "An unexpected error occurred"
}, status: :internal_server_error
end
end
2 changes: 1 addition & 1 deletion app/controllers/api/v1/sync_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ def create

render json: {
error: "internal_server_error",
message: "Error: #{e.message}"
message: "An unexpected error occurred"
}, status: :internal_server_error
end

Expand Down
2 changes: 1 addition & 1 deletion app/controllers/api/v1/trades_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -292,7 +292,7 @@ def log_and_render_error(action, exception)
Rails.logger.error exception.backtrace.join("\n")
render json: {
error: "internal_server_error",
message: "Error: #{exception.message}"
message: "An unexpected error occurred"
}, status: :internal_server_error
end

Expand Down
10 changes: 5 additions & 5 deletions app/controllers/api/v1/transactions_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ def index

render json: {
error: "internal_server_error",
message: "Error: #{e.message}"
message: "An unexpected error occurred"
}, status: :internal_server_error
end

Expand All @@ -61,7 +61,7 @@ def show

render json: {
error: "internal_server_error",
message: "Error: #{e.message}"
message: "An unexpected error occurred"
}, status: :internal_server_error
end

Expand Down Expand Up @@ -102,7 +102,7 @@ def create

render json: {
error: "internal_server_error",
message: "Error: #{e.message}"
message: "An unexpected error occurred"
}, status: :internal_server_error
end

Expand Down Expand Up @@ -148,7 +148,7 @@ def update

render json: {
error: "internal_server_error",
message: "Error: #{e.message}"
message: "An unexpected error occurred"
}, status: :internal_server_error
end

Expand All @@ -171,7 +171,7 @@ def destroy

render json: {
error: "internal_server_error",
message: "Error: #{e.message}"
message: "An unexpected error occurred"
}, status: :internal_server_error
end

Expand Down
6 changes: 3 additions & 3 deletions app/controllers/api/v1/valuations_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ def show

render json: {
error: "internal_server_error",
message: "Error: #{e.message}"
message: "An unexpected error occurred"
}, status: :internal_server_error
end

Expand Down Expand Up @@ -100,7 +100,7 @@ def create

render json: {
error: "internal_server_error",
message: "Error: #{e.message}"
message: "An unexpected error occurred"
}, status: :internal_server_error
end

Expand Down Expand Up @@ -181,7 +181,7 @@ def update

render json: {
error: "internal_server_error",
message: "Error: #{e.message}"
message: "An unexpected error occurred"
}, status: :internal_server_error
end

Expand Down
4 changes: 2 additions & 2 deletions app/controllers/settings/providers_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -71,8 +71,8 @@ def update
redirect_to settings_providers_path, notice: "No changes were made"
end
rescue => error
Rails.logger.error("Failed to update provider settings: #{error.message}")
flash.now[:alert] = "Failed to update provider settings: #{error.message}"
Rails.logger.error("Failed to update provider settings: #{error.class} - #{error.message}")
flash.now[:alert] = "Failed to update provider settings. Please try again."
prepare_show_context
render :show, status: :unprocessable_entity
end
Expand Down
6 changes: 4 additions & 2 deletions app/controllers/webhooks_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,8 @@ def plaid
render json: { received: true }, status: :ok
rescue => error
Sentry.capture_exception(error)
render json: { error: "Invalid webhook: #{error.message}" }, status: :bad_request
Rails.logger.error("Webhook error: #{error.class} - #{error.message}")
render json: { error: "Invalid webhook" }, status: :bad_request
end

def plaid_eu
Expand All @@ -31,7 +32,8 @@ def plaid_eu
render json: { received: true }, status: :ok
rescue => error
Sentry.capture_exception(error)
render json: { error: "Invalid webhook: #{error.message}" }, status: :bad_request
Rails.logger.error("Webhook error: #{error.class} - #{error.message}")
render json: { error: "Invalid webhook" }, status: :bad_request
end

def stripe
Expand Down
19 changes: 19 additions & 0 deletions test/controllers/api/v1/auth_controller_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,25 @@ class Api::V1::AuthControllerTest < ActionDispatch::IntegrationTest
assert_equal "Device information is required", response_data["error"]
end

test "should reject signup with invalid device_type before committing any state" do
# Pre-validation catches bad device_type and returns 400 without creating
# user/family/device/token. Guards against a partial-commit state where the
# account exists but the mobile session handoff fails.
assert_no_difference([ "User.count", "MobileDevice.count", "Doorkeeper::AccessToken.count" ]) do
post "/api/v1/auth/signup", params: {
user: {
email: "newuser@example.com",
password: "SecurePass123!",
first_name: "New",
last_name: "User"
},
device: @device_info.merge(device_type: "windows") # not in allowlist
}
end

assert_response :bad_request
end

test "should not signup with invalid password" do
assert_no_difference("User.count") do
post "/api/v1/auth/signup", params: {
Expand Down
9 changes: 5 additions & 4 deletions test/controllers/settings/providers_controller_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -270,16 +270,17 @@ class Settings::ProvidersControllerTest < ActionDispatch::IntegrationTest
# We'll force an error by making the []= method raise
Setting.expects(:[]=).with("plaid_client_id", "test").raises(StandardError.new("Database error")).once

# Mock logger to verify error is logged
Rails.logger.expects(:error).with(regexp_matches(/Failed to update provider settings.*Database error/)).once
# Mock logger to verify error is logged (pin both the exception class
# name and the message so a regression that drops one still fails).
Rails.logger.expects(:error).with(regexp_matches(/Failed to update provider settings: StandardError - Database error/)).once

patch settings_providers_url, params: {
setting: { plaid_client_id: "test" }
}

# Controller should handle the error gracefully
# Controller should handle the error gracefully with generic message (no internal details)
assert_response :unprocessable_entity
assert_equal "Failed to update provider settings: Database error", flash[:alert]
assert_equal "Failed to update provider settings. Please try again.", flash[:alert]
end
end

Expand Down
Loading