Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

introduce custom filenames for idp/sp cert/key #395

Merged
merged 2 commits into from
Oct 1, 2024
Merged
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
54 changes: 39 additions & 15 deletions src/onelogin/saml2/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,24 @@ def set_cert_path(self, path):
"""
self._paths['cert'] = path

def set_sp_cert_filename(self, filename):
"""
Set the filename of the SP certificate
"""
self._sp['cert_filename'] = filename

def set_sp_key_filename(self, filename):
"""
Set the filename of the SP key
"""
self._sp['key_filename'] = filename

def set_idp_cert_filename(self, filename):
"""
Set the filename of the idp certificate
"""
self._idp['cert_filename'] = filename

def get_lib_path(self):
"""
Returns lib path
Expand All @@ -214,26 +232,27 @@ def get_schemas_path(self):

def _load_settings_from_dict(self, settings):
"""
Loads settings info from a settings Dict
Loads settings info from a settings Dict, adds default values and validates the settings

:param settings: SAML Toolkit Settings
:type settings: dict

:returns: True if the settings info is valid
:rtype: boolean
"""
self._sp = settings.get('sp', {})
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Order should not be changed.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The order doesn't matter to the functionality, as the '_load_settings_from_dict' method is only called during initialization. To make the code easier I use the get_idp_cert() method to check if the config option or file actually exists and has content. Using the old order we would require way more code as the get_idp_cert() uses the data from the _idp dict (and so does the get_sp_cert() method)

self._idp = settings.get('idp', {})
self._strict = settings.get('strict', True)
self._debug = settings.get('debug', False)
self._security = settings.get('security', {})
self._contacts = settings.get('contactPerson', {})
self._organization = settings.get('organization', {})
self._add_default_values()

self._errors = []
errors = self.check_settings(settings)

if len(errors) == 0:
self._errors = []
self._sp = settings['sp']
self._idp = settings.get('idp', {})
self._strict = settings.get('strict', True)
self._debug = settings.get('debug', False)
self._security = settings.get('security', {})
self._contacts = settings.get('contactPerson', {})
self._organization = settings.get('organization', {})

self._add_default_values()
return True

self._errors = errors
Expand Down Expand Up @@ -328,6 +347,11 @@ def _add_default_values(self):
self._sp.setdefault('x509cert', '')
self._sp.setdefault('privateKey', '')

# Set the default filenames for the certificates and keys
self._idp.setdefault('cert_filename', 'idp.crt')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Better inject them in the _add_default_values method

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But it is already part of the _add_default_values method

self._sp.setdefault('cert_filename', 'sp.crt')
self._sp.setdefault('key_filename', 'sp.key')

self._security.setdefault('requestedAuthnContext', True)
self._security.setdefault('requestedAuthnContextComparison', 'exact')
self._security.setdefault('failOnAuthnContextMismatch', False)
Expand Down Expand Up @@ -389,7 +413,7 @@ def check_idp_settings(self, settings):
if 'security' in settings:
security = settings['security']

exists_x509 = bool(idp.get('x509cert'))
exists_x509 = bool(self.get_idp_cert())
exists_fingerprint = bool(idp.get('certFingerprint'))

exists_multix509sign = 'x509certMulti' in idp and \
Expand Down Expand Up @@ -566,7 +590,7 @@ def get_sp_key(self):
:rtype: string or None
"""
key = self._sp.get('privateKey')
key_file_name = self._paths['cert'] + 'sp.key'
key_file_name = self._paths['cert'] + self._sp['key_filename']

if not key and exists(key_file_name):
with open(key_file_name) as f:
Expand All @@ -581,7 +605,7 @@ def get_sp_cert(self):
:rtype: string or None
"""
cert = self._sp.get('x509cert')
cert_file_name = self._paths['cert'] + 'sp.crt'
cert_file_name = self._paths['cert'] + self._sp['cert_filename']

if not cert and exists(cert_file_name):
with open(cert_file_name) as f:
Expand Down Expand Up @@ -612,7 +636,7 @@ def get_idp_cert(self):
:rtype: string
"""
cert = self._idp.get('x509cert')
cert_file_name = self.get_cert_path() + 'idp.crt'
cert_file_name = self.get_cert_path() + self._idp['cert_filename']
if not cert and exists(cert_file_name):
with open(cert_file_name) as f:
cert = f.read()
Expand Down
16 changes: 16 additions & 0 deletions tests/data/customPath/certs/Test_Root_CA.crt
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
----- Begin Certificate -----
MIICgTCCAeoCCQCbOlrWDdX7FTANBgkqhkiG9w0BAQUFADCBhDELMAkGA1UEBhM
CTk8xGDAWBgNVBAgTD0FuZHJlYXMgU29sYmVyZzEMMAoGA1UEBxMDRm9vMRAwDg
YDVQQKEwdVTklORVRUMRgwFgYDVQQDEw9mZWlkZS5lcmxhbmcubm8xITAfBgkqh
kiG9w0BCQEWEmFuZHJlYXNAdW5pbmV0dC5ubzAeFw0wNzA2MTUxMjAxMzVaFw0w
NzA4MTQxMjAxMzVaMIGEMQswCQYDVQQGEwJOTzEYMBYGA1UECBMPQW5kcmVhcyB
Tb2xiZXJnMQwwCgYDVQQHEwNGb28xEDAOBgNVBAoTB1VOSU5FVFQxGDAWBgNVBA
MTD2ZlaWRlLmVybGFuZy5ubzEhMB8GCSqGSIb3DQEJARYSYW5kcmVhc0B1bmluZ
XR0Lm5vMIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKBgQDivbhR7P516x/S3BqK
xupQe0LONoliupiBOesCO3SHbDrl3+q9IbfnfmE04rNuMcPsIxB161TdDpIesLC
n7c8aPHISKOtPlAeTZSnb8QAu7aRjZq3+PbrP5uW3TcfCGPtKTytHOge/OlJbo0
78dVhXQ14d1EDwXJW1rRXuUt4C8QIDAQABMA0GCSqGSIb3DQEBBQUAA4GBACDVf
p86HObqY+e8BUoWQ9+VMQx1ASDohBjwOsg2WykUqRXF+dLfcUH9dWR63CtZIKFD
bStNomPnQz7nbK+onygwBspVEbnHuUihZq3ZUdmumQqCw4Uvs/1Uvq3orOo/WJV
hTyvLgFVK2QarQ4/67OZfHd7R+POBXhophSMv1ZOo
-----END CERTIFICATE-----
19 changes: 15 additions & 4 deletions tests/src/OneLogin/saml2_tests/settings_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -95,12 +95,23 @@ def testLoadSettingsFromDict(self):
except Exception as e:
self.assertIn("Invalid dict settings: sp_cert_not_found_and_required", str(e))

# test if the cert-file is loaded correct with the default filename
settings_info = self.loadSettingsJSON()
settings_info["security"]["nameIdEncrypted"] = True
del settings_info["idp"]["x509cert"]
settings_info['security']['nameIdEncrypted'] = True
del settings_info['idp']['x509cert']
settings_7 = OneLogin_Saml2_Settings(settings_info)
self.assertEqual(len(settings_7.get_errors()), 0)

# test if the cert-file is loaded correct with a custom filename
settings_info['idp']['cert_filename'] = "Test_Root_CA.crt"
settings_8 = OneLogin_Saml2_Settings(settings_info)
self.assertEqual(len(settings_8.get_errors()), 0)

# test for the correct error, if there is no cert at all
settings_info['idp']['cert_filename'] = "not_existing_file.crt"
try:
settings_7 = OneLogin_Saml2_Settings(settings_info)
self.assertNotEqual(len(settings_7.get_errors()), 0)
settings_9 = OneLogin_Saml2_Settings(settings_info)
self.assertNotEqual(len(settings_9.get_errors()), 0)
except Exception as e:
self.assertIn("Invalid dict settings: idp_cert_not_found_and_required", str(e))

Expand Down
Loading