Skip to content

Fetch IDP metadata using requests to support custom server certificates root CAs#415

Merged
pitbulk merged 1 commit intoSAML-Toolkits:masterfrom
maykinmedia:issue/403-support-self-signed-certificates
Oct 4, 2024
Merged

Fetch IDP metadata using requests to support custom server certificates root CAs#415
pitbulk merged 1 commit intoSAML-Toolkits:masterfrom
maykinmedia:issue/403-support-self-signed-certificates

Conversation

@sergei-maertens
Copy link
Copy Markdown
Contributor

Closes #403

Using requests allows us to easily customize the CA_BUNDLE to use when verifying the server certificate, instead of having to disable SSL certificate verification alltogether.

@sergei-maertens
Copy link
Copy Markdown
Contributor Author

@pitbulk I didn't realize that requests is not a dependency yet, but IMO this is the easiest way to support this. Happy to discuss alternatives though, if you feel that adding the library is too much.

@pitbulk
Copy link
Copy Markdown
Contributor

pitbulk commented Jul 10, 2024

@sergei-maertens if I'm not wrong, urllib.request.urlopen accepts cafile and capath parameters, now that we are forcing in the new release to use python3 > 3.7, you can adapt your PR to keep using urllib.request.urlopen and accept in the method those new parameters.

Test it and let me know if works as well as requests.

@sergei-maertens
Copy link
Copy Markdown
Contributor Author

sergei-maertens commented Jul 11, 2024

I'll look into it asap, thanks for the feedback!

edit: what would your approach be to provide the cafile/capath parameters. An envvar, or something in the settings dict? I'm leaning towards the former. not relevant, doesn't seem like this code is called interally in many places

@pitbulk
Copy link
Copy Markdown
Contributor

pitbulk commented Oct 1, 2024

@sergei-maertens do you plan to rework on the PR?

@sergei-maertens
Copy link
Copy Markdown
Contributor Author

I had completely forgotten about it, sorry! I can still pick it up

When retrieving the IDP metadata, you can now optionally specify the the
capath or cafile to use for certificate verification, rather than just
enabling/disabling it.

This allows TLS verification of server certificates that are not in the
system root store (such as when using private CAs).
@sergei-maertens
Copy link
Copy Markdown
Contributor Author

@pitbulk I've updated the PR with the suggested changes, now only the stdlib is used :)

@pitbulk pitbulk merged commit 7eb6b63 into SAML-Toolkits:master Oct 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

OneLogin_Saml2_IdPMetadataParser.get_metadata not compatible with self-signed certificates

2 participants