diff --git a/python/nav/etc/jwt.conf b/python/nav/etc/webfront/jwt.conf similarity index 61% rename from python/nav/etc/jwt.conf rename to python/nav/etc/webfront/jwt.conf index 4b4ad4cbfe..ab308998c1 100644 --- a/python/nav/etc/jwt.conf +++ b/python/nav/etc/webfront/jwt.conf @@ -1,7 +1,19 @@ -# This file contains configuration for JWT issuers that this NAV instance -# should accept tokens from. - -# Each issuer is defined in a section. The name of the section should be +# This file contains configuration for JWT tokens. +# This includes tokens generated by NAV itself and external issuers. + +# The settings in the `nav` section are for tokens issued by NAV itself. +# Comment this in and fill out the values if you want to activate local JWT tokens. +# The section name should NOT be changed. + +#[nav] +# Absolute path to private key in PEM format +#private_key= +# Absolute path to public key in PEM format. +#public_key= +# Used for the 'iss' and 'aud' claims of generated tokens. +#name= + +# Custom JWT issuers can be defined in a section. The name of the section should be # the issuers name, the same value as the 'iss' claim will be in tokens # generated by this issuer. This value is often a URL, but does not # have to be. diff --git a/python/nav/jwtconf.py b/python/nav/jwtconf.py index 453392ba67..5d8287aba5 100644 --- a/python/nav/jwtconf.py +++ b/python/nav/jwtconf.py @@ -1,6 +1,8 @@ import logging +from os.path import join from functools import partial import configparser +from typing import Dict, Any from nav.config import ConfigurationError, NAVConfigParser @@ -8,35 +10,65 @@ class JWTConf(NAVConfigParser): - """jwt.conf config parser""" + """webfront/jwt.conf config parser""" - DEFAULT_CONFIG_FILES = ('jwt.conf',) + DEFAULT_CONFIG_FILES = [join('webfront', 'jwt.conf')] + NAV_SECTION = "nav" - def get_issuers_setting(self): - issuers_settings = dict() - for section in self.sections(): - try: + def get_issuers_setting(self) -> Dict[str, Any]: + """Parses the webfront/jwt.conf config file and returns a dictionary that can + be used as settings for the `drf-oidc-auth` django extension. + If the parsing fails, an empty dict is returned. + """ + try: + external_settings = self._get_settings_for_external_tokens() + local_settings = self._get_settings_for_nav_issued_tokens() + external_settings.update(local_settings) + return external_settings + except ConfigurationError as error: + _logger.error('Error reading jwtconfig: %s', error) + return dict() + + def _get_settings_for_external_tokens(self): + settings = dict() + try: + for section in self.sections(): + if section == self.NAV_SECTION: + continue get = partial(self.get, section) + issuer = self._validate_issuer(section) key = self._validate_key(get('key')) aud = self._validate_audience(get('aud')) key_type = self._validate_type(get('keytype')) if key_type == 'PEM': - key = self._read_file(key) + key = self._read_key_from_path(key) claims_options = { 'aud': {'values': [aud], 'essential': True}, } - issuers_settings[section] = { + settings[issuer] = { 'key': key, 'type': key_type, 'claims_options': claims_options, } - except (configparser.Error, ConfigurationError) as error: - _logger.error('Error collecting stats for %s: %s', section, error) - return issuers_settings + except ( + configparser.NoSectionError, + configparser.NoOptionError, + ) as error: + raise ConfigurationError(error) + return settings - def _read_file(self, file): - with open(file, "r") as f: - return f.read() + def _read_key_from_path(self, path): + try: + with open(path, "r") as f: + return f.read() + except FileNotFoundError: + raise ConfigurationError( + "Could not find file %s to read PEM key from", path + ) + except PermissionError: + raise ConfigurationError( + "Could not access file %s to read PEM key from", path + ) def _validate_key(self, key): if not key: @@ -50,7 +82,63 @@ def _validate_type(self, key_type): ) return key_type + def _validate_issuer(self, section): + if not section: + raise ConfigurationError("Invalid 'issuer': 'issuer' must not be empty") + try: + nav_name = self.get_nav_name() + except ConfigurationError: + pass + else: + if section == nav_name: + raise ConfigurationError( + "Invalid 'issuer': %s collides with internal issuer name %s", + section, + ) + return section + def _validate_audience(self, audience): if not audience: raise ConfigurationError("Invalid 'aud': 'aud' must not be empty") return audience + + def _get_nav_token_config_option(self, option): + try: + get = partial(self.get, self.NAV_SECTION) + return get(option) + except ( + configparser.NoSectionError, + configparser.NoOptionError, + ) as error: + raise ConfigurationError(error) + + def get_nav_private_key(self): + path = self._get_nav_token_config_option('private_key') + return self._read_key_from_path(path) + + def get_nav_public_key(self): + path = self._get_nav_token_config_option('public_key') + return self._read_key_from_path(path) + + def get_nav_name(self): + name = self._get_nav_token_config_option('name') + if not name: + raise ConfigurationError("Invalid 'name': 'name' must not be empty") + return name + + def _get_settings_for_nav_issued_tokens(self): + if not self.has_section(self.NAV_SECTION): + return {} + name = self.get_nav_name() + claims_options = { + 'aud': {'values': [name], 'essential': True}, + 'token_type': {'values': ['access_token'], 'essential': True}, + } + settings = { + name: { + 'type': "PEM", + 'key': self.get_nav_public_key(), + 'claims_options': claims_options, + } + } + return settings diff --git a/tests/unittests/jwtconf_test.py b/tests/unittests/jwtconf_test.py index abd9114628..d79f50704c 100644 --- a/tests/unittests/jwtconf_test.py +++ b/tests/unittests/jwtconf_test.py @@ -1,4 +1,4 @@ -from mock import patch +from mock import patch, mock_open from unittest import TestCase from nav.jwtconf import JWTConf from nav.config import ConfigurationError @@ -8,7 +8,7 @@ class TestJWTConf(TestCase): def setUp(self): pass - def test_valid_jwks_config_should_pass(self): + def test_issuer_settings_include_valid_jwks_issuer(self): config = u""" [jwks-issuer] keytype=JWKS @@ -16,20 +16,23 @@ def test_valid_jwks_config_should_pass(self): key=www.example.com """ expected_settings = { - 'jwks-issuer': { - 'key': 'www.example.com', - 'type': 'JWKS', - 'claims_options': { - 'aud': {'values': ['nav'], 'essential': True}, - }, - } + 'key': 'www.example.com', + 'type': 'JWKS', + 'claims_options': { + 'aud': {'values': ['nav'], 'essential': True}, + }, } + + def read_file_patch(self, file): + return "key" + with patch.object(JWTConf, 'DEFAULT_CONFIG', config): - jwtconf = JWTConf() - settings = jwtconf.get_issuers_setting() - self.assertEqual(settings, expected_settings) + with patch.object(JWTConf, '_read_key_from_path', read_file_patch): + jwtconf = JWTConf() + settings = jwtconf.get_issuers_setting() + self.assertEqual(settings['jwks-issuer'], expected_settings) - def test_valid_pem_config_should_pass(self): + def test_issuer_settings_include_valid_pem_issuer(self): config = u""" [pem-issuer] keytype=PEM @@ -38,58 +41,80 @@ def test_valid_pem_config_should_pass(self): """ pem_key = "PEM KEY" expected_settings = { - 'pem-issuer': { - 'key': pem_key, - 'type': 'PEM', - 'claims_options': { - 'aud': {'values': ['nav'], 'essential': True}, - }, - } + 'key': pem_key, + 'type': 'PEM', + 'claims_options': { + 'aud': {'values': ['nav'], 'essential': True}, + }, } def read_file_patch(self, file): return pem_key with patch.object(JWTConf, 'DEFAULT_CONFIG', config): - with patch.object(JWTConf, '_read_file', read_file_patch): + with patch.object(JWTConf, '_read_key_from_path', read_file_patch): jwtconf = JWTConf() settings = jwtconf.get_issuers_setting() - self.assertEqual(settings, expected_settings) + self.assertEqual(settings['pem-issuer'], expected_settings) - def test_invalid_ketype_should_fail(self): + def test_issuer_settings_include_valid_local_issuer(self): config = u""" - [pem-issuer] - keytype=Fake - aud=nav - key=key + [nav] + private_key=key + public_key=key + name=nav """ + key = "PEM KEY" + expected_settings = { + 'key': key, + 'type': 'PEM', + 'claims_options': { + 'aud': {'values': ['nav'], 'essential': True}, + 'token_type': {'values': ['access_token'], 'essential': True}, + }, + } + + def read_file_patch(self, file): + return key + with patch.object(JWTConf, 'DEFAULT_CONFIG', config): - jwtconf = JWTConf() - settings = jwtconf.get_issuers_setting() - self.assertEqual(settings, dict()) + with patch.object(JWTConf, '_read_key_from_path', read_file_patch): + jwtconf = JWTConf() + settings = jwtconf.get_issuers_setting() + self.assertEqual(settings['nav'], expected_settings) - def test_empty_key_should_fail(self): + def test_invalid_config_for_internal_tokens_should_return_empty_dict(self): config = u""" - [pem-issuer] - keytype=JWKS - aud=nav - key= + [wrong-section-name] + private_key=key + public_key=key + name=nav-issuer """ + + def read_file_patch(self, file): + return "key" + with patch.object(JWTConf, 'DEFAULT_CONFIG', config): - jwtconf = JWTConf() - settings = jwtconf.get_issuers_setting() + with patch.object(JWTConf, '_read_key_from_path', read_file_patch): + jwtconf = JWTConf() + settings = jwtconf.get_issuers_setting() self.assertEqual(settings, dict()) - def test_empty_aud_should_fail(self): + def test_invalid_config_for_external_tokens_should_return_empty_dict(self): config = u""" [pem-issuer] - keytype=JWKS - aud= - key=key + keytype=INVALID + aud=nav + key=key_path """ + + def read_file_patch(self, file): + return "key" + with patch.object(JWTConf, 'DEFAULT_CONFIG', config): - jwtconf = JWTConf() - settings = jwtconf.get_issuers_setting() + with patch.object(JWTConf, '_read_key_from_path', read_file_patch): + jwtconf = JWTConf() + settings = jwtconf.get_issuers_setting() self.assertEqual(settings, dict()) def test_validate_key_should_raise_error_if_key_is_empty(self): @@ -130,3 +155,187 @@ def test_PEM_should_be_a_valid_type(self): jwtconf = JWTConf() validated_type = jwtconf._validate_type(type) self.assertEqual(validated_type, type) + + def test_validate_issuer_should_fail_if_external_name_matches_local_name(self): + config = u""" + [nav] + private_key=key + public_key=key + name=issuer-name + [issuer-name] + keytype=PEM + aud=aud + key=key + """ + key = "key_value" + + def read_file_patch(self, file): + return key + + with patch.object(JWTConf, 'DEFAULT_CONFIG', config): + with patch.object(JWTConf, '_read_key_from_path', read_file_patch): + jwtconf = JWTConf() + with self.assertRaises(ConfigurationError): + jwtconf._validate_issuer('issuer-name') + + def test_validate_issuer_should_raise_error_if_issuer_is_empty(self): + jwtconf = JWTConf() + with self.assertRaises(ConfigurationError): + jwtconf._validate_issuer("") + + def test_get_nav_private_key_returns_correct_private_key(self): + config = u""" + [nav] + private_key=key + public_key=key + name=issuer-name + """ + key = "private-key" + + def read_file_patch(self, file): + return key + + with patch.object(JWTConf, 'DEFAULT_CONFIG', config): + with patch.object(JWTConf, '_read_key_from_path', read_file_patch): + jwtconf = JWTConf() + self.assertEqual(jwtconf.get_nav_private_key(), key) + + def test_get_nav_public_key_returns_correct_public_key(self): + config = u""" + [nav] + private_key=key + public_key=key + name=issuer-name + """ + key = "private-key" + + def read_file_patch(self, file): + return key + + with patch.object(JWTConf, 'DEFAULT_CONFIG', config): + with patch.object(JWTConf, '_read_key_from_path', read_file_patch): + jwtconf = JWTConf() + self.assertEqual(jwtconf.get_nav_public_key(), key) + + def test_get_nav_name_should_raise_error_if_name_empty(self): + config = u""" + [nav] + private_key=key + public_key=key + name= + """ + with patch.object(JWTConf, 'DEFAULT_CONFIG', config): + jwtconf = JWTConf() + with self.assertRaises(ConfigurationError): + jwtconf.get_nav_name() + + def test_get_nav_name_returns_configured_name(self): + config = u""" + [nav] + private_key=key + public_key=key + name=nav + """ + with patch.object(JWTConf, 'DEFAULT_CONFIG', config): + jwtconf = JWTConf() + self.assertEqual(jwtconf.get_nav_name(), "nav") + + def test_missing_option_should_raise_error(self): + config_with_missing_keytype = u""" + [pem-issuer] + aud=nav + key=key_path + """ + + def read_file_patch(self, file): + return "key" + + with patch.object(JWTConf, 'DEFAULT_CONFIG', config_with_missing_keytype): + with patch.object(JWTConf, '_read_key_from_path', read_file_patch): + jwtconf = JWTConf() + with self.assertRaises(ConfigurationError): + jwtconf._get_settings_for_external_tokens() + + def test_non_existing_file_should_raise_error(self): + config = u""" + [pem-issuer] + aud=nav + key=key_path + """ + with patch.object(JWTConf, 'DEFAULT_CONFIG', config): + jwtconf = JWTConf() + with self.assertRaises(ConfigurationError): + jwtconf._read_key_from_path("fakepath") + + def test_return_correct_key_if_file_exists(self): + jwtconf = JWTConf() + mock_key = "key" + with patch("builtins.open", mock_open(read_data=mock_key)): + self.assertEqual(jwtconf._read_key_from_path("path"), mock_key) + + def test_file_with_permission_problems_should_raise_error(self): + config = u""" + [pem-issuer] + aud=nav + key=key_path + """ + with patch("builtins.open", mock_open(read_data="key")) as mocked_open: + mocked_open.side_effect = PermissionError + with patch.object(JWTConf, 'DEFAULT_CONFIG', config): + jwtconf = JWTConf() + with self.assertRaises(ConfigurationError): + jwtconf._read_key_from_path("fakepath") + + def test_empty_config_should_give_empty_issuer_settings(self): + config = u""" + """ + expected_settings = {} + with patch.object(JWTConf, 'DEFAULT_CONFIG', config): + jwtconf = JWTConf() + settings = jwtconf.get_issuers_setting() + self.assertEqual(settings, expected_settings) + + def test_empty_config_should_give_empty_external_settings(self): + config = u""" + """ + expected_settings = {} + with patch.object(JWTConf, 'DEFAULT_CONFIG', config): + jwtconf = JWTConf() + settings = jwtconf._get_settings_for_external_tokens() + self.assertEqual(settings, expected_settings) + + def test_empty_config_should_give_empty_local_settings(self): + config = u""" + """ + expected_settings = {} + with patch.object(JWTConf, 'DEFAULT_CONFIG', config): + jwtconf = JWTConf() + settings = jwtconf._get_settings_for_nav_issued_tokens() + self.assertEqual(settings, expected_settings) + + def test_settings_should_include_local_and_external_settings(self): + config = u""" + [nav] + private_key=key + public_key=key + name=local-issuer + [jwks-issuer] + keytype=JWKS + aud=nav + key=www.example.com + [pem-issuer] + keytype=PEM + aud=aud + key=key + """ + + def read_file_patch(self, file): + return "key" + + with patch.object(JWTConf, 'DEFAULT_CONFIG', config): + with patch.object(JWTConf, '_read_key_from_path', read_file_patch): + jwtconf = JWTConf() + settings = jwtconf.get_issuers_setting() + assert 'jwks-issuer' in settings + assert 'pem-issuer' in settings + assert 'local-issuer' in settings