From 607f65a0a50f5f4618ec998833e58b7b665677e9 Mon Sep 17 00:00:00 2001 From: Matt Jankowski Date: Wed, 22 Jan 2025 09:55:44 -0500 Subject: [PATCH] Use `expect` for api/v1 and api/web push subs controllers (#33682) --- .../api/v1/push/subscriptions_controller.rb | 4 ++-- .../api/web/push_subscriptions_controller.rb | 4 ++-- .../api/v1/push/subscriptions_spec.rb | 14 +++++++++++ .../api/web/push_subscriptions_spec.rb | 24 +++++++++++++++++++ 4 files changed, 42 insertions(+), 4 deletions(-) diff --git a/app/controllers/api/v1/push/subscriptions_controller.rb b/app/controllers/api/v1/push/subscriptions_controller.rb index d74b5d958f..f2c52f2846 100644 --- a/app/controllers/api/v1/push/subscriptions_controller.rb +++ b/app/controllers/api/v1/push/subscriptions_controller.rb @@ -56,12 +56,12 @@ class Api::V1::Push::SubscriptionsController < Api::BaseController end def subscription_params - params.require(:subscription).permit(:endpoint, :standard, keys: [:auth, :p256dh]) + params.expect(subscription: [:endpoint, :standard, keys: [:auth, :p256dh]]) end def data_params return {} if params[:data].blank? - params.require(:data).permit(:policy, alerts: Notification::TYPES) + params.expect(data: [:policy, alerts: Notification::TYPES]) end end diff --git a/app/controllers/api/web/push_subscriptions_controller.rb b/app/controllers/api/web/push_subscriptions_controller.rb index 7eb51c6846..2711071b4a 100644 --- a/app/controllers/api/web/push_subscriptions_controller.rb +++ b/app/controllers/api/web/push_subscriptions_controller.rb @@ -66,7 +66,7 @@ class Api::Web::PushSubscriptionsController < Api::Web::BaseController end def subscription_params - @subscription_params ||= params.require(:subscription).permit(:standard, :endpoint, keys: [:auth, :p256dh]) + @subscription_params ||= params.expect(subscription: [:standard, :endpoint, keys: [:auth, :p256dh]]) end def web_push_subscription_params @@ -82,6 +82,6 @@ class Api::Web::PushSubscriptionsController < Api::Web::BaseController end def data_params - @data_params ||= params.require(:data).permit(:policy, alerts: Notification::TYPES) + @data_params ||= params.expect(data: [:policy, alerts: Notification::TYPES]) end end diff --git a/spec/requests/api/v1/push/subscriptions_spec.rb b/spec/requests/api/v1/push/subscriptions_spec.rb index 15d469d7a9..f2b457705e 100644 --- a/spec/requests/api/v1/push/subscriptions_spec.rb +++ b/spec/requests/api/v1/push/subscriptions_spec.rb @@ -107,6 +107,13 @@ RSpec.describe 'API V1 Push Subscriptions' do it_behaves_like 'validation error' end + + it 'gracefully handles invalid nested params' do + post api_v1_push_subscription_path, params: { subscription: 'invalid' }, headers: headers + + expect(response) + .to have_http_status(400) + end end describe 'PUT /api/v1/push/subscription' do @@ -133,6 +140,13 @@ RSpec.describe 'API V1 Push Subscriptions' do policy: alerts_payload[:data][:policy] ) end + + it 'gracefully handles invalid nested params' do + put api_v1_push_subscription_path(endpoint_push_subscription), params: { data: 'invalid' }, headers: headers + + expect(response) + .to have_http_status(400) + end end describe 'GET /api/v1/push/subscription' do diff --git a/spec/requests/api/web/push_subscriptions_spec.rb b/spec/requests/api/web/push_subscriptions_spec.rb index a903dc6f89..42545b3d6e 100644 --- a/spec/requests/api/web/push_subscriptions_spec.rb +++ b/spec/requests/api/web/push_subscriptions_spec.rb @@ -52,4 +52,28 @@ RSpec.describe 'API Web Push Subscriptions' do end end end + + describe 'POST /api/web/push_subscriptions' do + before { sign_in Fabricate :user } + + it 'gracefully handles invalid nested params' do + post api_web_push_subscriptions_path, params: { subscription: 'invalid' } + + expect(response) + .to have_http_status(400) + end + end + + describe 'PUT /api/web/push_subscriptions' do + before { sign_in Fabricate :user } + + let(:subscription) { Fabricate :web_push_subscription } + + it 'gracefully handles invalid nested params' do + put api_web_push_subscription_path(subscription), params: { data: 'invalid' } + + expect(response) + .to have_http_status(400) + end + end end