From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <pve-devel-bounces@lists.proxmox.com>
Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68])
	by lore.proxmox.com (Postfix) with ESMTPS id 6E0801FF15C
	for <inbox@lore.proxmox.com>; Wed, 13 Nov 2024 13:46:23 +0100 (CET)
Received: from firstgate.proxmox.com (localhost [127.0.0.1])
	by firstgate.proxmox.com (Proxmox) with ESMTP id 51ACB151BF;
	Wed, 13 Nov 2024 13:46:21 +0100 (CET)
Date: Wed, 13 Nov 2024 13:46:14 +0100
From: Fabian =?iso-8859-1?q?Gr=FCnbichler?= <f.gruenbichler@proxmox.com>
To: Proxmox VE development discussion <pve-devel@lists.proxmox.com>
References: <20240901165512.687801-1-thomas@atskinner.net>
 <20240901165512.687801-3-thomas@atskinner.net>
In-Reply-To: <20240901165512.687801-3-thomas@atskinner.net>
MIME-Version: 1.0
User-Agent: astroid/0.16.0 (https://github.com/astroidmail/astroid)
Message-Id: <1731501469.ghld71gk9w.astroid@yuna.none>
X-SPAM-LEVEL: Spam detection results:  0
 AWL 0.047 Adjusted score from AWL reputation of From: address
 BAYES_00                 -1.9 Bayes spam probability is 0 to 1%
 DMARC_MISSING             0.1 Missing DMARC policy
 KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment
 SPF_HELO_NONE           0.001 SPF: HELO does not publish an SPF Record
 SPF_PASS               -0.001 SPF: sender matches SPF record
Subject: Re: [pve-devel] [PATCH openid 1/1] fix #4411: openid: add library
 code for openid groups support
X-BeenThere: pve-devel@lists.proxmox.com
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: Proxmox VE development discussion <pve-devel.lists.proxmox.com>
List-Unsubscribe: <https://lists.proxmox.com/cgi-bin/mailman/options/pve-devel>, 
 <mailto:pve-devel-request@lists.proxmox.com?subject=unsubscribe>
List-Archive: <http://lists.proxmox.com/pipermail/pve-devel/>
List-Post: <mailto:pve-devel@lists.proxmox.com>
List-Help: <mailto:pve-devel-request@lists.proxmox.com?subject=help>
List-Subscribe: <https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel>, 
 <mailto:pve-devel-request@lists.proxmox.com?subject=subscribe>
Reply-To: Proxmox VE development discussion <pve-devel@lists.proxmox.com>
Cc: Thomas Skinner <thomas@atskinner.net>
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Errors-To: pve-devel-bounces@lists.proxmox.com
Sender: "pve-devel" <pve-devel-bounces@lists.proxmox.com>

this change actually does a lot more, right? it returns *all* claims,
and also doesn't do anything groups specific at all, so the patch
subject is not quite correct..

I haven't tested this yet, but since all it does is allow arbitrary
values instead of just empty ones, it should be fine. definitely would
require some testing with different IdP implementations to ensure no
regressions..

On September 1, 2024 6:55 pm, Thomas Skinner wrote:
> Signed-off-by: Thomas Skinner <thomas@atskinner.net>
> ---
>  proxmox-openid/src/lib.rs | 55 +++++++++++++++++++++++++++++++++------
>  1 file changed, 47 insertions(+), 8 deletions(-)
> 
> diff --git a/proxmox-openid/src/lib.rs b/proxmox-openid/src/lib.rs
> index fe65fded..bf8c650b 100644
> --- a/proxmox-openid/src/lib.rs
> +++ b/proxmox-openid/src/lib.rs
> @@ -15,8 +15,11 @@ pub use auth_state::*;
>  use openidconnect::{
>      //curl::http_client,
>      core::{
> -        CoreAuthDisplay, CoreAuthPrompt, CoreAuthenticationFlow, CoreClient, CoreGenderClaim,
> -        CoreIdTokenClaims, CoreIdTokenVerifier, CoreProviderMetadata,
> +        CoreAuthDisplay, CoreAuthPrompt, CoreAuthenticationFlow, CoreErrorResponseType,
> +        CoreGenderClaim, CoreIdTokenVerifier, CoreJsonWebKey, CoreJsonWebKeyType,
> +        CoreJsonWebKeyUse, CoreJweContentEncryptionAlgorithm, CoreJwsSigningAlgorithm,
> +        CoreProviderMetadata, CoreRevocableToken, CoreRevocationErrorResponse,
> +        CoreTokenIntrospectionResponse, CoreTokenType,
>      },
>      AdditionalClaims,
>      AuthenticationContextClass,
> @@ -24,6 +27,9 @@ use openidconnect::{
>      ClientId,
>      ClientSecret,
>      CsrfToken,
> +    EmptyExtraTokenFields,
> +    IdTokenClaims,
> +    IdTokenFields,
>      IssuerUrl,
>      Nonce,
>      OAuth2TokenResponse,
> @@ -31,15 +37,47 @@ use openidconnect::{
>      PkceCodeVerifier,
>      RedirectUrl,
>      Scope,
> +    StandardErrorResponse,
> +    StandardTokenResponse,
>      UserInfoClaims,
>  };
>  
>  /// Stores Additional Claims into a serde_json::Value;
> -#[derive(Debug, Deserialize, Serialize)]
> +#[derive(Clone, Debug, Default, Deserialize, PartialEq, Serialize)]
>  pub struct GenericClaims(Value);
>  impl AdditionalClaims for GenericClaims {}
>  
>  pub type GenericUserInfoClaims = UserInfoClaims<GenericClaims, CoreGenderClaim>;
> +pub type GenericIdTokenClaims = IdTokenClaims<GenericClaims, CoreGenderClaim>;
> +
> +pub type GenericIdTokenFields = IdTokenFields<
> +    GenericClaims,
> +    EmptyExtraTokenFields,
> +    CoreGenderClaim,
> +    CoreJweContentEncryptionAlgorithm,
> +    CoreJwsSigningAlgorithm,
> +    CoreJsonWebKeyType,
> +>;
> +
> +pub type GenericTokenResponse = StandardTokenResponse<GenericIdTokenFields, CoreTokenType>;
> +
> +pub type GenericClient = openidconnect::Client<
> +    GenericClaims,
> +    CoreAuthDisplay,
> +    CoreGenderClaim,
> +    CoreJweContentEncryptionAlgorithm,
> +    CoreJwsSigningAlgorithm,
> +    CoreJsonWebKeyType,
> +    CoreJsonWebKeyUse,
> +    CoreJsonWebKey,
> +    CoreAuthPrompt,
> +    StandardErrorResponse<CoreErrorResponseType>,
> +    GenericTokenResponse,
> +    CoreTokenType,
> +    CoreTokenIntrospectionResponse,
> +    CoreRevocableToken,
> +    CoreRevocationErrorResponse,
> +>;
>  
>  #[derive(Debug, Deserialize, Serialize, Clone)]
>  pub struct OpenIdConfig {
> @@ -56,7 +94,7 @@ pub struct OpenIdConfig {
>  }
>  
>  pub struct OpenIdAuthenticator {
> -    client: CoreClient,
> +    client: GenericClient,
>      config: OpenIdConfig,
>  }
>  
> @@ -120,8 +158,9 @@ impl OpenIdAuthenticator {
>  
>          let provider_metadata = CoreProviderMetadata::discover(&issuer_url, http_client)?;
>  
> -        let client = CoreClient::from_provider_metadata(provider_metadata, client_id, client_key)
> -            .set_redirect_uri(RedirectUrl::new(String::from(redirect_url))?);
> +        let client =
> +            GenericClient::from_provider_metadata(provider_metadata, client_id, client_key)
> +                .set_redirect_uri(RedirectUrl::new(String::from(redirect_url))?);
>  
>          Ok(Self {
>              client,
> @@ -195,7 +234,7 @@ impl OpenIdAuthenticator {
>          &self,
>          code: &str,
>          private_auth_state: &PrivateAuthState,
> -    ) -> Result<(CoreIdTokenClaims, GenericUserInfoClaims), Error> {
> +    ) -> Result<(GenericIdTokenClaims, GenericUserInfoClaims), Error> {
>          let code = AuthorizationCode::new(code.to_string());
>          // Exchange the code with a token.
>          let token_response = self
> @@ -206,7 +245,7 @@ impl OpenIdAuthenticator {
>              .map_err(|err| format_err!("Failed to contact token endpoint: {}", err))?;
>  
>          let id_token_verifier: CoreIdTokenVerifier = self.client.id_token_verifier();
> -        let id_token_claims: &CoreIdTokenClaims = token_response
> +        let id_token_claims: &GenericIdTokenClaims = token_response
>              .extra_fields()
>              .id_token()
>              .expect("Server did not return an ID token")
> -- 
> 2.39.2
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
> 
> 
> 


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel