From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) by lore.proxmox.com (Postfix) with ESMTPS id 0E4E71FF139 for ; Tue, 27 Jan 2026 19:47:13 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 0F1A92E53; Tue, 27 Jan 2026 19:47:35 +0100 (CET) Message-ID: <6a990c3a-32c6-4caa-90b3-3fdd3e645e99@proxmox.com> Date: Tue, 27 Jan 2026 19:47:30 +0100 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Beta To: Proxmox VE development discussion , Dominik Rusovac References: <20251218124210.10483-1-d.rusovac@proxmox.com> <20251218124210.10483-3-d.rusovac@proxmox.com> Content-Language: en-US From: Thomas Lamprecht In-Reply-To: <20251218124210.10483-3-d.rusovac@proxmox.com> X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1769539585482 X-SPAM-LEVEL: Spam detection results: 0 AWL -0.022 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 RCVD_IN_VALIDITY_CERTIFIED_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_RPBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_SAFE_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Subject: Re: [pve-devel] [PATCH pve-manager 1/1] fix #7149: ui: prohibit realm update on login X-BeenThere: pve-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox VE development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: Proxmox VE development discussion Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Errors-To: pve-devel-bounces@lists.proxmox.com Sender: "pve-devel" stumbled upon this when writing the debian/changelog for a new package version bump: subjet is rather misleading, this does not update anything on login, a side-effect of this change is that the pre-selected realm saved in the browser state, which is then also selected on login. But after this patch, a realm update is still not prohibited for when a user changes the realm on login, unlike your subject suggests, and that is a good thing, as doing so would introduce a regression. A better subject would have been: "ui: user create: do not change pre-selected realm in browser state" or: "ui: realm selector: do no leak changes on user create to local users default" or: "ui: do not set realm selected on user create as default realm of logged-in user" plus prefixing the "fix #..." is naturally fine in our current documented style. Just to give a few example of what I would prefer due to being much clearer in what this commit does, there isn't one right style On 18/12/2025 13:42, Dominik Rusovac wrote: > The realm selection during user creation is no longer stateful. Hence, Wording the commit message in this form makes it sound like the realm selection was changed to not be stateful in a *previous* commit and that caused some regression or the like that this commit fixes. Rather just use the present tense, as a commit changes (from its own POV) something in its present, past things is rather useful to describe the status quo before this commit, or other commits that came before it in the commit log chain. > as desired, adding a new user does not affect the realm on the login > page. Besides some potential language improvements I'd find something like: "Make the authentication realm selector from the user creation dialogue stateless. This avoids odd effects for the logged in user, as we use that browser-local saved state for the pre-selected realm on a fresh login. So if, e.g., a @pam user created and @pve user, on the next login that @pam user suddenly had the @pve realm pre-selected." more telling. Noting his all in such detail because git commit messages are quite important, and while they most often are really fine to be relatively short, they definitively should not be confusing and suggest different changes that what's in them. That can throw one of during checking the git log, bisecting or also when writing changelogs and not looking closely, and then that might leak from that even in the release nodes and proliferate even to various other parts (nowadays some LLM might even pick it up and invent some further BS on top of it...). Anyhow, should have been caught on review, so no fault from your side, but please try to keep this somewhat in mind for future commits/patches. > Signed-off-by: Dominik Rusovac > --- > www/manager6/dc/UserEdit.js | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/www/manager6/dc/UserEdit.js b/www/manager6/dc/UserEdit.js > index 7541816b..84d9cfa8 100644 > --- a/www/manager6/dc/UserEdit.js > +++ b/www/manager6/dc/UserEdit.js > @@ -99,6 +99,7 @@ Ext.define('PVE.dc.UserEdit', { > column1.splice(1, 0, { > xtype: 'pmxRealmComboBox', > name: 'realm', > + stateful: false, // realm is not saved between page reloads > fieldLabel: gettext('Realm'), > allowBlank: false, > matchFieldWidth: false, _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel