-
Notifications
You must be signed in to change notification settings - Fork 345
Fix Ruby 2.4 deprecations #216
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,6 +6,8 @@ script: bundle exec rake | |
rvm: | ||
- 2.1.8 | ||
- 2.2.4 | ||
- 2.3.4 | ||
- 2.4.1 | ||
- ruby-head | ||
matrix: | ||
allow_failures: | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -16,7 +16,7 @@ def validate_password_archive | |
|
||
# validate is the password used in the past | ||
def password_archive_included? | ||
unless deny_old_passwords.is_a? Fixnum | ||
unless deny_old_passwords.is_a? 1.class | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why not use 1.class #=> Fixnum
1.is_a? Integer #=> true There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The maintainers of this repo seemed to have disappeared, so a few of us are maintaining a fork at https://github.com/devise-security/devise-security. Feel free to make a PR with these changes over there. |
||
if deny_old_passwords.is_a? TrueClass and archive_count > 0 | ||
self.deny_old_passwords = archive_count | ||
else | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -13,7 +13,7 @@ module PasswordExpirable | |
|
||
# is an password change required? | ||
def need_change_password? | ||
if self.expire_password_after.is_a? Fixnum or self.expire_password_after.is_a? Float | ||
if expired_password_after_numeric? | ||
self.password_changed_at.nil? or self.password_changed_at < self.expire_password_after.seconds.ago | ||
else | ||
false | ||
|
@@ -22,15 +22,15 @@ def need_change_password? | |
|
||
# set a fake datetime so a password change is needed and save the record | ||
def need_change_password! | ||
if self.expire_password_after.is_a? Fixnum or self.expire_password_after.is_a? Float | ||
if expired_password_after_numeric? | ||
need_change_password | ||
self.save(:validate => false) | ||
end | ||
end | ||
|
||
# set a fake datetime so a password change is needed | ||
def need_change_password | ||
if self.expire_password_after.is_a? Fixnum or self.expire_password_after.is_a? Float | ||
if expired_password_after_numeric? | ||
self.password_changed_at = self.expire_password_after.seconds.ago | ||
end | ||
|
||
|
@@ -39,7 +39,7 @@ def need_change_password | |
|
||
self.password_changed_at | ||
end | ||
|
||
def expire_password_after | ||
self.class.expire_password_after | ||
end | ||
|
@@ -51,6 +51,12 @@ def update_password_changed | |
self.password_changed_at = Time.now if (self.new_record? or self.encrypted_password_changed?) and not self.password_changed_at_changed? | ||
end | ||
|
||
def expired_password_after_numeric? | ||
return @_numeric if defined?(@_numeric) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is it actually a good idea to memoize this? Could there be a scenario where this gets checked and then the column is made to be expired_password_after_numeric? #=> true
self.expire_password_after = nil
expired_password_after_numeric? #=> true There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Unfortunately I don't know the code well enough to say. |
||
@_numeric ||= self.expire_password_after.is_a?(1.class) || | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good point |
||
self.expire_password_after.is_a?(Float) | ||
end | ||
|
||
module ClassMethods | ||
::Devise::Models.config(self, :expire_password_after) | ||
end | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -61,7 +61,7 @@ class TestParanoidVerification < ActiveSupport::TestCase | |
test 'when code not match upon verification code, should not set paranoid_verified_at' do | ||
user = User.new(paranoid_verification_code: 'abcde') | ||
user.verify_code('wrong') | ||
assert_equal(nil, user.paranoid_verified_at) | ||
assert_nil(user.paranoid_verified_at) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. rip |
||
end | ||
|
||
test 'when code not match upon verification code too many attempts should generate new code' do | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is pre Rails 4.2.8 compatibility important? If so, is this the way you want to support it? Ruby 2.4+ requires Rails 4.2.8+ because of changes in the
json
gem.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMO, its easy enough to upgrade to 4.2.[8, 9, 10] we should not need to support previous versions