commit eccb3ab9474f5088eb74cd606806153bdd463575
parent 3246251f295fd16dba21ec9d32afbbb104bd9c8e
Author: Mathijs van Veluw <black.dex@gmail.com>
Date: Wed, 13 Dec 2023 17:49:35 +0100
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
Diffstat:
3 files changed, 53 insertions(+), 25 deletions(-)
diff --git 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
@@ -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
@@ -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,