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

[Gabriel Goller]

On Mon Feb 5, 2024 at 4:45 PM CET, Thomas Lamprecht wrote:
> Am 01/02/2024 um 15:24 schrieb Gabriel Goller:
> > [..]
> 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.
In the commit messages or as a comment above the regex?
> 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)
Hmm AFAIK on pve we don't have any limitation at all, it just has to be
a string. It's probably best if I copy the same regex to pve although I
don't want to suddenly have user input rejected.
The pve regex would get stricter, thus it would be a breaking change.
> > 
> > [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 at 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.
Yes, fair point.