From 85bbabcbdfab84c236a7e1d1c57eb37602cf009b Mon Sep 17 00:00:00 2001 From: MechaCat02 Date: Wed, 27 May 2026 21:31:08 +0200 Subject: [PATCH] feat(api): app members CRUD endpoints MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds /api/v1/admin/apps/{id_or_slug}/members[/{user_id}]: - GET list members (joined with admin_users via list_for_app_enriched) - POST grant membership — 201 with enriched DTO 409 on duplicate (promotions go through PATCH on purpose so the UI can surface "already a member" cleanly) 422 if the target user is deactivated 422 if the target's instance_role isn't `member` — owners and admins already have implicit authority, so an explicit row would be dead weight - PATCH change role — 200 with enriched DTO 404 if no existing membership (use POST to create) - DELETE remove — 204, idempotent (matches the repo's `remove` contract; 204 also when the row never existed) All four gated on `Capability::AppAdmin(app_id)`. Editors and viewers get 403 from list and never see the dashboard's Members tab. No last-app-admin guard: owners implicitly satisfy AppAdmin via `role_grants`, so removing the last explicit app_admin row cannot permanently orphan an app — an owner can always re-issue grants. Wires through picloud/src/lib.rs by splitting the Postgres app_members repo Arc into two trait views (AppMembersRepository for CRUD, AuthzRepo for the existing capability lookups) without re-instantiating against the pool. Co-Authored-By: Claude Opus 4.7 (1M context) --- crates/manager-core/src/app_members_api.rs | 331 +++++++++++++++++++++ crates/manager-core/src/lib.rs | 2 + crates/picloud/src/lib.rs | 37 ++- 3 files changed, 358 insertions(+), 12 deletions(-) create mode 100644 crates/manager-core/src/app_members_api.rs diff --git a/crates/manager-core/src/app_members_api.rs b/crates/manager-core/src/app_members_api.rs new file mode 100644 index 0000000..9c8bd91 --- /dev/null +++ b/crates/manager-core/src/app_members_api.rs @@ -0,0 +1,331 @@ +//! `/api/v1/admin/apps/{id_or_slug}/members/*` — CRUD over the +//! `app_members` table (blueprint §11.6). +//! +//! Every endpoint is gated on `Capability::AppAdmin(app_id)` after +//! resolving the app from `id_or_slug`. Editors and viewers receive +//! 403 from list and never see the dashboard's Members tab. +//! +//! POST is **non-idempotent on purpose**: a duplicate `(app_id, +//! user_id)` returns 409 rather than upsert-200, so the UI can show +//! "already a member — promote / demote them instead" cleanly. Role +//! changes go through PATCH. +//! +//! No last-app-admin guard: owners always implicitly satisfy +//! `Capability::AppAdmin(_)` (authz::role_grants), so removing the +//! final explicit `app_admin` membership cannot orphan an app. + +use std::sync::Arc; + +use axum::extract::{Path, State}; +use axum::http::StatusCode; +use axum::response::{IntoResponse, Json, Response}; +use axum::routing::{get, patch}; +use axum::{Extension, Router}; +use chrono::{DateTime, Utc}; +use picloud_shared::{AdminUserId, AppId, AppRole, InstanceRole, Principal}; +use serde::{Deserialize, Serialize}; +use serde_json::json; +use uuid::Uuid; + +use crate::admin_user_repo::{AdminUserRepository, AdminUserRepositoryError, AdminUserRow}; +use crate::app_members_repo::{ + AppMembersRepository, AppMembersRepositoryError, AppMembershipDetail, AppMembershipRow, +}; +use crate::app_repo::AppRepository; +use crate::authz::{require, AuthzDenied, AuthzRepo, Capability}; +use crate::repo::ScriptRepositoryError; + +#[derive(Clone)] +pub struct AppMembersState { + pub apps: Arc, + pub users: Arc, + pub members: Arc, + pub authz: Arc, +} + +pub fn app_members_router(state: AppMembersState) -> Router { + Router::new() + .route( + "/apps/{id_or_slug}/members", + get(list_members).post(grant_member), + ) + .route( + "/apps/{id_or_slug}/members/{user_id}", + patch(patch_member).delete(remove_member), + ) + .with_state(state) +} + +// ---------------------------------------------------------------------------- +// DTOs +// ---------------------------------------------------------------------------- + +#[derive(Debug, Serialize)] +pub struct AppMemberDto { + pub user_id: AdminUserId, + pub username: String, + pub email: Option, + pub instance_role: InstanceRole, + pub is_active: bool, + pub role: AppRole, + pub created_at: DateTime, +} + +impl From for AppMemberDto { + fn from(d: AppMembershipDetail) -> Self { + Self { + user_id: d.user_id, + username: d.username, + email: d.email, + instance_role: d.instance_role, + is_active: d.is_active, + role: d.role, + created_at: d.created_at, + } + } +} + +/// Compose a DTO from an `AdminUserRow` (fetched for validation) and +/// the `AppMembershipRow` returned by `upsert`. Saves a re-fetch on +/// POST/PATCH at the cost of trusting the two inputs reference the +/// same user_id — caller's responsibility. +fn compose_dto(user: AdminUserRow, membership: AppMembershipRow) -> AppMemberDto { + AppMemberDto { + user_id: user.id, + username: user.username, + email: user.email, + instance_role: user.instance_role, + is_active: user.is_active, + role: membership.role, + created_at: membership.created_at, + } +} + +#[derive(Debug, Deserialize)] +pub struct GrantMemberRequest { + pub user_id: AdminUserId, + pub role: AppRole, +} + +#[derive(Debug, Deserialize)] +pub struct PatchMemberRequest { + pub role: AppRole, +} + +// ---------------------------------------------------------------------------- +// Handlers +// ---------------------------------------------------------------------------- + +async fn list_members( + State(s): State, + Extension(principal): Extension, + Path(id_or_slug): Path, +) -> Result>, AppMembersApiError> { + let app = resolve_app(&*s.apps, &id_or_slug).await?; + require(s.authz.as_ref(), &principal, Capability::AppAdmin(app.id)).await?; + let rows = s.members.list_for_app_enriched(app.id).await?; + Ok(Json(rows.into_iter().map(Into::into).collect())) +} + +async fn grant_member( + State(s): State, + Extension(principal): Extension, + Path(id_or_slug): Path, + Json(input): Json, +) -> Result<(StatusCode, Json), AppMembersApiError> { + let app = resolve_app(&*s.apps, &id_or_slug).await?; + require(s.authz.as_ref(), &principal, Capability::AppAdmin(app.id)).await?; + + let user = s + .users + .get(input.user_id) + .await? + .ok_or(AppMembersApiError::UserNotFound(input.user_id))?; + validate_grant_target(&user)?; + + if s.members.find(user.id, app.id).await?.is_some() { + return Err(AppMembersApiError::AlreadyMember { + username: user.username, + }); + } + + let row = s.members.upsert(app.id, user.id, input.role).await?; + Ok((StatusCode::CREATED, Json(compose_dto(user, row)))) +} + +async fn patch_member( + State(s): State, + Extension(principal): Extension, + Path((id_or_slug, user_id)): Path<(String, Uuid)>, + Json(input): Json, +) -> Result, AppMembersApiError> { + let app = resolve_app(&*s.apps, &id_or_slug).await?; + require(s.authz.as_ref(), &principal, Capability::AppAdmin(app.id)).await?; + + let user_id = AdminUserId::from(user_id); + let user = s + .users + .get(user_id) + .await? + .ok_or(AppMembersApiError::UserNotFound(user_id))?; + + if s.members.find(user_id, app.id).await?.is_none() { + return Err(AppMembersApiError::MembershipNotFound); + } + + let row = s.members.upsert(app.id, user_id, input.role).await?; + Ok(Json(compose_dto(user, row))) +} + +async fn remove_member( + State(s): State, + Extension(principal): Extension, + Path((id_or_slug, user_id)): Path<(String, Uuid)>, +) -> Result { + let app = resolve_app(&*s.apps, &id_or_slug).await?; + require(s.authz.as_ref(), &principal, Capability::AppAdmin(app.id)).await?; + s.members + .remove(app.id, AdminUserId::from(user_id)) + .await?; + Ok(StatusCode::NO_CONTENT) +} + +// ---------------------------------------------------------------------------- +// Validation + helpers +// ---------------------------------------------------------------------------- + +fn validate_grant_target(user: &AdminUserRow) -> Result<(), AppMembersApiError> { + if !user.is_active { + return Err(AppMembersApiError::TargetInactive { + username: user.username.clone(), + }); + } + if user.instance_role != InstanceRole::Member { + return Err(AppMembersApiError::TargetNotMember { + username: user.username.clone(), + instance_role: user.instance_role, + }); + } + Ok(()) +} + +async fn resolve_app( + apps: &dyn AppRepository, + ident: &str, +) -> Result { + if let Ok(uuid) = ident.parse::() { + if let Some(app) = apps.get_by_id(AppId::from(uuid)).await? { + return Ok(app); + } + return Err(AppMembersApiError::AppNotFound(ident.to_string())); + } + apps.get_by_slug_or_history(ident) + .await? + .map(|l| l.app) + .ok_or_else(|| AppMembersApiError::AppNotFound(ident.to_string())) +} + +// ---------------------------------------------------------------------------- +// Errors +// ---------------------------------------------------------------------------- + +#[derive(Debug, thiserror::Error)] +pub enum AppMembersApiError { + #[error("app not found: {0}")] + AppNotFound(String), + + #[error("user not found: {0}")] + UserNotFound(AdminUserId), + + #[error("no membership exists for this user on this app")] + MembershipNotFound, + + #[error("{username} is already a member of this app — use PATCH to change their role")] + AlreadyMember { username: String }, + + #[error("{username} is deactivated and cannot be added as a member")] + TargetInactive { username: String }, + + #[error( + "{username} has instance_role {instance_role:?} and already has implicit access \ + on every app — no explicit membership needed" + )] + TargetNotMember { + username: String, + instance_role: InstanceRole, + }, + + #[error("forbidden")] + Forbidden, + + #[error("authorization repo error: {0}")] + AuthzRepo(String), + + #[error("repository error: {0}")] + Members(#[from] AppMembersRepositoryError), + + #[error("user repository error: {0}")] + Users(#[from] AdminUserRepositoryError), + + #[error("repository error: {0}")] + Apps(#[from] ScriptRepositoryError), +} + +impl From for AppMembersApiError { + fn from(d: AuthzDenied) -> Self { + match d { + AuthzDenied::Denied => Self::Forbidden, + AuthzDenied::Repo(e) => Self::AuthzRepo(e.to_string()), + } + } +} + +impl IntoResponse for AppMembersApiError { + fn into_response(self) -> Response { + let (status, body) = match &self { + Self::AppNotFound(_) + | Self::UserNotFound(_) + | Self::MembershipNotFound + | Self::Apps(ScriptRepositoryError::NotFound(_)) => { + (StatusCode::NOT_FOUND, json!({ "error": self.to_string() })) + } + Self::AlreadyMember { .. } | Self::Apps(ScriptRepositoryError::Conflict(_)) => { + (StatusCode::CONFLICT, json!({ "error": self.to_string() })) + } + Self::TargetInactive { .. } | Self::TargetNotMember { .. } => ( + StatusCode::UNPROCESSABLE_ENTITY, + json!({ "error": self.to_string() }), + ), + Self::Forbidden => (StatusCode::FORBIDDEN, json!({ "error": self.to_string() })), + Self::AuthzRepo(e) => { + tracing::error!(error = %e, "app members authz repo error"); + ( + StatusCode::INTERNAL_SERVER_ERROR, + json!({ "error": "internal error" }), + ) + } + Self::Members(e) => { + tracing::error!(error = %e, "app members repo error"); + ( + StatusCode::INTERNAL_SERVER_ERROR, + json!({ "error": "internal error" }), + ) + } + Self::Users(e) => { + tracing::error!(error = %e, "admin users repo error"); + ( + StatusCode::INTERNAL_SERVER_ERROR, + json!({ "error": "internal error" }), + ) + } + Self::Apps(ScriptRepositoryError::Db(e)) => { + tracing::error!(error = %e, "apps repo error in app_members"); + ( + StatusCode::INTERNAL_SERVER_ERROR, + json!({ "error": "internal error" }), + ) + } + }; + (status, Json(body)).into_response() + } +} diff --git a/crates/manager-core/src/lib.rs b/crates/manager-core/src/lib.rs index e2e2df5..1d090e7 100644 --- a/crates/manager-core/src/lib.rs +++ b/crates/manager-core/src/lib.rs @@ -12,6 +12,7 @@ pub mod api_key_repo; pub mod api_keys_api; pub mod app_bootstrap; pub mod app_domain_repo; +pub mod app_members_api; pub mod app_members_repo; pub mod app_repo; pub mod apps_api; @@ -45,6 +46,7 @@ pub use api_key_repo::{ pub use api_keys_api::{api_keys_router, ApiKeysState}; pub use app_bootstrap::{seed_hello_world_if_fresh, HelloWorldOutcome}; pub use app_domain_repo::{AppDomainRepository, NewAppDomain, PostgresAppDomainRepository}; +pub use app_members_api::{app_members_router, AppMembersApiError, AppMembersState}; pub use app_members_repo::{ AppMembersRepository, AppMembersRepositoryError, AppMembershipDetail, AppMembershipRow, PostgresAppMembersRepository, diff --git a/crates/picloud/src/lib.rs b/crates/picloud/src/lib.rs index 52f1aab..ef59a73 100644 --- a/crates/picloud/src/lib.rs +++ b/crates/picloud/src/lib.rs @@ -10,14 +10,15 @@ use axum::middleware::from_fn_with_state; use axum::{routing::get, Json, Router}; use picloud_executor_core::{Engine, Limits}; use picloud_manager_core::{ - admin_router, admins_router, api_keys_router, apps_api, apps_router, auth_router, - compile_routes, migrations, require_authenticated, route_admin_router, AdminSessionRepository, - AdminState, AdminUserRepository, AdminsState, ApiKeyRepository, ApiKeysState, - AppDomainRepository, AppRepository, AppsState, AuthState, AuthzRepo, - PostgresAdminSessionRepository, PostgresAdminUserRepository, PostgresApiKeyRepository, - PostgresAppDomainRepository, PostgresAppMembersRepository, PostgresAppRepository, - PostgresExecutionLogRepository, PostgresExecutionLogSink, PostgresRouteRepository, - PostgresScriptRepository, RepoResolver, RouteAdminState, RouteRepository, SandboxCeiling, + admin_router, admins_router, api_keys_router, app_members_router, apps_api, apps_router, + auth_router, compile_routes, migrations, require_authenticated, route_admin_router, + AdminSessionRepository, AdminState, AdminUserRepository, AdminsState, ApiKeyRepository, + ApiKeysState, AppDomainRepository, AppMembersRepository, AppMembersState, AppRepository, + AppsState, AuthState, AuthzRepo, PostgresAdminSessionRepository, PostgresAdminUserRepository, + PostgresApiKeyRepository, PostgresAppDomainRepository, PostgresAppMembersRepository, + PostgresAppRepository, PostgresExecutionLogRepository, PostgresExecutionLogSink, + PostgresRouteRepository, PostgresScriptRepository, RepoResolver, RouteAdminState, + RouteRepository, SandboxCeiling, }; use picloud_orchestrator_core::routing::{AppDomainTable, RouteTable}; use picloud_orchestrator_core::{ @@ -79,6 +80,7 @@ fn read_session_ttl() -> Duration { /// the `require_admin` middleware. The data plane /// (`/api/v1/execute/{id}`, the user-route fallthrough, `/healthz`, /// `/version`) stays open — it's the public ingress for user scripts. +#[allow(clippy::too_many_lines)] pub async fn build_app(pool: PgPool, auth: AuthDeps) -> anyhow::Result { let engine = Arc::new(Engine::new(Limits::default())); @@ -89,9 +91,13 @@ pub async fn build_app(pool: PgPool, auth: AuthDeps) -> anyhow::Result { let apps_repo: Arc = Arc::new(PostgresAppRepository::new(pool.clone())); let domains_repo: Arc = Arc::new(PostgresAppDomainRepository::new(pool.clone())); - // Authz: app_members repo doubles as the AuthzRepo impl for the - // per-handler capability checks introduced in Phase 3.5. - let authz: Arc = Arc::new(PostgresAppMembersRepository::new(pool)); + // The Postgres app_members repo implements both `AppMembersRepository` + // (CRUD over the table) and `AuthzRepo` (single-row membership lookup + // for capability checks). Construct it once and clone the Arc into + // both trait views — same allocation, two vtables. + let members_concrete = Arc::new(PostgresAppMembersRepository::new(pool)); + let members: Arc = members_concrete.clone(); + let authz: Arc = members_concrete; // Compile the routes table once at startup; admin writes refresh it. let route_table = Arc::new(RouteTable::new()); @@ -159,9 +165,15 @@ pub async fn build_app(pool: PgPool, auth: AuthDeps) -> anyhow::Result { ttl: auth.ttl, }; let admins_state = AdminsState { - users: auth.users, + users: auth.users.clone(), sessions: auth.sessions, keys: auth.keys.clone(), + authz: authz.clone(), + }; + let app_members_state = AppMembersState { + apps: apps_state.apps.clone(), + users: auth.users, + members, authz, }; let api_keys_state = ApiKeysState { keys: auth.keys }; @@ -177,6 +189,7 @@ pub async fn build_app(pool: PgPool, auth: AuthDeps) -> anyhow::Result { .merge(route_admin_router(route_admin)) .merge(admins_router(admins_state)) .merge(apps_router(apps_state)) + .merge(app_members_router(app_members_state)) .merge(api_keys_router(api_keys_state)) .layer(from_fn_with_state( auth_state.clone(),