* [PATCH datacenter-manager 1/1] fix #7135: openid auth: improve error logging
@ 2026-08-21 10:21 Thomas Ellmenreich
2026-08-21 11:35 ` applied: " Lukas Wagner
0 siblings, 1 reply; 2+ messages in thread
From: Thomas Ellmenreich @ 2026-08-21 10:21 UTC (permalink / raw)
To: pdm-devel; +Cc: Thomas Ellmenreich
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 <t.ellmenreich@proxmox.com>
Reviewed-by: Nicolas Frey <n.frey@proxmox.com>
---
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<String, Error> {
- let (domains, _digest) = pdm_config::domains::config()?;
- let config: OpenIdRealmConfig = domains.lookup("openid", &realm)?;
+ let url_result: Result<String, Error> = 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
^ permalink raw reply related [flat|nested] 2+ messages in thread
* applied: [PATCH datacenter-manager 1/1] fix #7135: openid auth: improve error logging
2026-08-21 10:21 [PATCH datacenter-manager 1/1] fix #7135: openid auth: improve error logging Thomas Ellmenreich
@ 2026-08-21 11:35 ` Lukas Wagner
0 siblings, 0 replies; 2+ messages in thread
From: Lukas Wagner @ 2026-08-21 11:35 UTC (permalink / raw)
To: pdm-devel, Thomas Ellmenreich
On Fri, 21 Aug 2026 12:21:47 +0200, Thomas Ellmenreich wrote:
> 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.
>
> [...]
Applied, thanks for the patch!
I think the way you implemented this makes sense. Having the error with the
entire context in the system logs should make troubleshooting way easier. We
could still consider returning more details in the API response later, if we
desire to, the current change does not limit us from doing so.
It appears that PBS suffers from the same issue, could you maybe send a patch
for there as well? (cc @Chris)
[1/1] fix #7135: openid auth: improve error logging
commit: e1cd6869fbe448e0d58e6fbc3f0f7a740c0a8dd6
Best regards,
--
Lukas Wagner <l.wagner@proxmox.com>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2026-08-21 11:40 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-21 10:21 [PATCH datacenter-manager 1/1] fix #7135: openid auth: improve error logging Thomas Ellmenreich
2026-08-21 11:35 ` applied: " Lukas Wagner
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.