Commit Graph

4645 Commits

Author SHA1 Message Date
Tom Hughes
24ebaa4e83 Allow relative redirects when raise_on_open_redirects is enabled 2022-03-10 00:41:49 +00:00
Jean Boussier
9feaf7eaae Fix a typo in http_basic_authenticate_with 2022-03-04 16:10:38 +01:00
Jean Boussier
9ebfb149ed Better handle basic authentication without a password
https://github.com/rails/rails/pull/43209 immediately rejects
the request if no password is passed, but there are legitimate
uses for accepting authentication without a password.
2022-03-04 14:40:04 +01:00
Jean Boussier
2fd34270eb Eager load controllers view_context_class
These classes are relatively small, however they include lots of
modules as helpers. And if any of the included module hold constants
including it cause the global constant cache to be invalidated
which is really bad for performance.

So when eager loading is enabled we create all the possible classes
as part of the application boot.
2022-03-02 08:47:54 +01:00
Brad Trick
880a1bedb9 Allow skip_forgery_protection if no protection set
Calling `skip_forgery_protection` without first calling
`protect_from_forgery`--either manually or through default
settings--raises an `ArgumentError` because `verify_authenticity_token`
has not been defined as a callback.

Since Rails 7.0 adds `skip_forgery_protection` to the
`Rails::WelcomeController` (PR #42864), this behavior means that setting
`default_protect_from_forgery` to false and visiting the Rails Welcome
page (`/`) raises an error.

This behavior also created an issue for `ActionMailbox` that was
previously fixed in the Mailbox controller by running
`skip_forgery_protection` only if `default_protect_from_forgery` was
true (PR #35935).

This PR addresses the underlying issue by setting the `raise` option for
`skip_before_action` to default to false inside
`skip_forgery_protection`.

The fix is implemented in `request_forgery_protection.rb`. The change to
`ActionMailbox`'s `base_controller.rb` removes the now-unnecessary
check of `default_protect_from_forgery`.

The tests added in `request_forgery_protection_test.rb` and
`routing_test.rb` both raise an error when run against the current
codebase and pass with the changes noted above.
2022-02-27 21:58:42 -05:00
Rafael Mendonça França
eaccffaa8c
Merge pull request #44523 from SValkanov/apidocs_edit_actioncontroller_doc
Edit ActionController Base API docs [ci-skip]
2022-02-25 14:50:20 -05:00
Jon Dufresne
c2e756a944 Remove body content from redirect responses
Modern browsers don't render this HTML so it goes unused in practice.
The delivered bytes are therefore a small waste (although very small)
and unnecessary and could be optimized away.

Additionally, the HTML fails validation. Using the W3C v.Nu, we see the
following errors:

    Warning: Consider adding a lang attribute to the html start tag to declare the language of this document.

    Error: Start tag seen without seeing a doctype first. Expected <!DOCTYPE html>.

    Error: Element head is missing a required instance of child element title.

These errors may surface in site-wide compliance tests (either internal
tests or external contractual tests). Avoid the false positives by
removing the HTML.

While these warnings and errors could be resolved, it would be simpler
on future maintenance to remove the body altogether (especially as it
isn't rendered by the browser). As the same string is copied around a
few places, this removes multiple touch points to resolve the current
validation errors as well as new ones.

Many other frameworks and web servers don't include an HTML body on
redirect, so there isn't a reason for Rails to do so. By removing the
custom Rails HTML, there are fewing "fingerprints" that a malicious bot
could use to identify the backend technologies.

Application controllers that wish to add a response body after calling
redirect_to can continue to do so.
2022-02-25 13:31:54 -04:00
allergo
3ad9e57eda Edit ActionController Base API docs [ci-skip]
Fix information on Sessions - CookieStore
2022-02-24 00:02:44 +02:00
Jonathan Hefner
497ab719d0
Merge pull request #44509 from jonathanhefner/apidocs-cross-link-docs
Cross-link API docs [ci-skip]
2022-02-23 12:08:41 -06:00
Jonathan Hefner
8e080d04ce
Merge pull request #44505 from jonathanhefner/apidocs-improve-http-authentication-doc
Improve HTTP authentication API docs [ci-skip]
2022-02-23 12:06:56 -06:00
Petrik
94050d7dde Move PermissionsPolicy docs to ActionDispatch::HTTP::PermissionsPolicy [ci-skip]
As most of the PermissionsPolicy is defined in
ActionDispatch::HTTP::PermissionsPolicy, it should include most of the
documentation. ActionController::Metal::PermissionsPolicy should
describe controller overrides.

This PR also makes the documentation more similar to the
ActionDispatch::HTTP::ContentSecurityPolicy documentation.

Note:
The Feature-Policy header has been renamed to Permissions-Policy
in the specification. The Permissions-Policy requires a different
implementation and isn't yet supported by all browsers. To avoid
having to rename this middleware in the future we use the new
name for the middleware but keep the old header name in the
documentation for now.

Co-authored-by: Jonathan Hefner <jonathan@hefner.pro>
2022-02-22 21:27:39 +01:00
Petrik
646c631869 Remove fixed TODO ActionController::Metal::ContentSecurityPolicy [ci-skip]
The ActionController::Metal::ContentSecurityPolicy has documentation so
the TODO for documentation can be removed.
2022-02-21 21:10:47 +01:00
Jonathan Hefner
a199aaedb8 Cross-link API docs [ci-skip]
RDoc will automatically format and link API references as long as they
are not already marked up as inline code.

This commit removes markup from various API references so that those
references will link to the relevant API docs.
2022-02-21 11:45:25 -06:00
Jonathan Hefner
eb7a0fcec0 Improve HTTP authentication API docs [ci-skip]
This adds docs for a few public methods, and fixes a handful of
formatting issues.
2022-02-21 11:45:01 -06:00
Jonathan Hefner
9dbf7a58a2 Fix formatting of parameters doc [ci-skip] 2022-02-21 11:11:11 -06:00
Jonathan Hefner
a801aa7cde Mark up inline code [ci-skip] 2022-02-21 11:11:11 -06:00
Jonathan Hefner
e37adfed4e Add Oxford commas [ci-skip] 2022-02-21 11:11:11 -06:00
Jonathan Hefner
07bee949c4 Replace backticks with RDoc markup [ci-skip]
RDoc does not support backticks the way that Markdown does.  Instead,
inline code must be wrapped with `+` or `<tt>`.
2022-02-21 11:11:11 -06:00
Jonathan Hefner
0d3effc97e Replace "overwrite" with "override" [ci-skip]
"Overwrite" means "destructively replace", and is more suitable when,
for example, talking about writing data to a location.

"Override" means "supersede", and is more suitable when, for example,
talking about redifining methods in a subclass.
2022-02-21 11:11:11 -06:00
Jonathan Hefner
5fdbd217d1 Fix typos [ci-skip] 2022-02-21 11:11:11 -06:00
Jean Boussier
d32767884d Copy over the IsolatedExecutionState in AC::Live
Fix: https://github.com/rails/rails/issues/44496

It's really unfortunate, but since thread locals were copied
since a decade and we moved most of them into IsolatedExecutionState
we now need to copy it too to keep backward compatibility.

However I think it's one more sign that AC::Live should be
rethought.
2022-02-21 11:40:52 +01:00
Petrik
a134bd72aa Add documentation for controller CSP methods [skip-ci]
Co-authored-by: Jonathan Hefner <jonathan@hefner.pro>
2022-02-17 19:28:11 +01:00
David Heinemeier Hansson
41478f7074 Make #to_fs the default replacement for #to_s(:format)
#to_formatted_s is too cumbersome.
2022-02-07 12:41:21 +01:00
Jean Boussier
4f12bcd7f1 Remove the deprecated urlsafe_csrf_tokens configuration
Ref: https://github.com/rails/rails/pull/43817

Normally we remove deprecated code much later, but in this case
it's in the way of https://github.com/rails/rails/pull/44283
so I think it would make sense to remove it now.
2022-02-01 10:35:43 +01:00
Alberto Almagro
f906af86b9 Fix nested bullet list indentation [ci skip]
This patch fixes the indentation of a nested bullet list in the
documentation of `ActionController::Parameters` that appeared as inside
a box in api.rubyonrails.org documentation website.
2022-01-18 23:25:22 +01:00
Yutaka Kamei
0b7f37fbed
Initialize ActionController::Parameters with @logging_context
`params` contains `@logging_context` in its instance to notify
unpermitted parameters including the context through Rails
Instrumentation API. However, the logging context disappeared when
`params` is updated with some methods, such as `require`, `slice`,
`merge`, etc, so the subscriber of `unpermitted_parameters` could not
get the information.

This patch tries to initialize `Parameters` with `@logging_context`
where it makes sense to pass the information. The following methods will
be affected with this patch:

* `require`
* `deep_dup`
* `slice`
* `except`
* `extract!`
* `transform_values`
* `transform_keys`
* `deep_transform_keys`
* `select`
* `reject`
* `compact`
* `merge`
* `reverse_merge`
2021-12-16 23:11:49 +09:00
Aaron Patterson
8159996ea0
Dup arrays that get "converted"
We don't want to expose these cache keys to users because users can
mutate the key causing the cache to behave inconsistently.

Fixes: #43681
2021-12-15 14:30:27 -08:00
Rafael Mendonça França
eb80dd39dc
Fix ruby warnings 2021-12-15 02:48:55 +00:00
Rafael Mendonça França
ab754e95d8
Merge pull request #43817 from etiennebarrie/deprecate-non-url-safe-csrf-tokens
Deprecate non-URL-safe CSRF tokens
2021-12-15 01:48:51 +00:00
Alex Ghiculescu
054fa96bac ActionController::TestCase: reset instance variables after each request
`ActionController::TestCase` keeps a `@controller` instance variable, which represents the controller being tested. At the end of each request inside a test, its [params and format](https://github.com/rails/rails/blob/main/actionpack/lib/action_controller/metal/testing.rb) are reset. But any other instance variables set in the test aren't reset. This creates a problem if you do something like this:

```ruby
class UserController
  def show
    @user ||= User.find(params[:id])
    render plain: @user.name
  end
end
```

```ruby

test "gets the user" do
  get :show, params: { id: users(:one).id }
  assert "one", response.body

  get :show, params: { id: users(:two).id }
  assert "two", response.body
end
```

The second assertion will fail, because `@user` won't be re-assigned in the second test (due to `||=`). This example is a bit contrived, but it shows how instance variables persisting between requests can lead to surprising outcomes.

This PR fixes this by clearing all instance variables that were created on the controller while a request was processed.

It explicitly excludes instance variables that were created *before* any requests were run. And it leaves the instance variable around until the *next* request in the test. This means that you can still do this:

```ruby

test "gets the user" do
  @controller.user = users(:one) # assuming `Controller#user` users an ivar internally, you can set the ivar here...

  get :show_current
  assert "one", response.body

  assert_equal users(:one), @controller.user # and you can read the ivar here
end
```
2021-12-09 16:22:54 -06:00
Rafael Mendonça França
58ecdd0cf2
Deprecate to_s(format) in favor of to_formatted_s(format)
Ruby 3.1 is going to introduce an [optimization][] that makes interpolation
of some types of objects faster, unless there is a custom implementation
of to_s. Since Rails is overriding `to_s` for a bunch of core classes it
means that this optimization in Rails applications will be disabled.

In order to allow Rails applications to use this optimization in
the future we are deprecating all the reasons we override `to_s` in
those core classes in favor of using `to_formatted_s`.

[optimization]: b08dacfea3
2021-12-06 19:22:05 +00:00
Alex Ghiculescu
39cde506c6 Fix crash in ActionController::TestCase in rspec
Fixes https://github.com/rails/rails-controller-testing/issues/70
2021-11-30 14:01:33 -06:00
Alex Ghiculescu
5046d1cce9 Wrap ActionController::TestCase with Rails executor
Update actionpack/lib/action_controller/test_case.rb

Co-authored-by: Jean Boussier <jean.boussier@gmail.com>
2021-11-26 17:54:47 -06:00
Rafael Mendonça França
9606aa70b1
Merge pull request #43703 from p8/actionpack/fix-params-inspect-in-docs
Update inspect output of ActionController::Parameters in docs [skip-ci]
2021-11-23 15:44:54 -05:00
Petrik
9ee8b08aec Update inspect output of ActionController::Parameters in docs [skip-ci]
In 74cb9a6f3823cc64ef97ed6d9250a4d743abf426 a `#` was added to the
inspect of ActionController::Parameters.
This change adds `#` to the inspect output of
ActionController::Parameters in the documentation.
2021-11-23 21:36:28 +01:00
Ari Karim
da870578a4 Fix Typo in actio_controller::params_wrapper 2021-11-22 14:52:12 +03:00
Jean Boussier
e974c58bf9 Set the execution context from AC::Metal rather than AbstractController
The later is used by totally different codepaths such as mailers.

Fix: https://github.com/rails/rails/pull/43598
2021-11-15 15:36:28 +01:00
Jean Boussier
6bad959565 Extract ActiveSupport::ExecutionContext out of ActiveRecord::QueryLogs
I'm working on a standardized error reporting interface
(https://github.com/rails/rails/issues/43472) and it has the same need
for a `context` than Active Record's query logs.

Just like query logs, when reporting an error you want to know what the
current controller or job is, etc.

So by extracting it we can allow both API to use the same store.
2021-11-10 09:36:02 +01:00
Kasper Timm Hansen
66ffb4684d [ci skip] nope the called method is on the routes module 2021-11-06 04:10:49 +01:00
Kasper Timm Hansen
32e2ac5fd4 [ci skip] Ah, we also need to annotate the link as a ref 2021-11-06 04:04:28 +01:00
Kasper Timm Hansen
0ec672dff4 [ci skip] rdoc please I have a life 2021-11-06 03:59:01 +01:00
Kasper Timm Hansen
2fa7c2fc26 [ci skip] Link the view side url_for instead, try to shorten the shown text 2021-11-06 03:52:48 +01:00
Kasper Timm Hansen
f2ca082d7b [ci skip] Fix some more documentation links 2021-11-06 03:43:46 +01:00
Kasper Timm Hansen
754c0f8a15 [ci skip] Update documenation formatting to link to methods/constants and fix code fences 2021-11-06 03:32:21 +01:00
Kasper Timm Hansen
1000465d85 Simplify allow_other_host extraction so it's easier to read 2021-11-06 03:27:20 +01:00
Kasper Timm Hansen
c3758a71af Raise ActionController::Redirecting::UnsafeRedirectError for unsafe redirect_to redirects.
This allows `rescue_from` to be used to add a default fallback route:

```ruby
rescue_from ActionController::Redirecting::UnsafeRedirectError do
  redirect_to root_url
end
```

Co-Authored-By: Chris Oliver <excid3@gmail.com>
2021-11-05 03:23:12 +01:00
Kasper Timm Hansen
b36c9ca378 [ci skip] Explain redirect_to's open redirect protection, how to enable etc. 2021-11-05 03:12:37 +01:00
Kasper Timm Hansen
8bbf7d2c9a Replace complex conditional logic with an explaining comment
The logic was there to make a regression test pass https://github.com/rails/rails/blob/main/actionpack/test/controller/redirect_test.rb#L80
Originally added in https://github.com/rails/rails/pull/35054/files#diff-37c836f1b2c45e0f25de5adebb8ddaf02967632efaa5f9407b88fc163e297244R80

Since the regression was added at a time that `redirect_to` didn't have a
method level `allow_other_host` nor the class-level `raise_on_open_redirects`
we should be able to omit passing along the `allow_other_host` option.

Instead you're meant to enable `raise_on_open_redirects` in the new framework defaults.
2021-11-05 02:40:03 +01:00
danmcge
a2b3e3d523 Add url_from to verify a URL is internal and safe to redirect to
Closes https://github.com/rails/rails/pull/43327

Co-Authored-By: Kasper Timm Hansen <kaspth@gmail.com>
2021-11-05 01:56:37 +01:00
Kasper Timm Hansen
25418676cd Split out squished, referer or fallback, logic into distinct branches 2021-11-04 04:54:38 +01:00