From bc49392140e193daeba72c6539402f6614ccf5db Mon Sep 17 00:00:00 2001 From: Lance Edgar Date: Wed, 14 Aug 2024 16:58:08 -0500 Subject: [PATCH] fix: prevent delete for built-in roles --- src/wuttaweb/forms/base.py | 1 - src/wuttaweb/grids/base.py | 8 ++- src/wuttaweb/templates/base.mako | 20 ++++--- .../templates/grids/vue_template.mako | 3 +- src/wuttaweb/views/master.py | 54 +++++++++++-------- src/wuttaweb/views/roles.py | 15 ++++++ tests/views/test_master.py | 42 ++++++++------- tests/views/test_roles.py | 16 ++++++ 8 files changed, 106 insertions(+), 53 deletions(-) diff --git a/src/wuttaweb/forms/base.py b/src/wuttaweb/forms/base.py index be76882..80eec2d 100644 --- a/src/wuttaweb/forms/base.py +++ b/src/wuttaweb/forms/base.py @@ -677,7 +677,6 @@ class Form: # fill in the blanks if anything got missed for key in fields: if key not in schema: - log.warning("key '%s' not in schema", key) node = colander.SchemaNode(colander.String(), name=key) schema.add(node) diff --git a/src/wuttaweb/grids/base.py b/src/wuttaweb/grids/base.py index 5d3f94b..add7517 100644 --- a/src/wuttaweb/grids/base.py +++ b/src/wuttaweb/grids/base.py @@ -421,6 +421,8 @@ class Grid: for i, record in enumerate(original_data): original_record = record + record = dict(record) + # convert data if needed, for json compat record = make_json_safe(record, # TODO: is this a good idea? @@ -433,9 +435,11 @@ class Grid: # add action urls to each record for action in self.actions: - url = action.get_url(record, i) key = f'_action_url_{action.key}' - record[key] = url + if key not in record: + url = action.get_url(original_record, i) + if url: + record[key] = url data.append(record) diff --git a/src/wuttaweb/templates/base.mako b/src/wuttaweb/templates/base.mako index ef5a295..eab8bf0 100644 --- a/src/wuttaweb/templates/base.mako +++ b/src/wuttaweb/templates/base.mako @@ -378,19 +378,23 @@ tag="a" href="${master.get_action_url('edit', instance)}" icon-left="edit" label="Edit This" /> - + % if instance_deletable: + + % endif % elif master.editing: - + % if instance_deletable: + + % endif % elif master.deleting: % for action in grid.actions: - ${action.render_icon_and_label()} diff --git a/src/wuttaweb/views/master.py b/src/wuttaweb/views/master.py index 4009901..f66031d 100644 --- a/src/wuttaweb/views/master.py +++ b/src/wuttaweb/views/master.py @@ -204,6 +204,8 @@ class MasterView(View): i.e. it should have a :meth:`delete()` view. Default value is ``True``. + See also :meth:`is_deletable()`. + .. attribute:: form_fields List of columns for the model form. @@ -458,6 +460,9 @@ class MasterView(View): instance = self.get_instance() instance_title = self.get_instance_title(instance) + if not self.is_deletable(instance): + return self.redirect(self.get_action_url('view', instance)) + # nb. this form proper is not readonly.. form = self.make_model_form(instance, cancel_url_fallback=self.get_action_url('view', instance), @@ -839,6 +844,10 @@ class MasterView(View): defaults.update(context) context = defaults + # add crud flags if we have an instance + if 'instance' in context: + context['instance_deletable'] = self.is_deletable(context['instance']) + # first try the template path most specific to this view template_prefix = self.get_template_prefix() mako_path = f'{template_prefix}/{template}.mako' @@ -986,26 +995,7 @@ class MasterView(View): """ query = self.get_query(session=session) if query: - data = query.all() - - # determine which columns are relevant for data set - if not columns: - columns = self.get_grid_columns() - if not columns: - model_class = self.get_model_class() - if model_class: - columns = get_model_fields(self.config, model_class) - if not columns: - raise ValueError("cannot determine columns for the grid") - columns = set(columns) - columns.update(self.get_model_key()) - - # prune data fields for which no column is defined - for i, record in enumerate(data): - data[i]= dict([(key, record[key]) - for key in columns]) - - return data + return query.all() return [] @@ -1158,14 +1148,32 @@ class MasterView(View): def get_action_url_delete(self, obj, i): """ - Returns the "delete" grid action URL for the given object. + Returns the "delete" grid action URL for the given object, if + applicable. Most typically this is like ``/widgets/XXX/delete`` where ``XXX`` represents the object's key/ID. - Calls :meth:`get_action_url()` under the hood. + This first calls :meth:`is_deletable()` and if that is false, + this method will return ``None``. + + Calls :meth:`get_action_url()` to generate the true URL. """ - return self.get_action_url('delete', obj) + if self.is_deletable(obj): + return self.get_action_url('delete', obj) + + def is_deletable(self, obj): + """ + Returns a boolean indicating whether "delete" should be + allowed for the given model instance (and for current user). + + By default this always return ``True``; subclass can override + if needed. + + Note that the use of this method implies :attr:`deletable` is + true, so the method does not need to check that flag. + """ + return True def make_model_form(self, model_instance=None, **kwargs): """ diff --git a/src/wuttaweb/views/roles.py b/src/wuttaweb/views/roles.py index f667775..327f4fa 100644 --- a/src/wuttaweb/views/roles.py +++ b/src/wuttaweb/views/roles.py @@ -69,6 +69,21 @@ class RoleView(MasterView): # notes g.set_renderer('notes', self.grid_render_notes) + def is_deletable(self, role): + """ """ + session = self.app.get_session(role) + auth = self.app.get_auth_handler() + + # prevent delete for built-in roles + if role is auth.get_role_administrator(session): + return False + if role is auth.get_role_authenticated(session): + return False + if role is auth.get_role_anonymous(session): + return False + + return True + def configure_form(self, f): """ """ super().configure_form(f) diff --git a/tests/views/test_master.py b/tests/views/test_master.py index 253e59d..34b58ef 100644 --- a/tests/views/test_master.py +++ b/tests/views/test_master.py @@ -22,9 +22,8 @@ class TestMasterView(WebTestCase): def test_defaults(self): with patch.multiple(master.MasterView, create=True, model_name='Widget', - viewable=False, - editable=False, - deletable=False): + model_key='uuid', + configurable=True): master.MasterView.defaults(self.pyramid_config) ############################## @@ -392,6 +391,13 @@ class TestMasterView(WebTestCase): model = self.app.model self.app.save_setting(self.session, 'foo', 'bar') self.session.commit() + setting = self.session.query(model.Setting).one() + view = self.make_view() + + # empty by default + self.assertFalse(hasattr(master.MasterView, 'model_class')) + data = view.get_grid_data(session=self.session) + self.assertEqual(data, []) # basic logic with Setting model with patch.multiple(master.MasterView, create=True, @@ -399,16 +405,7 @@ class TestMasterView(WebTestCase): view = master.MasterView(self.request) data = view.get_grid_data(session=self.session) self.assertEqual(len(data), 1) - self.assertEqual(data[0], {'name': 'foo', 'value': 'bar'}) - - # error if model not known - view = master.MasterView(self.request) - self.assertFalse(hasattr(master.MasterView, 'model_class')) - def get_query(session=None): - session = session or self.session - return session.query(model.Setting) - with patch.object(view, 'get_query', new=get_query): - self.assertRaises(ValueError, view.get_grid_data, session=self.session) + self.assertIs(data[0], setting) def test_configure_grid(self): model = self.app.model @@ -754,15 +751,24 @@ class TestMasterView(WebTestCase): def delete_instance(setting): self.app.delete_setting(self.session, setting['name']) - # post request to save settings self.request.method = 'POST' self.request.POST = {} with patch.object(view, 'delete_instance', new=delete_instance): + + # enforces "instance not deletable" rules + with patch.object(view, 'is_deletable', return_value=False): + response = view.delete() + # nb. should get redirect back to view page + self.assertEqual(response.status_code, 302) + # setting remains in DB + self.assertEqual(self.session.query(model.Setting).count(), 1) + + # post request to delete setting response = view.delete() - # nb. should get redirect back to view page - self.assertEqual(response.status_code, 302) - # setting should be gone from DB - self.assertEqual(self.session.query(model.Setting).count(), 0) + # nb. should get redirect back to view page + self.assertEqual(response.status_code, 302) + # setting should be gone from DB + self.assertEqual(self.session.query(model.Setting).count(), 0) def test_delete_instance(self): model = self.app.model diff --git a/tests/views/test_roles.py b/tests/views/test_roles.py index 4cfc977..2d95819 100644 --- a/tests/views/test_roles.py +++ b/tests/views/test_roles.py @@ -31,6 +31,22 @@ class TestRoleView(WebTestCase): view.configure_grid(grid) self.assertTrue(grid.is_linked('name')) + def test_is_deletable(self): + model = self.app.model + auth = self.app.get_auth_handler() + blokes = model.Role(name="Blokes") + self.session.add(blokes) + self.session.commit() + view = self.make_view() + + # deletable by default + self.assertTrue(view.is_deletable(blokes)) + + # built-in roles not deletable + self.assertFalse(view.is_deletable(auth.get_role_administrator(self.session))) + self.assertFalse(view.is_deletable(auth.get_role_authenticated(self.session))) + self.assertFalse(view.is_deletable(auth.get_role_anonymous(self.session))) + def test_configure_form(self): model = self.app.model role = model.Role(name="Foo")