mirror of
https://github.com/NVIDIA/OpenShell.git
synced 2026-10-23 06:30:28 +08:00
feat(providersv2): add path auth_style (#1622)
Signed-off-by: Calum Murray <cmurray@redhat.com>
This commit is contained in:
@@ -96,6 +96,7 @@ mod tests {
|
||||
header_name: String::new(),
|
||||
query_param: String::new(),
|
||||
refresh: None,
|
||||
path_template: String::new(),
|
||||
},
|
||||
CredentialProfile {
|
||||
name: "secondary".to_string(),
|
||||
@@ -106,6 +107,7 @@ mod tests {
|
||||
header_name: String::new(),
|
||||
query_param: String::new(),
|
||||
refresh: None,
|
||||
path_template: String::new(),
|
||||
},
|
||||
],
|
||||
endpoints: Vec::new(),
|
||||
|
||||
@@ -16,6 +16,8 @@ use serde::{Deserialize, Deserializer, Serialize, Serializer, de};
|
||||
use std::collections::{HashMap, HashSet};
|
||||
use std::sync::OnceLock;
|
||||
|
||||
const PATH_TEMPLATE_CREDENTIAL_PLACEHOLDER: &str = "{credential}";
|
||||
|
||||
const BUILT_IN_PROFILE_YAMLS: &[&str] = &[
|
||||
include_str!("../../../providers/claude-code.yaml"),
|
||||
include_str!("../../../providers/codex.yaml"),
|
||||
@@ -86,6 +88,8 @@ pub struct CredentialProfile {
|
||||
pub query_param: String,
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
pub refresh: Option<CredentialRefreshProfile>,
|
||||
#[serde(default, skip_serializing_if = "String::is_empty")]
|
||||
pub path_template: String,
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, Deserialize, Serialize, PartialEq, Eq)]
|
||||
@@ -285,6 +289,7 @@ impl ProviderTypeProfile {
|
||||
.refresh
|
||||
.as_ref()
|
||||
.map(credential_refresh_from_proto),
|
||||
path_template: credential.path_template.clone(),
|
||||
})
|
||||
.collect(),
|
||||
endpoints: profile.endpoints.iter().map(endpoint_from_proto).collect(),
|
||||
@@ -349,6 +354,7 @@ impl ProviderTypeProfile {
|
||||
header_name: credential.header_name.clone(),
|
||||
query_param: credential.query_param.clone(),
|
||||
refresh: credential.refresh.as_ref().map(credential_refresh_to_proto),
|
||||
path_template: credential.path_template.clone(),
|
||||
})
|
||||
.collect(),
|
||||
endpoints: self.endpoints.iter().map(endpoint_to_proto).collect(),
|
||||
@@ -1003,6 +1009,31 @@ pub fn validate_profile_set(
|
||||
));
|
||||
}
|
||||
}
|
||||
"path" => {
|
||||
let path_template = credential.path_template.trim();
|
||||
if path_template.is_empty() {
|
||||
diagnostics.push(ProfileValidationDiagnostic::error(
|
||||
source,
|
||||
profile_id,
|
||||
"credentials.path_template",
|
||||
"path_template is required for path auth",
|
||||
));
|
||||
} else {
|
||||
let count = path_template
|
||||
.matches(PATH_TEMPLATE_CREDENTIAL_PLACEHOLDER)
|
||||
.count();
|
||||
if count != 1 {
|
||||
diagnostics.push(ProfileValidationDiagnostic::error(
|
||||
source,
|
||||
profile_id,
|
||||
"credentials.path_template",
|
||||
format!(
|
||||
"path_template should contain {{credential}} exactly once, {path_template} contains {{credential}} {count} times",
|
||||
),
|
||||
));
|
||||
}
|
||||
}
|
||||
}
|
||||
"query" => {
|
||||
if credential.query_param.trim().is_empty() {
|
||||
diagnostics.push(ProfileValidationDiagnostic::error(
|
||||
@@ -1351,6 +1382,63 @@ credentials:
|
||||
assert!(exported.contains("client_secret"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn credential_fields_round_trip_through_proto_and_yaml() {
|
||||
let profile = parse_profile_yaml(
|
||||
r"
|
||||
id: multi-auth
|
||||
display_name: Multi Auth
|
||||
credentials:
|
||||
- name: basic_cred
|
||||
env_vars: [BASIC_TOKEN]
|
||||
auth_style: basic
|
||||
- name: bearer_cred
|
||||
env_vars: [BEARER_TOKEN]
|
||||
auth_style: bearer
|
||||
header_name: authorization
|
||||
- name: query_cred
|
||||
env_vars: [QUERY_TOKEN]
|
||||
auth_style: query
|
||||
query_param: api_key
|
||||
- name: path_cred
|
||||
env_vars: [PATH_TOKEN]
|
||||
auth_style: path
|
||||
path_template: /v1/{credential}/resources
|
||||
",
|
||||
)
|
||||
.expect("profile should parse");
|
||||
|
||||
let diagnostics = validate_profile_set(&[("multi-auth.yaml".to_string(), profile.clone())]);
|
||||
assert!(
|
||||
diagnostics.is_empty(),
|
||||
"unexpected diagnostics: {diagnostics:?}"
|
||||
);
|
||||
|
||||
assert_eq!(profile.credentials[1].header_name, "authorization");
|
||||
assert_eq!(profile.credentials[2].query_param, "api_key");
|
||||
assert_eq!(
|
||||
profile.credentials[3].path_template,
|
||||
"/v1/{credential}/resources"
|
||||
);
|
||||
|
||||
let from_proto = ProviderTypeProfile::from_proto(&profile.to_proto());
|
||||
assert_eq!(from_proto.credentials[1].header_name, "authorization");
|
||||
assert_eq!(from_proto.credentials[2].query_param, "api_key");
|
||||
assert_eq!(
|
||||
from_proto.credentials[3].path_template,
|
||||
"/v1/{credential}/resources"
|
||||
);
|
||||
|
||||
let exported = profile_to_yaml(&from_proto).expect("yaml");
|
||||
let reparsed = parse_profile_yaml(&exported).expect("re-parse");
|
||||
assert_eq!(reparsed.credentials[1].header_name, "authorization");
|
||||
assert_eq!(reparsed.credentials[2].query_param, "api_key");
|
||||
assert_eq!(
|
||||
reparsed.credentials[3].path_template,
|
||||
"/v1/{credential}/resources"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn profile_json_round_trip_preserves_compact_dto_shape() {
|
||||
let profile = get_default_profile("github").expect("github profile");
|
||||
@@ -1458,6 +1546,13 @@ credentials:
|
||||
- name: api_key
|
||||
env_vars: [BROKEN_TOKEN, ""]
|
||||
auth_style: unknown
|
||||
- name: path_key
|
||||
env_vars: [PATH_TOKEN]
|
||||
auth_style: path
|
||||
- name: path_key_bad
|
||||
env_vars: [PATH_TOKEN_BAD]
|
||||
auth_style: path
|
||||
path_template: /v1/{key}/resources
|
||||
discovery:
|
||||
credentials: [api_key, missing_key]
|
||||
endpoints:
|
||||
@@ -1478,6 +1573,11 @@ binaries: ["", /usr/bin/broken]
|
||||
assert!(messages.contains(&"duplicate credential env var 'BROKEN_TOKEN'"));
|
||||
assert!(messages.contains(&"credential env var must not be empty"));
|
||||
assert!(messages.contains(&"query_param is required for query auth"));
|
||||
assert!(messages.contains(&"path_template is required for path auth"));
|
||||
assert!(messages.iter().any(|message| {
|
||||
message.contains("should contain {credential} exactly once")
|
||||
&& message.contains("0 times")
|
||||
}));
|
||||
assert!(messages.contains(&"unsupported auth_style: unknown"));
|
||||
assert!(messages.contains(&"unknown discovery credential: missing_key"));
|
||||
assert!(
|
||||
|
||||
@@ -1737,6 +1737,7 @@ mod tests {
|
||||
auth_style: "bearer".to_string(),
|
||||
header_name: "authorization".to_string(),
|
||||
query_param: String::new(),
|
||||
path_template: String::new(),
|
||||
refresh: Some(ProviderCredentialRefresh {
|
||||
strategy: ProviderCredentialRefreshStrategy::Oauth2ClientCredentials as i32,
|
||||
token_url: "https://auth.example.com/token".to_string(),
|
||||
@@ -1794,6 +1795,7 @@ mod tests {
|
||||
header_name: "authorization".to_string(),
|
||||
query_param: String::new(),
|
||||
refresh: None,
|
||||
path_template: String::new(),
|
||||
}
|
||||
}
|
||||
|
||||
@@ -3202,6 +3204,7 @@ mod tests {
|
||||
auth_style: "bearer".to_string(),
|
||||
header_name: "authorization".to_string(),
|
||||
query_param: String::new(),
|
||||
path_template: String::new(),
|
||||
refresh: Some(ProviderCredentialRefresh {
|
||||
strategy: ProviderCredentialRefreshStrategy::Oauth2RefreshToken
|
||||
as i32,
|
||||
|
||||
@@ -67,7 +67,7 @@ The following Providers v2 design items are not part of the current behavior:
|
||||
|
||||
| Roadmap item | Current behavior |
|
||||
|---|---|
|
||||
| Profile-driven explicit credential injection | Profile `auth_style`, `header_name`, and `query_param` fields are stored and validated, but runtime injection still depends on environment placeholders generated from provider credentials. |
|
||||
| Profile-driven explicit credential injection | Profile `auth_style`, `header_name`, `query_param`, and `path_template` fields are stored and validated, but runtime injection still depends on environment placeholders generated from provider credentials. |
|
||||
| Endpoint and binary scoped credential injection | Provider profile endpoints and binaries affect policy composition. They do not yet restrict which outbound requests can receive credential injection. |
|
||||
| Credential verification on create | `openshell provider create` does not yet probe provider verification endpoints or expose `--no-verify`. |
|
||||
| Automatic credential scope extraction | OpenShell does not yet inspect upstream provider responses to discover credential scopes. |
|
||||
@@ -158,12 +158,13 @@ credentials:
|
||||
env_vars: [CUSTOM_API_TOKEN]
|
||||
required: true
|
||||
|
||||
# Accepted values: basic, bearer, header, query.
|
||||
# Accepted values: basic, bearer, header, query, path.
|
||||
# These fields describe the intended credential placement.
|
||||
# Runtime injection still uses env placeholder resolution today.
|
||||
auth_style: bearer
|
||||
header_name: authorization
|
||||
query_param: api_key
|
||||
path_template: /v1/{credential}/resources
|
||||
|
||||
refresh:
|
||||
# Accepted values:
|
||||
@@ -239,7 +240,7 @@ binaries:
|
||||
|
||||
`category` groups profiles in `openshell provider list-profiles`. Use one of the values in the category enum.
|
||||
|
||||
`credentials` declares the credential names, environment variables, auth metadata, and optional refresh metadata for the provider type. The current runtime still exposes configured credential keys as placeholder environment variables and resolves placeholders in outbound HTTP requests.
|
||||
`credentials` declares the credential names, environment variables, auth metadata, and optional refresh metadata for the provider type. The `auth_style` field accepts `basic`, `bearer`, `header`, `query`, or `path`. When `auth_style` is `path`, set `path_template` to a URL path containing the `{credential}` placeholder exactly once (for example, `/v1/{credential}/resources`). The current runtime still exposes configured credential keys as placeholder environment variables and resolves placeholders in outbound HTTP requests.
|
||||
|
||||
`discovery` controls what `--from-existing` scans when
|
||||
`providers_v2_enabled=true`. Each entry in `discovery.credentials` must name a
|
||||
|
||||
@@ -902,6 +902,7 @@ message ProviderProfileCredential {
|
||||
string header_name = 6;
|
||||
string query_param = 7;
|
||||
ProviderCredentialRefresh refresh = 8;
|
||||
string path_template = 9;
|
||||
}
|
||||
|
||||
enum ProviderCredentialRefreshStrategy {
|
||||
|
||||
Reference in New Issue
Block a user