lynndylanhurley / lynndylanhurley/devise_token_auth

Lockable does not work as expected

Open
#1,610 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Ruby
Stars
3.6k
Forks
1.1k
PR merge metrics
No merged PRs in 30d

Description

Hello!

Thank you very much for this gem. I use it a lot and it saves me so much time and effort.

I think I have found a bug in the sessions controller, surrounding how it handles the Lockable strategy.

**Version**
devise_token_auth (1.2.2)
devise (4.9.2)

**Configuration**
config.lock_strategy = :failed_attempts
config.maximum_attempts = 10
config.unlock_keys = [:email]
config.unlock_strategy = :time
config.unlock_in = 30.minutes
config.last_attempt_warning = true

**Expected Behaviour**
I have maximum_attempts set to 10 and last_attempt_warning is set to TRUE, so I expect that:
* A pre-lock warning is returned after the 9th failed sign-in attempt.
* An account-locked message is returned after the 10th failed sign-in attempt.

**Actual Behaviour**
* After the 9th and 10th failed sign-in attempts, I see the bad_credentials message.
* After the 11th failed sign-in attempt, I see a message that the account is now locked.
* When users sees the bad_credentials message after the 10th failed sign-in attempt, they still expect to be able to sign-in. However, they cannot, because the account is already locked.
* The user never sees a pre-lock warning.

I was able to bypass this behaviour with the following override to the sessions controller, but this is obviously not an ideal solution:
```
def render_create_error_bad_credentials
if @resource.respond_to?(:locked_at) && @resource.failed_attempts == Devise.maximum_attempts - 1
render_error(401, I18n.t('devise.failure.last_attempt'))
elsif @resource.respond_to?(:locked_at) && @resource.failed_attempts >= Devise.maximum_attempts
render_create_error_account_locked
else
super
end
end
```

Contributor guide

Open the contributing guide

Research direction

Start in the sessions controller at render_create_error_bad_credentials and compare its handling of failed_attempts with the Lockable configuration described in the issue. Verify the behavior around the 9th, 10th, and 11th failed sign-in attempts; done means the pre-lock warning appears on the 9th attempt and the account-locked message appears on the 10th.

Written by the indexing model from the issue text.

Assessment

Tech stack
rails, ruby
Domain
api, authentication
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
38/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.