Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[MNOE-209] Unlimited failed attempts of login #162

Merged
merged 2 commits into from
Nov 15, 2017
Merged

[MNOE-209] Unlimited failed attempts of login #162

merged 2 commits into from
Nov 15, 2017

Conversation

x4d3
Copy link
Contributor

@x4d3 x4d3 commented Nov 29, 2016

user session is not returning either not found, either a user with password_not valid if the password is valid
Resource needed to be returned properly to be able to be updated in the devise-3.5.10/lib/devise/models/lockable.rb

@x4d3 x4d3 added the bug label Nov 29, 2016
ouranos
ouranos approved these changes Nov 29, 2016
View reviewed changes
Copy link
Contributor

@ouranos ouranos left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

Except for the specs ;)

@x4d3
Copy link
Contributor Author

x4d3 commented Dec 6, 2016

@ouranos Specs are passing

@x4d3 x4d3 force-pushed the fix/mnoe-196-functional-able-to-login-without-confirmation-done-via-email branch from edcf76f to 5eb2da5 Compare January 10, 2017 17:09
@x4d3 x4d3 force-pushed the fix/mnoe-196-functional-able-to-login-without-confirmation-done-via-email branch from adcea6f to 254935d Compare April 3, 2017 08:35
@ouranos ouranos modified the milestone: v3.3 Jun 20, 2017
@ouranos ouranos changed the base branch from master to 3.3 June 20, 2017 07:37
@x4d3 x4d3 closed this Jul 18, 2017
@x4d3 x4d3 deleted the fix/mnoe-196-functional-able-to-login-without-confirmation-done-via-email branch July 18, 2017 08:21
@x4d3 x4d3 restored the fix/mnoe-196-functional-able-to-login-without-confirmation-done-via-email branch July 18, 2017 08:23
@x4d3 x4d3 reopened this Jul 18, 2017
@x4d3
[MNOE-209] Unlimited failed attempts of login
2a9cb6c 
user session is not returning either not found, either a user with password_not valid if the password is valid
Resource needed to be returned properly to be able to be updated in the devise-3.5.10/lib/devise/models/lockable.rb
@x4d3 x4d3 force-pushed the fix/mnoe-196-functional-able-to-login-without-confirmation-done-via-email branch from 254935d to 2a9cb6c Compare November 13, 2017 15:46
@x4d3
Copy link
Contributor Author

x4d3 commented Nov 13, 2017

@ouranos I've rebased and squashed. Ready for final review.
You may test that the locking works on an express by adding in an express pointing to that version.
config/initializers/devise.rb

  config.lock_strategy = :failed_attempts
  config.maximum_attempts = 3
  config.last_attempt_warning = true
  config.paranoid = false
end

ouranos
ouranos approved these changes Nov 14, 2017
View reviewed changes
Copy link
Contributor

@ouranos ouranos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

I'll do some testing locally too

core/lib/devise/strategies/remote_authenticatable.rb Outdated
# mapping.to is a wrapper over the resource model
resource = mapping.to.new
resource = mapping.to.authenticate(auth_params)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd keep mapping.to.remote_authenticate(auth_params)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you meant mapping.to.new.remote_authentication(auth_params) ?

core/lib/devise/strategies/remote_authenticatable.rb Outdated
@@ -24,9 +25,10 @@ def authenticate!
#
# If the block returns true the resource will be loged in
# If the block returns false the authentication will fail!
if validate(resource){ resource = resource.remote_authentication(auth_params) }
if validate(resource){ resource.password_valid }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Extra space

core/lib/devise/strategies/remote_authenticatable.rb Outdated
success!(resource)
end

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Extra line

@x4d3
Copy link
Contributor Author

x4d3 commented Nov 14, 2017

@ouranos I've addressed your comments.

@x4d3 x4d3 force-pushed the fix/mnoe-196-functional-able-to-login-without-confirmation-done-via-email branch from b031b05 to b470997 Compare November 14, 2017 15:44
@ouranos ouranos merged commit 018e918 into maestrano:3.3 Nov 15, 2017
aluqueGH pushed a commit to aluqueGH/mno-enterprise that referenced this pull request Jul 10, 2018
@alexnoox
Merge pull request maestrano#162 from clemthenem/320_prevent_toastr_d…
4779946 
…uplicate

[MNOE-320] App Comparison - Multiple notification displayed when more than 4 apps
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ouranos ouranos ouranos approved these changes

Assignees

@ouranos ouranos

Labels
bug
Projects
None yet
Milestone
v3.3
Development

Successfully merging this pull request may close these issues.
2 participants
@x4d3 @ouranos