Fixes pentest issue DG25-23 from 2025-09-02

crates/defguard_core/src/db/models/oauth2client.rs

@@ -1,5 +1,5 @@
 use model_derive::Model;
-use sqlx::{Error as SqlxError, PgPool, query_as};
+use sqlx::{Error as SqlxError, PgExecutor, PgPool, query_as};
 
 use super::NewOpenIDClient;
 use crate::{
@@ -55,20 +55,36 @@ impl OAuth2Client {
 
 impl OAuth2Client<Id> {
     /// Find client by 'client_id`.
-    pub(crate) async fn find_by_client_id(
-        pool: &PgPool,
+    pub(crate) async fn find_by_client_id<'e, E>(
+        executor: E,
         client_id: &str,
-    ) -> Result<Option<Self>, SqlxError> {
+    ) -> Result<Option<Self>, SqlxError>
+    where
+        E: PgExecutor<'e>,
+    {
         query_as!(
             Self,
             "SELECT id, client_id, client_secret, redirect_uri, scope, name, enabled \
             FROM oauth2client WHERE client_id = $1",
             client_id
         )
-        .fetch_optional(pool)
+        .fetch_optional(executor)
         .await
     }
 
+    pub(crate) async fn clear_authorizations<'e, E>(&self, executor: E) -> Result<(), SqlxError>
+    where
+        E: PgExecutor<'e>,
+    {
+        sqlx::query!(
+            "DELETE FROM oauth2authorizedapp WHERE oauth2client_id = $1",
+            self.id
+        )
+        .execute(executor)
+        .await?;
+        Ok(())
+    }
+
     /// Find using `client_id` and `client_secret`; must be `enabled`.
     pub(crate) async fn find_by_auth(
         pool: &PgPool,

crates/defguard_core/src/handlers/app_info.rs

@@ -1,8 +1,9 @@
 use axum::{extract::State, http::StatusCode};
 use serde_json::json;
 
-use super::{ApiResponse, ApiResult, VERSION};
+use super::{ApiResponse, ApiResult};
 use crate::{
+    VERSION,
     appstate::AppState,
     auth::SessionInfo,
     db::{Settings, WireguardNetwork},

crates/defguard_core/src/handlers/mod.rs

@@ -1,7 +1,7 @@
 use axum::{
     Json,
     extract::{FromRef, FromRequestParts},
-    http::{HeaderName, HeaderValue, StatusCode, request::Parts},
+    http::{StatusCode, request::Parts},
     response::{IntoResponse, Response},
 };
 use axum_client_ip::InsecureClientIp;
@@ -14,7 +14,6 @@ use webauthn_rs::prelude::RegisterPublicKeyCredential;
 #[cfg(feature = "wireguard")]
 use crate::db::Device;
 use crate::{
-    VERSION,
     appstate::AppState,
     auth::SessionInfo,
     db::{Id, NoId, User, UserInfo, WebHook},
@@ -219,10 +218,6 @@ impl IntoResponse for WebError {
 impl IntoResponse for ApiResponse {
     fn into_response(self) -> Response {
         let mut response = Json(self.json).into_response();
-        response.headers_mut().insert(
-            HeaderName::from_static("x-defguard-version"),
-            HeaderValue::from_static(VERSION),
-        );
         *response.status_mut() = self.status;
         response
     }

crates/defguard_core/src/handlers/openid_clients.rs

@@ -89,15 +89,20 @@ pub async fn change_openid_client(
         "User {} updating OpenID client {client_id}...",
         session.user.username
     );
-    let status = match OAuth2Client::find_by_client_id(&appstate.pool, &client_id).await? {
+    let mut transaction = appstate.pool.begin().await?;
+    let status = match OAuth2Client::find_by_client_id(&mut *transaction, &client_id).await? {
         Some(mut client) => {
             // store client before mods
             let before = client.clone();
             client.name = data.name;
             client.redirect_uri = data.redirect_uri;
             client.enabled = data.enabled;
             client.scope = data.scope;
-            client.save(&appstate.pool).await?;
+            client.save(&mut *transaction).await?;
+            if before.scope != client.scope {
+                client.clear_authorizations(&mut *transaction).await?;
+            }
+            transaction.commit().await?;
             info!(
                 "User {} updated OpenID client {client_id} ({})",
                 session.user.username, client.name

crates/defguard_core/src/handlers/pagination.rs

@@ -1,12 +1,11 @@
 use axum::{
     body::Body,
-    http::{HeaderName, HeaderValue},
     response::{IntoResponse, Response},
 };
 use reqwest::StatusCode;
 use serde::Serialize;
 
-use crate::{VERSION, error::WebError};
+use crate::error::WebError;
 
 /// Query params for paginated endpoints
 #[derive(Debug, Deserialize, Default)]
@@ -51,13 +50,6 @@ where
             }
         };
 
-        let mut response = Response::new(Body::from(json));
-
-        response.headers_mut().insert(
-            HeaderName::from_static("x-defguard-version"),
-            HeaderValue::from_static(VERSION),
-        );
-
-        response
+        Response::new(Body::from(json))
     }
 }

crates/defguard_core/tests/integration/api/openid.rs

@@ -610,6 +610,159 @@ async fn test_openid_authorization_code_with_pkce(_: PgPoolOptions, options: PgC
         .unwrap();
 }
 
+#[sqlx::test]
+async fn dg25_23_test_openid_client_scope_change_clears_authorizations(
+    _: PgPoolOptions,
+    options: PgConnectOptions,
+) {
+    let pool = setup_pool(options).await;
+    let (client, state) = make_client_with_state(pool).await;
+    let admin = User::find_by_username(&state.pool, "admin")
+        .await
+        .unwrap()
+        .unwrap();
+
+    // Authenticate admin
+    let auth = Auth::new("admin", "pass123");
+    let response = client.post("/api/v1/auth").json(&auth).send().await;
+    assert_eq!(response.status(), StatusCode::OK);
+
+    // Create OAuth2 client with initial scopes
+    let oauth2client = NewOpenIDClient {
+        name: "Test Client".into(),
+        redirect_uri: vec!["http://localhost:3000/".into()],
+        scope: vec!["openid".into(), "email".into()],
+        enabled: true,
+    };
+
+    let response = client
+        .post("/api/v1/oauth")
+        .json(&oauth2client)
+        .send()
+        .await;
+    assert_eq!(response.status(), StatusCode::CREATED);
+    let oauth2client: OAuth2Client<Id> = response.json().await;
+
+    // Authorize the client - simulate user authorization
+    let response = client
+        .post(format!(
+            "/api/v1/oauth/authorize?\
+            response_type=code&\
+            client_id={}&\
+            redirect_uri=http%3A%2F%2Flocalhost%3A3000&\
+            scope=openid email&\
+            state=ABCDEF&\
+            allow=true&\
+            nonce=blabla",
+            oauth2client.client_id
+        ))
+        .send()
+        .await;
+    assert_eq!(response.status(), StatusCode::FOUND);
+
+    // Verify that the authorization was created
+    use defguard_core::db::OAuth2AuthorizedApp;
+    let authorized_app = OAuth2AuthorizedApp::find_by_user_and_oauth2client_id(
+        &state.pool,
+        admin.id,
+        oauth2client.id,
+    )
+    .await
+    .unwrap();
+    assert!(
+        authorized_app.is_some(),
+        "Authorization should exist before scope change"
+    );
+
+    // Update the client with different scopes
+    let updated_client = NewOpenIDClient {
+        name: "Test Client".into(),
+        redirect_uri: vec!["http://localhost:3000/".into()],
+        scope: vec!["openid".into(), "profile".into()], // Changed from email to profile
+        enabled: true,
+    };
+
+    let response = client
+        .put(format!("/api/v1/oauth/{}", oauth2client.client_id))
+        .json(&updated_client)
+        .send()
+        .await;
+    assert_eq!(response.status(), StatusCode::OK);
+
+    // Verify that the authorization was cleared after scope change
+    let authorized_app_after = OAuth2AuthorizedApp::find_by_user_and_oauth2client_id(
+        &state.pool,
+        admin.id,
+        oauth2client.id,
+    )
+    .await
+    .unwrap();
+    assert!(
+        authorized_app_after.is_none(),
+        "Authorization should be cleared after scope change"
+    );
+
+    // Test that updating without scope changes does NOT clear authorizations
+
+    // Re-authorize the client
+    let response = client
+        .post(format!(
+            "/api/v1/oauth/authorize?\
+            response_type=code&\
+            client_id={}&\
+            redirect_uri=http%3A%2F%2Flocalhost%3A3000&\
+            scope=openid profile&\
+            state=ABCDEF2&\
+            allow=true&\
+            nonce=blabla2",
+            oauth2client.client_id
+        ))
+        .send()
+        .await;
+    assert_eq!(response.status(), StatusCode::FOUND);
+
+    // Verify authorization exists again
+    let authorized_app = OAuth2AuthorizedApp::find_by_user_and_oauth2client_id(
+        &state.pool,
+        admin.id,
+        oauth2client.id,
+    )
+    .await
+    .unwrap();
+    assert!(
+        authorized_app.is_some(),
+        "Authorization should exist after re-authorization"
+    );
+
+    // Update the client without changing scopes (only name)
+    let same_scope_update = NewOpenIDClient {
+        name: "Test Client Updated Name".into(),
+        redirect_uri: vec!["http://localhost:3000/".into()],
+        scope: vec!["openid".into(), "profile".into()], // Same scopes
+        enabled: true,
+    };
+
+    let response = client
+        .put(format!("/api/v1/oauth/{}", oauth2client.client_id))
+        .json(&same_scope_update)
+        .send()
+        .await;
+    assert_eq!(response.status(), StatusCode::OK);
+
+    // Verify that the authorization still exists when scopes haven't changed
+    let authorized_app_preserved = OAuth2AuthorizedApp::find_by_user_and_oauth2client_id(
+        &state.pool,
+        admin.id,
+        oauth2client.id,
+    )
+    .await
+    .unwrap();
+    assert!(
+        authorized_app_preserved.is_some(),
+        "Authorization should be preserved when scopes don't change"
+    );
+}
+
 #[sqlx::test]
 async fn dg25_22_test_respect_openid_scope_in_userinfo(
     _: PgPoolOptions,