Skip to content

Commit

Permalink
[SM-1096] Unify State Variable Naming (#895)
Browse files Browse the repository at this point in the history
## 🎟️ Tracking

<!-- Paste the link to the Jira or GitHub issue or otherwise describe /
point to where this change is coming from. -->

https://bitwarden.atlassian.net/browse/SM-1096

## 📔 Objective

<!-- Describe what the purpose of this PR is, for example what bug
you're fixing or new feature you're adding. -->

The naming around state files and paths is inconsistent. This PR aims to
make the usage consistent.

Every mention of state for access token auth should follow this naming
logic:

`state_dir` <- This refers to the state directory, where state files are
stored. The sdk is currently only aware of state files, directory
handling is still being done in bws.
`state_file` <- This refers to a specific state file.

In our code we don't append naming to variables to indicate the type
(Hungarian Notation), so I thought it pertinent to truncate `_path` from
these, preferring more descriptive `state_dir` and `state_file` for the
names.

Thanks @dani-garcia for pointing this out
[here](#559 (comment)).

⚠️ **Note** ⚠️ This also simplifies the `state_file_dir` key in the
`bws` config file to `state_dir`. There will be more changes to this
functionality with SM-1174, so I will wait to update the changelog for
the next bws version until all this is completed.

## ⏰ Reminders before review

- Contributor guidelines followed
- All formatters and local linters executed and passed
- Written new unit and / or integration tests where applicable
- Protected functional changes with optionality (feature flags)
- Used internationalization (i18n) for all UI strings
- CI builds passed
- Communicated to DevOps any deployment requirements
- Updated any necessary documentation (Confluence, contributing docs) or
informed the documentation
  team

## 🦮 Reviewer guidelines

<!-- Suggested interactions but feel free to use (or not) as you desire!
-->

- 👍 (`:+1:`) or similar for great changes
- 📝 (`:memo:`) or ℹ️ (`:information_source:`) for notes or general info
- ❓ (`:question:`) for questions
- 🤔 (`:thinking:`) or 💭 (`:thought_balloon:`) for more open inquiry
that's not quite a confirmed
  issue and could potentially benefit from discussion
- 🎨 (`:art:`) for suggestions / improvements
- ❌ (`:x:`) or ⚠️ (`:warning:`) for more significant problems or
concerns needing attention
- 🌱 (`:seedling:`) or ♻️ (`:recycle:`) for future improvements or
indications of technical debt
- ⛏ (`:pick:`) for minor or nitpick changes
  • Loading branch information
coltonhurst authored Jul 25, 2024
1 parent 63783e5 commit dfdbf7d
Show file tree
Hide file tree
Showing 9 changed files with 26 additions and 29 deletions.
2 changes: 1 addition & 1 deletion crates/bws/src/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ pub(crate) enum ProfileKey {
server_base,
server_api,
server_identity,
state_file_dir,
state_dir,
}

#[derive(Copy, Clone, PartialEq, Eq, PartialOrd, Ord, ValueEnum, Debug)]
Expand Down
6 changes: 3 additions & 3 deletions crates/bws/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ pub(crate) struct Profile {
pub server_base: Option<String>,
pub server_api: Option<String>,
pub server_identity: Option<String>,
pub state_file_dir: Option<String>,
pub state_dir: Option<String>,
}

impl ProfileKey {
Expand All @@ -29,7 +29,7 @@ impl ProfileKey {
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),
ProfileKey::state_dir => p.state_dir = Some(value),
}
}
}
Expand Down Expand Up @@ -117,7 +117,7 @@ impl Profile {
server_base: Some(url.to_string()),
server_api: None,
server_identity: None,
state_file_dir: None,
state_dir: None,
})
}
pub(crate) fn api_url(&self) -> Result<String> {
Expand Down
6 changes: 3 additions & 3 deletions crates/bws/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -84,8 +84,8 @@ async fn process_commands() -> Result<()> {
})
.transpose()?;

let state_file_path = state::get_state_file_path(
profile.and_then(|p| p.state_file_dir).map(Into::into),
let state_file = state::get_state_file(
profile.and_then(|p| p.state_dir).map(Into::into),
access_token_obj.access_token_id.to_string(),
)?;

Expand All @@ -96,7 +96,7 @@ async fn process_commands() -> Result<()> {
.auth()
.login_access_token(&AccessTokenLoginRequest {
access_token,
state_file: state_file_path,
state_file,
})
.await?;

Expand Down
15 changes: 6 additions & 9 deletions crates/bws/src/state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,18 +2,15 @@ use std::path::PathBuf;

use color_eyre::eyre::Result;

pub(crate) fn get_state_file_path(
state_file_dir: Option<PathBuf>,
pub(crate) fn get_state_file(
state_dir: Option<PathBuf>,
access_token_id: String,
) -> Result<Option<PathBuf>> {
if let Some(mut state_file_path) = state_file_dir {
state_file_path.push(access_token_id);
if let Some(mut state_dir) = state_dir {
std::fs::create_dir_all(&state_dir)?;
state_dir.push(access_token_id);

if let Some(parent_folder) = state_file_path.parent() {
std::fs::create_dir_all(parent_folder)?;
}

return Ok(Some(state_file_path));
return Ok(Some(state_dir));
}

Ok(None)
Expand Down
6 changes: 3 additions & 3 deletions languages/go/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -36,13 +36,13 @@ bitwardenClient, _ := sdk.NewBitwardenClient(&apiURL, &identityURL)

### Login

To login using an access token. Define some `statePath` and pass it to use state, or pass `nil`
To login using an access token. Define some `stateFile` and pass it to use state, or pass `nil`
instead to not use state.

```go
statePath := os.Getenv("STATE_PATH")
stateFile := os.Getenv("STATE_FILE")

err := bitwardenClient.AccessTokenLogin(accessToken, &statePath)
err := bitwardenClient.AccessTokenLogin(accessToken, &stateFile)
```

---
Expand Down
6 changes: 3 additions & 3 deletions languages/go/bitwarden_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import (
)

type BitwardenClientInterface interface {
AccessTokenLogin(accessToken string, statePath *string) error
AccessTokenLogin(accessToken string, stateFile *string) error
Projects() ProjectsInterface
Secrets() SecretsInterface
Close()
Expand Down Expand Up @@ -52,8 +52,8 @@ func NewBitwardenClient(apiURL *string, identityURL *string) (BitwardenClientInt
}, nil
}

func (c *BitwardenClient) AccessTokenLogin(accessToken string, statePath *string) error {
req := AccessTokenLoginRequest{AccessToken: accessToken, StateFile: statePath}
func (c *BitwardenClient) AccessTokenLogin(accessToken string, stateFile *string) error {
req := AccessTokenLoginRequest{AccessToken: accessToken, StateFile: stateFile}
command := Command{AccessTokenLogin: &req}

responseStr, err := c.commandRunner.RunCommand(command)
Expand Down
6 changes: 3 additions & 3 deletions languages/go/example/example.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,15 +21,15 @@ func main() {
organizationIDStr := os.Getenv("ORGANIZATION_ID")
projectName := os.Getenv("PROJECT_NAME")

// Configuring the statePath is optional, pass nil
// Configuring the stateFile is optional, pass nil
// in AccessTokenLogin() to not use state
statePath := os.Getenv("STATE_PATH")
stateFile := os.Getenv("STATE_FILE")

if projectName == "" {
projectName = "NewTestProject" // default value
}

err := bitwardenClient.AccessTokenLogin(accessToken, &statePath)
err := bitwardenClient.AccessTokenLogin(accessToken, &stateFile)
if err != nil {
panic(err)
}
Expand Down
4 changes: 2 additions & 2 deletions languages/python/bitwarden_sdk/bitwarden_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,9 @@ def __init__(self, settings: ClientSettings = None):
self.inner = bitwarden_py.BitwardenClient(settings_json)

def access_token_login(self, access_token: str,
state_file_path: str = None):
state_file: str = None):
self._run_command(
Command(access_token_login=AccessTokenLoginRequest(access_token, state_file_path))
Command(access_token_login=AccessTokenLoginRequest(access_token, state_file))
)

def secrets(self):
Expand Down
4 changes: 2 additions & 2 deletions languages/ruby/examples/example.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@

token = ENV['ACCESS_TOKEN']
organization_id = ENV['ORGANIZATION_ID']
state_path = ENV['STATE_PATH']
state_file = ENV['STATE_FILE']

# Configuring the URLS is optional, set them to nil to use the default values
api_url = ENV['API_URL']
Expand All @@ -12,7 +12,7 @@
bitwarden_settings = BitwardenSDKSecrets::BitwardenSettings.new(api_url, identity_url)

bw_client = BitwardenSDKSecrets::BitwardenClient.new(bitwarden_settings)
response = bw_client.access_token_login(token, state_path)
response = bw_client.access_token_login(token, state_file)
puts response

# CREATE project
Expand Down

0 comments on commit dfdbf7d

Please sign in to comment.