Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion VISION.md
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ The relay enforces all access control. Channel membership is the only gate.
| Type | Visibility | Join | Create |
|------|-----------|------|--------|
| **Open channels** | Searchable by all members | Self-join | Any member |
| **Private channels** | Hidden, invite-only | Invited by an owner/admin | Any member |
| **Private channels** | Hidden, invite-only | Invited by member | Any member |
| **DMs** | Participants only | N/A (up to 9) | Any member |
| **Guests** | Scoped to specific channels | Invited | N/A |

Expand Down
18 changes: 8 additions & 10 deletions crates/buzz-db/src/channel.rs
Original file line number Diff line number Diff line change
Expand Up @@ -371,9 +371,9 @@ async fn acquire_channel_membership_lock(
/// Role enforcement:
/// - Open channels: `invited_by` is optional; role is forced to `Member` regardless of
/// what the caller passes — callers cannot self-assign elevated roles.
/// - Private channels: requires an `invited_by` who is an active owner/admin, the channel
/// creator bootstrapping their own first membership, or the target adding themselves
/// (idempotent re-add — an active member's *role* still cannot change this way).
/// - Private channels: requires an `invited_by` who is an active member, or the channel
/// creator bootstrapping their own first membership. Any active member may add an
/// ordinary member, guest, or bot; only owners/admins may grant elevated roles.
/// - Elevated roles (`Owner`, `Admin`) may only be granted by an existing owner/admin,
/// even on open channels.
///
Expand Down Expand Up @@ -421,14 +421,12 @@ pub async fn add_member(
DbError::InvalidData(format!("invalid role in database: {inviter_role_str}"))
})?;

// Only owners/admins may extend private-channel access to another
// identity. `inviter == pubkey` keeps a member's own idempotent
// re-add working; it is not a role-escalation hole, because the
// active-role-change guard below still rejects a self-targeted
// promotion from any non-elevated caller.
if !inviter_role.is_elevated() && inviter != pubkey {
// Any active member may extend private-channel access with an
// ordinary role. Granting owner/admin remains reserved for an
// existing owner/admin.
if role.is_elevated() && !inviter_role.is_elevated() {
return Err(DbError::AccessDenied(
"only owners/admins may add private-channel members".to_string(),
"only owners/admins may grant elevated roles".to_string(),
));
}
}
Expand Down
33 changes: 16 additions & 17 deletions crates/buzz-relay/src/handlers/side_effects.rs
Original file line number Diff line number Diff line change
Expand Up @@ -358,23 +358,22 @@ pub async fn validate_admin_event(
let target_pubkey =
extract_p_tag(event).ok_or_else(|| anyhow::anyhow!("missing p tag"))?;

// PUT_USER: open channels allow any authenticated user. Private
// channels only let owners/admins add another identity; otherwise
// any compromised member could extend access to channel history.
//
// A self-targeted add skips this check so an idempotent re-add
// still works. That is not a way into a private channel: ingest's
// `check_channel_membership` rejects a non-member (and a
// soft-removed member) before this validator runs, and `add_member`
// independently requires the self-inviter to hold an active role.
// Self-promotion is caught by the role-change guard below.
if channel.visibility == "private"
&& target_pubkey != actor_bytes
&& !actor_role.is_some_and(|r| r.is_elevated())
{
return Err(anyhow::anyhow!(
"only owners/admins may add private-channel members"
));
// PUT_USER: open channels allow any authenticated user; private channels
// require the actor to be an existing active member. Any active member may
// add an ordinary member, guest, or bot, but only owners/admins may grant
// an elevated role.
if channel.visibility == "private" {
if actor_role.is_none() {
return Err(anyhow::anyhow!("actor not authorized"));
}

if requested_role.is_some_and(|role| role.is_elevated())
&& !actor_role.is_some_and(|role| role.is_elevated())
{
return Err(anyhow::anyhow!(
"only owners/admins may grant elevated roles"
));
}
}

// Changing an ACTIVE existing member's role is privileged in both
Expand Down
113 changes: 66 additions & 47 deletions crates/buzz-test-client/tests/e2e_relay.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2249,69 +2249,88 @@ async fn add_member_with_role_ws(
(ok.accepted, ok.message)
}

/// Only owners/admins can add another identity to a private channel.
/// Any active member can add any ordinary role to a private channel.
#[tokio::test]
#[ignore]
async fn test_private_channel_member_cannot_invite() {
async fn test_private_channel_any_member_can_invite() {
let url = relay_url();
let owner_keys = Keys::generate();
let member_keys = Keys::generate();
let invitee_keys = Keys::generate();
let actors = [
("member", Keys::generate()),
("guest", Keys::generate()),
("bot", Keys::generate()),
];

// Connect as owner and create a private channel.
let mut owner_client = BuzzTestClient::connect(&url, &owner_keys)
.await
.expect("connect as owner");
let channel_id = create_private_channel_ws(&mut owner_client, &owner_keys).await;

// Owner adds member_keys as a regular member.
let (accepted, msg) = add_member_ws(
&mut owner_client,
&channel_id,
&member_keys.public_key().to_hex(),
&owner_keys,
)
.await;
assert!(accepted, "owner should add member, got: {msg}");
// Seed one actor for each ordinary active role.
for (role, keys) in &actors {
let (accepted, msg) = add_member_with_role_ws(
&mut owner_client,
&channel_id,
&keys.public_key().to_hex(),
role,
&owner_keys,
)
.await;
assert!(accepted, "owner should add {role} actor, got: {msg}");
}

// Connect as the regular member.
let mut member_client = BuzzTestClient::connect(&url, &member_keys)
.await
.expect("connect as member");
// Exercise the full ordinary-role target matrix. Relay and DB authorization
// both run here, unlike the Desktop/mobile policy-unit-test mirrors.
for (actor_role, actor_keys) in &actors {
let mut actor_client = BuzzTestClient::connect(&url, actor_keys)
.await
.unwrap_or_else(|err| panic!("connect as {actor_role}: {err}"));

for target_role in ["member", "guest", "bot"] {
let target_keys = Keys::generate();
let target_pubkey_hex = target_keys.public_key().to_hex();
let (accepted, msg) = add_member_with_role_ws(
&mut actor_client,
&channel_id,
&target_pubkey_hex,
target_role,
actor_keys,
)
.await;
assert!(
accepted,
"private-channel {actor_role} should add {target_role}, got: {msg}"
);
assert_eq!(
member_role(&url, &owner_keys, &channel_id, &target_pubkey_hex).await,
Some(target_role.to_string()),
"private-channel {actor_role} add must persist the {target_role} role"
);
}

// Regular member tries to invite a third user.
let (accepted, msg) = add_member_ws(
&mut member_client,
&channel_id,
&invitee_keys.public_key().to_hex(),
&member_keys,
)
.await;
assert!(
!accepted,
"regular member must not add another private-channel identity: {msg}"
);
assert!(
msg.contains("owners/admins"),
"rejection should name the owner/admin requirement, got: {msg}"
);
// Re-adding oneself stays idempotent — the huddle bot-add and kind:9021
// paths depend on a self-targeted PUT_USER working.
let (accepted, msg) = add_member_with_role_ws(
&mut actor_client,
&channel_id,
&actor_keys.public_key().to_hex(),
actor_role,
actor_keys,
)
.await;
assert!(
accepted,
"self-targeted {actor_role} re-add must stay idempotent, got: {msg}"
);

// The same member re-adding *themselves* stays idempotent — the huddle
// bot-add and kind:9021 paths depend on a self-targeted PUT_USER working.
let (accepted, msg) = add_member_ws(
&mut member_client,
&channel_id,
&member_keys.public_key().to_hex(),
&member_keys,
)
.await;
assert!(
accepted,
"self-targeted re-add must stay idempotent, got: {msg}"
);
actor_client
.disconnect()
.await
.unwrap_or_else(|err| panic!("disconnect {actor_role}: {err}"));
}

owner_client.disconnect().await.expect("disconnect owner");
member_client.disconnect().await.expect("disconnect member");
}

/// An admin — not just the owner — can still add to a private channel.
Expand Down
28 changes: 13 additions & 15 deletions desktop/src/features/channels/lib/channelMemberAdmission.test.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@ test("open channels accept adds from anyone, member or not", () => {
);
});

test("private channels accept adds only from owners/admins", () => {
for (const selfRole of ["owner", "admin"]) {
test("private channels accept adds from every active member role", () => {
for (const selfRole of ["owner", "admin", "member", "bot", "guest"]) {
assert.equal(
canAddChannelMembers({
channelType: "stream",
Expand All @@ -35,17 +35,15 @@ test("private channels accept adds only from owners/admins", () => {
);
}

for (const selfRole of ["member", "bot", "guest", null]) {
assert.equal(
canAddChannelMembers({
channelType: "stream",
visibility: "private",
selfRole,
}),
false,
`${selfRole} must not be able to add`,
);
}
assert.equal(
canAddChannelMembers({
channelType: "stream",
visibility: "private",
selfRole: null,
}),
false,
"a non-member must not be able to add",
);
});

test("DMs never accept adds, even from an owner", () => {
Expand All @@ -67,13 +65,13 @@ test("DMs never accept adds, even from an owner", () => {
);
});

test("unknown visibility fails closed for non-elevated callers", () => {
test("unknown visibility fails closed", () => {
assert.equal(
canAddChannelMembers({ channelType: "stream", selfRole: "member" }),
false,
);
assert.equal(
canAddChannelMembers({ channelType: "stream", selfRole: "owner" }),
true,
false,
);
});
13 changes: 8 additions & 5 deletions desktop/src/features/channels/lib/channelMemberAdmission.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,8 @@
*
* - DMs: nobody — membership is fixed at creation.
* - Open channels: anyone, member or not.
* - Private channels: owners/admins only. A plain member extending access to
* channel history is exactly what the relay now rejects, so the affordance
* must not be offered.
* - Private channels: any active member. The relay separately reserves elevated
* role grants and active-role changes for owners/admins.
*
* Unknown visibility fails closed — the relay is the authority and a hidden
* button is cheaper than an opaque rejection.
Expand All @@ -28,9 +27,13 @@ export function canAddChannelMembers({
return true;
}

return selfRole === "owner" || selfRole === "admin";
if (visibility === "private") {
return selfRole != null;
}

return false;
}

/** Explains a denied add so the user isn't left guessing at a missing button. */
export const PRIVATE_CHANNEL_ADD_DENIED_MESSAGE =
"Only channel owners and admins can add people to a private channel.";
"Only channel members can add people to a private channel.";
4 changes: 2 additions & 2 deletions desktop/src/features/channels/ui/MembersSidebar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -251,8 +251,8 @@ export function MembersSidebar({
visibility: channel?.visibility,
selfRole: selfMember?.role,
});
// Distinguish "you can't add here" from "nothing to add" so a plain member of
// a private channel gets the reason instead of a silently missing affordance.
// Distinguish "you can't add here" from "nothing to add" so a non-member
// viewing a private channel gets the reason instead of a silently missing affordance.
const showPrivateAddDeniedNotice =
!canAddMembers &&
selfMember !== null &&
Expand Down
22 changes: 9 additions & 13 deletions desktop/tests/e2e/channels.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3828,7 +3828,7 @@ test("members sidebar retains distinct same-persona managed agents", async ({
await expect(page.getByText("Pinky", { exact: true })).toHaveCount(2);
});

test("private-channel members cannot add people without owner/admin", async ({
test("private-channel members can add people and managed agents without admin", async ({
page,
}) => {
await installMockBridge(page, {
Expand All @@ -3842,26 +3842,22 @@ test("private-channel members cannot add people without owner/admin", async ({
});
await page.goto("/");
// secret-projects is a private (non-DM) channel where the current user is a
// plain member. The relay rejects their kind:9000, so the affordance is
// withheld and the reason shown instead of failing after the fact.
// plain member. Active members may add ordinary members and bots; only
// elevated-role grants and role changes require owner/admin authority.
await openMembersSidebar(page, "secret-projects");

await expect(page.getByTestId("members-sidebar-add-denied")).toBeVisible();
// The field stays, but only as a filter over existing members.
await expect(page.getByTestId("members-sidebar-add-denied")).toHaveCount(0);
await expect(
page.getByTestId("channel-management-search-users"),
).toHaveAttribute("placeholder", "Search people and agents");
).toHaveAttribute("placeholder", "Add people and agents");

await page.getByTestId("channel-management-search-users").fill("char");
await expect(page.getByText("Not in this channel")).toHaveCount(0);
await expect(
page.getByTestId(
`channel-user-search-result-${TEST_IDENTITIES.charlie.pubkey}`,
),
).toHaveCount(0);
await page
.getByTestId(`channel-user-search-result-${TEST_IDENTITIES.charlie.pubkey}`)
.click();
await expect(
page.getByTestId(`sidebar-member-${TEST_IDENTITIES.charlie.pubkey}`),
).toHaveCount(0);
).toContainText("charlie");
});

test("open-channel members can add people and managed agents without admin", async ({
Expand Down
9 changes: 5 additions & 4 deletions mobile/lib/features/channels/channel.dart
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ const Object _sentinel = Object();
/// Shown when a private-channel add is refused, so a missing Invite action
/// reads as a rule rather than a bug.
const privateChannelAddDeniedMessage =
'Only channel owners and admins can add people to a private channel.';
'Only channel members can add people to a private channel.';

@immutable
class Channel {
Expand Down Expand Up @@ -85,12 +85,13 @@ class Channel {

/// Whether [selfRole] may add *another* identity here, mirroring the relay's
/// kind:9000 authority (`validate_admin_event` + `add_member`): DMs never,
/// open channels always, private channels owners/admins only. An unknown
/// visibility fails closed — the relay is the authority.
/// open channels always, private channels for any active member. Elevated
/// grants remain reserved for owners/admins. Unknown visibility fails closed.
bool canAddMembers(String? selfRole) {
if (isDm) return false;
if (visibility == 'open') return true;
return selfRole == 'owner' || selfRole == 'admin';
if (visibility == 'private') return selfRole != null;
return false;
}

bool get isArchived => archivedAt != null;
Expand Down
2 changes: 1 addition & 1 deletion mobile/lib/features/channels/compose_bar/helpers.dart
Original file line number Diff line number Diff line change
Expand Up @@ -279,7 +279,7 @@ Future<_NonMemberAddOutcome> _addMentionedNonMembers(
];
if (pending.isEmpty) return _NonMemberAddOutcome.empty;

// A plain member of a private channel cannot add anyone: skip the doomed
// A non-member cannot add anyone to a private channel: skip the doomed
// kind:9000 rather than trading it for a relay rejection.
if (!canAddMembers) {
return _NonMemberAddOutcome(
Expand Down
Loading
Loading