Skip to content

Commit

Permalink
Improve CLI login session handling (#1123)
Browse files Browse the repository at this point in the history
Use server address with port for CLI session key in keyring and add
localhost replacing localhost with 127.0.0.1 for all protocols. Adapt
intergration tests for this change and bump CLI crate version.
  • Loading branch information
BartoszCiesla authored Aug 11, 2024
1 parent 5ff2a13 commit 33b913a
Show file tree
Hide file tree
Showing 8 changed files with 27 additions and 22 deletions.
4 changes: 2 additions & 2 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion cli/Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "cli"
version = "0.4.1"
version = "0.5.0"
edition = "2021"
authors = ["bartosz.ciesla@gmail.com"]
repository = "https://github.com/iggy-rs/iggy"
Expand Down
4 changes: 2 additions & 2 deletions integration/tests/cli/common/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -215,8 +215,8 @@ impl IggyCmdTest {
test_case.verify_command(assert);
}

pub(crate) fn get_sever_ip_address(&self) -> Option<String> {
self.server.get_server_ip_addr()
pub(crate) fn get_tcp_server_address(&self) -> Option<String> {
self.server.get_raw_tcp_addr()
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use predicates::str::{contains, starts_with};
use serial_test::parallel;
use std::fmt::{Display, Formatter, Result};

const IGGY_SERVICE: &str = "iggy:127.0.0.1";
const IGGY_SERVICE: &str = "iggy";

#[derive(Debug)]
enum UsingToken {
Expand All @@ -34,12 +34,16 @@ struct TestLoginOptions {
}

impl TestLoginOptions {
fn new(token_name: String, using_token: UsingToken) -> Self {
fn new(token_name: String, using_token: UsingToken, server_address: String) -> Self {
Self {
token_name: token_name.clone(),
token_value: None,
using_token,
keyring: Entry::new(IGGY_SERVICE, &token_name).unwrap_or_else(|_| {
keyring: Entry::new(
format!("{IGGY_SERVICE}:{server_address}").as_str(),
&token_name,
)
.unwrap_or_else(|_| {
panic!(
"Failed to get keyring service data for {} service and {} token name",
IGGY_SERVICE, token_name
Expand Down Expand Up @@ -101,16 +105,21 @@ pub async fn should_be_successful() {
let mut iggy_cmd_test = IggyCmdTest::default();

iggy_cmd_test.setup().await;
let server_address = iggy_cmd_test.get_tcp_server_address();
assert!(server_address.is_some());
let server_address = server_address.unwrap();
iggy_cmd_test
.execute_test(TestLoginOptions::new(
String::from("sample-token"),
UsingToken::Value,
server_address.clone(),
))
.await;
iggy_cmd_test
.execute_test(TestLoginOptions::new(
String::from("access-token"),
UsingToken::Name,
server_address,
))
.await;
}
4 changes: 2 additions & 2 deletions integration/tests/cli/system/test_cli_session_scenario.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ pub async fn should_be_successful() {
let mut iggy_cmd_test = IggyCmdTest::default();

iggy_cmd_test.setup().await;
let server_address = iggy_cmd_test.get_sever_ip_address();
let server_address = iggy_cmd_test.get_tcp_server_address();
assert!(server_address.is_some());
let server_address = server_address.unwrap();

Expand Down Expand Up @@ -72,7 +72,7 @@ pub async fn should_be_successful() {
iggy_cmd_test
.execute_test(TestMeCmd::new(
Protocol::Tcp,
Scenario::FailureDueToSessionTimeout,
Scenario::FailureDueToSessionTimeout(server_address),
))
.await;
}
8 changes: 4 additions & 4 deletions integration/tests/cli/system/test_me_command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ pub(super) enum Scenario {
SuccessWithCredentials,
SuccessWithoutCredentials,
FailureWithoutCredentials,
FailureDueToSessionTimeout,
FailureDueToSessionTimeout(String),
}

impl Protocol {
Expand Down Expand Up @@ -63,7 +63,7 @@ impl IggyCmdTestCase for TestMeCmd {
match &self.scenario {
Scenario::SuccessWithCredentials => command.with_env_credentials(),
Scenario::FailureWithoutCredentials => command.disable_backtrace(),
Scenario::FailureDueToSessionTimeout => command.disable_backtrace(),
Scenario::FailureDueToSessionTimeout(_) => command.disable_backtrace(),
_ => command,
}
}
Expand All @@ -81,8 +81,8 @@ impl IggyCmdTestCase for TestMeCmd {
.failure()
.stderr(diff("Error: CommandError(Iggy command line tool error\n\nCaused by:\n Missing iggy server credentials)\n"));
}
Scenario::FailureDueToSessionTimeout => {
command_state.failure().stderr(diff("Error: CommandError(Login session expired for Iggy server: 127.0.0.1, please login again or use other authentication method)\n"));
Scenario::FailureDueToSessionTimeout(server_address) => {
command_state.failure().stderr(diff(format!("Error: CommandError(Login session expired for Iggy server: {server_address}, please login again or use other authentication method)\n")));
}
}
}
Expand Down
10 changes: 3 additions & 7 deletions sdk/src/args.rs
Original file line number Diff line number Diff line change
Expand Up @@ -280,18 +280,14 @@ const TCP_TRANSPORT: &str = "tcp";
impl Args {
pub fn get_server_address(&self) -> Option<String> {
match self.transport.as_str() {
QUIC_TRANSPORT => Some(self.quic_server_address.split(':').next().unwrap().into()),
QUIC_TRANSPORT => Some(self.quic_server_address.replace("localhost", "127.0.0.1")),
HTTP_TRANSPORT => Some(
self.http_api_url
.clone()
.replace("http://", "")
.replace("localhost", "127.0.0.1")
.split(':')
.next()
.unwrap()
.into(),
.replace("localhost", "127.0.0.1"),
),
TCP_TRANSPORT => Some(self.tcp_server_address.split(':').next().unwrap().into()),
TCP_TRANSPORT => Some(self.tcp_server_address.replace("localhost", "127.0.0.1")),
_ => None,
}
}
Expand Down
2 changes: 1 addition & 1 deletion server/Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "server"
version = "0.4.1"
version = "0.4.2"
edition = "2021"
build = "src/build.rs"

Expand Down

0 comments on commit 33b913a

Please sign in to comment.