[pve-devel] [PATCH proxmox v2 5/5] fix #4234: openid: add library functions for optional userinfo endpoint

[Thomas Skinner]

On Fri, Jan 24, 2025 at 3:17 AM Fabian Grünbichler
<f.gruenbichler at proxmox.com> wrote:
>
> On December 16, 2024 5:14 am, Thomas Skinner wrote:
> > Signed-off-by: Thomas Skinner <thomas at atskinner.net>
> > ---
> >  proxmox-openid/src/lib.rs | 30 +++++++++++++++++++++++++++++-
> >  1 file changed, 29 insertions(+), 1 deletion(-)
> >
> > diff --git a/proxmox-openid/src/lib.rs b/proxmox-openid/src/lib.rs
> > index fe65fded..87be1c8a 100644
> > --- a/proxmox-openid/src/lib.rs
> > +++ b/proxmox-openid/src/lib.rs
> > @@ -31,6 +31,7 @@ use openidconnect::{
> >      PkceCodeVerifier,
> >      RedirectUrl,
> >      Scope,
> > +    StandardClaims,
> >      UserInfoClaims,
> >  };
> >
> > @@ -195,6 +196,15 @@ impl OpenIdAuthenticator {
> >          &self,
> >          code: &str,
> >          private_auth_state: &PrivateAuthState,
> > +    ) -> Result<(CoreIdTokenClaims, GenericUserInfoClaims), Error> {
> > +        self.verify_authorization_code_userinfo(code, private_auth_state, true)
>
> this default here is the wrong way round (to preserve the old behaviour,
> we should pass in `false`).

Good catch! I missed this in my debugging. Will get this fixed in v3.

> > +    }
> > +
> > +    pub fn verify_authorization_code_userinfo(
> > +        &self,
> > +        code: &str,
> > +        private_auth_state: &PrivateAuthState,
> > +        disable_userinfo: bool,
> >      ) -> Result<(CoreIdTokenClaims, GenericUserInfoClaims), Error> {
> >          let code = AuthorizationCode::new(code.to_string());
> >          // Exchange the code with a token.
> > @@ -213,6 +223,14 @@ impl OpenIdAuthenticator {
> >              .claims(&id_token_verifier, &private_auth_state.nonce)
> >              .map_err(|err| format_err!("Failed to verify ID token: {}", err))?;
> >
> > +        if disable_userinfo {
> > +            let empty_userinfo_claims = UserInfoClaims::new(
> > +                StandardClaims::new(id_token_claims.subject().clone()),
> > +                GenericClaims(Value::Null),
> > +            );
> > +            return Ok((id_token_claims.clone(), empty_userinfo_claims));
> > +        }
> > +
> >          let userinfo_claims: GenericUserInfoClaims = self
> >              .client
> >              .user_info(token_response.access_token().to_owned(), None)?
> > @@ -227,9 +245,19 @@ impl OpenIdAuthenticator {
> >          &self,
> >          code: &str,
> >          private_auth_state: &PrivateAuthState,
> > +    ) -> Result<Value, Error> {
> > +        self.verify_authorization_code_simple_userinfo(code, private_auth_state, true)
>
> same here

Ditto above.

> > +    }
> > +
> > +    /// Like verify_authorization_code_simple_userinfo(), but returns claims as serde_json::Value
> > +    pub fn verify_authorization_code_simple_userinfo(
> > +        &self,
> > +        code: &str,
> > +        private_auth_state: &PrivateAuthState,
> > +        disable_userinfo: bool,
> >      ) -> Result<Value, Error> {
> >          let (id_token_claims, userinfo_claims) =
> > -            self.verify_authorization_code(code, private_auth_state)?;
> > +            self.verify_authorization_code_userinfo(code, private_auth_state, disable_userinfo)?;
> >
> >          let mut data = serde_json::to_value(id_token_claims)?;
> >
> > --
> > 2.39.5
> >
> >
> > _______________________________________________
> > pve-devel mailing list
> > pve-devel at lists.proxmox.com
> > https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
> >
> >
> >
>