Skip to content

Commit

Permalink
Refactor LoginMethod & AuthSettings (#241)
Browse files Browse the repository at this point in the history
Refactor `LoginMethod` to contain details from `AuthSettings`. I believe this more accurately represents the available state for Service Accounts, Organization keys and other login methods like TDE where KDF aren't applicable.

To facilitate these the following changes were made:

- `request_prelogin` is now directly called instead of implicitly in `determine_password_hash`.
- Since `AuthSettings` is removed, the helper for `derive_user_password_hash` is gone, and the master key needs to be manually created. I believe this moves us in a direction where we avoid re-building it constantly.
  • Loading branch information
Hinton authored Oct 3, 2023
1 parent caf3521 commit db2236c
Show file tree
Hide file tree
Showing 23 changed files with 227 additions and 219 deletions.
14 changes: 0 additions & 14 deletions crates/bitwarden-napi/src-ts/bitwarden_client/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import {
ClientSettings,
Convert,
ResponseForAPIKeyLoginResponse,
ResponseForPasswordLoginResponse,
ResponseForSecretIdentifiersResponse,
ResponseForSecretResponse,
ResponseForSecretsDeleteResponse,
Expand All @@ -19,19 +18,6 @@ export class BitwardenClient {
this.client = new rust.BitwardenClient(settingsJson, loggingLevel ?? LogLevel.Info);
}

async login(email: string, password: string): Promise<ResponseForPasswordLoginResponse> {
const response = await this.client.runCommand(
Convert.commandToJson({
passwordLogin: {
email: email,
password: password,
},
}),
);

return Convert.toResponseForPasswordLoginResponse(response);
}

async loginWithAccessToken(accessToken: string): Promise<ResponseForAPIKeyLoginResponse> {
const commandInput = Convert.commandToJson({
accessTokenLogin: {
Expand Down
2 changes: 1 addition & 1 deletion crates/bitwarden-uniffi/src/auth/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use std::sync::Arc;

use bitwarden::{
auth::{password::MasterPasswordPolicyOptions, RegisterKeyResponse},
client::auth_settings::Kdf,
client::kdf::Kdf,
};

use crate::{error::Result, Client};
Expand Down
2 changes: 1 addition & 1 deletion crates/bitwarden-uniffi/src/docs.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use bitwarden::{
auth::password::MasterPasswordPolicyOptions,
client::auth_settings::Kdf,
client::kdf::Kdf,
mobile::crypto::InitCryptoRequest,
tool::{ExportFormat, PassphraseGeneratorRequest, PasswordGeneratorRequest},
vault::{Cipher, CipherView, Collection, Folder, FolderView, Send, SendListView, SendView},
Expand Down
2 changes: 1 addition & 1 deletion crates/bitwarden/src/auth/client_auth.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use super::{
register::{make_register_keys, register},
RegisterKeyResponse, RegisterRequest,
};
use crate::{client::auth_settings::Kdf, error::Result, Client};
use crate::{client::kdf::Kdf, error::Result, Client};

pub struct ClientAuth<'a> {
pub(crate) client: &'a mut crate::Client,
Expand Down
6 changes: 3 additions & 3 deletions crates/bitwarden/src/auth/login/access_token.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use crate::{
api::{request::AccessTokenRequest, response::IdentityTokenResponse},
login::{response::two_factor::TwoFactorProviders, PasswordLoginResponse},
},
client::{AccessToken, LoginMethod},
client::{AccessToken, LoginMethod, ServiceAccountLoginMethod},
crypto::{EncString, SymmetricCryptoKey},
error::{Error, Result},
util::{decode_token, BASE64_ENGINE},
Expand Down Expand Up @@ -59,11 +59,11 @@ pub(crate) async fn access_token_login(
r.access_token.clone(),
r.refresh_token.clone(),
r.expires_in,
LoginMethod::AccessToken {
LoginMethod::ServiceAccount(ServiceAccountLoginMethod::AccessToken {
service_account_id: access_token.service_account_id,
client_secret: access_token.client_secret,
organization_id,
},
}),
);

client.initialize_crypto_single_key(encryption_key);
Expand Down
31 changes: 15 additions & 16 deletions crates/bitwarden/src/auth/login/api_key.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,9 @@ use serde::{Deserialize, Serialize};
use crate::{
auth::{
api::{request::ApiTokenRequest, response::IdentityTokenResponse},
login::{
determine_password_hash, response::two_factor::TwoFactorProviders,
PasswordLoginResponse,
},
login::{response::two_factor::TwoFactorProviders, PasswordLoginResponse},
},
client::LoginMethod,
client::{LoginMethod, UserLoginMethod},
crypto::EncString,
error::{Error, Result},
util::decode_token,
Expand All @@ -28,24 +25,26 @@ pub(crate) async fn api_key_login(
let response = request_api_identity_tokens(client, input).await?;

if let IdentityTokenResponse::Authenticated(r) = &response {
client.set_tokens(
r.access_token.clone(),
r.refresh_token.clone(),
r.expires_in,
LoginMethod::ApiKey {
client_id: input.client_id.to_owned(),
client_secret: input.client_secret.to_owned(),
},
);

let access_token_obj = decode_token(&r.access_token)?;

// This should always be Some() when logging in with an api key
let email = access_token_obj
.email
.ok_or(Error::Internal("Access token doesn't contain email"))?;

let _ = determine_password_hash(client, &email, &input.password).await?;
let kdf = client.prelogin(email.clone()).await?;

client.set_tokens(
r.access_token.clone(),
r.refresh_token.clone(),
r.expires_in,
LoginMethod::User(UserLoginMethod::ApiKey {
client_id: input.client_id.to_owned(),
client_secret: input.client_secret.to_owned(),
email,
kdf,
}),
);

let user_key = EncString::from_str(r.key.as_deref().unwrap()).unwrap();
let private_key = EncString::from_str(r.private_key.as_deref().unwrap()).unwrap();
Expand Down
21 changes: 9 additions & 12 deletions crates/bitwarden/src/auth/login/mod.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
#[cfg(feature = "internal")]
use {
crate::{
client::{auth_settings::AuthSettings, Client},
client::{kdf::Kdf, Client},
error::Result,
},
bitwarden_api_identity::{
Expand Down Expand Up @@ -40,21 +40,18 @@ pub(crate) use access_token::access_token_login;
pub use access_token::{AccessTokenLoginRequest, AccessTokenLoginResponse};

#[cfg(feature = "internal")]
async fn determine_password_hash(
client: &mut Client,
email: &str,
password: &str,
) -> Result<String> {
let pre_login = request_prelogin(client, email.to_owned()).await?;
let auth_settings = AuthSettings::new(pre_login, email.to_owned());
let password_hash = auth_settings.derive_user_password_hash(password)?;
client.set_auth_settings(auth_settings);
async fn determine_password_hash(email: &str, kdf: &Kdf, password: &str) -> Result<String> {
use crate::crypto::{HashPurpose, MasterKey};

Ok(password_hash)
let master_key = MasterKey::derive(password.as_bytes(), email.as_bytes(), kdf)?;
master_key.derive_master_key_hash(password.as_bytes(), HashPurpose::ServerAuthorization)
}

#[cfg(feature = "internal")]
async fn request_prelogin(client: &mut Client, email: String) -> Result<PreloginResponseModel> {
pub(crate) async fn request_prelogin(
client: &mut Client,
email: String,
) -> Result<PreloginResponseModel> {
let request_model = PreloginRequestModel::new(email);
let config = client.get_api_configurations().await;
Ok(accounts_prelogin_post(&config.identity, Some(request_model)).await?)
Expand Down
14 changes: 10 additions & 4 deletions crates/bitwarden/src/auth/login/password.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ use crate::{
api::request::PasswordTokenRequest,
login::{determine_password_hash, TwoFactorRequest},
},
client::LoginMethod,
client::{kdf::Kdf, LoginMethod},
crypto::EncString,
Client,
};
Expand All @@ -29,20 +29,24 @@ pub(crate) async fn password_login(
client: &mut Client,
input: &PasswordLoginRequest,
) -> Result<PasswordLoginResponse> {
use crate::client::UserLoginMethod;

info!("password logging in");
debug!("{:#?}, {:#?}", client, input);

let password_hash = determine_password_hash(client, &input.email, &input.password).await?;
let password_hash = determine_password_hash(&input.email, &input.kdf, &input.password).await?;
let response = request_identity_tokens(client, input, &password_hash).await?;

if let IdentityTokenResponse::Authenticated(r) = &response {
client.set_tokens(
r.access_token.clone(),
r.refresh_token.clone(),
r.expires_in,
LoginMethod::Username {
LoginMethod::User(UserLoginMethod::Username {
client_id: "web".to_owned(),
},
email: input.email.to_owned(),
kdf: input.kdf.to_owned(),
}),
);

let user_key = EncString::from_str(r.key.as_deref().unwrap()).unwrap();
Expand Down Expand Up @@ -77,6 +81,8 @@ pub struct PasswordLoginRequest {
pub password: String,
// Two-factor authentication
pub two_factor: Option<TwoFactorRequest>,
/// Kdf from prelogin
pub kdf: Kdf,
}

#[derive(Serialize, Deserialize, Debug, JsonSchema)]
Expand Down
5 changes: 4 additions & 1 deletion crates/bitwarden/src/auth/login/two_factor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,10 @@ pub(crate) async fn send_two_factor_email(
client: &mut Client,
input: &TwoFactorEmailRequest,
) -> Result<()> {
let password_hash = determine_password_hash(client, &input.email, &input.password).await?;
// TODO: This should be resolved from the client
let kdf = client.prelogin(input.email.clone()).await?;

let password_hash = determine_password_hash(&input.email, &kdf, &input.password).await?;

let config = client.get_api_configurations().await;
bitwarden_api_api::apis::two_factor_api::two_factor_send_email_login_post(
Expand Down
2 changes: 1 addition & 1 deletion crates/bitwarden/src/auth/register.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use schemars::JsonSchema;
use serde::{Deserialize, Serialize};

use crate::{
client::auth_settings::Kdf,
client::kdf::Kdf,
crypto::{HashPurpose, MasterKey, RsaKeyPair},
error::Result,
util::default_pbkdf2_iterations,
Expand Down
64 changes: 34 additions & 30 deletions crates/bitwarden/src/auth/renew.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
use std::time::{Duration, Instant};

#[cfg(feature = "internal")]
use crate::auth::api::request::ApiTokenRequest;
use crate::{auth::api::request::ApiTokenRequest, client::UserLoginMethod};
use crate::{
auth::api::{request::AccessTokenRequest, response::IdentityTokenResponse},
client::{Client, LoginMethod},
client::{Client, LoginMethod, ServiceAccountLoginMethod},
error::{Error, Result},
};

Expand All @@ -18,37 +18,41 @@ pub(crate) async fn renew_token(client: &mut Client) -> Result<()> {

let res = match login_method {
#[cfg(feature = "internal")]
LoginMethod::Username { client_id } => {
let refresh = client
.refresh_token
.as_deref()
.ok_or(Error::NotAuthenticated)?;
LoginMethod::User(u) => match u {
UserLoginMethod::Username { client_id, .. } => {
let refresh = client
.refresh_token
.as_deref()
.ok_or(Error::NotAuthenticated)?;

crate::auth::api::request::RenewTokenRequest::new(
refresh.to_owned(),
client_id.to_owned(),
)
.send(&client.__api_configurations)
.await?
}
#[cfg(feature = "internal")]
LoginMethod::ApiKey {
client_id,
client_secret,
} => {
ApiTokenRequest::new(client_id, client_secret)
.send(&client.__api_configurations)
.await?
}
LoginMethod::AccessToken {
service_account_id,
client_secret,
..
} => {
AccessTokenRequest::new(*service_account_id, client_secret)
crate::auth::api::request::RenewTokenRequest::new(
refresh.to_owned(),
client_id.to_owned(),
)
.send(&client.__api_configurations)
.await?
}
}
UserLoginMethod::ApiKey {
client_id,
client_secret,
..
} => {
ApiTokenRequest::new(client_id, client_secret)
.send(&client.__api_configurations)
.await?
}
},
LoginMethod::ServiceAccount(s) => match s {
ServiceAccountLoginMethod::AccessToken {
service_account_id,
client_secret,
..
} => {
AccessTokenRequest::new(*service_account_id, client_secret)
.send(&client.__api_configurations)
.await?
}
},
};

match res {
Expand Down
Loading

0 comments on commit db2236c

Please sign in to comment.