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

Block public access to some expensive parameters #1084

Merged

Conversation

Mr0grog
Copy link
Member

@Mr0grog Mr0grog commented Feb 3, 2023

Some parameters for the various /versions collections cause expensive queries, so they are disallowed for public, non-logged-in usage.

Solves part of #1070.

Some parameters for the various `/versions` collections cause expensive queries, so they are disallowed for public, non-logged-in usage.

Solves part of #1070.
@Mr0grog
Copy link
Member Author

Mr0grog commented Feb 3, 2023

Thinking about this some more, it might still make sense to allow include_change_from_* for #show. We could do this by changing block_params_for_public_users to take a first argument indicating which methods:

# First argument indicates which methods:
block_params_for_public_users :all, [:bad, :params]
block_params_for_public_users [:index, :sampled], [:other, :bad, :params]

# Or as a hash:
block_params_for_public_users {
  all => [:bad, :params],
  [:index, :sampled] => [:other, :bad, :params]
}

# Or just provide a method to use with `before_action`:
before_action do
  block_params_for_public_users [:bad, :params]
end
before_action(only: [:index, :sampled]) do
  block_params_for_public_users [:other, :bad, :params]
end

@Mr0grog Mr0grog changed the title Block public access to some /versions parameters Block public access to some expensive parameters Feb 6, 2023
@Mr0grog
Copy link
Member Author

Mr0grog commented Feb 6, 2023

This now covers all controllers, and therefore fixes #1070.

I also wound up going with a slightly different, more explicit style:

block_params_for_public_users actions: :all,
                              params: [:source_metadata, :status]
block_params_for_public_users actions: [:index, :sampled],
                              params: [
                                :include_change_from_previous,
                                :include_change_from_earliest
                              ]

@Mr0grog Mr0grog linked an issue Feb 6, 2023 that may be closed by this pull request
@Mr0grog Mr0grog merged commit e02fd14 into main Feb 7, 2023
@Mr0grog Mr0grog deleted the 1070-some-parameters-are-just-too-expensive-to-be-public branch February 7, 2023 00:02
Mr0grog added a commit that referenced this pull request Feb 7, 2023
Mr0grog added a commit to edgi-govdata-archiving/web-monitoring-ops that referenced this pull request Feb 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Audit API for parameters that can heavily de-optimize requests
1 participant