Lockable locks account instead of IP #1944

Closed
AstonJ opened this Issue Jun 23, 2012 · 17 comments

Comments

Projects
None yet
4 participants
@AstonJ
Contributor

AstonJ commented Jun 23, 2012

I'm just trying out lockable, and it seems it locks the account rather than the IP. There are a few problems with this - generally, locking is a measure to prevent someone trying to hack into someone else's account, and this method ends up with the person who owns the account getting penalised (when it might not have been there fault).

Also, it is open to abuse - people can go around purposely locking the accounts of people they do not like.

I think it would be worth considering changing the lock to the IP, that way the 'attacker' is prevented from further hack attempts (usually set to something like 15 minute sign-in ban), and they can't 'punish' someone else on purpose (and it stops them moving on to the next target - people who have a grudge against the site might do this). Of course it means they will need to know the email addresses, but this is not difficult as most email addresses are public (and especially in 'communities' where everyone knows everyone).

Any thoughts on this?

@josevalim

This comment has been minimized.

Show comment Hide comment
@josevalim

josevalim Jun 24, 2012

Member

Yes, request throttling is another way to address this problem and could be
implemented either related to lockable or completely decoupled. The reason
it is not baked into devise is because there are already plugins that does
it for you, you just need to make the protect devise related actions.

Sent from my iPhone

Member

josevalim commented Jun 24, 2012

Yes, request throttling is another way to address this problem and could be
implemented either related to lockable or completely decoupled. The reason
it is not baked into devise is because there are already plugins that does
it for you, you just need to make the protect devise related actions.

Sent from my iPhone

@AstonJ

This comment has been minimized.

Show comment Hide comment
@AstonJ

AstonJ Jun 24, 2012

Contributor

Hi Jose, thanks for the reply.

I didn't realise there were plug-ins for this, though, do you still think it's worth changing the default Devise lockable behaviour? I fear the current implementation is open to abuse, and locking by IP seems to be a more effective solution (without the risk of abuse).

I could try to implement this into Devise if you like? Perhaps as an option if you don't want the default behaviour changed?

Contributor

AstonJ commented Jun 24, 2012

Hi Jose, thanks for the reply.

I didn't realise there were plug-ins for this, though, do you still think it's worth changing the default Devise lockable behaviour? I fear the current implementation is open to abuse, and locking by IP seems to be a more effective solution (without the risk of abuse).

I could try to implement this into Devise if you like? Perhaps as an option if you don't want the default behaviour changed?

@AstonJ

This comment has been minimized.

Show comment Hide comment
@AstonJ

AstonJ Jun 24, 2012

Contributor

Hi Jose,

I think I've come up with a solution that won't require any major modification, in fact there would be very little change to the current implementation.

We could simply add a 'last_failed_sign_in_ip' field to the Users table.
Then, whenever there is a failed log-in, the IP is recorded into that field.
And, Devise lockable works as usual.
But, if an account is locked...
Then, Devise checks whether the IP of the current request matches the IP in the 'last_failed_sign_in_ip' field;
If the IP's match - Devise enforces the lock as usual
If they don't match - Devise allows the log-in

This way only the person/ip who triggered the lock, gets locked out.

What do you think?

Contributor

AstonJ commented Jun 24, 2012

Hi Jose,

I think I've come up with a solution that won't require any major modification, in fact there would be very little change to the current implementation.

We could simply add a 'last_failed_sign_in_ip' field to the Users table.
Then, whenever there is a failed log-in, the IP is recorded into that field.
And, Devise lockable works as usual.
But, if an account is locked...
Then, Devise checks whether the IP of the current request matches the IP in the 'last_failed_sign_in_ip' field;
If the IP's match - Devise enforces the lock as usual
If they don't match - Devise allows the log-in

This way only the person/ip who triggered the lock, gets locked out.

What do you think?

@rodrigoflores

This comment has been minimized.

Show comment Hide comment
@rodrigoflores

rodrigoflores Jun 26, 2012

Contributor

@AstonJ

I see that both solutions are valid (the current one and the one you suggested). It depends on requirements and each application needs.

However, I think (and this is a personal opinion that may not reflects @josevalim's or @carlosantoniodasilva's opinion about this matter) that it goes beyond Devise's scope and Devise's responsibility as an authentication solution. So, you may implement it outside Devise and protect Devise related controllers with it.

You can write a Devise module on a different project and add it on your projects. Also, lockable may have proved itself useful the way it is and then I don't think it should be removed.

Any thoughts @josevalim and @carlosantoniodasilva ?

Contributor

rodrigoflores commented Jun 26, 2012

@AstonJ

I see that both solutions are valid (the current one and the one you suggested). It depends on requirements and each application needs.

However, I think (and this is a personal opinion that may not reflects @josevalim's or @carlosantoniodasilva's opinion about this matter) that it goes beyond Devise's scope and Devise's responsibility as an authentication solution. So, you may implement it outside Devise and protect Devise related controllers with it.

You can write a Devise module on a different project and add it on your projects. Also, lockable may have proved itself useful the way it is and then I don't think it should be removed.

Any thoughts @josevalim and @carlosantoniodasilva ?

@AstonJ

This comment has been minimized.

Show comment Hide comment
@AstonJ

AstonJ Jun 26, 2012

Contributor

Hi Rodrigo

Thanks for the reply :)

I'm not sure if anyone else agrees, but I think the current implementation is open to abuse, and likely to be abused. I have run community sites for several years, and know only too well that occasionally you will get trolls or problematic members - and if they work out that they can lock people out of their accounts, they will be more than tempted.

Imagine if the account of an administrator was continually locked out for 24 hours or more? (Perhaps while troublemakers run riot on the site.) It doesn't bear thinking about lol.

I feel the proposed implementation fixes the exploit, while retaining (and strengthening) the original purpose - to prevent people from guessing other people's accounts and/or dictionary attacks.

I don't mean to keep going on about it, but I just know from experience that troublemakers will be quick to exploit loopholes like this.

Cheers,

Aston

Contributor

AstonJ commented Jun 26, 2012

Hi Rodrigo

Thanks for the reply :)

I'm not sure if anyone else agrees, but I think the current implementation is open to abuse, and likely to be abused. I have run community sites for several years, and know only too well that occasionally you will get trolls or problematic members - and if they work out that they can lock people out of their accounts, they will be more than tempted.

Imagine if the account of an administrator was continually locked out for 24 hours or more? (Perhaps while troublemakers run riot on the site.) It doesn't bear thinking about lol.

I feel the proposed implementation fixes the exploit, while retaining (and strengthening) the original purpose - to prevent people from guessing other people's accounts and/or dictionary attacks.

I don't mean to keep going on about it, but I just know from experience that troublemakers will be quick to exploit loopholes like this.

Cheers,

Aston

@josevalim

This comment has been minimized.

Show comment Hide comment
@josevalim

josevalim Jun 26, 2012

Member

Hello Aston,

If someone can change IPs easily (or they are doing a coordinate attack), the best solution would be to have a failed sign in log table (IP access and account) so we can properly block users doing coordinate attacks. Otherwise, your solution would not block an account if you have two IPs trying to brute-force into the same account. However, this is out of Devise scope since it adds an extra table and I believe it would be better tackled as a plugin.

We are aware of the limitations you mention about lockable and we should consider improving its documentation if that's not obvious to other developers.

Thanks for your input!

Member

josevalim commented Jun 26, 2012

Hello Aston,

If someone can change IPs easily (or they are doing a coordinate attack), the best solution would be to have a failed sign in log table (IP access and account) so we can properly block users doing coordinate attacks. Otherwise, your solution would not block an account if you have two IPs trying to brute-force into the same account. However, this is out of Devise scope since it adds an extra table and I believe it would be better tackled as a plugin.

We are aware of the limitations you mention about lockable and we should consider improving its documentation if that's not obvious to other developers.

Thanks for your input!

@AstonJ

This comment has been minimized.

Show comment Hide comment
@AstonJ

AstonJ Jun 26, 2012

Contributor

Hi Jose,

Yes that was my original idea too (separate table to track failed login IPs) but after discussing it with @excid3 we thought it would be too much for Devise which is meant to be a simple solution, and like you said would probably better as a Devise gem.

Regarding people changing IPs easily - yes that would be the compromise, although I think them having to do so would be a deterrent in itself.

Tho saying that... maybe there could be two levels in lockable. One which works as I mentioned (so only the person/IP of the last failed log-in get's locked out - out of say 5 failed attempts) and an additional increased level (say 30 or so failed log-in attempts) where the original version kicks in (ie Devise blocks the account for everyone) and perhaps the account then has to be unlocked via email as it indicates an aggressive dictionary attack (as they are also changing IPs). Perhaps an email is sent to admin too.

That way there would be no need for additional tables, and the implementation isn't that far from what you have now - yet you have a much more robust and secure system :-)

Of course the ideal solution is a FailedLogin model/table, which tracks IPs, Time and UserID - but I think the above is an excellent compromise that I would be more than happy with :)

Anyway, I respect your decision, thanks again for taking the time to reply.

Contributor

AstonJ commented Jun 26, 2012

Hi Jose,

Yes that was my original idea too (separate table to track failed login IPs) but after discussing it with @excid3 we thought it would be too much for Devise which is meant to be a simple solution, and like you said would probably better as a Devise gem.

Regarding people changing IPs easily - yes that would be the compromise, although I think them having to do so would be a deterrent in itself.

Tho saying that... maybe there could be two levels in lockable. One which works as I mentioned (so only the person/IP of the last failed log-in get's locked out - out of say 5 failed attempts) and an additional increased level (say 30 or so failed log-in attempts) where the original version kicks in (ie Devise blocks the account for everyone) and perhaps the account then has to be unlocked via email as it indicates an aggressive dictionary attack (as they are also changing IPs). Perhaps an email is sent to admin too.

That way there would be no need for additional tables, and the implementation isn't that far from what you have now - yet you have a much more robust and secure system :-)

Of course the ideal solution is a FailedLogin model/table, which tracks IPs, Time and UserID - but I think the above is an excellent compromise that I would be more than happy with :)

Anyway, I respect your decision, thanks again for taking the time to reply.

@josevalim

This comment has been minimized.

Show comment Hide comment
@josevalim

josevalim Jun 27, 2012

Member

This approach means adding two more fields, one for the ip and another one for the ip related counting. Not sure if it is really worthy compared against simply logging the failed attempts in another table. In any case, would you be willing to try a simple patch/proof of concept (for example simply adding these two extra columns and incrementing them on ip access) so we can see how much complexity it would add to the current implementation?

Member

josevalim commented Jun 27, 2012

This approach means adding two more fields, one for the ip and another one for the ip related counting. Not sure if it is really worthy compared against simply logging the failed attempts in another table. In any case, would you be willing to try a simple patch/proof of concept (for example simply adding these two extra columns and incrementing them on ip access) so we can see how much complexity it would add to the current implementation?

@AstonJ

This comment has been minimized.

Show comment Hide comment
@AstonJ

AstonJ Jun 27, 2012

Contributor

Hi Jose,

I guess that would work too, but if you want to keep new fields to a minimum, then we could work with just one additional field 'last_failed_sign_in_ip' as there is already one for failed_attempts.

If we went with just one additional field then I envisage it would work something like this:

/Logging/
normal devise behaviour: When there is a failed login, Devise increments the failed_attempts field
new behaviour: Devise adds the IP of the failed request to the last_failed_sign_in_ip field
Devise continues to increment failed_attempts and overwrites last_failed_sign_in_ip each time there is a failed login.

/Checking & locking/
When a user tries to log in (I'm guessing) Devise checks to see if failed attempts is over X amount, then looks at the locked_at time and whether that has expired (depending on what's set in config). If all clear then the user is allowed to log-in (and those fields reset), if not they are denied access.

The new system would be the same except it would ignore the check if the IP of the current user does not match the IP of the last_failed_sign_in_ip, however it would not reset the fields if locked_at field is less than 24 hours old.

That means it will keep incrementing failed_attempts (to protect against someone continually changing IP addresses). Then in addition to the checks described above, Devise would also check whether failed_attempts meets the second (higher) new config setting and then either locks the account for everyone (which then has to be unlocked via email - perhaps telling the user what's going on, and if it wasn't them to contact admin) or it silently sends an email to admin so they can investigate.


That's the sort of thing I had in mind - you could go more minimal if you wanted to by removing the email steps.

I am happy to give this a go so you can see how it effects the codebase - just let me know which implementation you would prefer :)

Contributor

AstonJ commented Jun 27, 2012

Hi Jose,

I guess that would work too, but if you want to keep new fields to a minimum, then we could work with just one additional field 'last_failed_sign_in_ip' as there is already one for failed_attempts.

If we went with just one additional field then I envisage it would work something like this:

/Logging/
normal devise behaviour: When there is a failed login, Devise increments the failed_attempts field
new behaviour: Devise adds the IP of the failed request to the last_failed_sign_in_ip field
Devise continues to increment failed_attempts and overwrites last_failed_sign_in_ip each time there is a failed login.

/Checking & locking/
When a user tries to log in (I'm guessing) Devise checks to see if failed attempts is over X amount, then looks at the locked_at time and whether that has expired (depending on what's set in config). If all clear then the user is allowed to log-in (and those fields reset), if not they are denied access.

The new system would be the same except it would ignore the check if the IP of the current user does not match the IP of the last_failed_sign_in_ip, however it would not reset the fields if locked_at field is less than 24 hours old.

That means it will keep incrementing failed_attempts (to protect against someone continually changing IP addresses). Then in addition to the checks described above, Devise would also check whether failed_attempts meets the second (higher) new config setting and then either locks the account for everyone (which then has to be unlocked via email - perhaps telling the user what's going on, and if it wasn't them to contact admin) or it silently sends an email to admin so they can investigate.


That's the sort of thing I had in mind - you could go more minimal if you wanted to by removing the email steps.

I am happy to give this a go so you can see how it effects the codebase - just let me know which implementation you would prefer :)

@josevalim

This comment has been minimized.

Show comment Hide comment
@josevalim

josevalim Jun 27, 2012

Member

Thanks for the reply. If I get this properly, you would use one extra field (last_failed_sign_in_ip) and set the locked_at field when the account is first blocked for that given IP address. This way, we can know the IP address and when the account was locked for that IP, correct?

One pending issue with this approach is that it does not consider the fact that an user may accidentally block his account after trying (let's say) 10 times from the same IP. So far, you have only mentioned that the account will be unlocked after some time, but under such circumstances, we should provide a way for them to simply unlock it (maybe simply linking to the send me unblock instructions page).

Also, we need consider what we are trying to avoid here. You have mentioned before this makes the system more robust and secure. I agree it makes it more robust, but we gain very little in terms of security. The only gain I can see is that today, once the user signs in, the account is effectively unlocked so someone can try to hijack it again, while in your scenario, we have a small gap (in between the account is blocked for an IP and fully blocked) when the user is still allowed to sign in but that given IP attacker cannot continue trying during a period (usually 24h).

Another approach which may be more effective against vandalism and to improve security is to simply include a captcha after 3-5 failed attempts. Brute-forces attacks are limited and those wishing to simply block someone's account, now need to pass through an extra hassle (going through the captcha 7 or 15 times is discouraging enough).

Member

josevalim commented Jun 27, 2012

Thanks for the reply. If I get this properly, you would use one extra field (last_failed_sign_in_ip) and set the locked_at field when the account is first blocked for that given IP address. This way, we can know the IP address and when the account was locked for that IP, correct?

One pending issue with this approach is that it does not consider the fact that an user may accidentally block his account after trying (let's say) 10 times from the same IP. So far, you have only mentioned that the account will be unlocked after some time, but under such circumstances, we should provide a way for them to simply unlock it (maybe simply linking to the send me unblock instructions page).

Also, we need consider what we are trying to avoid here. You have mentioned before this makes the system more robust and secure. I agree it makes it more robust, but we gain very little in terms of security. The only gain I can see is that today, once the user signs in, the account is effectively unlocked so someone can try to hijack it again, while in your scenario, we have a small gap (in between the account is blocked for an IP and fully blocked) when the user is still allowed to sign in but that given IP attacker cannot continue trying during a period (usually 24h).

Another approach which may be more effective against vandalism and to improve security is to simply include a captcha after 3-5 failed attempts. Brute-forces attacks are limited and those wishing to simply block someone's account, now need to pass through an extra hassle (going through the captcha 7 or 15 times is discouraging enough).

@AstonJ

This comment has been minimized.

Show comment Hide comment
@AstonJ

AstonJ Jun 27, 2012

Contributor

Hi Jose,

Not quite as you described - but close :)

Devise will lock the account after say 5 failed login attempts as normal but when it comes to making the check (to deny or allow) a login, Devise will look at the IP of the current request, and the IP in the last_failed_sign_in_ip field and if they match Devise will lock account as normal (say for 20 minutes) if they don't match Devise will allow the login (presuming it was the correct password).

So at this stage, we get protection against someone trying to guess someone else's password - yet (if the IP's don't match) it does not penalise the owner of the account. So we eradicate the exploit that currently exists - so people can't go round locking other people's accounts.

So that leaves us with dealing with coordinated dictionary attacks - i.e sophisticated attacks by those who have the ability to switch IPs.

That's where the second lock amount comes in - so let's say 5 failed attempts for normal IP lockouts (as described above), and say 40 for total lockouts:

# Number of authentication tries before locking an account by ip
# To deal with people trying to guess someone else's account password
config.maximum_attempts_by_ip = 5

# Number of total authentication tries before fully locking an account
# To deal with more sophisticated dictionary attacks from multiple IPs
config.maximum_attempts_by_total = 40

# Sophisticated dictionary attack period
# Resets failed attempts after this period
config.sophisticated_attack_peroiod_check = 1.day

So here, Devise is not just checking for 5 failed attempts - but also for 40. With the 5 failed attempts it works as described above, but when the failed attempts hit 40 we know two things:

  • There is a coordinated attack against this account
  • It is a sophisticated attack because we know they are using multiple IPs (or someone is persistent i.e. keeps trying to hack an account despite having to wait 20 minutes between each try after 5 attempts)

So then the account is locked down fully and must then either by unlocked by email (as per the current Devise option/implementation) or we can lock it for a longer period (say two hours?) and silently notify admin - just depends on how you want it to behave really.

The benefits of this system:

  • We deal with people trying to guess other people's passwords
  • We remove exploit where people can lock down other peoples accounts
  • We are able to deal with sophisticated attacks from multiple IPs

(If any of this doesn't make sense let me know and I will try to rephrase it)

Contributor

AstonJ commented Jun 27, 2012

Hi Jose,

Not quite as you described - but close :)

Devise will lock the account after say 5 failed login attempts as normal but when it comes to making the check (to deny or allow) a login, Devise will look at the IP of the current request, and the IP in the last_failed_sign_in_ip field and if they match Devise will lock account as normal (say for 20 minutes) if they don't match Devise will allow the login (presuming it was the correct password).

So at this stage, we get protection against someone trying to guess someone else's password - yet (if the IP's don't match) it does not penalise the owner of the account. So we eradicate the exploit that currently exists - so people can't go round locking other people's accounts.

So that leaves us with dealing with coordinated dictionary attacks - i.e sophisticated attacks by those who have the ability to switch IPs.

That's where the second lock amount comes in - so let's say 5 failed attempts for normal IP lockouts (as described above), and say 40 for total lockouts:

# Number of authentication tries before locking an account by ip
# To deal with people trying to guess someone else's account password
config.maximum_attempts_by_ip = 5

# Number of total authentication tries before fully locking an account
# To deal with more sophisticated dictionary attacks from multiple IPs
config.maximum_attempts_by_total = 40

# Sophisticated dictionary attack period
# Resets failed attempts after this period
config.sophisticated_attack_peroiod_check = 1.day

So here, Devise is not just checking for 5 failed attempts - but also for 40. With the 5 failed attempts it works as described above, but when the failed attempts hit 40 we know two things:

  • There is a coordinated attack against this account
  • It is a sophisticated attack because we know they are using multiple IPs (or someone is persistent i.e. keeps trying to hack an account despite having to wait 20 minutes between each try after 5 attempts)

So then the account is locked down fully and must then either by unlocked by email (as per the current Devise option/implementation) or we can lock it for a longer period (say two hours?) and silently notify admin - just depends on how you want it to behave really.

The benefits of this system:

  • We deal with people trying to guess other people's passwords
  • We remove exploit where people can lock down other peoples accounts
  • We are able to deal with sophisticated attacks from multiple IPs

(If any of this doesn't make sense let me know and I will try to rephrase it)

@josevalim

This comment has been minimized.

Show comment Hide comment
@josevalim

josevalim Jun 27, 2012

Member

In order to properly detect the sophisticated attack period, you need to use the locked_at column or an extra column to store when the account was first locked by the IP, right? Unless I am wrong, the first and third benefits are also available today, except that we treat them with the same severity (i.e. lock the account), the only fix is that attackers can't lock other people's account.

So the two points from my previous comment stand: 1) we need a way to tell the user that an IP was locked (via e-mail or on screen notification, in case the user locks his own account) and 2) we need to evaluate how this option would stand against other simpler options like a captcha.

Member

josevalim commented Jun 27, 2012

In order to properly detect the sophisticated attack period, you need to use the locked_at column or an extra column to store when the account was first locked by the IP, right? Unless I am wrong, the first and third benefits are also available today, except that we treat them with the same severity (i.e. lock the account), the only fix is that attackers can't lock other people's account.

So the two points from my previous comment stand: 1) we need a way to tell the user that an IP was locked (via e-mail or on screen notification, in case the user locks his own account) and 2) we need to evaluate how this option would stand against other simpler options like a captcha.

@AstonJ

This comment has been minimized.

Show comment Hide comment
@AstonJ

AstonJ Jun 27, 2012

Contributor

There's already a locked_at column - so we could just use that (and continue to set it as it is now - the difference is how we deal with it once it's triggered and later checked).

Re benefits 1 & 3 in current implementation - in a way yes (which is why I think this proposal fits so well) but in the current implementation there is just one setting, and it has the negative side effects of A) being open to abuse, B) not being able to discern between the different types of attacks (and deal with them accordingly).

With regards to notification - yes, for the IP based lock it would just say something like "Due to a number of failed logins your IP is being prevented from logging into this account for 15 minutes - please try again later". For the big lock down, there would be another message and/or email depending on how you wanted to deal with it (silently with longer lock-out or via an email unlock).

With regards to using captcha instead - yes that could be simpler and better :) (Are there any downsides to using it?)

You decide and let us know :p and thanks for considering improving this aspect of Devise.

Contributor

AstonJ commented Jun 27, 2012

There's already a locked_at column - so we could just use that (and continue to set it as it is now - the difference is how we deal with it once it's triggered and later checked).

Re benefits 1 & 3 in current implementation - in a way yes (which is why I think this proposal fits so well) but in the current implementation there is just one setting, and it has the negative side effects of A) being open to abuse, B) not being able to discern between the different types of attacks (and deal with them accordingly).

With regards to notification - yes, for the IP based lock it would just say something like "Due to a number of failed logins your IP is being prevented from logging into this account for 15 minutes - please try again later". For the big lock down, there would be another message and/or email depending on how you wanted to deal with it (silently with longer lock-out or via an email unlock).

With regards to using captcha instead - yes that could be simpler and better :) (Are there any downsides to using it?)

You decide and let us know :p and thanks for considering improving this aspect of Devise.

@josevalim

This comment has been minimized.

Show comment Hide comment
@josevalim

josevalim Jun 28, 2012

Member

With regards to using captcha instead - yes that could be simpler and better :) (Are there any downsides to using it?)

I cannot think of any downside. So should we provide a simple mechanism for people to use captcha or add an IP column to lockable?

/cc @rafaelfranca @rodrigoflores @carlosantoniodasilva

Member

josevalim commented Jun 28, 2012

With regards to using captcha instead - yes that could be simpler and better :) (Are there any downsides to using it?)

I cannot think of any downside. So should we provide a simple mechanism for people to use captcha or add an IP column to lockable?

/cc @rafaelfranca @rodrigoflores @carlosantoniodasilva

@josevalim

This comment has been minimized.

Show comment Hide comment
@josevalim

josevalim Jun 28, 2012

Member

Ah, there is a limitation with captcha. It does not work for API requests (so you can lock an account using APIs).

Member

josevalim commented Jun 28, 2012

Ah, there is a limitation with captcha. It does not work for API requests (so you can lock an account using APIs).

@rodrigoflores

This comment has been minimized.

Show comment Hide comment
@rodrigoflores

rodrigoflores Jun 28, 2012

Contributor

I prefer adding the IP column on lockable. I'm not a huge fan of captchas and the solution may compromise if one find a flaw on the captcha library. In the other hand, the IP approach seems robust.

Contributor

rodrigoflores commented Jun 28, 2012

I prefer adding the IP column on lockable. I'm not a huge fan of captchas and the solution may compromise if one find a flaw on the captcha library. In the other hand, the IP approach seems robust.

@ronalchn

This comment has been minimized.

Show comment Hide comment
@ronalchn

ronalchn Aug 17, 2012

Contributor

If you are looking by IP, it would probably work better to create a new "failed IPs" table, rather than doing it by user account, which has limitations such as only being able to store a single IP, and is more vulnerable to a single IP brute-forcing multiple user accounts at the same time.

I think captcha is a good idea, and it would be beneficial to add captcha as a new module. I already use recaptcha on my account registrations, and I think it would be useful to make it so that the RegistrationsController does not have to be overriden to add recaptcha.

For API requests, after a number of failed requests, you can disable that IP address - requiring a captcha, thus preventing an application from logging on via API.

Contributor

ronalchn commented Aug 17, 2012

If you are looking by IP, it would probably work better to create a new "failed IPs" table, rather than doing it by user account, which has limitations such as only being able to store a single IP, and is more vulnerable to a single IP brute-forcing multiple user accounts at the same time.

I think captcha is a good idea, and it would be beneficial to add captcha as a new module. I already use recaptcha on my account registrations, and I think it would be useful to make it so that the RegistrationsController does not have to be overriden to add recaptcha.

For API requests, after a number of failed requests, you can disable that IP address - requiring a captcha, thus preventing an application from logging on via API.

@josevalim josevalim closed this Jan 11, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment