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

fix: More robust token validation #7407

Merged
merged 1 commit into from Feb 21, 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
108 changes: 94 additions & 14 deletions crates/turborepo-auth/src/auth/login.rs
Expand Up @@ -4,13 +4,10 @@ pub use error::Error;
use reqwest::Url;
use tokio::sync::OnceCell;
use tracing::warn;
use turborepo_api_client::{Client, TokenClient};
use turborepo_ui::start_spinner;
use turborepo_api_client::{CacheClient, Client, TokenClient};
use turborepo_ui::{start_spinner, BOLD, UI};

use crate::{
auth::{check_user_token, extract_vercel_token},
error, ui, LoginOptions, Token,
};
use crate::{auth::extract_vercel_token, error, ui, LoginOptions, Token};

const DEFAULT_HOST_NAME: &str = "127.0.0.1";
const DEFAULT_PORT: u16 = 9789;
Expand All @@ -21,7 +18,9 @@ const DEFAULT_PORT: u16 = 9789;
///
/// First checks if an existing option has been passed in, then if the login is
/// to Vercel, checks if the user has a Vercel CLI token on disk.
pub async fn login<T: Client + TokenClient>(options: &LoginOptions<'_, T>) -> Result<Token, Error> {
pub async fn login<T: Client + TokenClient + CacheClient>(
options: &LoginOptions<'_, T>,
) -> Result<Token, Error> {
let LoginOptions {
api_client,
ui,
Expand All @@ -32,14 +31,32 @@ pub async fn login<T: Client + TokenClient>(options: &LoginOptions<'_, T>) -> Re
sso_team: _,
} = *options; // Deref or we get double references for each of these

// I created a closure that gives back a closure since the `is_valid` checks do
// a call to get the user, so instead of doing that multiple times we have
// `is_valid` give back the user email.
//
// In the future I want to make the Token have some non-skewable information and
// be able to get rid of this, but it works for now.
let valid_token_callback = |message: &str, ui: &UI| {
let message = message.to_string();
let ui = *ui;
move |user_email: &str| {
println!("{}", ui.apply(BOLD.apply_to(message)));
ui::print_cli_authorized(user_email, &ui);
}
};
// Check if passed in token exists first.
if !force {
if let Some(token) = existing_token {
if Token::existing(token.to_string())
.is_valid(api_client)
let token = Token::existing(token.into());
if token
.is_valid(
api_client,
Some(valid_token_callback("Existing token found!", ui)),
)
.await?
{
return check_user_token(token, ui, api_client, "Existing token found!").await;
return Ok(token);
}
}

Expand All @@ -48,8 +65,16 @@ pub async fn login<T: Client + TokenClient>(options: &LoginOptions<'_, T>) -> Re
// 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;
let token = Token::existing(token);
if token
.is_valid(
api_client,
Some(valid_token_callback("Existing Vercel token found!", ui)),
)
.await?
{
return Ok(token);
}
}
}
}
Expand Down Expand Up @@ -108,11 +133,12 @@ mod tests {
use std::{assert_matches::assert_matches, sync::atomic::AtomicUsize};

use async_trait::async_trait;
use reqwest::{RequestBuilder, Response};
use reqwest::{Method, RequestBuilder, Response};
use turborepo_api_client::Client;
use turborepo_ui::UI;
use turborepo_vercel_api::{
Membership, Role, SpacesResponse, Team, TeamsResponse, User, UserResponse, VerifiedSsoUser,
CachingStatus, CachingStatusResponse, Membership, Role, SpacesResponse, Team,
TeamsResponse, User, UserResponse, VerifiedSsoUser,
};
use turborepo_vercel_api_mock::start_test_server;

Expand Down Expand Up @@ -267,6 +293,60 @@ mod tests {
}
}

#[async_trait]
Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps out of scope for this PR but as of rust 1.75 we don't need async_trait any more. If you need to specify things like Send (the default for async_trait traits) you can use return position impl instead.

async fn get_artifact(...) -> Result<Option<Response>, turborepo_api_client::Error>
// or
fn get_artifact(...) -> impl Future<Output=Result<Option<Response>, turborepo_api_client::Error>> + Send

The side effect being no more Pin<Box<dyn Future>> :)

See here: https://github.com/vercel/turbo/blob/main/crates/turborepo-repository/src/discovery.rs#L43-L57

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 quickly tried to swap out [async_trait] but I had a bunch of errors around the Output not being a Future, so I'll migrate these in another PR

impl CacheClient for MockApiClient {
async fn get_artifact(
&self,
_hash: &str,
_token: &str,
_team_id: Option<&str>,
_team_slug: Option<&str>,
_method: Method,
) -> Result<Option<Response>, turborepo_api_client::Error> {
unimplemented!("get_artifact")
}
async fn put_artifact(
&self,
_hash: &str,
_artifact_body: &[u8],
_duration: u64,
_tag: Option<&str>,
_token: &str,
_team_id: Option<&str>,
_team_slug: Option<&str>,
) -> Result<(), turborepo_api_client::Error> {
unimplemented!("set_artifact")
}
async fn fetch_artifact(
&self,
_hash: &str,
_token: &str,
_team_id: Option<&str>,
_team_slug: Option<&str>,
) -> Result<Option<Response>, turborepo_api_client::Error> {
unimplemented!("fetch_artifact")
}
async fn artifact_exists(
&self,
_hash: &str,
_token: &str,
_team_id: Option<&str>,
_team_slug: Option<&str>,
) -> Result<Option<Response>, turborepo_api_client::Error> {
unimplemented!("artifact_exists")
}
async fn get_caching_status(
&self,
_token: &str,
_team_id: Option<&str>,
_team_slug: Option<&str>,
) -> Result<CachingStatusResponse, turborepo_api_client::Error> {
Ok(CachingStatusResponse {
status: CachingStatus::Enabled,
})
}
}

#[tokio::test]
async fn test_login() {
let port = port_scanner::request_open_port().unwrap();
Expand Down
56 changes: 5 additions & 51 deletions crates/turborepo-auth/src/auth/mod.rs
Expand Up @@ -5,17 +5,17 @@ mod sso;
pub use login::*;
pub use logout::*;
pub use sso::*;
use turborepo_api_client::{Client, TokenClient};
use turborepo_ui::{BOLD, UI};
use turborepo_api_client::{CacheClient, Client, TokenClient};
use turborepo_ui::UI;

use crate::{ui, LoginServer, Token};
use crate::LoginServer;

const VERCEL_TOKEN_DIR: &str = "com.vercel.cli";
const VERCEL_TOKEN_FILE: &str = "auth.json";

pub struct LoginOptions<'a, T>
where
T: Client + TokenClient,
T: Client + TokenClient + CacheClient,
{
pub ui: &'a UI,
pub login_url: &'a str,
Expand All @@ -28,7 +28,7 @@ where
}
impl<'a, T> LoginOptions<'a, T>
where
T: Client + TokenClient,
T: Client + TokenClient + CacheClient,
{
pub fn new(
ui: &'a UI,
Expand All @@ -48,52 +48,6 @@ where
}
}

async fn check_user_token(
token: &str,
ui: &UI,
api_client: &(impl Client + TokenClient),
message: &str,
) -> Result<Token, Error> {
let response_user = api_client.get_user(token).await?;
println!("{}", ui.apply(BOLD.apply_to(message)));
ui::print_cli_authorized(&response_user.user.email, ui);
Ok(Token::Existing(token.to_string()))
}

async fn check_sso_token(
token: &str,
sso_team: &str,
ui: &UI,
api_client: &(impl Client + TokenClient),
message: &str,
) -> Result<Token, Error> {
let (result_user, result_teams) =
tokio::join!(api_client.get_user(token), api_client.get_teams(token),);

let token = Token::existing(token.into());

match (result_user, result_teams) {
(Ok(response_user), Ok(response_teams)) => {
if response_teams
.teams
.iter()
.any(|team| team.slug == sso_team)
{
if token.is_valid(api_client).await? {
println!("{}", ui.apply(BOLD.apply_to(message)));
ui::print_cli_authorized(&response_user.user.email, ui);
Ok(token)
} else {
Err(Error::SSOTokenExpired(sso_team.to_string()))
}
} else {
Err(Error::SSOTeamNotFound(sso_team.to_string()))
}
}
(Err(e), _) | (_, Err(e)) => Err(Error::APIError(e)),
}
}

fn extract_vercel_token() -> Result<String, Error> {
let vercel_config_dir =
turborepo_dirs::vercel_config_dir().ok_or_else(|| Error::ConfigDirNotFound)?;
Expand Down