- ### Context
Since version 2.0.0, Omniauth no longer recognizes `GET` request
on the auth path (`/users/auth/<provider>`). `POST` is the only
verb that is by default recognized in order to mitigate CSRF
attack. 66110da85e/lib/omniauth/strategy.rb (L205)
Ultimatelly, when a user try to access `GET /users/auth/facebook`,
Devise [passthru action](6d32d2447c/app/controllers/devise/omniauth_callbacks_controller.rb (L6))
will be called which just return a raw 404 page.
### Problem
There is no problem per se and everything work. However the
advantage of not matching GET request at the router layer allows
to get that same 404 page stylized for "free" (Rails ending up
rendering the 404 page of the app).
I believe it's also more consistent and less surprising for users
if this passthru action don't get called.
### Drawback
An application can no longer override the `passthru` to perform
the logic it wants (i.e. redirect the user).
If this is a dealbreaker, feel free to close this PR :).
Rails allow procs and lambda with either zero or more argument. Devise
however always tried to call instance_eval on those values, which does
always pass one argument: self.
There was a PR to fix this specific problem in Devise https://github.com/heartcombo/devise/pull/4627,
before the arity check was fixed in rails itself: https://github.com/rails/rails/pull/30391.
But even if the problem was fixed in Rails, Devise was still calling
the proc/lambas with instance_eval. That meant the fix added to Rails
did not apply to Devise.
The fix is to let Rails handle the :from and :reply_to defaults. We do
that by unsetting the headers instead of trying to replicate Rails handling
in Devise. This lets Rails handle it when setting up the mailer.
DEPRECATION WARNING: SchemaMigration no longer inherits from
ActiveRecord::Base. If you want to use the default connection, remove
this argument. If you want to use a specific connection, instantiate
MigrationContext with the connection's schema migration, for example
`MigrationContext.new(path, Dog.connection.schema_migration)`.
Even though this is considered an internal / non-public / nodoc method,
it seems some libraries relied on it internally, causing some breakage.
Known libraries so far are `devise-security` and
`devise-pwned_password`.
Closes#5580
We have an number of conditions due to how dirty tracking changed around
Rails 5.1, that implement methods using one or another method call. I
might need more of this for mongo upgrades based on an initial
investigation, plus this makes the code really hard to reason about
sometimes with these many conditionals.
While I want to drop support for older versions of Rails soon, this
centralization of dirty methods that are used by devise conditionally
simplifies the usage considerably across the board, moves the version
condition to a single place, and will make it easier to refactor later
once we drop older Rails version by simply removing the `devise_*`
versions of the methods, alongside the prefix on the method calls for
the most part, since those methods follow the naming of the newer Rails
versions.
It appears some people use the recall functionality with a redirect
response, and Devise starting on version 4.9 was overriding that status
code to the configured `error_status` for better Turbo support, which
broke the redirect functionality / expectation.
While I don't think it's really great usage of the recall functionality,
or at least it was unexpected usage, it's been working like that
basically forever where recalling would use the status code of the
recalled action, so this at least keeps it more consistent with that
behavior by respecting redirects and keeping that response as a redirect
based on the configured status, which should also work with Turbo I
believe, and makes this less of a breaking change.
Closes#5570Closes#5561 (it was closed previously, but related / closes with an
actual change now.)
While introducing this on turbo, looks like no specific test was added,
so this at least covers that a bit.
It needs some conditional checks since not all supported Rails +
Responders version work with the customization, so there's one test for
the hardcoded status version too, which can be removed in the future.
Rails main / 7.1.0.alpha introduced a change to improve typography by
default, by converting all apostrophes to be single quotation marks.
https://github.com/rails/rails/pull/45463
The change caused all our text based matching to fail, this updates the
tests to ensure compatibility.
Model tests were changed to test against the error type & information
rather than the translated string, which I think is an improvement
overall that should make them a little less brittle. I thought of using
[of_kind?] but that isn't available on all Rails versions we currently
support, while `added?` is. The drawback is that `added?` require full
details like the `:confirmation` example which requires the related
attribute that is being confirmed, but that's a small price to pay.
Integration tests were changed to match on a regexp that accepts both
quotes. I could've used a simple `.` to match anything there, but
thought I'd just keep it specific for clarity on what it is really
expected to match there. Plus, since it's integration testing against a
rendered response body, it's better to match the actual text rather than
resort on other ways. (like using I18n directly, etc.)
[of_kind?] https://api.rubyonrails.org/classes/ActiveModel/Errors.html#method-i-of_kind-3F
We can't just "swap" those model properties, as that sets instance vars
on the classes that get reverted to their "previous" value, which ends
up leaving the instance vars set as `nil`. However, our logic for those
model/class properties actually checks for `defined?` as a way to
override them, and delegates up to `Devise` global config if they are
not defined, so leaving instance vars back with `nil` values isn't
enough, we need to actually remove them.
This introduces a new test helper specifically for overriding those
model configs so that we can do proper cleanup.
This is an attempt to address the Webrat / Nokogiri compatibility issue
[discussed here]. It monkeypatches Webrat to explicitly add the old
default arguments to the invocation of to_xpath.
Move monkey patch to its own file under test/support/webrat.
I really need to get rid of webrat.
Closes#5475
[discussed here] https://github.com/sparklemotion/nokogiri/issues/2469
Expand tests to check for the actual validatable exception message
This was raising a `FrozenError` on Ruby < 3 where interpolated strings
were considered frozen. This [changed in Ruby 3], since such strings are
dynamic there's no point in freezing them by default.
The test wasn't catching this because `FrozenError` actually inherits
from `RuntimeError`:
>> FrozenError.ancestors
=> [FrozenError, RuntimeError, StandardError, Exception, Object ...]
So the exception check passed. Now we're also checking for the error
message to ensure it raised the exception we really expected there.
Closes#5465
[changed in Ruby 3] https://bugs.ruby-lang.org/issues/17104
Co-authored-by: Martin <martin@edv-beratung-meier.de>
This changes the OmniAuth "sign in" links to use buttons, which can be
wrapped in an actual HTML form with a method POST, making them work
better with and without Turbo in the app. It doesn't require rails/ujs
anymore in case of a non-Turbo app, as it previously did with links +
method=POST.
Turbo is disabled for those OmniAuth buttons, as they simply don't work
trying to follow the redirect to the OmniAuth provider via fetch,
causing CORS issues/errors.
Rails implemented a CSRF token storage strategy to allow storing the
CSRF tokens outside of the sessios (for example, in an encrypted
cookie), and changed how the value is kept around during the request
cycle, by using a request.env value.
We still want to ensure the final session value is cleaned correctly in
the test, but the implementation needed to change since we can't simply
delete from the session anymore, we need to make sure we call the Rails
methods for resetting the current storage strategy so it works with all
of them.
https://github.com/rails/rails/pull/44283
It appears setting the `rack.session` to a simple hash doesn't work
anymore as it now has a few additional methods Rails is relying on to
determine whether it's enabled or not:
https://github.com/rails/rails/pull/42231
Failure:
NoMethodError: undefined method `enabled?' for {}:Hash
rails (f55cdafe4b82) actionpack/lib/action_dispatch/middleware/flash.rb:62:in `commit_flash'
Turns we we don't seem to need to set `rack.session` for the tests here.
Remove `ActiveSupport::Dependencies.reference`
This was deleted from Rails: 14d4edd7c3
As far as I can tell, it was meant to add a performance boost at some point in the past but doesn't seem to do anything useful these days.
Resetting failed attempts after sign in happened inside a warden hook
specific for the lockable module, but that was hidden inside the hook
implementation and didn't allow any user customization.
One such customization needed for example is to direct these updates to
a write DB when using a multi-DB setup. With the logic hidden in the
warden hook this wasn't possible, now that it's exposed in a model
method much like trackable, we can override the model method to wrap it
in a connection switch block for example, point to a write DB, and
simply call `super`.
Closes#5310
Related to #5264 and #5133
This allows us to remove the dependency on the XML serializer provided
by the external `activemodel-serializers-xml` gem, and eliminates the
following deprecation warning:
DEPRECATION WARNING: ActiveModel::Errors#to_xml is deprecated and
will be removed in Rails 6.2.
Please note: this does not mean Devise doesn't support XML, it simply
means our test suite will use JSON to test non-navigatable formats
instead of XML, for simplicity. Devise's job is not to test object
serialization, so as long as your objects properly serialize to
XML/JSON/any other format, it should work out of the box.
* Make test for validation to be Rails 6.1 compatible
The `ActiveModel::Errors` has been changed in Rails 6.1.
https://github.com/rails/rails/pull/32313
* Add gemfile for Rails 6.1
* Add CI matrix for Rails 6.1
Deprecate `BLACKLIST_FOR_SERIALIZATION` constant in favor of a more
descriptive name `UNSAFE_ATTRIBUTES_FOR_SERIALIZATION`, removing
unnecessary usage of the word `blacklist` from devise.
The previous constant still works but will emit a warning if used, to
allow anyone still depending on it to upgrade.
This includes an internal backport of the Rails `deprecate_constant`
implementation that exists on Rails 5.1+ to be able to deprecate it
properly in prior versions, while we support those. (which I intend to
drop soon.)
I ran into an issue where options[:except] is a frozen array, which
explodes when we try to concat values in `serializable_hash`. To fix this
we dup the `:except` option before concatenating with the other options
there.
Closes#5278.
This is warning on Ruby 2.7, and seems unnecessary since Rails 5+
adopted kwargs approach. We still need to handle the difference for
Rails < 5 for now, while we support it (which I would like to remove
soon.)
Mocha deprecation warning at
...../active_support/dependencies.rb:324:in `require':
Require 'mocha/test_unit', 'mocha/minitest' or 'mocha/api'
instead of 'mocha/setup'.
