Skip to content

Commit

Permalink
Fix case where sqlachemy db driver raises exception (#69)
Browse files Browse the repository at this point in the history
If query a numeric column with a string value, some drivers (pyscopg) will throw
a DataError. This is similar fix to mongo, peewee, pony datastores.
  • Loading branch information
jwag956 committed May 9, 2019
1 parent 210fa0b commit c06cd13
Show file tree
Hide file tree
Showing 2 changed files with 38 additions and 7 deletions.
21 changes: 15 additions & 6 deletions flask_security/datastore.py
Original file line number Diff line number Diff line change
Expand Up @@ -241,24 +241,33 @@ def __init__(self, db, user_model, role_model):

def get_user(self, identifier):
from sqlalchemy import func as alchemyFn
from sqlalchemy.exc import DataError

user_model_query = self.user_model.query
if hasattr(self.user_model, 'roles'):
from sqlalchemy.orm import joinedload
user_model_query = user_model_query.options(joinedload('roles'))

rv = self.user_model.query.get(identifier)
if rv is not None:
return rv
try:
rv = user_model_query.get(identifier)
if rv is not None:
return rv
except DataError:
# This can happen if trying a string against a numeric column
pass

# Not PK - iterate through other attributes and look for 'identifier'
for attr in get_identity_attributes():
# Look for exact case-insensitive match - 'ilike' honors wild cards
# which isn't what we want.
query = alchemyFn.lower(getattr(self.user_model, attr)) \
== alchemyFn.lower(identifier)
rv = user_model_query.filter(query).first()
if rv is not None:
return rv
try:
rv = user_model_query.filter(query).first()
if rv is not None:
return rv
except DataError:
pass

def find_user(self, **kwargs):
query = self.user_model.query
Expand Down
24 changes: 23 additions & 1 deletion tests/test_datastore.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,13 @@
Datastore tests
"""

from mock import MagicMock, patch
from pytest import raises
from utils import init_app_with_options, get_num_queries, is_sqlalchemy

from flask_security import RoleMixin, Security, UserMixin
from flask_security.datastore import Datastore, UserDatastore
from flask_security.datastore import Datastore, UserDatastore, \
SQLAlchemyUserDatastore


class User(UserMixin):
Expand Down Expand Up @@ -108,6 +110,26 @@ def test_get_user(app, datastore):
assert user is not None


def test_get_user_raise(app, sqlalchemy_datastore):
"""
Some drivers will raise exceptions if we attempt to query a
numeric column with a string value
"""
from sqlalchemy.exc import DataError
init_app_with_options(app, sqlalchemy_datastore)

mock_query = MagicMock(name='querymock')
mock_query.options.return_value = mock_query
mock_query.filter.return_value.first.return_value = None
mock_query.get.side_effect = DataError('stmt', 'params',
SQLAlchemyUserDatastore)
with patch.object(sqlalchemy_datastore.user_model, 'query', mock_query):
with app.app_context():
user = sqlalchemy_datastore.get_user('1')
# this should be none (and not raise an exception)
assert user is None


def test_find_user(app, datastore):
init_app_with_options(app, datastore)

Expand Down

0 comments on commit c06cd13

Please sign in to comment.