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.

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

Could you move login_lock! out of protected #46

Closed
itay-grudev opened this issue Feb 12, 2017 · 9 comments
Closed

Could you move login_lock! out of protected #46

itay-grudev opened this issue Feb 12, 2017 · 9 comments

Comments

@itay-grudev
Copy link

Could you move login_lock! out of protected like login_unlock!. It's quite useful for testing purposes and invoking it with send is rather ugly.

@itay-grudev
Copy link
Author

itay-grudev commented Feb 12, 2017

Besides we can use this feature of the library to lock a user manually. It may also be useful to have an argument for explicitly setting a lock duration or infinity.

@ebihara99999
Copy link
Contributor

ebihara99999 commented Mar 1, 2017

For a testing purpose, i personally think It's better to implement login_lock! as a test helper than moving login_lock! out of protected.

Besides we can use this feature of the library to lock a user manually. It may also be useful to have an argument for explicitly setting a lock duration or infinity.

Would you tell me the use case? I guess you want to manage locked status of users independently for some reasons, for example, user A: locked( because the user broke terms of service ), user B: unlocked. Then, login_lock! is against brute_force_protection, and the use case is different from intention. It might be a good idea to implement manual user lock status and the management feature.

@Ch4s3
Copy link
Contributor

Ch4s3 commented Mar 24, 2017

@itay-grudev if you could clarify re: @ebihara99999's question, I'll take a look.

@itay-grudev
Copy link
Author

itay-grudev commented Mar 25, 2017

The login login_lock! method's visibility scope is protected and cannot be used externally. The reason why I need it is for testing.

I literally mean test whether a controller action lock/unlocks a user.

@itay-grudev
Copy link
Author

But also because it may be useful for purposes other than testing moving it out of the protected scope may be more useful than making a login_lock! test helper.

@itay-grudev
Copy link
Author

@ebihara99999 One may have some other method of detecting fraudulent login attempts and may want to lock a certain user manually.

@ebihara99999
Copy link
Contributor

@itay-grudev Sorry it's been too long. For the testing purpose, as I said, it would be better to implement a test helper method. But it seems reasonable to need #login_lock for the general use, not only for brute force protection.
In this case, it's necessary to add or refactoring existing one into a lockable module like devise, and I don't know how much time to need...
What do you think, @Ch4s3 ? Need to add this feature to the load map?

@ebihara99999
Copy link
Contributor

@Ch4s3
No discussion has been updated recently, and the subject seems fully discussed.
I think it would be better to close this.

@Ch4s3
Copy link
Contributor

Ch4s3 commented Dec 12, 2017

I'm going to close this, but would reopen it if people are still interested.

@Ch4s3 Ch4s3 closed this as completed Dec 12, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants