refactor(server): add ChannelMember extractor for channel-scoped routes (#83) #110

Merged

icub3d merged 1 commit from feature/83-channel-member into main

2026-07-11 04:00:12 +00:00

icub3d commented

2026-07-11 03:59:50 +00:00

Owner

Finishes #83. The first checklist item — one crate::error::ApiError + crate-wide ApiResult — landed in #104; this PR delivers the second: the ChannelMember extractor.

Closes #83.

What it does

ChannelMember (server/src/permissions.rs) is a FromRequestParts extractor for routes under /channels/:channel_id/…. It runs the prologue those handlers repeated by hand: authentication + server membership (same checks as MemberUser), resolving the channel_id path parameter, channel existence (404 channel not found), and channel-scoped effective_permissions — computed once. The old UserPermissions + effective_permissions pairing computed base permissions twice per request; converted handlers now do it once.

Converted handlers (11)

messages: create / list / get / update / delete
reactions: put_reaction / delete_own / delete_user / list_reactors
attachments: upload_attachment
threads: create_thread

Each loses its ensure_channel_exists + effective_permissions prologue and checks bits via member.has(…). channels::update_channel/delete_channel keep the explicit call — there the channel is the target (step 6), not the parent.

Also

permissions.rs had its own private PermissionRejection/ErrorBody — exactly the duplication this issue is about. Both extractors now reject with crate::error::ApiError, and they share an authed_member() prologue.
Canonical check-order doc updated: channel resolution/permissions now happen in the extractor (step 1), ahead of the in-handler feature-flag check. The one observable consequence — a bogus channel returns 404 even when the feature is disabled (previously 403) — is pinned by a new regression test (reactions::tests::missing_channel_404s_even_when_reactions_disabled).

No wire-format changes; client and SDKs untouched.

Verification

cargo test --workspace — 210 passed (all existing router-level handler tests exercise the converted routes; 1 new)
cargo clippy --workspace --all-targets -- -D warnings, cargo fmt --check — clean

🤖 Generated with Claude Code

Finishes #83. The first checklist item — one `crate::error::ApiError` + crate-wide `ApiResult` — landed in #104; this PR delivers the second: the **`ChannelMember` extractor**. Closes #83. ## What it does `ChannelMember` (`server/src/permissions.rs`) is a `FromRequestParts` extractor for routes under `/channels/:channel_id/…`. It runs the prologue those handlers repeated by hand: authentication + server membership (same checks as `MemberUser`), resolving the `channel_id` path parameter, channel existence (`404 channel not found`), and channel-scoped `effective_permissions` — computed **once**. The old `UserPermissions` + `effective_permissions` pairing computed base permissions twice per request; converted handlers now do it once. ## Converted handlers (11) - `messages`: create / list / get / update / delete - `reactions`: put_reaction / delete_own / delete_user / list_reactors - `attachments`: upload_attachment - `threads`: create_thread Each loses its `ensure_channel_exists` + `effective_permissions` prologue and checks bits via `member.has(…)`. `channels::update_channel`/`delete_channel` keep the explicit call — there the channel is the *target* (step 6), not the parent. ## Also - `permissions.rs` had its own private `PermissionRejection`/`ErrorBody` — exactly the duplication this issue is about. Both extractors now reject with `crate::error::ApiError`, and they share an `authed_member()` prologue. - Canonical check-order doc updated: channel resolution/permissions now happen in the extractor (step 1), ahead of the in-handler feature-flag check. The one observable consequence — a bogus channel returns 404 even when the feature is disabled (previously 403) — is pinned by a new regression test (`reactions::tests::missing_channel_404s_even_when_reactions_disabled`). No wire-format changes; client and SDKs untouched. ## Verification - `cargo test --workspace` — 210 passed (all existing router-level handler tests exercise the converted routes; 1 new) - `cargo clippy --workspace --all-targets -- -D warnings`, `cargo fmt --check` — clean 🤖 Generated with [Claude Code](https://claude.com/claude-code)

icub3d added 1 commit

2026-07-11 03:59:50 +00:00

refactor(server): add ChannelMember extractor for channel-scoped routes (#83 )

Security Audit / Gitleaks (Secrets) (pull_request) Successful in 7s

Details

Security Audit / Trivy (SCA & IaC) (pull_request) Failing after 17s

Details

Security Audit / PNPM Audit (JS SCA) (pull_request) Failing after 23s

Details

Security Audit / Semgrep (SAST) (pull_request) Failing after 1m3s

Details

CI / Client — test & lint (pull_request) Successful in 1m5s

Details

Security Audit / Cargo Audit (Rust SCA) (pull_request) Failing after 3m29s

Details

CI / Server — test & lint (pull_request) Successful in 5m10s

Details

778d42ede6

The second half of #83 (the ApiError consolidation landed in #104): a
ChannelMember extractor that runs the prologue every /channels/:channel_id
handler repeated by hand — auth + membership, resolving the channel_id path
param, channel existence (404), and channel-scoped effective_permissions.

- Convert the 11 channel-scoped handlers (messages x5, reactions x4,
  attachments upload, create_thread) to it; each also drops the duplicate
  base-permission lookup the old UserPermissions + effective_permissions
  pairing performed.
- Replace permissions.rs's own PermissionRejection/ErrorBody duplicates with
  crate::error::ApiError; share the auth+membership prologue between
  MemberUser and ChannelMember.
- Update the canonical check-order doc: channel resolution now happens in the
  extractor, ahead of in-handler feature-flag checks. New regression test pins
  the one observable consequence (bogus channel 404s even when the feature is
  disabled).

Closes #83.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>