This is an automated email from the ASF dual-hosted git repository. krishvishal pushed a commit to branch sim-workload-faults in repository https://gitbox.apache.org/repos/asf/iggy.git
commit a1c8918d874a107e112ce599fc1b5b051cce7f9b Author: Krishna Vishal <[email protected]> AuthorDate: Sat Aug 15 12:08:49 2026 +0530 fix(simulator): send the client PAT shape through the dispatch layer Both personal-access-token operations were refused with `InvalidCommand` on every attempt through the dispatch shell, never once committing, while the raw path accepted them. `SimClient` sent the REPLICATED shape, `[user_id][name][expiry][token_hash]`, where a client sends `[name][expiry]` and the server resolves the acting user from the session and mints the token and its hash itself in `maybe_rewrite_pat_request`. A client cannot produce the replicated shape, since it does not know the hash; the previous code filled it with a stub. That went unnoticed because the raw path has no dispatch layer and therefore no rewrite, so a request submitted there must already be replicated and the wrong shape was the right one. Both remain reachable, chosen by which path the client took rather than by configuration, mirroring the register/login split that already exists for sessions. Through the shell the tokens now commit. --- core/simulator/src/client.rs | 54 ++++++++++++++++++++++++++++++++----- core/simulator/src/lib.rs | 63 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 111 insertions(+), 6 deletions(-) diff --git a/core/simulator/src/client.rs b/core/simulator/src/client.rs index 88e0d9d08..f70bd5a48 100644 --- a/core/simulator/src/client.rs +++ b/core/simulator/src/client.rs @@ -31,6 +31,10 @@ use iggy_binary_protocol::requests::messages::{ use iggy_binary_protocol::requests::partitions::{ CreatePartitionsRequest, DeletePartitionsRequest, }; +use iggy_binary_protocol::requests::personal_access_tokens::{ + CreatePersonalAccessTokenRequest as WireCreatePersonalAccessTokenRequest, + DeletePersonalAccessTokenRequest as WireDeletePersonalAccessTokenRequest, +}; use iggy_binary_protocol::requests::segments::DeleteSegmentsRequest; use iggy_binary_protocol::requests::streams::{ CreateStreamRequest, DeleteStreamRequest, PurgeStreamRequest, UpdateStreamRequest, @@ -79,6 +83,21 @@ pub struct SimClient { /// the body a pure function of the seed. See [`SimClient::next_message_id`]. message_counter: Cell<u64>, session: Cell<u64>, + /// Whether this client talks to the server's real dispatch layer. + /// + /// It changes what a PAT request must contain. A real client sends + /// `[name][expiry]` and the server mints the token and its hash in + /// `maybe_rewrite_pat_request`, rewriting the request into the replicated + /// form before it reaches consensus. The simulator's raw path has no dispatch + /// layer and therefore no rewrite, so a request submitted there has to arrive + /// already in the replicated form. + /// + /// The same split already exists for sessions (`register` for the raw path, + /// `login` for the shell); this makes it explicit for the one op family whose + /// BODY differs rather than just its envelope. Set by + /// `Simulator::shell_login_via`, so it follows the path the client actually + /// took rather than being configured separately. + shell_wire: Cell<bool>, } impl SimClient { @@ -90,9 +109,17 @@ impl SimClient { partition_counter: Cell::new(0), message_counter: Cell::new(0), session: Cell::new(0), + shell_wire: Cell::new(false), } } + /// Mark this client as talking to the real dispatch layer, so PAT requests + /// carry the client wire shape rather than the replicated one. See + /// [`SimClient::shell_wire`]. + pub fn set_shell_wire(&self) { + self.shell_wire.set(true); + } + #[must_use] pub const fn client_id(&self) -> u128 { self.client_id @@ -503,14 +530,23 @@ impl SimClient { name: &str, expiry: u64, ) -> Message<RoutedRequestHeader> { + let name = WireName::new(name).expect("PAT name must be valid"); + // Through dispatch, send what a real client sends: the server resolves + // the acting user from the session and mints the token and its hash in + // `maybe_rewrite_pat_request`, rewriting this into the replicated form + // before consensus sees it. A client cannot produce that form itself -- + // it does not know the hash -- so sending it here is what made every PAT + // request fail to decode as `InvalidCommand`. + if self.shell_wire.get() { + let wire = WireCreatePersonalAccessTokenRequest { name, expiry }; + return self.build_request(Operation::CreatePersonalAccessToken, &wire.to_bytes()); + } + // Raw path: no dispatch layer, so no rewrite ever happens and the request + // has to arrive already replicated. let wire = CreatePersonalAccessTokenRequest { user_id: 0, - name: WireName::new(name).expect("PAT name must be valid"), + name, expiry, - // Deterministic stub for the simulator. Production servers mint - // this in `maybe_rewrite_pat_request` on the primary; the - // simulator drives the wire path directly without that rewrite - // step. token_hash: [b'a'; 64], }; self.build_request(Operation::CreatePersonalAccessToken, &wire.to_bytes()) @@ -519,9 +555,15 @@ impl SimClient { /// # Panics /// Panics if `name` is not a valid `WireName`. pub fn delete_personal_access_token(&self, name: &str) -> Message<RoutedRequestHeader> { + let name = WireName::new(name).expect("PAT name must be valid"); + // See `create_personal_access_token` for why the shape depends on the path. + if self.shell_wire.get() { + let wire = WireDeletePersonalAccessTokenRequest { name }; + return self.build_request(Operation::DeletePersonalAccessToken, &wire.to_bytes()); + } let wire = DeletePersonalAccessTokenRequest { user_id: 0, - name: WireName::new(name).expect("PAT name must be valid"), + name, only_if_expired: false, }; self.build_request(Operation::DeletePersonalAccessToken, &wire.to_bytes()) diff --git a/core/simulator/src/lib.rs b/core/simulator/src/lib.rs index 27135d4b0..a266dc65d 100644 --- a/core/simulator/src/lib.rs +++ b/core/simulator/src/lib.rs @@ -573,6 +573,9 @@ impl Simulator { )); } + // From here this client talks the real client protocol; see + // `SimClient::shell_wire`. + client.set_shell_wire(); let msg = client .login(replica::SHELL_ROOT_USERNAME, replica::SHELL_ROOT_PASSWORD) .into_generic(); @@ -3596,6 +3599,66 @@ mod tests { // Wiring a snapshot coordinator is what closes that gap. } + /// Personal-access-token requests commit through the dispatch layer. + /// + /// They used to be refused there with `InvalidCommand` on every attempt, + /// because `SimClient` sent the REPLICATED shape + /// (`[user_id][name][expiry][token_hash]`) rather than the client one + /// (`[name][expiry]`). A client cannot produce the replicated shape: it does + /// not know the token hash, which the server mints in + /// `maybe_rewrite_pat_request` after resolving the acting user from the + /// session. The raw path has no dispatch layer and so no rewrite, which is + /// why the wrong shape worked there and hid the bug. + /// + /// Asserts a commit rather than merely a reply: a denial is also a reply, and + /// it was the denials that went unnoticed for so long. + #[test] + fn personal_access_tokens_commit_through_the_dispatch_shell() { + server_common::MemoryPool::init_pool(&server_common::MemoryPoolConfigOther { + enabled: false, + size: iggy_common::IggyByteSize::from(0u64), + bucket_capacity: 1, + }); + + let replica_count: u8 = 3; + let client_id: u128 = 1; + let network_opts = packet::PacketSimulatorOptions { + node_count: replica_count, + client_count: 1, + seed: 0x9A7_0001, + ..packet::PacketSimulatorOptions::default() + }; + let mut sim = Simulator::with_shards_shell( + usize::from(replica_count), + 1, + std::iter::once(client_id), + network_opts, + ); + let client = SimClient::new(client_id); + sim.shell_login(&client); + + let create = client.create_personal_access_token("wl-pat-token", 0); + let request = create.header().request; + sim.submit_request(client_id, 0, create.into_generic()); + let mut status = None; + for _ in 0..400 { + if let Some(reply) = sim + .step() + .into_iter() + .find(|reply| reply.header().request == request) + { + status = Some(reply.header().status); + break; + } + } + assert_eq!( + status, + Some(0), + "creating a personal access token through dispatch was refused; a \ + nonzero status here means the request never reached consensus" + ); + } + /// A client that dials a BACKUP still gets a working session, and the login /// travels as a forwarded consensus proposal rather than a redirect. ///
