Re: [pbs-devel] [PATCH proxmox-backup] fix #5190: api-types: openid acr format regex

[Thomas Lamprecht <t.lamprecht@proxmox.com>]

Am 01/02/2024 um 15:24 schrieb Gabriel Goller:
> Allow more complex strings for the acr-value when using openid. The
> openid documentation only specifies the acr-value *should* be an URI [0].
> Implemented a regex that loosely disallows some of the reserved URI
> characters specified in the RFC [1].
> A value like "urn:mace:incommon:iap:silver" should work.

on it's own the change itself looks relatively OK, what I'm missing is
a more direct reference to what the issue was in the report, as while
your example closely matches that, it would be still great to actually
mention this explicitly.

The other thing is that the reporter writes that it works fine for
Proxmox VE already, so what's the limitation there, does your patch
aligns it to that, and if not it would great to state why you chose
a different approach (which can be fine, but really should be reasoned
about)

> 
> [0]: https://openid.net/specs/openid-connect-core-1_0.html
> [1]: https://www.rfc-editor.org/rfc/rfc2396.txt
> 
> Signed-off-by: Gabriel Goller <g.goller@proxmox.com>
> ---
>  pbs-api-types/src/lib.rs    | 3 +++
>  pbs-api-types/src/openid.rs | 5 +++--
>  2 files changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/pbs-api-types/src/lib.rs b/pbs-api-types/src/lib.rs
> index 795ff2a6..2668db3e 100644
> --- a/pbs-api-types/src/lib.rs
> +++ b/pbs-api-types/src/lib.rs
> @@ -178,6 +178,9 @@ const_regex! {
>      /// any identifier command line tools work with.
>      pub PROXMOX_SAFE_ID_REGEX = concat!(r"^", PROXMOX_SAFE_ID_REGEX_STR!(), r"$");
>  
> +    /// Regex that (loosely) matches URIs according to [RFC 2396](https://www.rfc-editor.org/rfc/rfc2396.txt)
> +    pub URI_REGEX = r#"^[^\x00-\x1F\x7F <>#"]*$"#;

Could be also good to expand on the "loosely" part, maybe also go
for a name like `GENERIC_URI_REGEX`, which might better signal that
this is not what some have with URI (often mistaken as URL) in mind.