From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [IPv6:2a01:7e0:0:424::9]) by lore.proxmox.com (Postfix) with ESMTPS id 7135F1FF15C for ; Fri, 17 Oct 2025 10:01:45 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id B8AE321A39; Fri, 17 Oct 2025 10:02:06 +0200 (CEST) Date: Fri, 17 Oct 2025 10:01:59 +0200 From: Fabian =?iso-8859-1?q?Gr=FCnbichler?= To: Proxmox Datacenter Manager development discussion References: <20251014133044.337162-1-s.sterz@proxmox.com> In-Reply-To: <20251014133044.337162-1-s.sterz@proxmox.com> MIME-Version: 1.0 User-Agent: astroid/0.17.0 (https://github.com/astroidmail/astroid) Message-Id: <1760687955.r4ebnztngk.astroid@yuna.none> X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1760688119133 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.048 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Subject: Re: [pdm-devel] [PATCH datacenter-manager/yew-comp 0/8] openid support for PDM X-BeenThere: pdm-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox Datacenter Manager development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: Proxmox Datacenter Manager development discussion Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: pdm-devel-bounces@lists.proxmox.com Sender: "pdm-devel" On October 14, 2025 3:30 pm, Shannon Sterz wrote: > this series adds openid support to PDM. the implementation is based on > PBS' implementation with a some adaptions: > > - smaller refactorings to use more data types instead of simply putting > them together with serde_json::json! > - move variables into format strings where possible > - only support the HttpOnly variant of the authentication flow > > when going through this i at first wanted to put most of the api > endpoints' logic into a proxmox-rs crate. however, i decided against > that as that would have created a couple of other problems. i'll outline > different options below and why i decided against them: > > - access-control: the login endpoint needs to be able to sign a ticket. > currently access-control does not have access to the keyring that > would be necessary for that. the keyring is available in auth-api, but > making it public there has possible other downsides. such as suddenly > making it very hard to audit which parts of our code have access to > the keyring through auth-api. the keyring is technically public there already (else PDM couldn't sign tickets using it as below ;)) - so I would not object in principle to also using it in access-control, it's not like that is some random crate that has no business being concerned with that. I do get the point (raised off-list) that access-control parts could be used in the front-end as well, but that can be solved by either splitting it further or using feature guards where appropriate.. the main benefit of having this in access-control would be at some point migrating PBS over as well, but that will surely cause more churn anyway, so as long as it is kept in mind that there is code duplication there (which is true for the whole auth stack anyway), it's not something that needs to be solved now IMHO. > - auth-api: the login endpoint would need access to the domains and user > configs. the first to setup the openid login against the correct host. > the latter for the auto-create feature when logging in users that have > no user information in the config yet. > the user config could be obtained by depending on access-control. > albeit, that would have required untangling some circular dependencies > between auth-api and access-control. the domain config, however, is > currently not in a proxmox-rs crate. so we would have needed to factor > that out first, which would create quite a bit of churn. > - a new crate/openid crate: this mostly combines the drawbacks of the > previous two options. so i discarded that as an option too. > > if we still want to move the code to a shared proxmox-rs crate, i can > revise this series. however, i think this is a sensible approach for > now. > > the series also includes adaptions for proxmox-yew-comp to adapt to > openid login flow and add some missing ui around default realms. > > Changelog > --------- > > the first two patches where taken from a different series [1] and slightly > adapted: > > - remove a useless log statement > - instead of referring to "openid authentication" correctly call it > "openid authorization" > - remove a useless sort() call > > [1]: https://lore.proxmox.com/all/20251008151936.386950-1-s.sterz@proxmox.com/ > > proxmox-yew-comp: > > Shannon Sterz (5): > login_panel/realm_selector: use default realm provided by api > login_panel/realm_selector: add support for openid realm logins > auth view: add openid icon to openid menu option > auth edit openid: add a default realm checkbox > utils/login panel: move openid redirection authorization helper to > utils > > src/auth_edit_openid.rs | 11 +- > src/auth_view.rs | 2 +- > src/login_panel.rs | 312 +++++++++++++++++++++++++++++++--------- > src/realm_selector.rs | 83 ++++++++++- > src/utils.rs | 32 +++++ > 5 files changed, 357 insertions(+), 83 deletions(-) > > > proxmox-datacenter-manager: > > Shannon Sterz (3): > api-types: add default field to openid realm config > server: api: add support for adding openid realms and openid logins > ui: enable openid realms in realm panel > > Cargo.toml | 2 +- > lib/pdm-api-types/src/openid.rs | 3 + > server/Cargo.toml | 1 + > server/src/api/access/mod.rs | 2 + > server/src/api/access/openid.rs | 311 +++++++++++++++++++++++++ > server/src/api/config/access/mod.rs | 2 + > server/src/api/config/access/openid.rs | 290 +++++++++++++++++++++++ > server/src/auth/mod.rs | 6 +- > ui/src/configuration/mod.rs | 1 + > 9 files changed, 616 insertions(+), 2 deletions(-) > create mode 100644 server/src/api/access/openid.rs > create mode 100644 server/src/api/config/access/openid.rs > > > Summary over all repositories: > 14 files changed, 973 insertions(+), 85 deletions(-) > > -- > Generated by git-murpp 0.8.1 > > > _______________________________________________ > pdm-devel mailing list > pdm-devel@lists.proxmox.com > https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel > > > _______________________________________________ pdm-devel mailing list pdm-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel