Re: [pmg-devel] [PATCH multiple 0/7] PMG TFA support

[Stoiko Ivanov <s.ivanov@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
> 
>