Skip to content

Commit 743dfa9

Browse files
committed
fix(providers): harden dynamic token grants
Signed-off-by: Taylor Mutch <taylormutch@gmail.com>
1 parent b5a54e0 commit 743dfa9

15 files changed

Lines changed: 962 additions & 91 deletions

File tree

‎Cargo.lock‎

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎crates/openshell-cli/src/main.rs‎

Lines changed: 65 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -716,7 +716,7 @@ impl From<CliEditor> for openshell_cli::ssh::Editor {
716716
#[derive(Subcommand, Debug)]
717717
enum ProviderCommands {
718718
/// Create a provider config.
719-
#[command(group = clap::ArgGroup::new("cred_source").required(true).args(["from_existing", "credentials", "from_gcloud_adc"]), help_template = LEAF_HELP_TEMPLATE, next_help_heading = "FLAGS")]
719+
#[command(group = clap::ArgGroup::new("cred_source").required(true).args(["from_existing", "credentials", "from_gcloud_adc", "runtime_credentials"]), help_template = LEAF_HELP_TEMPLATE, next_help_heading = "FLAGS")]
720720
Create {
721721
/// Provider name.
722722
#[arg(long)]
@@ -727,23 +727,27 @@ enum ProviderCommands {
727727
provider_type: String,
728728

729729
/// Load provider credentials/config from existing local state.
730-
#[arg(long, conflicts_with_all = ["credentials", "from_gcloud_adc"])]
730+
#[arg(long, conflicts_with_all = ["credentials", "from_gcloud_adc", "runtime_credentials"])]
731731
from_existing: bool,
732732

733733
/// Provider credential pair (`KEY=VALUE`) or env lookup key (`KEY`).
734734
#[arg(
735735
long = "credential",
736736
value_name = "KEY[=VALUE]",
737-
conflicts_with_all = ["from_existing", "from_gcloud_adc"]
737+
conflicts_with_all = ["from_existing", "from_gcloud_adc", "runtime_credentials"]
738738
)]
739739
credentials: Vec<String>,
740740

741741
/// Configure credentials from gcloud Application Default Credentials
742742
/// (`~/.config/gcloud/application_default_credentials.json`).
743743
/// Only valid for google-vertex-ai providers.
744-
#[arg(long, group = "cred_source", conflicts_with_all = ["from_existing", "credentials"])]
744+
#[arg(long, group = "cred_source", conflicts_with_all = ["from_existing", "credentials", "runtime_credentials"])]
745745
from_gcloud_adc: bool,
746746

747+
/// Create a provider whose required credentials are resolved at runtime by the gateway/sandbox.
748+
#[arg(long, conflicts_with_all = ["from_existing", "credentials", "from_gcloud_adc"])]
749+
runtime_credentials: bool,
750+
747751
/// Provider config key/value pair.
748752
#[arg(long = "config", value_name = "KEY=VALUE")]
749753
config: Vec<String>,
@@ -2814,15 +2818,17 @@ async fn main() -> Result<()> {
28142818
from_existing,
28152819
credentials,
28162820
from_gcloud_adc,
2821+
runtime_credentials,
28172822
config,
28182823
} => {
2819-
run::provider_create(
2824+
run::provider_create_with_options(
28202825
endpoint,
28212826
&name,
28222827
provider_type.as_str(),
28232828
from_existing,
28242829
&credentials,
28252830
from_gcloud_adc,
2831+
runtime_credentials,
28262832
&config,
28272833
&tls,
28282834
)
@@ -3892,6 +3898,60 @@ mod tests {
38923898
}
38933899
}
38943900

3901+
#[test]
3902+
fn provider_create_requires_credential_source() {
3903+
let err = Cli::try_parse_from([
3904+
"openshell",
3905+
"provider",
3906+
"create",
3907+
"--name",
3908+
"spiffe-token-demo",
3909+
"--type",
3910+
"spiffe-token-demo",
3911+
])
3912+
.expect_err("provider create should require a credential source");
3913+
3914+
assert!(err.to_string().contains("--runtime-credentials"));
3915+
}
3916+
3917+
#[test]
3918+
fn provider_create_accepts_runtime_credentials() {
3919+
let cli = Cli::try_parse_from([
3920+
"openshell",
3921+
"provider",
3922+
"create",
3923+
"--name",
3924+
"spiffe-token-demo",
3925+
"--type",
3926+
"spiffe-token-demo",
3927+
"--runtime-credentials",
3928+
])
3929+
.expect("provider create should parse runtime credentials");
3930+
3931+
match cli.command {
3932+
Some(Commands::Provider {
3933+
command:
3934+
Some(ProviderCommands::Create {
3935+
name,
3936+
provider_type,
3937+
from_existing,
3938+
credentials,
3939+
from_gcloud_adc,
3940+
runtime_credentials,
3941+
..
3942+
}),
3943+
}) => {
3944+
assert_eq!(name, "spiffe-token-demo");
3945+
assert_eq!(provider_type, "spiffe-token-demo");
3946+
assert!(!from_existing);
3947+
assert!(credentials.is_empty());
3948+
assert!(!from_gcloud_adc);
3949+
assert!(runtime_credentials);
3950+
}
3951+
other => panic!("expected provider create command, got: {other:?}"),
3952+
}
3953+
}
3954+
38953955
#[test]
38963956
fn provider_create_rejects_from_gcloud_adc_with_from_existing() {
38973957
let err = Cli::try_parse_from([

‎crates/openshell-cli/src/run.rs‎

Lines changed: 83 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -4459,7 +4459,8 @@ fn missing_credentials_error(provider_type: &str) -> miette::Report {
44594459

44604460
miette::miette!(
44614461
"no credentials resolved for provider type '{provider_type}'. \
4462-
Use --credential KEY[=VALUE] or --from-existing with the appropriate env vars set."
4462+
Use --credential KEY[=VALUE], --runtime-credentials for runtime-resolved profile credentials, \
4463+
or --from-existing with the appropriate env vars set."
44634464
)
44644465
}
44654466

@@ -4474,14 +4475,45 @@ pub async fn provider_create(
44744475
config: &[String],
44754476
tls: &TlsOptions,
44764477
) -> Result<()> {
4477-
if from_gcloud_adc && (from_existing || !credentials.is_empty()) {
4478+
provider_create_with_options(
4479+
server,
4480+
name,
4481+
provider_type,
4482+
from_existing,
4483+
credentials,
4484+
from_gcloud_adc,
4485+
false,
4486+
config,
4487+
tls,
4488+
)
4489+
.await
4490+
}
4491+
4492+
#[allow(clippy::too_many_arguments)]
4493+
pub async fn provider_create_with_options(
4494+
server: &str,
4495+
name: &str,
4496+
provider_type: &str,
4497+
from_existing: bool,
4498+
credentials: &[String],
4499+
from_gcloud_adc: bool,
4500+
runtime_credentials: bool,
4501+
config: &[String],
4502+
tls: &TlsOptions,
4503+
) -> Result<()> {
4504+
if from_gcloud_adc && (from_existing || !credentials.is_empty() || runtime_credentials) {
44784505
return Err(miette::miette!(
4479-
"--from-gcloud-adc cannot be combined with --from-existing or --credential"
4506+
"--from-gcloud-adc cannot be combined with --from-existing or --credential; it also cannot be combined with --runtime-credentials"
44804507
));
44814508
}
4482-
if from_existing && !credentials.is_empty() {
4509+
if from_existing && (!credentials.is_empty() || runtime_credentials) {
44834510
return Err(miette::miette!(
4484-
"--from-existing cannot be combined with --credential"
4511+
"--from-existing cannot be combined with --credential or --runtime-credentials"
4512+
));
4513+
}
4514+
if runtime_credentials && !credentials.is_empty() {
4515+
return Err(miette::miette!(
4516+
"--runtime-credentials cannot be combined with --credential"
44854517
));
44864518
}
44874519

@@ -4544,11 +4576,25 @@ pub async fn provider_create(
45444576
if from_existing {
45454577
return Err(missing_credentials_error(&provider_type));
45464578
}
4547-
let allows_refresh_bootstrap = fetch_provider_profile(&mut client, &provider_type)
4548-
.await
4549-
.ok()
4550-
.is_some_and(|profile| provider_profile_allows_refresh_bootstrap(&profile));
4551-
if !allows_refresh_bootstrap {
4579+
if !from_gcloud_adc && !runtime_credentials {
4580+
return Err(missing_credentials_error(&provider_type));
4581+
}
4582+
let allows_empty_credentials = if runtime_credentials {
4583+
provider_profile_allows_empty_credentials(
4584+
&fetch_provider_profile(&mut client, &provider_type).await?,
4585+
)
4586+
} else {
4587+
fetch_provider_profile(&mut client, &provider_type)
4588+
.await
4589+
.ok()
4590+
.is_some_and(|profile| provider_profile_allows_empty_credentials(&profile))
4591+
};
4592+
if !allows_empty_credentials {
4593+
if runtime_credentials {
4594+
return Err(miette::miette!(
4595+
"--runtime-credentials is only valid for provider profiles whose required credentials are resolved at runtime"
4596+
));
4597+
}
45524598
return Err(missing_credentials_error(&provider_type));
45534599
}
45544600
}
@@ -4643,8 +4689,8 @@ pub async fn provider_create(
46434689
Ok(())
46444690
}
46454691

4646-
fn provider_profile_allows_refresh_bootstrap(profile: &ProviderProfile) -> bool {
4647-
ProviderTypeProfile::from_proto(profile).allows_gateway_refresh_bootstrap()
4692+
fn provider_profile_allows_empty_credentials(profile: &ProviderProfile) -> bool {
4693+
ProviderTypeProfile::from_proto(profile).allows_empty_provider_credentials()
46484694
}
46494695

46504696
pub async fn provider_get(server: &str, name: &str, tls: &TlsOptions) -> Result<()> {
@@ -7523,7 +7569,7 @@ mod tests {
75237569
inferred_provider_type, mtls_certs_exist_for_gateway, package_managed_tls_dirs,
75247570
parse_cli_setting_value, parse_credential_expiry_cli_value, parse_credential_expiry_pairs,
75257571
parse_credential_pairs, parse_driver_config_json, plaintext_gateway_is_remote,
7526-
progress_step_from_metadata, provider_profile_allows_refresh_bootstrap,
7572+
progress_step_from_metadata, provider_profile_allows_empty_credentials,
75277573
provisioning_timeout_message, ready_false_condition_message, refresh_status_header,
75287574
refresh_status_row, resolve_from, sandbox_should_persist, sandbox_upload_plan,
75297575
service_expose_status_error, service_url_for_gateway,
@@ -7546,8 +7592,8 @@ mod tests {
75467592
};
75477593
use openshell_core::proto::{
75487594
Provider, ProviderCredentialRefresh, ProviderCredentialRefreshStatus,
7549-
ProviderCredentialRefreshStrategy, ProviderProfile, ProviderProfileCredential,
7550-
SandboxCondition, SandboxStatus, datamodel::v1::ObjectMeta,
7595+
ProviderCredentialRefreshStrategy, ProviderCredentialTokenGrant, ProviderProfile,
7596+
ProviderProfileCredential, SandboxCondition, SandboxStatus, datamodel::v1::ObjectMeta,
75517597
};
75527598

75537599
struct EnvVarGuard {
@@ -7774,7 +7820,7 @@ mod tests {
77747820
}
77757821

77767822
#[test]
7777-
fn refresh_bootstrap_requires_all_required_credentials_to_be_gateway_mintable() {
7823+
fn empty_provider_credentials_require_all_required_credentials_to_be_runtime_resolvable() {
77787824
let refresh_token_profile = ProviderProfile {
77797825
credentials: vec![ProviderProfileCredential {
77807826
name: "MS_GRAPH_ACCESS_TOKEN".to_string(),
@@ -7787,10 +7833,26 @@ mod tests {
77877833
}],
77887834
..Default::default()
77897835
};
7790-
assert!(provider_profile_allows_refresh_bootstrap(
7836+
assert!(provider_profile_allows_empty_credentials(
77917837
&refresh_token_profile
77927838
));
77937839

7840+
let token_grant_profile = ProviderProfile {
7841+
credentials: vec![ProviderProfileCredential {
7842+
name: "ACCESS_TOKEN".to_string(),
7843+
required: true,
7844+
token_grant: Some(ProviderCredentialTokenGrant {
7845+
token_endpoint: "https://auth.example.com/token".to_string(),
7846+
..Default::default()
7847+
}),
7848+
..Default::default()
7849+
}],
7850+
..Default::default()
7851+
};
7852+
assert!(provider_profile_allows_empty_credentials(
7853+
&token_grant_profile
7854+
));
7855+
77947856
let mixed_static_profile = ProviderProfile {
77957857
credentials: vec![
77967858
ProviderProfileCredential {
@@ -7811,7 +7873,7 @@ mod tests {
78117873
],
78127874
..Default::default()
78137875
};
7814-
assert!(!provider_profile_allows_refresh_bootstrap(
7876+
assert!(!provider_profile_allows_empty_credentials(
78157877
&mixed_static_profile
78167878
));
78177879

@@ -7827,7 +7889,7 @@ mod tests {
78277889
}],
78287890
..Default::default()
78297891
};
7830-
assert!(provider_profile_allows_refresh_bootstrap(
7892+
assert!(provider_profile_allows_empty_credentials(
78317893
&optional_refresh_profile
78327894
));
78337895
}
@@ -8888,7 +8950,7 @@ mod tests {
88888950
}
88898951

88908952
#[test]
8891-
fn refresh_bootstrap_allows_oauth2_refresh_token() {
8953+
fn empty_provider_credentials_allow_oauth2_refresh_token() {
88928954
use openshell_core::proto::{
88938955
ProviderCredentialRefresh, ProviderCredentialRefreshStrategy, ProviderProfile,
88948956
ProviderProfileCredential,
@@ -8907,7 +8969,7 @@ mod tests {
89078969
..Default::default()
89088970
};
89098971
assert!(
8910-
provider_profile_allows_refresh_bootstrap(&profile),
8972+
provider_profile_allows_empty_credentials(&profile),
89118973
"Oauth2RefreshToken should be allowed for refresh bootstrap"
89128974
);
89138975
}

0 commit comments

Comments
 (0)