Clean up the generated form for application registration and update views

docs/getting_started.rst

@@ -245,9 +245,9 @@ Start the development server::
 
 Point your browser to http://127.0.0.1:8000/o/applications/register/ lets create an application.
 
-Fill the form as show in the screenshot below and before save take note of ``Client id`` and ``Client secret``, we will use it in a minute.
+Fill the form as show in the screenshot below and after saving take note of the ``client secret`` (possibly shown in the flash message) and the ``client ID``, we will use them both in a minute.
 
-If you want to use this application with OIDC and ``HS256`` (see :doc:`OpenID Connect <oidc>`), uncheck ``Hash client secret`` to allow verifying tokens using JWT signatures. This means your client secret will be stored in cleartext but is the only way to successfully use signed JWT's with ``HS256``.
+If you want to use this application with OIDC and ``HS256`` (see :doc:`OpenID Connect <oidc>`), uncheck ``Hash client secret`` to allow verifying tokens using JWT signatures. Unchecking that means your client secret will be stored on the server in cleartext but is the only way to successfully use signed JWT's with ``HS256``.
 
 .. note::
     ``RS256`` is the more secure algorithm for signing your JWTs. Only use ``HS256`` if you must.

docs/install.rst

@@ -5,7 +5,7 @@ Install with pip::
 
     pip install django-oauth-toolkit
 
-Add ``oauth2_provider`` to your ``INSTALLED_APPS``
+Enable and configure Django's messages framework, and add ``oauth2_provider`` to your ``INSTALLED_APPS``
 
 .. code-block:: python
 

oauth2_provider/migrations/0013_improve_application_hash_client_secret_help_text.py

@@ -0,0 +1,18 @@
+# Generated by Django 5.0.7 on 2024-08-04 11:47
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('oauth2_provider', '0012_add_token_checksum'),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name='application',
+            name='hash_client_secret',
+            field=models.BooleanField(default=True, help_text='Uncheck if you need to support OIDC with JWT and HS256.'),
+        ),
+    ]

oauth2_provider/models.py

@@ -135,7 +135,9 @@ class AbstractApplication(models.Model):
         db_index=True,
         help_text=_("Hashed on Save. Copy it now if this is a new secret."),
     )
-    hash_client_secret = models.BooleanField(default=True)
+    hash_client_secret = models.BooleanField(
+        default=True, help_text=_("Uncheck if you need to support OIDC with JWT and HS256.")
+    )
     name = models.CharField(max_length=255, blank=True)
     skip_authorization = models.BooleanField(default=False)
 

oauth2_provider/templates/oauth2_provider/application_client_secret_message.html

@@ -0,0 +1,9 @@
+{% load i18n %}
+
+{% block message_content %}
+{% blocktranslate %}
+The application client secret is:
+<div style="font-family: monospace; overflow-wrap: break-word; width: 100%;">{{ client_secret }}</div>
+This will only be shown once, so copy it now!
+{% endblocktranslate %}
+{% endblock %}

oauth2_provider/templates/oauth2_provider/application_detail.html

@@ -5,16 +5,29 @@
     <div class="block-center">
         <h3 class="block-center-heading">{{ application.name }}</h3>
 
+        {% if messages %}
+        <ul class="messages">
+            {% for message in messages %}
+            <li{% if message.tags %} class="{{ message.tags }}"{% endif %}>
+                {% if message.level == DEFAULT_MESSAGE_LEVELS.ERROR %}Important: {% endif %}
+                {{ message }}
+            </li>
+            {% endfor %}
+        </ul>
+        {% endif %}
+
         <ul class="unstyled">
             <li>
-                <p><b>{% trans "Client id" %}</b></p>
-                <input class="input-block-level" type="text" value="{{ application.client_id }}" readonly>
+                <p><b>{% trans "Client ID" %}</b></p>
+                <p>{{ application.client_id }}</p>
             </li>
 
+            {% if client_secret %}
             <li>
                 <p><b>{% trans "Client secret" %}</b></p>
-                <input class="input-block-level" type="text" value="{{ application.client_secret }}" readonly>
+                <p>{{ client_secret }}</p>
             </li>
+            {% endif %}
 
             <li>
                 <p><b>{% trans "Hash client secret" %}</b></p>

oauth2_provider/templates/oauth2_provider/application_form.html

@@ -3,12 +3,22 @@
 {% load i18n %}
 {% block content %}
     <div class="block-center">
-        <form class="form-horizontal" method="post" action="{% block app-form-action-url %}{% url 'oauth2_provider:update' application.pk %}{% endblock app-form-action-url %}">
-            <h3 class="block-center-heading">
-                {% block app-form-title %}
-                    {% trans "Edit application" %} {{ application.name }}
-                {% endblock app-form-title %}
-            </h3>
+        <h3 class="block-center-heading">
+            {% block app-form-title %}
+                {% trans "Edit application" %} {{ application.name }}
+            {% endblock app-form-title %}
+        </h3>
+
+        {% if application.client_id %}
+        <div class="control-group">
+            <label class="control-label">{% translate "Client ID" %}</label>
+            <div class="controls">
+                {{ application.client_id }}
+            </div>
+        </div>
+        {% endif %}
+
+        <form id="application-form" class="form-horizontal" method="post" action="{% block app-form-action-url %}{% url 'oauth2_provider:update' application.id %}{% endblock app-form-action-url %}">
             {% csrf_token %}
 
             {% for field in form %}
@@ -22,21 +32,21 @@ <h3 class="block-center-heading">
                     </div>
                 </div>
             {% endfor %}
+        </form>
 
-            <div class="control-group {% if form.non_field_errors %}error{% endif %}">
-                {% for error in form.non_field_errors %}
-                    <span class="help-inline">{{ error }}</span>
-                {% endfor %}
-            </div>
+        <div class="control-group {% if form.non_field_errors %}error{% endif %}">
+            {% for error in form.non_field_errors %}
+                <span class="help-inline">{{ error }}</span>
+            {% endfor %}
+        </div>
 
-            <div class="control-group">
-                <div class="controls">
-                    <a class="btn" href="{% block app-form-back-url %}{% url "oauth2_provider:detail" application.pk %}{% endblock app-form-back-url %}">
-                        {% trans "Go Back" %}
-                    </a>
-                    <button type="submit" class="btn btn-primary">{% trans "Save" %}</button>
-                </div>
+        <div class="control-group">
+            <div class="controls">
+                <a class="btn" href="{% block app-form-back-url %}{% url "oauth2_provider:detail" application.id %}{% endblock app-form-back-url %}">
+                    {% trans "Go Back" %}
+                </a>
+                <button form="application-form" type="submit" class="btn btn-primary">{% trans "Save" %}</button>
             </div>
-        </form>
+        </div>
     </div>
 {% endblock %}

oauth2_provider/views/application.py

@@ -1,5 +1,7 @@
+from django.contrib import messages
 from django.contrib.auth.mixins import LoginRequiredMixin
 from django.forms.models import modelform_factory
+from django.template.loader import render_to_string
 from django.urls import reverse_lazy
 from django.views.generic import CreateView, DeleteView, DetailView, ListView, UpdateView
 
@@ -32,8 +34,6 @@
             get_application_model(),
             fields=(
                 "name",
-                "client_id",
-                "client_secret",
                 "hash_client_secret",
                 "client_type",
                 "authorization_grant_type",
@@ -46,6 +46,17 @@
 
     def form_valid(self, form):
         form.instance.user = self.request.user
+        # If we are hashing the client secret, display the cleartext value in a flash message with
+        # Django's messages framework
+        if form.cleaned_data["hash_client_secret"]:
+            messages.add_message(
+                self.request,
+                messages.SUCCESS,
+                render_to_string(
+                    "oauth2_provider/application_client_secret_message.html",
+                    {"client_secret": form.instance.client_secret},
+                ),
+            )
         return super().form_valid(form)
 
 
@@ -57,6 +68,12 @@
     context_object_name = "application"
     template_name = "oauth2_provider/application_detail.html"
 
+    def get_context_data(self, **kwargs):
+        ctx = super().get_context_data(**kwargs)
+        if not ctx["application"].hash_client_secret:
+            ctx["client_secret"] = ctx["application"].client_secret
+        return ctx
+
 
 class ApplicationList(ApplicationOwnerIsUserMixin, ListView):
     """
@@ -93,9 +110,6 @@
             get_application_model(),
             fields=(
                 "name",
-                "client_id",
-                "client_secret",
-                "hash_client_secret",
                 "client_type",
                 "authorization_grant_type",
                 "redirect_uris",

tests/test_application_views.py

@@ -37,25 +37,34 @@ def test_get_form_class(self):
     def test_application_registration_user(self):
         self.client.login(username="foo_user", password="123456")
 
+        get_response = self.client.get(reverse("oauth2_provider:register"))
+        self.assertEqual(get_response.status_code, 200)
+
+        self.assertNotIn("client_id", get_response.context["form"].fields)
+        self.assertNotIn("client_secret", get_response.context["form"].fields)
+
         form_data = {
             "name": "Foo app",
-            "client_id": "client_id",
-            "client_secret": "client_secret",
             "client_type": Application.CLIENT_CONFIDENTIAL,
             "redirect_uris": "http://example.com",
             "post_logout_redirect_uris": "http://other_example.com",
             "authorization_grant_type": Application.GRANT_AUTHORIZATION_CODE,
             "algorithm": "",
         }
 
+        # Check that all fields in form_data are form fields
+        for field in form_data.keys():
+            self.assertIn(field, get_response.context["form"].fields.keys())
+
         response = self.client.post(reverse("oauth2_provider:register"), form_data)
         self.assertEqual(response.status_code, 302)
 
         app = get_application_model().objects.get(name="Foo app")
         self.assertEqual(app.user.username, "foo_user")
         app = Application.objects.get()
         self.assertEqual(app.name, form_data["name"])
-        self.assertEqual(app.client_id, form_data["client_id"])
+        self.assertIsNotNone(app.client_id)
+        self.assertIsNotNone(app.client_secret)
         self.assertEqual(app.redirect_uris, form_data["redirect_uris"])
         self.assertEqual(app.post_logout_redirect_uris, form_data["post_logout_redirect_uris"])
         self.assertEqual(app.client_type, form_data["client_type"])
@@ -97,12 +106,21 @@ def test_application_detail_owner(self):
 
         response = self.client.get(reverse("oauth2_provider:detail", args=(self.app_foo_1.pk,)))
         self.assertEqual(response.status_code, 200)
+        self.assertNotIn("client_secret", response.context)
         self.assertContains(response, self.app_foo_1.name)
         self.assertContains(response, self.app_foo_1.redirect_uris)
         self.assertContains(response, self.app_foo_1.post_logout_redirect_uris)
         self.assertContains(response, self.app_foo_1.client_type)
         self.assertContains(response, self.app_foo_1.authorization_grant_type)
 
+        # We don't allow users to update this, setting it False to test context
+        self.app_foo_1.hash_client_secret = False
+        self.app_foo_1.save()
+
+        response = self.client.get(reverse("oauth2_provider:detail", args=(self.app_foo_1.pk,)))
+        self.assertEqual(response.status_code, 200)
+        self.assertIn("client_secret", response.context)
+
     def test_application_detail_not_owner(self):
         self.client.login(username="foo_user", password="123456")
 
@@ -112,21 +130,36 @@ def test_application_detail_not_owner(self):
     def test_application_update(self):
         self.client.login(username="foo_user", password="123456")
 
+        get_response = self.client.get(reverse("oauth2_provider:update", args=(self.app_foo_1.pk,)))
+        self.assertEqual(get_response.status_code, 200)
+
+        self.assertNotIn("client_id", get_response.context["form"].fields)
+        self.assertNotIn("client_secret", get_response.context)
+        self.assertNotIn("client_secret", get_response.context["form"].fields)
+        self.assertNotIn("hash_client_secret", get_response.context["form"].fields)
+
+        new_app_name = self.app_foo_1.name + " - Updated"
+
         form_data = {
-            "client_id": "new_client_id",
+            "name": new_app_name,
             "redirect_uris": "http://new_example.com",
             "post_logout_redirect_uris": "http://new_other_example.com",
             "client_type": Application.CLIENT_PUBLIC,
             "authorization_grant_type": Application.GRANT_OPENID_HYBRID,
         }
+
+        # Check that all fields in form_data are form fields
+        for field in form_data.keys():
+            self.assertIn(field, get_response.context["form"].fields.keys())
+
         response = self.client.post(
             reverse("oauth2_provider:update", args=(self.app_foo_1.pk,)),
             data=form_data,
         )
         self.assertRedirects(response, reverse("oauth2_provider:detail", args=(self.app_foo_1.pk,)))
 
         self.app_foo_1.refresh_from_db()
-        self.assertEqual(self.app_foo_1.client_id, form_data["client_id"])
+        self.assertEqual(self.app_foo_1.name, new_app_name)
         self.assertEqual(self.app_foo_1.redirect_uris, form_data["redirect_uris"])
         self.assertEqual(self.app_foo_1.post_logout_redirect_uris, form_data["post_logout_redirect_uris"])
         self.assertEqual(self.app_foo_1.client_type, form_data["client_type"])