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

chore: add force flag to login command #7378

Merged
merged 1 commit into from Feb 16, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
32 changes: 18 additions & 14 deletions crates/turborepo-auth/src/auth/login.rs
Expand Up @@ -27,26 +27,30 @@ pub async fn login<T: Client + TokenClient>(options: &LoginOptions<'_, T>) -> Re
ui,
login_url: login_url_configuration,
login_server,
sso_team: _,
existing_token,
force,
sso_team: _,
} = *options; // Deref or we get double references for each of these

// Check if passed in token exists first.
if let Some(token) = existing_token {
if Token::existing(token.to_string())
.is_valid(api_client)
.await?
{
return check_user_token(token, ui, api_client, "Existing token found!").await;
if !force {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I get really confused by negative if blocks, and almost always feel like they should be inverted. Not blocking at all, but if you feel like restructuring, something like this could be nice:

if force {
  return login()
}

if let Some(token) = existing_token {
  return "yay";
}

login();

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm down to do something like this, but I'll make it in another stack because this'll bork / change up the tests

if let Some(token) = existing_token {
if Token::existing(token.to_string())
.is_valid(api_client)
.await?
{
return check_user_token(token, ui, api_client, "Existing token found!").await;
}
}
}

// If the user is logging into Vercel, check for an existing `vc` token.
if login_url_configuration.contains("vercel.com") {
// The extraction can return an error, but we don't want to fail the login if
// the token is not found.
if let Ok(token) = extract_vercel_token() {
return check_user_token(&token, ui, api_client, "Existing Vercel token found!").await;
// If the user is logging into Vercel, check for an existing `vc` token.
if login_url_configuration.contains("vercel.com") {
// The extraction can return an error, but we don't want to fail the login if
// the token is not found.
if let Ok(token) = extract_vercel_token() {
return check_user_token(&token, ui, api_client, "Existing Vercel token found!")
.await;
}
}
}

Expand Down
2 changes: 2 additions & 0 deletions crates/turborepo-auth/src/auth/mod.rs
Expand Up @@ -24,6 +24,7 @@ where

pub sso_team: Option<&'a str>,
pub existing_token: Option<&'a str>,
pub force: bool,
}
impl<'a, T> LoginOptions<'a, T>
where
Expand All @@ -42,6 +43,7 @@ where
login_server,
sso_team: None,
existing_token: None,
force: false,
}
}
}
Expand Down
42 changes: 23 additions & 19 deletions crates/turborepo-auth/src/auth/sso.rs
Expand Up @@ -37,32 +37,36 @@ pub async fn sso_login<'a, T: Client + TokenClient>(
login_server,
sso_team,
existing_token,
force,
} = *options;

let sso_team = sso_team.ok_or(Error::EmptySSOTeam)?;
// Check if token exists first. Must be there for the user and contain the
// sso_team passed into this function.
if let Some(token) = existing_token {
if Token::existing(token.to_string())
.is_valid(api_client)
.await?
{
return check_sso_token(token, sso_team, ui, api_client, "Existing token found!").await;
if !force {
if let Some(token) = existing_token {
if Token::existing(token.to_string())
.is_valid(api_client)
.await?
{
return check_sso_token(token, sso_team, ui, api_client, "Existing token found!")
.await;
}
}
}

// No existing turbo token found. If the user is logging into Vercel, check for
// an existing `vc` token with correct scope.
if login_url_configuration.contains("vercel.com") {
if let Ok(token) = extract_vercel_token() {
return check_sso_token(
&token,
sso_team,
ui,
api_client,
"Existing Vercel token found!",
)
.await;
// No existing turbo token found. If the user is logging into Vercel, check for
// an existing `vc` token with correct scope.
if login_url_configuration.contains("vercel.com") {
if let Ok(token) = extract_vercel_token() {
return check_sso_token(
&token,
sso_team,
ui,
api_client,
"Existing Vercel token found!",
)
.await;
}
}
}

Expand Down
22 changes: 17 additions & 5 deletions crates/turborepo-lib/src/cli/mod.rs
Expand Up @@ -483,6 +483,10 @@ pub enum Command {
Login {
#[clap(long = "sso-team")]
sso_team: Option<String>,
/// Force a login to receive a new token. Will overwrite any existing
/// tokens for the given login url.
#[clap(long = "force", short = 'f')]
force: bool,
},
/// Logout to your Vercel account
Logout {},
Expand Down Expand Up @@ -1115,7 +1119,7 @@ pub async fn run(

Ok(0)
}
Command::Login { sso_team } => {
Command::Login { sso_team, force } => {
let event = CommandEventBuilder::new("login").with_parent(&root_telemetry);
event.track_call();
if cli_args.test_run {
Expand All @@ -1124,14 +1128,15 @@ pub async fn run(
}

let sso_team = sso_team.clone();
let force = *force;

let mut base = CommandBase::new(cli_args, repo_root, version, ui);
let event_child = event.child();

if let Some(sso_team) = sso_team {
login::sso_login(&mut base, &sso_team, event_child).await?;
login::sso_login(&mut base, &sso_team, event_child, force).await?;
} else {
login::login(&mut base, event_child).await?;
login::login(&mut base, event_child, force).await?;
}

Ok(0)
Expand Down Expand Up @@ -1954,7 +1959,10 @@ mod test {
assert_eq!(
Args::try_parse_from(["turbo", "login"]).unwrap(),
Args {
command: Some(Command::Login { sso_team: None }),
command: Some(Command::Login {
sso_team: None,
force: false
}),
..Args::default()
}
);
Expand All @@ -1964,7 +1972,10 @@ mod test {
command_args: vec![],
global_args: vec![vec!["--cwd", "../examples/with-yarn"]],
expected_output: Args {
command: Some(Command::Login { sso_team: None }),
command: Some(Command::Login {
sso_team: None,
force: false,
}),
cwd: Some(Utf8PathBuf::from("../examples/with-yarn")),
..Args::default()
},
Expand All @@ -1978,6 +1989,7 @@ mod test {
expected_output: Args {
command: Some(Command::Login {
sso_team: Some("my-team".to_string()),
force: false,
}),
cwd: Some(Utf8PathBuf::from("../examples/with-yarn")),
..Args::default()
Expand Down
9 changes: 8 additions & 1 deletion crates/turborepo-lib/src/commands/login.rs
Expand Up @@ -10,6 +10,7 @@ pub async fn sso_login(
base: &mut CommandBase,
sso_team: &str,
telemetry: CommandEventBuilder,
force: bool,
) -> Result<(), Error> {
telemetry.track_login_method(LoginMethod::SSO);
let api_client: APIClient = base.api_client()?;
Expand All @@ -18,6 +19,7 @@ pub async fn sso_login(
let options = LoginOptions {
existing_token: base.config()?.token(),
sso_team: Some(sso_team),
force,
..LoginOptions::new(&ui, &login_url_config, &api_client, &DefaultLoginServer)
};

Expand Down Expand Up @@ -55,13 +57,18 @@ pub async fn sso_login(
Ok(())
}

pub async fn login(base: &mut CommandBase, telemetry: CommandEventBuilder) -> Result<(), Error> {
pub async fn login(
base: &mut CommandBase,
telemetry: CommandEventBuilder,
force: bool,
) -> Result<(), Error> {
telemetry.track_login_method(LoginMethod::Standard);
let api_client: APIClient = base.api_client()?;
let ui = base.ui;
let login_url_config = base.config()?.login_url().to_string();
let options = LoginOptions {
existing_token: base.config()?.token(),
force,
..LoginOptions::new(&ui, &login_url_config, &api_client, &DefaultLoginServer)
};

Expand Down
1 change: 1 addition & 0 deletions turborepo-tests/integration/tests/turbo-help.t
Expand Up @@ -263,6 +263,7 @@ Test help flag for login command
Options:
--sso-team <SSO_TEAM>
--version
-f, --force Force a login to receive a new token. Will overwrite any existing tokens for the given login url
--skip-infer Skip any attempts to infer which version of Turbo the project is configured to use
--no-update-notifier Disable the turbo update notification
--api <API> Override the endpoint for API calls
Expand Down