Respect the Qgis SSL configuration on trusted certificates and trusted root CA configured in settings#870
Conversation
trusted root CA configured in settings
|
Hi @lleirborras thanks for PR, we will take a look at this. |
wonder-sk
left a comment
wonder-sk
left a comment
There was a problem hiding this comment.
Thanks for your contribution!
Nice idea to use QGIS trusted CA certs - I thought QGIS API provided only the custom ones added by the user, but it actually returns all of them, so we don't need to deal with truststore module...
| auth_manager = QgsApplication.authManager() | ||
| ca_certs = auth_manager.trustedCaCertsCache() | ||
|
|
||
| if not ca_certs: | ||
| return | ||
|
|
||
| # Convert QSslCertificate objects to PEM text | ||
| pem_blocks = [] | ||
| for cert in ca_certs: | ||
| pem_data = bytes(cert.toPem()).decode("ascii") | ||
| if pem_data: | ||
| pem_blocks.append(pem_data) | ||
|
|
||
| if not pem_blocks: | ||
| return | ||
|
|
||
| qgis_ca_pem = "\n".join(pem_blocks) |
There was a problem hiding this comment.
I believe this could be replaced by
QgsApplication.authManager().trustedCaCertsPemText()| # 2. On macOS: patch MerginClient's bundled cert.pem so the fallback | ||
| # code path (which ignores SSL_CERT_FILE) also trusts QGIS CAs. | ||
| if sys.platform == "darwin": | ||
| plugin_dir = os.path.dirname(os.path.realpath(__file__)) | ||
| bundled_cert = os.path.join(plugin_dir, "mergin", "cert.pem") | ||
| if os.path.exists(bundled_cert): | ||
| marker = "# --- QGIS trusted CAs ---" | ||
| with open(bundled_cert, "r") as f: | ||
| existing = f.read() | ||
| if marker not in existing: | ||
| with open(bundled_cert, "a") as f: | ||
| f.write(f"\n{marker}\n") | ||
| f.write(qgis_ca_pem) |
There was a problem hiding this comment.
I would prefer not to have existing files in the plugin's directory modified. Additionally, there is an issue that if the QGIS trusted CAs have been written once, they won't get updated if some certs are manually added/removed later.
My preference would be this: in the mergin python module, we should add a new module-wide function - e.g. mergin.set_trusted_certificates() which could have a file path to certificates file as the only argument. Then, whenever MerginClient is created, its internal urllib configuration would be set to use that (and in case there is no certificates file provided, it would fall back to the existing CA cert handling https://github.com/MerginMaps/python-api-client/blob/master/mergin/client.py#L174). Hopefully that should be cleaner, without two different levels of mac-specific handling... What do you think?
| try: | ||
| setup_qgis_ssl_for_mergin_client() | ||
| except Exception: | ||
| pass |
There was a problem hiding this comment.
It would be better to move this code to initGui() rather than running it immediately when the module is imported. Also, can we avoid except Exception: pass so that we do not silence potentially serious issues?
3 participants
Also, fixes #820