Skip to content

Commit

Permalink
Add Support For Passing Nonce and Configurable JWT Expiry (#22)
Browse files Browse the repository at this point in the history 
* Add Support For Passing Nonce and Configurable JWT Expiry

* Added support to the Python client to allow the nonce to be set in
  the auth URL
* Added a client parameter to allow JWT expiry to be configured.
* Updated some tests to check nonce validation.

* Configurable Nonce/Expiry -- PR Review Suggestions

* Client now throws an exception if an out of range expiry
  was given on initialization; the value is furthermore
  clamped to between 0 seconds and 5 minutes where it's used.
* Doc strings have been updated to reflect what the underlying
  OIDC API specifies around the state and nonce.
* Tests have been added to verify that the expiry is checked
  correctly.

* * Fixed flake8 errors on a test.
  • Loading branch information
@rpcope1
rpcope1 authored Feb 2, 2024
1 parent 1ee3e5b commit 0716495
Show file tree
Hide file tree
Showing 3 changed files with 90 additions and 21 deletions.
48 changes: 36 additions & 12 deletions duo_universal/client.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
CLIENT_ID_LENGTH = 20
CLIENT_SECRET_LENGTH = 40
JTI_LENGTH = 36
MINIMUM_STATE_LENGTH = 22
MINIMUM_STATE_LENGTH = 16
MAXIMUM_STATE_LENGTH = 1024
STATE_LENGTH = 36
SUCCESS_STATUS_CODE = 200
Expand All @@ -33,6 +33,12 @@
MIN=MINIMUM_STATE_LENGTH,
MAX=MAXIMUM_STATE_LENGTH
)
ERR_NONCE_LEN = ('Nonce must be at least {MIN} characters long and no longer than {MAX} characters').format(
MIN=MINIMUM_STATE_LENGTH,
MAX=MAXIMUM_STATE_LENGTH
)
ERR_EXP_SECONDS_TOO_LONG = 'Client may not be configured for a JWT expiry longer than five minutes.'
ERR_EXP_SECONDS_TOO_SHORT = 'Invalid JWT expiry duration.'

API_HOST_URI_FORMAT = "https://{}"
OAUTH_V1_HEALTH_CHECK_ENDPOINT = "https://{}/oauth/v1/health_check"
Expand All @@ -48,6 +54,9 @@ class DuoException(Exception):


class Client:
@property
def _clamped_expiry_duration(self):
return max(min(FIVE_MINUTES_IN_SECONDS, self._exp_seconds), 1)

def _generate_rand_alphanumeric(self, length):
"""
Expand All @@ -71,7 +80,7 @@ def _generate_rand_alphanumeric(self, length):
return ''.join(generator.choice(characters) for i in range(length))

def _validate_init_config(self, client_id, client_secret,
api_host, redirect_uri):
api_host, redirect_uri, exp_seconds):
"""
Verifies __init__ parameters
Expand All @@ -81,6 +90,7 @@ def _validate_init_config(self, client_id, client_secret,
client_secret -- Client secret for the application in Duo
host -- Duo api host
redirect_uri -- Uri to redirect to after a successful auth
exp_seconds -- The JWT expiry window
Raises:
Expand All @@ -94,8 +104,14 @@ def _validate_init_config(self, client_id, client_secret,
raise DuoException(ERR_API_HOST)
if not redirect_uri:
raise DuoException(ERR_REDIRECT_URI)

def _validate_create_auth_url_inputs(self, username, state):
if exp_seconds > FIVE_MINUTES_IN_SECONDS:
raise DuoException(ERR_EXP_SECONDS_TOO_LONG)
elif exp_seconds < 0:
raise DuoException(ERR_EXP_SECONDS_TOO_SHORT)

def _validate_create_auth_url_inputs(self, username, state, nonce=None):
if nonce and (MINIMUM_STATE_LENGTH >= len(nonce) or len(nonce) >= MAXIMUM_STATE_LENGTH):
raise DuoException(ERR_NONCE_LEN)
if not state or not (MINIMUM_STATE_LENGTH <= len(state) <= MAXIMUM_STATE_LENGTH):
raise DuoException(ERR_STATE_LEN)
if not username:
Expand All @@ -106,14 +122,15 @@ def _create_jwt_args(self, endpoint):
'iss': self._client_id,
'sub': self._client_id,
'aud': endpoint,
'exp': time.time() + FIVE_MINUTES_IN_SECONDS,
'exp': time.time() + self._clamped_expiry_duration,
'jti': self._generate_rand_alphanumeric(JTI_LENGTH)
}

return jwt_args

def __init__(self, client_id, client_secret, host,
redirect_uri, duo_certs=DEFAULT_CA_CERT_PATH, use_duo_code_attribute=True, http_proxy=None):
redirect_uri, duo_certs=DEFAULT_CA_CERT_PATH, use_duo_code_attribute=True, http_proxy=None,
exp_seconds=FIVE_MINUTES_IN_SECONDS):
"""
Initializes instance of Client class
Expand All @@ -126,12 +143,14 @@ def __init__(self, client_id, client_secret, host,
duo_certs -- (Optional) Provide custom CA certs
use_duo_code_attribute -- (Optional: default true) Flag to use `duo_code` instead of `code` for returned authorization parameter
http_proxy -- (Optional) HTTP proxy to tunnel requests through
exp_seconds -- (Optional) The number of seconds used for JWT expiry. Must be be at most 5 minutes.
"""

self._validate_init_config(client_id,
client_secret,
host,
redirect_uri)
redirect_uri,
exp_seconds)

self._client_id = client_id
self._client_secret = client_secret
Expand All @@ -153,6 +172,7 @@ def __init__(self, client_id, client_secret, host,
self._http_proxy = {'https': http_proxy}
else:
self._http_proxy = None
self._exp_seconds = exp_seconds

def generate_state(self):
"""
Expand Down Expand Up @@ -198,21 +218,23 @@ def health_check(self):

return res

def create_auth_url(self, username, state):
def create_auth_url(self, username, state, nonce=None):
"""Generate uri to Duo's prompt
Arguments:
username -- username trying to authenticate with Duo
state -- Randomly generated character string of at least 22
chars returned to the integration by Duo after 2FA
state -- Randomly generated character string of at least 16
and at most 1024 characters returned to the integration by Duo after 2FA
nonce -- Randomly generated character string of at least 16
and at most 1024 characters used as the nonce for the underlying OIDC flow
Returns:
Authorization uri to redirect to for the Duo prompt
"""

self._validate_create_auth_url_inputs(username, state)
self._validate_create_auth_url_inputs(username, state, nonce=nonce)

authorize_endpoint = OAUTH_V1_AUTHORIZE_ENDPOINT.format(self._api_host)

Expand All @@ -222,7 +244,7 @@ def create_auth_url(self, username, state):
'client_id': self._client_id,
'iss': self._client_id,
'aud': API_HOST_URI_FORMAT.format(self._api_host),
'exp': time.time() + FIVE_MINUTES_IN_SECONDS,
'exp': time.time() + self._clamped_expiry_duration,
'state': state,
'response_type': 'code',
'duo_uname': username,
Expand All @@ -237,6 +259,8 @@ def create_auth_url(self, username, state):
'client_id': self._client_id,
'request': request_jwt,
}
if nonce:
all_args['nonce'] = nonce

query_string = urlencode(all_args)
authorization_uri = "{}?{}".format(authorize_endpoint, query_string)
Expand Down
38 changes: 29 additions & 9 deletions tests/test_setup_client.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
REDIRECT_URI = "https://www.example.com"
CA_CERT_NEW = "/path/to/cert/ca_cert_new.pem"
PROXY_HOST = "http://proxy.example.com:8001"
EXP_SECONDS = client.FIVE_MINUTES_IN_SECONDS
NONE = None


Expand All @@ -26,7 +27,7 @@ def test_short_client_id(self):
"""
with self.assertRaises(client.DuoException) as e:
self.client._validate_init_config(SHORT_CLIENT_ID, CLIENT_SECRET,
HOST, REDIRECT_URI)
HOST, REDIRECT_URI, EXP_SECONDS)
self.assertEqual(e, client.ERR_CLIENT_ID)

def test_long_client_id(self):
Expand All @@ -35,7 +36,7 @@ def test_long_client_id(self):
"""
with self.assertRaises(client.DuoException) as e:
self.client._validate_init_config(LONG_CLIENT_ID, CLIENT_SECRET,
HOST, REDIRECT_URI)
HOST, REDIRECT_URI, EXP_SECONDS)
self.assertEqual(e, client.ERR_CLIENT_ID)

def test_no_client_id(self):
Expand All @@ -44,7 +45,7 @@ def test_no_client_id(self):
"""
with self.assertRaises(client.DuoException) as e:
self.client._validate_init_config(NONE, CLIENT_SECRET,
HOST, REDIRECT_URI)
HOST, REDIRECT_URI, EXP_SECONDS)
self.assertEqual(e, client.ERR_CLIENT_ID)

def test_short_client_secret(self):
Expand All @@ -53,7 +54,7 @@ def test_short_client_secret(self):
"""
with self.assertRaises(client.DuoException) as e:
self.client._validate_init_config(CLIENT_ID, SHORT_CLIENT_SECRET,
HOST, REDIRECT_URI)
HOST, REDIRECT_URI, EXP_SECONDS)
self.assertEqual(e, client.ERR_CLIENT_SECRET)

def test_long_client_secret(self):
Expand All @@ -62,7 +63,7 @@ def test_long_client_secret(self):
"""
with self.assertRaises(client.DuoException) as e:
self.client._validate_init_config(CLIENT_ID, LONG_CLIENT_SECRET,
HOST, REDIRECT_URI)
HOST, REDIRECT_URI, EXP_SECONDS)
self.assertEqual(e, client.ERR_CLIENT_SECRET)

def test_no_client_secret(self):
Expand All @@ -71,7 +72,7 @@ def test_no_client_secret(self):
"""
with self.assertRaises(client.DuoException) as e:
self.client._validate_init_config(CLIENT_ID, NONE,
HOST, REDIRECT_URI)
HOST, REDIRECT_URI, EXP_SECONDS)
self.assertEqual(e, client.ERR_CLIENT_SECRET)

def test_no_host(self):
Expand All @@ -80,7 +81,7 @@ def test_no_host(self):
"""
with self.assertRaises(client.DuoException) as e:
self.client._validate_init_config(CLIENT_ID, CLIENT_SECRET,
NONE, REDIRECT_URI)
NONE, REDIRECT_URI, EXP_SECONDS)
self.assertEqual(e, client.ERR_API_HOST)

def test_no_redirect_uri(self):
Expand All @@ -89,15 +90,34 @@ def test_no_redirect_uri(self):
"""
with self.assertRaises(client.DuoException) as e:
self.client._validate_init_config(CLIENT_ID, CLIENT_SECRET,
HOST, NONE)
HOST, NONE, EXP_SECONDS)
self.assertEqual(e, client.ERR_REDIRECT_URI)

def test_exp_seconds_too_long(self):
"""
Test to validate that a user can't set an expiry longer than 5 minutes.
"""
with self.assertRaises(client.DuoException) as e:
self.client._validate_init_config(CLIENT_ID, CLIENT_SECRET, HOST, REDIRECT_URI, 2 * EXP_SECONDS)
self.assertEqual(e, client.ERR_EXP_SECONDS_TOO_LONG)
# Even if the end user forcefully sets the expiry, ensure the clamped value is in spec.
self.client._exp_seconds = 2 * EXP_SECONDS
self.assertEqual(self.client._clamped_expiry_duration, client.FIVE_MINUTES_IN_SECONDS)

def test_exp_seconds_too_short(self):
"""
Test to validate that a user can't set an expiry shorter than 0.
"""
with self.assertRaises(client.DuoException) as e:
self.client._validate_init_config(CLIENT_ID, CLIENT_SECRET, HOST, REDIRECT_URI, -1)
self.assertEqual(e, client.ERR_EXP_SECONDS_TOO_SHORT)

def test_successful(self):
"""
Test successful _validate_init_config
"""
self.client._validate_init_config(CLIENT_ID, CLIENT_SECRET,
HOST, REDIRECT_URI)
HOST, REDIRECT_URI, EXP_SECONDS)

def test_no_duo_cert(self):
self.assertEqual(self.client._duo_certs, client.DEFAULT_CA_CERT_PATH)
Expand Down
25 changes: 25 additions & 0 deletions tests/test_validate_create_auth_url_inputs.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,24 @@ def test_long_state(self):
self.client._validate_create_auth_url_inputs(USERNAME, LONG_LENGTH)
self.assertEqual(e, client.ERR_STATE)

def test_short_nonce(self):
"""
Test _validate_create_auth_url_inputs
throws a DuoException if the nonce is set and is too short
"""
with self.assertRaises(client.DuoException) as e:
self.client._validate_create_auth_url_inputs(USERNAME, STATE, nonce=SHORT_STATE)
self.assertEqual(e, client.ERR_NONCE_LEN)

def test_long_nonce(self):
"""
Test _validate_create_auth_url_inputs
throws a DuoException if the nonce is set and is too long
"""
with self.assertRaises(client.DuoException) as e:
self.client._validate_create_auth_url_inputs(USERNAME, STATE, nonce=LONG_LENGTH)
self.assertEqual(e, client.ERR_NONCE_LEN)

def test_no_username(self):
"""
Test _validate_create_auth_url_inputs
Expand All @@ -60,6 +78,13 @@ def test_success(self):
"""
self.client._validate_create_auth_url_inputs(USERNAME, STATE)

def test_success_with_nonce(self):
"""
Test _validate_create_auth_url_inputs
does not throw an error for valid inputs
"""
self.client._validate_create_auth_url_inputs(USERNAME, STATE, nonce=STATE)


if __name__ == '__main__':
unittest.main()

0 comments on commit 0716495

Please sign in to comment.