From 6b5d8f85f5a1decb3b00e0d3c0fa86ad69d276a0 Mon Sep 17 00:00:00 2001 From: Eric Ortega <24722023+ejortega@users.noreply.github.com> Date: Fri, 4 Aug 2023 08:40:58 -0500 Subject: [PATCH 1/4] fix: update lockfile detection to search root directory first (#1173) --- CHANGELOG.md | 3 +++ cli/src/commands/parse.rs | 26 ++++++++++++++++++++++++++ 2 files changed, 29 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index fda6cc40f..d46e825b2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,9 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). ### Changed - Include lockfile paths when analyzing projects +### Fixed +- Detect lockfile in current path before searching in subdirectories + ## [5.5.0] - 2023-07-18 ### Added diff --git a/cli/src/commands/parse.rs b/cli/src/commands/parse.rs index f3c3332c7..7499a5bac 100644 --- a/cli/src/commands/parse.rs +++ b/cli/src/commands/parse.rs @@ -181,7 +181,26 @@ fn find_manifest_format(path: &Path) -> Option<(LockfileFormat, Option) // Look for formats which already have a lockfile generated. let manifest_lockfile = formats.find_map(|format| { + // Check the root directory for a lockfile + let root_dir_entries = match fs::read_dir(manifest_dir) { + Ok(entries) => entries, + Err(_) => return None, + }; + + for entry_result in root_dir_entries { + match entry_result { + Ok(entry) => { + if format.parser().is_path_lockfile(&entry.path()) { + return Some((format, entry.path())); + } + }, + Err(_) => continue, + } + } + + // If not found in root, then use WalkDir to search in the subdirectories let manifest_lockfile = WalkDir::new(manifest_dir) + .min_depth(1) .into_iter() .flatten() .find(|entry| format.parser().is_path_lockfile(entry.path()))?; @@ -288,4 +307,11 @@ mod tests { assert_eq!(parsed.format, expected_format, "{}", file); } } + + #[test] + fn it_can_detect_correct_lockfile() { + let cli_cargo_toml = PathBuf::from("../Cargo.toml"); + let (_, path) = find_manifest_format(&cli_cargo_toml).unwrap(); + assert!(path.unwrap().ends_with("cli/Cargo.lock")); + } } From 180fdeca5603d06d5d80462336fbc6a161163756 Mon Sep 17 00:00:00 2001 From: Kyle Willmon Date: Mon, 7 Aug 2023 11:29:04 -0500 Subject: [PATCH 2/4] Use locksmith tokens (#1166) This patch updates CLI to generate and use Locksmith tokens (aka "API keys") For backwards compatibility, OIDC refresh tokens are still supported in the settings.yaml or via the PHYLUM_API_KEY environment variable. But they are no longer generated. --- Cargo.lock | 1 + cli/Cargo.toml | 1 + cli/src/api/endpoints.rs | 5 ++ cli/src/api/mod.rs | 51 ++++++----- cli/src/app.rs | 31 +++++-- cli/src/auth/mod.rs | 4 + cli/src/auth/oidc.rs | 84 +++++++++++++++---- cli/src/auth/server.rs | 38 ++++++--- cli/src/commands/auth.rs | 56 +++++++++---- cli/src/commands/extensions/api.rs | 3 +- cli/src/test.rs | 41 ++++++++- docs/command_line_tool/phylum_auth_login.md | 3 + .../command_line_tool/phylum_auth_register.md | 3 + 13 files changed, 244 insertions(+), 77 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index bae92379d..8ed9e1f96 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3654,6 +3654,7 @@ dependencies = [ "toml 0.7.6", "unicode-width", "url", + "uuid", "vuln-reach", "vulnreach_types", "walkdir", diff --git a/cli/Cargo.toml b/cli/Cargo.toml index f3dddfabf..51ab32113 100644 --- a/cli/Cargo.toml +++ b/cli/Cargo.toml @@ -67,6 +67,7 @@ deno_ast = { version = "0.27.2", features = ["transpiling"] } birdcage = { version = "0.2.0" } libc = "0.2.135" ignore = { version = "0.4.20", optional = true } +uuid = "1.4.1" [dev-dependencies] assert_cmd = "2.0.4" diff --git a/cli/src/api/endpoints.rs b/cli/src/api/endpoints.rs index 03864b842..d51e91df4 100644 --- a/cli/src/api/endpoints.rs +++ b/cli/src/api/endpoints.rs @@ -160,6 +160,11 @@ pub fn oidc_discovery(api_uri: &str) -> Result { Ok(get_api_path(api_uri)?.join(".well-known/openid-configuration")?) } +/// GET /.well-known/locksmith-configuration +pub fn locksmith_discovery(api_uri: &str) -> Result { + Ok(get_api_path(api_uri)?.join(".well-known/locksmith-configuration")?) +} + /// POST /reachability/vulnerabilities pub fn vulnreach(api_uri: &str) -> Result { Ok(parse_base_url(api_uri)?.join("reachability/vulnerabilities")?) diff --git a/cli/src/api/mod.rs b/cli/src/api/mod.rs index ef63e84fa..af55eceeb 100644 --- a/cli/src/api/mod.rs +++ b/cli/src/api/mod.rs @@ -2,7 +2,6 @@ use std::collections::HashSet; use std::time::Duration; use anyhow::{anyhow, Context}; -use phylum_types::types::auth::TokenResponse; use phylum_types::types::common::{JobId, ProjectId}; use phylum_types::types::group::{ CreateGroupRequest, CreateGroupResponse, ListGroupMembersResponse, ListUserGroupsResponse, @@ -28,7 +27,8 @@ use crate::api::endpoints::BaseUriError; use crate::app::USER_AGENT; use crate::auth::jwt::RealmRole; use crate::auth::{ - fetch_oidc_server_settings, handle_auth_flow, handle_refresh_tokens, jwt, AuthAction, UserInfo, + fetch_locksmith_server_settings, handle_auth_flow, handle_refresh_tokens, jwt, AuthAction, + UserInfo, }; use crate::config::{AuthInfo, Config}; use crate::types::{ @@ -164,18 +164,22 @@ impl PhylumApi { pub async fn new(mut config: Config, request_timeout: Option) -> Result { // Do we have a refresh token? let ignore_certs = config.ignore_certs(); - let tokens: TokenResponse = match &config.auth_info.offline_access() { - Some(refresh_token) => { - handle_refresh_tokens(refresh_token, ignore_certs, &config.connection.uri) - .await - .context("Token refresh failed")? + let refresh_token = match config.auth_info.offline_access() { + Some(refresh_token) => refresh_token.clone(), + None => { + let refresh_token = + handle_auth_flow(AuthAction::Login, None, ignore_certs, &config.connection.uri) + .await + .context("User login failed")?; + config.auth_info.set_offline_access(refresh_token.clone()); + refresh_token }, - None => handle_auth_flow(AuthAction::Login, ignore_certs, &config.connection.uri) - .await - .context("User login failed")?, }; - config.auth_info.set_offline_access(tokens.refresh_token.clone()); + let access_token = + handle_refresh_tokens(&refresh_token, ignore_certs, &config.connection.uri) + .await + .context("Token refresh failed")?; let mut headers = HeaderMap::new(); // the cli runs a command or a few short commands then exits, so we do @@ -183,7 +187,7 @@ impl PhylumApi { // here and be done. headers.insert( "Authorization", - HeaderValue::from_str(&format!("Bearer {}", tokens.access_token)).unwrap(), + HeaderValue::from_str(&format!("Bearer {}", access_token)).unwrap(), ); headers.insert("Accept", HeaderValue::from_str("application/json").unwrap()); @@ -195,7 +199,7 @@ impl PhylumApi { .build()?; // Try to parse token's roles. - let roles = jwt::user_roles(tokens.access_token.as_str()).unwrap_or_default(); + let roles = jwt::user_roles(access_token.as_str()).unwrap_or_default(); Ok(Self { config, client, roles }) } @@ -205,13 +209,14 @@ impl PhylumApi { /// credentials. It is the duty of the calling code to save any changes pub async fn login( mut auth_info: AuthInfo, + token_name: Option, ignore_certs: bool, api_uri: &str, reauth: bool, ) -> Result { let action = if reauth { AuthAction::Reauth } else { AuthAction::Login }; - let tokens = handle_auth_flow(action, ignore_certs, api_uri).await?; - auth_info.set_offline_access(tokens.refresh_token); + let refresh_token = handle_auth_flow(action, token_name, ignore_certs, api_uri).await?; + auth_info.set_offline_access(refresh_token); Ok(auth_info) } @@ -220,11 +225,13 @@ impl PhylumApi { /// credentials. It is the duty of the calling code to save any changes pub async fn register( mut auth_info: AuthInfo, + token_name: Option, ignore_certs: bool, api_uri: &str, ) -> Result { - let tokens = handle_auth_flow(AuthAction::Register, ignore_certs, api_uri).await?; - auth_info.set_offline_access(tokens.refresh_token); + let refresh_token = + handle_auth_flow(AuthAction::Register, token_name, ignore_certs, api_uri).await?; + auth_info.set_offline_access(refresh_token); Ok(auth_info) } @@ -238,10 +245,12 @@ impl PhylumApi { /// Get information about the authenticated user pub async fn user_info(&self) -> Result { - let oidc_settings = - fetch_oidc_server_settings(self.config.ignore_certs(), &self.config.connection.uri) - .await?; - self.get(oidc_settings.userinfo_endpoint).await + let locksmith_settings = fetch_locksmith_server_settings( + self.config.ignore_certs(), + &self.config.connection.uri, + ) + .await?; + self.get(locksmith_settings.userinfo_endpoint).await } /// Create a new project diff --git a/cli/src/app.rs b/cli/src/app.rs index a863c5f1e..82a54ed79 100644 --- a/cli/src/app.rs +++ b/cli/src/app.rs @@ -188,16 +188,33 @@ pub fn add_subcommands(command: Command) -> Command { .about("Manage authentication, registration, and API keys") .arg_required_else_help(true) .subcommand_required(true) - .subcommand(Command::new("register").about("Register a new account")) .subcommand( - Command::new("login").about("Login to an existing account").arg( - Arg::new("reauth") - .action(ArgAction::SetTrue) - .short('r') - .long("reauth") - .help("Force a login prompt"), + Command::new("register").about("Register a new account").arg( + Arg::new("token-name") + .action(ArgAction::Set) + .short('n') + .long("token-name") + .help("API token name"), ), ) + .subcommand( + Command::new("login") + .about("Login to an existing account") + .arg( + Arg::new("reauth") + .action(ArgAction::SetTrue) + .short('r') + .long("reauth") + .help("Force a login prompt"), + ) + .arg( + Arg::new("token-name") + .action(ArgAction::Set) + .short('n') + .long("token-name") + .help("API token name"), + ), + ) .subcommand( Command::new("status").about("Return the current authentication status"), ) diff --git a/cli/src/auth/mod.rs b/cli/src/auth/mod.rs index 08a464230..530677969 100644 --- a/cli/src/auth/mod.rs +++ b/cli/src/auth/mod.rs @@ -6,3 +6,7 @@ mod ip_addr_ext; pub mod jwt; mod oidc; mod server; + +pub fn is_locksmith_token(token: impl AsRef) -> bool { + token.as_ref().starts_with("ph0_") +} diff --git a/cli/src/auth/oidc.rs b/cli/src/auth/oidc.rs index 922127f82..0e112e5a6 100644 --- a/cli/src/auth/oidc.rs +++ b/cli/src/auth/oidc.rs @@ -9,14 +9,16 @@ use anyhow::{anyhow, Context, Result}; use base64::engine::general_purpose; use base64::Engine as _; use maplit::hashmap; -use phylum_types::types::auth::{AuthorizationCode, RefreshToken, TokenResponse}; +use phylum_types::types::auth::{AccessToken, AuthorizationCode, RefreshToken, TokenResponse}; use rand::distributions::Alphanumeric; use rand::{thread_rng, Rng}; use reqwest::Url; use serde::{Deserialize, Serialize}; use sha2::{Digest, Sha256}; +use uuid::Uuid; use super::ip_addr_ext::IpAddrExt; +use super::is_locksmith_token; use crate::api::endpoints; use crate::app::USER_AGENT; @@ -25,6 +27,9 @@ pub const OIDC_SCOPES: [&str; 2] = ["openid", "offline_access"]; /// OIDC Client id used to identify this client to the oidc server pub const OIDC_CLIENT_ID: &str = "phylum_cli"; +/// Client ID for Locksmith tokens +pub const LOCKSMITH_CLIENT_ID: &str = "locksmith"; + #[derive(Debug, PartialEq, Eq, Clone, Copy)] pub enum AuthAction { Login, @@ -94,26 +99,32 @@ impl<'a> From<&'a CodeVerifier> for &'a str { /// We only deserialize the ones we care about. #[derive(Debug, Serialize, Deserialize)] pub struct OidcServerSettings { - pub issuer: Url, + pub token_endpoint: Url, +} + +/// Locksmith URLs +#[derive(Debug, Serialize, Deserialize)] +pub struct LocksmithServerSettings { pub authorization_endpoint: Url, pub token_endpoint: Url, pub userinfo_endpoint: Url, } - /// Using config information, build the url for the keycloak login page. pub fn build_auth_url( action: AuthAction, - oidc_settings: &OidcServerSettings, + locksmith_settings: &LocksmithServerSettings, callback_url: &Url, code_challenge: &ChallengeCode, state: impl AsRef, ) -> Result { let mut auth_url = match action { // Login uses the oidc defined /auth endpoint as is - AuthAction::Login | AuthAction::Reauth => oidc_settings.authorization_endpoint.to_owned(), + AuthAction::Login | AuthAction::Reauth => { + locksmith_settings.authorization_endpoint.to_owned() + }, // Register uses the non-standard /registrations endpoint AuthAction::Register => { - let mut auth_url = oidc_settings.authorization_endpoint.to_owned(); + let mut auth_url = locksmith_settings.authorization_endpoint.to_owned(); auth_url .path_segments_mut() .map_err(|_| anyhow!("Can not be base url"))? @@ -126,7 +137,7 @@ pub fn build_auth_url( auth_url .query_pairs_mut() .clear() - .append_pair("client_id", OIDC_CLIENT_ID) + .append_pair("client_id", LOCKSMITH_CLIENT_ID) .append_pair("code_challenge", code_challenge.into()) .append_pair("code_challenge_method", "S256") .append_pair("redirect_uri", callback_url.as_ref()) @@ -175,18 +186,42 @@ pub async fn fetch_oidc_server_settings( } } +pub async fn fetch_locksmith_server_settings( + ignore_certs: bool, + api_uri: &str, +) -> Result { + let client = reqwest::Client::builder() + .user_agent(USER_AGENT.as_str()) + .danger_accept_invalid_certs(ignore_certs) + .build()?; + let response = client + .get(endpoints::locksmith_discovery(api_uri)?) + .header("Accept", "application/json") + .timeout(Duration::from_secs(5)) + .send() + .await?; + + if let Err(error) = response.error_for_status_ref() { + Err(anyhow!(response.text().await?)).context(error) + } else { + Ok(response.json::().await?) + } +} + fn build_grant_type_auth_code_post_body( redirect_url: &Url, authorization_code: &AuthorizationCode, code_verfier: &CodeVerifier, + token_name: Option, ) -> Result> { let body = hashmap! { - "client_id".to_owned() => OIDC_CLIENT_ID.to_owned(), + "client_id".to_owned() => LOCKSMITH_CLIENT_ID.to_owned(), "code".to_owned() => authorization_code.into(), "code_verifier".to_owned() => code_verfier.into(), "grant_type".to_owned() => "authorization_code".to_owned(), // Must match previous request to /authorize but not redirected to by server "redirect_uri".to_owned() => redirect_url.to_string(), + "name".to_owned() => token_name.unwrap_or_else(|| format!("phylum-cli-{}", Uuid::new_v4().as_hyphenated())), }; Ok(body) } @@ -204,16 +239,21 @@ fn build_grant_type_refresh_token_post_body( /// Acquire tokens with the auth code pub async fn acquire_tokens( - oidc_settings: &OidcServerSettings, + locksmith_settings: &LocksmithServerSettings, redirect_url: &Url, authorization_code: &AuthorizationCode, code_verifier: &CodeVerifier, + token_name: Option, ignore_certs: bool, -) -> Result { - let token_url = oidc_settings.token_endpoint.clone(); +) -> Result { + let token_url = locksmith_settings.token_endpoint.clone(); - let body = - build_grant_type_auth_code_post_body(redirect_url, authorization_code, code_verifier)?; + let body = build_grant_type_auth_code_post_body( + redirect_url, + authorization_code, + code_verifier, + token_name, + )?; let client = reqwest::Client::builder() .user_agent(USER_AGENT.as_str()) @@ -243,7 +283,7 @@ pub async fn acquire_tokens( err } else { - Ok(response.json::().await?) + Ok(response.json::().await?) } } @@ -281,9 +321,21 @@ pub async fn handle_refresh_tokens( refresh_token: &RefreshToken, ignore_certs: bool, api_uri: &str, -) -> Result { +) -> Result { + // Locksmith tokens are their own access tokens + if is_locksmith_token(refresh_token) { + return Ok(AccessToken::new(refresh_token)); + } + let oidc_settings = fetch_oidc_server_settings(ignore_certs, api_uri).await?; - refresh_tokens(&oidc_settings, refresh_token, ignore_certs).await + refresh_tokens(&oidc_settings, refresh_token, ignore_certs) + .await + .map(|token| token.access_token) +} + +#[derive(Debug, Clone, Deserialize)] +pub struct LocksmithTokenResponse { + pub token: RefreshToken, } /// Represents the userdata stored for an authentication token. diff --git a/cli/src/auth/server.rs b/cli/src/auth/server.rs index 6cbdf3e9f..6bbd3a498 100644 --- a/cli/src/auth/server.rs +++ b/cli/src/auth/server.rs @@ -8,7 +8,7 @@ use futures::TryFutureExt; use hyper::{Body, Request, Response, Server}; #[cfg(not(test))] use open; -use phylum_types::types::auth::{AuthorizationCode, TokenResponse}; +use phylum_types::types::auth::{AuthorizationCode, RefreshToken}; use rand::distributions::Alphanumeric; use rand::{thread_rng, Rng}; use reqwest::Url; @@ -18,8 +18,8 @@ use tokio::sync::oneshot::{self, Sender}; use tokio::sync::Mutex; use super::oidc::{ - acquire_tokens, build_auth_url, check_if_routable, fetch_oidc_server_settings, AuthAction, - ChallengeCode, CodeVerifier, OidcServerSettings, + acquire_tokens, build_auth_url, check_if_routable, fetch_locksmith_server_settings, AuthAction, + ChallengeCode, CodeVerifier, LocksmithServerSettings, }; #[cfg(test)] use crate::test::open; @@ -130,7 +130,7 @@ async fn keycloak_callback_handler(request: Request) -> Result + 'static, @@ -168,7 +168,7 @@ async fn spawn_server_and_get_auth_code( }); let authorization_url = - build_auth_url(redirect_type, oidc_settings, &callback_url, code_challenge, state)?; + build_auth_url(redirect_type, locksmith_settings, &callback_url, code_challenge, state)?; log::debug!("Authorization url is {}", authorization_url); @@ -219,15 +219,26 @@ async fn spawn_server_and_get_auth_code( /// Handle the user login/registration flow. pub async fn handle_auth_flow( auth_action: AuthAction, + token_name: Option, ignore_certs: bool, api_uri: &str, -) -> Result { - let oidc_settings = fetch_oidc_server_settings(ignore_certs, api_uri).await?; +) -> Result { + let locksmith_settings = fetch_locksmith_server_settings(ignore_certs, api_uri).await?; let (code_verifier, challenge_code) = CodeVerifier::generate(64)?; let state: String = thread_rng().sample_iter(&Alphanumeric).take(32).map(char::from).collect(); let (auth_code, callback_url) = - spawn_server_and_get_auth_code(&oidc_settings, auth_action, &challenge_code, state).await?; - acquire_tokens(&oidc_settings, &callback_url, &auth_code, &code_verifier, ignore_certs).await + spawn_server_and_get_auth_code(&locksmith_settings, auth_action, &challenge_code, state) + .await?; + acquire_tokens( + &locksmith_settings, + &callback_url, + &auth_code, + &code_verifier, + token_name, + ignore_certs, + ) + .await + .map(|tokens| tokens.token) } #[cfg(test)] @@ -243,7 +254,8 @@ mod test { #[tokio::test] async fn when_started_with_good_configuration_spawn_server_and_get_auth_code_is_successful( ) -> Result<()> { - let oidc_settings = build_oidc_server_settings_mock_response("https://127.0.0.1/oauth"); + let locksmith_settings = + build_locksmith_server_settings_mock_response("https://127.0.0.1/oauth"); let (_verifier, challenge) = CodeVerifier::generate(64).expect("Failed to build PKCE verifier and challenge"); @@ -251,7 +263,7 @@ mod test { let state: String = thread_rng().sample_iter(&Alphanumeric).take(32).map(char::from).collect(); - spawn_server_and_get_auth_code(&oidc_settings, AuthAction::Login, &challenge, state) + spawn_server_and_get_auth_code(&locksmith_settings, AuthAction::Login, &challenge, state) .await?; Ok(()) @@ -266,7 +278,7 @@ mod test { let (_verifier, _challenge) = CodeVerifier::generate(64).expect("Failed to build PKCE verifier and challenge"); - let result = handle_auth_flow(AuthAction::Login, false, &api_uri).await?; + let result = handle_auth_flow(AuthAction::Login, None, false, &api_uri).await?; log::debug!("{:?}", result); @@ -282,7 +294,7 @@ mod test { let (_verifier, _challenge) = CodeVerifier::generate(64).expect("Failed to build PKCE verifier and challenge"); - let result = handle_auth_flow(AuthAction::Register, false, &api_uri).await?; + let result = handle_auth_flow(AuthAction::Register, None, false, &api_uri).await?; log::debug!("{:?}", result); diff --git a/cli/src/commands/auth.rs b/cli/src/commands/auth.rs index f821d7cb1..e549d8454 100644 --- a/cli/src/commands/auth.rs +++ b/cli/src/commands/auth.rs @@ -2,20 +2,32 @@ use std::path::Path; use anyhow::{anyhow, Context, Result}; use clap::ArgMatches; +use log::debug; use phylum_types::types::auth::RefreshToken; use tokio::io::{self, AsyncBufReadExt, BufReader}; use crate::api::PhylumApi; +use crate::auth::is_locksmith_token; use crate::commands::{CommandResult, ExitCode}; use crate::config::{save_config, Config}; use crate::{auth, print_user_success, print_user_warning}; /// Register a user. Opens a browser, and redirects the user to the oauth server /// registration page -async fn handle_auth_register(mut config: Config, config_path: &Path) -> Result<()> { +async fn handle_auth_register( + mut config: Config, + config_path: &Path, + matches: &ArgMatches, +) -> Result<()> { let api_uri = &config.connection.uri; let ignore_certs = config.ignore_certs(); - config.auth_info = PhylumApi::register(config.auth_info, ignore_certs, api_uri).await?; + config.auth_info = PhylumApi::register( + config.auth_info, + matches.get_one("token-name").cloned(), + ignore_certs, + api_uri, + ) + .await?; save_config(config_path, &config).map_err(|error| anyhow!(error))?; Ok(()) } @@ -29,30 +41,46 @@ async fn handle_auth_login( ) -> Result<()> { let api_uri = &config.connection.uri; let ignore_certs = config.ignore_certs(); - config.auth_info = - PhylumApi::login(config.auth_info, ignore_certs, api_uri, matches.get_flag("reauth")) - .await?; + config.auth_info = PhylumApi::login( + config.auth_info, + matches.get_one("token-name").cloned(), + ignore_certs, + api_uri, + matches.get_flag("reauth"), + ) + .await?; save_config(config_path, &config).map_err(|error| anyhow!(error))?; Ok(()) } /// Display the current authentication status to the user. pub async fn handle_auth_status(config: Config, timeout: Option) -> CommandResult { - if config.auth_info.offline_access().is_none() { - print_user_warning!("User is not currently authenticated"); - return Ok(ExitCode::NotAuthenticated); - } + let auth_type = match config.auth_info.offline_access() { + Some(token) if is_locksmith_token(token) => "API key", + Some(_) => "OpenID Connect", + None => { + print_user_warning!("User is not currently authenticated"); + return Ok(ExitCode::NotAuthenticated); + }, + }; // Create a client with our auth token attached. let api = PhylumApi::new(config, timeout).await?; let user_info = api.user_info().await; + debug!("User info reponse: {:?}", user_info); + match user_info { Ok(user) => { print_user_success!( - "Currently authenticated as '{}' with long lived refresh token", - user.email + "Currently authenticated as '{}<{}>' via {}", + user.name.map_or_else(Default::default, |mut n| { + n.push(' '); + n + }), + user.email, + auth_type, ); Ok(ExitCode::Ok) }, @@ -77,9 +105,9 @@ pub async fn handle_auth_token(config: &Config, matches: &clap::ArgMatches) -> C if matches.get_flag("bearer") { let api_uri = &config.connection.uri; - let tokens = + let access_token = auth::handle_refresh_tokens(refresh_token, config.ignore_certs(), api_uri).await?; - println!("{}", tokens.access_token); + println!("{}", access_token); Ok(ExitCode::Ok) } else { println!("{refresh_token}"); @@ -127,7 +155,7 @@ pub async fn handle_auth( timeout: Option, ) -> CommandResult { match matches.subcommand() { - Some(("register", _)) => match handle_auth_register(config, config_path).await { + Some(("register", _)) => match handle_auth_register(config, config_path, matches).await { Ok(_) => { print_user_success!("{}", "User successfuly regsistered"); Ok(ExitCode::Ok) diff --git a/cli/src/commands/extensions/api.rs b/cli/src/commands/extensions/api.rs index 379dca557..b87779741 100644 --- a/cli/src/commands/extensions/api.rs +++ b/cli/src/commands/extensions/api.rs @@ -211,8 +211,7 @@ async fn get_access_token( let access_token = crate::auth::handle_refresh_tokens(&refresh_token, ignore_certs, &config.connection.uri) - .await? - .access_token; + .await?; Ok(access_token) } diff --git a/cli/src/test.rs b/cli/src/test.rs index 036f2ff53..45b87a355 100644 --- a/cli/src/test.rs +++ b/cli/src/test.rs @@ -20,22 +20,26 @@ pub mod mockito { use phylum_types::types::auth::{AccessToken, IdToken, RefreshToken, TokenResponse}; use reqwest::Url; + use serde_json::json; use wiremock::matchers::{method, path}; use wiremock::{Mock, MockBuilder, MockServer, Request, Respond, ResponseTemplate}; use crate::api::{PhylumApi, PhylumApiError}; - use crate::auth::OidcServerSettings; + use crate::auth::{LocksmithServerSettings, OidcServerSettings}; use crate::config::{AuthInfo, Config, ConnectionInfo}; pub const DUMMY_REFRESH_TOKEN: &str = "DUMMY_REFRESH_TOKEN"; + pub const DUMMY_LOCKSMITH_TOKEN: &str = "ph0_DUMMY_TOKEN"; pub const DUMMY_ACCESS_TOKEN: &str = "DUMMY_ACCESS_TOKEN"; pub const DUMMY_ID_TOKEN: &str = "DUMMY_ID_TOKEN"; pub const DUMMY_AUTH_CODE: &str = "DUMMY_AUTH_CODE"; pub const OIDC_URI: &str = "api/v0/.well-known/openid-configuration"; + pub const LOCKSMITH_URI: &str = "api/v0/.well-known/locksmith-configuration"; pub const AUTH_URI: &str = "auth"; pub const USER_URI: &str = "user"; pub const TOKEN_URI: &str = "token"; + pub const LOCKSMITH_TOKEN_URI: &str = "locksmith-token"; pub struct ResponderFn(F) where @@ -67,10 +71,16 @@ pub mod mockito { pub fn build_oidc_server_settings_mock_response(base_uri: &str) -> OidcServerSettings { let base_url = Url::from_str(base_uri).expect("Failed to parse base url"); - OidcServerSettings { - issuer: base_url.clone(), + OidcServerSettings { token_endpoint: base_url.join(TOKEN_URI).unwrap() } + } + + pub fn build_locksmith_server_settings_mock_response( + base_uri: &str, + ) -> LocksmithServerSettings { + let base_url = Url::from_str(base_uri).expect("Failed to parse base url"); + LocksmithServerSettings { authorization_endpoint: base_url.join(AUTH_URI).unwrap(), - token_endpoint: base_url.join(TOKEN_URI).unwrap(), + token_endpoint: base_url.join(LOCKSMITH_TOKEN_URI).unwrap(), userinfo_endpoint: base_url.join(USER_URI).unwrap(), } } @@ -89,6 +99,18 @@ pub mod mockito { .mount(&mock_server) .await; + let base_url = mock_server.uri(); + + // Set Locksmith Server Settings Response + Mock::given(method("GET")) + .and(path(LOCKSMITH_URI)) + .respond_with_fn(move |_| { + let locksmith_response = build_locksmith_server_settings_mock_response(&base_url); + ResponseTemplate::new(200).set_body_json(locksmith_response) + }) + .mount(&mock_server) + .await; + // Set auth endpoint response Mock::given(method("POST")) .and(path(AUTH_URI)) @@ -119,6 +141,17 @@ pub mod mockito { .mount(&mock_server) .await; + // Set locksmith token endpoint response + Mock::given(method("POST")) + .and(path(LOCKSMITH_TOKEN_URI)) + .respond_with_fn(|_| { + ResponseTemplate::new(200).set_body_json(json!({ + "token": DUMMY_LOCKSMITH_TOKEN, + })) + }) + .mount(&mock_server) + .await; + mock_server } diff --git a/docs/command_line_tool/phylum_auth_login.md b/docs/command_line_tool/phylum_auth_login.md index 1373be7d1..c587e7da2 100644 --- a/docs/command_line_tool/phylum_auth_login.md +++ b/docs/command_line_tool/phylum_auth_login.md @@ -16,6 +16,9 @@ Usage: phylum auth login [OPTIONS] -r, --reauth   Force a login prompt +-n, --token-name +  API token name + -v, --verbose...   Increase the level of verbosity (the maximum is -vvv) diff --git a/docs/command_line_tool/phylum_auth_register.md b/docs/command_line_tool/phylum_auth_register.md index 2fed717aa..b68858190 100644 --- a/docs/command_line_tool/phylum_auth_register.md +++ b/docs/command_line_tool/phylum_auth_register.md @@ -13,6 +13,9 @@ Usage: phylum auth register [OPTIONS] ### Options +-n, --token-name +  API token name + -v, --verbose...   Increase the level of verbosity (the maximum is -vvv) From db10bc4721b8d5c67b30621c262921114da46e3e Mon Sep 17 00:00:00 2001 From: Christian Duerr Date: Fri, 4 Aug 2023 18:00:06 +0200 Subject: [PATCH 3/4] Revert "fix: update lockfile detection to search root directory first (#1173)" This reverts commit 6b5d8f85f5a1decb3b00e0d3c0fa86ad69d276a0. --- CHANGELOG.md | 3 --- cli/src/commands/parse.rs | 26 -------------------------- 2 files changed, 29 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d46e825b2..fda6cc40f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,9 +11,6 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). ### Changed - Include lockfile paths when analyzing projects -### Fixed -- Detect lockfile in current path before searching in subdirectories - ## [5.5.0] - 2023-07-18 ### Added diff --git a/cli/src/commands/parse.rs b/cli/src/commands/parse.rs index 7499a5bac..f3c3332c7 100644 --- a/cli/src/commands/parse.rs +++ b/cli/src/commands/parse.rs @@ -181,26 +181,7 @@ fn find_manifest_format(path: &Path) -> Option<(LockfileFormat, Option) // Look for formats which already have a lockfile generated. let manifest_lockfile = formats.find_map(|format| { - // Check the root directory for a lockfile - let root_dir_entries = match fs::read_dir(manifest_dir) { - Ok(entries) => entries, - Err(_) => return None, - }; - - for entry_result in root_dir_entries { - match entry_result { - Ok(entry) => { - if format.parser().is_path_lockfile(&entry.path()) { - return Some((format, entry.path())); - } - }, - Err(_) => continue, - } - } - - // If not found in root, then use WalkDir to search in the subdirectories let manifest_lockfile = WalkDir::new(manifest_dir) - .min_depth(1) .into_iter() .flatten() .find(|entry| format.parser().is_path_lockfile(entry.path()))?; @@ -307,11 +288,4 @@ mod tests { assert_eq!(parsed.format, expected_format, "{}", file); } } - - #[test] - fn it_can_detect_correct_lockfile() { - let cli_cargo_toml = PathBuf::from("../Cargo.toml"); - let (_, path) = find_manifest_format(&cli_cargo_toml).unwrap(); - assert!(path.unwrap().ends_with("cli/Cargo.lock")); - } } From 3ee4001059f8f732868e7ee293158d0aadc25a68 Mon Sep 17 00:00:00 2001 From: Christian Duerr Date: Fri, 4 Aug 2023 18:43:16 +0200 Subject: [PATCH 4/4] Reverse search direction for manifests' lockfiles Previously when a manifest was specified as the target for an analyze or parse operation, the directory would be walked downwards depth-first to find a lockfile for this manifest. This would both prefer lockfiles farther away from the manifest and allow lockfiles from subdirectories. This patch reverses the search direction for lockfiles, since they're generally at or above their corresponding manifest. The new logic prefers lockfiles closest to the manifest and searches the directory tree upwards from there. Closes #1170. --- CHANGELOG.md | 3 ++ cli/src/commands/parse.rs | 81 ++++++++++++++++++++++++++++++++++++--- 2 files changed, 78 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fda6cc40f..6923c8635 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,9 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). ### Changed - Include lockfile paths when analyzing projects +### Fixed +- Search for manifests' lockfiles in parent, rather than child directories + ## [5.5.0] - 2023-07-18 ### Added diff --git a/cli/src/commands/parse.rs b/cli/src/commands/parse.rs index f3c3332c7..5690ba957 100644 --- a/cli/src/commands/parse.rs +++ b/cli/src/commands/parse.rs @@ -7,7 +7,6 @@ use std::{fs, io}; use anyhow::{anyhow, Context, Result}; use phylum_lockfile::{LockfileFormat, Package, PackageVersion, Parse, ThirdPartyVersion}; use phylum_types::types::package::{PackageDescriptor, PackageDescriptorAndLockfile}; -use walkdir::WalkDir; use crate::commands::{CommandResult, ExitCode}; use crate::{config, print_user_warning}; @@ -181,11 +180,10 @@ fn find_manifest_format(path: &Path) -> Option<(LockfileFormat, Option) // Look for formats which already have a lockfile generated. let manifest_lockfile = formats.find_map(|format| { - let manifest_lockfile = WalkDir::new(manifest_dir) - .into_iter() - .flatten() - .find(|entry| format.parser().is_path_lockfile(entry.path()))?; - Some((format, manifest_lockfile.path().to_owned())) + let manifest_lockfile = find_direntry_upwards::<32, _>(manifest_dir, |path| { + format.parser().is_path_lockfile(path) + })?; + Some((format, manifest_lockfile)) }); // Return existing lockfile or format capable of generating it. @@ -251,8 +249,36 @@ fn filter_packages(mut packages: Vec) -> Vec { .collect() } +/// Find a file by walking from a directory towards the root. +/// +/// `MAX_DEPTH` is the maximum number of directory traversals before the search +/// will be abandoned. A `MAX_DEPTH` of `0` will only search the `origin` +/// directory. +fn find_direntry_upwards( + mut origin: &Path, + mut predicate: P, +) -> Option +where + P: FnMut(&Path) -> bool, +{ + for _ in 0..=MAX_DEPTH { + for entry in fs::read_dir(origin).ok()?.flatten() { + let path = entry.path(); + if predicate(&path) { + return Some(path); + } + } + + origin = origin.parent()?; + } + + None +} + #[cfg(test)] mod tests { + use std::fs::{self, File}; + use super::*; #[test] @@ -288,4 +314,47 @@ mod tests { assert_eq!(parsed.format, expected_format, "{}", file); } } + + #[test] + fn find_lockfile_for_manifest() { + let tempdir = tempfile::tempdir().unwrap(); + let tempdir = tempdir.path().canonicalize().unwrap(); + + // Create manifest. + let manifest_dir = tempdir.join("manifest"); + let manifest_path = manifest_dir.join("Cargo.toml"); + fs::create_dir_all(&manifest_dir).unwrap(); + File::create(&manifest_path).unwrap(); + + // Ensure lockfiles below manifest are ignored. + + // Create lockfile below manifest. + let child_lockfile_dir = manifest_dir.join("sub"); + fs::create_dir_all(&child_lockfile_dir).unwrap(); + File::create(child_lockfile_dir.join("Cargo.lock")).unwrap(); + + let (_, path) = find_manifest_format(&manifest_path).unwrap(); + + assert_eq!(path, None); + + // Accept lockfiles above the manifest. + + // Create lockfile above manifest. + let parent_lockfile_path = tempdir.join("Cargo.lock"); + File::create(&parent_lockfile_path).unwrap(); + + let (_, path) = find_manifest_format(&manifest_path).unwrap(); + + assert_eq!(path, Some(parent_lockfile_path)); + + // Prefer lockfiles "closer" to the manifest. + + // Create lockfile in the manifest directory. + let sibling_lockfile_path = manifest_dir.join("Cargo.lock"); + File::create(&sibling_lockfile_path).unwrap(); + + let (_, path) = find_manifest_format(&manifest_path).unwrap(); + + assert_eq!(path, Some(sibling_lockfile_path)); + } }