Ensure passwords are not subject to another order when storing

[BookOfGreg]

Replaced #286

For anyone using UUIDs as default in system, OR for anyone using a default sort order, use reorder rather than order to avoid chained orders from taking effect.

It's unclear if Mongodb uses reorder vs order but I assume it to be ActiveRecord compatible since none of the other tests exploded.

[dillonwelch]

https://github.com/devise-security/devise-security/pull/289/checks?check_run_id=2135460923 looks like your test fails on this version combo.

Fix that up and this is good to go.

[BookOfGreg]

Is that a genuine test failure? Looks like only MongoID + Rails 4.2 + ruby 2.5, given it works on ruby 2.6 I'm inclined to believe it's a random failure not the new tests.

[dillonwelch]

So it does look genuine, as the test failure is on your new test.

  1) Error:
TestPasswordArchivable#test_default_sort_orders_do_not_affect_archiving:
ArgumentError: wrong number of arguments (given 0, expected 1)
    /home/runner/work/devise-security/devise-security/gemfiles/vendor/bundle/gems/mongoid-4.0.2/lib/mongoid/scopable.rb:90:in `default_scope'
    /home/runner/work/devise-security/devise-security/test/test_password_archivable.rb:79:in `<class:OldPassword>'
    /home/runner/work/devise-security/devise-security/test/test_password_archivable.rb:78:in `block in <class:TestPasswordArchivable>'

The line of code that's failing:

default_scope { order(created_at: :asc) }

And here is the relevant method in mongoid 4.0.2: https://github.com/mongodb/mongoid/blob/v4.0.2/lib/mongoid/scopable.rb#L66

Based on the doc comment, my guess is that they want one of the following:

default_scope where(active: true)
default_scope ->{ where(active: true) }

It looks like this behavior was changed in this commit , which is included in versions 5+, which explains why this only failed on this one version.

[BookOfGreg]

Oh wow I'm sorry to have caused you to go out of your way for that. You're totally right.
I'll get that fixed up for you next time I'm on that laptop.

[dillonwelch]

Not a problem. Always happy to assist anyone who submits issues or PRs :)

[coveralls]

Coverage increased (+0.1%) to 97.615% when pulling c342a50 on BookOfGreg:fix/ordering into d27a172 on devise-security:master.

[dillonwelch]

Now that we no longer support Rails 4.2, that failing test is not an issue and everything is 🟢 !