diff --git a/duo_universal/client.py b/duo_universal/client.py index 579edd8..4e1bc03 100644 --- a/duo_universal/client.py +++ b/duo_universal/client.py @@ -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 @@ -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" @@ -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): """ @@ -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 @@ -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: @@ -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: @@ -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 @@ -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 @@ -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): """ @@ -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) @@ -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, @@ -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) diff --git a/tests/test_setup_client.py b/tests/test_setup_client.py index 6cccbda..2902a91 100644 --- a/tests/test_setup_client.py +++ b/tests/test_setup_client.py @@ -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 @@ -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): @@ -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): @@ -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): @@ -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): @@ -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): @@ -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): @@ -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): @@ -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) diff --git a/tests/test_validate_create_auth_url_inputs.py b/tests/test_validate_create_auth_url_inputs.py index 23a538c..313a5c6 100644 --- a/tests/test_validate_create_auth_url_inputs.py +++ b/tests/test_validate_create_auth_url_inputs.py @@ -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 @@ -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()