[Security] Bump devise from 2.2.8 to 3.5.10

[dependabot-preview]

I extended this dependabot PR with quite a few commits to adapt OFN to devise 3.5.10. Ready for Review!

What to test:

We need to manually test all the login/logout/signup/forgotten password workflows in the app.

Dependabot original PR

Bumps devise from 2.2.8 to 3.5.10. This update includes a security fix.

Vulnerabilities fixed

Sourced from The Ruby Advisory Database.

Devise Gem for Ruby Unauthorized Access Using Remember Me Cookie
Devise version before 3.5.4 uses cookies to implement a "Remember me"
functionality. However, it generates the same cookie for all devices. If an
attacker manages to steal a remember me cookie and the user does not change
the password frequently, the cookie can be used to gain access to the
application indefinitely.

Patched versions: >= 3.5.4
Unaffected versions: none

Release notes

Sourced from devise's releases.

v3.5.3

• bug fixes
  • Fix password reset for records where confirmation_required? is disabled and
    confirmation_sent_at is nil. (by @​andygeers)
  • Allow resources with no email field to be recoverable (and do not clear the
    reset password token if the model was already persisted). (by @​seddy, @​stanhu)
• enhancements
  • Upon setting Devise.send_password_change_notification = true a user will receive notification when their password has been changed.

v3.5.2

• enhancements
  • Perform case insensitive basic authorization matching
• bug fixes
  • Do not use digests for password confirmation token
  • Fix infinite redirect in Rails 4.2 authenticated routes
  • Autoload Devise::Encryptor to avoid errors on thread-safe mode
• deprecations
  • config.expire_auth_token_on_timeout was removed

v3.4.1

• enhancements
  • Devise default views now have a similar markup to Rails scaffold views. (by @​udaysinghcode, @​cllns)
  • Passing now: true to the set_flash_message helper now sets the message into
    the flash.now Hash. (by @​hbriggs)
• bugfixes
  • Fixed an regression with translation of flash messages for when the authentication_keys
    config is a Hash. (by @​lucasmazza)

v3.4.0

• enhancements
  • Support added for Rails 4.2. Devise now depends on the responders gem due
    the extraction of the respond_with API from Rails. (by @​lucasmazza)
  • The Simple Form templates follow the same change from 3.3.0 by using Log in and adding
    a hint about the minimum password length when validatable is enabled. (by @​aried3r)
  • Controller generator added as devise:controllers SCOPE. You can use the -c flag
    to pick which controllers (unlocks, confirmations, etc) you want to generate. (by @​Chun-Yang)
  • Removed the hardcoded references for "email" in the flash messages. If you are using
    different attributes as the authentication_keys they will be interpolated in the
    messages instead. (by @​timoschilling)
• bug fix
  • Fixed a regression where the devise generator would fail with a ConnectionNotEstablished
    exception when executed inside a mountable engine. (by @​lucasmazza)
  • Ensure to return symbols in find_scope! fixing a previous regression from 3.3.0 (by @​micat)
  • Ensure all causes of failed login have the same error message (by @​pjungwir)
  • The last_attempt_warning now takes effect when generating the unauthenticated
    message for your users. To keep the current behavior, this flag is now true
    by default. (by @​lucasmazza)

Changelog

Sourced from devise's changelog.

3.5.10 - 2016-05-15

• bug fixes
  • Fix overwriting the remember_token when a valid one already exists (by @​ralinchimev).

3.5.9 - 2016-05-02

• bug fixes
  • Fix strategy checking in Lockable#unlock_strategy_enabled? for :none
    and :undefined strategies. (by @​f3ndot)

3.5.8 - 2016-04-25

• bug fixes
  • Fix the e-mail confirmation instructions send when a user updates the email address from nil

3.5.7 - 2016-04-18

• bug fixes
  • Fix the extend_remember_period configuration. When set to false it does
    not update the cookie expiration anymore.(by @​ulissesalmeida)

3.5.6 - 2016-01-02

• bug fixes
  • Fix type coercion of the rememberable timestamp stored on cookies.

3.5.5 - 2016-22-01

• bug fixes
  • Bring back remember_expired? implementation
  • Ensure timeouts are not triggered if remember me is being used

3.5.4 - 2016-18-01

• bug fixes
  • Store creation timestamps on remember cookies

3.5.3 - 2015-12-10

• bug fixes

  • Fix password reset for records where confirmation_required? is disabled and
    confirmation_sent_at is nil. (by @​andygeers)
  • Allow resources with no email field to be recoverable (and do not clear the
    reset password token if the model was already persisted). (by @​seddy, @​stanhu)

• enhancements

  • Upon setting Devise.send_password_change_notification = true a user will receive notification when their password has been changed.

3.5.2 - 2015-08-10

... (truncated)

Commits

• 321fe1d Release 3.5.10
• a7dcf98 Fix overwriting the remember_token when a valid one already exists (#4101)
• 7e658a2 Release 3.5.9
• 0252f0e Extract list of both strategies into class constant
• 07e907e 🪲 Fix strategy checking in #unlock_strategy_enabled? for :none and und...
• e9ed3e2 Support for older rails versions.
• 2fa6735 Lock mime-types to ~> 2.99
• b8cddc3 Release 3.5.8
• 1d57169 Send confirmation instructions when a user updates the email address from nil
• 812c1de Release 3.5.7 version.
• Additional commits viewable in compare view

Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting @dependabot rebase.

---

Dependabot commands and options

You can trigger Dependabot actions by commenting on this PR:

• @dependabot rebase will rebase this PR
• @dependabot recreate will recreate this PR, overwriting any edits that have been made to it
• @dependabot merge will merge this PR after your CI passes on it
• @dependabot squash and merge will squash and merge this PR after your CI passes on it
• @dependabot cancel merge will cancel a previously requested merge and block automerging
• @dependabot reopen will reopen this PR if it is closed
• @dependabot ignore this [patch|minor|major] version will close this PR and stop Dependabot creating any more for this minor/major version (unless you reopen the PR or upgrade to it). To ignore the version in this PR you can just close it
• @dependabot ignore this dependency will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)
• @dependabot use these labels will set the current labels as the default for future PRs for this repo and language
• @dependabot use these reviewers will set the current reviewers as the default for future PRs for this repo and language
• @dependabot use these assignees will set the current assignees as the default for future PRs for this repo and language
• @dependabot use this milestone will set the current milestone as the default for future PRs for this repo and language
• @dependabot badge me will comment on this PR with code to add a "Dependabot enabled" badge to your readme

Additionally, you can set the following in your Dependabot dashboard:

• Update frequency (including time of day and day of week)
• Automerge options (never/patch/minor, and dev/runtime dependencies)
• Pull request limits (per update run and/or open at any time)
• Out-of-range updates (receive only lockfile updates, if desired)
• Security updates (receive only security updates, if desired)

Finally, you can contact us by mentioning @dependabot.

[luisramos0]

This PR has been moved to the new security private repo https://github.com/openfoodfoundation/ofn-security/issues/3
I think we should delete this issue permanently as it is a OFN security vulnerability publicly documented.
I dont have privileges to delete issues in this repo.

[dependabot-preview]

OK, I won't notify you again about this release, but will get in touch when a new version is available. If you'd rather skip all updates until the next major or minor version, let me know by commenting @dependabot ignore this major version or @dependabot ignore this minor version.

If you change your mind, just re-open this PR and I'll resolve any conflicts on it.

[mkllnk]

I don't see any way of deleting a pull request either. Apparently Github staff would delete it on request if it contained sensitive information. Not sure if this PR is sensitive enough for that.

I still think that someone who wants to exploit this will search Github for projects with this dependency. The chance that someone is inspired to use this after crawling through closed pull requests is very low.

But this is feedback we should give to Dependabot. Github displays security alerts only to collaborators or owners. Dependabot (now owned by Github) breaches that rule.

[luisramos0]

very interesting @mkllnk 👍

[luisramos0]

Can we just mention @dependabot to get this feedback across?
First we acknowledge dependabot's amazing job and help at making us progress with updating our dependencies, it has been amazing for us!!! 👏
And then we send some feedback: by opening this PR dependabot is basically making public a security vulnerability in OFN. A problem we are unable to solve right now. This is not a problem when the PR is successful and we manage to upgrade to a version that fixes the security vulnerability. Only when for some reason we cant go ahead with the upgrade.

[dependabot-preview]

👋 If you need additional help with Dependabot, please fill out GitHub's Support form and your request will be routed to the right team at GitHub.

Be sure to include the details of any troubleshooting steps you've tried so far.

[luisramos0]

Now that we are on rails 4, I am re-opening this PR to seee if we can now make this step.

btw, the high severity security vulnerability related to devise is only solved after upgrading to devise 4.7.1 ⚠️

[luisramos0]

@dependabot rebase

[dependabot-preview]

Looks like this PR is closed. If you re-open it, I'll rebase it, as long as no-one else has edited it.

[luisramos0]

@dependabot recreate

[dependabot-preview]

Looks like this PR is closed. If you re-open it I'll rebase it as long as no-one else has edited it (you can use @dependabot reopen if the branch has been deleted).

[luisramos0]

@dependabot reopen

[luisramos0]

@dependabot rebase

[luisramos0]

I added a few commits.
There are still 2 relevant specs failing:

• spec/models/spree/user_spec.rb:106
• spec/features/consumer/confirm_invitation_spec.rb:18

I think it's related to the fact that now the token that is sent by email is not the same as the token stored in the DB so we may need to do this trick in our specs as well:
spree/spree_auth_devise@fe7941f#diff-92ee7e66f2a9527fd813e41f0419d310R96

There's also this embedded shop spec broken:

• spec/features/consumer/shopping/embedded_shopfronts_spec.rb:91
  Logout is redirecting to /shops and not the specific shop page.

[luisramos0]

This was tough 🤕 but it's working now.
The reset_password_token stored in the DB is an encrypted version of the token sent in the email. The fix is simple but it took me a while to get to it.

I think there's only a problem with logout on embedded pages, I'll have a look tomorrow. Almost there 🤞

[sauloperez]

where did this go then? How is it now implemented?

[luisramos0]

We need to get our code review process faster, I mean, this was written 1 month ago, I am not the author anymore.
Let's see what this Luis from one month ago wanted to do.... I'll investigate now.

[luisramos0]

ok, got it. this is now done in vanilla devise:
https://rubydoc.info/github/plataformatec/devise/master/Devise/Models/Recoverable/ClassMethods#send_reset_password_instructions-instance_method

It comes into our User.rb through the recoverable decorator 👍 and we dont need to change it on our side anymore.

[sauloperez]

Looks pretty good to me

[mkllnk]

@luisramos0 I found one pre-existing issue. Usually I would still approve the PR to get the current improvements in but to save on testing, we should do the change here. Do you agree?

[filipefurtad0]

Hi @luisramos0 !

I manually test the workflows for signup, forgotten password, logging-in and -out:

• Signup workflow - all good:

  • inserted signup credentials
  • received email w/token
  • activated account
  • after redirection, logged in with used credentials
  • received welcome email

• Forgotten password workflow - all good:

  • clicked "Forgot password", inserted email-address, clicked "reset password"
  • received reset password email with token
  • after redirection, inserted matching passwords
  • redirection to account

• Additional checkups - all good:

  • logging in only works with the correct password
  • logging in grants access to /account page and previous shopping session
  • logout is effective, i.e., /account page is not accessible after logout
  • tokens are only valid once
  • (re)setting passwords must match

Ready to go.

[RachL]

@luisramos0 can we close this one?