fix: prevent delete for built-in roles

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)
 

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,8 +435,10 @@ 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}'
+                if key not in record:
+                    url = action.get_url(original_record, i)
+                    if url:
                         record[key] = url
 
             data.append(record)

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:
              <wutta-button once type="is-danger"
                            tag="a" href="${master.get_action_url('delete', instance)}"
                            icon-left="trash"
                            label="Delete This" />
+         % endif
      % elif master.editing:
          <wutta-button once
                        tag="a" href="${master.get_action_url('view', instance)}"
                        icon-left="eye"
                        label="View This" />
+         % if instance_deletable:
              <wutta-button once type="is-danger"
                            tag="a" href="${master.get_action_url('delete', instance)}"
                            icon-left="trash"
                            label="Delete This" />
+         % endif
      % elif master.deleting:
          <wutta-button once
                        tag="a" href="${master.get_action_url('view', instance)}"

src/wuttaweb/templates/grids/vue_template.mako

@@ -24,7 +24,8 @@
                            label="Actions"
                            v-slot="props">
           % for action in grid.actions:
-              <a :href="props.row._action_url_${action.key}"
+              <a v-if="props.row._action_url_${action.key}"
+                 :href="props.row._action_url_${action.key}"
                  class="${action.link_class}">
                 ${action.render_icon_and_label()}
               </a>

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()
+            return 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 []
 
@@ -1158,15 +1148,33 @@ 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.
         """
+        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):
         """
         Create and return a :class:`~wuttaweb.forms.base.Form`

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)

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,
+                            model_key='uuid',
-                            editable=False,
+                            configurable=True):
-                            deletable=False):
             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'})
+            self.assertIs(data[0], setting)
-
-        # 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)
 
     def test_configure_grid(self):
         model = self.app.model
@@ -754,10 +751,19 @@ 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)

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")