all lists on lists.proxmox.com
 help / color / mirror / Atom feed
From: Lukas Wagner <l.wagner@proxmox.com>
To: Proxmox Backup Server development discussion
	<pbs-devel@lists.proxmox.com>,
	Christoph Heiss <c.heiss@proxmox.com>
Subject: Re: [pbs-devel] [PATCH proxmox{, -backup} v5 00/11] fix #5379: introduce default auth realm option
Date: Fri, 4 Apr 2025 15:34:14 +0200	[thread overview]
Message-ID: <121326f3-bbdf-46b7-9a86-cc4a9225ea91@proxmox.com> (raw)
In-Reply-To: <20250321134541.1106117-1-c.heiss@proxmox.com>

On  2025-03-21 14:45, Christoph Heiss wrote:
> Fixes #5379 [0].
> 
> First, it adds an updatable `default` field to all existing editable
> realms. Then it converts the PAM and PBS built-in realms to proper
> realms, instead of being hard-coded in-between somewhere. 
> In turns this enables editing of these realms, allowing setting whether
> these realms should be the default for login or not.
> 
> For patch #3 onwards, proxmox-backup needs patches #1 & #2 applied to
> pbs-api-types and a bump thereof.
> 
> The proxmox-widget-toolkit parts have already been applied [1] and
> proxmox-backup also pulls in the required version (introduced with pwt
> 4.3.1, proxmox-backup pulls in >= 4.3.3).
> 
> W.r.t. the inconsistency as discovered/discussed in [2], the (current)
> behaviour is not changed in this series. Since both PVE and PBS use the
> same realm login dialog from proxmox-widget-toolkit, I'd rather fix it
> separately -- to avoid blocking this series on a completely separate
> issue, which might still need some discussing.
> 
> [0] https://bugzilla.proxmox.com/show_bug.cgi?id=5379
> [1] https://lore.proxmox.com/pbs-devel/d56c6e30-61d7-452b-afaa-5215d8538b4e@proxmox.com/#t
> [2] https://lists.proxmox.com/pipermail/pbs-devel/2024-August/010429.html
> 

Gave this a quick test on the lastest master branches. Setting a default realm
works as expected. The statefulness of the login window even if a default
is configured [your 2] still feels odd to me, but as you have said, this is consistent with
PVE and can be changed in a separate series, if we decide that we want that.

Only skimmed over the code since I already did a review of v1;
looks good to me so far. The nits that Shannon raised can be fixed in v6 or in a followup.

Reviewed-by: Lukas Wagner <l.wagner@proxmox.com>
Tested-by: Lukas Wagner <l.wagner@proxmox.com>

-- 
- Lukas



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel


  parent reply	other threads:[~2025-04-04 13:35 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-03-21 13:45 Christoph Heiss
2025-03-21 13:45 ` [pbs-devel] [PATCH proxmox v5 1/2] fix #5379: api-types: add `default` field for all realm types Christoph Heiss
2025-03-21 16:04   ` Shannon Sterz
2025-03-24  9:44     ` Christoph Heiss
2025-03-21 13:45 ` [pbs-devel] [PATCH proxmox v5 2/2] api-types: introduce proper types for PAM and PBS realms Christoph Heiss
2025-03-21 13:45 ` [pbs-devel] [PATCH proxmox-backup v5 03/11] fix #5379: api2: access: add `default` property for all realm types Christoph Heiss
2025-03-21 13:45 ` [pbs-devel] [PATCH proxmox-backup v5 04/11] fix #5379: api2: access: set default realm accordingly on individual update Christoph Heiss
2025-03-21 13:45 ` [pbs-devel] [PATCH proxmox-backup v5 05/11] config: use new dedicated PAM and PBS realm types Christoph Heiss
2025-03-21 13:45 ` [pbs-devel] [PATCH proxmox-backup v5 06/11] api2: access: add update support for built-in PAM realm Christoph Heiss
2025-03-21 13:45 ` [pbs-devel] [PATCH proxmox-backup v5 07/11] api2: access: add update support for built-in PBS realm Christoph Heiss
2025-03-21 13:45 ` [pbs-devel] [PATCH proxmox-backup v5 08/11] www: AccessControl: make `useTypeInUrl` property per-realm Christoph Heiss
2025-03-21 13:45 ` [pbs-devel] [PATCH proxmox-backup v5 09/11] www: AccessControl: enable default realm checkbox for all realms Christoph Heiss
2025-03-21 13:45 ` [pbs-devel] [PATCH proxmox-backup v5 10/11] www: utils: make built-in PBS realm editable using new AuthSimplePanel Christoph Heiss
2025-03-21 13:45 ` [pbs-devel] [PATCH proxmox-backup v5 11/11] docs: user-management: document `pam` and `pbs` authentication realm Christoph Heiss
2025-04-04 13:34 ` Lukas Wagner [this message]
2025-04-05 17:12 ` [pbs-devel] applied-series: [PATCH proxmox{, -backup} v5 00/11] fix #5379: introduce default auth realm option Thomas Lamprecht

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=121326f3-bbdf-46b7-9a86-cc4a9225ea91@proxmox.com \
    --to=l.wagner@proxmox.com \
    --cc=c.heiss@proxmox.com \
    --cc=pbs-devel@lists.proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal