Skip to content

Strip NyxID-org authz: remove visible_org_ids/require_org_writer/OrgRole branches (owner-only) #270

@chronoai-shining

Description

@chronoai-shining

Summary

Collapse the object-layer authorizer to a pure owner-only model: remove the NyxID-org membership/visibility machinery (Ownership.org_id, the org_role param + Rule-3 in allows(), Authorizer::visible_org_ids, Authorizer::require_org_writer) and simplify every call site so a non-owner non-admin is denied without any NyxID org lookup. This change alone fixes the live GET /api/v1/goals 503.

This is the first PR of the Strip NyxID organization concept (owner-only model) milestone. It removes the consumers of OrgRole / user_orgs / NyxIdClient::new(cache_ttl) so the fkst-shared definitions can be deleted in the follow-up (#271). It does NOT yet remove the org_id field from GoalDoc/SessionDoc/DTOs — those are #272 — so the Ownership { owner_user_id } builders in goals.rs/sessions.rs/goals_submit.rs simply stop passing the (still-present) doc.org_id.

Problem / Motivation

  • The owner-only redesign (v0.2.2) already removed the SA-based org checks; the remaining user-token org logic (visible_org_idsuser_orgs, require_org_writer, the OrgRole matrix in allows()) is now dead weight that still gates real endpoints.
  • LIVE BUG (fixed by removal, NOT by patching): authz.rs::visible_org_ids calls NyxIdClient::user_orgs, which deserializes a bare Vec<OrgSummary> while prod NyxID returns {"orgs":[...]}. The serde error → AppError::Unavailable("authorization service unavailable") → hard 503 on GET /api/v1/goals today. Removing visible_org_ids (and its only caller routes/goals.rs::list) eliminates the 503. Do not propose fixing the deserialization — the whole path goes.
  • require_org_writer already admin-bypasses then unconditionally Err(Forbidden), so every org branch it guards is equivalent to a plain owner-or-admin-else-Forbidden once removed.

Proposed Solution — Implementation Spec

backend/fkst-control-plane/src/authz.rs

  1. L42 use crate::nyxid::{NyxIdClient, OrgRole}; → drop OrgRole, keep NyxIdClient (still used by the Authorizer.nyxid field / nyxid() accessor for the credential proxy). Becomes use crate::nyxid::NyxIdClient;.
  2. L53-57 struct Ownership<'a>: delete the pub org_id: Option<&'a str> field (L56). End state { pub owner_user_id: Option<&'a str> }.
  3. L63-92 fn allows(ctx, res, org_role: Option<OrgRole>, action): remove the org_role parameter AND Rule-3 (the if let Some(role) = org_role { match role { Admin => true, Member => Read|Write, Viewer => Read } }, L79-86). New signature fn allows(ctx: &AuthContext, res: Ownership<'_>, action: Action) -> bool; body = Rule1 (admin perm, L69-72) → Rule2 (owner==ctx.user_id, L73-78) → Rule4 (owner_user_id.is_none() → true, L87-90) → false.
  4. L16-36 module doc: drop the Rule-3 / org-role / fail-soft / forwarded-user-token / Ornn-mirror paragraphs; re-list the ordered rules as admin / owner / legacy-ownerless / deny.
  5. L127-169 Authorizer::authorize: remove let org_role = None; (L154) + its [Refactor]: Remove dead exchange_token/proxy_github/DelegatedToken (and SA service_token + NYXID_CLIENT_ID/SECRET once owner-only lands) #257 comment (L149-153); change the call at L156 to allows(ctx, res, action). Remove the doc-comment mention (L122-124) that authorize returns Unavailable on NyxID failure during an org lookup (no org lookup remains).
  6. L171-204 visible_org_ids: DELETE the whole method (sole caller of user_orgs; source of the 503).
  7. L206-224 require_org_writer: DELETE the whole method.
  8. Tests (#[cfg(test)], L227-534): update helper own(owner, org)own(owner) (L245-250); drop the org_role/Some("org-1") args from every surviving allows()/own() call; drop the from_secs(30) cache_ttl arg from NyxIdClient::new(...) test ctors (L374,397,448,502,523). DELETE the org-only tests: viewer_reads_only (276-283), member_writes_not_manages (287-294), admin_manages (298-305), owner_only_client_skips_org_role_lookup (367-388), owner_only_client_require_org_writer_is_forbidden_not_unavailable (390-407), org_member_can_read_org_goal (428-436), non_owner_org_resource_read_is_not_found (438-459). PRESERVE (drop org arg only) the [Bug]: NyxID rejects service-account tokens on human-only endpoints — per-session key revoke leaks & SA-based org authz is non-functional #216/[Feature]: Make the NyxID service account optional — provision per-session token + Ornn from the user token alone (owner-only) #219/[Refactor]: Remove dead exchange_token/proxy_github/DelegatedToken (and SA service_token + NYXID_CLIENT_ID/SECRET once owner-only lands) #257 owner-only safety net: owner_has_full_access, stranger_is_denied_everything, legacy_doc_open_to_authenticated, admin_permission_bypasses, non_admin_permission_does_not_bypass, read_denial_maps_to_not_found, write_denial_maps_to_forbidden, non_owner_no_org_read_is_not_found, session_owner_from_memory_has_full_access, session_stranger_read_is_not_found, owner_path_makes_no_nyxid_call_even_when_a_client_is_present, legacy_no_owner_path_makes_no_nyxid_call.

backend/fkst-control-plane/src/goals/issue_store.rs

  1. L378-405 list(...): drop the visible_org_ids: &[String] param (L381); the visibility predicate (L390-397) collapses to g.owner_user_id == owner_user_id (drop the || g.org_id...contains(o) branch, L391-394). Update the doc comment.
  2. Test call sites: list("user-1", &[], ...) → drop the &[] arg (L1084, L1160). DELETE list_filters_by_in_memory_visibility (L1165-1189, an org-visibility scenario that no longer exists); keep list_filters_by_owner_and_status_newest_first with the new arity. (Note: goal() fixture at L871 still has org_id: None — that field stays until Remove org_id from goal/session models, DTOs (OpenAPI), and the goal-issue marker #272; leave it.)

backend/fkst-control-plane/src/routes/goals.rs

  1. L397 list: delete let org_ids = state.authz.visible_org_ids(&ctx).await?;; L412 state.goals.list(&ctx.user_id, &org_ids, status, limit, offset) → drop &org_ids. Update the list doc-comments (L373/L383) from "owned + org" to "owned". This removes the second source of the 503.
  2. L311-314 create: delete the if let Some(ref org_id) = request.org_id { require_org_writer } block. (CreateGoalRequest.org_id field stays until Remove org_id from goal/session models, DTOs (OpenAPI), and the goal-issue marker #272; the field is simply no longer read here — but since this block is its only authz use, removing it now is correct; Remove org_id from goal/session models, DTOs (OpenAPI), and the goal-issue marker #272 deletes the field.)
  3. L897-914 trigger: remove the if let Some(ref org_id) = goal.org_id { require_org_writer } else { Forbidden } org branch; collapse to owner-or-admin allowed, else Forbidden("only the owner can trigger"). Update doc-comment L824-825.
  4. goal_ownership (L272-277): drop org_id: doc.org_id.as_deref() (L275) from the Ownership literal (required by the Ownership shape change in step 2).

backend/fkst-control-plane/src/routes/goals_submit.rs

  1. authorize_object (L352-369): remove the match &goal.org_id { Some => require_org_writer, None => Forbidden } tail (L363-368); after the owner/admin short-circuit (L357-362), the else is unconditionally Forbidden("only the owner can submit this goal"). Update the fn + call-site doc-comments (L66, L70-73, L132-133, L349-351).

backend/fkst-control-plane/src/routes/repos.rs

  1. fkst_setup (L121-125): remove the if let Some(ref org_id) = request.org_id { require_org_writer } block. L60 delete SetupRequest.org_id (this org_id IS a NyxID org — it calls require_org_writer — and is distinct from org_login). After removal SetupRequest has no fields: keep it as an empty #[serde(deny_unknown_fields)] body for forward-compat (the handler already does body...unwrap_or_default()), and update the #[utoipa::path] request_body description (L95-98) + the module doc (L14, "An optional org_id adds an org-writer check"). Leave SetupRepoRef/SetupResponse.repo and the owner/name path params untouched (GitHub repo, not NyxID org).

backend/fkst-control-plane/src/routes/repos_tests.rs

  1. DELETE org_id_with_non_writer_is_403 (L285-293) — it asserts the deleted require_org_writer returns 403. Keep missing_repo_setup_permission_is_403 and malformed_owner_or_name_is_400.

Scope Check

  • ✅ In scope: NyxID-org authz primitives + their call sites; the SetupRequest.org_id NyxID-org gate; the GoalIssueStore::list org-visibility filter.
  • ❌ Out of scope / KEEP: org_login repo placement; SetupRepoRef/repo owner; the org_id fields on GoalDoc/SessionDoc/CreateGoalRequest/GoalView/SessionView/GoalMarker (deleted in Remove org_id from goal/session models, DTOs (OpenAPI), and the goal-issue marker #272 — they remain present and are simply no longer read by authz after this PR). The Authorizer.nyxid field + nyxid() accessor stay.

Acceptance Criteria / Definition of Done

  • Ownership has only owner_user_id; allows() takes (ctx, res, action); visible_org_ids and require_org_writer are gone.
  • GET /api/v1/goals returns the caller's owned goals and never 503s (add/keep a regression test that the list path makes no NyxID org call).
  • A non-owner non-admin is Forbidden on goal trigger and submit, and on fkst-setup (no org fallback).
  • All preserved owner-only authz tests pass; the seven org-only tests and org_id_with_non_writer_is_403 and list_filters_by_in_memory_visibility are deleted (not weakened).
  • cargo test -p fkst-control-plane green; cargo clippy clean. (OrgRole/user_orgs/cache_ttl still exist in fkst-shared after this PR — they are deleted in Remove user_orgs/OrgSummary/OrgRole/orgs_cache from fkst-shared + nyxid_org_cache_ttl_secs #271; the tree compiles because this PR only removes their use, keeping NyxIdClient::new's third arg in test ctors until Remove user_orgs/OrgSummary/OrgRole/orgs_cache from fkst-shared + nyxid_org_cache_ttl_secs #271.)

Implementation note for the agent: keep NyxIdClient::new's cache_ttl arg in the authz test ctors in THIS PR (it is still part of the signature); #271 drops the parameter and updates those same lines. Order #270 before #271.

Metadata

Metadata

Labels

backendRust/Axum backend worksa:downstream-authzsa-design context: downstream proxy-trust auth + RBACtype:refactorInternal restructuring; no functional change.

Type

No type
No fields configured for issues without a type.

Projects

No projects

Relationships

None yet

Development

No branches or pull requests

Issue actions