Add explicit wrap_parameters to all controllers (#2680)
* Add explicit wrap_parameters to controllers for flat JSON API support Virtual attributes (has_rich_text, has_one_attached, delegated setters, ActiveModel attrs) are not in Model.attribute_names, so wrap_parameters auto-detection silently drops them from flat JSON requests. Add explicit include: lists matching each controller's permitted params. * Convert short-form wrap_parameters to explicit include: lists Defense-in-depth: these controllers only have real-column params today, so auto-detection works, but explicit lists prevent future regressions if virtual attributes are added. * Add flat JSON param tests for all wrap_parameters controllers 17 new tests covering every controller with wrap_parameters, verifying that flat (unwrapped) JSON payloads are correctly wrapped and processed. Focuses on virtual attributes that would be silently dropped without explicit include: lists.
This commit is contained in:
@@ -1,5 +1,5 @@
|
|||||||
class Account::JoinCodesController < ApplicationController
|
class Account::JoinCodesController < ApplicationController
|
||||||
wrap_parameters :account_join_code
|
wrap_parameters :account_join_code, include: %i[ usage_limit ]
|
||||||
|
|
||||||
before_action :set_join_code
|
before_action :set_join_code
|
||||||
before_action :ensure_admin, only: %i[ update destroy ]
|
before_action :ensure_admin, only: %i[ update destroy ]
|
||||||
|
|||||||
@@ -1,5 +1,5 @@
|
|||||||
class Account::SettingsController < ApplicationController
|
class Account::SettingsController < ApplicationController
|
||||||
wrap_parameters :account
|
wrap_parameters :account, include: %i[ name ]
|
||||||
|
|
||||||
before_action :ensure_admin, only: :update
|
before_action :ensure_admin, only: :update
|
||||||
before_action :set_account
|
before_action :set_account
|
||||||
|
|||||||
@@ -1,4 +1,6 @@
|
|||||||
class Boards::ColumnsController < ApplicationController
|
class Boards::ColumnsController < ApplicationController
|
||||||
|
wrap_parameters :column, include: %i[ name color ]
|
||||||
|
|
||||||
include BoardScoped
|
include BoardScoped
|
||||||
|
|
||||||
before_action :set_column, only: %i[ show update destroy ]
|
before_action :set_column, only: %i[ show update destroy ]
|
||||||
|
|||||||
@@ -1,4 +1,6 @@
|
|||||||
class BoardsController < ApplicationController
|
class BoardsController < ApplicationController
|
||||||
|
wrap_parameters :board, include: %i[ name all_access auto_postpone_period_in_days public_description ]
|
||||||
|
|
||||||
include FilterScoped
|
include FilterScoped
|
||||||
|
|
||||||
before_action :set_board, except: %i[ index new create ]
|
before_action :set_board, except: %i[ index new create ]
|
||||||
|
|||||||
@@ -1,4 +1,6 @@
|
|||||||
class Cards::Comments::ReactionsController < ApplicationController
|
class Cards::Comments::ReactionsController < ApplicationController
|
||||||
|
wrap_parameters :reaction, include: %i[ content ]
|
||||||
|
|
||||||
include CardScoped
|
include CardScoped
|
||||||
|
|
||||||
before_action :set_comment
|
before_action :set_comment
|
||||||
|
|||||||
@@ -1,4 +1,6 @@
|
|||||||
class Cards::ReactionsController < ApplicationController
|
class Cards::ReactionsController < ApplicationController
|
||||||
|
wrap_parameters :reaction, include: %i[ content ]
|
||||||
|
|
||||||
include CardScoped
|
include CardScoped
|
||||||
|
|
||||||
before_action :set_reactable
|
before_action :set_reactable
|
||||||
|
|||||||
@@ -1,4 +1,6 @@
|
|||||||
class Cards::StepsController < ApplicationController
|
class Cards::StepsController < ApplicationController
|
||||||
|
wrap_parameters :step, include: %i[ content completed ]
|
||||||
|
|
||||||
include CardScoped
|
include CardScoped
|
||||||
|
|
||||||
before_action :set_step, only: %i[ show edit update destroy ]
|
before_action :set_step, only: %i[ show edit update destroy ]
|
||||||
|
|||||||
@@ -1,4 +1,6 @@
|
|||||||
class CardsController < ApplicationController
|
class CardsController < ApplicationController
|
||||||
|
wrap_parameters :card, include: %i[ title description image created_at last_active_at ]
|
||||||
|
|
||||||
include FilterScoped
|
include FilterScoped
|
||||||
|
|
||||||
before_action :set_board, only: %i[ create ]
|
before_action :set_board, only: %i[ create ]
|
||||||
|
|||||||
@@ -1,4 +1,6 @@
|
|||||||
class My::AccessTokensController < ApplicationController
|
class My::AccessTokensController < ApplicationController
|
||||||
|
wrap_parameters :access_token, include: %i[ description permission ]
|
||||||
|
|
||||||
skip_before_action :require_account
|
skip_before_action :require_account
|
||||||
|
|
||||||
def index
|
def index
|
||||||
|
|||||||
@@ -1,5 +1,5 @@
|
|||||||
class Notifications::SettingsController < ApplicationController
|
class Notifications::SettingsController < ApplicationController
|
||||||
wrap_parameters :user_settings
|
wrap_parameters :user_settings, include: %i[ bundle_email_frequency ]
|
||||||
|
|
||||||
before_action :set_settings
|
before_action :set_settings
|
||||||
|
|
||||||
|
|||||||
@@ -1,4 +1,6 @@
|
|||||||
class Signups::CompletionsController < ApplicationController
|
class Signups::CompletionsController < ApplicationController
|
||||||
|
wrap_parameters :signup, include: %i[ full_name ]
|
||||||
|
|
||||||
layout "public"
|
layout "public"
|
||||||
|
|
||||||
disallow_account_scope
|
disallow_account_scope
|
||||||
|
|||||||
@@ -1,4 +1,6 @@
|
|||||||
class SignupsController < ApplicationController
|
class SignupsController < ApplicationController
|
||||||
|
wrap_parameters :signup, include: %i[ email_address ]
|
||||||
|
|
||||||
disallow_account_scope
|
disallow_account_scope
|
||||||
allow_unauthenticated_access
|
allow_unauthenticated_access
|
||||||
rate_limit to: 10, within: 3.minutes, only: :create, with: -> { redirect_to new_signup_path, alert: "Try again later." }
|
rate_limit to: 10, within: 3.minutes, only: :create, with: -> { redirect_to new_signup_path, alert: "Try again later." }
|
||||||
|
|||||||
@@ -1,4 +1,6 @@
|
|||||||
class Users::JoinsController < ApplicationController
|
class Users::JoinsController < ApplicationController
|
||||||
|
wrap_parameters :user, include: %i[ name avatar ]
|
||||||
|
|
||||||
layout "public"
|
layout "public"
|
||||||
|
|
||||||
def new
|
def new
|
||||||
|
|||||||
@@ -1,4 +1,6 @@
|
|||||||
class Users::PushSubscriptionsController < ApplicationController
|
class Users::PushSubscriptionsController < ApplicationController
|
||||||
|
wrap_parameters :push_subscription, include: %i[ endpoint p256dh_key auth_key ]
|
||||||
|
|
||||||
before_action :set_push_subscriptions
|
before_action :set_push_subscriptions
|
||||||
|
|
||||||
def index
|
def index
|
||||||
|
|||||||
@@ -1,5 +1,5 @@
|
|||||||
class Users::RolesController < ApplicationController
|
class Users::RolesController < ApplicationController
|
||||||
wrap_parameters :user
|
wrap_parameters :user, include: %i[ role ]
|
||||||
|
|
||||||
before_action :set_user
|
before_action :set_user
|
||||||
before_action :ensure_permission_to_administer_user
|
before_action :ensure_permission_to_administer_user
|
||||||
|
|||||||
@@ -1,4 +1,6 @@
|
|||||||
class UsersController < ApplicationController
|
class UsersController < ApplicationController
|
||||||
|
wrap_parameters :user, include: %i[ name avatar ]
|
||||||
|
|
||||||
before_action :set_user, except: %i[ index ]
|
before_action :set_user, except: %i[ index ]
|
||||||
before_action :ensure_permission_to_change_user, only: %i[ update destroy ]
|
before_action :ensure_permission_to_change_user, only: %i[ update destroy ]
|
||||||
|
|
||||||
|
|||||||
@@ -1,4 +1,6 @@
|
|||||||
class WebhooksController < ApplicationController
|
class WebhooksController < ApplicationController
|
||||||
|
wrap_parameters :webhook, include: %i[ name url subscribed_actions ]
|
||||||
|
|
||||||
include BoardScoped
|
include BoardScoped
|
||||||
|
|
||||||
before_action :ensure_admin
|
before_action :ensure_admin
|
||||||
|
|||||||
@@ -62,6 +62,189 @@ class FlatJsonParamsTest < ActionDispatch::IntegrationTest
|
|||||||
assert_response :created
|
assert_response :created
|
||||||
end
|
end
|
||||||
|
|
||||||
|
test "create card with flat JSON" do
|
||||||
|
assert_difference -> { Card.count }, +1 do
|
||||||
|
post board_cards_path(boards(:writebook)),
|
||||||
|
params: { title: "Flat card", description: "<p>Flat description</p>" },
|
||||||
|
as: :json
|
||||||
|
end
|
||||||
|
|
||||||
|
assert_response :created
|
||||||
|
card = Card.last
|
||||||
|
assert_equal "Flat card", card.title
|
||||||
|
assert_equal "Flat description", card.description.to_plain_text
|
||||||
|
end
|
||||||
|
|
||||||
|
test "update card with flat JSON" do
|
||||||
|
card = cards(:logo)
|
||||||
|
|
||||||
|
put card_path(card),
|
||||||
|
params: { title: "Flat update", description: "<p>Updated flat</p>" },
|
||||||
|
as: :json
|
||||||
|
|
||||||
|
assert_response :success
|
||||||
|
card.reload
|
||||||
|
assert_equal "Flat update", card.title
|
||||||
|
assert_equal "Updated flat", card.description.to_plain_text
|
||||||
|
end
|
||||||
|
|
||||||
|
test "create board with flat JSON" do
|
||||||
|
assert_difference -> { Board.count }, +1 do
|
||||||
|
post boards_path, params: { name: "Flat board" }, as: :json
|
||||||
|
end
|
||||||
|
|
||||||
|
assert_response :created
|
||||||
|
assert_equal "Flat board", Board.last.name
|
||||||
|
end
|
||||||
|
|
||||||
|
test "update board with flat JSON" do
|
||||||
|
board = boards(:writebook)
|
||||||
|
|
||||||
|
put board_path(board),
|
||||||
|
params: { name: "Flat board", auto_postpone_period_in_days: 7, public_description: "<p>Flat public desc</p>" },
|
||||||
|
as: :json
|
||||||
|
|
||||||
|
assert_response :no_content
|
||||||
|
board.reload
|
||||||
|
assert_equal "Flat board", board.name
|
||||||
|
assert_equal 7.days, board.entropy.auto_postpone_period
|
||||||
|
assert_equal "Flat public desc", board.public_description.to_plain_text
|
||||||
|
end
|
||||||
|
|
||||||
|
test "create column with flat JSON" do
|
||||||
|
board = boards(:writebook)
|
||||||
|
|
||||||
|
assert_difference -> { board.columns.count }, +1 do
|
||||||
|
post board_columns_path(board), params: { name: "Flat Column" }, as: :json
|
||||||
|
end
|
||||||
|
|
||||||
|
assert_response :created
|
||||||
|
assert_equal "Flat Column", Column.last.name
|
||||||
|
end
|
||||||
|
|
||||||
|
test "update column with flat JSON" do
|
||||||
|
column = columns(:writebook_in_progress)
|
||||||
|
|
||||||
|
put board_column_path(column.board, column), params: { name: "Flat Updated" }, as: :json
|
||||||
|
|
||||||
|
assert_response :no_content
|
||||||
|
assert_equal "Flat Updated", column.reload.name
|
||||||
|
end
|
||||||
|
|
||||||
|
test "create step with flat JSON" do
|
||||||
|
card = cards(:logo)
|
||||||
|
|
||||||
|
assert_difference -> { card.steps.count }, +1 do
|
||||||
|
post card_steps_path(card), params: { content: "Flat step" }, as: :json
|
||||||
|
end
|
||||||
|
|
||||||
|
assert_response :created
|
||||||
|
assert_equal "Flat step", Step.last.content
|
||||||
|
end
|
||||||
|
|
||||||
|
test "update step with flat JSON" do
|
||||||
|
card = cards(:logo)
|
||||||
|
step = card.steps.create!(content: "Original")
|
||||||
|
|
||||||
|
put card_step_path(card, step), params: { content: "Flat updated" }, as: :json
|
||||||
|
|
||||||
|
assert_response :success
|
||||||
|
assert_equal "Flat updated", step.reload.content
|
||||||
|
end
|
||||||
|
|
||||||
|
test "create card reaction with flat JSON" do
|
||||||
|
card = cards(:logo)
|
||||||
|
|
||||||
|
assert_difference -> { card.reactions.count }, +1 do
|
||||||
|
post card_reactions_path(card), params: { content: "🎉" }, as: :json
|
||||||
|
end
|
||||||
|
|
||||||
|
assert_response :created
|
||||||
|
end
|
||||||
|
|
||||||
|
test "create comment reaction with flat JSON" do
|
||||||
|
comment = comments(:logo_agreement_kevin)
|
||||||
|
|
||||||
|
assert_difference -> { comment.reactions.count }, +1 do
|
||||||
|
post card_comment_reactions_path(comment.card, comment), params: { content: "👍" }, as: :json
|
||||||
|
end
|
||||||
|
|
||||||
|
assert_response :created
|
||||||
|
end
|
||||||
|
|
||||||
|
test "create access token with flat JSON" do
|
||||||
|
assert_difference -> { identities(:kevin).access_tokens.count }, +1 do
|
||||||
|
post my_access_tokens_path, params: { description: "Flat token", permission: "read" }, as: :json
|
||||||
|
end
|
||||||
|
|
||||||
|
assert_response :created
|
||||||
|
assert_equal "Flat token", @response.parsed_body["description"]
|
||||||
|
end
|
||||||
|
|
||||||
|
test "update user with flat JSON" do
|
||||||
|
put user_path(users(:david)), params: { name: "Flat Name" }, as: :json
|
||||||
|
|
||||||
|
assert_response :no_content
|
||||||
|
assert_equal "Flat Name", users(:david).reload.name
|
||||||
|
end
|
||||||
|
|
||||||
|
test "create webhook with flat JSON" do
|
||||||
|
board = boards(:writebook)
|
||||||
|
|
||||||
|
assert_difference -> { Webhook.count }, +1 do
|
||||||
|
post board_webhooks_path(board),
|
||||||
|
params: { name: "Flat Webhook", url: "https://example.com/flat", subscribed_actions: [ "card_published" ] },
|
||||||
|
as: :json
|
||||||
|
end
|
||||||
|
|
||||||
|
assert_response :created
|
||||||
|
assert_equal "Flat Webhook", Webhook.last.name
|
||||||
|
end
|
||||||
|
|
||||||
|
test "update webhook with flat JSON" do
|
||||||
|
webhook = webhooks(:active)
|
||||||
|
|
||||||
|
patch board_webhook_path(webhook.board, webhook),
|
||||||
|
params: { name: "Flat Updated", subscribed_actions: [ "card_published" ] },
|
||||||
|
as: :json
|
||||||
|
|
||||||
|
assert_response :success
|
||||||
|
assert_equal "Flat Updated", webhook.reload.name
|
||||||
|
end
|
||||||
|
|
||||||
|
test "create signup with flat JSON" do
|
||||||
|
sign_out
|
||||||
|
email = "flatjson-#{SecureRandom.hex(6)}@example.com"
|
||||||
|
|
||||||
|
untenanted do
|
||||||
|
assert_difference -> { Identity.count }, +1 do
|
||||||
|
post signup_path, params: { email_address: email }, as: :json
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
test "complete signup with flat JSON" do
|
||||||
|
signup = Signup.new(email_address: "flatjson-#{SecureRandom.hex(6)}@example.com", full_name: "Flat User")
|
||||||
|
signup.create_identity || raise("Failed to create identity")
|
||||||
|
logout_and_sign_in_as signup.identity
|
||||||
|
|
||||||
|
untenanted do
|
||||||
|
assert_difference -> { Account.count }, +1 do
|
||||||
|
post signup_completion_path, params: { full_name: "Flat JSON User" }, as: :json
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
assert_response :created
|
||||||
|
end
|
||||||
|
|
||||||
|
test "update user via join with flat JSON" do
|
||||||
|
logout_and_sign_in_as :david
|
||||||
|
|
||||||
|
post users_joins_path, params: { name: "Flat Join" }, as: :json
|
||||||
|
|
||||||
|
assert_equal "Flat Join", users(:david).reload.name
|
||||||
|
end
|
||||||
|
|
||||||
private
|
private
|
||||||
def stub_dns_resolution(*ips)
|
def stub_dns_resolution(*ips)
|
||||||
dns_mock = mock("dns")
|
dns_mock = mock("dns")
|
||||||
|
|||||||
Reference in New Issue
Block a user