Skip to content

Commit 682377c

Browse files
Address review: allow revocation, cover the guards, redraw on team change
- `SetLinkPermissions` only guards grants. `None` revokes link access, and blocking it left a user unable to tighten an over-shared object in exactly the race the guard exists for. - Cover the guards themselves. `UpdateManager` marks an object as having a pending permissions change synchronously, before it issues any request, so the new tests can assert whether a dispatch reached the target. - Subscribe to `UserWorkspaces`: nothing else re-rendered the dialog on a team change, so the invite form and link row stayed on screen under the old team's policy while the guards refused to act on them. - Rename the unresolvable-window test, which named the teamless case but tests a window `UserWorkspaces` never saw, and correct the two docs that said an unresolved team denies when only an unresolvable window does.
1 parent ec270fe commit 682377c

4 files changed

Lines changed: 176 additions & 20 deletions

File tree

app/src/drive/sharing/dialog/mod.rs

Lines changed: 41 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ use crate::word_block_editor::{
6060
WordBlockStyles,
6161
};
6262
use crate::workspace::{ToastStack, WorkspaceAction};
63-
use crate::workspaces::user_workspaces::{TeamContext, UserWorkspaces};
63+
use crate::workspaces::user_workspaces::{TeamContext, UserWorkspaces, UserWorkspacesEvent};
6464
use crate::{TelemetryEvent, send_telemetry_from_ctx};
6565

6666
mod inheritance;
@@ -262,6 +262,10 @@ impl SharingDialog {
262262
},
263263
);
264264

265+
ctx.subscribe_to_model(&UserWorkspaces::handle(ctx), |me, _, event, ctx| {
266+
me.handle_user_workspaces_event(event, ctx);
267+
});
268+
265269
let invite_form = EmailInviteForm {
266270
email_editor: ctx.add_typed_action_view(|ctx| {
267271
let mut view = WordBlockEditorView::new(
@@ -302,6 +306,26 @@ impl SharingDialog {
302306
}
303307
}
304308

309+
/// The link-sharing controls are drawn from this window's team policy, so a team change has
310+
/// to redraw: the invite form and the link-sharing row appear and disappear with it, and a
311+
/// control left on screen under the old team's policy is one the action guards now refuse.
312+
fn handle_user_workspaces_event(
313+
&mut self,
314+
event: &UserWorkspacesEvent,
315+
ctx: &mut ViewContext<Self>,
316+
) {
317+
let changes_this_windows_policy = match event {
318+
UserWorkspacesEvent::TeamsChanged => true,
319+
UserWorkspacesEvent::WindowTeamChanged { window_id } => *window_id == ctx.window_id(),
320+
// Everything else this model emits is workspace administration, which does not
321+
// change which team this window shares as.
322+
_ => false,
323+
};
324+
if changes_this_windows_policy {
325+
ctx.notify();
326+
}
327+
}
328+
305329
fn handle_update_manager_event(
306330
&mut self,
307331
event: &UpdateManagerEvent,
@@ -478,23 +502,27 @@ impl SharingDialog {
478502
/// The team this dialog's window is sharing as, resolved afresh on every read so a window
479503
/// that moves to another team is never governed by the team it opened with.
480504
///
481-
/// `None` when the dialog's window cannot be located, which for a live dialog means its
482-
/// window is gone: `RootView::new` registers every window as it is created.
505+
/// `None` only when no scope can be resolved at all: this view is no longer in a window,
506+
/// its window was never registered with `UserWorkspaces`, or its window names a team that
507+
/// has since left the current workspace. A window that simply *has* no team is a different
508+
/// thing — it resolves a scope, whose `team_uid()` is then `None`.
483509
fn team_scope<'a>(&self, app: &'a AppContext) -> Option<TeamContext<'a>> {
484510
UserWorkspaces::as_ref(app).team_context(&self.self_handle, app)
485511
}
486512

487513
/// Whether this window's team permits sharing the target with anyone who holds its link.
488-
/// A window whose team cannot be resolved denies: offering a sharing channel whose
489-
/// governing policy is unknown is the unsafe direction to guess in.
514+
///
515+
/// A window with no team permits it, per `TeamScope`'s contract. Only a window with no
516+
/// resolvable scope denies, because offering a sharing channel when we cannot say which
517+
/// policy governs it is the unsafe direction to guess in.
490518
fn can_anyone_with_link_share(&self, app: &AppContext) -> bool {
491519
self.team_scope(app).is_some_and(|scope| {
492520
UserWorkspaces::as_ref(app).is_anyone_with_link_sharing_enabled(&scope)
493521
})
494522
}
495523

496524
/// Whether this window's team permits sharing the target directly with named people. See
497-
/// [`Self::can_anyone_with_link_share`] for why an unresolvable team denies.
525+
/// [`Self::can_anyone_with_link_share`] for what an unresolvable scope means.
498526
fn can_direct_link_share(&self, app: &AppContext) -> bool {
499527
self.team_scope(app)
500528
.is_some_and(|scope| UserWorkspaces::as_ref(app).is_direct_link_sharing_enabled(&scope))
@@ -1634,8 +1662,10 @@ impl SharingDialog {
16341662
fn send_invitations(&mut self, ctx: &mut ViewContext<Self>) {
16351663
// Re-read the policy instead of trusting the render that put the form on screen: an
16361664
// open dialog whose window moved to a team that forbids direct sharing must not send
1637-
// the invitations that team now disallows.
1665+
// the invitations that team now disallows. Redraw on the way out so the form the user
1666+
// just submitted disappears rather than sitting there as a dead control.
16381667
if !self.can_direct_link_share(ctx) {
1668+
ctx.notify();
16391669
return;
16401670
}
16411671

@@ -2956,8 +2986,10 @@ impl TypedActionView for SharingDialog {
29562986
self.set_open_menu(OpenMenuState::None, ctx);
29572987
// The menu's items were built when it opened. Re-read the policy so a window
29582988
// that has since moved to a team forbidding link sharing cannot act on the
2959-
// permission it was offered under the old one.
2960-
if !self.can_anyone_with_link_share(ctx) {
2989+
// permission it was offered under the old one. Only granting is blocked:
2990+
// `None` revokes link access, and a user tightening an over-shared object must
2991+
// not be turned away by the very policy that wants it tightened.
2992+
if access_level.is_some() && !self.can_anyone_with_link_share(ctx) {
29612993
ctx.notify();
29622994
return;
29632995
}

app/src/drive/sharing/dialog/mod_tests.rs

Lines changed: 129 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,22 @@
11
use chrono::Local;
22
use session_sharing_protocol::common::SessionId;
3-
use warpui::{App, SingletonEntity, ViewHandle};
4-
5-
use super::{SharingDialog, SharingDialogMode};
6-
use crate::drive::sharing::ShareableObject;
3+
use warpui::{App, SingletonEntity, TypedActionView, ViewHandle};
4+
5+
use super::{SharingDialog, SharingDialogAction, SharingDialogMode};
6+
use crate::auth::UserUid;
7+
use crate::cloud_object::model::persistence::CloudModel;
8+
use crate::cloud_object::{CloudObject, Owner};
9+
use crate::drive::sharing::{ShareableObject, SharingAccessLevel};
10+
use crate::server::ids::{ClientId, ServerId, SyncId};
711
use crate::terminal::TerminalView;
812
use crate::terminal::shared_session::manager::Manager;
913
use crate::terminal::shared_session::{SharedSessionSource, SharedSessionStatus};
1014
use crate::test_util::add_window_with_terminal;
1115
use crate::test_util::terminal::{
1216
add_window_with_id_and_terminal, initialize_app_for_terminal_view,
1317
};
18+
use crate::workflows::workflow::Workflow;
19+
use crate::workflows::{CloudWorkflow, CloudWorkflowModel};
1420
use crate::workspaces::team::{Team, TeamVisibility};
1521
use crate::workspaces::user_workspaces::UserWorkspaces;
1622
use crate::workspaces::workspace::{
@@ -310,24 +316,139 @@ fn link_sharing_gates_follow_a_window_onto_its_new_team() {
310316
});
311317
}
312318

319+
/// Distinct from a window that has no team, which resolves a scope and is *permitted* to share.
320+
/// A window `UserWorkspaces` has never seen resolves no scope at all, and there is then no
321+
/// policy to consult.
313322
#[test]
314-
fn link_sharing_gates_deny_when_the_dialogs_window_has_no_team() {
323+
fn link_sharing_gates_deny_when_the_dialogs_window_cannot_be_resolved() {
315324
App::test((), |mut app| async move {
316325
initialize_app_for_terminal_view(&mut app);
317326

318327
let (window_id, _terminal) = add_window_with_id_and_terminal(&mut app, None);
319328
install_workspace_with_teams(&mut app, vec![team_with_link_sharing(123, "team", true)]);
320329

321-
// Deliberately left unregistered: no window team can be resolved for this dialog.
330+
// Deliberately never registered: `add_window_with_id_and_terminal` roots the window in
331+
// a `TerminalView`, so nothing calls `UserWorkspaces::register_window` for it.
322332
let dialog = app.add_typed_action_view(window_id, |ctx| SharingDialog::new(None, ctx));
323333

324334
dialog.read(&app, |dialog, ctx| {
325335
assert!(
326336
!dialog.can_anyone_with_link_share(ctx),
327-
"a dialog whose window team cannot be resolved must not offer a sharing channel \
328-
governed by an unknown policy"
337+
"a dialog with no resolvable scope must not offer a sharing channel governed by \
338+
an unknown policy"
329339
);
330340
assert!(!dialog.can_direct_link_share(ctx));
331341
});
332342
});
333343
}
344+
345+
/// Puts a Warp Drive object in the cloud model under a server id, so the sharing dialog can
346+
/// target it and `UpdateManager` can find it.
347+
fn add_shareable_object(app: &mut App) -> ServerId {
348+
let object_uid: ServerId = 789.into();
349+
let mut object = CloudWorkflow::new_local(
350+
CloudWorkflowModel {
351+
data: Workflow::new("shared workflow", "echo shared"),
352+
},
353+
Owner::User {
354+
user_uid: UserUid::new("owner"),
355+
},
356+
None,
357+
ClientId::default(),
358+
);
359+
object.id = SyncId::ServerId(object_uid);
360+
361+
let cloud_model = CloudModel::handle(&*app);
362+
cloud_model.update(app, |cloud_model, _| {
363+
cloud_model.add_object(object.id, object);
364+
});
365+
object_uid
366+
}
367+
368+
/// Whether a permissions change reached the object. `UpdateManager` marks the object
369+
/// synchronously, before it issues any request, so a dispatch the dialog refused leaves this
370+
/// clear.
371+
fn permissions_change_reached_object(app: &App, object_uid: ServerId) -> bool {
372+
app.read(|ctx| {
373+
CloudModel::as_ref(ctx)
374+
.get_by_uid(&object_uid.uid())
375+
.expect("the targeted object should be in the cloud model")
376+
.metadata()
377+
.pending_changes_statuses
378+
.has_pending_permissions_change
379+
})
380+
}
381+
382+
fn set_link_permissions(
383+
dialog: &ViewHandle<SharingDialog>,
384+
access_level: Option<SharingAccessLevel>,
385+
app: &mut App,
386+
) {
387+
dialog.update(app, |dialog, ctx| {
388+
dialog.handle_action(&SharingDialogAction::SetLinkPermissions(access_level), ctx);
389+
});
390+
}
391+
392+
/// The link-sharing menu builds its items once, when it opens, so acting on one has to re-read
393+
/// the policy. Deleting the guard in the `SetLinkPermissions` handler must fail this test.
394+
#[test]
395+
fn set_link_permissions_refuses_to_grant_under_a_forbidding_team() {
396+
App::test((), |mut app| async move {
397+
initialize_app_for_terminal_view(&mut app);
398+
399+
let (window_id, _terminal) = add_window_with_id_and_terminal(&mut app, None);
400+
let forbidden_team = team_with_link_sharing(456, "forbids-sharing", false);
401+
install_workspace_with_teams(&mut app, vec![forbidden_team.clone()]);
402+
403+
let user_workspaces = UserWorkspaces::handle(&app);
404+
user_workspaces.update(&mut app, |user_workspaces, ctx| {
405+
user_workspaces.set_team_for_window(window_id, forbidden_team.uid, ctx);
406+
});
407+
408+
let object_uid = add_shareable_object(&mut app);
409+
let dialog = app.add_typed_action_view(window_id, |ctx| {
410+
SharingDialog::new(Some(ShareableObject::WarpDriveObject(object_uid)), ctx)
411+
});
412+
413+
set_link_permissions(&dialog, Some(SharingAccessLevel::View), &mut app);
414+
assert!(
415+
!permissions_change_reached_object(&app, object_uid),
416+
"granting link access under a team that forbids it must not reach the object"
417+
);
418+
419+
// Revoking is how a user tightens an over-shared object, so the guard must let it
420+
// through: the forbidding policy is the reason to allow this, not to block it.
421+
set_link_permissions(&dialog, None, &mut app);
422+
assert!(
423+
permissions_change_reached_object(&app, object_uid),
424+
"revoking link access must go through even under a team that forbids granting it"
425+
);
426+
});
427+
}
428+
429+
#[test]
430+
fn set_link_permissions_grants_under_a_permitting_team() {
431+
App::test((), |mut app| async move {
432+
initialize_app_for_terminal_view(&mut app);
433+
434+
let (window_id, _terminal) = add_window_with_id_and_terminal(&mut app, None);
435+
let permitted_team = team_with_link_sharing(123, "permits-sharing", true);
436+
install_workspace_with_teams(&mut app, vec![permitted_team.clone()]);
437+
438+
let user_workspaces = UserWorkspaces::handle(&app);
439+
user_workspaces.update(&mut app, |user_workspaces, ctx| {
440+
user_workspaces.set_team_for_window(window_id, permitted_team.uid, ctx);
441+
});
442+
443+
let object_uid = add_shareable_object(&mut app);
444+
let dialog = app.add_typed_action_view(window_id, |ctx| {
445+
SharingDialog::new(Some(ShareableObject::WarpDriveObject(object_uid)), ctx)
446+
});
447+
448+
set_link_permissions(&dialog, Some(SharingAccessLevel::View), &mut app);
449+
assert!(
450+
permissions_change_reached_object(&app, object_uid),
451+
"the guard must let a grant through when the window's team permits link sharing"
452+
);
453+
});
454+
}

app/src/workspaces/user_workspaces.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1950,6 +1950,10 @@ impl UserWorkspaces {
19501950
};
19511951
}
19521952

1953+
// No window reaches this today: one is only registered teamless while the user has no
1954+
// teams, and `reconcile_window_team_assignments` moves it onto a team the moment any
1955+
// appear. It stops being dead the first time a caller reads a
1956+
// [`TeamContextForOperation`] captured before its window had a team.
19531957
if self.has_teams() {
19541958
return LinkSharingSettings::UNRESTRICTED;
19551959
}

app/src/workspaces/workspace.rs

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -979,9 +979,8 @@ pub struct LinkSharingSettings {
979979

980980
impl LinkSharingSettings {
981981
/// The policy in force when neither a team nor a workspace governs link sharing: both
982-
/// channels are permitted. Distinct from `Default`, whose all-`false` shape is the
983-
/// starting point for decoding a policy that *is* governed.
984-
pub const UNRESTRICTED: Self = Self {
982+
/// channels are permitted. Deliberately not `Default`, which is all-`false`.
983+
pub(crate) const UNRESTRICTED: Self = Self {
985984
anyone_with_link_sharing_enabled: true,
986985
direct_link_sharing_enabled: true,
987986
};

0 commit comments

Comments
 (0)