Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Convert params.permit to params.expect #5357

Merged
merged 5 commits into from
Dec 22, 2024
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
1 change: 1 addition & 0 deletions .rubocop.yml
Original file line number Diff line number Diff line change
Expand Up @@ -272,6 +272,7 @@ Style/CaseLikeIf:

Style/HashAsLastArrayItem:
Enabled: true
EnforcedStyle: no_braces

Style/HashLikeCase:
Enabled: true
Expand Down
2 changes: 1 addition & 1 deletion app/avo/concerns/auditable.rb
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ def in_audited_transaction(auditable:, admin_github_user:, action:, fields:, arg
changes = merge_changes!(changes, record.attributes.slice("id").transform_values { [_1, _1] }) if changes.key?("id")
changes = merge_changes!(changes, record.attributes.compact.transform_values { [_1, nil] }) if record.destroyed?

[key, { changes:, unchanged: record.attributes.except(*changes.keys) }]
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rubocop side-effect. Not related to this change but necessary to keep lint passing (while also improving how expect syntax is used)

[key, changes:, unchanged: record.attributes.except(*changes.keys)]
end

audit = Audit.create!(
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/api/v1/api_keys_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ def otp
end

def key_param
params.permit(:api_key).require(:api_key)
params.expect(:api_key)
end

def api_key_create_params
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/api/v1/deletions_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ def validate_gem_and_version
render_forbidden response_with_mfa_warning("You do not have permission to delete this gem.")
else
begin
version = params.permit(:version).require(:version)
version = params.expect(:version)
platform = params.permit(:platform).fetch(:platform, nil)
@version = @rubygem.find_version!(number: version, platform: platform)
rescue ActiveRecord::RecordNotFound
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ def revoke
return render plain: "Can't fetch public key from GitHub", status: :unauthorized if key.empty_public_key?
return render plain: "Invalid GitHub Signature", status: :unauthorized unless key.valid_github_signature?(signature, request.body.read.chomp)

tokens = params.permit(_json: %i[token type url]).require(:_json).index_by { |t| hashed_key(t.require(:token)) }
tokens = params.expect(_json: [%i[token type url]]).index_by { |t| hashed_key(t.require(:token)) }
api_keys = ApiKey.where(hashed_key: tokens.keys).index_by(&:hashed_key)
resp = tokens.map do |hashed_key, t|
api_key = api_keys[hashed_key]
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/api/v1/hook_relay_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ def hook_relay_report_params
end

def authenticate_hook_relay_report
account_id, hook_id = params.permit(%i[account_id hook_id]).require(%i[account_id hook_id])
account_id, hook_id = params.expect(%i[account_id hook_id])

ActiveSupport::SecureCompareRotator.new(ENV.fetch("HOOK_RELAY_ACCOUNT_ID", "")).secure_compare!(account_id)
ActiveSupport::SecureCompareRotator.new(ENV.fetch("HOOK_RELAY_HOOK_ID", "")).secure_compare!(hook_id)
Expand Down
4 changes: 2 additions & 2 deletions app/controllers/api/v1/oidc/api_key_roles_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ def index
end

def show
render json: @api_key.user.oidc_api_key_roles.find_by!(token: params.permit(:token).require(:token))
render json: @api_key.user.oidc_api_key_roles.find_by!(token: params.expect(:token))
end

def assume_role
Expand Down Expand Up @@ -60,7 +60,7 @@ def assume_role
private

def set_api_key_role
@api_key_role = OIDC::ApiKeyRole.active.find_by!(token: params.permit(:token).require(:token))
@api_key_role = OIDC::ApiKeyRole.active.find_by!(token: params.expect(:token))
@provider = @api_key_role.provider
end

Expand Down
2 changes: 1 addition & 1 deletion app/controllers/api/v1/oidc/id_tokens_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,6 @@ def index
end

def show
render json: @api_key.user.oidc_id_tokens.find(params.permit(:id).require(:id))
render json: @api_key.user.oidc_id_tokens.find(params[:id])
end
end
2 changes: 1 addition & 1 deletion app/controllers/api/v1/oidc/providers_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,6 @@ def index
end

def show
render json: OIDC::Provider.find(params.permit(:id).require(:id))
render json: OIDC::Provider.find(params[:id])
end
end
Original file line number Diff line number Diff line change
Expand Up @@ -39,11 +39,11 @@ def find_rubygem
end

def find_rubygem_trusted_publisher
@rubygem_trusted_publisher = @rubygem.oidc_rubygem_trusted_publishers.find(params.permit(:id).require(:id))
@rubygem_trusted_publisher = @rubygem.oidc_rubygem_trusted_publishers.find(params[:id])
end

def set_trusted_publisher_type
trusted_publisher_type = params.permit(:trusted_publisher_type).require(:trusted_publisher_type)
trusted_publisher_type = params.expect(:trusted_publisher_type)

@trusted_publisher_type = OIDC::TrustedPublisher.all.find { |type| type.polymorphic_name == trusted_publisher_type }

Expand Down
2 changes: 1 addition & 1 deletion app/controllers/api/v1/owners_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ def render_not_found
end

def email_param
params.permit(:email).require(:email)
params.expect(:email)
end

def ownership_params
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/api/v1/rubygems_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ def create

gem_body = attestations = nil
if %w[multipart/form-data multipart/mixed].include?(request.media_type)
gem_body = params.permit(:gem).require(:gem)
gem_body = params.expect(:gem)
return render_bad_request("gem is not a file upload") unless gem_body.is_a?(ActionDispatch::Http::UploadedFile)
return render_bad_request("missing attestations") unless (attestations = params[:attestations]).is_a?(String)
attestations = ActiveSupport::JSON.decode(attestations)
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/api/v1/searches_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,6 @@ def search_not_available_error(error)
end

def query_params
params.permit(:query).require(:query)
params.expect(:query)
end
end
2 changes: 1 addition & 1 deletion app/controllers/api/v1/timeframe_versions_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ def ensure_valid_timerange
end

def from_time
@from_time ||= Time.iso8601(params.permit(:from).require(:from))
@from_time ||= Time.iso8601(params.expect(:from))
rescue ArgumentError
raise InvalidTimeframeParameterError, "the from parameter must be iso8601 formatted"
end
Expand Down
12 changes: 6 additions & 6 deletions app/controllers/api_keys_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
end

def edit
@api_key = current_user.api_keys.find(params.permit(:id).require(:id))
@api_key = current_user.api_keys.find(params[:id])
return unless @api_key.soft_deleted?

flash[:error] = t(".invalid_key")
Expand Down Expand Up @@ -48,7 +48,7 @@
end

def update
@api_key = current_user.api_keys.find(params.permit(:id).require(:id))
@api_key = current_user.api_keys.find(params[:id])
@api_key.assign_attributes(api_key_update_params(@api_key))

if @api_key.errors.present?
Expand All @@ -65,7 +65,7 @@
end

def destroy
api_key = current_user.api_keys.find(params.permit(:id).require(:id))
api_key = current_user.api_keys.find(params[:id])

if api_key.expire!
flash[:notice] = t(".success", name: api_key.name)
Expand Down Expand Up @@ -93,19 +93,19 @@
when "create"
new_profile_api_key_path
when "update"
edit_profile_api_key_path(params.permit(:id).require(:id))
edit_profile_api_key_path(params[:id])

Check warning on line 96 in app/controllers/api_keys_controller.rb

View check run for this annotation

Codecov / codecov/patch

app/controllers/api_keys_controller.rb#L96

Added line #L96 was not covered by tests
else
super
end
end

def api_key_create_params
ApiKeysHelper.api_key_params(params.permit(api_key: [:name, *ApiKey::API_SCOPES, :mfa, :rubygem_id, :expires_at]).require(:api_key))
ApiKeysHelper.api_key_params(params.expect(api_key: [:name, *ApiKey::API_SCOPES, :mfa, :rubygem_id, :expires_at]))
end

def api_key_update_params(existing_api_key = nil)
ApiKeysHelper.api_key_params(
params.permit(api_key: [*ApiKey::API_SCOPES, :mfa, :rubygem_id, { scopes: [ApiKey::API_SCOPES] }]).require(:api_key), existing_api_key
params.expect(api_key: [*ApiKey::API_SCOPES, :mfa, :rubygem_id, scopes: ApiKey::API_SCOPES]), existing_api_key
)
end
end
2 changes: 1 addition & 1 deletion app/controllers/concerns/jwt_validation.rb
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ def jwt_key_or_secret
end

def decode_jwt
@jwt = JSON::JWT.decode_compact_serialized(params.permit(:jwt).require(:jwt), jwt_key_or_secret)
@jwt = JSON::JWT.decode_compact_serialized(params.expect(:jwt), jwt_key_or_secret)
rescue JSON::JWT::InvalidFormat, JSON::ParserError, ArgumentError => e
# invalid base64 raises ArgumentError
render_bad_request(e)
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/concerns/latest_version.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ def latest_version
end

def latest_version_by_slug
@latest_version = @rubygem.find_version_by_slug!(params.permit(:version_id).require(:version_id))
@latest_version = @rubygem.find_version_by_slug!(params.expect(:version_id))
end
end
end
4 changes: 2 additions & 2 deletions app/controllers/concerns/webauthn_verifiable.rb
Original file line number Diff line number Diff line change
Expand Up @@ -64,14 +64,14 @@ def challenge
end

def credential_params
params.permit(credentials: PERMITTED_CREDENTIALS).require(:credentials)
params.expect(credentials: PERMITTED_CREDENTIALS)
end

PERMITTED_CREDENTIALS = [
:id,
:type,
:rawId,
:authenticatorAttachment,
{ response: %i[authenticatorData attestationObject clientDataJSON signature userHandle] }
response: %i[authenticatorData attestationObject clientDataJSON signature userHandle]
].freeze
end
4 changes: 2 additions & 2 deletions app/controllers/email_confirmations_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -72,11 +72,11 @@ def confirm_email
end

def email_params
params.permit(email_confirmation: :email).require(:email_confirmation).require(:email)
params.expect(email_confirmation: :email).require(:email)
end

def token_params
params.permit(:token).require(:token)
params.expect(:token)
end

def login_failure(message)
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/multifactor_auths_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ def recovery
private

def level_param
params.permit(:level).require(:level)
params.expect(:level)
end

def issuer
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/notifiers_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ def update
private

def notifier_params
params.permit(ownerships: %i[push owner ownership_request]).require(:ownerships)
params.expect(ownerships: [%i[push owner ownership_request]])
end

def notifier_options(param)
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/oauth_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ def create
end

def failure
render_forbidden params.permit(:message).require(:message)
render_forbidden params.expect(:message)
end

def development_log_in_as
Expand Down
19 changes: 9 additions & 10 deletions app/controllers/oidc/api_key_roles_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ def verify_session_redirect_path
def find_api_key_role
@api_key_role = current_user.oidc_api_key_roles
.includes(:provider)
.find_by!(token: params.permit(:token).require(:token))
.find_by!(token: params.expect(:token))
end

def redirect_for_deleted
Expand All @@ -113,19 +113,18 @@ def redirect_for_deleted
PERMITTED_API_KEY_ROLE_PARAMS = [
:name,
:oidc_provider_id,
{
api_key_permissions: [:valid_for, { scopes: [], gems: [] }],
access_policy: {
statements_attributes: [
:effect,
{ principal: :oidc, conditions_attributes: %i[operator claim value] }
]
}
api_key_permissions: [:valid_for, scopes: [], gems: []],
access_policy: {
statements_attributes: [[
:effect,
principal: :oidc,
conditions_attributes: [%i[operator claim value]]
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @segiddins this line doesn't fail any tests either way, but it looks like this is how the code works. I guess we don't pass conditions in a test?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's possible

]]
}
].freeze

def api_key_role_params
params.permit(oidc_api_key_role: PERMITTED_API_KEY_ROLE_PARAMS).require(:oidc_api_key_role)
params.expect(oidc_api_key_role: PERMITTED_API_KEY_ROLE_PARAMS)
end

def add_default_params(rubygem, statement, condition)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ module OIDC::Concerns::TrustedPublisherCreation
end

def set_trusted_publisher_type
trusted_publisher_type = params.permit(create_params_key => :trusted_publisher_type).require(create_params_key).require(:trusted_publisher_type)
trusted_publisher_type = params.expect(create_params_key => :trusted_publisher_type).require(:trusted_publisher_type)

@trusted_publisher_type = OIDC::TrustedPublisher.all.find { |type| type.polymorphic_name == trusted_publisher_type }

Expand Down
2 changes: 1 addition & 1 deletion app/controllers/oidc/id_tokens_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,6 @@ def show
private

def find_id_token
@id_token = current_user.oidc_id_tokens.find(params.permit(:id).require(:id))
@id_token = current_user.oidc_id_tokens.find(params[:id])
end
end
8 changes: 4 additions & 4 deletions app/controllers/oidc/pending_trusted_publishers_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -48,18 +48,18 @@ def destroy
private

def create_params
params.permit(
params.expect(
create_params_key => [
:rubygem_name,
:trusted_publisher_type,
{ trusted_publisher_attributes: @trusted_publisher_type.permitted_attributes }
trusted_publisher_attributes: @trusted_publisher_type.permitted_attributes
]
).require(create_params_key)
)
end

def create_params_key = :oidc_pending_trusted_publisher

def find_pending_trusted_publisher
@pending_trusted_publisher = authorize current_user.oidc_pending_trusted_publishers.find(params.permit(:id).require(:id))
@pending_trusted_publisher = authorize current_user.oidc_pending_trusted_publishers.find(params[:id])
end
end
2 changes: 1 addition & 1 deletion app/controllers/oidc/providers_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,6 @@ def show
private

def find_provider
@provider = OIDC::Provider.find(params.permit(:id).require(:id))
@provider = OIDC::Provider.find(params[:id])
end
end
8 changes: 4 additions & 4 deletions app/controllers/oidc/rubygem_trusted_publishers_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -41,12 +41,12 @@ def destroy
private

def create_params
params.permit(
params.expect(
create_params_key => [
:trusted_publisher_type,
{ trusted_publisher_attributes: @trusted_publisher_type.permitted_attributes }
trusted_publisher_attributes: @trusted_publisher_type.permitted_attributes
]
).require(create_params_key)
)
end

def create_params_key = :oidc_rubygem_trusted_publisher
Expand All @@ -57,7 +57,7 @@ def find_rubygem
end

def find_rubygem_trusted_publisher
@rubygem_trusted_publisher = authorize @rubygem.oidc_rubygem_trusted_publishers.find(params.permit(:id).require(:id))
@rubygem_trusted_publisher = authorize @rubygem.oidc_rubygem_trusted_publishers.find(params[:id])
end

def gh_actions_trusted_publisher
Expand Down
4 changes: 2 additions & 2 deletions app/controllers/organizations_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -46,10 +46,10 @@ def update
private

def find_organization
@organization = Organization.find_by_handle!(params.permit(:id).require(:id))
@organization = Organization.find_by_handle!(params[:id])
end

def organization_params
params.permit(organization: [:name]).require(:organization)
params.expect(organization: %i[name])
end
end
4 changes: 2 additions & 2 deletions app/controllers/owners_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -90,11 +90,11 @@ def find_ownership
end

def token_params
params.permit(:token).require(:token)
params.expect(:token)
end

def handle_params
params.permit(:handle).require(:handle)
params.expect(:handle)
end

def update_params
Expand Down
Loading