diff --git a/docs/getting_started.rst b/docs/getting_started.rst index e95618723..a5e4a68f8 100644 --- a/docs/getting_started.rst +++ b/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 `), 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 `), 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. diff --git a/docs/install.rst b/docs/install.rst index 3d46c507d..d985fc41d 100644 --- a/docs/install.rst +++ b/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 diff --git a/oauth2_provider/migrations/0013_improve_application_hash_client_secret_help_text.py b/oauth2_provider/migrations/0013_improve_application_hash_client_secret_help_text.py new file mode 100644 index 000000000..1c590654b --- /dev/null +++ b/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.'), + ), + ] diff --git a/oauth2_provider/models.py b/oauth2_provider/models.py index 621ce5b34..c903d57e2 100644 --- a/oauth2_provider/models.py +++ b/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) diff --git a/oauth2_provider/templates/oauth2_provider/application_client_secret_message.html b/oauth2_provider/templates/oauth2_provider/application_client_secret_message.html new file mode 100644 index 000000000..eaada0f05 --- /dev/null +++ b/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: +
{{ client_secret }}
+This will only be shown once, so copy it now! +{% endblocktranslate %} +{% endblock %} diff --git a/oauth2_provider/templates/oauth2_provider/application_detail.html b/oauth2_provider/templates/oauth2_provider/application_detail.html index 74b71ee74..c34133067 100644 --- a/oauth2_provider/templates/oauth2_provider/application_detail.html +++ b/oauth2_provider/templates/oauth2_provider/application_detail.html @@ -5,16 +5,29 @@

{{ application.name }}

+ {% if messages %} +
    + {% for message in messages %} + + {% if message.level == DEFAULT_MESSAGE_LEVELS.ERROR %}Important: {% endif %} + {{ message }} + + {% endfor %} +
+ {% endif %} +
  • -

    {% trans "Client id" %}

    - +

    {% trans "Client ID" %}

    +

    {{ application.client_id }}

    • + {% if client_secret %}

  • {% trans "Client secret" %}

    - +

    {{ client_secret }}

    • + {% endif %}

  • {% trans "Hash client secret" %}

    diff --git a/oauth2_provider/templates/oauth2_provider/application_form.html b/oauth2_provider/templates/oauth2_provider/application_form.html index 7d8c07989..2a2b20b2d 100644 --- a/oauth2_provider/templates/oauth2_provider/application_form.html +++ b/oauth2_provider/templates/oauth2_provider/application_form.html @@ -3,12 +3,22 @@ {% load i18n %} {% block content %}
    -
    -

    - {% block app-form-title %} - {% trans "Edit application" %} {{ application.name }} - {% endblock app-form-title %} -

    +

    + {% block app-form-title %} + {% trans "Edit application" %} {{ application.name }} + {% endblock app-form-title %} +

    + + {% if application.client_id %} +
    + +
    + {{ application.client_id }} +
    +
    + {% endif %} + + {% csrf_token %} {% for field in form %} @@ -22,21 +32,21 @@

{% endfor %} + -
- {% for error in form.non_field_errors %} - {{ error }} - {% endfor %} -
+
+ {% for error in form.non_field_errors %} + {{ error }} + {% endfor %} +
-
-
- - {% trans "Go Back" %} - - -
+
+
+ + {% trans "Go Back" %} + +
- +
{% endblock %} diff --git a/oauth2_provider/views/application.py b/oauth2_provider/views/application.py index b896c45e3..202412003 100644 --- a/oauth2_provider/views/application.py +++ b/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 @@ def get_form_class(self): get_application_model(), fields=( "name", - "client_id", - "client_secret", "hash_client_secret", "client_type", "authorization_grant_type", @@ -46,6 +46,17 @@ def get_form_class(self): 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 @@ class ApplicationDetail(ApplicationOwnerIsUserMixin, DetailView): 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 @@ def get_form_class(self): get_application_model(), fields=( "name", - "client_id", - "client_secret", - "hash_client_secret", "client_type", "authorization_grant_type", "redirect_uris", diff --git a/tests/test_application_views.py b/tests/test_application_views.py index 88617807d..d02c43633 100644 --- a/tests/test_application_views.py +++ b/tests/test_application_views.py @@ -37,10 +37,14 @@ 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", @@ -48,6 +52,10 @@ def test_application_registration_user(self): "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) @@ -55,7 +63,8 @@ def test_application_registration_user(self): 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,13 +130,28 @@ 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, @@ -126,7 +159,7 @@ def test_application_update(self): 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"])