diff --git a/clients/python/src/objectstore_client/auth.py b/clients/python/src/objectstore_client/auth.py index 0343985e..28d5c8df 100644 --- a/clients/python/src/objectstore_client/auth.py +++ b/clients/python/src/objectstore_client/auth.py @@ -55,7 +55,10 @@ def sign_for_scope(self, usecase: str, scope: Scope) -> str: claims = { "res": { "os:usecase": usecase, - **{k: str(v) for k, v in scope.dict().items()}, + "scopes": [ + {"name": key, "value": str(value)} + for key, value in scope.dict().items() + ], }, "permissions": self.permissions, "exp": datetime.now(tz=UTC) + timedelta(seconds=self.expiry_seconds), diff --git a/clients/rust/src/auth.rs b/clients/rust/src/auth.rs index 13b29077..40474796 100644 --- a/clients/rust/src/auth.rs +++ b/clients/rust/src/auth.rs @@ -1,4 +1,4 @@ -use std::collections::{BTreeMap, HashSet}; +use std::collections::HashSet; use jsonwebtoken::{Algorithm, EncodingKey, Header, encode, get_current_timestamp}; use objectstore_types::scope; @@ -89,13 +89,17 @@ pub struct TokenGenerator { permissions: HashSet, } +#[derive(Serialize, Deserialize)] +struct JwtScope { + name: String, + value: String, +} + #[derive(Serialize, Deserialize)] struct JwtRes { #[serde(rename = "os:usecase")] usecase: String, - - #[serde(flatten)] - scopes: BTreeMap, + scopes: Vec, } #[derive(Serialize, Deserialize)] @@ -161,7 +165,10 @@ impl TokenGenerator { scopes: scope .scopes() .iter() - .map(|scope| (scope.name().to_string(), scope.value().to_string())) + .map(|scope| JwtScope { + name: scope.name().to_string(), + value: scope.value().to_string(), + }) .collect(), }, }; diff --git a/clients/rust/tests/e2e.rs b/clients/rust/tests/e2e.rs index 2982200c..de093f60 100644 --- a/clients/rust/tests/e2e.rs +++ b/clients/rust/tests/e2e.rs @@ -26,8 +26,13 @@ struct JwtClaims { struct JwtRes { #[serde(rename = "os:usecase")] usecase: String, - #[serde(flatten)] - scopes: BTreeMap, + scopes: Vec, +} + +#[derive(Serialize)] +struct JwtScope { + name: String, + value: String, } /// Signs a static token for the given usecase and scopes. @@ -44,7 +49,10 @@ fn sign_static_token(usecase: &str, scopes: &[(&str, &str)]) -> String { usecase: usecase.into(), scopes: scopes .iter() - .map(|(k, v)| (k.to_string(), v.to_string())) + .map(|(k, v)| JwtScope { + name: k.to_string(), + value: v.to_string(), + }) .collect(), }, }; @@ -413,7 +421,7 @@ async fn batch_operations() { .unwrap(); assert_eq!(get1.metadata.compression, None); assert!(get1.metadata.time_created.is_some()); - assert_eq!(get1.payload().await.unwrap().as_ref(), b"first object"); + assert_eq!(&get1.payload().await.unwrap()[..], b"first object"); // GET key-2 (automatic decompression) let get2 = gets @@ -423,7 +431,7 @@ async fn batch_operations() { .unwrap(); assert_eq!(get2.metadata.compression, None); assert!(get2.metadata.time_created.is_some()); - assert_eq!(get2.payload().await.unwrap().as_ref(), b"second object"); + assert_eq!(&get2.payload().await.unwrap()[..], b"second object"); // DELETE key-3 assert!(deletes.contains("key-3"), "missing delete for key-3"); diff --git a/objectstore-server/docs/architecture.md b/objectstore-server/docs/architecture.md index 69440a01..1fa458bd 100644 --- a/objectstore-server/docs/architecture.md +++ b/objectstore-server/docs/architecture.md @@ -72,8 +72,9 @@ Tokens must include: - **Header**: `kid` (key ID) and `alg: EdDSA` - **Claims**: `aud: "objectstore"`, `iss: "sentry"` or `"relay"`, `exp` (expiration timestamp) -- **Resource claims** (`res`): the usecase and scope values the token grants - access to (e.g., `{"os:usecase": "attachments", "org": "123"}`) +- **Resource claims** (`res`): the usecase and an ordered `scopes` array the + token grants access to (e.g., `{"os:usecase": "attachments", "scopes": + [{"name": "org", "value": "123"}]}`) - **Permissions**: array of granted operations (`object.read`, `object.write`, `object.delete`) @@ -90,7 +91,10 @@ limiting what any token signed by that key can do. On every operation, [`AuthAwareService`](auth::AuthAwareService) verifies that the token's scopes and permissions cover the requested [`ObjectContext`](objectstore_service::id::ObjectContext) and operation type. -Scope values in the token can use wildcards to grant broad access. +Scope values in the token can use wildcards to grant broad access. Scope +matching is order-sensitive and prefix-based: a token for `org=1, project=*` +matches `org=1, project=10` and `org=1, project=10, shard=blue`, but not +`project=10, org=1`. ## Configuration diff --git a/objectstore-server/src/auth/context.rs b/objectstore-server/src/auth/context.rs index 49a9f69e..20037766 100644 --- a/objectstore-server/src/auth/context.rs +++ b/objectstore-server/src/auth/context.rs @@ -1,4 +1,4 @@ -use std::collections::{BTreeMap, HashSet}; +use std::collections::HashSet; use jsonwebtoken::{Algorithm, Header, TokenData, Validation, decode, decode_header}; use objectstore_service::id::ObjectContext; @@ -9,13 +9,20 @@ use crate::auth::error::AuthError; use crate::auth::key_directory::PublicKeyDirectory; use crate::auth::util::StringOrWildcard; +/// Ordered scope entry stored in JWT resource claims and [`AuthContext`]. +#[derive(Deserialize, Serialize, Debug, Clone, PartialEq, Eq)] +pub struct JwtScope { + /// Scope name, such as `org` or `project`. + name: String, + /// Scope value, or `*` to authorize any value at that position. + value: StringOrWildcard, +} + #[derive(Deserialize, Serialize, Debug, Clone)] struct JwtRes { #[serde(rename = "os:usecase")] usecase: String, - - #[serde(flatten)] - scope: BTreeMap, + scopes: Vec, } #[derive(Deserialize, Serialize, Debug, Clone)] @@ -47,7 +54,7 @@ pub struct AuthContext { /// The scope elements that this request may act on. /// /// See also: [`ObjectContext::scopes`]. - pub scopes: BTreeMap, + pub scopes: Vec<(String, StringOrWildcard)>, /// The permissions that this request has been granted. pub permissions: HashSet, @@ -116,7 +123,13 @@ impl AuthContext { let verified_claims = verified_claims.ok_or(AuthError::VerificationFailure)?; let usecase = verified_claims.claims.res.usecase; - let scope = verified_claims.claims.res.scope; + let scopes = verified_claims + .claims + .res + .scopes + .into_iter() + .map(|scope| (scope.name, scope.value)) + .collect(); // Taking the intersection here ensures the `AuthContext` does not have any permissions // that `key_config.max_permissions` doesn't have, even if the token tried to grant them. @@ -129,7 +142,7 @@ impl AuthContext { Ok(AuthContext { usecase, - scopes: scope, + scopes, permissions, }) } @@ -148,16 +161,24 @@ impl AuthContext { return Err(AuthError::NotPermitted); } - 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, + // All authorized scopes need to match a prefix of the requested scopes. Order matters. + let mut request_scopes = context.scopes.iter(); + for (authorized_name, authorized_value) in &self.scopes { + // Always reject when the requested scope is less specific (shorter) than the authorized scope. + let Some(request_scope) = request_scopes.next() else { + return Err(AuthError::NotPermitted); }; + let authorized = authorized_name == request_scope.name() + && match authorized_value { + StringOrWildcard::String(s) => s == request_scope.value(), + StringOrWildcard::Wildcard => true, + }; if !authorized { return Err(AuthError::NotPermitted); } } + // `request_scopes` could contain more values which we didn't consume, which means that the + // request is for a subscope of what this `AuthContext` authorizes, which is fine. Ok(()) } @@ -194,7 +215,7 @@ mod tests { max_permissions, }; PublicKeyDirectory { - keys: BTreeMap::from([(TEST_EDDSA_KID.into(), public_key)]), + keys: std::collections::BTreeMap::from([(TEST_EDDSA_KID.into(), public_key)]), } } @@ -223,8 +244,10 @@ mod tests { serde_json::from_value(json!({ "res": { "os:usecase": usecase, - "org": org, - "project": proj, + "scopes": [ + {"name": "org", "value": org}, + {"name": "project", "value": proj}, + ], }, "permissions": permissions, })) @@ -235,7 +258,16 @@ mod tests { AuthContext { usecase: "attachments".into(), permissions, - scopes: serde_json::from_value(json!({"org": org, "project": proj})).unwrap(), + scopes: vec![ + ( + "org".into(), + StringOrWildcard::deserialize(json!(org)).unwrap(), + ), + ( + "project".into(), + StringOrWildcard::deserialize(json!(proj)).unwrap(), + ), + ], } } @@ -373,22 +405,83 @@ MC4CAQAwBQYDK2VwBCIEIKwVoE4TmTfWoqH3HgLVsEcHs9PHNe+ar/Hp6e4To8pK Ok(()) } - // Allowed: + // Not allowed: // auth_context: org.123 / proj.456 // object: org.123 #[test] - fn test_assert_authorized_org_only_path_allowed() -> Result<(), AuthError> { + fn test_assert_authorized_less_specific_request_scope_fails() -> Result<(), AuthError> { let auth_context = sample_auth_context("123", "456", max_permission()); let object = ObjectContext { usecase: "attachments".into(), scopes: Scopes::from_iter([Scope::create("org", "123").unwrap()]), }; + let result = auth_context.assert_authorized(Permission::ObjectRead, &object); + assert_eq!(result, Err(AuthError::NotPermitted)); + + Ok(()) + } + + // Allowed: + // auth_context: org.123 / proj.* + // object: org.123 / proj.456 / extra.789 + #[test] + fn test_assert_authorized_more_specific_request_scope_succeeds() -> Result<(), AuthError> { + let auth_context = sample_auth_context("123", "*", max_permission()); + let object = ObjectContext { + usecase: "attachments".into(), + scopes: Scopes::from_iter([ + Scope::create("org", "123").unwrap(), + Scope::create("project", "456").unwrap(), + Scope::create("extra", "789").unwrap(), + ]), + }; + auth_context.assert_authorized(Permission::ObjectRead, &object)?; Ok(()) } + // Allowed: + // auth_context: (empty vec) + // object: org.123 + #[test] + fn test_assert_authorized_empty_scope_allows_any_request() -> Result<(), AuthError> { + let auth_context = AuthContext { + usecase: "attachments".into(), + permissions: max_permission(), + scopes: vec![], + }; + let object = ObjectContext { + usecase: "attachments".into(), + scopes: Scopes::from_iter([Scope::create("org", "123").unwrap()]), + }; + + auth_context.assert_authorized(Permission::ObjectRead, &object)?; + + Ok(()) + } + + // Not allowed: + // auth_context: org.123 / proj.* + // object: proj.456 / org.123 + #[test] + fn test_assert_authorized_scope_wrong_order_fails() -> Result<(), AuthError> { + let auth_context = sample_auth_context("123", "*", max_permission()); + let object = ObjectContext { + usecase: "attachments".into(), + scopes: Scopes::from_iter([ + Scope::create("project", "456").unwrap(), + Scope::create("org", "123").unwrap(), + ]), + }; + + let result = auth_context.assert_authorized(Permission::ObjectRead, &object); + assert_eq!(result, Err(AuthError::NotPermitted)); + + Ok(()) + } + // Not allowed: // auth_context: org.123 / proj.456 // object: org.123 / proj.999 diff --git a/objectstore-types/src/scope.rs b/objectstore-types/src/scope.rs index 0aaf2616..104714b6 100644 --- a/objectstore-types/src/scope.rs +++ b/objectstore-types/src/scope.rs @@ -27,9 +27,10 @@ //! 1. **Organization** — they define a hierarchical folder-like structure //! within a usecase. The storage path directly reflects the scope hierarchy //! (e.g. `org.17/project.42/objects/{key}`). -//! 2. **Authorization** — JWT tokens include scope claims that are matched -//! against the request's scopes. A token scoped to `organization=17` can -//! only access objects under that organization. +//! 2. **Authorization** — JWT tokens include ordered scope claims that are +//! matched against the request's scopes by prefix. A token scoped to +//! `organization=17` can only access objects under that organization, and +//! `organization=17/project=*` matches deeper nested scopes in that order. //! 3. **Compartmentalization** — scopes isolate impact through rate limits and //! killswitches, guaranteeing quality of service between tenants. //!