Skip to content

Commit

Permalink
Provide AUTH fallback support for connection URLs with a username com…
Browse files Browse the repository at this point in the history
…ponent

Prior to ACL support, redis-py ignored the username component of
Connection URLs. With ACL support, usernames are no longer ignored and
are used to authenticate against an ACL rule. Some cloud vendors with
managed Redis instances (like Heroku) provide connection URLs with a
username component pre-ACL that is not intended to be used. Sending that
username to Redis servers < 6.0.0 results in an error. Attempt to detect
this condition and retry the AUTH command with only the password such
that authentication continues to work for these users.

Fixes #1274
  • Loading branch information
andymccurdy committed Jan 31, 2020
1 parent ab1c659 commit e39ae45
Show file tree
Hide file tree
Showing 4 changed files with 34 additions and 1 deletion.
9 changes: 9 additions & 0 deletions CHANGES
Original file line number Diff line number Diff line change
@@ -1,3 +1,12 @@
* 3.4.1 (in development)
* Prior to ACL support, redis-py ignored the username component of
Connection URLs. With ACL support, usernames are no longer ignored and
are used to authenticate against an ACL rule. Some cloud vendors with
managed Redis instances (like Heroku) provide connection URLs with a
username component pre-ACL that is not intended to be used. Sending that
username to Redis servers < 6.0.0 results in an error. Attempt to detect
this condition and retry the AUTH command with only the password such
that authentication continues to work for these users. #1274
* 3.4.0
* Allow empty pipelines to be executed if there are WATCHed keys.
This is a convenient way to test if any of the watched keys changed
Expand Down
2 changes: 2 additions & 0 deletions redis/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
from redis.utils import from_url
from redis.exceptions import (
AuthenticationError,
AuthenticationWrongNumberOfArgsError,
BusyLoadingError,
ChildDeadlockedError,
ConnectionError,
Expand All @@ -35,6 +36,7 @@ def int_or_str(value):

__all__ = [
'AuthenticationError',
'AuthenticationWrongNumberOfArgsError',
'BlockingConnectionPool',
'BusyLoadingError',
'ChildDeadlockedError',
Expand Down
16 changes: 15 additions & 1 deletion redis/connection.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
sendall, shutdown, ssl_wrap_socket)
from redis.exceptions import (
AuthenticationError,
AuthenticationWrongNumberOfArgsError,
BusyLoadingError,
ChildDeadlockedError,
ConnectionError,
Expand Down Expand Up @@ -135,6 +136,8 @@ class BaseParser(object):
'max number of clients reached': ConnectionError,
'Client sent AUTH, but no password is set': AuthenticationError,
'invalid password': AuthenticationError,
'wrong number of arguments for \'auth\' command':
AuthenticationWrongNumberOfArgsError,
},
'EXECABORT': ExecAbortError,
'LOADING': BusyLoadingError,
Expand Down Expand Up @@ -630,7 +633,18 @@ def on_connect(self):
# avoid checking health here -- PING will fail if we try
# to check the health prior to the AUTH
self.send_command('AUTH', *auth_args, check_health=False)
if nativestr(self.read_response()) != 'OK':

try:
auth_response = self.read_response()
except AuthenticationWrongNumberOfArgsError:
# a username and password were specified but the Redis
# server seems to be < 6.0.0 which expects a single password
# arg. retry auth with just the password.
# https://github.com/andymccurdy/redis-py/issues/1274
self.send_command('AUTH', self.password, check_health=False)
auth_response = self.read_response()

if nativestr(auth_response) != 'OK':
raise AuthenticationError('Invalid Username or Password')

# if a client_name is given, set it
Expand Down
8 changes: 8 additions & 0 deletions redis/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -72,3 +72,11 @@ class LockNotOwnedError(LockError):
class ChildDeadlockedError(Exception):
"Error indicating that a child process is deadlocked after a fork()"
pass


class AuthenticationWrongNumberOfArgsError(ResponseError):
"""
An error to indicate that the wrong number of args
were sent to the AUTH command
"""
pass

1 comment on commit e39ae45

@itamarhaber
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@andymccurdy thanks for that - it's exactly the dirty workaround that I had in mind as well :)

Please sign in to comment.