django-rest-framework-jwt icon indicating copy to clipboard operation
django-rest-framework-jwt copied to clipboard

Fix is_active error message

Open iliasgal opened this issue 8 years ago • 2 comments
trafficstars

Fix for issue #303

If user is disabled (is_active=False), the authenticate function returns None, so the user variable in:

user = authenticate(**credentials)

is always None.

Therefore, the if statement below is always False and the commands are never executed.

if not user.is_active:
    msg = _('User account is disabled.')
    raise serializers.ValidationError(msg)

My suggestion is to move the check for if not user.is_active: under the else statement.

We get the user with a query based on the given username. We also add an exception if ObjectDoesNotExist.

If user exists, then we check if it is active or not. If the ObjectDoesNotExist is thrown, then it means that the login credentials were not correct.

 try:
    user = User.objects.get(**{self.username_field: attrs.get(self.username_field)})
    if not user.is_active:
        msg = _('User account is disabled.')
        raise serializers.ValidationError(msg)
    except ObjectDoesNotExist:
        msg = _('Unable to log in with provided credentials.')
        raise serializers.ValidationError(msg)

iliasgal avatar Oct 25 '17 14:10 iliasgal

Waiting for the merge :)

manan avatar Nov 25 '17 01:11 manan

And just FYI, this commit will not have expected behaviour in the case where someone sends a username of an inactive user and an INCORRECT password.

According to the commit, the response will be 'User account is disabled.' but it should be 'Unable to log in with provided credentials.' as the password in itself is incorrect. The way I see it is, the only time 'User account is disabled.' should be returned is when the credentials are correct but the user is inactive.

manan avatar Nov 25 '17 03:11 manan