5.0.4 mixins#2
Closed
aak-code wants to merge 382 commits into
Closed
Conversation
The build is breaking with 2.x (which is expected), so this is a step to get it to green on GA.
Move build to GitHub Actions
This reverts commit 628f2fb. We should be run green on OmniAuth 2.x now.
I'm sure more people will hit issues so I'm trying to add more guidance here about how to upgrade... maybe that should be in its own wiki but I'll keep it all in the changelog for now.
Improve OmniAuth version check to allow anything from 1.0 forward
Fix deprecation warning on Rails 6.1 Related changes in Rails: rails/rails#38256 rails/rails#38536
And remove dupe entry in the exclude matrix.
In order to get Ruby 3 working we needed to install `rexml` as part of
the test dependencies, only done on the main Gemfile (Rails 6.1) and the
6.0 versions. (which are the only ones supported by Ruby 3.)
Devise itself doesn't require `rexml` as it does nothing with it, but a
dependency we use during tests seem to require it. I was able to track
it down to omniauth-openid -> rack-openid -> ruby-openid requiring it:
https://github.com/openid/ruby-openid/blob/13a88ad6442133a613d2b7d6601991a84b34630d/lib/openid/yadis/xrds.rb#L1
So while we have tests using omniauth-openid, we'll need this require in
place as well. Ideally that upstream version of ruby-openid should have
it, but it seems that one isn't updated in a while.
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.
The test suite was failing on Rails 6.0 + Ruby 3 with errors like:
Expected "{\"errors\":\"#<ActiveModel::Errors:0x000055f2e6cb8188>\"}"
to include "{\"errors\":{".
The ActiveModel::Errors object wasn't being serialized to JSON as
expected, and this only happened with that combination of Ruby/Rails.
Upon further investigation, this was caused by a change in Ruby and
fixed in Rails in this PR: rails/rails#39697
(which describes in more details the exact same problem and links to the
Ruby bug tracker with more information).
That fix was backported to 6-0-stable in June 2020, but hasn't been
officially released in a stable version yet: (there have been only
security fixes since then for 6.0)
rails/rails@75f6539
Since the branch contains the fix, I'm pointing directly to it to get
the tests passing. We can't tell if there'll be a new stable 6.0 release
at this point, but hopefully yes, in which case we can go back at
pointing to it.
2.2.10 is causing the dependency resolution on Rails 6-0-stable to fail:
```
Bundler could not find compatible versions for gem "railties":
In Gemfile-rails-6-0:
devise was resolved to 4.7.3, which depends on
railties (>= 4.1.0)
rails was resolved to 6.0.3.5, which depends on
railties (= 6.0.3.5)
responders (~> 3.0) was resolved to 3.0.1, which depends on
railties (>= 5.0)
Took 27.49 seconds
```
https://github.com/heartcombo/devise/runs/1905780158?check_suite_focus=true#step:5:23
The `railties` version 6.0.3.5 should work, given the other two are
using >= declarations, but it fails in 2.2.10.
Downgrading to 2.2.9 works.
Support Ruby 3+ officially, remove final Rails 6.1 deprecations, drop test dependency for XML serialization
This reverts commit 1ba53dc. Let's give the latest bundler (2.2.15 as of today) a try again.
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 heartcombo#5310 Related to heartcombo#5264 and heartcombo#5133
…attempts Create a model hook around the lockable warden hook to reset attempts
Add metadata for RubyGems
- It says that the option is added to devise_for, but it is actually added to the devise method in the model.
…odules Fix comment in some modules [ci skip]
Update a couple other modules that still referred to `devise_for` to point to `devise`, and make all of them more consistent. We can only mention `devise`, that should be clear enough about it being options for the model method.
…ence` (heartcombo#5357) Remove `ActiveSupport::Dependencies.reference` This was deleted from Rails: rails/rails@14d4edd 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.
…5397) Changes deprecated `ActiveSupport::Dependencies.constantize(model_name)` to `model_name.constantize` Co-authored-by: Carlos Antonio da Silva <carlosantoniodasilva@gmail.com>
"Mini Test" was used in heartcombo#5012 but "minitest" is the correct product name. See also: https://github.com/minitest/minitest/blob/master/README.rdoc#description > minitest provides a complete suite of testing facilities > supporting TDD, BDD, and benchmarking. In this description, "minitest" is used. [ci skip] Signed-off-by: Carlos Antonio da Silva <carlosantoniodasilva@gmail.com>
…eartcombo#5822) A previous change heartcombo#4834 introduced a downcase call to each attribute, so that it'd fix an invalid grammar issue on some languages like English that were showing `Email` in the middle of flash message sentences. However, it caused a bug with German which uses the word `E-Mail` and at the beginning of the sentence, causing it to be converted to `E-mail` incorrectly. The fix here will only downcase the first char of each word, and convert it back to upcase at the beginning of the sentence, which should work for both the original fix (English message), and for the new bug (German message) If we end up running into any more of these edge cases with the message, we might roll it all back and provide a different set of interpolation values for the original vs downcased translations, so people can use what makes the most sense for each language without us having to manually massage these strings. Fixes heartcombo#5820
…et" button (heartcombo#5824) This mirrors heartcombo#5515
…combo#5825) The config exists at the model/resource class from the registerable module, but it was not being honored, instead we were directly relying on the main Devise config. Now this can be configured and honored per-model/resource class, as expected. This is similar to heartcombo#5429 and `sign_in_after_reset_password` fix.
Build pushes to the main branch and open PRs, but not pushes to other branches. Allow workflow_dispatch to build them manually if we want to.
heartcombo#5826) Extract a couple small duplicate checks into a method, enabling it as a hook that can be overridden if necessary. It's going to be particularly useful on a flow I'm working on / testing out, to avoid having to copy over the whole block of code from the controller to customize it. We have a similar hook on the registration controller for `sign_in_after_change_password?`, which was also moved to protected. While not much practical change, it hopefully shows better the intention that it's a method users can override if they need, similar to a few other methods in controllers. Also move `update_needs_confirmation?` down to private, as this one in particular I don't think we intended to allow overriding, as it has no practical behavior change other than the flash message.
Check the related issue for more info and explanation: heartcombo#5828 (comment) [ci skip]
… is always saved (heartcombo#5784) Fix security issue in the `Confirmable` "change email" flow, where a user can end up confirming an email address that they have no access to. The flow for this is: 1. Attacker registers `attacker1@email.com` 2. Attacker changes their email to `attacker2@email.com`, but does not yet confirm this 3. Attacker submits two concurrent "change email" requests a. one changing to `attacker2@email.com` b. one changing to `victim@email.com` When request 3.a is run, the `Confirmable.postpone_email_change_until_confirmation_and_regenerate_confirmation_token` method sets both the `unconfirmed_email` and `confirmation_token` properties. But as the `unconfirmed_email` value is the same as the model already had from step 2, this attribute is not included in the SQL `UPDATE` statement. The SQL `UPDATE` statement only updates the `confirmation_token`. This token is emailed to the `attacker2@email.com` address. If the "victim" race request (3.b) completes first, it will update both the `unconfirmed_email` and the `confirmation_token`. But then request 3.a will replace just the token. The model's end state is having the confirmation token that was sent to the attacker, but with the `unconfirmed_email` of the victim. When the attacker follows the confirmation link, they will have confirmed the victim's email address, on an account that the attacker controls. Co-authored-by: Carlos Antonio da Silva <carlosantoniodasilva@gmail.com>
When the Timeoutable module is enabled in Devise, the `FailureApp#redirect_url` method returns request.referrer, which is attacker-controllable, without validation for any non-GET request that results in a session timeout. An attacker who hosts a page with an auto-submitting cross-origin form can cause a victim with an expired Devise session to be redirected to an arbitrary external URL. This contrasts with the GET timeout path (which uses server-side `attempted_path`) and Devise's own `store_location_for` mechanism (which strips external hosts via `extract_path_from_location`), both of which are protected; only the non-GET timeout redirect path is unprotected. The fix is to apply the same handling logic from `store_location_for` an extract only the path from the request referer, stripping out the host entirely, to prevent the external redirect. GHSA-jp94-3292-c3xv
Allows Devise to mount on multiple Rails engines within the same
application by generating scope-specific controller sets
(`<Scope>::Devise::*Controller`) that inherit from each engine's
own ApplicationController. This lets engine-specific helpers,
callbacks, and concerns be available inside Devise actions.
Usage:
# config/initializers/devise.rb
config.controller_scopes = [:table_solution, :table_check, :tc_server]
For each scope listed, a controller set is generated at boot/reload:
<Scope>::Devise::BaseController < <Scope>::ApplicationController
<Scope>::Devise::SessionsController < <Scope>::Devise::BaseController
<Scope>::Devise::PasswordsController < <Scope>::Devise::BaseController
<Scope>::Devise::RegistrationsController < <Scope>::Devise::BaseController
<Scope>::Devise::ConfirmationsController < <Scope>::Devise::BaseController
<Scope>::Devise::UnlocksController < <Scope>::Devise::BaseController
<Scope>::Devise::OmniauthCallbacksController < <Scope>::Devise::BaseController
Action code is provided by `Devise::Mixins::*` modules included
into both the generated subclasses and the original upstream
controllers. The upstream `Devise::*Controller` classes are kept
intact (now thin shims that include the mixin) so default callers
that don't set `controller_scopes` continue to work unchanged and
upstream releases merge with minimal conflict surface.
Changes:
- New: lib/devise/controllers/generator.rb
- New: lib/devise/mixins/{base,session,password,registration,
confirmation,unlock,omniauth_callback}.rb
- lib/devise.rb: autoload Mixins, add `controller_scopes` mattr
- lib/devise/controllers/helpers.rb: devise_controller? also
recognises classes that include Devise::Mixins::Base
- lib/devise/mailers/helpers.rb: prepend router_name into
template path so engine-mounted scopes find their views
- lib/devise/mapping.rb: default router_name to
Devise.available_router_name
- lib/devise/rails.rb: to_prepare hook drives the generator
- app/controllers/devise/*_controller.rb: thinned to
`include Devise::Mixins::*` (no behavior change for the
default :devise scope)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
20 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.