Skip to content

Commit 3595b60

Browse files
committed
fix: authorize watch creation and enumeration on the holder
1 parent 10dfc4f commit 3595b60

6 files changed

Lines changed: 259 additions & 31 deletions

File tree

api/src/routes/notifications.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -181,6 +181,9 @@ fn map_watch_dispatch_error(error: WatchDispatchError, operation: &str) -> Serve
181181
WatchDispatchError::CapExceeded => {
182182
ServerError::Conflict("notification watch subscription cap reached".to_string())
183183
}
184+
// A holder-side denial answers exactly as the create-time check does, so
185+
// an unreadable path never separates into a distinct existence signal.
186+
WatchDispatchError::Unauthorized => ServerError::Forbidden,
184187
WatchDispatchError::Internal(reason) => ServerError::InternalError(reason),
185188
WatchDispatchError::Remote(reason) => {
186189
warn!(operation, reason = %reason, "notification holder proxy failed");

operations/src/notifications/dispatch.rs

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,10 +16,11 @@ use crate::notifications::list::{ListNotificationsInput, ListNotificationsOperat
1616
use crate::notifications::mark_read::{MarkReadInput, MarkReadOperation};
1717
use crate::notifications::placement::resolve_inbox_holder;
1818
use crate::notifications::unread::{UnreadCountInput, UnreadCountOperation};
19+
use crate::notifications::watch::authorization::list_authorized_watch_subscriptions;
1920
use crate::notifications::watch::interest::schedule_watch_interest_publish;
2021
use crate::notifications::watch::subscriptions::{
21-
WATCH_SUBSCRIPTION_CAP_REACHED, WatchSubscriptionError, create_replicated_watch_subscription,
22-
delete_replicated_watch_subscription, list_watch_subscriptions,
22+
WATCH_SUBSCRIPTION_CAP_REACHED, WATCH_SUBSCRIPTION_UNAUTHORIZED, WatchSubscriptionError,
23+
create_replicated_watch_subscription, delete_replicated_watch_subscription,
2324
};
2425

2526
/// Outcome of serving a user's inbox read op through the resolved holder.
@@ -36,13 +37,16 @@ pub enum NotificationDispatchError {
3637
}
3738

3839
/// Watch CRUD dispatch outcome. Distinguishes the per-user cap so the REST layer
39-
/// can answer 409 instead of a generic proxy failure.
40+
/// can answer 409, and an unauthorized watched path so it can answer 403, instead
41+
/// of a generic proxy failure.
4042
#[derive(Debug, Error)]
4143
pub enum WatchDispatchError {
4244
#[error("no inbox holder is currently available")]
4345
Unavailable,
4446
#[error("notification watch subscription cap reached")]
4547
CapExceeded,
48+
#[error("{WATCH_SUBSCRIPTION_UNAUTHORIZED}")]
49+
Unauthorized,
4650
#[error("holder proxy failed: {0}")]
4751
Remote(String),
4852
#[error("{0}")]
@@ -216,6 +220,7 @@ pub async fn create_watch_for_user(
216220
.await
217221
.map_err(|error| match error {
218222
WatchSubscriptionError::CapExceeded => WatchDispatchError::CapExceeded,
223+
WatchSubscriptionError::Unauthorized => WatchDispatchError::Unauthorized,
219224
other => WatchDispatchError::Internal(other.to_string()),
220225
})?;
221226
schedule_watch_interest_publish(context).await;
@@ -230,6 +235,8 @@ pub async fn create_watch_for_user(
230235
.map_err(|reason| {
231236
if reason == WATCH_SUBSCRIPTION_CAP_REACHED {
232237
WatchDispatchError::CapExceeded
238+
} else if reason == WATCH_SUBSCRIPTION_UNAUTHORIZED {
239+
WatchDispatchError::Unauthorized
233240
} else {
234241
WatchDispatchError::Remote(reason)
235242
}
@@ -274,9 +281,9 @@ pub async fn list_watches_for_user(
274281
) -> Result<Vec<WatchSubscription>, WatchDispatchError> {
275282
let holder = resolve_holder(context, owner).await?;
276283
if holder == local_node_id {
277-
list_watch_subscriptions(&context.storage_handle, owner)
284+
list_authorized_watch_subscriptions(context, owner)
278285
.await
279-
.map_err(|error| WatchDispatchError::Internal(error.to_string()))
286+
.map_err(WatchDispatchError::Internal)
280287
} else {
281288
let net_handle = context
282289
.net_handle

operations/src/notifications/incoming.rs

Lines changed: 105 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -25,13 +25,13 @@ use crate::notifications::outbox::NOTIFICATION_OUTBOX_DRAIN_BATCH_SIZE;
2525
use crate::notifications::placement::resolve_inbox_holder;
2626
use crate::notifications::protocol::{NotificationTransportMessage, notification_message_kind};
2727
use crate::notifications::unread::{UnreadCountInput, UnreadCountOperation};
28+
use crate::notifications::watch::authorization::list_authorized_watch_subscriptions;
2829
use crate::notifications::watch::expand::expand_watch_events;
2930
use crate::notifications::watch::interest::{
3031
mark_watch_interest_dirty, schedule_watch_interest_publish,
3132
};
3233
use crate::notifications::watch::subscriptions::{
3334
create_replicated_watch_subscription, delete_replicated_watch_subscription,
34-
list_watch_subscriptions,
3535
};
3636

3737
const NOTIFICATION_MAX_FUTURE_SKEW_MS: u64 = 5 * 60 * 1000;
@@ -245,9 +245,9 @@ async fn build_response(
245245
return NotificationTransportMessage::Reject(reason);
246246
}
247247

248-
match list_watch_subscriptions(&context.storage_handle, owner).await {
248+
match list_authorized_watch_subscriptions(context, owner).await {
249249
Ok(subscriptions) => NotificationTransportMessage::WatchList { subscriptions },
250-
Err(error) => NotificationTransportMessage::Reject(error.to_string()),
250+
Err(error) => NotificationTransportMessage::Reject(error),
251251
}
252252
}
253253
NotificationTransportMessage::DeliverWatchEvents { events } => {
@@ -653,7 +653,9 @@ mod tests {
653653
unread_count_remote,
654654
};
655655
use crate::notifications::inbox::upsert_inbox_records;
656-
use crate::notifications::watch::subscriptions::create_watch_subscription;
656+
use crate::notifications::watch::subscriptions::{
657+
WATCH_SUBSCRIPTION_UNAUTHORIZED, create_watch_subscription, list_watch_subscriptions,
658+
};
657659
use aruna_core::keyspaces::{
658660
AUTH_KEYSPACE, NOTIFICATION_INBOX_KEYSPACE, NOTIFICATION_WATCH_INTEREST_KEYSPACE,
659661
};
@@ -1446,6 +1448,7 @@ mod tests {
14461448
.await;
14471449

14481450
let owner = recipient_for_holder(&config, b.net.node_id(), realm_id);
1451+
install_watch_authorization(&b, realm_id, owner, &[]).await;
14491452
let mask = WatchEventMask::from_kinds([WatchEventKind::DataUploaded]);
14501453
let prefix = data_path("prefix");
14511454
let created = create_watch_remote(&a.net, b.net.node_id(), owner, prefix.clone(), mask)
@@ -1525,6 +1528,104 @@ mod tests {
15251528
);
15261529
}
15271530

1531+
// The holder persists and replicates the subscription, so a proxying peer's
1532+
// assertion is not authority to watch: an owner without READ is refused there
1533+
// too, and nothing durable is written.
1534+
#[tokio::test]
1535+
async fn create_requires_read() {
1536+
let realm_id = RealmId::from_bytes([86u8; 32]);
1537+
let a = spawn(realm_id, [86u8; 32]).await;
1538+
let b = spawn(realm_id, [87u8; 32]).await;
1539+
connect(&a, &b).await;
1540+
let config = install_config(
1541+
&b,
1542+
realm_id,
1543+
&[
1544+
(a.net.node_id(), RealmNodeKind::Server),
1545+
(b.net.node_id(), RealmNodeKind::Server),
1546+
],
1547+
)
1548+
.await;
1549+
1550+
let owner = recipient_for_holder(&config, b.net.node_id(), realm_id);
1551+
// The watched group exists and is readable, just not by `owner`.
1552+
install_watch_authorization(&b, realm_id, UserId::new(Ulid::r#gen(), realm_id), &[]).await;
1553+
1554+
let error = create_watch_remote(
1555+
&a.net,
1556+
b.net.node_id(),
1557+
owner,
1558+
data_path("prefix"),
1559+
WatchEventMask::from_kinds([WatchEventKind::DataUploaded]),
1560+
)
1561+
.await
1562+
.expect_err("an owner without READ must be refused by the holder");
1563+
assert_eq!(error, WATCH_SUBSCRIPTION_UNAUTHORIZED);
1564+
assert!(
1565+
list_watch_subscriptions(&b.context.storage_handle, owner)
1566+
.await
1567+
.expect("list succeeds")
1568+
.is_empty(),
1569+
"an unauthorized create must not persist watch state"
1570+
);
1571+
}
1572+
1573+
// Enumeration shares the delivery authorization result: once READ is revoked
1574+
// the watch stops being listed, though the stored row itself survives.
1575+
#[tokio::test]
1576+
async fn revoked_watch_unlisted() {
1577+
let realm_id = RealmId::from_bytes([84u8; 32]);
1578+
let a = spawn(realm_id, [84u8; 32]).await;
1579+
let b = spawn(realm_id, [85u8; 32]).await;
1580+
connect(&a, &b).await;
1581+
let config = install_config(
1582+
&b,
1583+
realm_id,
1584+
&[
1585+
(a.net.node_id(), RealmNodeKind::Server),
1586+
(b.net.node_id(), RealmNodeKind::Server),
1587+
],
1588+
)
1589+
.await;
1590+
1591+
let owner = recipient_for_holder(&config, b.net.node_id(), realm_id);
1592+
install_watch_authorization(&b, realm_id, owner, &[]).await;
1593+
let created = create_watch_remote(
1594+
&a.net,
1595+
b.net.node_id(),
1596+
owner,
1597+
data_path("prefix"),
1598+
WatchEventMask::from_kinds([WatchEventKind::DataUploaded]),
1599+
)
1600+
.await
1601+
.expect("authorized create succeeds");
1602+
assert_eq!(
1603+
list_watches_remote(&a.net, b.net.node_id(), owner)
1604+
.await
1605+
.expect("list succeeds"),
1606+
vec![created]
1607+
);
1608+
1609+
// Re-owning the group drops the original owner's READ.
1610+
install_watch_authorization(&b, realm_id, UserId::new(Ulid::r#gen(), realm_id), &[]).await;
1611+
1612+
assert!(
1613+
list_watches_remote(&a.net, b.net.node_id(), owner)
1614+
.await
1615+
.expect("list after revocation succeeds")
1616+
.is_empty(),
1617+
"a watch whose READ was revoked must not be enumerable"
1618+
);
1619+
assert_eq!(
1620+
list_watch_subscriptions(&b.context.storage_handle, owner)
1621+
.await
1622+
.expect("stored row survives")
1623+
.len(),
1624+
1,
1625+
"the row is filtered at the surface, not silently deleted"
1626+
);
1627+
}
1628+
15281629
fn upload_event(realm_id: RealmId, actor: UserId, path: &str) -> WatchEvent {
15291630
let (_, key) = path.split_once('/').expect("bucket/key fixture");
15301631
WatchEvent {

0 commit comments

Comments
 (0)