Skip to content

Commit

Permalink
Make nonce cache timeout configurable (#62)
Browse files Browse the repository at this point in the history
* make nonce cache timeout configuravle

Django's default cache timeout is 300 seconds (5 minutes). Users who took longer that 5 minutes to create their accounts would run into the error "Failed to verify returned nonce value"

* fix isort lint error
  • Loading branch information
kelvin-muchiri authored Nov 2, 2023
1 parent 5a9e846 commit f4e2e19
Show file tree
Hide file tree
Showing 6 changed files with 117 additions and 6 deletions.
3 changes: 2 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -7,4 +7,5 @@ __pycache__/
build/
dist/
.tox/
.eggs
.eggs
.python-version
3 changes: 2 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,8 @@ OPENID_CONNECT_AUTH_SERVERS = {
"REDIRECT_URI": "http://localhost:8000/oidc/msft/callback",
"RESPONSE_TYPE": "id_token",
"RESPONSE_MODE": "form_post",
"USE_NONCES": True
"USE_NONCES": True,
"NONCE_CACHE_TIMEOUT": 1800,
}
}
...
Expand Down
10 changes: 7 additions & 3 deletions oidc/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,6 @@
import oidc.settings as default
from oidc.utils import str_to_bool

config = getattr(settings, "OPENID_CONNECT_AUTH_SERVERS", {})
default_config = getattr(default, "OPENID_CONNECT_AUTH_SERVERS", {})["default"]

REDIRECT_AFTER_AUTH = "redirect_after_auth"


Expand All @@ -43,6 +40,8 @@ def __init__(self, auth_server: str) -> None:
"""
Initializes an OpenID Connect Client object
"""
config = getattr(settings, "OPENID_CONNECT_AUTH_SERVERS", {})
default_config = getattr(default, "OPENID_CONNECT_AUTH_SERVERS", {})["default"]
self.auth_server = auth_server
self.authorization_endpoint = config[auth_server].get("AUTHORIZATION_ENDPOINT")
self.client_id = config[auth_server].get("CLIENT_ID")
Expand All @@ -61,6 +60,10 @@ def __init__(self, auth_server: str) -> None:
self.cache_nonces = str_to_bool(
config[auth_server].get("USE_NONCES") or default_config["USE_NONCES"]
)
self.nonce_cache_timeout = int(
config[auth_server].get("NONCE_CACHE_TIMEOUT")
or default_config["NONCE_CACHE_TIMEOUT"]
)

def _retrieve_jwks_related_to_kid(self, kid: str) -> Optional[str]:
"""
Expand Down Expand Up @@ -149,6 +152,7 @@ def login(self, redirect_after: Optional[str] = None) -> str:
cache.set(
nonce,
{"auth_server": self.auth_server, "redirect_after": redirect_after},
self.nonce_cache_timeout,
)
url += f"&nonce={nonce}"

Expand Down
1 change: 1 addition & 0 deletions oidc/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,5 +32,6 @@
"RESPONSE_TYPE": "id_token",
"RESPONSE_MODE": "form_post",
"USE_NONCES": True,
"NONCE_CACHE_TIMEOUT": 1800,
}
}
3 changes: 2 additions & 1 deletion oidc/viewsets.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@
OpenIDClient,
TokenVerificationFailed,
)
from oidc.client import config as auth_config
from oidc.utils import str_to_bool

default_config = getattr(default, "OPENID_CONNECT_VIEWSET_CONFIG", {})
Expand Down Expand Up @@ -98,6 +97,8 @@ def __init__(self, *args, **kwargs):
)

def _get_client(self, auth_server: str) -> Optional[OpenIDClient]:
auth_config = getattr(settings, "OPENID_CONNECT_AUTH_SERVERS", {})

if auth_server in auth_config:
return OpenIDClient(auth_server)
return None
Expand Down
103 changes: 103 additions & 0 deletions tests/test_client.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
"""Tests for module oidc.client"""
import secrets
from unittest.mock import patch

from django.core.cache import cache
from django.http import HttpResponseRedirect
from django.test import TestCase
from django.test.utils import override_settings

from oidc.client import OpenIDClient

OPENID_CONNECT_AUTH_SERVERS = {
"default": {
"AUTHORIZATION_ENDPOINT": "example.com/oauth2/v2.0/authorize",
"CLIENT_ID": "client",
"JWKS_ENDPOINT": "example.com/discovery/v2.0/keys",
"SCOPE": "openid profile",
"TOKEN_ENDPOINT": "example.com/oauth2/v2.0/token",
"END_SESSION_ENDPOINT": "http://localhost:3000",
"REDIRECT_URI": "http://localhost:8000/oidc/msft/callback",
"RESPONSE_TYPE": "code",
"RESPONSE_MODE": "form_post",
"USE_NONCES": False,
"NONCE_CACHE_TIMEOUT": 600,
}
}


class OpenIDClientTestCase(TestCase):
"""Tests for class OpenIDClient"""

def setUp(self) -> None:
super().setUp()

self.maxDiff = None

@override_settings(OPENID_CONNECT_AUTH_SERVERS=OPENID_CONNECT_AUTH_SERVERS)
@patch.object(cache, "set")
@patch.object(secrets, "randbits")
def test_login(self, mock_randbits, mock_cache_set):
"""Returns redirect URL"""
mock_randbits.return_value = "123"
expected_url = (
"example.com/oauth2/v2.0/authorize?"
"client_id=client&"
"redirect_uri=http://localhost:8000/oidc/msft/callback&"
"scope=openid%20profile&"
"response_type=code&"
"response_mode=form_post&"
"nonce=123"
)
client = OpenIDClient("default")
result = client.login()
mock_cache_set.assert_called_once_with(
"123",
{"auth_server": "default", "redirect_after": None},
600,
)
self.assertIsInstance(result, HttpResponseRedirect)
self.assertEqual(result.url, expected_url)

# `redirect_after` arg is passed
mock_cache_set.reset_mock()
result = client.login("foo")
mock_cache_set.assert_called_once_with(
"123",
{"auth_server": "default", "redirect_after": "foo"},
600,
)
self.assertIsInstance(result, HttpResponseRedirect)
self.assertEqual(result.url, expected_url)

@override_settings(
OPENID_CONNECT_AUTH_SERVERS={
"default": {
**OPENID_CONNECT_AUTH_SERVERS["default"],
"NONCE_CACHE_TIMEOUT": None,
}
}
)
@patch.object(cache, "set")
@patch.object(secrets, "randbits")
def test_login_nonce_timeout_missing(self, mock_randbits, mock_cache_set):
"""Uses default nonce timeout on login if timeout not set"""
mock_randbits.return_value = "123"
expected_url = (
"example.com/oauth2/v2.0/authorize?"
"client_id=client&"
"redirect_uri=http://localhost:8000/oidc/msft/callback&"
"scope=openid%20profile&"
"response_type=code&"
"response_mode=form_post&"
"nonce=123"
)
client = OpenIDClient("default")
result = client.login()
mock_cache_set.assert_called_once_with(
"123",
{"auth_server": "default", "redirect_after": None},
1800,
)
self.assertIsInstance(result, HttpResponseRedirect)
self.assertEqual(result.url, expected_url)

0 comments on commit f4e2e19

Please sign in to comment.