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

django-defender not working on my custom login view #173

Open
ashmlk opened this issue Dec 4, 2020 · 10 comments
Open

django-defender not working on my custom login view #173

ashmlk opened this issue Dec 4, 2020 · 10 comments

Comments

@ashmlk
Copy link

ashmlk commented Dec 4, 2020

Hello, I am trying to add watch_login() decorator to y custom login view however, my user is still able to login after passing the lockout limit which I currently set to 3.

My current settings is:

DEFENDER_LOGIN_FAILURE_LIMIT = 3
DEFENDER_LOCK_OUT_BY_IP_AND_USERNAME = True
DEFENDER_COOLOFF_TIME = 300
DEFENDER_LOCKOUT_TEMPLATE = 'user_locked_out.html'

And my custom login is:

@watch_login()
def user_login(request):
    if request.user.is_authenticated:
        if request.user.is_active:
            return redirect('home:home')   
    if request.method == 'POST':    
        username = request.POST.get('username')
        password = request.POST.get('password')
        user = authenticate(username=username, password=password)
        if user is not None:
            if user.is_active:
                login(request, user)
                return redirect('home:home')
            else:
                return HttpResponse('Account is disabled')
        else:
            messages.error(request,'Sorry, the username or password you entered is not correct')     
            return redirect('main:user_login')           
    return render(request, 'main/user_login.html')

And for my cache I am using Redis so currently, I have:

CACHES = {
    "default": {
        "BACKEND": "redis_cache.RedisCache",
        "LOCATION": "redis://127.0.0.1:6379/1",
        "OPTIONS": {
            "CLIENT_CLASS": "django_redis.client.DefaultClient",
            "IGNORE_EXCEPTIONS": True,  
        },
        "KEY_PREFIX": "redis_one"
    }
}

I do not see any errors except that the django-defender is doing nothing I was wondering how I could fix this issue.

@kencochrane
Copy link
Collaborator

If you are logging access attempts are you seeing any entries in the database for you login attempts?

@ashmlk
Copy link
Author

ashmlk commented Dec 5, 2020

No I am not saving the logs in the database, I believe the issue may be my watch_login() decorator?

EDIT

I'd like to also note that when I use watch_login (Without parentheses) I get an error:

'function' object has no attribute 'get'

@kencochrane
Copy link
Collaborator

What version of defender, Python, and django are you using?

Maybe use the decorator on the urls.py like shown here?

https://stackoverflow.com/questions/5796897/wrapping-decorating-a-function-in-urls-py-vs-in-views-py/5797020#5797020

@ashmlk
Copy link
Author

ashmlk commented Dec 5, 2020

My python version is 3.8, Django version is 3.0, and my defender is 0.8.0.

I have already tried wrapping it around in my urls.py I still get the same error function' object has no attribute 'get'

@kencochrane
Copy link
Collaborator

There might be an exception getting thrown from the decorator that is causing this error, can you post the full stack trace?

@ashmlk
Copy link
Author

ashmlk commented Dec 5, 2020

THIS METHOD HAS FLAWS

I was able to fix this issue. The problem was that inside my login function I was redirecting to the login page again which was causing the watch_login() decorator to never return true on this condition: https://github.com/jazzband/django-defender/blob/a1d526f3187d2d86152edec537b012bca3495794/defender/decorators.py#L29-L34

So now my login view is:

@watch_login(status_code=302)
def user_login(request):
    if request.user.is_authenticated:
        if request.user.is_active:
            return redirect('home:home')   
    if request.method == 'POST':    
        username = request.POST.get('username')
        password = request.POST.get('password')
        user = authenticate(username=username, password=password)
        if user is not None:
            if user.is_active:
                login(request, user)
                return redirect('home:home')
            else:
                return HttpResponse('Account is disabled')
        else:
            messages.error(request,'Sorry, the username or password you entered is not correct')     
            return render(request, 'main/user_login.html')     # this line was edited from redirect() to render()
    return render(request, 'main/user_login.html')

I think this issue can be marked as closed if my method seems to work correctly

@kencochrane
Copy link
Collaborator

I'm glad you have it working, feel free to close this issue unless there is something else we can help you with.

@ashmlk
Copy link
Author

ashmlk commented Dec 6, 2020

There is a slight issue with this approach and it is that that when we do not redirect after incorrect credentials, the POST form fields are not cleared from the browser and can cause the duplicate submission of the form. Django redirects with status code 302 on the wrong login, however, here:

if status_code == 302: # standard Django login view
login_unsuccessful = (
response
and not response.has_header("location")
and response.status_code != status_code
)

This will cause watch_login() to always fail since we are returning 302 for both successful and unsuccessful login attempts. Now I do not know what would be the best status code for this case. I am currently using 303, but I don't think it's the best option here.

@kencochrane
Copy link
Collaborator

OK, have you looked at how the official Django login view handles it, maybe you can copy what they did to get yours working correctly.

@ashmlk
Copy link
Author

ashmlk commented Dec 6, 2020

I have taken a look at Django's LoginView and it seems that after failed login, Django renders the form with the error messages which means that the previous POST parameters are still saved on the browser. As far as my understanding this seems to be an odd issue with watch_login() which if we redirect on failed login with status 302 the behaviour of watch_login() will cause it to never return true. However, I may be completely wrong please correct me if I am wrong. But as far as I understand, either we need to return a different status_code when redirecting (303 or 307 I am still uncertain about that) or watch_login() needs to accept redirects with status code 302 on a failed login which I believe that also cause new challenges.

References:

https://docs.djangoproject.com/en/3.1/topics/auth/default/#django.contrib.auth.views.LoginView

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

2 participants