public inbox for pmg-devel@lists.proxmox.com
 help / color / mirror / Atom feed
From: Stoiko Ivanov <s.ivanov@proxmox.com>
To: Wolfgang Bumiller <w.bumiller@proxmox.com>
Cc: pmg-devel@lists.proxmox.com
Subject: Re: [pmg-devel] [PATCH multiple 0/7] PMG TFA support
Date: Fri, 26 Nov 2021 18:34:48 +0100	[thread overview]
Message-ID: <20211126183448.75d4e5ed@rosa.proxmox.com> (raw)
In-Reply-To: <20211126135524.117846-1-w.bumiller@proxmox.com>

Huge thanks for your work on this!

* gave it a quick spin on a freshly setup PMG from ISO (by adding a user
  and setting up TOTP for it) - works as advertised
* quickly glanced over the code (e.g. diffing API2/TFA.pm between PVE
  and PMG) - sent a mail with 2 minor notes to one of the patches
* will do a more thorough review on Monday

Thanks again!


On Fri, 26 Nov 2021 14:55:04 +0100
Wolfgang Bumiller <w.bumiller@proxmox.com> wrote:

> This touches multiple repos as it required some more ground-work on the
> rust side:
> 
> 1) proxmox-tfa
>    Aside from fixups and maintenance, patch 4 is the important one:
>    The `origin` in the webauthn configuration is now *optional*.
> 
>    Note that the origin is generally required for webauthn, however, we
>    also have clusters where the origin shouldn't be pinned cluster-wide.
> 
>    This does not really affect PVE as there we store the webauthn
>    configuration separately and apply it only when it is used, but in
>    PBS it's kept directly in tfa.json, and PMG for now does this too,
>    although we *could* move it to pmg.conf or some other synced file if
>    we wanted?
>    That would in theory remove the need for this, but I think this is
>    actually a more appropriate API anyway, since the two other parts of
>    the config stay the same across a cluster, and the origin can simply
>    be provided as an overriding parameter to the methods which actually
>    make use of it.
> 
> 2) proxmox-perl-rs
>    pmg-rs is now moved into here, also, this contains fixups for the
>    proxmox-tfa-crate-using pve-side.
>    Since the newly introduced parameters are at the end and optional,
>    and perlmod 0.9 supports trailing Option<> parameters as actual
>    *optional* parameters, this may in theory even be API compatible with
>    PVE, so hopefully no `Breaks` on old pve-access-control is required,
>    but we'll see.
Option<> at the end translating to optional parameters in perl sounds very
nice! (thinking about replacing the set_proxy method for my ACME-proxy
series)


> 
> 3) pmg-api
>    Same login & TFA api updates as in PVE. The config API path is
>    different, but that's not shared code anyway ;-)
>    API2/TFA.pm is very similar to PVE, I think I got the method schemas
>    wright, but I'm not used to the permissions system in PMG so please
>    double-check this.
>    The actual changes to the login code path is much shorter than in PVE
>    since we did not actually have TFA support in there yet.
> 
> 4) pmg-gui
>    For now this only adds TFA login and the `TfaView` from WTK. The
>    config (which in this case only means webauthn settings) part isn't
>    there yet.
> 
> 
> _______________________________________________
> pmg-devel mailing list
> pmg-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel
> 
> 





  parent reply	other threads:[~2021-11-26 17:35 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-11-26 13:55 Wolfgang Bumiller
2021-11-26 13:55 ` [pmg-devel] [PATCH api 1/6] add tfa.json and its lock methods Wolfgang Bumiller
2021-11-26 13:55 ` [pmg-devel] [PATCH api 2/6] add PMG::TFAConfig module Wolfgang Bumiller
2021-11-26 13:55 ` [pmg-devel] [PATCH api 3/6] add TFA API Wolfgang Bumiller
2021-11-26 17:29   ` Stoiko Ivanov
2021-11-26 13:55 ` [pmg-devel] [PATCH api 4/6] add tfa config api Wolfgang Bumiller
2021-11-26 13:55 ` [pmg-devel] [PATCH api 5/6] implement tfa authentication Wolfgang Bumiller
2021-11-26 13:55 ` [pmg-devel] [PATCH api 6/6] provide qrcode.min.js from libjs-qrcodejs Wolfgang Bumiller
2021-11-26 13:55 ` [pmg-devel] [PATCH gui] add TFA components Wolfgang Bumiller
2021-11-26 13:55 ` [pmg-devel] [PATCH perl-rs 1/7] pve: bump perlmod to 0.9 Wolfgang Bumiller
2021-11-26 13:55 ` [pmg-devel] [PATCH perl-rs 2/7] pve: update to proxmox-tfa 2.0 Wolfgang Bumiller
2021-11-26 13:55 ` [pmg-devel] [PATCH perl-rs 3/7] pve: bump d/control Wolfgang Bumiller
2021-11-26 13:55 ` [pmg-devel] [PATCH perl-rs 4/7] import pmg-rs Wolfgang Bumiller
2021-11-26 13:55 ` [pmg-devel] [PATCH perl-rs 5/7] pmg: bump perlmod to 0.9 Wolfgang Bumiller
2021-11-26 13:55 ` [pmg-devel] [PATCH perl-rs 6/7] pmg: add tfa module Wolfgang Bumiller
2021-11-26 13:55 ` [pmg-devel] [PATCH perl-rs 7/7] pmg: bump d/control Wolfgang Bumiller
2021-11-26 13:55 ` [pmg-devel] [PATCH proxmox 1/6] tfa: fix typo in docs Wolfgang Bumiller
2021-11-26 13:55 ` [pmg-devel] [PATCH proxmox 2/6] tfa: add WebauthnConfig::digest method Wolfgang Bumiller
2021-11-26 13:55 ` [pmg-devel] [PATCH proxmox 3/6] tfa: let OriginUrl deref to its inner Url, add FromStr impl Wolfgang Bumiller
2021-11-26 13:55 ` [pmg-devel] [PATCH proxmox 4/6] tfa: make configured webauthn origin optional Wolfgang Bumiller
2021-11-26 13:55 ` [pmg-devel] [PATCH proxmox 5/6] tfa: clippy fixes Wolfgang Bumiller
2021-11-26 13:55 ` [pmg-devel] [PATCH proxmox 6/6] bump proxmox-tfa to 2.0.0-1 Wolfgang Bumiller
2021-11-26 17:34 ` Stoiko Ivanov [this message]
2021-11-28 21:17 ` [pmg-devel] applied-series: Re: [PATCH multiple 0/7] PMG TFA support 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=20211126183448.75d4e5ed@rosa.proxmox.com \
    --to=s.ivanov@proxmox.com \
    --cc=pmg-devel@lists.proxmox.com \
    --cc=w.bumiller@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal