1
0
Fork 0
mirror of https://gitlab.com/bramw/baserow.git synced 2025-04-11 07:51:20 +00:00

Merge branch '265-refactor-the-get_-methods-of-the-handlers-so-that-they-do-not-check-for-permissions' into 'develop'

Resolve "Refactor the get_* methods of the handlers so that they do not check for permissions"

Closes 

See merge request 
This commit is contained in:
Bram Wiepjes 2021-02-09 15:27:08 +00:00
commit 166627d717
18 changed files with 91 additions and 111 deletions
backend
src/baserow
api
applications
groups/invitations
contrib/database
core
tests/baserow
changelog.md

View file

@ -219,7 +219,8 @@ class ApplicationView(APIView):
def get(self, request, application_id):
"""Selects a single application and responds with a serialized version."""
application = CoreHandler().get_application(request.user, application_id)
application = CoreHandler().get_application(application_id)
application.group.has_user(request.user, raise_error=True)
return Response(get_application_serializer(application).data)
@extend_schema(
@ -261,12 +262,12 @@ class ApplicationView(APIView):
"""Updates the application if the user belongs to the group."""
application = CoreHandler().get_application(
request.user, application_id,
application_id,
base_queryset=Application.objects.select_for_update()
)
application = CoreHandler().update_application(
request.user, application, name=data['name'])
request.user, application, name=data['name']
)
return Response(get_application_serializer(application).data)
@extend_schema(
@ -301,7 +302,7 @@ class ApplicationView(APIView):
"""Deletes an existing application if the user belongs to the group."""
application = CoreHandler().get_application(
request.user, application_id,
application_id,
base_queryset=Application.objects.select_for_update()
)
CoreHandler().delete_application(request.user, application)

View file

@ -168,10 +168,8 @@ class GroupInvitationView(APIView):
def get(self, request, group_invitation_id):
"""Selects a single group invitation and responds with a serialized version."""
group_invitation = CoreHandler().get_group_invitation(
request.user,
group_invitation_id
)
group_invitation = CoreHandler().get_group_invitation(group_invitation_id)
group_invitation.group.has_user(request.user, 'ADMIN', raise_error=True)
return Response(GroupInvitationSerializer(group_invitation).data)
@extend_schema(
@ -213,7 +211,6 @@ class GroupInvitationView(APIView):
"""Updates the group invitation if the user belongs to the group."""
group_invitation = CoreHandler().get_group_invitation(
request.user,
group_invitation_id,
base_queryset=GroupInvitation.objects.select_for_update()
)
@ -259,7 +256,6 @@ class GroupInvitationView(APIView):
"""Deletes an existing group_invitation if the user belongs to the group."""
group_invitation = CoreHandler().get_group_invitation(
request.user,
group_invitation_id,
base_queryset=GroupInvitation.objects.select_for_update()
)

View file

@ -74,7 +74,8 @@ class FieldsView(APIView):
has access to that group.
"""
table = TableHandler().get_table(request.user, table_id)
table = TableHandler().get_table(table_id)
table.database.group.has_user(request.user, raise_error=True)
fields = Field.objects.filter(table=table).select_related('content_type')
data = [
@ -128,7 +129,8 @@ class FieldsView(APIView):
type_name = data.pop('type')
field_type = field_type_registry.get(type_name)
table = TableHandler().get_table(request.user, table_id)
table = TableHandler().get_table(table_id)
table.database.group.has_user(request.user, raise_error=True)
# Because each field type can raise custom exceptions while creating the
# field we need to be able to map those to the correct API exceptions which are
@ -175,7 +177,8 @@ class FieldView(APIView):
def get(self, request, field_id):
"""Selects a single field and responds with a serialized version."""
field = FieldHandler().get_field(request.user, field_id)
field = FieldHandler().get_field(field_id)
field.table.database.group.has_user(request.user, raise_error=True)
serializer = field_type_registry.get_serializer(field, FieldSerializer)
return Response(serializer.data)
@ -226,7 +229,8 @@ class FieldView(APIView):
"""Updates the field if the user belongs to the group."""
field = FieldHandler().get_field(
request.user, field_id, base_queryset=Field.objects.select_for_update()
field_id,
base_queryset=Field.objects.select_for_update()
).specific
type_name = type_from_data_or_registry(request.data, field_type_registry, field)
field_type = field_type_registry.get(type_name)
@ -276,7 +280,7 @@ class FieldView(APIView):
def delete(self, request, field_id):
"""Deletes an existing field if the user belongs to the group."""
field = FieldHandler().get_field(request.user, field_id)
field = FieldHandler().get_field(field_id)
FieldHandler().delete_field(request.user, field)
return Response(status=204)

View file

@ -195,7 +195,9 @@ class RowsView(APIView):
provide a search query.
"""
table = TableHandler().get_table(request.user, table_id)
table = TableHandler().get_table(table_id)
table.database.group.has_user(request.user, raise_error=True)
TokenHandler().check_table_permissions(request, 'read', table, False)
search = request.GET.get('search')
order_by = request.GET.get('order_by')
@ -287,7 +289,7 @@ class RowsView(APIView):
according to the tables field types.
"""
table = TableHandler().get_table(request.user, table_id)
table = TableHandler().get_table(table_id)
TokenHandler().check_table_permissions(request, 'create', table, False)
model = table.get_model()
@ -361,7 +363,7 @@ class RowView(APIView):
and table_id.
"""
table = TableHandler().get_table(request.user, table_id)
table = TableHandler().get_table(table_id)
TokenHandler().check_table_permissions(request, 'read', table, False)
model = table.get_model()
@ -426,7 +428,7 @@ class RowView(APIView):
table_id. Also the post data is validated according to the tables field types.
"""
table = TableHandler().get_table(request.user, table_id)
table = TableHandler().get_table(table_id)
TokenHandler().check_table_permissions(request, 'update', table, False)
field_ids = RowHandler().extract_field_ids_from_dict(request.data)
@ -481,7 +483,7 @@ class RowView(APIView):
table_id.
"""
table = TableHandler().get_table(request.user, table_id)
table = TableHandler().get_table(table_id)
TokenHandler().check_table_permissions(request, 'delete', table, False)
RowHandler().delete_row(request.user, table, row_id)

View file

@ -64,9 +64,10 @@ class TablesView(APIView):
"""Lists all the tables of a database."""
database = CoreHandler().get_application(
request.user, database_id,
database_id,
base_queryset=Database.objects
)
database.group.has_user(request.user, raise_error=True)
tables = Table.objects.filter(database=database)
serializer = TableSerializer(tables, many=True)
return Response(serializer.data)
@ -111,7 +112,7 @@ class TablesView(APIView):
"""Creates a new table in a database."""
database = CoreHandler().get_application(
request.user, database_id,
database_id,
base_queryset=Database.objects
)
table = TableHandler().create_table(
@ -155,7 +156,8 @@ class TableView(APIView):
def get(self, request, table_id):
"""Responds with a serialized table instance."""
table = TableHandler().get_table(request.user, table_id)
table = TableHandler().get_table(table_id)
table.database.group.has_user(request.user, raise_error=True)
serializer = TableSerializer(table)
return Response(serializer.data)
@ -194,7 +196,7 @@ class TableView(APIView):
table = TableHandler().update_table(
request.user,
TableHandler().get_table(request.user, table_id),
TableHandler().get_table(table_id),
base_queryset=Table.objects.select_for_update(),
name=data['name']
)
@ -232,6 +234,6 @@ class TableView(APIView):
TableHandler().delete_table(
request.user,
TableHandler().get_table(request.user, table_id)
TableHandler().get_table(table_id)
)
return Response(status=204)

View file

@ -113,7 +113,8 @@ class GridViewView(APIView):
"""
view_handler = ViewHandler()
view = view_handler.get_view(request.user, view_id, GridView)
view = view_handler.get_view(view_id, GridView)
view.table.database.group.has_user(request.user, raise_error=True)
model = view.table.get_model()
queryset = model.objects.all().enhance_by_fields()
@ -192,7 +193,8 @@ class GridViewView(APIView):
requested fields.
"""
view = ViewHandler().get_view(request.user, view_id, GridView)
view = ViewHandler().get_view(view_id, GridView)
view.table.database.group.has_user(request.user, raise_error=True)
model = view.table.get_model(field_ids=data['field_ids'])
results = model.objects.filter(pk__in=data['row_ids'])
@ -251,7 +253,7 @@ class GridViewView(APIView):
"""
handler = ViewHandler()
view = handler.get_view(request.user, view_id, GridView)
view = handler.get_view(view_id, GridView)
handler.update_grid_view_field_options(
request.user,
view,

View file

@ -88,7 +88,8 @@ class ViewsView(APIView):
has access to that group.
"""
table = TableHandler().get_table(request.user, table_id)
table = TableHandler().get_table(table_id)
table.database.group.has_user(request.user, raise_error=True)
views = View.objects.filter(table=table).select_related('content_type')
if filters:
@ -152,7 +153,7 @@ class ViewsView(APIView):
def post(self, request, data, table_id, filters, sortings):
"""Creates a new view for a user."""
table = TableHandler().get_table(request.user, table_id)
table = TableHandler().get_table(table_id)
view = ViewHandler().create_view(
request.user, table, data.pop('type'), **data)
@ -201,7 +202,8 @@ class ViewView(APIView):
def get(self, request, view_id, filters, sortings):
"""Selects a single view and responds with a serialized version."""
view = ViewHandler().get_view(request.user, view_id)
view = ViewHandler().get_view(view_id)
view.table.database.group.has_user(request.user, raise_error=True)
serializer = view_type_registry.get_serializer(
view,
ViewSerializer,
@ -250,7 +252,7 @@ class ViewView(APIView):
def patch(self, request, view_id, filters, sortings):
"""Updates the view if the user belongs to the group."""
view = ViewHandler().get_view(request.user, view_id).specific
view = ViewHandler().get_view(view_id).specific
view_type = view_type_registry.get_by_model(view)
data = validate_data_custom_fields(
view_type.type, view_type_registry, request.data,
@ -298,7 +300,7 @@ class ViewView(APIView):
def delete(self, request, view_id):
"""Deletes an existing view if the user belongs to the group."""
view = ViewHandler().get_view(request.user, view_id)
view = ViewHandler().get_view(view_id)
ViewHandler().delete_view(request.user, view)
return Response(status=204)
@ -341,7 +343,8 @@ class ViewFiltersView(APIView):
has access to that group.
"""
view = ViewHandler().get_view(request.user, view_id)
view = ViewHandler().get_view(view_id)
view.table.database.group.has_user(request.user, raise_error=True)
filters = ViewFilter.objects.filter(view=view)
serializer = ViewFilterSerializer(filters, many=True)
return Response(serializer.data)
@ -391,7 +394,7 @@ class ViewFiltersView(APIView):
"""Creates a new filter for the provided view."""
view_handler = ViewHandler()
view = view_handler.get_view(request.user, view_id)
view = view_handler.get_view(view_id)
# We can safely assume the field exists because the CreateViewFilterSerializer
# has already checked that.
field = Field.objects.get(pk=data['field'])
@ -566,7 +569,8 @@ class ViewSortingsView(APIView):
has access to that group.
"""
view = ViewHandler().get_view(request.user, view_id)
view = ViewHandler().get_view(view_id)
view.table.database.group.has_user(request.user, raise_error=True)
sortings = ViewSort.objects.filter(view=view)
serializer = ViewSortSerializer(sortings, many=True)
return Response(serializer.data)
@ -616,7 +620,7 @@ class ViewSortingsView(APIView):
"""Creates a new sort for the provided view."""
view_handler = ViewHandler()
view = view_handler.get_view(request.user, view_id)
view = view_handler.get_view(view_id)
# We can safely assume the field exists because the CreateViewSortSerializer
# has already checked that.
field = Field.objects.get(pk=data['field'])

View file

@ -424,10 +424,9 @@ class LinkRowFieldType(FieldType):
if 'link_row_table' in values and isinstance(values['link_row_table'], int):
from baserow.contrib.database.table.handler import TableHandler
values['link_row_table'] = TableHandler().get_table(
user,
values['link_row_table']
)
table = TableHandler().get_table(values['link_row_table'])
table.database.group.has_user(user, raise_error=True)
values['link_row_table'] = table
return values

View file

@ -22,12 +22,10 @@ logger = logging.getLogger(__name__)
class FieldHandler:
def get_field(self, user, field_id, field_model=None, base_queryset=None):
def get_field(self, field_id, field_model=None, base_queryset=None):
"""
Selects a field with a given id from the database.
:param user: The user on whose behalf the field is requested.
:type user: User
:param field_id: The identifier of the field that must be returned.
:type field_id: int
:param field_model: If provided that model's objects are used to select the
@ -56,9 +54,6 @@ class FieldHandler:
except Field.DoesNotExist:
raise FieldDoesNotExist(f'The field with id {field_id} does not exist.')
group = field.table.database.group
group.has_user(user, raise_error=True)
return field
def create_field(self, user, table, type_name, primary=False,

View file

@ -18,12 +18,10 @@ from .signals import table_created, table_updated, table_deleted
class TableHandler:
def get_table(self, user, table_id, base_queryset=None):
def get_table(self, table_id, base_queryset=None):
"""
Selects a table with a given id from the database.
:param user: The user on whose behalf the table is requested.
:type user: User
:param table_id: The identifier of the table that must be returned.
:type table_id: int
:param base_queryset: The base queryset from where to select the table
@ -42,9 +40,6 @@ class TableHandler:
except Table.DoesNotExist:
raise TableDoesNotExist(f'The table with id {table_id} doe not exist.')
group = table.database.group
group.has_user(user, raise_error=True)
return table
def create_table(self, user, database, fill_example=False, data=None,

View file

@ -22,13 +22,11 @@ from .signals import (
class ViewHandler:
def get_view(self, user, view_id, view_model=None, base_queryset=None):
def get_view(self, view_id, view_model=None, base_queryset=None):
"""
Selects a view and checks if the user has access to that view. If everything
is fine the view is returned.
:param user: The user on whose behalf the view is requested.
:type user: User
:param view_id: The identifier of the view that must be returned.
:type view_id: int
:param view_model: If provided that models objects are used to select the
@ -57,9 +55,6 @@ class ViewHandler:
except View.DoesNotExist:
raise ViewDoesNotExist(f'The view with id {view_id} does not exist.')
group = view.table.database.group
group.has_user(user, raise_error=True)
return view
def create_view(self, user, table, type_name, **kwargs):
@ -176,6 +171,8 @@ class ViewHandler:
provided view.
"""
grid_view.table.database.group.has_user(user, raise_error=True)
if not fields:
fields = Field.objects.filter(table=grid_view.table)

View file

@ -20,7 +20,8 @@ class TablePageType(PageType):
try:
handler = TableHandler()
handler.get_table(user, table_id)
table = handler.get_table(table_id)
table.database.group.has_user(user, raise_error=True)
except (UserNotInGroupError, TableDoesNotExist):
return False

View file

@ -298,7 +298,7 @@ class CoreHandler:
return group_invitation
def get_group_invitation(self, user, group_invitation_id, base_queryset=None):
def get_group_invitation(self, group_invitation_id, base_queryset=None):
"""
Selects a group invitation with a given id from the database.
@ -325,8 +325,6 @@ class CoreHandler:
f'The group invitation with id {group_invitation_id} does not exist.'
)
group_invitation.group.has_user(user, 'ADMIN', raise_error=True)
return group_invitation
def create_group_invitation(self, user, group, email, permissions, message,
@ -486,7 +484,7 @@ class CoreHandler:
return group_user
def get_application(self, user, application_id, base_queryset=None):
def get_application(self, application_id, base_queryset=None):
"""
Selects an application with a given id from the database.
@ -515,8 +513,6 @@ class CoreHandler:
f'The application with id {application_id} does not exist.'
)
application.group.has_user(user, raise_error=True)
return application
def create_application(self, user, group, type_name, **kwargs):

View file

@ -18,24 +18,21 @@ from baserow.contrib.database.fields.exceptions import (
@pytest.mark.django_db
def test_get_field(data_fixture):
user = data_fixture.create_user()
user_2 = data_fixture.create_user()
data_fixture.create_user()
text = data_fixture.create_text_field(user=user)
handler = FieldHandler()
with pytest.raises(FieldDoesNotExist):
handler.get_field(user=user, field_id=99999)
handler.get_field(field_id=99999)
with pytest.raises(UserNotInGroupError):
handler.get_field(user=user_2, field_id=text.id)
field = handler.get_field(user=user, field_id=text.id)
field = handler.get_field(field_id=text.id)
assert text.id == field.id
assert text.name == field.name
assert isinstance(field, Field)
field = handler.get_field(user=user, field_id=text.id, field_model=TextField)
field = handler.get_field(field_id=text.id, field_model=TextField)
assert text.id == field.id
assert text.name == field.name
@ -44,7 +41,7 @@ def test_get_field(data_fixture):
# If the error is raised we know for sure that the query has resolved.
with pytest.raises(AttributeError):
handler.get_field(
user=user, field_id=text.id,
field_id=text.id,
base_queryset=Field.objects.prefetch_related('UNKNOWN')
)

View file

@ -21,24 +21,20 @@ from baserow.contrib.database.views.models import GridView, GridViewFieldOptions
def test_get_database_table(data_fixture):
user = data_fixture.create_user()
table = data_fixture.create_database_table(user=user)
table_2 = data_fixture.create_database_table()
data_fixture.create_database_table()
handler = TableHandler()
with pytest.raises(UserNotInGroupError):
handler.get_table(user=user, table_id=table_2.id)
with pytest.raises(TableDoesNotExist):
handler.get_table(user=user, table_id=99999)
handler.get_table(table_id=99999)
# If the error is raised we know for sure that the base query has resolved.
with pytest.raises(AttributeError):
handler.get_table(
user=user,
table_id=table.id,
base_queryset=Table.objects.prefetch_related('UNKNOWN')
)
table_copy = handler.get_table(user=user, table_id=table.id)
table_copy = handler.get_table(table_id=table.id)
assert table_copy.id == table.id

View file

@ -22,18 +22,15 @@ from baserow.contrib.database.fields.exceptions import FieldNotInTable
@pytest.mark.django_db
def test_get_view(data_fixture):
user = data_fixture.create_user()
user_2 = data_fixture.create_user()
data_fixture.create_user()
grid = data_fixture.create_grid_view(user=user)
handler = ViewHandler()
with pytest.raises(ViewDoesNotExist):
handler.get_view(user=user, view_id=99999)
handler.get_view(view_id=99999)
with pytest.raises(UserNotInGroupError):
handler.get_view(user=user_2, view_id=grid.id)
view = handler.get_view(user=user, view_id=grid.id)
view = handler.get_view(view_id=grid.id)
assert view.id == grid.id
assert view.name == grid.name
@ -41,7 +38,7 @@ def test_get_view(data_fixture):
assert not view.filters_disabled
assert isinstance(view, View)
view = handler.get_view(user=user, view_id=grid.id, view_model=GridView)
view = handler.get_view(view_id=grid.id, view_model=GridView)
assert view.id == grid.id
assert view.name == grid.name
@ -52,7 +49,7 @@ def test_get_view(data_fixture):
# If the error is raised we know for sure that the query has resolved.
with pytest.raises(AttributeError):
handler.get_view(
user=user, view_id=grid.id,
view_id=grid.id,
base_queryset=View.objects.prefetch_related('UNKNOWN')
)
@ -196,6 +193,15 @@ def test_update_grid_view_field_options(send_mock, data_fixture):
}
)
with pytest.raises(UserNotInGroupError):
ViewHandler().update_grid_view_field_options(
user=data_fixture.create_user(),
grid_view=grid_view,
field_options={
'strange_format': {'height': 150},
}
)
with pytest.raises(UnrelatedFieldError):
ViewHandler().update_grid_view_field_options(
user=user,

View file

@ -282,7 +282,7 @@ def test_get_group_invitation_by_token(data_fixture):
def test_get_group_invitation(data_fixture):
user = data_fixture.create_user()
user_2 = data_fixture.create_user()
user_3 = data_fixture.create_user()
data_fixture.create_user()
group_user = data_fixture.create_user_group(user=user)
data_fixture.create_user_group(
user=user_2,
@ -297,18 +297,9 @@ def test_get_group_invitation(data_fixture):
handler = CoreHandler()
with pytest.raises(GroupInvitationDoesNotExist):
handler.get_group_invitation(user=user, group_invitation_id=999999)
handler.get_group_invitation(group_invitation_id=999999)
with pytest.raises(UserNotInGroupError):
handler.get_group_invitation(user=user_3, group_invitation_id=invitation.id)
with pytest.raises(UserInvalidGroupPermissionsError):
handler.get_group_invitation(user=user_2, group_invitation_id=invitation.id)
invitation2 = handler.get_group_invitation(
user=user,
group_invitation_id=invitation.id
)
invitation2 = handler.get_group_invitation(group_invitation_id=invitation.id)
assert invitation.id == invitation2.id
assert invitation.invited_by_id == invitation2.invited_by_id
@ -596,25 +587,20 @@ def test_accept_group_invitation(data_fixture):
@pytest.mark.django_db
def test_get_application(data_fixture):
user_1 = data_fixture.create_user()
user_2 = data_fixture.create_user()
data_fixture.create_user()
application_1 = data_fixture.create_database_application(user=user_1)
handler = CoreHandler()
with pytest.raises(ApplicationDoesNotExist):
handler.get_application(user=user_1, application_id=0)
handler.get_application(application_id=0)
with pytest.raises(UserNotInGroupError):
handler.get_application(user=user_2, application_id=application_1.id)
application_1_copy = handler.get_application(
user=user_1, application_id=application_1.id
)
application_1_copy = handler.get_application(application_id=application_1.id)
assert application_1_copy.id == application_1.id
assert isinstance(application_1_copy, Application)
database_1_copy = handler.get_application(
user=user_1, application_id=application_1.id, base_queryset=Database.objects
application_id=application_1.id, base_queryset=Database.objects
)
assert database_1_copy.id == application_1.id
assert isinstance(database_1_copy, Database)

View file

@ -15,6 +15,7 @@
none; property.
* Fail hard when the web-frontend can't reach the backend because of a network error.
* Use UTC time in the date picker.
* Refactored handler get_* methods so that they never check for permissions.
## Released (2021-02-04)