From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gate001.proxmox.com (gate001.proxmox.com [IPv6:2a0f:8001:1:32::40]) by lore.proxmox.com (Postfix) with ESMTPS id AC0C11FF0AD for ; Fri, 21 Aug 2026 12:21:55 +0200 (CEST) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id E8F7821530; Fri, 21 Aug 2026 12:21:54 +0200 (CEST) From: Thomas Ellmenreich To: pdm-devel@lists.proxmox.com Subject: [PATCH datacenter-manager 1/1] fix #7135: openid auth: improve error logging Date: Fri, 21 Aug 2026 12:21:47 +0200 Message-ID: <20260821102147.220586-1-t.ellmenreich@proxmox.com> X-Mailer: git-send-email 2.47.3 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1787307684334 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.704 Adjusted score from AWL reputation of From: address DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment (newer systems) RCVD_IN_DNSWL_MED -2.3 Sender listed at https://www.dnswl.org/, medium trust SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Message-ID-Hash: VZXJURDQLGKMJHQYNU2WKHTZTQAQRYZN X-Message-ID-Hash: VZXJURDQLGKMJHQYNU2WKHTZTQAQRYZN X-MailFrom: t.ellmenreich@proxmox.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header CC: Thomas Ellmenreich X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox Datacenter Manager development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: Improve logging when getting the authorization URL fails. Previously, the details of the error were completly swallowed, making it difficult to diagnose problems connecting to the OpenID Connect Server. The new log statement prints out the produced error with all of the contexts provided to `anyhow`, instead of just the one on top of the stack. Doing so should make OpenID connection errors easier to debug. Fixes: https://bugzilla.proxmox.com/show_bug.cgi?id=7135 Signed-off-by: Thomas Ellmenreich Reviewed-by: Nicolas Frey --- This patch fixes this: [1] Bugzilla issue, although I'm not completely sure on the implementation. In the issues discussion, an improvement of the the error sent to the client is proposed, but I don't think that to be the correct approach. To avoid sharing unnecessary information with the client [2], I instead recommend logging the failed retrieval of an authorization URL on the server, especially since, in the case of this issue, only an admin during setup should need this information. That said, the error logged with this current implementation can be very verbose, as seen in the example below, although I still find it useful to understand the state of things. Options Explored ---------------- - I considered only providing more information to the client for certain error cases, but our use of the Anyhow crate makes such a solution a bigger intervention, which I did not find adequate. - Rather than the current solution, I thought there might be a way to automatically log an error via the api macro. I looked around, but I could not find anything similar. How I tested: ------------- I spun up a Keycloak instance through Docker and added it as an OpenId Connect Server to my PDM instance. Since I knowingly misconfigured it with https instead of http, the attempt to login then logged the following error (after the patch): ``` # Line breaks added for readability ... proxmox-datacenter-privileged-api[616]: could not get opneid auth url: Request failed: ureq request failed - native-tls: error:0A00010B:SSL routines:tls_validate_record_header:wrong version number: ../ssl/record/methods/tlsany_meth.c:77:: native-tls: error:0A00010B:SSL routines:tls_validate_record_header:wrong version number: ../ssl/record/methods/tlsany_meth.c:77: ``` Changelog --------- * Since RFC (thanks @Nicolas) + Fixing of typos + Better explanation in the commit message [1]: https://bugzilla.proxmox.com/show_bug.cgi?id=7135 [2]: https://cheatsheetseries.owasp.org/cheatsheets/Error_Handling_Cheat_Sheet.html server/src/api/access/openid.rs | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/server/src/api/access/openid.rs b/server/src/api/access/openid.rs index 5048fde3..725e1d30 100644 --- a/server/src/api/access/openid.rs +++ b/server/src/api/access/openid.rs @@ -271,14 +271,16 @@ pub fn openid_auth_url( redirect_url: String, _rpcenv: &mut dyn RpcEnvironment, ) -> Result { - let (domains, _digest) = pdm_config::domains::config()?; - let config: OpenIdRealmConfig = domains.lookup("openid", &realm)?; + let url_result: Result = try_block!({ + let (domains, _digest) = pdm_config::domains::config()?; + let config: OpenIdRealmConfig = domains.lookup("openid", &realm)?; - let open_id = openid_authenticator(&config, &redirect_url)?; + let open_id = openid_authenticator(&config, &redirect_url)?; - let url = open_id.authorize_url(PDM_RUN_DIR_M!(), &realm)?; + open_id.authorize_url(PDM_RUN_DIR_M!(), &realm) + }); - Ok(url) + url_result.inspect_err(|err| log::error!("could not get openid auth url: {err:#}")) } #[sortable] -- 2.47.3