ref(service): Make object key optional on insert

objectstore-server/src/auth/context.rs

@@ -1,7 +1,7 @@
 use std::collections::{BTreeMap, HashSet};
 
 use jsonwebtoken::{Algorithm, DecodingKey, Header, TokenData, Validation, decode, decode_header};
-use objectstore_service::id::ObjectId;
+use objectstore_service::id::ObjectContext;
 use secrecy::ExposeSecret;
 use serde::{Deserialize, Serialize};
 use thiserror::Error;
@@ -33,9 +33,13 @@ pub enum Permission {
 #[non_exhaustive]
 pub struct AuthContext {
     /// The objectstore usecase that this request may act on.
+    ///
+    /// See also: [`ObjectContext::usecase`].
     pub usecase: String,
 
-    /// The scope elements that this request may act on. See also: [`ObjectId::scopes`].
+    /// The scope elements that this request may act on.
+    ///
+    /// See also: [`ObjectContext::scopes`].
     pub scopes: BTreeMap<String, StringOrWildcard>,
 
     /// The permissions that this request has been granted.
@@ -188,8 +192,12 @@ impl AuthContext {
         })
     }
 
-    fn fail_if_enforced(&self, perm: &Permission, id: &ObjectId) -> Result<(), AuthError> {
-        tracing::debug!(?self, ?perm, ?id, "Authorization failed");
+    fn fail_if_enforced(
+        &self,
+        perm: &Permission,
+        context: &ObjectContext,
+    ) -> Result<(), AuthError> {
+        tracing::debug!(?self, ?perm, ?context, "Authorization failed");
         if self.enforce {
             return Err(AuthError::NotPermitted);
         }
@@ -201,19 +209,23 @@ impl AuthContext {
     ///
     /// The passed-in `perm` is checked against this `AuthContext`'s `permissions`. If it is not
     /// present, then the operation is not authorized.
-    pub fn assert_authorized(&self, perm: Permission, id: &ObjectId) -> Result<(), AuthError> {
-        if !self.permissions.contains(&perm) || self.usecase != id.usecase {
-            self.fail_if_enforced(&perm, id)?;
+    pub fn assert_authorized(
+        &self,
+        perm: Permission,
+        context: &ObjectContext,
+    ) -> Result<(), AuthError> {
+        if !self.permissions.contains(&perm) || self.usecase != context.usecase {
+            self.fail_if_enforced(&perm, context)?;
         }
 
-        for scope in &id.scopes {
+        for scope in &context.scopes {
             let authorized = match self.scopes.get(scope.name()) {
                 Some(StringOrWildcard::String(s)) => s == scope.value(),
                 Some(StringOrWildcard::Wildcard) => true,
                 None => false,
             };
             if !authorized {
-                self.fail_if_enforced(&perm, id)?;
+                self.fail_if_enforced(&perm, context)?;
             }
         }
 
@@ -408,14 +420,13 @@ MC4CAQAwBQYDK2VwBCIEIKwVoE4TmTfWoqH3HgLVsEcHs9PHNe+ar/Hp6e4To8pK
         Ok(())
     }
 
-    fn sample_object_path(org: &str, project: &str) -> ObjectId {
-        ObjectId {
+    fn sample_object_context(org: &str, project: &str) -> ObjectContext {
+        ObjectContext {
             usecase: "attachments".into(),
             scopes: Scopes::from_iter([
                 Scope::create("org", org).unwrap(),
                 Scope::create("project", project).unwrap(),
             ]),
-            key: "abcde".into(),
         }
     }
 
@@ -425,7 +436,7 @@ MC4CAQAwBQYDK2VwBCIEIKwVoE4TmTfWoqH3HgLVsEcHs9PHNe+ar/Hp6e4To8pK
     #[test]
     fn test_assert_authorized_exact_scope_allowed() -> Result<(), AuthError> {
         let auth_context = sample_auth_context("123", "456", max_permission());
-        let object = sample_object_path("123", "456");
+        let object = sample_object_context("123", "456");
 
         auth_context.assert_authorized(Permission::ObjectRead, &object)?;
 
@@ -438,7 +449,7 @@ MC4CAQAwBQYDK2VwBCIEIKwVoE4TmTfWoqH3HgLVsEcHs9PHNe+ar/Hp6e4To8pK
     #[test]
     fn test_assert_authorized_wildcard_project_allowed() -> Result<(), AuthError> {
         let auth_context = sample_auth_context("123", "*", max_permission());
-        let object = sample_object_path("123", "456");
+        let object = sample_object_context("123", "456");
 
         auth_context.assert_authorized(Permission::ObjectRead, &object)?;
 
@@ -451,10 +462,9 @@ MC4CAQAwBQYDK2VwBCIEIKwVoE4TmTfWoqH3HgLVsEcHs9PHNe+ar/Hp6e4To8pK
     #[test]
     fn test_assert_authorized_org_only_path_allowed() -> Result<(), AuthError> {
         let auth_context = sample_auth_context("123", "456", max_permission());
-        let object = ObjectId {
+        let object = ObjectContext {
             usecase: "attachments".into(),
             scopes: Scopes::from_iter([Scope::create("org", "123").unwrap()]),
-            key: "abcde".into(),
         };
 
         auth_context.assert_authorized(Permission::ObjectRead, &object)?;
@@ -471,13 +481,13 @@ MC4CAQAwBQYDK2VwBCIEIKwVoE4TmTfWoqH3HgLVsEcHs9PHNe+ar/Hp6e4To8pK
     #[test]
     fn test_assert_authorized_scope_mismatch_fails() -> Result<(), AuthError> {
         let auth_context = sample_auth_context("123", "456", max_permission());
-        let object = sample_object_path("123", "999");
+        let object = sample_object_context("123", "999");
 
         let result = auth_context.assert_authorized(Permission::ObjectRead, &object);
         assert_eq!(result, Err(AuthError::NotPermitted));
 
         let auth_context = sample_auth_context("123", "456", max_permission());
-        let object = sample_object_path("999", "456");
+        let object = sample_object_context("999", "456");
 
         let result = auth_context.assert_authorized(Permission::ObjectRead, &object);
         assert_eq!(result, Err(AuthError::NotPermitted));
@@ -489,7 +499,7 @@ MC4CAQAwBQYDK2VwBCIEIKwVoE4TmTfWoqH3HgLVsEcHs9PHNe+ar/Hp6e4To8pK
     fn test_assert_authorized_wrong_usecase_fails() -> Result<(), AuthError> {
         let mut auth_context = sample_auth_context("123", "456", max_permission());
         auth_context.usecase = "debug-files".into();
-        let object = sample_object_path("123", "456");
+        let object = sample_object_context("123", "456");
 
         let result = auth_context.assert_authorized(Permission::ObjectRead, &object);
         assert_eq!(result, Err(AuthError::NotPermitted));
@@ -501,7 +511,7 @@ MC4CAQAwBQYDK2VwBCIEIKwVoE4TmTfWoqH3HgLVsEcHs9PHNe+ar/Hp6e4To8pK
     fn test_assert_authorized_auth_context_missing_permission_fails() -> Result<(), AuthError> {
         let auth_context =
             sample_auth_context("123", "456", HashSet::from([Permission::ObjectRead]));
-        let object = sample_object_path("123", "456");
+        let object = sample_object_context("123", "456");
 
         let result = auth_context.assert_authorized(Permission::ObjectWrite, &object);
         assert_eq!(result, Err(AuthError::NotPermitted));
@@ -515,7 +525,7 @@ MC4CAQAwBQYDK2VwBCIEIKwVoE4TmTfWoqH3HgLVsEcHs9PHNe+ar/Hp6e4To8pK
         let mut auth_context =
             sample_auth_context("123", "456", HashSet::from([Permission::ObjectRead]));
         // Object's scope is not covered by the auth context
-        let object = sample_object_path("999", "999");
+        let object = sample_object_context("999", "999");
 
         // Auth fails for two reasons, but because enforcement is off, it should not return an error
         auth_context.enforce = false;

objectstore-server/src/auth/service.rs

@@ -1,6 +1,6 @@
 use axum::extract::FromRequestParts;
 use axum::http::{StatusCode, header, request::Parts};
-use objectstore_service::id::ObjectId;
+use objectstore_service::id::{ObjectContext, ObjectId};
 use objectstore_service::{PayloadStream, StorageService};
 use objectstore_types::Metadata;
 
@@ -39,41 +39,44 @@ pub struct AuthAwareService {
 }
 
 impl AuthAwareService {
-    fn assert_authorized(&self, perm: Permission, id: &ObjectId) -> anyhow::Result<()> {
+    fn assert_authorized(&self, perm: Permission, context: &ObjectContext) -> anyhow::Result<()> {
         if self.enforce {
-            let context = self
+            let auth = self
                 .context
                 .as_ref()
                 .ok_or(AuthError::VerificationFailure)?;
-            context.assert_authorized(perm, id)?;
+            auth.assert_authorized(perm, context)?;
         }
 
         Ok(())
     }
 
-    /// Auth-aware wrapper around [`StorageService::put_object`].
-    pub async fn put_object(
+    /// Auth-aware wrapper around [`StorageService::insert_object`].
+    pub async fn insert_object(
         &self,
-        id: ObjectId,
+        context: ObjectContext,
+        key: Option<String>,
         metadata: &Metadata,
         stream: PayloadStream,
     ) -> anyhow::Result<ObjectId> {
-        self.assert_authorized(Permission::ObjectWrite, &id)?;
-        self.service.put_object(id, metadata, stream).await
+        self.assert_authorized(Permission::ObjectWrite, &context)?;
+        self.service
+            .insert_object(context, key, metadata, stream)
+            .await
     }
 
     /// Auth-aware wrapper around [`StorageService::get_object`].
     pub async fn get_object(
         &self,
         id: &ObjectId,
     ) -> anyhow::Result<Option<(Metadata, PayloadStream)>> {
-        self.assert_authorized(Permission::ObjectRead, id)?;
+        self.assert_authorized(Permission::ObjectRead, id.context())?;
         self.service.get_object(id).await
     }
 
     /// Auth-aware wrapper around [`StorageService::delete_object`].
     pub async fn delete_object(&self, id: &ObjectId) -> anyhow::Result<()> {
-        self.assert_authorized(Permission::ObjectDelete, id)?;
+        self.assert_authorized(Permission::ObjectDelete, id.context())?;
         self.service.delete_object(id).await
     }
 }

objectstore-server/src/endpoints/helpers.rs

@@ -1,9 +1,17 @@
-use objectstore_service::id::ObjectId;
+use objectstore_service::id::{ObjectContext, ObjectId};
 
-pub fn populate_sentry_scope(path: &ObjectId) {
+pub fn populate_sentry_context(context: &ObjectContext) {
     sentry::configure_scope(|s| {
-        s.set_tag("usecase", &path.usecase);
-        s.set_extra("scope", path.scopes.as_storage_path().to_string().into());
-        s.set_extra("key", path.key.clone().into());
+        s.set_tag("usecase", &context.usecase);
+        for scope in &context.scopes {
+            s.set_tag(&format!("scope.{}", scope.name()), scope.value());
+        }
+    });
+}
+
+pub fn populate_sentry_object_id(id: &ObjectId) {
+    populate_sentry_context(id.context());
+    sentry::configure_scope(|s| {
+        s.set_extra("key", id.key().into());
     });
 }

objectstore-server/src/endpoints/objects.rs

@@ -10,7 +10,7 @@ use axum::response::{IntoResponse, Response};
 use axum::routing;
 use axum::{Json, Router};
 use futures_util::{StreamExt, TryStreamExt};
-use objectstore_service::id::{ObjectId, Scope, Scopes};
+use objectstore_service::id::{ObjectContext, ObjectId, Scope, Scopes};
 use objectstore_types::Metadata;
 use serde::{Deserialize, Serialize, de};
 
@@ -48,17 +48,19 @@ async fn objects_post(
     headers: HeaderMap,
     body: Body,
 ) -> ApiResult<Response> {
-    let id = params.create_object_id();
-    helpers::populate_sentry_scope(&id);
+    let context = params.into_context();
+    helpers::populate_sentry_context(&context);
 
     let mut metadata =
         Metadata::from_headers(&headers, "").context("extracting metadata from headers")?;
     metadata.time_created = Some(SystemTime::now());
 
     let stream = body.into_data_stream().map_err(io::Error::other).boxed();
-    let response_path = service.put_object(id, &metadata, stream).await?;
+    let response_id = service
+        .insert_object(context, None, &metadata, stream)
+        .await?;
     let response = Json(InsertObjectResponse {
-        key: response_path.key.to_string(),
+        key: response_id.key().to_string(),
     });
 
     Ok((StatusCode::CREATED, response).into_response())
@@ -69,7 +71,7 @@ async fn object_get(
     Path(params): Path<ObjectParams>,
 ) -> ApiResult<Response> {
     let id = params.into_object_id();
-    helpers::populate_sentry_scope(&id);
+    helpers::populate_sentry_object_id(&id);
 
     let Some((metadata, stream)) = service.get_object(&id).await? else {
         return Ok(StatusCode::NOT_FOUND.into_response());
@@ -86,7 +88,7 @@ async fn object_head(
     Path(params): Path<ObjectParams>,
 ) -> ApiResult<Response> {
     let id = params.into_object_id();
-    helpers::populate_sentry_scope(&id);
+    helpers::populate_sentry_object_id(&id);
 
     let Some((metadata, _stream)) = service.get_object(&id).await? else {
         return Ok(StatusCode::NOT_FOUND.into_response());
@@ -106,16 +108,20 @@ async fn object_put(
     body: Body,
 ) -> ApiResult<Response> {
     let id = params.into_object_id();
-    helpers::populate_sentry_scope(&id);
+    helpers::populate_sentry_object_id(&id);
 
     let mut metadata =
         Metadata::from_headers(&headers, "").context("extracting metadata from headers")?;
     metadata.time_created = Some(SystemTime::now());
 
+    let ObjectId { context, key } = id;
     let stream = body.into_data_stream().map_err(io::Error::other).boxed();
-    let response_path = service.put_object(id, &metadata, stream).await?;
+    let response_id = service
+        .insert_object(context, Some(key), &metadata, stream)
+        .await?;
+
     let response = Json(InsertObjectResponse {
-        key: response_path.key.to_string(),
+        key: response_id.key.to_string(),
     });
 
     Ok((StatusCode::OK, response).into_response())
@@ -126,14 +132,14 @@ async fn object_delete(
     Path(params): Path<ObjectParams>,
 ) -> ApiResult<impl IntoResponse> {
     let id = params.into_object_id();
-    helpers::populate_sentry_scope(&id);
+    helpers::populate_sentry_object_id(&id);
 
     service.delete_object(&id).await?;
 
     Ok(StatusCode::NO_CONTENT)
 }
 
-/// Path parameters used for collection-level endpoints.
+/// Path parameters used for collection-level endpoints without a key.
 ///
 /// This is meant to be used with the axum `Path` extractor.
 #[derive(Clone, Debug, Deserialize)]
@@ -144,9 +150,12 @@ struct CollectionParams {
 }
 
 impl CollectionParams {
-    /// Converts the params into a new [`ObjectId`] with a random unique `key`.
-    pub fn create_object_id(self) -> ObjectId {
-        ObjectId::random(self.usecase, self.scopes)
+    /// Converts the params into an [`ObjectContext`].
+    pub fn into_context(self) -> ObjectContext {
+        ObjectContext {
+            usecase: self.usecase,
+            scopes: self.scopes,
+        }
     }
 }
 
@@ -164,11 +173,7 @@ struct ObjectParams {
 impl ObjectParams {
     /// Converts the params into an [`ObjectId`].
     pub fn into_object_id(self) -> ObjectId {
-        ObjectId {
-            usecase: self.usecase,
-            scopes: self.scopes,
-            key: self.key,
-        }
+        ObjectId::from_parts(self.usecase, self.scopes, self.key)
     }
 }