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

Feat/multiple securities #141

Merged
merged 3 commits into from
Jan 15, 2025
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions core/core/src/sf_core/map_std_impl/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use map_std::{
unstable::{
security::{resolve_security, SecurityMap},
HttpCallError as MapHttpCallError, HttpCallHeadError as MapHttpCallHeadError,
HttpRequest as MapHttpRequest, HttpResponse as MapHttpResponse, MapStdUnstable, MapValue,
HttpRequest as MapHttpRequest, HttpRequestSecurity as MapHttpRequestSecurity, HttpResponse as MapHttpResponse, MapStdUnstable, MapValue,
SetOutputError, TakeContextError,
},
MapStdFull,
Expand Down Expand Up @@ -90,9 +90,11 @@ impl MapStdUnstable for MapStdImpl {
}
}

fn http_call(&mut self, mut params: MapHttpRequest) -> Result<Handle, MapHttpCallError> {
fn http_call(&mut self, mut params: MapHttpRequest, security: Option<MapHttpRequestSecurity>) -> Result<Handle, MapHttpCallError> {
let security_map = self.security.as_ref().unwrap();
resolve_security(security_map, &mut params)?;
if let Some(ref security) = security {
resolve_security(security_map, &mut params, security)?;
}

// IDEA: add profile, provider info as well?
params
Expand Down
19 changes: 12 additions & 7 deletions core/core_to_map_std/src/unstable/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,14 @@ macro_rules! map_value {
};
}

#[derive(Deserialize)]
#[serde(tag = "kind", content = "ids")]
#[serde(rename_all = "kebab-case")]
pub enum HttpRequestSecurity {
FirstValid(Vec<String>),
All(Vec<String>),
}

pub struct HttpRequest {
/// HTTP method - will be used as-is.
pub method: String,
Expand All @@ -192,9 +200,7 @@ pub struct HttpRequest {
/// Multiple values with the same key will be repeated in the query string, no joining will be performed.
pub query: MultiMap,
/// Body as bytes.
pub body: Option<Vec<u8>>,
/// Security configuration
pub security: Option<String>,
pub body: Option<Vec<u8>>
}
pub struct HttpResponse {
/// Status code of the response.
Expand Down Expand Up @@ -302,7 +308,7 @@ pub trait MapStdUnstable {
fn stream_close(&mut self, handle: Handle) -> std::io::Result<()>;

// http
fn http_call(&mut self, params: HttpRequest) -> Result<Handle, HttpCallError>;
fn http_call(&mut self, params: HttpRequest, security: Option<HttpRequestSecurity>) -> Result<Handle, HttpCallError>;
fn http_call_head(&mut self, handle: Handle) -> Result<HttpResponse, HttpCallHeadError>;

// input and output
Expand All @@ -324,7 +330,7 @@ define_exchange_map_to_core! {
url: String,
headers: HeadersMultiMap,
query: MultiMap,
security: Option<String>,
security: Option<HttpRequestSecurity>,
body: Option<Vec<u8>>,
} -> enum Response {
Ok {
Expand All @@ -341,9 +347,8 @@ define_exchange_map_to_core! {
url,
headers,
query,
security,
body,
});
}, security);

match handle {
Ok(handle) => Response::Ok {
Expand Down
88 changes: 64 additions & 24 deletions core/core_to_map_std/src/unstable/security.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use sf_std::{
HeaderName,
};

use super::{HttpCallError, HttpRequest, MapValue, MapValueObject};
use super::{HttpCallError, HttpRequest, HttpRequestSecurity, MapValue, MapValueObject};

pub enum ApiKeyPlacement {
Header,
Expand Down Expand Up @@ -319,25 +319,65 @@ pub fn prepare_security_map(
pub fn resolve_security(
security_map: &SecurityMap,
params: &mut HttpRequest,
security: &HttpRequestSecurity
) -> Result<(), HttpCallError> {
let security = match params.security {
None => return Ok(()),
Some(ref security) => security,
};
match security {
HttpRequestSecurity::FirstValid(ref ids) => {
let mut first_error = None;
for id in ids {
match try_resolve_security(security_map, params, id) {
Ok(()) => return Ok(()),
Err(err) => {
if first_error.is_none() {
first_error = Some(err);
}
}
}
}

match first_error {
None => Ok(()),
Some(err) => return Err(HttpCallError::InvalidSecurityConfiguration(
err
))
}
}
HttpRequestSecurity::All(ref ids) => {
let mut all_errors = Vec::new();
for id in ids {
match try_resolve_security(security_map, params, id) {
Ok(()) => (),
Err(err) => all_errors.push(err.to_string())
}
}

let security_config = security_map.get(security.as_str());
if all_errors.len() > 0 {
return Err(HttpCallError::InvalidSecurityConfiguration(
all_errors.join("\n")
))
}
Ok(())
}
}
}
fn try_resolve_security(
security_map: &SecurityMap,
params: &mut HttpRequest,
security: &str
) -> Result<(), String> {
let security_config = security_map.get(security);

match security_config {
None => {
return Err(HttpCallError::InvalidSecurityConfiguration(format!(
return Err(format!(
"Security configuration for {} is missing",
security
)));
));
}
Some(SecurityMapValue::Error(err)) => {
return Err(HttpCallError::InvalidSecurityConfiguration(
return Err(
SecurityMisconfiguredError::format_errors(std::slice::from_ref(err)),
));
);
}
Some(SecurityMapValue::Security(Security::Http(HttpSecurity::Basic {
username,
Expand Down Expand Up @@ -384,10 +424,10 @@ pub fn resolve_security(
if let Some(body) = &params.body {
let mut body =
serde_json::from_slice::<serde_json::Value>(body).map_err(|e| {
HttpCallError::InvalidSecurityConfiguration(format!(
format!(
"Failed to parse body: {}",
e
))
)
})?;

let keys = if name.starts_with('/') {
Expand All @@ -397,50 +437,50 @@ pub fn resolve_security(
};

if keys.is_empty() {
return Err(HttpCallError::InvalidSecurityConfiguration(format!(
return Err(format!(
"Invalid field name '{}'",
name
)));
));
}

let mut nested = &mut body;
for (key_idx, key) in keys.iter().enumerate() {
if nested.is_array() {
let key_number = match key.parse::<usize>() {
Ok(n) => n,
Err(_) => return Err(HttpCallError::InvalidSecurityConfiguration(format!(
Err(_) => return Err(format!(
"Field value on path '/{}' is an array but provided key cannot be parsed as a number",
&keys[0..=key_idx].join("/")
)))
))
};
nested = &mut nested[key_number];
} else if nested.is_object() {
nested = &mut nested[key];
} else {
return Err(HttpCallError::InvalidSecurityConfiguration(format!(
return Err(format!(
"Field value on path '/{}' must be an object or an array",
&keys[0..=key_idx].join("/")
)));
));
}
}
*nested = serde_json::Value::from(apikey.to_string());

params.body = Some(serde_json::to_vec(&body).map_err(|e| {
HttpCallError::InvalidSecurityConfiguration(format!(
format!(
"Failed to serialize body: {}",
e
))
)
})?);
} else {
return Err(HttpCallError::InvalidSecurityConfiguration(
return Err(
"Api key placement is set to body but the body is empty".to_string(),
));
);
}
}
(ApiKeyPlacement::Body, None) => {
return Err(HttpCallError::InvalidSecurityConfiguration(
return Err(
"Missing body type".to_string(),
));
);
}
},
}
Expand Down
1 change: 1 addition & 0 deletions core/interpreter_js/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,7 @@ mod test {
fn http_call(
&mut self,
_params: map_std::unstable::HttpRequest,
_security: Option<map_std::unstable::HttpRequestSecurity>
) -> Result<sf_std::abi::Handle, map_std::unstable::HttpCallError> {
Ok(1)
}
Expand Down
10 changes: 8 additions & 2 deletions core_js/map-std/src/unstable.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@ export type FetchOptions = {
headers?: MultiMap,
query?: MultiMap,
body?: AnyValue,
security?: string,
/** Security configs to apply to this request. Specifying a string is equal to using `first-valid` */
security?: string | { kind: 'first-valid', ids: string[] } | { kind: 'all', ids: string[] },
Copy link
Member

Choose a reason for hiding this comment

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

@Jakub-Vacek @janhalama what do you think about this API? Most importantly won't it be trouble for Edgar?

Copy link
Member

@Jakub-Vacek Jakub-Vacek Jan 15, 2025

Choose a reason for hiding this comment

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

I think this will be fine for newer version(s) of Edgar where we use stronger models. We will have to explain LLM difference between first-valid and all and nudge him towards first valid
:)

Copy link
Contributor

Choose a reason for hiding this comment

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

Looks good to me.

This is my workaround for using multiple security schemes with Pipedrive. It will be much easier to implement it now including the fallback to next security scheme in case of 401 error.

Copy link
Member

Choose a reason for hiding this comment

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

As it is implemented now, it will just resolve first security. Not do call and try another.
For Pipedrive it was migration from api_key to oauth bearerToken (btw that naming "consistency" 🙈). But in general how often users have both configured?

};

// Can't use Record<string, AnyValue> but can use { [s in string]: AnyValue }. Typescript go brr.
Expand Down Expand Up @@ -202,14 +203,19 @@ export function fetch(url: string, options: FetchOptions): HttpRequest {
finalBody = Array.from(bodyBuffer.inner.data);
}

let security = options.security
if (typeof security === "string") {
security = { kind: "first-valid", ids: [security] }
}

const response = messageExchange({
kind: 'http-call',
method: options.method ?? 'GET',
url,
headers,
query: ensureMultimap(options.query ?? {}),
body: finalBody,
security: options.security,
security,
});

if (response.kind === 'ok') {
Expand Down
6 changes: 6 additions & 0 deletions examples/comlinks/src/localhost.provider.json
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,12 @@
"id": "basic_auth",
"type": "http",
"scheme": "basic"
},
{
"id": "authic_base",
"type": "apiKey",
"in": "header",
"name": "X-API-KEY"
}
]
}
2 changes: 2 additions & 0 deletions examples/comlinks/src/wasm-sdk.example.localhost.map.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ var Example = ({ input, parameters, services }) => {
'x-custom-header': [parameters.PARAM]
},
security: 'basic_auth',
// security: { kind: "first-valid", ids: ['authic_base', 'basic_auth'] },
// security: { kind: "all", ids: ['authic_base', 'basic_auth'] },
query: {
'foo': ['bar', 'baz'],
'qux': ['2']
Expand Down
2 changes: 1 addition & 1 deletion examples/python/__main__.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ def do_GET(self):
{ "id": 1 },
provider = "localhost",
parameters = { "PARAM": "parameter_value" },
security = { "basic_auth": { "username": "username", "password": "password" } }
security = { "basic_auth": { "username": "username", "password": "password" }, "authic_base": { "apikey": "api_key_value" } }
)
print(f"RESULT: {r}")
except PerformError as e:
Expand Down
Loading