From 6bafd808af67c7db45b4fb3da0eebc301fc952bc Mon Sep 17 00:00:00 2001 From: Jonathon Klobucar Date: Sat, 15 Aug 2026 22:03:09 -0700 Subject: [PATCH] fix(client,core,server): stop admin channel actions from desyncing the control stream MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The macOS client's control-stream reader dispatched on the message-type byte but never drained the length-prefixed payload for CreateChannelResponse (0x40), the UpdateChannel/UpdateProfile MetadataUpdateResponse (0x41/0x42), or the DeleteChannel AdminResponse (0x43) — they all fell into the unhandled default case. Every admin channel action (and every profile update, by any user) left that frame unread on the stream, so the next loop iteration misread the previous frame's length prefix as a new message type and corrupted all control- stream parsing for the rest of the session. Add UniFFI records/decoders for the three response shapes (uniffi_bindings.rs) and wire them into the client's message switch, draining and decoding each payload via the existing receiveHardenedPayload path. Failures now surface as a system event instead of being silently dropped, and a rejection with the server's exact "Admin required" message clears the client's isAdmin flag immediately rather than leaving stale admin UI visible until reconnect. Also fix channel ordering: new channels always got position 0 (colliding with each other), and get_server_snapshot iterated a DashMap with no sort, so the position field documented for "custom ordering" never actually ordered anything. New channels now get the next position, and the snapshot sorts by (position, channel_id) to match the DB's own ORDER BY. Verified: full cargo test --workspace, fmt, and clippy -D warnings all clean; macOS client builds and AuraTests passes (three unrelated pre-existing failures confirmed present on unmodified main: MlsProtocolTests/testThreePartyMlsGroup, FuzzTests/ testServerProfileWithRandomData, ConnectionRetryTests/ testSavedConnectionParameters). Signed-off-by: Jonathon Klobucar --- .../Aura/Services/QuicNetworkClient.swift | 102 ++++++++++++++++++ crates/aura-core/src/uniffi_bindings.rs | 60 +++++++++++ crates/aura-server/src/state.rs | 17 ++- 3 files changed, 177 insertions(+), 2 deletions(-) diff --git a/clients/macos/Aura/Services/QuicNetworkClient.swift b/clients/macos/Aura/Services/QuicNetworkClient.swift index 2c867e7..5d9510d 100644 --- a/clients/macos/Aura/Services/QuicNetworkClient.swift +++ b/clients/macos/Aura/Services/QuicNetworkClient.swift @@ -1218,6 +1218,18 @@ public class QuicNetworkClient { case Self.MSG_CHANNEL_DELETED: // 0x47 - The channel we were in was deleted await handleChannelDeleted(stream: stream) + case Self.MSG_CREATE_CHANNEL: // 0x40 - Reply to our CreateChannelRequest + await handleCreateChannelResponse(stream: stream) + + case Self.MSG_UPDATE_CHANNEL: // 0x41 - Reply to our UpdateChannelRequest + await handleUpdateChannelResponse(stream: stream) + + case Self.MSG_UPDATE_PROFILE: // 0x42 - Reply to our UpdateProfile request + await handleUpdateProfileResponse(stream: stream) + + case Self.MSG_DELETE_CHANNEL: // 0x43 - Reply to our DeleteChannelRequest + await handleDeleteChannelResponse(stream: stream) + default: print(String(format: "[QuicClient] Unknown message type: 0x%02X", type)) break @@ -1436,6 +1448,96 @@ public class QuicNetworkClient { } } + /// Handle the server's reply to our CreateChannelRequest. On success the + /// new channel arrives via the ServerState broadcast that follows, so we + /// only need to surface failures here. + private func handleCreateChannelResponse(stream: NWConnection) async { + do { + let payload = try await receiveHardenedPayload(maxLen: Self.MAX_CONTROL_PACKET_SIZE, on: stream) + let response = try decodeCreateChannelResponse(data: payload) + if !response.success { + print("[QuicClient] Create channel failed: \(response.errorMessage)") + await MainActor.run { + self.demoteIfAdminRequired(response.errorMessage) + self.systemEvents.append( + SystemEvent(content: "Failed to create channel: \(response.errorMessage)")) + } + } + } catch { + print("[QuicClient] Failed to parse CreateChannelResponse: \(error)") + } + } + + /// Handle the server's reply to our UpdateChannelRequest. + private func handleUpdateChannelResponse(stream: NWConnection) async { + do { + let payload = try await receiveHardenedPayload(maxLen: Self.MAX_CONTROL_PACKET_SIZE, on: stream) + let response = try decodeMetadataUpdateResponse(data: payload) + if !response.success { + print("[QuicClient] Update channel failed: \(response.errorMessage)") + await MainActor.run { + self.demoteIfAdminRequired(response.errorMessage) + self.systemEvents.append( + SystemEvent(content: "Failed to update channel: \(response.errorMessage)")) + } + } + } catch { + print("[QuicClient] Failed to parse UpdateChannel MetadataUpdateResponse: \(error)") + } + } + + /// Handle the server's reply to our UpdateProfile request. Not + /// admin-gated, so no `isAdmin` bookkeeping here. + private func handleUpdateProfileResponse(stream: NWConnection) async { + do { + let payload = try await receiveHardenedPayload(maxLen: Self.MAX_CONTROL_PACKET_SIZE, on: stream) + let response = try decodeMetadataUpdateResponse(data: payload) + if !response.success { + print("[QuicClient] Update profile failed: \(response.errorMessage)") + await MainActor.run { + self.systemEvents.append( + SystemEvent(content: "Failed to update profile: \(response.errorMessage)")) + } + } + } catch { + print("[QuicClient] Failed to parse UpdateProfile MetadataUpdateResponse: \(error)") + } + } + + /// Handle the server's reply to our DeleteChannelRequest. + private func handleDeleteChannelResponse(stream: NWConnection) async { + do { + let payload = try await receiveHardenedPayload(maxLen: Self.MAX_CONTROL_PACKET_SIZE, on: stream) + let response = try decodeAdminResponse(data: payload) + if !response.success { + print("[QuicClient] Delete channel failed: \(response.errorMessage)") + await MainActor.run { + self.demoteIfAdminRequired(response.errorMessage) + self.systemEvents.append( + SystemEvent(content: "Failed to delete channel: \(response.errorMessage)")) + } + } + } catch { + print("[QuicClient] Failed to parse DeleteChannel AdminResponse: \(error)") + } + } + + /// The server re-checks admin status on every request rather than + /// trusting a cached flag, but our own `isAdmin` is only set once at + /// login (see AuthResponse handling above) and never refreshed. If a + /// request comes back rejected specifically because admin rights are + /// gone, drop the local flag immediately instead of waiting for a + /// reconnect to notice the mismatch — this hides the create/edit/delete + /// UI right away rather than letting the user hit the same rejection + /// repeatedly. + @MainActor + private func demoteIfAdminRequired(_ errorMessage: String) { + if isAdmin && errorMessage == "Admin required" { + print("[QuicClient] Server rejected admin action; clearing stale isAdmin flag") + isAdmin = false + } + } + /// Handle ServerState snapshot (Protobuf via UniFFI) private func handleServerState(stream: NWConnection) async { do { diff --git a/crates/aura-core/src/uniffi_bindings.rs b/crates/aura-core/src/uniffi_bindings.rs index b488d00..c701b07 100644 --- a/crates/aura-core/src/uniffi_bindings.rs +++ b/crates/aura-core/src/uniffi_bindings.rs @@ -655,6 +655,28 @@ pub struct ChannelDeletedRecord { pub fallback_channel_id: String, } +/// Server's reply to a `CreateChannelRequest` (MSG_CREATE_CHANNEL, 0x40). +#[derive(Debug, Clone, uniffi::Record)] +pub struct CreateChannelResponseRecord { + pub success: bool, + pub channel_id: String, + pub error_message: String, +} + +/// Server's reply to `UpdateChannelRequest` (0x41) or `UpdateProfile` (0x42). +#[derive(Debug, Clone, uniffi::Record)] +pub struct MetadataUpdateResponseRecord { + pub success: bool, + pub error_message: String, +} + +/// Server's reply to `DeleteChannelRequest` (0x43) or `DeleteUserRequest` (0x44). +#[derive(Debug, Clone, uniffi::Record)] +pub struct AdminResponseRecord { + pub success: bool, + pub error_message: String, +} + #[derive(Debug, Clone, uniffi::Enum)] pub enum MlsGroupType { Voice, @@ -845,6 +867,44 @@ pub fn decode_channel_deleted(data: Vec) -> Result, +) -> Result { + use prost::Message; + let proto = aura_protocol::CreateChannelResponse::decode(&data[..]) + .map_err(|_| AudioError::PacketParseError)?; + Ok(CreateChannelResponseRecord { + success: proto.success, + channel_id: proto.channel_id, + error_message: proto.error_message, + }) +} + +#[uniffi::export] +pub fn decode_metadata_update_response( + data: Vec, +) -> Result { + use prost::Message; + let proto = aura_protocol::MetadataUpdateResponse::decode(&data[..]) + .map_err(|_| AudioError::PacketParseError)?; + Ok(MetadataUpdateResponseRecord { + success: proto.success, + error_message: proto.error_message, + }) +} + +#[uniffi::export] +pub fn decode_admin_response(data: Vec) -> Result { + use prost::Message; + let proto = aura_protocol::AdminResponse::decode(&data[..]) + .map_err(|_| AudioError::PacketParseError)?; + Ok(AdminResponseRecord { + success: proto.success, + error_message: proto.error_message, + }) +} + #[uniffi::export] pub fn encode_join_channel_request(req: JoinChannelRequestRecord) -> Vec { use prost::Message; diff --git a/crates/aura-server/src/state.rs b/crates/aura-server/src/state.rs index ef3e450..d57be4a 100644 --- a/crates/aura-server/src/state.rs +++ b/crates/aura-server/src/state.rs @@ -558,6 +558,15 @@ impl ServerState { }); } + // DashMap iteration order is unspecified; sort so clients see a + // stable, position-respecting order (mirrors the DB's + // `ORDER BY position, channel_id` in `Database::get_all_channels`). + channels.sort_by(|a, b| { + a.position + .cmp(&b.position) + .then_with(|| a.channel_id.cmp(&b.channel_id)) + }); + let profiles: Vec = self.profiles.iter().map(|p| p.value().clone()).collect(); info!( @@ -1299,6 +1308,10 @@ impl ServerState { ) -> Result { let (icon_type, icon_data) = self.convert_proto_icon(icon); let channel_type = 0; // Default to Regular for manually created channels + // Append after existing channels instead of colliding at 0 — new + // channels previously all landed at position 0, leaving their + // relative order to DashMap iteration (see get_server_snapshot). + let position = self.channel_metadata.len() as i32; let channel_id = self.db.upsert_channel( None, @@ -1306,7 +1319,7 @@ impl ServerState { &comment, icon_type, &icon_data, - 0, + position, channel_type, )?; @@ -1319,7 +1332,7 @@ impl ServerState { comment, icon_type, icon_data, - position: 0, + position, channel_type, }, );