Skip to content

Commit 2ac683e

Browse files
committed
fix: exclude unauthorized watches from the per-user cap
1 parent 4686040 commit 2ac683e

1 file changed

Lines changed: 193 additions & 8 deletions

File tree

operations/src/notifications/watch/subscriptions.rs

Lines changed: 193 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@ pub async fn create_watch_subscription(
8080
now_ms: u64,
8181
) -> Result<WatchSubscription, WatchSubscriptionError> {
8282
let subscription = validated_subscription(owner, path_prefix, event_mask, now_ms)?;
83-
create_subscription(storage, subscription, None).await
83+
create_subscription(storage, subscription, None, None).await
8484
}
8585

8686
/// The one holder-side create path, used by both the local API arm and the
@@ -109,8 +109,13 @@ pub async fn create_replicated_watch_subscription(
109109
return Err(WatchSubscriptionError::Unauthorized);
110110
}
111111
let replication = watch_upsert_replication(local_node_id, &subscription)?;
112-
let subscription =
113-
create_subscription(&context.storage_handle, subscription, Some(replication)).await?;
112+
let subscription = create_subscription(
113+
&context.storage_handle,
114+
subscription,
115+
Some(replication),
116+
Some(context),
117+
)
118+
.await?;
114119
schedule_replication(context).await;
115120
Ok(subscription)
116121
}
@@ -153,13 +158,14 @@ async fn create_subscription(
153158
storage: &StorageHandle,
154159
subscription: WatchSubscription,
155160
replication: Option<WatchReplication>,
161+
authorizer: Option<&DriverContext>,
156162
) -> Result<WatchSubscription, WatchSubscriptionError> {
157-
match create_once(storage, &subscription, replication.as_ref()).await {
163+
match create_once(storage, &subscription, replication.as_ref(), authorizer).await {
158164
Ok(()) => Ok(subscription),
159165
Err(CreateFailure::Cap) => Err(WatchSubscriptionError::CapExceeded),
160166
Err(CreateFailure::Fatal(error)) => Err(WatchSubscriptionError::Storage(error)),
161167
Err(CreateFailure::Conflict) => {
162-
match create_once(storage, &subscription, replication.as_ref()).await {
168+
match create_once(storage, &subscription, replication.as_ref(), authorizer).await {
163169
Ok(()) => Ok(subscription),
164170
Err(CreateFailure::Cap) => Err(WatchSubscriptionError::CapExceeded),
165171
Err(CreateFailure::Fatal(error)) => Err(WatchSubscriptionError::Storage(error)),
@@ -175,6 +181,7 @@ async fn create_once(
175181
storage: &StorageHandle,
176182
subscription: &WatchSubscription,
177183
replication: Option<&WatchReplication>,
184+
authorizer: Option<&DriverContext>,
178185
) -> Result<(), CreateFailure> {
179186
let txn_id = match storage
180187
.send_storage_effect(StorageEffect::StartTransaction { read: false })
@@ -211,8 +218,22 @@ async fn create_once(
211218
}
212219
};
213220
if existing.len() >= NOTIFICATION_WATCH_PER_USER_CAP {
214-
abort_txn(storage, txn_id).await;
215-
return Err(CreateFailure::Cap);
221+
let cap_reached = match authorizer {
222+
None => true,
223+
Some(context) => {
224+
match authorized_cap_reached(context, storage, subscription.owner, txn_id).await {
225+
Ok(reached) => reached,
226+
Err(failure) => {
227+
abort_txn(storage, txn_id).await;
228+
return Err(failure);
229+
}
230+
}
231+
}
232+
};
233+
if cap_reached {
234+
abort_txn(storage, txn_id).await;
235+
return Err(CreateFailure::Cap);
236+
}
216237
}
217238

218239
let subscription_write = match watch_subscription_write_entry(subscription) {
@@ -277,6 +298,68 @@ async fn create_once(
277298
}
278299
}
279300

301+
/// Whether the owner already holds the per-user cap of subscriptions that still
302+
/// pass the authorization check. Consulted only once the raw row count reaches
303+
/// the cap, so a create below the cap adds no auth checks. Rows the owner can no
304+
/// longer read are excluded, so a revoked row never permanently holds a cap
305+
/// slot. The scan stops at the cap, bounding a rejected create.
306+
async fn authorized_cap_reached(
307+
context: &DriverContext,
308+
storage: &StorageHandle,
309+
owner: UserId,
310+
txn_id: TxnId,
311+
) -> Result<bool, CreateFailure> {
312+
let mut authorized = 0usize;
313+
let mut start = None;
314+
loop {
315+
let (values, next) = match storage
316+
.send_storage_effect(StorageEffect::Iter {
317+
key_space: NOTIFICATION_WATCH_SUBSCRIPTIONS_KEYSPACE.to_string(),
318+
prefix: Some(watch_subscription_prefix(owner)),
319+
start: start.map(IterStart::After),
320+
limit: NOTIFICATION_WATCH_PER_USER_CAP.saturating_add(1),
321+
txn_id: Some(txn_id),
322+
})
323+
.await
324+
{
325+
Event::Storage(StorageEvent::IterResult {
326+
values,
327+
next_start_after,
328+
}) => (values, next_start_after),
329+
Event::Storage(StorageEvent::Error { error }) => return Err(classify(error)),
330+
other => {
331+
return Err(CreateFailure::Fatal(format!(
332+
"unexpected storage event: {other:?}"
333+
)));
334+
}
335+
};
336+
for (_, value) in values {
337+
let subscription = WatchSubscription::from_bytes(&value)
338+
.map_err(|error| CreateFailure::Fatal(error.to_string()))?;
339+
if is_watch_authorized(
340+
context,
341+
subscription.owner.realm_id,
342+
subscription.owner,
343+
&subscription.path_prefix,
344+
subscription.event_mask,
345+
)
346+
.await
347+
.map_err(CreateFailure::Fatal)?
348+
{
349+
authorized += 1;
350+
if authorized >= NOTIFICATION_WATCH_PER_USER_CAP {
351+
return Ok(true);
352+
}
353+
}
354+
}
355+
match next {
356+
Some(next) => start = Some(next),
357+
None => break,
358+
}
359+
}
360+
Ok(false)
361+
}
362+
280363
pub async fn delete_watch_subscription(
281364
storage: &StorageHandle,
282365
owner: UserId,
@@ -641,7 +724,12 @@ async fn abort_and_classify(
641724
#[cfg(test)]
642725
mod tests {
643726
use super::*;
644-
use aruna_core::structs::{RealmId, WatchEventKind};
727+
use aruna_core::NodeId;
728+
use aruna_core::keyspaces::AUTH_KEYSPACE;
729+
use aruna_core::structs::{
730+
Actor, GroupAuthorizationDocument, RealmAuthorizationDocument, RealmId, WatchEventKind,
731+
data_watch_resource_path,
732+
};
645733
use aruna_storage::FjallStorage;
646734
use tempfile::tempdir;
647735

@@ -663,6 +751,64 @@ mod tests {
663751
])
664752
}
665753

754+
fn data_mask() -> WatchEventMask {
755+
WatchEventMask::from_kinds([WatchEventKind::DataUploaded])
756+
}
757+
758+
fn node(seed: u8) -> NodeId {
759+
iroh::SecretKey::from_bytes(&[seed; 32]).public()
760+
}
761+
762+
fn test_context(storage: StorageHandle) -> DriverContext {
763+
DriverContext {
764+
storage_handle: storage,
765+
net_handle: None,
766+
blob_handle: None,
767+
metadata_handle: None,
768+
task_handle: None,
769+
}
770+
}
771+
772+
async fn install_auth(
773+
storage: &StorageHandle,
774+
realm_id: RealmId,
775+
owner: UserId,
776+
group_id: Ulid,
777+
node_id: NodeId,
778+
) {
779+
let actor = Actor {
780+
node_id,
781+
user_id: owner,
782+
realm_id,
783+
};
784+
let realm_auth = RealmAuthorizationDocument::new_default_realm_doc(realm_id);
785+
let group_auth =
786+
GroupAuthorizationDocument::new_default_group_doc(owner, realm_id, group_id);
787+
for (key, value) in [
788+
(
789+
realm_id.as_bytes().to_vec(),
790+
realm_auth.to_bytes(&actor).unwrap(),
791+
),
792+
(
793+
group_id.to_bytes().to_vec(),
794+
group_auth.to_bytes(&actor).unwrap(),
795+
),
796+
] {
797+
match storage
798+
.send_storage_effect(StorageEffect::Write {
799+
key_space: AUTH_KEYSPACE.to_string(),
800+
key: key.into(),
801+
value: value.into(),
802+
txn_id: None,
803+
})
804+
.await
805+
{
806+
Event::Storage(StorageEvent::WriteResult { .. }) => {}
807+
other => panic!("unexpected auth write event: {other:?}"),
808+
}
809+
}
810+
}
811+
666812
#[tokio::test]
667813
async fn create_then_list_roundtrips() {
668814
let (_dir, storage) = temp_storage();
@@ -743,6 +889,45 @@ mod tests {
743889
);
744890
}
745891

892+
// A user churned through revocations: every stored row is now unreadable.
893+
// Those dead rows must not permanently occupy cap slots, so an authorized
894+
// create still succeeds even though the raw row count is already at the cap.
895+
#[tokio::test]
896+
async fn cap_skips_unauthorized() {
897+
let (_dir, storage) = temp_storage();
898+
let realm_id = RealmId([7u8; 32]);
899+
let owner = UserId::new(Ulid::from_bytes([1u8; 16]), realm_id);
900+
let group_id = Ulid::from_bytes([2u8; 16]);
901+
let node_id = node(3);
902+
install_auth(&storage, realm_id, owner, group_id, node_id).await;
903+
904+
let dead_prefix = data_watch_resource_path(Ulid::nil(), node_id, "bucket", "");
905+
for index in 0..NOTIFICATION_WATCH_PER_USER_CAP {
906+
create_watch_subscription(
907+
&storage,
908+
owner,
909+
dead_prefix.clone(),
910+
data_mask(),
911+
index as u64,
912+
)
913+
.await
914+
.expect("dead row create");
915+
}
916+
917+
let context = test_context(storage);
918+
let authorized_prefix = data_watch_resource_path(group_id, node_id, "bucket", "reports/");
919+
create_replicated_watch_subscription(
920+
&context,
921+
node_id,
922+
owner,
923+
authorized_prefix,
924+
data_mask(),
925+
9_999,
926+
)
927+
.await
928+
.expect("authorized create despite dead rows at cap");
929+
}
930+
746931
#[tokio::test]
747932
async fn cap_is_scoped_per_owner() {
748933
let (_dir, storage) = temp_storage();

0 commit comments

Comments
 (0)