Skip to content
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
2 changes: 2 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion bigtable-bench/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ async fn main() -> anyhow::Result<()> {
let worker_bytes = Arc::clone(&total_bytes);
let worker_handle = tokio::spawn(async move {
let context = ObjectContext {
usecase: "bench".into(),
usecase: "bench".parse().unwrap(),
scopes: Scopes::from_iter([
Scope::create("org", "bench").unwrap(), // INVARIANT: valid scope name
]),
Expand Down
13 changes: 6 additions & 7 deletions clients/rust/src/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use bytes::Bytes;
use futures_util::stream::BoxStream;
use objectstore_types::metadata::{Compression, ExpirationPolicy};
use objectstore_types::scope;
use objectstore_types::usecase::{self, InvalidUsecaseName};
use objectstore_types::usecase::{InvalidUsecaseName, Usecase as UsecaseName};
use reqwest::RequestBuilder;
use url::Url;

Expand Down Expand Up @@ -158,7 +158,7 @@ impl ClientBuilder {
/// normalized.
#[derive(Debug, Clone)]
pub struct Usecase {
name: Arc<str>,
name: Arc<UsecaseName>,
compression: Option<Compression>,
expiration_policy: ExpirationPolicy,
}
Expand All @@ -184,9 +184,8 @@ impl Usecase {
/// Returns [`InvalidUsecaseName`] if the name violates the naming rules on
/// [`Usecase`].
pub fn try_new(name: &str) -> Result<Self, InvalidUsecaseName> {
usecase::validate_usecase_name(name)?;
Ok(Self {
name: name.into(),
name: Arc::new(UsecaseName::new(name)?),
compression: Some(Compression::Zstd),
expiration_policy: ExpirationPolicy::default(),
})
Expand All @@ -195,7 +194,7 @@ impl Usecase {
/// Returns the name of this usecase.
#[inline]
pub fn name(&self) -> &str {
&self.name
self.name.as_str()
}

/// Returns the compression algorithm to use for operations within this usecase.
Expand Down Expand Up @@ -472,7 +471,7 @@ impl Session {
segments
.push("v1")
.push("objects")
.push(&self.scope.usecase.name)
.push(self.scope.usecase.name())
.push(&self.scope.scopes.as_api_path().to_string())
.extend(object_key.split('/'));
drop(segments);
Expand Down Expand Up @@ -567,7 +566,7 @@ impl Session {
Some("complete") => "objects:multipart:complete",
_ => "objects:multipart",
})
.push(&self.scope.usecase.name)
.push(self.scope.usecase.name())
.push(&self.scope.scopes.as_api_path().to_string());
if let Some(object_key) = object_key.filter(|key| !key.is_empty()) {
segments.extend(object_key.split('/'));
Expand Down
2 changes: 2 additions & 0 deletions objectstore-options/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ publish = false

[dependencies]
objectstore-log = { workspace = true }
objectstore-types = { workspace = true }
objectstore-typed-options = { workspace = true, features = ["derive"] }
serde = { workspace = true, features = ["derive"] }
tokio = { workspace = true, features = ["rt", "time"] }
Expand All @@ -19,3 +20,4 @@ tokio = { workspace = true, features = ["rt", "time"] }
testing = []

[dev-dependencies]
serde_json = { workspace = true }
17 changes: 15 additions & 2 deletions objectstore-options/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
use std::collections::BTreeMap;

use objectstore_typed_options::SentryOptions;
use objectstore_types::usecase::Usecase;
use serde::{Deserialize, Serialize};

pub use objectstore_typed_options::Error;
Expand Down Expand Up @@ -59,11 +60,11 @@ impl Options {
/// contexts and discard all requests.
#[derive(Debug, Deserialize, Serialize, PartialEq)]
pub struct Killswitch {
/// Optional usecase to match.
/// Optional validated usecase name to match exactly.
///
/// If `None`, matches any usecase.
#[serde(default)]
pub usecase: Option<String>,
pub usecase: Option<Usecase>,

/// Scopes to match.
///
Expand Down Expand Up @@ -95,4 +96,16 @@ mod tests {
fn schema_is_valid() {
let _ = Options::get();
}

#[test]
fn killswitch_deserialization_validates_usecase() {
let switch: Killswitch = serde_json::from_str(r#"{"usecase":"Debug_Files"}"#).unwrap();
assert_eq!(switch.usecase.as_ref().unwrap().as_str(), "Debug_Files");
let serialized = serde_json::to_value(&switch).unwrap();
assert_eq!(serialized["usecase"], "Debug_Files");

let wildcard: Killswitch = serde_json::from_str("{}").unwrap();
assert!(wildcard.usecase.is_none());
assert!(serde_json::from_str::<Killswitch>(r#"{"usecase":"test/name"}"#).is_err());
}
}
13 changes: 7 additions & 6 deletions objectstore-server/src/auth/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ use jsonwebtoken::{Algorithm, Header, TokenData, Validation, decode, decode_head
use objectstore_service::id::ObjectContext;
use objectstore_types::auth::Permission;
use objectstore_types::presign::CanonicalRequest;
use objectstore_types::usecase::Usecase;
use serde::{Deserialize, Serialize};

use crate::auth::KeyId;
Expand All @@ -16,7 +17,7 @@ use crate::auth::util::StringOrWildcard;
#[derive(Deserialize, Serialize, Debug, Clone)]
struct JwtRes {
#[serde(rename = "os:usecase")]
usecase: String,
usecase: Usecase,

#[serde(flatten)]
scope: BTreeMap<String, StringOrWildcard>,
Expand All @@ -43,7 +44,7 @@ pub struct ScopedContext {
/// The objectstore usecase that this request may act on.
///
/// See also: [`ObjectContext::usecase`].
pub usecase: String,
pub usecase: Usecase,

/// The scope elements that this request may act on.
///
Expand Down Expand Up @@ -351,7 +352,7 @@ mod tests {

fn sample_auth_context(org: &str, proj: &str, permissions: HashSet<Permission>) -> AuthContext {
AuthContext::Scoped(ScopedContext {
usecase: "attachments".into(),
usecase: "attachments".parse().unwrap(),
permissions,
scopes: serde_json::from_value(json!({"org": org, "project": proj})).unwrap(),
})
Expand Down Expand Up @@ -457,7 +458,7 @@ MC4CAQAwBQYDK2VwBCIEIKwVoE4TmTfWoqH3HgLVsEcHs9PHNe+ar/Hp6e4To8pK

fn sample_object_context(org: &str, project: &str) -> ObjectContext {
ObjectContext {
usecase: "attachments".into(),
usecase: "attachments".parse().unwrap(),
scopes: Scopes::from_iter([
Scope::create("org", org).unwrap(),
Scope::create("project", project).unwrap(),
Expand Down Expand Up @@ -498,7 +499,7 @@ MC4CAQAwBQYDK2VwBCIEIKwVoE4TmTfWoqH3HgLVsEcHs9PHNe+ar/Hp6e4To8pK
fn test_assert_authorized_org_only_path_allowed() -> Result<(), AuthError> {
let auth_context = sample_auth_context("123", "456", max_permission());
let object = ObjectContext {
usecase: "attachments".into(),
usecase: "attachments".parse().unwrap(),
scopes: Scopes::from_iter([Scope::create("org", "123").unwrap()]),
};

Expand Down Expand Up @@ -536,7 +537,7 @@ MC4CAQAwBQYDK2VwBCIEIKwVoE4TmTfWoqH3HgLVsEcHs9PHNe+ar/Hp6e4To8pK
else {
panic!("expected a scoped auth context");
};
scoped.usecase = "debug-files".into();
scoped.usecase = "debug-files".parse().unwrap();
let auth_context = AuthContext::Scoped(scoped);
let object = sample_object_context("123", "456");

Expand Down
6 changes: 3 additions & 3 deletions objectstore-server/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1485,7 +1485,7 @@ mod tests {
figment::Jail::expect_with(|_jail| {
let expected = [
Killswitch {
usecase: Some("broken_usecase".into()),
usecase: Some("broken_usecase".parse().unwrap()),
scopes: BTreeMap::new(),
service: None,
},
Expand All @@ -1508,7 +1508,7 @@ mod tests {
service: None,
},
Killswitch {
usecase: Some("attachments".into()),
usecase: Some("attachments".parse().unwrap()),
scopes: BTreeMap::from([("org".into(), "42".into())]),
service: Some("test-*".into()),
},
Expand Down Expand Up @@ -1561,7 +1561,7 @@ mod tests {
scope_pct: Some(25),
rules: vec![
ThroughputRule {
usecase: Some("high_priority".to_string()),
usecase: Some("high_priority".parse().unwrap()),
scopes: vec![("org".to_string(), "123".to_string())],
rps: Some(500),
pct: None,
Expand Down
5 changes: 1 addition & 4 deletions objectstore-server/src/endpoints/multipart.rs
Original file line number Diff line number Diff line change
Expand Up @@ -102,10 +102,7 @@ async fn initiate_inner(
// TODO: Update time_created in `complete`, when we have a Service API to mutate metadata.
let metadata = Metadata::from_insert_headers(&headers, "", access_time)?;

state
.config
.usecases
.validate(&id.context().usecase, &metadata)?;
state.config.usecases.validate(id.usecase(), &metadata)?;

let upload_id = service.initiate_multipart(id.clone(), metadata).await?;

Expand Down
2 changes: 1 addition & 1 deletion objectstore-server/src/endpoints/objects.rs
Original file line number Diff line number Diff line change
Expand Up @@ -397,7 +397,7 @@ mod tests {
fn object_id(key: &str) -> ObjectId {
ObjectId::new(
ObjectContext {
usecase: "testing".into(),
usecase: "testing".parse().unwrap(),
scopes: Scopes::from_iter([Scope::create("org", "1").unwrap()]),
},
key.into(),
Expand Down
5 changes: 1 addition & 4 deletions objectstore-server/src/endpoints/resumable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -105,10 +105,7 @@ async fn create_session_for_id(
require_empty_body(content_length, body, "resumable session creation").await?;
let metadata = Metadata::from_insert_headers(&headers, "", access_time)?;

state
.config
.usecases
.validate(&id.context().usecase, &metadata)?;
state.config.usecases.validate(id.usecase(), &metadata)?;

let created = service
.create_upload_session(id.clone(), metadata, total_length)
Expand Down
68 changes: 27 additions & 41 deletions objectstore-server/src/extractors/id.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use axum::http::request::Parts;
use axum::response::{IntoResponse, Response};
use objectstore_service::id::{ObjectContext, ObjectId};
use objectstore_types::scope::{EMPTY_SCOPES, Scope, Scopes};
use objectstore_types::usecase;
use objectstore_types::usecase::Usecase;
use serde::{Deserialize, de};

use crate::extractors::Xt;
Expand Down Expand Up @@ -82,23 +82,12 @@ impl FromRequestParts<ServiceState> for Xt<ObjectId> {
/// This is meant to be used with the axum `Path` extractor.
#[derive(Clone, Debug, Deserialize)]
struct ObjectParams {
#[serde(deserialize_with = "deserialize_usecase")]
usecase: String,
usecase: Usecase,
#[serde(deserialize_with = "deserialize_scopes")]
scopes: Scopes,
key: String,
}

/// Deserializes and validates a usecase name.
fn deserialize_usecase<'de, D>(deserializer: D) -> Result<String, D::Error>
where
D: de::Deserializer<'de>,
{
let name = String::deserialize(deserializer)?;
usecase::validate_usecase_name(&name).map_err(de::Error::custom)?;
Ok(name)
}

/// Deserializes a `Scopes` instance from a string representation.
///
/// The string representation is a semicolon-separated list of `key=value` pairs, following the
Expand Down Expand Up @@ -167,15 +156,14 @@ impl FromRequestParts<ServiceState> for Xt<ObjectContext> {
/// (`/objects/{usecase}/{scopes}/{*key}`) routes — the extra `key` parameter is ignored.
#[derive(Clone, Debug, Deserialize)]
pub(super) struct ContextParams {
#[serde(deserialize_with = "deserialize_usecase")]
pub usecase: String,
pub usecase: Usecase,
#[serde(deserialize_with = "deserialize_scopes")]
pub scopes: Scopes,
}

fn populate_sentry_context(context: &ObjectContext) {
sentry::configure_scope(|s| {
s.set_tag("usecase", &context.usecase);
s.set_tag("usecase", context.usecase.as_str());
for scope in &context.scopes {
s.set_tag(&format!("scope.{}", scope.name()), scope.value());
}
Expand All @@ -196,31 +184,29 @@ mod tests {

#[test]
fn parse_usecase() {
for name in [
"Debug_Files-123".to_owned(),
"a".repeat(64),
"___".to_owned(),
] {
let deserializer: CowStrDeserializer<DeError> =
Cow::Borrowed(name.as_str()).into_deserializer();
assert_eq!(deserialize_usecase(deserializer).unwrap(), name);
}
for name in [
"",
"_",
"ab",
"testing/org=999/injected",
"test.name",
"test%2Fname",
"café",
for (name, valid) in [
("Debug_Files-123", true),
("___", true),
("_", false),
("test/name", false),
("test%2Fname", false),
] {
let deserializer: CowStrDeserializer<DeError> = Cow::Borrowed(name).into_deserializer();
assert!(deserialize_usecase(deserializer).is_err(), "{name:?}");
let params = serde_json::json!({
"usecase": name,
"scopes": "org=123",
"key": "my-key",
});
assert_eq!(
serde_json::from_value::<ObjectParams>(params.clone()).is_ok(),
valid,
"object usecase {name:?}",
);
assert_eq!(
serde_json::from_value::<ContextParams>(params).is_ok(),
valid,
"context usecase {name:?}",
);
}
let name = "a".repeat(65);
let deserializer: CowStrDeserializer<DeError> =
Cow::Borrowed(name.as_str()).into_deserializer();
assert!(deserialize_usecase(deserializer).is_err());
}

#[test]
Expand Down Expand Up @@ -443,7 +429,7 @@ mod tests {
async fn extract_object_id_killswitched() {
let config = Config {
killswitches: Killswitches::new(vec![Killswitch {
usecase: Some("blocked".into()),
usecase: Some("blocked".parse().unwrap()),
scopes: BTreeMap::new(),
service: None,
}]),
Expand Down Expand Up @@ -471,7 +457,7 @@ mod tests {
async fn extract_object_context_killswitched() {
let config = Config {
killswitches: Killswitches::new(vec![Killswitch {
usecase: Some("blocked".into()),
usecase: Some("blocked".parse().unwrap()),
scopes: BTreeMap::new(),
service: None,
}]),
Expand Down
Loading
Loading