Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SM-866: Add state to the SDK & BWS CLI #388

Merged
merged 48 commits into from
Dec 18, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
48 commits
Select commit Hold shift + click to select a range
1ac4cad
SM-866: Add initial state MVP (not building, save dump)
coltonhurst Nov 16, 2023
60d6351
SM-866: Change error handling if the state file is not found, plus ad…
coltonhurst Nov 30, 2023
a34ef5c
SM-866: Save encryption_key in state to generate Client EncryptionSet…
coltonhurst Dec 3, 2023
9ac9a27
SM-866: Remove get_client_state fn comment
coltonhurst Dec 3, 2023
5b2df50
Merge master branch into sm/sm-866
coltonhurst Dec 3, 2023
be1e21b
Merge branch 'master' into sm/sm-866
coltonhurst Dec 3, 2023
02da0b0
SM-866: Cargo clippy changes
coltonhurst Dec 4, 2023
ac6c714
SM-866: Fix failing test_register_initialize_crypto test
coltonhurst Dec 4, 2023
f5972f5
SM-866: Fix failing OS builds
coltonhurst Dec 4, 2023
4721b75
SM-866: Implement bws State
coltonhurst Dec 4, 2023
5e9714e
SM-866: Variable naming refactor
coltonhurst Dec 4, 2023
a913501
SM-1023: Use unix timestamps instead of the Instant type for token ex…
coltonhurst Dec 4, 2023
ee21fce
Merge branch 'master' into sm/sm-866
coltonhurst Dec 4, 2023
051b471
SM-866: Merge sm/sm-1032 into branch
coltonhurst Dec 4, 2023
3762e28
SM-866: Add new access_token_login_from_state function. Intermediary …
coltonhurst Dec 5, 2023
30bbf51
SM-866: Combine bws State and bitwarden StateManager into the bitward…
coltonhurst Dec 5, 2023
6bf16c2
SM-866: Refactor out encryption_key
coltonhurst Dec 5, 2023
b9d29e2
Merge master into branch
coltonhurst Dec 5, 2023
f23a0fb
SM-866: Fix failing tests
coltonhurst Dec 5, 2023
0cddb7e
Merge branch 'master' into sm/sm-866
coltonhurst Dec 10, 2023
566545d
SM-866: Resolve PR comments
coltonhurst Dec 12, 2023
429a1ff
Update crates/bitwarden/src/secrets_manager/secrets/get.rs
coltonhurst Dec 12, 2023
ff706e7
Merge master into branch sm/sm-866
coltonhurst Dec 12, 2023
6800d7c
SM-866: Update to state_file_dir
coltonhurst Dec 12, 2023
e3ddaf4
SM-866: Simplify writing to state file
coltonhurst Dec 12, 2023
9e6be99
Merge master into sm/sm-866
coltonhurst Dec 12, 2023
944ae2e
Change login_method to have the access token directly
dani-garcia Dec 12, 2023
398a935
SM-866: Utilize the set_login_method in login_from_state
coltonhurst Dec 12, 2023
5967ed4
Merge branch 'master' into sm/sm-866
coltonhurst Dec 12, 2023
fd58fc4
Merge branch 'master' into ps/login-method-access-token
dani-garcia Dec 12, 2023
9ee7003
Merge ps/login-method-access-token into sm/sm-866
coltonhurst Dec 12, 2023
8bb1016
Merge branch 'master' into sm/sm-866
coltonhurst Dec 12, 2023
2525fd4
SM-866: Write to state on renew_token
coltonhurst Dec 12, 2023
0bffdf5
SM-866: Simplify and_then to a map
coltonhurst Dec 12, 2023
16703f4
SM-866: Address PR comments
coltonhurst Dec 12, 2023
224a3a1
SM-866: Move state_file into AccessTokenLoginRequest struct
coltonhurst Dec 12, 2023
be0b374
SM-866: Minor updates
coltonhurst Dec 12, 2023
751ff0d
SM-866: Delete file that remained from merge
coltonhurst Dec 12, 2023
b1ba938
SM-866: Small refactorings
coltonhurst Dec 12, 2023
23421a7
Merge branch 'main' into sm/sm-866
coltonhurst Dec 12, 2023
d0c0c10
SM-866: Address PR comments
coltonhurst Dec 17, 2023
fad2bd3
Merge branch 'main' into sm/sm-866
coltonhurst Dec 17, 2023
0c8125f
SM-866: Update the changelog
coltonhurst Dec 17, 2023
f45b5f5
SM-866: Run prettier
coltonhurst Dec 17, 2023
ce50cf1
SM-866: Only create a state file if the state_file_dir is specified (…
coltonhurst Dec 17, 2023
43481d4
Merge branch 'main' into sm/sm-866
coltonhurst Dec 18, 2023
4443741
SM-866: Update usage of Error::Internal due to recent change
coltonhurst Dec 18, 2023
bd3c420
SM-866: Cargo fmt
coltonhurst Dec 18, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions crates/bitwarden/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,11 @@ adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).

## [Unreleased]

### Added

- Support for basic state to avoid reauthenticating when creating a new `Client`. This is a breaking
change because of adding `state_file` to the `AccessTokenLoginRequest` struct. (#388)

### Deprecated

- `client.access_token_login()` is now deprecated and will be removed in a future release. Please
Expand Down
2 changes: 1 addition & 1 deletion crates/bitwarden/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ async fn test() -> Result<()> {
let mut client = Client::new(Some(settings));

// Before we operate, we need to authenticate with a token
let token = AccessTokenLoginRequest { access_token: String::from("") };
let token = AccessTokenLoginRequest { access_token: String::from(""), state_file: None };
client.auth().login_access_token(&token).await.unwrap();

let org_id = SecretIdentifiersRequest { organization_id: Uuid::parse_str("00000000-0000-0000-0000-000000000000").unwrap() };
Expand Down
3 changes: 2 additions & 1 deletion crates/bitwarden/src/auth/client_auth.rs
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,8 @@ mod tests {
.auth()
.login_access_token(&AccessTokenLoginRequest {
access_token: "0.ec2c1d46-6a4b-4751-a310-af9601317f2d.C2IgxjjLF7qSshsbwe8JGcbM075YXw:X8vbvA0bduihIDe/qrzIQQ==".into(),
})
state_file: None,
},)
.await
.unwrap();
assert!(res.authenticated);
Expand Down
63 changes: 60 additions & 3 deletions crates/bitwarden/src/auth/login/access_token.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
use std::path::{Path, PathBuf};

use base64::Engine;
use chrono::Utc;
use schemars::JsonSchema;
use serde::{Deserialize, Serialize};
use uuid::Uuid;

use crate::{
auth::{
Expand All @@ -11,6 +15,7 @@
client::{AccessToken, LoginMethod, ServiceAccountLoginMethod},
crypto::{EncString, KeyDecryptable, SymmetricCryptoKey},
error::{Error, Result},
secrets_manager::state::{self, ClientState},
util::BASE64_ENGINE,
Client,
};
Expand All @@ -24,6 +29,25 @@

let access_token: AccessToken = input.access_token.parse()?;

if let Some(state_file) = &input.state_file {
if let Ok(organization_id) = load_tokens_from_state(client, state_file, &access_token) {
client.set_login_method(LoginMethod::ServiceAccount(
ServiceAccountLoginMethod::AccessToken {
access_token,
organization_id,
state_file: Some(state_file.to_path_buf()),
},
));

return Ok(AccessTokenLoginResponse {
authenticated: true,
reset_master_password: false,
force_password_reset: false,
two_factor: None,
});
}

Check warning on line 48 in crates/bitwarden/src/auth/login/access_token.rs

View check run for this annotation

Codecov / codecov/patch

crates/bitwarden/src/auth/login/access_token.rs#L33-L48

Added lines #L33 - L48 were not covered by tests
}

let response = request_access_token(client, &access_token).await?;

if let IdentityTokenResponse::Payload(r) = &response {
Expand All @@ -40,9 +64,7 @@
}

let payload: Payload = serde_json::from_slice(&decrypted_payload)?;

let encryption_key = BASE64_ENGINE.decode(payload.encryption_key)?;

let encryption_key = BASE64_ENGINE.decode(payload.encryption_key.clone())?;
let encryption_key = SymmetricCryptoKey::try_from(encryption_key.as_slice())?;

let access_token_obj: JWTToken = r.access_token.parse()?;
Expand All @@ -54,6 +76,11 @@
.parse()
.map_err(|_| Error::InvalidResponse)?;

if let Some(state_file) = &input.state_file {
let state = ClientState::new(r.access_token.clone(), payload.encryption_key);
_ = state::set(state_file, &access_token, state);

Check warning on line 81 in crates/bitwarden/src/auth/login/access_token.rs

View check run for this annotation

Codecov / codecov/patch

crates/bitwarden/src/auth/login/access_token.rs#L80-L81

Added lines #L80 - L81 were not covered by tests
}

client.set_tokens(
r.access_token.clone(),
r.refresh_token.clone(),
Expand All @@ -63,6 +90,7 @@
ServiceAccountLoginMethod::AccessToken {
access_token,
organization_id,
state_file: input.state_file.clone(),
},
));

Expand All @@ -82,12 +110,41 @@
.await
}

fn load_tokens_from_state(
client: &mut Client,
state_file: &Path,
access_token: &AccessToken,
) -> Result<Uuid> {
let client_state = state::get(state_file, access_token)?;

Check warning on line 118 in crates/bitwarden/src/auth/login/access_token.rs

View check run for this annotation

Codecov / codecov/patch

crates/bitwarden/src/auth/login/access_token.rs#L113-L118

Added lines #L113 - L118 were not covered by tests

let token: JWTToken = client_state.token.parse()?;

Check warning on line 120 in crates/bitwarden/src/auth/login/access_token.rs

View check run for this annotation

Codecov / codecov/patch

crates/bitwarden/src/auth/login/access_token.rs#L120

Added line #L120 was not covered by tests

if let Some(organization_id) = token.organization {
let time_till_expiration = (token.exp as i64) - Utc::now().timestamp();

if time_till_expiration > 0 {
let organization_id: Uuid = organization_id
.parse()
.map_err(|_| "Bad organization id.")?;
let encryption_key: SymmetricCryptoKey = client_state.encryption_key.parse()?;

Check warning on line 129 in crates/bitwarden/src/auth/login/access_token.rs

View check run for this annotation

Codecov / codecov/patch

crates/bitwarden/src/auth/login/access_token.rs#L122-L129

Added lines #L122 - L129 were not covered by tests

client.set_tokens(client_state.token, None, time_till_expiration as u64);
client.initialize_crypto_single_key(encryption_key);

return Ok(organization_id);
}
}

Check warning on line 136 in crates/bitwarden/src/auth/login/access_token.rs

View check run for this annotation

Codecov / codecov/patch

crates/bitwarden/src/auth/login/access_token.rs#L131-L136

Added lines #L131 - L136 were not covered by tests

Err(Error::InvalidStateFile)
}

Check warning on line 139 in crates/bitwarden/src/auth/login/access_token.rs

View check run for this annotation

Codecov / codecov/patch

crates/bitwarden/src/auth/login/access_token.rs#L138-L139

Added lines #L138 - L139 were not covered by tests

/// Login to Bitwarden with access token
#[derive(Serialize, Deserialize, Debug, JsonSchema)]
#[serde(rename_all = "camelCase", deny_unknown_fields)]
pub struct AccessTokenLoginRequest {
/// Bitwarden service API access token
pub access_token: String,
pub state_file: Option<PathBuf>,
coltonhurst marked this conversation as resolved.
Show resolved Hide resolved
}

#[derive(Serialize, Deserialize, Debug, JsonSchema)]
Expand Down
28 changes: 24 additions & 4 deletions crates/bitwarden/src/auth/renew.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,13 @@
auth::api::{request::AccessTokenRequest, response::IdentityTokenResponse},
client::{Client, LoginMethod, ServiceAccountLoginMethod},
error::{Error, Result},
secrets_manager::state::{self, ClientState},
};

pub(crate) async fn renew_token(client: &mut Client) -> Result<()> {
const TOKEN_RENEW_MARGIN_SECONDS: i64 = 5 * 60;

if let (Some(expires), Some(login_method)) = (&client.token_expires_in, &client.login_method) {
if let (Some(expires), Some(login_method)) = (&client.token_expires_on, &client.login_method) {
if Utc::now().timestamp() < expires - TOKEN_RENEW_MARGIN_SECONDS {
return Ok(());
}
Expand Down Expand Up @@ -43,13 +44,32 @@
}
},
LoginMethod::ServiceAccount(s) => match s {
ServiceAccountLoginMethod::AccessToken { access_token, .. } => {
AccessTokenRequest::new(
ServiceAccountLoginMethod::AccessToken {
access_token,
state_file,

Check warning on line 49 in crates/bitwarden/src/auth/renew.rs

View check run for this annotation

Codecov / codecov/patch

crates/bitwarden/src/auth/renew.rs#L47-L49

Added lines #L47 - L49 were not covered by tests
..
} => {
let result = AccessTokenRequest::new(

Check warning on line 52 in crates/bitwarden/src/auth/renew.rs

View check run for this annotation

Codecov / codecov/patch

crates/bitwarden/src/auth/renew.rs#L52

Added line #L52 was not covered by tests
access_token.access_token_id,
&access_token.client_secret,
)
.send(&client.__api_configurations)
.await?
.await?;

Check warning on line 57 in crates/bitwarden/src/auth/renew.rs

View check run for this annotation

Codecov / codecov/patch

crates/bitwarden/src/auth/renew.rs#L57

Added line #L57 was not covered by tests

if let (
IdentityTokenResponse::Authenticated(r),
Some(state_file),
Ok(enc_settings),
) = (&result, state_file, client.get_encryption_settings())

Check warning on line 63 in crates/bitwarden/src/auth/renew.rs

View check run for this annotation

Codecov / codecov/patch

crates/bitwarden/src/auth/renew.rs#L60-L63

Added lines #L60 - L63 were not covered by tests
{
if let Some(enc_key) = enc_settings.get_key(&None) {
let state =
ClientState::new(r.access_token.clone(), enc_key.to_base64());
_ = state::set(state_file, access_token, state);
}
}

Check warning on line 70 in crates/bitwarden/src/auth/renew.rs

View check run for this annotation

Codecov / codecov/patch

crates/bitwarden/src/auth/renew.rs#L65-L70

Added lines #L65 - L70 were not covered by tests

result

Check warning on line 72 in crates/bitwarden/src/auth/renew.rs

View check run for this annotation

Codecov / codecov/patch

crates/bitwarden/src/auth/renew.rs#L72

Added line #L72 was not covered by tests
coltonhurst marked this conversation as resolved.
Show resolved Hide resolved
}
},
};
Expand Down
9 changes: 6 additions & 3 deletions crates/bitwarden/src/client/client.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
use std::path::PathBuf;

use chrono::Utc;
use reqwest::header::{self};
use uuid::Uuid;
Expand Down Expand Up @@ -62,14 +64,15 @@ pub(crate) enum ServiceAccountLoginMethod {
AccessToken {
access_token: AccessToken,
organization_id: Uuid,
state_file: Option<PathBuf>,
},
}

#[derive(Debug)]
pub struct Client {
token: Option<String>,
pub(crate) refresh_token: Option<String>,
pub(crate) token_expires_in: Option<i64>,
pub(crate) token_expires_on: Option<i64>,
coltonhurst marked this conversation as resolved.
Show resolved Hide resolved
pub(crate) login_method: Option<LoginMethod>,

/// Use Client::get_api_configurations() to access this.
Expand Down Expand Up @@ -114,7 +117,7 @@ impl Client {
Self {
token: None,
refresh_token: None,
token_expires_in: None,
token_expires_on: None,
login_method: None,
__api_configurations: ApiConfigurations {
identity,
Expand Down Expand Up @@ -193,7 +196,7 @@ impl Client {
) {
self.token = Some(token.clone());
self.refresh_token = refresh_token;
self.token_expires_in = Some(Utc::now().timestamp() + expires_in as i64);
self.token_expires_on = Some(Utc::now().timestamp() + expires_in as i64);
self.__api_configurations.identity.oauth_access_token = Some(token.clone());
self.__api_configurations.api.oauth_access_token = Some(token);
}
Expand Down
6 changes: 6 additions & 0 deletions crates/bitwarden/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,12 @@ pub enum Error {
#[error("Received error message from server: [{}] {}", .status, .message)]
ResponseContent { status: StatusCode, message: String },

#[error("The state file version is invalid")]
InvalidStateFileVersion,

#[error("The state file could not be read")]
InvalidStateFile,

#[error("Internal error: {0}")]
Internal(Cow<'static, str>),
}
Expand Down
2 changes: 1 addition & 1 deletion crates/bitwarden/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@
//! let mut client = Client::new(Some(settings));
//!
//! // Before we operate, we need to authenticate with a token
//! let token = AccessTokenLoginRequest { access_token: String::from("") };
//! let token = AccessTokenLoginRequest { access_token: String::from(""), state_file: None };
//! client.auth().login_access_token(&token).await.unwrap();
//!
//! let org_id = SecretIdentifiersRequest { organization_id: Uuid::parse_str("00000000-0000-0000-0000-000000000000").unwrap() };
Expand Down
1 change: 1 addition & 0 deletions crates/bitwarden/src/secrets_manager/mod.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
pub mod projects;
pub mod secrets;
pub mod state;

mod client_projects;
mod client_secrets;
Expand Down
52 changes: 52 additions & 0 deletions crates/bitwarden/src/secrets_manager/state.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
use serde::{Deserialize, Serialize};

use crate::{
client::AccessToken,
crypto::{EncString, KeyDecryptable, KeyEncryptable},
error::{Error, Result},
};
use std::{fmt::Debug, path::Path};

const STATE_VERSION: u32 = 1;

#[cfg(feature = "secrets")]
#[derive(Serialize, Deserialize, Debug)]

Check warning on line 13 in crates/bitwarden/src/secrets_manager/state.rs

View check run for this annotation

Codecov / codecov/patch

crates/bitwarden/src/secrets_manager/state.rs#L13

Added line #L13 was not covered by tests
pub struct ClientState {
pub(crate) version: u32,
pub(crate) token: String,
pub(crate) encryption_key: String,
}

impl ClientState {
pub fn new(token: String, encryption_key: String) -> Self {
Self {
version: STATE_VERSION,
token,
encryption_key,
}
}

Check warning on line 27 in crates/bitwarden/src/secrets_manager/state.rs

View check run for this annotation

Codecov / codecov/patch

crates/bitwarden/src/secrets_manager/state.rs#L21-L27

Added lines #L21 - L27 were not covered by tests
}

pub fn get(state_file: &Path, access_token: &AccessToken) -> Result<ClientState> {
let file_content = std::fs::read_to_string(state_file)?;

Check warning on line 31 in crates/bitwarden/src/secrets_manager/state.rs

View check run for this annotation

Codecov / codecov/patch

crates/bitwarden/src/secrets_manager/state.rs#L30-L31

Added lines #L30 - L31 were not covered by tests

let encrypted_state: EncString = file_content.parse()?;
let decrypted_state: String = encrypted_state.decrypt_with_key(&access_token.encryption_key)?;
let client_state: ClientState = serde_json::from_str(&decrypted_state)?;

Check warning on line 35 in crates/bitwarden/src/secrets_manager/state.rs

View check run for this annotation

Codecov / codecov/patch

crates/bitwarden/src/secrets_manager/state.rs#L33-L35

Added lines #L33 - L35 were not covered by tests

if client_state.version != STATE_VERSION {
return Err(Error::InvalidStateFileVersion);
}

Ok(client_state)
}

Check warning on line 42 in crates/bitwarden/src/secrets_manager/state.rs

View check run for this annotation

Codecov / codecov/patch

crates/bitwarden/src/secrets_manager/state.rs#L37-L42

Added lines #L37 - L42 were not covered by tests

pub fn set(state_file: &Path, access_token: &AccessToken, state: ClientState) -> Result<()> {
let serialized_state: String = serde_json::to_string(&state)?;
let encrypted_state: EncString =
serialized_state.encrypt_with_key(&access_token.encryption_key)?;
let state_string: String = encrypted_state.to_string();

std::fs::write(state_file, state_string)
.map_err(|_| "Failure writing to the state file.".into())
}

Check warning on line 52 in crates/bitwarden/src/secrets_manager/state.rs

View check run for this annotation

Codecov / codecov/patch

crates/bitwarden/src/secrets_manager/state.rs#L44-L52

Added lines #L44 - L52 were not covered by tests
2 changes: 2 additions & 0 deletions crates/bws/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
### Added

- Ability to output secrets in an `env` format with `bws` (#320)
- Basic state to avoid reauthenticating every run, used when setting the `state_file_dir` key in the
config (#388)

## [0.3.1] - 2023-10-13

Expand Down
6 changes: 5 additions & 1 deletion crates/bws/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
pub server_base: Option<String>,
pub server_api: Option<String>,
pub server_identity: Option<String>,
pub state_file_dir: Option<String>,
}

// TODO: This could probably be derived with a macro if we start adding more fields
Expand All @@ -28,6 +29,7 @@
server_base,
server_api,
server_identity,
state_file_dir,
}

impl ProfileKey {
Expand All @@ -36,14 +38,15 @@
ProfileKey::server_base => p.server_base = Some(value),
ProfileKey::server_api => p.server_api = Some(value),
ProfileKey::server_identity => p.server_identity = Some(value),
ProfileKey::state_file_dir => p.state_file_dir = Some(value),

Check warning on line 41 in crates/bws/src/config.rs

View check run for this annotation

Codecov / codecov/patch

crates/bws/src/config.rs#L41

Added line #L41 was not covered by tests
}
}
}

pub(crate) const FILENAME: &str = "config";
pub(crate) const DIRECTORY: &str = ".bws";

fn get_config_path(config_file: Option<&Path>, ensure_folder_exists: bool) -> PathBuf {
pub(crate) fn get_config_path(config_file: Option<&Path>, ensure_folder_exists: bool) -> PathBuf {
let config_file = config_file.map(ToOwned::to_owned).unwrap_or_else(|| {
let base_dirs = BaseDirs::new().unwrap();
base_dirs.home_dir().join(DIRECTORY).join(FILENAME)
Expand Down Expand Up @@ -118,6 +121,7 @@
server_base: Some(url.to_string()),
server_api: None,
server_identity: None,
state_file_dir: None,

Check warning on line 124 in crates/bws/src/config.rs

View check run for this annotation

Codecov / codecov/patch

crates/bws/src/config.rs#L124

Added line #L124 was not covered by tests
})
}
pub(crate) fn api_url(&self) -> Result<String> {
Expand Down
Loading