manager: separate oidc service settings on another page (#68108)

src/authentic2/manager/forms.py

@@ -35,7 +35,7 @@ from authentic2.a2_rbac.utils import generate_slug, get_default_ou
 from authentic2.forms.fields import CheckPasswordField, NewPasswordField, ValidatedEmailField
 from authentic2.forms.mixins import SlugMixin
 from authentic2.forms.profile import BaseUserForm
-from authentic2.models import APIClient, PasswordReset
+from authentic2.models import APIClient, PasswordReset, Service
 from authentic2.passwords import generate_password
 from authentic2.utils.misc import (
     import_module_or_class,
@@ -921,3 +921,9 @@ class APIClientForm(forms.ModelForm):
             'apiclient_roles',
         )
         field_classes = {'apiclient_roles': ChooseRolesField}
+
+
+class ServiceForm(forms.ModelForm):
+    class Meta:
+        model = Service
+        fields = ['name', 'slug', 'ou', 'unauthorized_url']

src/authentic2/manager/service_views.py

@@ -95,20 +95,37 @@ class ServiceView(
         kwargs['form'] = self.get_form()
         ctx = super().get_context_data(**kwargs)
         ctx['roles_table'] = tables.RoleTable(self.object.roles.all())
-        ctx.update(self.object.get_manager_context_data())
         return ctx
 
 
 service_detail = ServiceView.as_view()
 
 
+class ServiceSettingsView(
+    ServiceMixin,
+    views.BaseDetailView,
+):
+    model = Service
+    pk_url_kwarg = 'service_pk'
+    template_name = 'authentic2/manager/service_settings.html'
+    permissions = ['authentic2.change_service']
+
+    def get_context_data(self, **kwargs):
+        self.service = self.object
+        ctx = super().get_context_data(**kwargs)
+        ctx.update(self.object.get_manager_context_data())
+        return ctx
+
+
+service_settings = ServiceSettingsView.as_view()
+
+
 class ServiceEditView(ServiceMixin, views.BaseEditView):
     model = Service
     pk_url_kwarg = 'service_pk'
     template_name = 'authentic2/manager/form.html'
     title = _('Edit service')
     permissions = ['authentic2.change_service']
-    fields = ['name', 'slug', 'ou', 'unauthorized_url']
     success_url = '..'
 
     def get_form_class(self):

src/authentic2/manager/static/authentic2/manager/css/style.scss

@@ -329,6 +329,15 @@ table.claims-table td.actions {
 	}
 }
 
+.service-field {
+	&--name {
+		font-weight: bold;
+	}
+	&--value {
+		margin-left: 1rem;
+	}
+}
+
 span.handle {
 	cursor: move;
 	display: inline-block;

src/authentic2/manager/templates/authentic2/manager/service.html

@@ -18,9 +18,7 @@
   {% if view.can_delete %}
   <a rel="popup" href="{% url "a2-manager-service-delete" service_pk=view.kwargs.service_pk %}">{% trans "Delete" %}</a>
   {% endif %}
-  {% if view.can_change %}
-  <a href="{% url "a2-manager-service-edit" service_pk=view.kwargs.service_pk %}">{% trans "Edit" %}</a>
-  {% endif %}
+  <a href="{% url "a2-manager-service-settings" service_pk=view.kwargs.service_pk %}">{% trans "Settings" %}</a>
   </span>
 {% endblock %}
 
@@ -42,10 +40,6 @@
 
 {% block main %}
 
-{% if extra_details_template %}
-  {% include extra_details_template %}
-{% endif %}
-
 <div class="section">
   <h3>{% trans "Roles of users allowed on this service" %}</h3>
   <div id="authorized-roles">

src/authentic2/manager/templates/authentic2/manager/service_settings.html

@@ -0,0 +1,41 @@
+{% extends "authentic2/manager/service.html" %}
+{% load i18n %}
+
+{% block breadcrumb %}
+{{ block.super }}
+  <a href="{% url 'a2-manager-service' service_pk=view.kwargs.service_pk %}">{{ object.name }}</a>
+{% endblock %}
+
+{% block buttons %}
+{% endblock %}
+
+{% block appbar %}
+  <h2>{% trans "Settings" %}</h2>
+  <span class="actions">
+  {% if view.can_change %}
+    <a href="{% url "a2-manager-service-settings-edit" service_pk=object.pk %}">{% trans "Edit" %}</a>
+  {% endif %}
+  </span>
+{% endblock %}
+
+
+{% block main %}
+<div class="section">
+  {% for field, value in object_fields_values %}
+  <div class="service-field">
+    <div class="service-field--name">{{ field|capfirst }}{% trans ":" %}</div>
+    <div class="service-field--value">{% if value == True %}{% trans "yes" %}
+      {% elif value == False %}{% trans "no" %}
+      {% else %}{{ value|linebreaksbr }}
+      {% endif %}</div>
+  </div>
+  {% endfor %}
+</div>
+
+{% if extra_details_template %}
+  {% include extra_details_template %}
+{% endif %}
+{% endblock %}
+
+{% block sidebar %}
+{% endblock %}

src/authentic2/manager/urls.py

@@ -169,9 +169,14 @@ urlpatterns = required(
         url(r'^services/$', service_views.listing, name='a2-manager-services'),
         url(r'^services/(?P<service_pk>\d+)/$', service_views.service_detail, name='a2-manager-service'),
         url(
-            r'^services/(?P<service_pk>\d+)/edit/$',
+            r'^services/(?P<service_pk>\d+)/settings/$',
+            service_views.service_settings,
+            name='a2-manager-service-settings',
+        ),
+        url(
+            r'^services/(?P<service_pk>\d+)/settings/edit/$',
             service_views.edit_service,
-            name='a2-manager-service-edit',
+            name='a2-manager-service-settings-edit',
         ),
         url(
             r'^services/(?P<service_pk>\d+)/delete/$',

src/authentic2/models.py

@@ -420,8 +420,6 @@ class Service(models.Model):
 
     objects = managers.ServiceManager()
 
-    manager_form_class = None
-
     def clean(self):
         errors = {}
 
@@ -505,8 +503,26 @@ class Service(models.Model):
 
         return super().delete(*args, **kwargs)
 
+    @property
+    def manager_form_class(self):
+        from .manager.forms import ServiceForm
+
+        return ServiceForm
+
+    def get_manager_fields(self):
+        return self.manager_form_class._meta.fields
+
+    def get_manager_fields_values(self):
+        for field_name in self.get_manager_fields():
+            field_value = getattr(self, field_name)
+            if not isinstance(field_value, bool) and not field_value:
+                continue
+            if hasattr(self, 'get_%s_display' % field_name):
+                field_value = getattr(self, 'get_%s_display' % field_name)()
+            yield self._meta.get_field(field_name).verbose_name, field_value
+
     def get_manager_context_data(self):
-        return {}
+        return {'object_fields_values': self.get_manager_fields_values()}
 
 
 Service._meta.natural_key = [['slug', 'ou']]

src/authentic2_idp_oidc/models.py

@@ -245,25 +245,12 @@ class OIDCClient(Service):
         return OIDCClientForm
 
     def get_manager_fields(self):
-        # add client id and secret
-        display_fields = ['client_id', 'client_secret'] + self.manager_form_class._meta.fields
-        # but remove name because it's already displayed
-        if 'name' in display_fields:
-            display_fields.remove('name')
-        for field in display_fields:
-            field_value = getattr(self, field)
-            if not isinstance(field_value, bool) and not field_value:
-                continue
-            if hasattr(self, 'get_%s_display' % field):
-                field_value = getattr(self, 'get_%s_display' % field)()
-            yield self._meta.get_field(field).verbose_name, field_value
+        return ['client_id', 'client_secret'] + super().get_manager_fields()
 
     def get_manager_context_data(self):
-        ctx = {
-            'claims': self.oidcclaim_set.all(),
-            'object_fields': self.get_manager_fields(),
-            'extra_details_template': 'authentic2_idp_oidc/manager/object_detail.html',
-        }
+        ctx = super().get_manager_context_data()
+        ctx['claims'] = self.oidcclaim_set.all()
+        ctx['extra_details_template'] = 'authentic2_idp_oidc/manager/object_detail.html'
         return ctx
 
 

src/authentic2_idp_oidc/templates/authentic2_idp_oidc/manager/object_detail.html

@@ -1,12 +1,5 @@
 {% load i18n %}
-{% for field, value in object_fields %}
-  <p>{{ field|capfirst }}{% trans ":" %}  {% if value == True %}{% trans "yes" %}
-  {% elif value == False %}{% trans "no" %}
-  {% else %}{{value}}
-  {% endif %}</p>
-{% endfor %}
-
-  <div class="section">
+<div class="section">
   <h3>{% trans "OIDC Claims" %}<a href="{% url "a2-manager-oidc-claim-add" service_pk=object.pk %}" class="button" rel="popup">{% trans "Add claim" %}</a></h3>
   {% if claims %}
   <table class="main claims-table" id="oidc-claims">

tests/test_manager.py

@@ -1252,19 +1252,16 @@ def test_manager_add_oidc_service(app, superuser):
     assert OIDCClient.objects.count() == 1
     assert OIDCClaim.objects.count() == len(oidc_app_settings.DEFAULT_MAPPINGS)
     assert resp.location == reverse('a2-manager-service', kwargs={'service_pk': OIDCClient.objects.get().pk})
-
     resp = resp.follow()
-    assert "<h3>OIDC Claims" in resp.text
-    assert "Add claim" in resp.text
-    assert resp.pyquery.remove_namespaces()('#oidc-claims tbody tr').length == len(
-        oidc_app_settings.DEFAULT_MAPPINGS
-    )
+    assert "Settings" in resp.text
+    assert "Delete" in resp.text
 
 
 def test_manager_edit_oidc_service(app, superuser):
     OIDCClient.objects.create(name='Test', slug='test', redirect_uris='http://example.com')
     resp = login(app, superuser, 'a2-manager-services')
     resp = resp.click('Test')
+    resp = resp.click('Settings')
     resp = resp.click('Edit')
     form = resp.form
     form['name'] = 'New Test'
@@ -1287,7 +1284,7 @@ def test_manager_delete_oidc_service(app, superuser):
 
 def test_manager_add_oidc_claim(app, superuser):
     client = OIDCClient.objects.create(name='Test', slug='test', redirect_uris='http://example.com')
-    resp = login(app, superuser, reverse('a2-manager-service', kwargs={'service_pk': client.pk}))
+    resp = login(app, superuser, reverse('a2-manager-service-settings', kwargs={'service_pk': client.pk}))
     resp = resp.click('Add claim')
     form = resp.form
     form['name'] = 'claim'
@@ -1301,7 +1298,7 @@ def test_manager_add_oidc_claim(app, superuser):
 def test_manager_edit_oidc_claim(app, superuser):
     client = OIDCClient.objects.create(name='Test', slug='test', redirect_uris='http://example.com')
     OIDCClaim.objects.create(client=client, name='claim', value='value', scopes='profile')
-    resp = login(app, superuser, reverse('a2-manager-service', kwargs={'service_pk': client.pk}))
+    resp = login(app, superuser, reverse('a2-manager-service-settings', kwargs={'service_pk': client.pk}))
     assert "claim" in resp.text
     resp = resp.click('Edit', index=1)
     form = resp.form
@@ -1315,9 +1312,9 @@ def test_manager_edit_oidc_claim(app, superuser):
 def test_manager_delete_oidc_claim(app, superuser):
     client = OIDCClient.objects.create(name='Test', slug='test', redirect_uris='http://example.com')
     OIDCClaim.objects.create(client=client, name='claim', value='value', scopes='profile')
-    resp = login(app, superuser, reverse('a2-manager-service', kwargs={'service_pk': client.pk}))
+    resp = login(app, superuser, reverse('a2-manager-service-settings', kwargs={'service_pk': client.pk}))
     assert "claim" in resp.text
-    resp = resp.click('Delete', index=1)
+    resp = resp.click('Delete')
     form = resp.form
     resp = form.submit()
     assert resp.location == reverse('a2-manager-service', kwargs={'service_pk': client.pk})