diff options
author | Mathijs van Veluw <[email protected]> | 2023-12-13 17:49:35 +0100 |
---|---|---|
committer | GitHub <[email protected]> | 2023-12-13 17:49:35 +0100 |
commit | eccb3ab9474f5088eb74cd606806153bdd463575 (patch) | |
tree | c7801fe292934ab8c75e184727d034691f536258 | |
parent | 3246251f295fd16dba21ec9d32afbbb104bd9c8e (diff) | |
download | vaultwarden-eccb3ab9474f5088eb74cd606806153bdd463575.tar.gz vaultwarden-eccb3ab9474f5088eb74cd606806153bdd463575.zip |
Decrease JWT Refresh/Auth token (#4163)
Large JWT's could cause issue because of header or body sizes of the
HTTP request could get too large when you are a member of a lot of organizations.
This PR removes these specific keys since they are not used either
client side or server side.
Because Bitwarden does add these in there JWT's i would suggest to keep
the code we had but then commented out as a reference.
Removing it and searching for this when needed would be a waist of time.
Fixes #4156
-rw-r--r-- | src/api/identity.rs | 27 | ||||
-rw-r--r-- | src/auth.rs | 14 | ||||
-rw-r--r-- | src/db/models/device.rs | 37 |
3 files changed, 53 insertions, 25 deletions
diff --git a/src/api/identity.rs b/src/api/identity.rs index e5611610..13e7ab43 100644 --- a/src/api/identity.rs +++ b/src/api/identity.rs @@ -103,8 +103,13 @@ async fn _refresh_login(data: ConnectData, conn: &mut DbConn) -> JsonResult { // Common let user = User::find_by_uuid(&device.user_uuid, conn).await.unwrap(); - let orgs = UserOrganization::find_confirmed_by_user(&user.uuid, conn).await; - let (access_token, expires_in) = device.refresh_tokens(&user, orgs, scope_vec); + // --- + // Disabled this variable, it was used to generate the JWT + // Because this might get used in the future, and is add by the Bitwarden Server, lets keep it, but then commented out + // See: https://github.com/dani-garcia/vaultwarden/issues/4156 + // --- + // let orgs = UserOrganization::find_confirmed_by_user(&user.uuid, conn).await; + let (access_token, expires_in) = device.refresh_tokens(&user, scope_vec); device.save(conn).await?; let result = json!({ @@ -260,8 +265,13 @@ async fn _password_login( } // Common - let orgs = UserOrganization::find_confirmed_by_user(&user.uuid, conn).await; - let (access_token, expires_in) = device.refresh_tokens(&user, orgs, scope_vec); + // --- + // Disabled this variable, it was used to generate the JWT + // Because this might get used in the future, and is add by the Bitwarden Server, lets keep it, but then commented out + // See: https://github.com/dani-garcia/vaultwarden/issues/4156 + // --- + // let orgs = UserOrganization::find_confirmed_by_user(&user.uuid, conn).await; + let (access_token, expires_in) = device.refresh_tokens(&user, scope_vec); device.save(conn).await?; let mut result = json!({ @@ -374,8 +384,13 @@ async fn _user_api_key_login( // Common let scope_vec = vec!["api".into()]; - let orgs = UserOrganization::find_confirmed_by_user(&user.uuid, conn).await; - let (access_token, expires_in) = device.refresh_tokens(&user, orgs, scope_vec); + // --- + // Disabled this variable, it was used to generate the JWT + // Because this might get used in the future, and is add by the Bitwarden Server, lets keep it, but then commented out + // See: https://github.com/dani-garcia/vaultwarden/issues/4156 + // --- + // let orgs = UserOrganization::find_confirmed_by_user(&user.uuid, conn).await; + let (access_token, expires_in) = device.refresh_tokens(&user, scope_vec); device.save(conn).await?; info!("User {} logged in successfully via API key. IP: {}", user.email, ip.ip); diff --git a/src/auth.rs b/src/auth.rs index e23aa32d..85b6359e 100644 --- a/src/auth.rs +++ b/src/auth.rs @@ -119,10 +119,16 @@ pub struct LoginJwtClaims { pub email: String, pub email_verified: bool, - pub orgowner: Vec<String>, - pub orgadmin: Vec<String>, - pub orguser: Vec<String>, - pub orgmanager: Vec<String>, + // --- + // Disabled these keys to be added to the JWT since they could cause the JWT to get too large + // Also These key/value pairs are not used anywhere by either Vaultwarden or Bitwarden Clients + // Because these might get used in the future, and they are added by the Bitwarden Server, lets keep it, but then commented out + // See: https://github.com/dani-garcia/vaultwarden/issues/4156 + // --- + // pub orgowner: Vec<String>, + // pub orgadmin: Vec<String>, + // pub orguser: Vec<String>, + // pub orgmanager: Vec<String>, // user security_stamp pub sstamp: String, diff --git a/src/db/models/device.rs b/src/db/models/device.rs index b80b47a1..00fe0ee6 100644 --- a/src/db/models/device.rs +++ b/src/db/models/device.rs @@ -59,12 +59,7 @@ impl Device { self.twofactor_remember = None; } - pub fn refresh_tokens( - &mut self, - user: &super::User, - orgs: Vec<super::UserOrganization>, - scope: Vec<String>, - ) -> (String, i64) { + pub fn refresh_tokens(&mut self, user: &super::User, scope: Vec<String>) -> (String, i64) { // If there is no refresh token, we create one if self.refresh_token.is_empty() { use data_encoding::BASE64URL; @@ -75,10 +70,17 @@ impl Device { let time_now = Utc::now().naive_utc(); self.updated_at = time_now; - let orgowner: Vec<_> = orgs.iter().filter(|o| o.atype == 0).map(|o| o.org_uuid.clone()).collect(); - let orgadmin: Vec<_> = orgs.iter().filter(|o| o.atype == 1).map(|o| o.org_uuid.clone()).collect(); - let orguser: Vec<_> = orgs.iter().filter(|o| o.atype == 2).map(|o| o.org_uuid.clone()).collect(); - let orgmanager: Vec<_> = orgs.iter().filter(|o| o.atype == 3).map(|o| o.org_uuid.clone()).collect(); + // --- + // Disabled these keys to be added to the JWT since they could cause the JWT to get too large + // Also These key/value pairs are not used anywhere by either Vaultwarden or Bitwarden Clients + // Because these might get used in the future, and they are added by the Bitwarden Server, lets keep it, but then commented out + // --- + // fn arg: orgs: Vec<super::UserOrganization>, + // --- + // let orgowner: Vec<_> = orgs.iter().filter(|o| o.atype == 0).map(|o| o.org_uuid.clone()).collect(); + // let orgadmin: Vec<_> = orgs.iter().filter(|o| o.atype == 1).map(|o| o.org_uuid.clone()).collect(); + // let orguser: Vec<_> = orgs.iter().filter(|o| o.atype == 2).map(|o| o.org_uuid.clone()).collect(); + // let orgmanager: Vec<_> = orgs.iter().filter(|o| o.atype == 3).map(|o| o.org_uuid.clone()).collect(); // Create the JWT claims struct, to send to the client use crate::auth::{encode_jwt, LoginJwtClaims, DEFAULT_VALIDITY, JWT_LOGIN_ISSUER}; @@ -93,11 +95,16 @@ impl Device { email: user.email.clone(), email_verified: !CONFIG.mail_enabled() || user.verified_at.is_some(), - orgowner, - orgadmin, - orguser, - orgmanager, - + // --- + // Disabled these keys to be added to the JWT since they could cause the JWT to get too large + // Also These key/value pairs are not used anywhere by either Vaultwarden or Bitwarden Clients + // Because these might get used in the future, and they are added by the Bitwarden Server, lets keep it, but then commented out + // See: https://github.com/dani-garcia/vaultwarden/issues/4156 + // --- + // orgowner, + // orgadmin, + // orguser, + // orgmanager, sstamp: user.security_stamp.clone(), device: self.uuid.clone(), scope, |