Skip to content

Commit

Permalink
Decrease JWT Refresh/Auth token (#4163)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
BlackDex authored Dec 13, 2023
1 parent 3246251 commit eccb3ab
Show file tree
Hide file tree
Showing 3 changed files with 53 additions and 25 deletions.
27 changes: 21 additions & 6 deletions src/api/identity.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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!({
Expand Down Expand Up @@ -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!({
Expand Down Expand Up @@ -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);
Expand Down
14 changes: 10 additions & 4 deletions src/auth.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
37 changes: 22 additions & 15 deletions src/db/models/device.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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};
Expand All @@ -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,
Expand Down

0 comments on commit eccb3ab

Please sign in to comment.