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

Fix peewee #352

Merged
merged 1 commit into from
Jun 25, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions CHANGES.rst
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,12 @@ Features
primary key.
- (:pr:`xxx`) Change ``USER_IDENTITY_ATTRIBUTES`` configuration variable semantics.

Fixed
+++++
- (:issue:`347`) Fix peewee. Turns out - due to lack of unit tests - peewee hasn't worked since
'permissions' were added in 3.3. Furthermore, changes in 3.4 around get_id and alternative tokens also
didn't work since peewee defines its own `get_id`.

Backwards Compatibility Concerns
+++++++++++++++++++++++++++++++++
- (:pr:`328`) Remove dependence on Flask-Mail and refactor. The ``send_mail_task`` and
Expand Down
11 changes: 9 additions & 2 deletions docs/quickstart.rst
Original file line number Diff line number Diff line change
Expand Up @@ -272,6 +272,7 @@ possible using MongoEngine:
class Role(db.Document, RoleMixin):
name = db.StringField(max_length=80, unique=True)
description = db.StringField(max_length=255)
permissions = db.StringField(max_length=255)

class User(db.Document, UserMixin):
email = db.StringField(max_length=255)
Expand Down Expand Up @@ -347,11 +348,14 @@ possible using Peewee:
# Create database connection object
db = FlaskDB(app)

class Role(db.Model, RoleMixin):
class Role(RoleMixin, db.Model):
name = CharField(unique=True)
description = TextField(null=True)
permissions = TextField(null=True)

class User(db.Model, UserMixin):
# N.B. order is important since db.Model also contains a get_id() -
# we need the one from UserMixin.
class User(UserMixin, db.Model):
email = TextField()
password = TextField()
active = BooleanField(default=True)
Expand All @@ -367,6 +371,9 @@ possible using Peewee:
name = property(lambda self: self.role.name)
description = property(lambda self: self.role.description)

def get_permissions(self):
return self.role.get_permissions()

# Setup Flask-Security
user_datastore = PeeweeUserDatastore(db, User, Role, UserRoles)
security = Security(app, user_datastore)
Expand Down
12 changes: 4 additions & 8 deletions flask_security/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -155,12 +155,10 @@ def roles_add(user, role):
if user_obj is None:
raise click.UsageError("ERROR: User not found.")

user, role = _datastore._prepare_role_modify_args(user_obj, role)
if user is None:
raise click.UsageError("Cannot find user.")
role = _datastore._prepare_role_modify_args(role)
if role is None:
raise click.UsageError("Cannot find role.")
if _datastore.add_role_to_user(user, role):
if _datastore.add_role_to_user(user_obj, role):
click.secho(
f'Role "{role}" added to user "{user}" successfully.', fg="green",
)
Expand All @@ -179,12 +177,10 @@ def roles_remove(user, role):
if user_obj is None:
raise click.UsageError("ERROR: User not found.")

user, role = _datastore._prepare_role_modify_args(user_obj, role)
if user is None:
raise click.UsageError("Cannot find user.")
role = _datastore._prepare_role_modify_args(role)
if role is None:
raise click.UsageError("Cannot find role.")
if _datastore.remove_role_from_user(user, role):
if _datastore.remove_role_from_user(user_obj, role):
click.secho(
f'Role "{role}" removed from user "{user}" successfully.', fg="green",
)
Expand Down
24 changes: 12 additions & 12 deletions flask_security/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -614,9 +614,8 @@ def get_permissions(self):
"""
Return set of permissions associated with role.

Either takes a comma separated string of permissions or
an interable of strings if permissions are in their own
table.
Supports permissions being a comma separated string, an iterable, or a set
based on how the underlying DB model was built.

.. versionadded:: 3.3.0
"""
Expand All @@ -636,9 +635,10 @@ def add_permissions(self, permissions):

:param permissions: a set, list, or single string.

Caller must commit to DB.

.. versionadded:: 3.3.0

.. deprecated:: 4.0.0
Use :meth:`.UserDatastore.add_permissions_to_role`
"""
if hasattr(self, "permissions"):
current_perms = self.get_permissions()
Expand All @@ -649,7 +649,7 @@ def add_permissions(self, permissions):
else:
perms = {permissions}
self.permissions = ",".join(current_perms.union(perms))
else:
else: # pragma: no cover
raise NotImplementedError("Role model doesn't have permissions")

def remove_permissions(self, permissions):
Expand All @@ -658,9 +658,10 @@ def remove_permissions(self, permissions):

:param permissions: a set, list, or single string.

Caller must commit to DB.

.. versionadded:: 3.3.0

.. deprecated:: 4.0.0
Use :meth:`.UserDatastore.remove_permissions_from_role`
"""
if hasattr(self, "permissions"):
current_perms = self.get_permissions()
Expand All @@ -671,7 +672,7 @@ def remove_permissions(self, permissions):
else:
perms = {permissions}
self.permissions = ",".join(current_perms.difference(perms))
else:
else: # pragma: no cover
raise NotImplementedError("Role model doesn't have permissions")


Expand Down Expand Up @@ -730,9 +731,8 @@ def has_permission(self, permission):

"""
for role in self.roles:
if hasattr(role, "permissions"):
if permission in role.get_permissions():
return True
if permission in role.get_permissions():
return True
return False

def get_security_payload(self):
Expand Down
75 changes: 62 additions & 13 deletions flask_security/datastore.py
Original file line number Diff line number Diff line change
Expand Up @@ -131,12 +131,10 @@ def __init__(self, user_model, role_model):
self.user_model = user_model
self.role_model = role_model

def _prepare_role_modify_args(self, user, role):
if isinstance(user, str):
user = self.find_user(email=user)
def _prepare_role_modify_args(self, role):
if isinstance(role, str):
role = self.find_role(role)
return user, role
return role

def _prepare_create_user_args(self, **kwargs):
kwargs.setdefault("active", True)
Expand Down Expand Up @@ -164,7 +162,7 @@ def add_role_to_user(self, user, role):
:param role: The role to add to the user. Can be a Role object or
string role name
"""
user, role = self._prepare_role_modify_args(user, role)
role = self._prepare_role_modify_args(role)
if role not in user.roles:
user.roles.append(role)
self.put(user)
Expand All @@ -179,13 +177,47 @@ def remove_role_from_user(self, user, role):
string role name
"""
rv = False
user, role = self._prepare_role_modify_args(user, role)
role = self._prepare_role_modify_args(role)
if role in user.roles:
rv = True
user.roles.remove(role)
self.put(user)
return rv

def add_permissions_to_role(self, role, permissions):
"""Add one or more permissions to role.

:param role: The role to modify. Can be a Role object or
string role name
:param permissions: a set, list, or single string.

Caller must commit to DB.

.. versionadded:: 4.0.0
"""

role = self._prepare_role_modify_args(role)
role.add_permissions(permissions)
self.put(role)
return role.get_permissions()

def remove_permissions_from_role(self, role, permissions):
"""Remove one or more permissions from a role.

:param role: The role to modify. Can be a Role object or
string role name
:param permissions: a set, list, or single string.

Caller must commit to DB.

.. versionadded:: 4.0.0
"""

role = self._prepare_role_modify_args(role)
role.remove_permissions(permissions)
self.put(role)
return role.get_permissions()

def toggle_active(self, user):
"""Toggles a user's active status. Always returns True."""
user.active = not user.active
Expand Down Expand Up @@ -531,12 +563,22 @@ def __init__(self, db, user_model, role_model, role_link):
self.UserRole = role_link

def find_user(self, case_insensitive=False, **kwargs):
# from peewee import fn as peeweeFn
# peeweeFn.Lower(column) == peeweeFn.Lower(identifier)
from peewee import fn as peeweeFn

# TODO - implement case_insensitive
try:
return self.user_model.filter(**kwargs).get()
if case_insensitive:
# While it is of course possible to pass in multiple keys to filter on
# that isn't the normal use case. If caller asks for case_insensitive
# AND gives multiple keys - throw an error.
if len(kwargs) > 1:
raise ValueError("Case insensitive option only supports single key")
attr, identifier = kwargs.popitem()
return self.user_model.get(
peeweeFn.lower(getattr(self.user_model, attr))
== peeweeFn.lower(identifier)
)
else:
return self.user_model.filter(**kwargs).get()
except self.user_model.DoesNotExist:
return None

Expand All @@ -562,7 +604,7 @@ def add_role_to_user(self, user, role):
:param user: The user to manipulate
:param role: The role to add to the user
"""
user, role = self._prepare_role_modify_args(user, role)
role = self._prepare_role_modify_args(role)
result = self.UserRole.select().where(
self.UserRole.user == user.id, self.UserRole.role == role.id
)
Expand All @@ -578,7 +620,7 @@ def remove_role_from_user(self, user, role):
:param user: The user to manipulate
:param role: The role to remove from the user
"""
user, role = self._prepare_role_modify_args(user, role)
role = self._prepare_role_modify_args(role)
result = self.UserRole.select().where(
self.UserRole.user == user, self.UserRole.role == role
)
Expand All @@ -605,7 +647,14 @@ def __init__(self, db, user_model, role_model):

@with_pony_session
def find_user(self, case_insensitive=False, **kwargs):
# TODO - implement case insensitive look ups.
if case_insensitive:
# While it is of course possible to pass in multiple keys to filter on
# that isn't the normal use case. If caller asks for case_insensitive
# AND gives multiple keys - throw an error.
if len(kwargs) > 1:
raise ValueError("Case insensitive option only supports single key")
# TODO - implement case insensitive look ups.

return self.user_model.get(**kwargs)

@with_pony_session
Expand Down
18 changes: 14 additions & 4 deletions tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -241,6 +241,7 @@ def mongoengine_setup(request, app, tmpdir, realdburl):
class Role(db.Document, RoleMixin):
name = StringField(required=True, unique=True, max_length=80)
description = StringField(max_length=255)
permissions = StringField(max_length=255)
meta = {"db_alias": db_name}

class User(db.Document, UserMixin):
Expand Down Expand Up @@ -459,11 +460,12 @@ def peewee_setup(request, app, tmpdir, realdburl):

db = FlaskDB(app)

class Role(db.Model, RoleMixin):
class Role(RoleMixin, db.Model):
name = CharField(unique=True, max_length=80)
description = TextField(null=True)
permissions = TextField(null=True)

class User(db.Model, UserMixin):
class User(UserMixin, db.Model):
email = TextField(unique=True, null=False)
fs_uniquifier = TextField(unique=True, null=False)
username = TextField(unique=True, null=True)
Expand Down Expand Up @@ -491,6 +493,9 @@ class UserRoles(db.Model):
name = property(lambda self: self.role.name)
description = property(lambda self: self.role.description)

def get_permissions(self):
return self.role.get_permissions()

with app.app_context():
for Model in (Role, User, UserRoles):
Model.drop_table()
Expand Down Expand Up @@ -637,7 +642,7 @@ def client_nc(request, sqlalchemy_app):
return app.test_client(use_cookies=False)


@pytest.fixture(params=["cl-sqlalchemy", "c2", "cl-mongo"])
@pytest.fixture(params=["cl-sqlalchemy", "c2", "cl-mongo", "cl-peewee"])
def clients(request, app, tmpdir, realdburl):
if request.param == "cl-sqlalchemy":
ds = sqlalchemy_setup(request, app, tmpdir, realdburl)
Expand All @@ -646,10 +651,15 @@ def clients(request, app, tmpdir, realdburl):
elif request.param == "cl-mongo":
ds = mongoengine_setup(request, app, tmpdir, realdburl)
elif request.param == "cl-peewee":
# TODO - this doesn't work yet (multiple connects).
ds = peewee_setup(request, app, tmpdir, realdburl)
elif request.param == "cl-pony":
# Not working yet.
ds = pony_setup(request, app, tmpdir, realdburl)
app.security = Security(app, datastore=ds)
populate_data(app)
if request.param == "cl-peewee":
# peewee is insistent on a single connection?
ds.db.close_db(None)
return app.test_client()


Expand Down
Loading