"Invalid Email or password." is grammatically incorrect, a change
introduced a while ago by #4014.
Signed-off-by: Carlos Antonio da Silva <carlosantoniodasilva@gmail.com>
It's an unauthenticated request, so return 401 Unauthorized like most
other similar requests.
Signed-off-by: Carlos Antonio da Silva <carlosantoniodasilva@gmail.com>
Used Rails' secure_compare method inside the definition of
secure_compare. This will handle the empty strings comparison and
return true when both the parameters are empty strings.
Fixes#4441, #4829
In Rack v3.1.0, the symbol for HTTP status code 422 was changed from `:unprocessable_entity` to `:unprocessable_content`.
As a result, when using rack 3.2 with the following configuration in `config/initializers/devise.rb`, a warning is shown on login failure:
```ruby
# config/initializers/devise.rb
Devise.setup do |config|
...
config.responder.error_status = :unprocessable_entity
```
Warning message:
```sh
/path-to-app/vendor/bundle/ruby/3.4.0/gems/devise-4.9.4/lib/devise/failure_app.rb:80: warning: Status code :unprocessable_entity is deprecated and will be removed in a future version of Rack. Please use :unprocessable_content instead.
```
This warning can be resolved by updating the config as follows:
```diff
# config/initializers/devise.rb
Devise.setup do |config|
...
+ config.responder.error_status = :unprocessable_content
- config.responder.error_status = :unprocessable_entity
```
This fixes the root cause of the warning for new apps by adjusting the generated config during `$ rails generate devise:install` depending on the rack version, so new apps using newer Rack versions generate `error_status = :unprocessable_content` instead of `:unprocessable_entity`.
Existing apps are handled by [latest versions of Rails, which will now transparently convert the code under the hood to avoid the Rack warning](https://github.com/rails/rails/pull/53383), and Devise will use that translation layer when available in the failure app to prevent the warning there as well (since that isn't covered by Rails automatic conversion).
Signed-off-by: Carlos Antonio da Silva <carlosantoniodasilva@gmail.com>
We need to explicitly pass the `locale` around from the options (passed
to `warden.authenticate!` for instance) or the `I18n.locale` when
logging out and redirecting the user via `throw :warden`, otherwise in a
multi-locale app we'd lose the locale previously set / passed around and
fallback to the default for that flash message.
This is a follow-up of the fixes in #5567 where we implemented the
locale passing logic down to the failure app, but it missed these places
where we were using `throw :warden`.
Closes#5812
This is no longer in Rails since v5, it's been extracted to
`protected_attributes`, and we're dropping support to older versions of
Rails in main.
https://github.com/rails/protected_attributes
All of these have been deprecated for years, if we're releasing a new
major version, let's take the opportunity to do some cleanup.
* Remove deprecated `:bypass` option from `sign_in` helper,
use `bypass_sign_in` instead.
* Remove deprecated `devise_error_messages!` helper,
use `render "devise/shared/error_messages", resource: resource` instead.
* Remove deprecated `scope` second argument from `sign_in(resource, :admin)`
controller test helper, use `sign_in(resource, scope: :admin)` instead.
* Remove deprecated `Devise::TestHelpers`,
use `Devise::Test::ControllerHelpers` instead.
Closes#5739
Devise hasn't been tested with Mongoid since Rails version 5, only 4.x was still running those tests.
This enables the tests again on all currently supported Rails versions, with their respective mongoid supported versions. There were a couple of minor tweaks to make it happen, namely:
* The way we were dropping the session before doesn't work in later versions so I changed back to calling `purge!` which appears to work fine. We used to call `Mongoid.purge!` but that changed in #4686.
* Some of the configs in the Rails test app were setting Active Record values when outside of the AR ORM tests, updated those to make sure they are not set when running mongoid ORM tests.
* The validations added to the shared admin code in tests were only checking for Rails version 5.1, but we need to use the same check for AR 5.1 that is used in code, otherwise it will try to use methods not available in mongoid there.
./devise/test/support/webrat/matchers.rb:6: warning:
Nokogiri::CSS.parse is deprecated and will be removed in a future
version of Nokogiri. Use Nokogiri::CSS::Parser#parse instead.
A common usage of I18n with different locales is to create some around
callback in the application controller that sets the locale for the
entire action, via params/url/user/etc., which ensure the locale is
respected for the duration of that action, and resets at the end.
Devise was not respecting the locale when the authenticate failed and
triggered the failure app, because that happens in a warden middleware
right up in the change, by that time the controller around callback had
already reset the locale back to its default, and the failure app would
just translate flash messages using the default locale.
Now we are passing the current locale down to the failure app via warden
options, and wrapping it with an around callback, which makes the
failure app respect the set I18n locale by the controller at the time
the authentication failure is triggered, working as expected. (much more
like a normal controller would.)
I chose to introduce a callback in the failure app so we could wrap the
whole `respond` action processing rather than adding individual `locale`
options to the `I18n.t` calls, because that should ensure other possible
`I18n.t` calls from overridden failure apps would respect the set locale
as well, and makes it more like one would implement in a controller. I
don't recommend people using callbacks in their own failure apps though,
as this is not going to be documented as a "feature" of failures apps,
it's considered "internal" and could be refactored at any point.
It is possible to override the locale with the new `i18n_locale` method,
which simply defaults to the passed locale from the controller.
Closes#5247Closes#5246
Related to: #3052, #4823, and possible others already closed.
Related to warden: (may be closed there afterwards)
https://github.com/wardencommunity/warden/issues/180https://github.com/wardencommunity/warden/issues/170
We still support super old versions, yes, and it doesn't like `ensure`
without a `begin..end` unfortunately.
I plan to remove this support soon, but for now I don't want to stop
supporting it yet.
- ### 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.
