Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 27 additions & 3 deletions oidc_provider/admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,12 @@
from uuid import uuid4

from django.contrib import admin
from django.contrib import messages
from django.forms import ModelForm
from django.utils.html import format_html
from django.utils.translation import gettext_lazy as _

from oidc_provider.lib.utils.client_credentials import hash_secret
from oidc_provider.lib.utils.sanitization import sanitize_client_id
from oidc_provider.models import Client
from oidc_provider.models import Code
Expand Down Expand Up @@ -36,19 +39,26 @@ def clean_client_id(self):
return str(randint(1, 999999)).zfill(6)

def clean_client_secret(self):
instance = getattr(self, "instance", None)
"""
Generate and hash a new secret when creating a confidential client.

The plaintext is stashed on the form so ``save_model`` can display it once
via a one-time admin message. On update the existing hash is preserved.
"""
instance = getattr(self, "instance", None)
self._plaintext_secret = ""
secret = ""

if instance and instance.pk:
if (self.cleaned_data["client_type"] == "confidential") and not instance.client_secret:
secret = sha224(uuid4().hex.encode()).hexdigest()
self._plaintext_secret = sha224(uuid4().hex.encode()).hexdigest()
elif (self.cleaned_data["client_type"] == "confidential") and instance.client_secret:
secret = instance.client_secret
else:
if self.cleaned_data["client_type"] == "confidential":
secret = sha224(uuid4().hex.encode()).hexdigest()
self._plaintext_secret = sha224(uuid4().hex.encode()).hexdigest()

secret = hash_secret(self._plaintext_secret) if self._plaintext_secret else secret
return secret


Expand Down Expand Up @@ -95,6 +105,20 @@ class ClientAdmin(admin.ModelAdmin):
search_fields = ["name"]
raw_id_fields = ["owner"]

def save_model(self, request, obj, form, change):
super().save_model(request, obj, form, change)
plaintext = getattr(form, "_plaintext_secret", None)
if plaintext:
self.message_user(
request,
format_html(
"<strong>Client secret (copy now — this will not be shown again):</strong>"
"<br><code style='font-size:1.1em; user-select:all;'>{}</code>",
plaintext,
),
level=messages.WARNING,
)


@admin.register(Code)
class CodeAdmin(admin.ModelAdmin):
Expand Down
9 changes: 6 additions & 3 deletions oidc_provider/lib/endpoints/introspection.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

from oidc_provider import settings
from oidc_provider.lib.errors import TokenIntrospectionError
from oidc_provider.lib.utils.client_credentials import verify_secret
from oidc_provider.lib.utils.common import run_processing_hook
from oidc_provider.lib.utils.oauth2 import extract_client_auth
from oidc_provider.lib.utils.sanitization import sanitize_client_id
Expand Down Expand Up @@ -48,12 +49,14 @@ def validate_params(self):
raise TokenIntrospectionError()

try:
self.client = Client.objects.get(
client_id=self.params["client_id"], client_secret=self.params["client_secret"]
)
client = Client.objects.get(client_id=self.params["client_id"])
except Client.DoesNotExist:
logger.debug("[Introspection] No valid client for id: %s", self.params["client_id"])
raise TokenIntrospectionError()
if not verify_secret(self.params["client_secret"], client.client_secret):
logger.debug("[Introspection] Invalid client secret for client: %s", self.params["client_id"])
raise TokenIntrospectionError()
self.client = client
if INTROSPECTION_SCOPE not in self.client.scope:
logger.debug(
"[Introspection] Client %s does not have introspection scope",
Expand Down
5 changes: 3 additions & 2 deletions oidc_provider/lib/endpoints/token.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
from oidc_provider import settings
from oidc_provider.lib.errors import TokenError
from oidc_provider.lib.errors import UserAuthError
from oidc_provider.lib.utils.client_credentials import verify_secret
from oidc_provider.lib.utils.oauth2 import extract_client_auth
from oidc_provider.lib.utils.sanitization import sanitize_client_id
from oidc_provider.lib.utils.token import create_id_token
Expand Down Expand Up @@ -54,9 +55,9 @@ def validate_params(self):
raise TokenError("invalid_client")

if self.client.client_type == "confidential":
if not (self.client.client_secret == self.params["client_secret"]):
if not verify_secret(self.params["client_secret"], self.client.client_secret):
logger.debug(
"[Token] Invalid client secret: client %s do not have secret %s",
"[Token] Invalid client secret: client %s does not have secret %s",
self.client.client_id,
self.client.client_secret,
)
Expand Down
12 changes: 12 additions & 0 deletions oidc_provider/lib/utils/client_credentials.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
from django.contrib.auth.hashers import check_password
from django.contrib.auth.hashers import make_password


def hash_secret(plaintext: str) -> str:
"""Hash a client secret using Django's configured password hasher (default: PBKDF2)."""
return make_password(plaintext)


def verify_secret(plaintext: str, hashed: str) -> bool:
"""Verify a submitted secret against a stored hash using Django's constant-time check."""
return check_password(plaintext, hashed)
5 changes: 4 additions & 1 deletion oidc_provider/tests/app/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
import django
from django.contrib.auth.backends import ModelBackend

from oidc_provider.lib.utils.client_credentials import hash_secret

try:
from urlparse import parse_qs
from urlparse import urlsplit
Expand All @@ -21,6 +23,7 @@
from oidc_provider.models import Token

FAKE_NONCE = "cb584e44c43ed6bd0bc2d9c7e242837d"
FAKE_CLIENT_SECRET = "test-client-secret"
FAKE_RANDOM_STRING = "".join(
random.choice(string.ascii_uppercase + string.digits) for _ in range(32)
)
Expand Down Expand Up @@ -61,7 +64,7 @@ def create_fake_client(response_type, is_public=False, require_consent=True):
client.client_type = "public"
client.client_secret = ""
else:
client.client_secret = str(random.randint(1, 999999)).zfill(6)
client.client_secret = hash_secret(FAKE_CLIENT_SECRET)
client.redirect_uris = ["http://example.com/"]
client.require_consent = require_consent
client.scope = ["openid", "email"]
Expand Down
123 changes: 123 additions & 0 deletions oidc_provider/tests/cases/test_admin.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,15 @@
from unittest.mock import MagicMock

from django.contrib import messages
from django.contrib.admin.sites import AdminSite
from django.contrib.auth import get_user_model
from django.contrib.messages.storage.fallback import FallbackStorage
from django.test import RequestFactory
from django.test import TestCase

from oidc_provider.admin import ClientAdmin
from oidc_provider.admin import ClientForm
from oidc_provider.lib.utils.client_credentials import verify_secret
from oidc_provider.models import Client
from oidc_provider.models import ResponseType
from oidc_provider.tests.app.utils import create_fake_user
Expand Down Expand Up @@ -96,3 +104,118 @@ def test_sanitizes_existing_client_id_with_control_characters(self):
# Should return sanitized client_id
client_id = form.clean_client_id()
self.assertEqual(client_id, "clienttest") # Control characters removed


class ClientFormSecretTest(TestCase):
def setUp(self):
self.user = create_fake_user()
self.code_response_type, _ = ResponseType.objects.get_or_create(
value="code", defaults={"description": "code (Authorization Code Flow)"}
)
self.base_form_data = {
"name": "Test Client",
"owner": self.user.pk,
"response_types": [self.code_response_type.pk],
"_redirect_uris": "http://example.com/callback",
}

def _make_form(self, client_type, instance=None):
data = {**self.base_form_data, "client_type": client_type}
return ClientForm(data=data, instance=instance)

def test_new_confidential_client_stores_hash_not_plaintext(self):
form = self._make_form("confidential")
self.assertTrue(form.is_valid(), form.errors)
stored = form.cleaned_data["client_secret"]
self.assertTrue(
stored.startswith("pbkdf2") or stored.startswith("bcrypt") or "$" in stored
) # Hashing algoritm is dependent on settings and Django version, so we check for common patterns
self.assertNotEqual(stored, form._plaintext_secret)

def test_new_confidential_client_stashes_plaintext_on_form(self):
form = self._make_form("confidential")
self.assertTrue(form.is_valid(), form.errors)
self.assertTrue(form._plaintext_secret)
self.assertTrue(verify_secret(form._plaintext_secret, form.cleaned_data["client_secret"]))

def test_new_public_client_has_no_secret(self):
form = self._make_form("public")
self.assertTrue(form.is_valid(), form.errors)
self.assertEqual(form.cleaned_data["client_secret"], "")
self.assertEqual(form._plaintext_secret, "")

def test_existing_confidential_client_with_secret_preserves_it(self):
client = Client.objects.create(
name="Existing",
owner=self.user,
client_type="confidential",
client_secret="already-hashed-value",
)
client.response_types.add(self.code_response_type)
form = self._make_form("confidential", instance=client)
self.assertTrue(form.is_valid(), form.errors)
self.assertEqual(form.cleaned_data["client_secret"], "already-hashed-value")
self.assertEqual(form._plaintext_secret, "")

def test_existing_confidential_client_without_secret_generates_new_hash(self):
client = Client.objects.create(
name="Existing No Secret",
owner=self.user,
client_type="confidential",
client_secret="",
)
client.response_types.add(self.code_response_type)
form = self._make_form("confidential", instance=client)
self.assertTrue(form.is_valid(), form.errors)
self.assertTrue(form._plaintext_secret)
self.assertTrue(verify_secret(form._plaintext_secret, form.cleaned_data["client_secret"]))


class ClientAdminSaveModelTest(TestCase):
def setUp(self):
self.user = create_fake_user()
self.site = AdminSite()
self.admin = ClientAdmin(Client, self.site)
self.factory = RequestFactory()
self.code_response_type, _ = ResponseType.objects.get_or_create(
value="code", defaults={"description": "code (Authorization Code Flow)"}
)

def _make_request(self):
request = self.factory.post("/")
request.user = self.user
request.session = "session"
request._messages = FallbackStorage(request)
return request

def _make_form_with_plaintext(self, plaintext):
form = MagicMock()
form._plaintext_secret = plaintext
return form

def test_save_model_shows_warning_message_with_plaintext_for_confidential_client(self):
obj = Client.objects.create(
name="New Confidential",
owner=self.user,
client_type="confidential",
client_secret="hashed-value",
)
request = self._make_request()
self.admin.save_model(request, obj, self._make_form_with_plaintext("the-plain-secret"), change=False)

stored = list(request._messages)
self.assertEqual(len(stored), 1)
self.assertEqual(stored[0].level, messages.WARNING)
self.assertIn("the-plain-secret", stored[0].message)

def test_save_model_shows_no_message_for_public_client(self):
obj = Client.objects.create(
name="New Public",
owner=self.user,
client_type="public",
client_secret="",
)
request = self._make_request()
self.admin.save_model(request, obj, self._make_form_with_plaintext(""), change=False)

self.assertEqual(list(request._messages), [])
33 changes: 33 additions & 0 deletions oidc_provider/tests/cases/test_client_credentials.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
from django.test import TestCase

from oidc_provider.lib.utils.client_credentials import hash_secret
from oidc_provider.lib.utils.client_credentials import verify_secret


class HashSecretTest(TestCase):
def test_returns_different_value_from_plaintext(self):
hashed = hash_secret("mysecret")
self.assertNotEqual(hashed, "mysecret")

def test_returns_string(self):
self.assertIsInstance(hash_secret("mysecret"), str)

def test_two_calls_produce_different_hashes(self):
self.assertNotEqual(hash_secret("mysecret"), hash_secret("mysecret"))


class VerifySecretTest(TestCase):
def test_correct_plaintext_returns_true(self):
hashed = hash_secret("correct")
self.assertTrue(verify_secret("correct", hashed))

def test_wrong_plaintext_returns_false(self):
hashed = hash_secret("correct")
self.assertFalse(verify_secret("wrong", hashed))

def test_empty_plaintext_returns_false(self):
hashed = hash_secret("correct")
self.assertFalse(verify_secret("", hashed))

def test_invalid_hash_returns_false(self):
self.assertFalse(verify_secret("anything", "not-a-valid-hash"))
3 changes: 2 additions & 1 deletion oidc_provider/tests/cases/test_introspection_endpoint.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
from django.core.urlresolvers import reverse

from oidc_provider.lib.utils.token import create_id_token
from oidc_provider.tests.app.utils import FAKE_CLIENT_SECRET
from oidc_provider.tests.app.utils import FAKE_RANDOM_STRING
from oidc_provider.tests.app.utils import create_fake_client
from oidc_provider.tests.app.utils import create_fake_token
Expand Down Expand Up @@ -67,7 +68,7 @@ def _make_request(self, **kwargs):
url = reverse("oidc_provider:token-introspection")
data = {
"client_id": kwargs.get("client_id", self.resource.client_id),
"client_secret": kwargs.get("client_secret", self.resource.client_secret),
"client_secret": kwargs.get("client_secret", FAKE_CLIENT_SECRET),
"token": kwargs.get("access_token", self.token.access_token),
}

Expand Down
Loading