From 3fe8f5e7e41d68cb6b2dac23f0a960bd636a2263 Mon Sep 17 00:00:00 2001 From: Rune Darrud Date: Sun, 12 Jul 2026 15:13:58 +0300 Subject: [PATCH 1/7] Add Public API read endpoints for members, groups, and collections The public organization API previously exposed only the write-side "/public/organization/import" endpoint. This adds the corresponding read endpoints so an organization-scoped API client can read back the members, groups, and collections it manages, along with their access associations: - GET /public/members and /public/members/ (with collection grants) - GET /public/members//group-ids - GET /public/groups and /public/groups/ (with collection grants) - GET /public/groups//member-ids - GET /public/collections and /public/collections/ (with group grants) All handlers reuse the existing PublicToken guard, so they are authorized by the same organization API key as the import endpoint, and every handler is scoped to the token's organization: a resource id that belongs to another organization returns 404 rather than leaking data. Collection responses intentionally omit the end-to-end encrypted name and key collections by id and externalId only. Co-Authored-By: Claude Opus 4.8 --- src/api/core/public.rs | 227 ++++++++++++++++++++++++++++++++++++++++- 1 file changed, 223 insertions(+), 4 deletions(-) diff --git a/src/api/core/public.rs b/src/api/core/public.rs index 3db25df9..7d9bd05a 100644 --- a/src/api/core/public.rs +++ b/src/api/core/public.rs @@ -6,23 +6,35 @@ use rocket::{ request::{FromRequest, Outcome}, serde::json::Json, }; +use serde_json::Value; use crate::{ CONFIG, - api::EmptyResult, + api::{EmptyResult, JsonResult}, auth, db::{ DbConn, models::{ - Group, GroupUser, Invitation, Membership, MembershipStatus, MembershipType, OrgPolicy, Organization, - OrganizationApiKey, OrganizationId, User, + Collection, CollectionGroup, CollectionId, CollectionUser, Group, GroupId, GroupUser, Invitation, + Membership, MembershipId, MembershipStatus, MembershipType, OrgPolicy, Organization, OrganizationApiKey, + OrganizationId, User, }, }, mail, }; pub fn routes() -> Vec { - routes![ldap_import] + routes![ + ldap_import, + get_members, + get_member, + get_member_group_ids, + get_groups, + get_group, + get_group_member_ids, + get_collections, + get_collection, + ] } #[derive(Deserialize)] @@ -196,6 +208,213 @@ async fn ldap_import(data: Json, token: PublicToken, conn: DbConn Ok(()) } +// These endpoints implement the read side of the organization Public API so an +// organization-scoped API client can read back the members, groups, and +// collections (and their associations) that the existing +// "/public/organization/import" endpoint writes. They all reuse the PublicToken +// guard, so they are authorized by the same organization API key. + +// Base member object. The list endpoint returns this as-is; the single-member +// endpoint extends it with "collections". +async fn member_to_json(member: &Membership, conn: &DbConn) -> Value { + let (name, email) = match User::find_by_uuid(&member.user_uuid, conn).await { + Some(user) => { + let name = if user.name.is_empty() { + Value::Null + } else { + Value::String(user.name) + }; + (name, Value::String(user.email)) + } + None => (Value::Null, Value::Null), + }; + + json!({ + "object": "member", + "id": member.uuid, + "userId": member.user_uuid, + "name": name, + "email": email, + "type": member.atype, + "externalId": member.external_id, + "resetPasswordEnrolled": member.reset_password_key.is_some(), + "status": member.status, + }) +} + +// Base group object. The list endpoint returns this as-is; the single-group +// endpoint extends it with "collections". +fn group_to_json(group: &Group) -> Value { + json!({ + "object": "group", + "id": group.uuid, + "name": group.name, + "accessAll": group.access_all, + "externalId": group.external_id, + }) +} + +// Base collection object. The Bitwarden Public API keys collections by id and +// externalId only; the name is deliberately omitted because it is end-to-end +// encrypted ciphertext. The single-collection endpoint extends it with "groups". +fn collection_to_json(collection: &Collection) -> Value { + json!({ + "object": "collection", + "id": collection.uuid, + "externalId": collection.external_id, + }) +} + +#[get("/public/members")] +async fn get_members(token: PublicToken, conn: DbConn) -> JsonResult { + let org_id = token.0; + let mut members_json = Vec::new(); + for member in Membership::find_by_org(&org_id, &conn).await { + members_json.push(member_to_json(&member, &conn).await); + } + + Ok(Json(json!({ + "object": "list", + "data": members_json, + "continuationToken": null, + }))) +} + +#[get("/public/members/")] +async fn get_member(member_id: MembershipId, token: PublicToken, conn: DbConn) -> JsonResult { + let org_id = token.0; + let Some(member) = Membership::find_by_uuid_and_org(&member_id, &org_id, &conn).await else { + err_code!(format!("Member {member_id} not found in organization"), 404); + }; + + let collections: Vec = CollectionUser::find_by_organization_and_user_uuid(&org_id, &member.user_uuid, &conn) + .await + .iter() + .map(|c| { + json!({ + "id": c.collection_uuid, + "readOnly": c.read_only, + "hidePasswords": c.hide_passwords, + "manage": c.manage, + }) + }) + .collect(); + + let mut member_json = member_to_json(&member, &conn).await; + member_json["collections"] = json!(collections); + + Ok(Json(member_json)) +} + +#[get("/public/members//group-ids")] +async fn get_member_group_ids(member_id: MembershipId, token: PublicToken, conn: DbConn) -> JsonResult { + let org_id = token.0; + if Membership::find_by_uuid_and_org(&member_id, &org_id, &conn).await.is_none() { + err_code!(format!("Member {member_id} not found in organization"), 404); + } + + // GroupUser links a group to a membership, so a member's group ids are the + // group uuids of the GroupUser rows referencing this membership. + let group_ids: Vec = + GroupUser::find_by_member(&member_id, &conn).await.into_iter().map(|gu| gu.groups_uuid).collect(); + + Ok(Json(json!(group_ids))) +} + +#[get("/public/groups")] +async fn get_groups(token: PublicToken, conn: DbConn) -> JsonResult { + let org_id = token.0; + let groups_json: Vec = Group::find_by_organization(&org_id, &conn).await.iter().map(group_to_json).collect(); + + Ok(Json(json!({ + "object": "list", + "data": groups_json, + "continuationToken": null, + }))) +} + +#[get("/public/groups/")] +async fn get_group(group_id: GroupId, token: PublicToken, conn: DbConn) -> JsonResult { + let org_id = token.0; + let Some(group) = Group::find_by_uuid_and_org(&group_id, &org_id, &conn).await else { + err_code!(format!("Group {group_id} not found in organization"), 404); + }; + + let collections: Vec = CollectionGroup::find_by_group(&group_id, &org_id, &conn) + .await + .iter() + .map(|c| { + json!({ + "id": c.collections_uuid, + "readOnly": c.read_only, + "hidePasswords": c.hide_passwords, + "manage": c.manage, + }) + }) + .collect(); + + let mut group_json = group_to_json(&group); + group_json["collections"] = json!(collections); + + Ok(Json(group_json)) +} + +#[get("/public/groups//member-ids")] +async fn get_group_member_ids(group_id: GroupId, token: PublicToken, conn: DbConn) -> JsonResult { + let org_id = token.0; + if Group::find_by_uuid_and_org(&group_id, &org_id, &conn).await.is_none() { + err_code!(format!("Group {group_id} not found in organization"), 404); + } + + // A group's member ids are the membership uuids of its GroupUser rows. + let member_ids: Vec = GroupUser::find_by_group(&group_id, &org_id, &conn) + .await + .into_iter() + .map(|gu| gu.users_organizations_uuid) + .collect(); + + Ok(Json(json!(member_ids))) +} + +#[get("/public/collections")] +async fn get_collections(token: PublicToken, conn: DbConn) -> JsonResult { + let org_id = token.0; + let collections_json: Vec = + Collection::find_by_organization(&org_id, &conn).await.iter().map(collection_to_json).collect(); + + Ok(Json(json!({ + "object": "list", + "data": collections_json, + "continuationToken": null, + }))) +} + +#[get("/public/collections/")] +async fn get_collection(collection_id: CollectionId, token: PublicToken, conn: DbConn) -> JsonResult { + let org_id = token.0; + let Some(collection) = Collection::find_by_uuid_and_org(&collection_id, &org_id, &conn).await else { + err_code!(format!("Collection {collection_id} not found in organization"), 404); + }; + + let groups: Vec = CollectionGroup::find_by_collection(&collection_id, &conn) + .await + .iter() + .map(|c| { + json!({ + "id": c.groups_uuid, + "readOnly": c.read_only, + "hidePasswords": c.hide_passwords, + "manage": c.manage, + }) + }) + .collect(); + + let mut collection_json = collection_to_json(&collection); + collection_json["groups"] = json!(groups); + + Ok(Json(collection_json)) +} + pub struct PublicToken(OrganizationId); #[rocket::async_trait] From 96b46c9ae9aa8f7eee206b6fddace56cf3aa4f93 Mon Sep 17 00:00:00 2001 From: Rune Darrud Date: Sun, 12 Jul 2026 15:27:16 +0300 Subject: [PATCH 2/7] Add smoke test for the organization Public API read endpoints Boots a throwaway instance against a temporary SQLite database seeded with two organizations (each with members, groups, collections and their access associations), mints an organization API token, and asserts on every read endpoint: member/group/collection lists and details, the direct member-to-collection grant on member detail, the group-to-collection grant on group and collection detail, and that collection responses omit the encrypted name. It also asserts the organization scoping boundary (ids owned by the second organization return 404 through the first org's token) and that an unauthenticated request returns 401. The script exits non-zero if any assertion fails, so it can be run as a check. It builds the binary when one is not supplied via VW_BIN, uses a throwaway port and temp directory, and cleans up on exit. Co-Authored-By: Claude Opus 4.8 --- scripts/smoke_public_api.sh | 310 ++++++++++++++++++++++++++++++++++++ 1 file changed, 310 insertions(+) create mode 100755 scripts/smoke_public_api.sh diff --git a/scripts/smoke_public_api.sh b/scripts/smoke_public_api.sh new file mode 100755 index 00000000..4f541a25 --- /dev/null +++ b/scripts/smoke_public_api.sh @@ -0,0 +1,310 @@ +#!/usr/bin/env bash +# +# Smoke test for the organization Public API read endpoints. +# +# Boots a throwaway Vaultwarden instance against a temporary SQLite database +# seeded with two organizations (each with its own members, groups, collections +# and access associations), mints an organization API token for the first org, +# then exercises every read endpoint and asserts on the response shapes. +# +# It also asserts the organization scoping boundary: ids that belong to the +# second organization must return HTTP 404 through the first org's token, and a +# request with no token must return HTTP 401. +# +# The script exits non-zero if any assertion fails, so it is usable as a check. +# +# Requirements: bash, curl, jq, sqlite3, and either a prebuilt binary passed via +# the VW_BIN environment variable or a cargo toolchain to build one. +# +# Usage: +# scripts/smoke_public_api.sh +# VW_BIN=/path/to/vaultwarden PORT=8123 scripts/smoke_public_api.sh + +set -euo pipefail + +SCRIPT_DIR=$(cd "$(dirname "$0")" && pwd) +REPO_ROOT=$(cd "$SCRIPT_DIR/.." && pwd) +cd "$REPO_ROOT" + +PORT="${PORT:-8079}" +VW_BIN="${VW_BIN:-$REPO_ROOT/target/debug/vaultwarden}" +API="http://127.0.0.1:$PORT" + +# ---- fixtures ------------------------------------------------------------- +ORG=22222222-2222-4222-8222-222222222222 +ORG2=99999999-9999-4999-8999-999999999999 +USER=11111111-1111-4111-8111-111111111111 +USER2=88888888-8888-4888-8888-888888888888 +MEMBER=33333333-3333-4333-8333-333333333333 +MEMBER2=aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa +GROUP=44444444-4444-4444-8444-444444444444 +GROUP2=bbbbbbbb-bbbb-4bbb-8bbb-bbbbbbbbbbbb +COLLECTION=55555555-5555-4555-8555-555555555555 +COLLECTION2=66666666-6666-4666-8666-666666666666 +APIKEYUUID=77777777-7777-4777-8777-777777777777 +APIKEY=smoketestapikey1234567890 + +# ---- prerequisites -------------------------------------------------------- +for tool in curl jq sqlite3; do + if ! command -v "$tool" >/dev/null 2>&1; then + echo "ERROR: required tool '$tool' is not installed" >&2 + exit 2 + fi +done + +if [ ! -x "$VW_BIN" ]; then + if command -v cargo >/dev/null 2>&1; then + echo "Building vaultwarden (sqlite feature); this can take a while..." + cargo build --features sqlite + else + echo "ERROR: no binary at '$VW_BIN' and no cargo toolchain to build one." >&2 + echo "Set VW_BIN to a prebuilt binary or install a Rust toolchain." >&2 + exit 2 + fi +fi + +# ---- workspace + cleanup -------------------------------------------------- +TMP=$(mktemp -d) +SERVER_PID="" +cleanup() { + if [ -n "$SERVER_PID" ]; then + kill "$SERVER_PID" >/dev/null 2>&1 || true + wait "$SERVER_PID" 2>/dev/null || true + fi + rm -rf "$TMP" +} +trap cleanup EXIT + +export DATA_FOLDER="$TMP" +export DATABASE_URL="sqlite://$TMP/db.sqlite3" +export ADMIN_TOKEN="smoketestadmintoken" +export ORG_GROUPS_ENABLED=true +export WEB_VAULT_ENABLED=false +export ROCKET_PORT="$PORT" +export ROCKET_ADDRESS=127.0.0.1 +export DOMAIN="http://localhost:$PORT" + +# ---- server helpers ------------------------------------------------------- +start_server() { + local logfile="$1" + "$VW_BIN" >"$logfile" 2>&1 & + SERVER_PID=$! + local i + for i in $(seq 1 90); do + if grep -q "Rocket has launched" "$logfile" 2>/dev/null; then + return 0 + fi + if ! kill -0 "$SERVER_PID" 2>/dev/null; then + echo "ERROR: server exited during startup. Log:" >&2 + cat "$logfile" >&2 + return 1 + fi + sleep 1 + done + echo "ERROR: server did not launch within 90s. Log:" >&2 + cat "$logfile" >&2 + return 1 +} + +stop_server() { + if [ -n "$SERVER_PID" ]; then + kill "$SERVER_PID" >/dev/null 2>&1 || true + wait "$SERVER_PID" 2>/dev/null || true + SERVER_PID="" + fi +} + +# ---- assertion helpers ---------------------------------------------------- +FAILS=0 +pass() { printf 'PASS: %s\n' "$1"; } +fail() { printf 'FAIL: %s\n' "$1"; FAILS=$((FAILS + 1)); } + +check_eq() { # label actual expected + if [ "$2" = "$3" ]; then + pass "$1" + else + fail "$1 (expected [$3], got [$2])" + fi +} + +# req METHOD PATH [TOKEN] -> sets HTTP_CODE, body written to $TMP/body +req() { + local method="$1" path="$2" token="${3:-}" + if [ -n "$token" ]; then + HTTP_CODE=$(curl -sS -o "$TMP/body" -w '%{http_code}' \ + -X "$method" -H "Authorization: Bearer $token" "$API$path") + else + HTTP_CODE=$(curl -sS -o "$TMP/body" -w '%{http_code}' -X "$method" "$API$path") + fi +} + +jqval() { jq -r "$1" "$TMP/body"; } + +jqcheck() { # label filter expected + check_eq "$1" "$(jqval "$2")" "$3" +} + +# ---- boot once to run migrations, then seed, then boot to serve ----------- +echo "== Booting once to create the database schema ==" +start_server "$TMP/boot1.log" +stop_server + +echo "== Seeding two organizations with members, groups and collections ==" +sqlite3 "$TMP/db.sqlite3" <&2 + exit 1 +fi +pass "minted organization API token" + +echo "" +echo "== Read endpoints ==" + +# 1. Members list (without collections/groups). +req GET "/api/public/members" "$TOKEN" +check_eq "members list -> 200" "$HTTP_CODE" "200" +jqcheck "members list is a list object" '.object' "list" +jqcheck "members list has continuationToken null" '.continuationToken' "null" +jqcheck "members list has one member" '.data | length' "1" +jqcheck "member object discriminator" '.data[0].object' "member" +jqcheck "member id" '.data[0].id' "$MEMBER" +jqcheck "member userId" '.data[0].userId' "$USER" +jqcheck "member email" '.data[0].email' "alice@example.com" +jqcheck "member name" '.data[0].name' "Alice Example" +jqcheck "member type" '.data[0].type' "2" +jqcheck "member status" '.data[0].status' "2" +jqcheck "member externalId" '.data[0].externalId' "ext-member-1" +jqcheck "member resetPasswordEnrolled" '.data[0].resetPasswordEnrolled' "false" +jqcheck "members list omits collections" '.data[0] | has("collections")' "false" + +# 2. Member detail (with direct collection grants). +req GET "/api/public/members/$MEMBER" "$TOKEN" +check_eq "member detail -> 200" "$HTTP_CODE" "200" +jqcheck "member detail id" '.id' "$MEMBER" +jqcheck "member detail carries one collection grant" '.collections | length' "1" +jqcheck "member detail collection id" '.collections[0].id' "$COLLECTION" +jqcheck "member detail collection readOnly" '.collections[0].readOnly' "true" +jqcheck "member detail collection hidePasswords" '.collections[0].hidePasswords' "false" +jqcheck "member detail collection manage" '.collections[0].manage' "false" + +# 3. Member group ids. +req GET "/api/public/members/$MEMBER/group-ids" "$TOKEN" +check_eq "member group-ids -> 200" "$HTTP_CODE" "200" +jqcheck "member group-ids is a bare array of one" 'length' "1" +jqcheck "member group-ids contains the group" '.[0]' "$GROUP" + +# 4. Groups list. +req GET "/api/public/groups" "$TOKEN" +check_eq "groups list -> 200" "$HTTP_CODE" "200" +jqcheck "groups list is a list object" '.object' "list" +jqcheck "groups list has one group" '.data | length' "1" +jqcheck "group object discriminator" '.data[0].object' "group" +jqcheck "group id" '.data[0].id' "$GROUP" +jqcheck "group name is plaintext" '.data[0].name' "Engineering" +jqcheck "group accessAll" '.data[0].accessAll' "false" +jqcheck "group externalId" '.data[0].externalId' "ext-group-1" + +# 5. Group detail (with collection grants). +req GET "/api/public/groups/$GROUP" "$TOKEN" +check_eq "group detail -> 200" "$HTTP_CODE" "200" +jqcheck "group detail id" '.id' "$GROUP" +jqcheck "group detail carries one collection grant" '.collections | length' "1" +jqcheck "group detail collection id" '.collections[0].id' "$COLLECTION" +jqcheck "group detail collection readOnly" '.collections[0].readOnly' "false" +jqcheck "group detail collection manage" '.collections[0].manage' "true" + +# 6. Group member ids. +req GET "/api/public/groups/$GROUP/member-ids" "$TOKEN" +check_eq "group member-ids -> 200" "$HTTP_CODE" "200" +jqcheck "group member-ids is a bare array of one" 'length' "1" +jqcheck "group member-ids contains the membership" '.[0]' "$MEMBER" + +# 7. Collections list (id + externalId only, no name). +req GET "/api/public/collections" "$TOKEN" +check_eq "collections list -> 200" "$HTTP_CODE" "200" +jqcheck "collections list is a list object" '.object' "list" +jqcheck "collections list has one collection" '.data | length' "1" +jqcheck "collection object discriminator" '.data[0].object' "collection" +jqcheck "collection id" '.data[0].id' "$COLLECTION" +jqcheck "collection externalId" '.data[0].externalId' "ext-collection-1" +jqcheck "collection omits the encrypted name" '.data[0] | has("name")' "false" + +# 8. Collection detail (with group grants). +req GET "/api/public/collections/$COLLECTION" "$TOKEN" +check_eq "collection detail -> 200" "$HTTP_CODE" "200" +jqcheck "collection detail id" '.id' "$COLLECTION" +jqcheck "collection detail externalId" '.externalId' "ext-collection-1" +jqcheck "collection detail omits the encrypted name" 'has("name")' "false" +jqcheck "collection detail carries one group grant" '.groups | length' "1" +jqcheck "collection detail group id" '.groups[0].id' "$GROUP" +jqcheck "collection detail group manage" '.groups[0].manage' "true" + +echo "" +echo "== Organization scoping boundary ==" + +# Ids that exist but belong to the other organization must not resolve. +req GET "/api/public/members/$MEMBER2" "$TOKEN" +check_eq "cross-org member -> 404" "$HTTP_CODE" "404" +req GET "/api/public/groups/$GROUP2" "$TOKEN" +check_eq "cross-org group -> 404" "$HTTP_CODE" "404" +req GET "/api/public/collections/$COLLECTION2" "$TOKEN" +check_eq "cross-org collection -> 404" "$HTTP_CODE" "404" + +echo "" +echo "== Authentication required ==" +req GET "/api/public/members" +check_eq "no token -> 401" "$HTTP_CODE" "401" + +echo "" +if [ "$FAILS" -ne 0 ]; then + echo "RESULT: $FAILS assertion(s) failed." + exit 1 +fi +echo "RESULT: all assertions passed." From 0f8d565258eaf7b9ffc5c9e64c895c6af88656f2 Mon Sep 17 00:00:00 2001 From: Rune Darrud Date: Mon, 10 Aug 2026 00:37:04 +0200 Subject: [PATCH 3/7] Add Public API member and group write endpoints Adds the write side of the organization Public API for members and groups, so an organization API client can make incremental changes instead of pushing a full directory snapshot through /public/organization/import, which revokes any member missing from the payload when overwriteExisting is set. Members: create (invite), update, delete, replace group ids, reinvite, revoke and restore. Groups: create, update, delete and replace member ids. Routes and request models follow the upstream Bitwarden Public API controllers. The handlers mirror the equivalent internal endpoints in organizations.rs but are guarded by PublicToken instead of AdminHeaders. A PublicToken carries only an organization, so the per-actor permission checks do not apply, while the guards protecting organization integrity are kept: the last confirmed owner cannot be demoted, revoked or deleted, org policies are still enforced on member changes, and group endpoints still require group support to be enabled. Public API writes have no acting user or device, so log_event_impl now takes both as optional and a new log_public_event records these events with a null actingUserId rather than leaving them out of the organization event log. Co-Authored-By: Claude Opus 5 --- src/api/core/events.rs | 28 +- src/api/core/public.rs | 603 ++++++++++++++++++++++++++++++++++++++++- 2 files changed, 617 insertions(+), 14 deletions(-) diff --git a/src/api/core/events.rs b/src/api/core/events.rs index 5518fa3c..d2b19d13 100644 --- a/src/api/core/events.rs +++ b/src/api/core/events.rs @@ -190,8 +190,8 @@ async fn post_events_collect(data: Json>, headers: Headers, event.r#type, org_id, org_id, - &headers.user.uuid, - headers.device.atype, + Some(&headers.user.uuid), + Some(headers.device.atype), Some(event_date), &headers.ip.ip, &conn, @@ -211,8 +211,8 @@ async fn post_events_collect(data: Json>, headers: Headers, event.r#type, cipher_uuid, &org_id, - &headers.user.uuid, - headers.device.atype, + Some(&headers.user.uuid), + Some(headers.device.atype), Some(event_date), &headers.ip.ip, &conn, @@ -278,7 +278,17 @@ pub async fn log_event( if !CONFIG.org_events_enabled() { return; } - log_event_impl(event_type, source_uuid, org_id, act_user_id, device_type, None, ip, conn).await; + log_event_impl(event_type, source_uuid, org_id, Some(act_user_id), Some(device_type), None, ip, conn).await; +} + +/// Log an organization event that was triggered through the Public API. +/// These are performed by an organization API client instead of a user, so there is +/// no acting user or device to record and both are stored as null. +pub async fn log_public_event(event_type: i32, source_uuid: &str, org_id: &OrganizationId, ip: &IpAddr, conn: &DbConn) { + if !CONFIG.org_events_enabled() { + return; + } + log_event_impl(event_type, source_uuid, org_id, None, None, None, ip, conn).await; } #[expect(clippy::too_many_arguments)] @@ -286,8 +296,8 @@ async fn log_event_impl( event_type: i32, source_uuid: &str, org_id: &OrganizationId, - act_user_id: &UserId, - device_type: i32, + act_user_id: Option<&UserId>, + device_type: Option, event_date: Option, ip: &IpAddr, conn: &DbConn, @@ -322,8 +332,8 @@ async fn log_event_impl( } event.org_uuid = Some(org_id.clone()); - event.act_user_uuid = Some(act_user_id.clone()); - event.device_type = Some(device_type); + event.act_user_uuid = act_user_id.cloned(); + event.device_type = device_type; event.ip_address = Some(ip.to_string()); event.save(conn).await.unwrap_or(()); } diff --git a/src/api/core/public.rs b/src/api/core/public.rs index 7d9bd05a..b6f091f1 100644 --- a/src/api/core/public.rs +++ b/src/api/core/public.rs @@ -1,4 +1,4 @@ -use std::collections::HashSet; +use std::collections::{HashMap, HashSet}; use chrono::Utc; use rocket::{ @@ -10,19 +10,22 @@ use serde_json::Value; use crate::{ CONFIG, - api::{EmptyResult, JsonResult}, + api::{EmptyResult, JsonResult, Notify, UpdateType}, auth, db::{ DbConn, models::{ - Collection, CollectionGroup, CollectionId, CollectionUser, Group, GroupId, GroupUser, Invitation, - Membership, MembershipId, MembershipStatus, MembershipType, OrgPolicy, Organization, OrganizationApiKey, - OrganizationId, User, + Collection, CollectionGroup, CollectionId, CollectionUser, EventType, Group, GroupId, GroupUser, + Invitation, Membership, MembershipId, MembershipStatus, MembershipType, OrgPolicy, Organization, + OrganizationApiKey, OrganizationId, User, }, }, mail, + util::NumberOrString, }; +use super::events::log_public_event; + pub fn routes() -> Vec { routes![ ldap_import, @@ -34,6 +37,17 @@ pub fn routes() -> Vec { get_group_member_ids, get_collections, get_collection, + post_member, + put_member, + delete_member, + put_member_group_ids, + post_member_reinvite, + post_member_revoke, + post_member_restore, + post_group, + put_group, + delete_group, + put_group_member_ids, ] } @@ -415,6 +429,585 @@ async fn get_collection(collection_id: CollectionId, token: PublicToken, conn: D Ok(Json(collection_json)) } +// These endpoints implement the write side of the organization Public API. Together +// with the read endpoints above they replace the need to drive every change through +// "/public/organization/import", which is a full directory snapshot and revokes any +// member missing from the payload when overwriteExisting is set. +// +// The write paths mirror the equivalent internal endpoints in `organizations.rs`, but +// are guarded by PublicToken instead of AdminHeaders. A PublicToken carries only an +// organization, so the per-actor permission checks of the internal API do not apply; +// the guards that protect organization integrity (last confirmed owner, org policies, +// group support being enabled) are kept. + +// Bitwarden models an assignment to a collection as its id plus the permissions granted. +// Upstream: https://github.com/bitwarden/server/blob/9ebe16587175b1c0e9208f84397bb75d0d595510/src/Api/AdminConsole/Public/Models/AssociationWithPermissionsBaseModel.cs +#[derive(Deserialize)] +#[serde(rename_all = "camelCase")] +struct AssociationData { + id: CollectionId, + #[serde(default)] + read_only: bool, + #[serde(default)] + hide_passwords: bool, + #[serde(default)] + manage: bool, +} + +#[derive(Deserialize)] +#[serde(rename_all = "camelCase")] +struct MemberCreateData { + email: String, + r#type: NumberOrString, + external_id: Option, + #[serde(default)] + collections: Vec, + #[serde(default)] + groups: Vec, + #[serde(default)] + permissions: HashMap, +} + +#[derive(Deserialize)] +#[serde(rename_all = "camelCase")] +struct MemberUpdateData { + r#type: NumberOrString, + external_id: Option, + #[serde(default)] + collections: Vec, + #[serde(default)] + groups: Vec, + #[serde(default)] + permissions: HashMap, +} + +#[derive(Deserialize)] +#[serde(rename_all = "camelCase")] +struct GroupCreateUpdateData { + name: String, + // Upstream dropped accessAll from its group model, but the Vaultwarden group still + // carries the flag, so it is accepted here and defaults to false when omitted. + #[serde(default)] + access_all: bool, + external_id: Option, + #[serde(default)] + collections: Vec, +} + +#[derive(Deserialize)] +#[serde(rename_all = "camelCase")] +struct GroupIdsData { + group_ids: Vec, +} + +#[derive(Deserialize)] +#[serde(rename_all = "camelCase")] +struct MemberIdsData { + member_ids: Vec, +} + +// HACK: We need the raw user-type to be sure custom role is selected to determine the access_all permission +// The from_str() will convert the custom role type into a manager role type +fn member_type_and_access_all( + r#type: NumberOrString, + permissions: &HashMap, +) -> Option<(MembershipType, bool)> { + let raw_type = &r#type.into_string(); + // MembershipType::from_str will convert custom (4) to manager (3) + let new_type = MembershipType::from_str(raw_type)?; + + // HACK: This converts the Custom role which has the `Manage all collections` box checked into an access_all flag + // Since the parent checkbox is not sent to the server we need to check and verify the child checkboxes + // If the box is not checked, the user will still be a manager, but not with the access_all permission + let access_all = new_type >= MembershipType::Admin + || (raw_type.eq("4") + && permissions.get("editAnyCollection") == Some(&json!(true)) + && permissions.get("deleteAnyCollection") == Some(&json!(true)) + && permissions.get("createNewCollections") == Some(&json!(true))); + + Some((new_type, access_all)) +} + +async fn validate_collections(collections: &[AssociationData], org_id: &OrganizationId, conn: &DbConn) -> EmptyResult { + let org_collections = Collection::find_by_organization(org_id, conn).await; + let org_collection_ids: HashSet<&CollectionId> = org_collections.iter().map(|c| &c.uuid).collect(); + if let Some(e) = collections.iter().find(|c| !org_collection_ids.contains(&c.id)) { + err!("Invalid collection", format!("Collection {} does not belong to organization {}!", e.id, org_id)) + } + Ok(()) +} + +async fn validate_groups(group_ids: &[GroupId], org_id: &OrganizationId, conn: &DbConn) -> EmptyResult { + let org_groups = Group::find_by_organization(org_id, conn).await; + let org_group_ids: HashSet<&GroupId> = org_groups.iter().map(|g| &g.uuid).collect(); + if let Some(e) = group_ids.iter().find(|g| !org_group_ids.contains(g)) { + err!("Invalid group", format!("Group {} does not belong to organization {}!", e, org_id)) + } + Ok(()) +} + +async fn validate_members(member_ids: &[MembershipId], org_id: &OrganizationId, conn: &DbConn) -> EmptyResult { + let org_memberships = Membership::find_by_org(org_id, conn).await; + let org_membership_ids: HashSet<&MembershipId> = org_memberships.iter().map(|m| &m.uuid).collect(); + if let Some(e) = member_ids.iter().find(|m| !org_membership_ids.contains(m)) { + err!("Invalid member", format!("Member {} does not belong to organization {}!", e, org_id)) + } + Ok(()) +} + +// Replace a member's collection assignments. Members of type Admin or Owner reach every +// collection through their type, so no explicit assignments are stored for them. +async fn set_member_collections( + member: &Membership, + collections: &[AssociationData], + org_id: &OrganizationId, + conn: &DbConn, +) -> EmptyResult { + for c in CollectionUser::find_by_organization_and_user_uuid(org_id, &member.user_uuid, conn).await { + c.delete(conn).await?; + } + + if !member.access_all { + for col in collections { + CollectionUser::save(&member.user_uuid, &col.id, col.read_only, col.hide_passwords, col.manage, conn) + .await?; + } + } + + Ok(()) +} + +async fn set_member_groups(member: &Membership, group_ids: &[GroupId], conn: &DbConn) -> EmptyResult { + GroupUser::delete_all_by_member(&member.uuid, conn).await?; + for group_id in group_ids { + let mut group_entry = GroupUser::new(group_id.clone(), member.uuid.clone()); + group_entry.save(conn).await?; + } + + Ok(()) +} + +async fn set_group_collections( + group: &Group, + collections: &[AssociationData], + org_id: &OrganizationId, + conn: &DbConn, +) -> EmptyResult { + CollectionGroup::delete_all_by_group(&group.uuid, org_id, conn).await?; + for col in collections { + let mut collection_group = + CollectionGroup::new(col.id.clone(), group.uuid.clone(), col.read_only, col.hide_passwords, col.manage); + collection_group.save(org_id, conn).await?; + } + + Ok(()) +} + +// A Public API client is not a member of the organization, so invites are recorded as +// coming from the organization itself, the same way "/public/organization/import" does. +async fn org_name_and_email(org_id: &OrganizationId, conn: &DbConn) -> Result<(String, String), crate::error::Error> { + let Some(org) = Organization::find_by_uuid(org_id, conn).await else { + err!("Error looking up organization") + }; + + Ok((org.name, org.billing_email)) +} + +#[post("/public/members", data = "")] +async fn post_member(data: Json, token: PublicToken, ip: auth::ClientIp, conn: DbConn) -> JsonResult { + let org_id = token.0; + let data = data.into_inner(); + + let Some((new_type, access_all)) = member_type_and_access_all(data.r#type, &data.permissions) else { + err!("Invalid type") + }; + + validate_collections(&data.collections, &org_id, &conn).await?; + validate_groups(&data.groups, &org_id, &conn).await?; + + let mut user_created = false; + let mut member_status = MembershipStatus::Invited as i32; + let user = match User::find_by_mail(&data.email, &conn).await { + None => { + if !CONFIG.invitations_allowed() { + err!(format!("User does not exist: {}", data.email)) + } + + if !CONFIG.is_email_domain_allowed(&data.email) { + err!("Email domain not eligible for invitations") + } + + if !CONFIG.mail_enabled() { + Invitation::new(&data.email).save(&conn).await?; + } + + let mut new_user = User::new(&data.email, None); + new_user.save(&conn).await?; + user_created = true; + new_user + } + Some(user) => { + if Membership::find_by_user_and_org(&user.uuid, &org_id, &conn).await.is_some() { + err!(format!("User already in organization: {}", data.email)) + } + + if !CONFIG.mail_enabled() { + if user.password_hash.is_empty() { + Invitation::new(&data.email).save(&conn).await?; + } else { + // automatically accept existing users if mail is disabled + member_status = MembershipStatus::Accepted as i32; + } + } + user + } + }; + + let (org_name, org_email) = org_name_and_email(&org_id, &conn).await?; + + let mut new_member = Membership::new(user.uuid.clone(), org_id.clone(), Some(org_email.clone())); + new_member.access_all = access_all; + new_member.atype = new_type as i32; + new_member.status = member_status; + new_member.set_external_id(data.external_id.clone()); + new_member.save(&conn).await?; + + if CONFIG.mail_enabled() + && let Err(e) = + mail::send_invite(&user, org_id.clone(), new_member.uuid.clone(), &org_name, Some(org_email)).await + { + // Upon error delete the user, invite and org member records when needed + if user_created { + user.delete(&conn).await?; + } else { + new_member.delete(&conn).await?; + } + + err!(format!("Error sending invite: {e:?} ")); + } + + log_public_event(EventType::OrganizationUserInvited as i32, &new_member.uuid, &org_id, &ip.ip, &conn).await; + + set_member_collections(&new_member, &data.collections, &org_id, &conn).await?; + set_member_groups(&new_member, &data.groups, &conn).await?; + + Ok(Json(member_to_json(&new_member, &conn).await)) +} + +#[put("/public/members/", data = "")] +async fn put_member( + member_id: MembershipId, + data: Json, + token: PublicToken, + ip: auth::ClientIp, + conn: DbConn, +) -> JsonResult { + let org_id = token.0; + let data = data.into_inner(); + + let Some((new_type, access_all)) = member_type_and_access_all(data.r#type, &data.permissions) else { + err!("Invalid type") + }; + + let Some(mut member) = Membership::find_by_uuid_and_org(&member_id, &org_id, &conn).await else { + err_code!(format!("Member {member_id} not found in organization"), 404); + }; + + validate_collections(&data.collections, &org_id, &conn).await?; + validate_groups(&data.groups, &org_id, &conn).await?; + + if member.atype == MembershipType::Owner + && new_type != MembershipType::Owner + && member.status == MembershipStatus::Confirmed as i32 + { + // Removing owner permission, check that there is at least one other confirmed owner + if Membership::count_confirmed_by_org_and_type(&org_id, MembershipType::Owner, &conn).await <= 1 { + err!("Can't delete the last owner") + } + } + + member.access_all = access_all; + member.atype = new_type as i32; + member.set_external_id(data.external_id.clone()); + + // This check is also done at accept_invite, _confirm_invite, _activate_member, edit_member, + // admin::update_membership_type. We need to perform the check after changing the type. + OrgPolicy::check_user_allowed(&member, "modify", &conn).await?; + + set_member_collections(&member, &data.collections, &org_id, &conn).await?; + set_member_groups(&member, &data.groups, &conn).await?; + + member.save(&conn).await?; + + log_public_event(EventType::OrganizationUserUpdated as i32, &member.uuid, &org_id, &ip.ip, &conn).await; + + Ok(Json(member_to_json(&member, &conn).await)) +} + +#[delete("/public/members/")] +async fn delete_member( + member_id: MembershipId, + token: PublicToken, + ip: auth::ClientIp, + conn: DbConn, + nt: Notify<'_>, +) -> EmptyResult { + let org_id = token.0; + let Some(member) = Membership::find_by_uuid_and_org(&member_id, &org_id, &conn).await else { + err_code!(format!("Member {member_id} not found in organization"), 404); + }; + + if member.atype == MembershipType::Owner && member.status == MembershipStatus::Confirmed as i32 { + // Removing owner, check that there is at least one other confirmed owner + if Membership::count_confirmed_by_org_and_type(&org_id, MembershipType::Owner, &conn).await <= 1 { + err!("Can't delete the last owner") + } + } + + log_public_event(EventType::OrganizationUserRemoved as i32, &member.uuid, &org_id, &ip.ip, &conn).await; + + if let Some(user) = User::find_by_uuid(&member.user_uuid, &conn).await { + // There is no device behind a Public API request, so no push device to exclude. + nt.send_user_update(UpdateType::SyncOrgKeys, &user, None, &conn).await; + + if !CONFIG.mail_enabled() + && !Membership::find_invited_by_user(&user.uuid, &conn).await.into_iter().any(|m| m.uuid != member.uuid) + { + Invitation::take(&user.email, &conn).await; + } + } + + member.delete(&conn).await +} + +#[put("/public/members//group-ids", data = "")] +async fn put_member_group_ids( + member_id: MembershipId, + data: Json, + token: PublicToken, + ip: auth::ClientIp, + conn: DbConn, +) -> EmptyResult { + let org_id = token.0; + if !CONFIG.org_groups_enabled() { + err!("Group support is disabled"); + } + + let Some(member) = Membership::find_by_uuid_and_org(&member_id, &org_id, &conn).await else { + err_code!(format!("Member {member_id} not found in organization"), 404); + }; + + let data = data.into_inner(); + validate_groups(&data.group_ids, &org_id, &conn).await?; + + set_member_groups(&member, &data.group_ids, &conn).await?; + + log_public_event(EventType::OrganizationUserUpdatedGroups as i32, &member.uuid, &org_id, &ip.ip, &conn).await; + + Ok(()) +} + +#[post("/public/members//reinvite")] +async fn post_member_reinvite(member_id: MembershipId, token: PublicToken, conn: DbConn) -> EmptyResult { + let org_id = token.0; + let Some(member) = Membership::find_by_uuid_and_org(&member_id, &org_id, &conn).await else { + err_code!(format!("Member {member_id} not found in organization"), 404); + }; + + if member.status != MembershipStatus::Invited as i32 { + err!("The user is already accepted or confirmed to the organization") + } + + let Some(user) = User::find_by_uuid(&member.user_uuid, &conn).await else { + err!("User not found.") + }; + + if !CONFIG.invitations_allowed() && user.password_hash.is_empty() { + err!("Invitations are not allowed.") + } + + let (org_name, org_email) = org_name_and_email(&org_id, &conn).await?; + + if CONFIG.mail_enabled() { + mail::send_invite(&user, org_id.clone(), member.uuid, &org_name, Some(org_email)).await?; + } else if user.password_hash.is_empty() { + Invitation::new(&user.email).save(&conn).await?; + } else { + Invitation::take(&user.email, &conn).await; + let mut member = member; + member.status = MembershipStatus::Accepted as i32; + member.save(&conn).await?; + } + + Ok(()) +} + +#[post("/public/members//revoke")] +async fn post_member_revoke( + member_id: MembershipId, + token: PublicToken, + ip: auth::ClientIp, + conn: DbConn, +) -> EmptyResult { + let org_id = token.0; + let Some(mut member) = Membership::find_by_uuid_and_org(&member_id, &org_id, &conn).await else { + err_code!(format!("Member {member_id} not found in organization"), 404); + }; + + if member.status <= MembershipStatus::Revoked as i32 { + err!("User is already revoked") + } + + if member.atype == MembershipType::Owner + && Membership::count_confirmed_by_org_and_type(&org_id, MembershipType::Owner, &conn).await <= 1 + { + err!("Organization must have at least one confirmed owner") + } + + member.revoke(); + member.save(&conn).await?; + + log_public_event(EventType::OrganizationUserRevoked as i32, &member.uuid, &org_id, &ip.ip, &conn).await; + + Ok(()) +} + +#[post("/public/members//restore")] +async fn post_member_restore( + member_id: MembershipId, + token: PublicToken, + ip: auth::ClientIp, + conn: DbConn, +) -> EmptyResult { + let org_id = token.0; + let Some(mut member) = Membership::find_by_uuid_and_org(&member_id, &org_id, &conn).await else { + err_code!(format!("Member {member_id} not found in organization"), 404); + }; + + if member.status >= MembershipStatus::Accepted as i32 { + err!("User is already active") + } + + member.restore(); + // This check is also done at accept_invite, _confirm_invite, _activate_member, edit_member, + // admin::update_membership_type. It needs to happen after restoring to see the correct status. + OrgPolicy::check_user_allowed(&member, "restore", &conn).await?; + member.save(&conn).await?; + + log_public_event(EventType::OrganizationUserRestored as i32, &member.uuid, &org_id, &ip.ip, &conn).await; + + Ok(()) +} + +#[post("/public/groups", data = "")] +async fn post_group( + data: Json, + token: PublicToken, + ip: auth::ClientIp, + conn: DbConn, +) -> JsonResult { + let org_id = token.0; + if !CONFIG.org_groups_enabled() { + err!("Group support is disabled"); + } + + let data = data.into_inner(); + validate_collections(&data.collections, &org_id, &conn).await?; + + let mut group = Group::new(org_id.clone(), data.name.clone(), data.access_all, data.external_id.clone()); + group.save(&conn).await?; + + set_group_collections(&group, &data.collections, &org_id, &conn).await?; + + log_public_event(EventType::GroupCreated as i32, &group.uuid, &org_id, &ip.ip, &conn).await; + + Ok(Json(group_to_json(&group))) +} + +#[put("/public/groups/", data = "")] +async fn put_group( + group_id: GroupId, + data: Json, + token: PublicToken, + ip: auth::ClientIp, + conn: DbConn, +) -> JsonResult { + let org_id = token.0; + if !CONFIG.org_groups_enabled() { + err!("Group support is disabled"); + } + + let Some(mut group) = Group::find_by_uuid_and_org(&group_id, &org_id, &conn).await else { + err_code!(format!("Group {group_id} not found in organization"), 404); + }; + + let data = data.into_inner(); + validate_collections(&data.collections, &org_id, &conn).await?; + + group.name.clone_from(&data.name); + group.access_all = data.access_all; + // Unlike the internal endpoint, the external_id is updatable here. The Public API is + // the directory integration surface, the same one "/public/organization/import" uses + // to assign external ids in the first place. + group.set_external_id(data.external_id.clone()); + group.save(&conn).await?; + + // Member assignments are owned by "/public/groups//member-ids" and are + // deliberately left untouched here. + set_group_collections(&group, &data.collections, &org_id, &conn).await?; + + log_public_event(EventType::GroupUpdated as i32, &group.uuid, &org_id, &ip.ip, &conn).await; + + Ok(Json(group_to_json(&group))) +} + +#[delete("/public/groups/")] +async fn delete_group(group_id: GroupId, token: PublicToken, ip: auth::ClientIp, conn: DbConn) -> EmptyResult { + let org_id = token.0; + if !CONFIG.org_groups_enabled() { + err!("Group support is disabled"); + } + + let Some(group) = Group::find_by_uuid_and_org(&group_id, &org_id, &conn).await else { + err_code!(format!("Group {group_id} not found in organization"), 404); + }; + + log_public_event(EventType::GroupDeleted as i32, &group.uuid, &org_id, &ip.ip, &conn).await; + + group.delete(&org_id, &conn).await +} + +#[put("/public/groups//member-ids", data = "")] +async fn put_group_member_ids( + group_id: GroupId, + data: Json, + token: PublicToken, + ip: auth::ClientIp, + conn: DbConn, +) -> EmptyResult { + let org_id = token.0; + if !CONFIG.org_groups_enabled() { + err!("Group support is disabled"); + } + + if Group::find_by_uuid_and_org(&group_id, &org_id, &conn).await.is_none() { + err_code!(format!("Group {group_id} not found in organization"), 404); + } + + let data = data.into_inner(); + validate_members(&data.member_ids, &org_id, &conn).await?; + + GroupUser::delete_all_by_group(&group_id, &org_id, &conn).await?; + for member_id in &data.member_ids { + let mut user_entry = GroupUser::new(group_id.clone(), member_id.clone()); + user_entry.save(&conn).await?; + + log_public_event(EventType::OrganizationUserUpdatedGroups as i32, member_id, &org_id, &ip.ip, &conn).await; + } + + Ok(()) +} + pub struct PublicToken(OrganizationId); #[rocket::async_trait] From 2a9c148f1d25a25f0ca18428f91a54e9cfe50b82 Mon Sep 17 00:00:00 2001 From: Rune Darrud Date: Mon, 10 Aug 2026 01:21:11 +0200 Subject: [PATCH 4/7] Add smoke test for the Public API member and group write endpoints Boots a throwaway instance against a seeded SQLite database and exercises every member and group write endpoint end to end, asserting on the resulting state rather than just the status code. Covers the guards that protect organization integrity: the last confirmed owner cannot be demoted, revoked or deleted; collections, groups and members from another organization are rejected; ids belonging to another organization return 404; and writes require a token. It also pins two behaviours that are easy to regress: a group update leaves member assignments alone, and every write is recorded in the event log with no acting user and no device type. Co-Authored-By: Claude Opus 5 --- scripts/smoke_public_api_write.sh | 454 ++++++++++++++++++++++++++++++ 1 file changed, 454 insertions(+) create mode 100755 scripts/smoke_public_api_write.sh diff --git a/scripts/smoke_public_api_write.sh b/scripts/smoke_public_api_write.sh new file mode 100755 index 00000000..6b988371 --- /dev/null +++ b/scripts/smoke_public_api_write.sh @@ -0,0 +1,454 @@ +#!/usr/bin/env bash +# +# Smoke test for the organization Public API member and group write endpoints. +# +# Boots a throwaway Vaultwarden instance against a temporary SQLite database +# seeded with two organizations, mints an organization API token for the first +# org, then exercises every write endpoint and asserts on the resulting state. +# +# Beyond the happy paths it asserts the guards that protect organization +# integrity: the last confirmed owner cannot be demoted, revoked or deleted; +# collections, groups and members from another organization are rejected; ids +# belonging to the second organization return HTTP 404; and a request with no +# token returns HTTP 401. It also asserts that a group update leaves member +# assignments alone, and that every write is recorded in the event log with no +# acting user, since a Public API client is not a user. +# +# The script exits non-zero if any assertion fails, so it is usable as a check. +# +# Requirements: bash, curl, jq, sqlite3, and either a prebuilt binary passed via +# the VW_BIN environment variable or a cargo toolchain to build one. +# +# Usage: +# scripts/smoke_public_api_write.sh +# VW_BIN=/path/to/vaultwarden PORT=8123 scripts/smoke_public_api_write.sh + +set -euo pipefail + +SCRIPT_DIR=$(cd "$(dirname "$0")" && pwd) +REPO_ROOT=$(cd "$SCRIPT_DIR/.." && pwd) +cd "$REPO_ROOT" + +PORT="${PORT:-8082}" +VW_BIN="${VW_BIN:-$REPO_ROOT/target/debug/vaultwarden}" +API="http://127.0.0.1:$PORT" + +# ---- fixtures ------------------------------------------------------------- +ORG=22222222-2222-4222-8222-222222222222 +ORG2=99999999-9999-4999-8999-999999999999 +USER=11111111-1111-4111-8111-111111111111 +USER2=88888888-8888-4888-8888-888888888888 +USER3=cccccccc-cccc-4ccc-8ccc-cccccccccccc +# MEMBER is the only confirmed owner, so it is the one the guards protect. +MEMBER=33333333-3333-4333-8333-333333333333 +MEMBER2=aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa +MEMBER3=dddddddd-dddd-4ddd-8ddd-dddddddddddd +GROUP=44444444-4444-4444-8444-444444444444 +GROUP2=bbbbbbbb-bbbb-4bbb-8bbb-bbbbbbbbbbbb +COLLECTION=55555555-5555-4555-8555-555555555555 +COLLECTION2=66666666-6666-4666-8666-666666666666 +APIKEYUUID=77777777-7777-4777-8777-777777777777 +APIKEY=smoketestapikey1234567890 + +NEW_EMAIL=newmember@example.com + +# ---- prerequisites -------------------------------------------------------- +for tool in curl jq sqlite3; do + if ! command -v "$tool" >/dev/null 2>&1; then + echo "ERROR: required tool '$tool' is not installed" >&2 + exit 2 + fi +done + +if [ ! -x "$VW_BIN" ]; then + if command -v cargo >/dev/null 2>&1; then + echo "Building vaultwarden (sqlite feature); this can take a while..." + cargo build --features sqlite + else + echo "ERROR: no binary at '$VW_BIN' and no cargo toolchain to build one." >&2 + echo "Set VW_BIN to a prebuilt binary or install a Rust toolchain." >&2 + exit 2 + fi +fi + +# ---- workspace + cleanup -------------------------------------------------- +TMP=$(mktemp -d) +SERVER_PID="" +cleanup() { + if [ -n "$SERVER_PID" ]; then + kill "$SERVER_PID" >/dev/null 2>&1 || true + wait "$SERVER_PID" 2>/dev/null || true + fi + rm -rf "$TMP" +} +trap cleanup EXIT + +export DATA_FOLDER="$TMP" +export DATABASE_URL="sqlite://$TMP/db.sqlite3" +export ADMIN_TOKEN="smoketestadmintoken" +export ORG_GROUPS_ENABLED=true +export ORG_EVENTS_ENABLED=true +export INVITATIONS_ALLOWED=true +export WEB_VAULT_ENABLED=false +export ROCKET_PORT="$PORT" +export ROCKET_ADDRESS=127.0.0.1 +export DOMAIN="http://localhost:$PORT" + +# ---- server helpers ------------------------------------------------------- +start_server() { + local logfile="$1" + "$VW_BIN" >"$logfile" 2>&1 & + SERVER_PID=$! + local i + for i in $(seq 1 90); do + if grep -q "Rocket has launched" "$logfile" 2>/dev/null; then + return 0 + fi + if ! kill -0 "$SERVER_PID" 2>/dev/null; then + echo "ERROR: server exited during startup. Log:" >&2 + cat "$logfile" >&2 + return 1 + fi + sleep 1 + done + echo "ERROR: server did not launch within 90s. Log:" >&2 + cat "$logfile" >&2 + return 1 +} + +stop_server() { + if [ -n "$SERVER_PID" ]; then + kill "$SERVER_PID" >/dev/null 2>&1 || true + wait "$SERVER_PID" 2>/dev/null || true + SERVER_PID="" + fi +} + +# ---- assertion helpers ---------------------------------------------------- +FAILS=0 +pass() { printf 'PASS: %s\n' "$1"; } +fail() { printf 'FAIL: %s\n' "$1"; FAILS=$((FAILS + 1)); } + +check_eq() { # label actual expected + if [ "$2" = "$3" ]; then + pass "$1" + else + fail "$1 (expected [$3], got [$2])" + fi +} + +# req METHOD PATH [TOKEN] -> sets HTTP_CODE, body written to $TMP/body +req() { + local method="$1" path="$2" token="${3:-}" + if [ -n "$token" ]; then + HTTP_CODE=$(curl -sS -o "$TMP/body" -w '%{http_code}' \ + -X "$method" -H "Authorization: Bearer $token" "$API$path") + else + HTTP_CODE=$(curl -sS -o "$TMP/body" -w '%{http_code}' -X "$method" "$API$path") + fi +} + +# reqj METHOD PATH TOKEN JSON -> same, with a JSON request body +reqj() { + local method="$1" path="$2" token="$3" body="$4" + if [ -n "$token" ]; then + HTTP_CODE=$(curl -sS -o "$TMP/body" -w '%{http_code}' \ + -X "$method" -H "Authorization: Bearer $token" \ + -H 'Content-Type: application/json' -d "$body" "$API$path") + else + HTTP_CODE=$(curl -sS -o "$TMP/body" -w '%{http_code}' \ + -X "$method" -H 'Content-Type: application/json' -d "$body" "$API$path") + fi +} + +jqval() { jq -r "$1" "$TMP/body"; } + +jqcheck() { # label filter expected + check_eq "$1" "$(jqval "$2")" "$3" +} + +sqlcheck() { # label sql expected + check_eq "$1" "$(sqlite3 "$TMP/db.sqlite3" "$2")" "$3" +} + +# ---- boot once to run migrations, then seed, then boot to serve ----------- +echo "== Booting once to create the database schema ==" +start_server "$TMP/boot1.log" +stop_server + +echo "== Seeding two organizations with members, groups and collections ==" +sqlite3 "$TMP/db.sqlite3" <&2 + exit 1 +fi +pass "minted organization API token" + +echo "" +echo "== Create a group ==" + +reqj POST "/api/public/groups" "$TOKEN" \ + "{\"name\":\"Platform\",\"externalId\":\"ext-new-group\",\"collections\":[{\"id\":\"$COLLECTION\",\"readOnly\":true,\"hidePasswords\":false,\"manage\":false}]}" +check_eq "create group -> 200" "$HTTP_CODE" "200" +jqcheck "created group discriminator" '.object' "group" +jqcheck "created group name" '.name' "Platform" +jqcheck "created group externalId" '.externalId' "ext-new-group" +jqcheck "created group accessAll defaults to false" '.accessAll' "false" +NEWGROUP=$(jqval '.id') + +req GET "/api/public/groups/$NEWGROUP" "$TOKEN" +check_eq "created group is readable -> 200" "$HTTP_CODE" "200" +jqcheck "created group kept its collection grant" '.collections | length' "1" +jqcheck "created group collection id" '.collections[0].id' "$COLLECTION" +jqcheck "created group collection readOnly" '.collections[0].readOnly' "true" + +echo "" +echo "== Group input validation ==" + +reqj POST "/api/public/groups" "$TOKEN" \ + "{\"name\":\"Bad\",\"collections\":[{\"id\":\"$COLLECTION2\"}]}" +check_eq "group with another org's collection -> 400" "$HTTP_CODE" "400" + +echo "" +echo "== Update a group ==" + +reqj PUT "/api/public/groups/$NEWGROUP" "$TOKEN" \ + "{\"name\":\"Platform Team\",\"externalId\":\"ext-new-group-2\",\"collections\":[]}" +check_eq "update group -> 200" "$HTTP_CODE" "200" +jqcheck "updated group name" '.name' "Platform Team" +jqcheck "updated group externalId" '.externalId' "ext-new-group-2" + +req GET "/api/public/groups/$NEWGROUP" "$TOKEN" +jqcheck "update replaced the collection grants" '.collections | length' "0" + +echo "" +echo "== Group member ids ==" + +reqj PUT "/api/public/groups/$NEWGROUP/member-ids" "$TOKEN" "{\"memberIds\":[\"$MEMBER3\"]}" +check_eq "set group member-ids -> 200" "$HTTP_CODE" "200" + +req GET "/api/public/groups/$NEWGROUP/member-ids" "$TOKEN" +jqcheck "group has one member" 'length' "1" +jqcheck "group member is the expected membership" '.[0]' "$MEMBER3" + +reqj PUT "/api/public/groups/$NEWGROUP/member-ids" "$TOKEN" "{\"memberIds\":[\"$MEMBER2\"]}" +check_eq "group member-ids from another org -> 400" "$HTTP_CODE" "400" + +# Members are owned by the member-ids endpoint, so a group update must leave +# them alone. The internal endpoint clears them, this one deliberately does not. +reqj PUT "/api/public/groups/$NEWGROUP" "$TOKEN" "{\"name\":\"Platform Team\",\"collections\":[]}" +check_eq "update group again -> 200" "$HTTP_CODE" "200" +req GET "/api/public/groups/$NEWGROUP/member-ids" "$TOKEN" +jqcheck "group update left member assignments intact" 'length' "1" + +echo "" +echo "== Create a member ==" + +reqj POST "/api/public/members" "$TOKEN" \ + "{\"email\":\"$NEW_EMAIL\",\"type\":2,\"externalId\":\"ext-new-member\",\"collections\":[{\"id\":\"$COLLECTION\",\"readOnly\":true,\"hidePasswords\":false,\"manage\":false}],\"groups\":[\"$GROUP\"]}" +check_eq "create member -> 200" "$HTTP_CODE" "200" +jqcheck "created member discriminator" '.object' "member" +jqcheck "created member email" '.email' "$NEW_EMAIL" +jqcheck "created member type" '.type' "2" +jqcheck "created member externalId" '.externalId' "ext-new-member" +jqcheck "created member is invited" '.status' "0" +NEWMEMBER=$(jqval '.id') + +req GET "/api/public/members/$NEWMEMBER" "$TOKEN" +jqcheck "created member kept its collection grant" '.collections | length' "1" +jqcheck "created member collection readOnly" '.collections[0].readOnly' "true" + +req GET "/api/public/members/$NEWMEMBER/group-ids" "$TOKEN" +jqcheck "created member joined the group" 'length' "1" +jqcheck "created member group id" '.[0]' "$GROUP" + +echo "" +echo "== Member input validation ==" + +reqj POST "/api/public/members" "$TOKEN" "{\"email\":\"$NEW_EMAIL\",\"type\":2}" +check_eq "duplicate member email -> 400" "$HTTP_CODE" "400" + +reqj POST "/api/public/members" "$TOKEN" \ + "{\"email\":\"other@example.com\",\"type\":2,\"groups\":[\"$GROUP2\"]}" +check_eq "member with another org's group -> 400" "$HTTP_CODE" "400" + +reqj POST "/api/public/members" "$TOKEN" "{\"email\":\"bad@example.com\",\"type\":99}" +check_eq "member with an unknown type -> 400" "$HTTP_CODE" "400" + +echo "" +echo "== Update a member ==" + +reqj PUT "/api/public/members/$NEWMEMBER" "$TOKEN" \ + "{\"type\":2,\"externalId\":\"ext-updated\",\"collections\":[{\"id\":\"$COLLECTION\",\"readOnly\":false,\"hidePasswords\":true,\"manage\":false}],\"groups\":[]}" +check_eq "update member -> 200" "$HTTP_CODE" "200" +jqcheck "updated member externalId" '.externalId' "ext-updated" + +req GET "/api/public/members/$NEWMEMBER" "$TOKEN" +jqcheck "update replaced the collection grants" '.collections | length' "1" +jqcheck "updated collection readOnly" '.collections[0].readOnly' "false" +jqcheck "updated collection hidePasswords" '.collections[0].hidePasswords' "true" + +req GET "/api/public/members/$NEWMEMBER/group-ids" "$TOKEN" +jqcheck "update cleared the group assignments" 'length' "0" + +echo "" +echo "== Member group ids ==" + +reqj PUT "/api/public/members/$NEWMEMBER/group-ids" "$TOKEN" "{\"groupIds\":[\"$GROUP\"]}" +check_eq "set member group-ids -> 200" "$HTTP_CODE" "200" +req GET "/api/public/members/$NEWMEMBER/group-ids" "$TOKEN" +jqcheck "member group-ids applied" '.[0]' "$GROUP" + +reqj PUT "/api/public/members/$NEWMEMBER/group-ids" "$TOKEN" "{\"groupIds\":[\"$GROUP2\"]}" +check_eq "member group-ids from another org -> 400" "$HTTP_CODE" "400" + +echo "" +echo "== Reinvite ==" + +req POST "/api/public/members/$NEWMEMBER/reinvite" "$TOKEN" +check_eq "reinvite an invited member -> 200" "$HTTP_CODE" "200" + +echo "" +echo "== Revoke and restore ==" + +req POST "/api/public/members/$MEMBER3/revoke" "$TOKEN" +check_eq "revoke a member -> 200" "$HTTP_CODE" "200" +req GET "/api/public/members/$MEMBER3" "$TOKEN" +jqcheck "revoked member has a revoked status" '.status < 0' "true" + +req POST "/api/public/members/$MEMBER3/revoke" "$TOKEN" +check_eq "revoking twice -> 400" "$HTTP_CODE" "400" + +req POST "/api/public/members/$MEMBER3/restore" "$TOKEN" +check_eq "restore a member -> 200" "$HTTP_CODE" "200" +req GET "/api/public/members/$MEMBER3" "$TOKEN" +jqcheck "restored member is confirmed again" '.status' "2" + +req POST "/api/public/members/$MEMBER3/restore" "$TOKEN" +check_eq "restoring an active member -> 400" "$HTTP_CODE" "400" + +echo "" +echo "== The last confirmed owner is protected ==" + +reqj PUT "/api/public/members/$MEMBER" "$TOKEN" "{\"type\":2}" +check_eq "demoting the last owner -> 400" "$HTTP_CODE" "400" + +req DELETE "/api/public/members/$MEMBER" "$TOKEN" +check_eq "deleting the last owner -> 400" "$HTTP_CODE" "400" + +req POST "/api/public/members/$MEMBER/revoke" "$TOKEN" +check_eq "revoking the last owner -> 400" "$HTTP_CODE" "400" + +req GET "/api/public/members/$MEMBER" "$TOKEN" +jqcheck "the last owner is untouched, type" '.type' "0" +jqcheck "the last owner is untouched, status" '.status' "2" + +echo "" +echo "== Organization scoping boundary ==" + +reqj PUT "/api/public/members/$MEMBER2" "$TOKEN" "{\"type\":2}" +check_eq "updating a member of another org -> 404" "$HTTP_CODE" "404" +req DELETE "/api/public/members/$MEMBER2" "$TOKEN" +check_eq "deleting a member of another org -> 404" "$HTTP_CODE" "404" +req POST "/api/public/members/$MEMBER2/revoke" "$TOKEN" +check_eq "revoking a member of another org -> 404" "$HTTP_CODE" "404" +reqj PUT "/api/public/groups/$GROUP2" "$TOKEN" "{\"name\":\"Hijacked\"}" +check_eq "updating a group of another org -> 404" "$HTTP_CODE" "404" +req DELETE "/api/public/groups/$GROUP2" "$TOKEN" +check_eq "deleting a group of another org -> 404" "$HTTP_CODE" "404" + +echo "" +echo "== Delete ==" + +req DELETE "/api/public/members/$NEWMEMBER" "$TOKEN" +check_eq "delete member -> 200" "$HTTP_CODE" "200" +req GET "/api/public/members/$NEWMEMBER" "$TOKEN" +check_eq "deleted member is gone -> 404" "$HTTP_CODE" "404" + +req DELETE "/api/public/groups/$NEWGROUP" "$TOKEN" +check_eq "delete group -> 200" "$HTTP_CODE" "200" +req GET "/api/public/groups/$NEWGROUP" "$TOKEN" +check_eq "deleted group is gone -> 404" "$HTTP_CODE" "404" + +echo "" +echo "== Authentication required ==" + +reqj POST "/api/public/groups" "" "{\"name\":\"NoToken\"}" +check_eq "create group with no token -> 401" "$HTTP_CODE" "401" +req DELETE "/api/public/members/$MEMBER3" +check_eq "delete member with no token -> 401" "$HTTP_CODE" "401" +req GET "/api/public/members/$MEMBER3" "$TOKEN" +check_eq "the unauthenticated delete changed nothing" "$HTTP_CODE" "200" + +echo "" +echo "== Writes are recorded in the event log without an acting user ==" + +stop_server + +# 1400 GroupCreated, 1401 GroupUpdated, 1402 GroupDeleted, +# 1500 OrganizationUserInvited, 1502 OrganizationUserUpdated, +# 1503 OrganizationUserRemoved, 1511 Revoked, 1512 Restored. +for pair in "1400:group created" "1401:group updated" "1402:group deleted" \ + "1500:member invited" "1502:member updated" "1503:member removed" \ + "1511:member revoked" "1512:member restored"; do + code="${pair%%:*}" + label="${pair#*:}" + got=$(sqlite3 "$TMP/db.sqlite3" \ + "SELECT COUNT(*) > 0 FROM event WHERE org_uuid='$ORG' AND event_type=$code;") + check_eq "event logged: $label" "$got" "1" +done + +sqlcheck "no Public API event records an acting user" \ + "SELECT COUNT(*) FROM event WHERE org_uuid='$ORG' AND act_user_uuid IS NOT NULL;" "0" +sqlcheck "no Public API event records a device type" \ + "SELECT COUNT(*) FROM event WHERE org_uuid='$ORG' AND device_type IS NOT NULL;" "0" +sqlcheck "Public API events still record the client address" \ + "SELECT COUNT(*) FROM event WHERE org_uuid='$ORG' AND ip_address IS NULL;" "0" + +echo "" +if [ "$FAILS" -ne 0 ]; then + echo "RESULT: $FAILS assertion(s) failed." + exit 1 +fi +echo "RESULT: all assertions passed." From b1473be414ad273c60a06a5dea0723efba0699d8 Mon Sep 17 00:00:00 2001 From: Rune Darrud Date: Mon, 10 Aug 2026 01:40:05 +0200 Subject: [PATCH 5/7] Harden the Public API member and group write endpoints Adversarial review of the new endpoints turned up several defects. Ownership is now out of reach of a Public API client. The internal endpoints only let an Owner grant, change or remove Owner, and that check cannot be applied here because there is no user behind the request. Since the organization API key can be created by an Admin, a client could previously promote itself to Owner and take over the organization. Granting the Owner role and acting on an existing owner are both refused now, which subsumes the narrower last confirmed owner guard. An omitted groups list on a member update no longer unassigns every group. Upstream leaves group access untouched when the field is absent and only collections reset on omission, so groups is optional now. An omitted accessAll on a group update no longer clears the flag, and an omitted externalId no longer clears the directory matching key that /public/organization/import relies on to match members and groups. Revoked members are reported with the upstream status of -1 instead of the internal offset encoding, matching every other serializer in the codebase. Restore refuses a member who is not revoked, rather than saving nothing and logging a restore that did not happen, and group creation is logged before its collection associations so a failure cannot leave an unaudited group. Co-Authored-By: Claude Opus 5 --- scripts/smoke_public_api_write.sh | 55 +++++++++++++--- src/api/core/public.rs | 104 +++++++++++++++++++----------- 2 files changed, 112 insertions(+), 47 deletions(-) diff --git a/scripts/smoke_public_api_write.sh b/scripts/smoke_public_api_write.sh index 6b988371..47db57a4 100755 --- a/scripts/smoke_public_api_write.sh +++ b/scripts/smoke_public_api_write.sh @@ -282,6 +282,20 @@ check_eq "update group again -> 200" "$HTTP_CODE" "200" req GET "/api/public/groups/$NEWGROUP/member-ids" "$TOKEN" jqcheck "group update left member assignments intact" 'length' "1" +echo "" +echo "== accessAll survives an update that omits it ==" + +reqj POST "/api/public/groups" "$TOKEN" "{\"name\":\"Full Access\",\"accessAll\":true,\"collections\":[]}" +check_eq "create an accessAll group -> 200" "$HTTP_CODE" "200" +jqcheck "created group has accessAll" '.accessAll' "true" +AAGROUP=$(jqval '.id') + +reqj PUT "/api/public/groups/$AAGROUP" "$TOKEN" "{\"name\":\"Full Access Renamed\",\"collections\":[]}" +check_eq "rename without accessAll -> 200" "$HTTP_CODE" "200" +jqcheck "omitted accessAll is preserved" '.accessAll' "true" +req DELETE "/api/public/groups/$AAGROUP" "$TOKEN" +check_eq "clean up the accessAll group -> 200" "$HTTP_CODE" "200" + echo "" echo "== Create a member ==" @@ -319,10 +333,12 @@ check_eq "member with an unknown type -> 400" "$HTTP_CODE" "400" echo "" echo "== Update a member ==" +# An omitted groups list must leave group membership alone, and an omitted externalId +# must not clear the directory matching key. reqj PUT "/api/public/members/$NEWMEMBER" "$TOKEN" \ - "{\"type\":2,\"externalId\":\"ext-updated\",\"collections\":[{\"id\":\"$COLLECTION\",\"readOnly\":false,\"hidePasswords\":true,\"manage\":false}],\"groups\":[]}" + "{\"type\":2,\"collections\":[{\"id\":\"$COLLECTION\",\"readOnly\":false,\"hidePasswords\":true,\"manage\":false}]}" check_eq "update member -> 200" "$HTTP_CODE" "200" -jqcheck "updated member externalId" '.externalId' "ext-updated" +jqcheck "omitted externalId is preserved" '.externalId' "ext-new-member" req GET "/api/public/members/$NEWMEMBER" "$TOKEN" jqcheck "update replaced the collection grants" '.collections | length' "1" @@ -330,7 +346,15 @@ jqcheck "updated collection readOnly" '.collections[0].readOnly' "false" jqcheck "updated collection hidePasswords" '.collections[0].hidePasswords' "true" req GET "/api/public/members/$NEWMEMBER/group-ids" "$TOKEN" -jqcheck "update cleared the group assignments" 'length' "0" +jqcheck "omitted groups leave membership alone" 'length' "1" + +# An explicit empty list does clear them. +reqj PUT "/api/public/members/$NEWMEMBER" "$TOKEN" \ + "{\"type\":2,\"externalId\":\"ext-updated\",\"collections\":[],\"groups\":[]}" +check_eq "update member with explicit empty groups -> 200" "$HTTP_CODE" "200" +jqcheck "explicit externalId is applied" '.externalId' "ext-updated" +req GET "/api/public/members/$NEWMEMBER/group-ids" "$TOKEN" +jqcheck "explicit empty groups clears membership" 'length' "0" echo "" echo "== Member group ids ==" @@ -355,7 +379,7 @@ echo "== Revoke and restore ==" req POST "/api/public/members/$MEMBER3/revoke" "$TOKEN" check_eq "revoke a member -> 200" "$HTTP_CODE" "200" req GET "/api/public/members/$MEMBER3" "$TOKEN" -jqcheck "revoked member has a revoked status" '.status < 0' "true" +jqcheck "revoked member reports the upstream revoked status" '.status' "-1" req POST "/api/public/members/$MEMBER3/revoke" "$TOKEN" check_eq "revoking twice -> 400" "$HTTP_CODE" "400" @@ -368,21 +392,32 @@ jqcheck "restored member is confirmed again" '.status' "2" req POST "/api/public/members/$MEMBER3/restore" "$TOKEN" check_eq "restoring an active member -> 400" "$HTTP_CODE" "400" +req POST "/api/public/members/$NEWMEMBER/restore" "$TOKEN" +check_eq "restoring an invited member -> 400" "$HTTP_CODE" "400" + echo "" -echo "== The last confirmed owner is protected ==" +echo "== Ownership is out of reach for a Public API client ==" + +reqj POST "/api/public/members" "$TOKEN" "{\"email\":\"owner@example.com\",\"type\":0}" +check_eq "creating an Owner -> 400" "$HTTP_CODE" "400" + +reqj PUT "/api/public/members/$MEMBER3" "$TOKEN" "{\"type\":0}" +check_eq "promoting a member to Owner -> 400" "$HTTP_CODE" "400" +req GET "/api/public/members/$MEMBER3" "$TOKEN" +jqcheck "the member was not promoted" '.type' "2" reqj PUT "/api/public/members/$MEMBER" "$TOKEN" "{\"type\":2}" -check_eq "demoting the last owner -> 400" "$HTTP_CODE" "400" +check_eq "demoting an owner -> 400" "$HTTP_CODE" "400" req DELETE "/api/public/members/$MEMBER" "$TOKEN" -check_eq "deleting the last owner -> 400" "$HTTP_CODE" "400" +check_eq "deleting an owner -> 400" "$HTTP_CODE" "400" req POST "/api/public/members/$MEMBER/revoke" "$TOKEN" -check_eq "revoking the last owner -> 400" "$HTTP_CODE" "400" +check_eq "revoking an owner -> 400" "$HTTP_CODE" "400" req GET "/api/public/members/$MEMBER" "$TOKEN" -jqcheck "the last owner is untouched, type" '.type' "0" -jqcheck "the last owner is untouched, status" '.status' "2" +jqcheck "the owner is untouched, type" '.type' "0" +jqcheck "the owner is untouched, status" '.status' "2" echo "" echo "== Organization scoping boundary ==" diff --git a/src/api/core/public.rs b/src/api/core/public.rs index b6f091f1..d790274f 100644 --- a/src/api/core/public.rs +++ b/src/api/core/public.rs @@ -243,6 +243,15 @@ async fn member_to_json(member: &Membership, conn: &DbConn) -> Value { None => (Value::Null, Value::Null), }; + // Revoked members carry their pre-revocation status offset by ACTIVATE_REVOKE_DIFF so + // it can be restored later. Upstream only knows -1, which is what the other serializers + // report too, so clamp it here rather than leaking the internal encoding. + let status = if member.status < MembershipStatus::Revoked as i32 { + MembershipStatus::Revoked as i32 + } else { + member.status + }; + json!({ "object": "member", "id": member.uuid, @@ -252,7 +261,7 @@ async fn member_to_json(member: &Membership, conn: &DbConn) -> Value { "type": member.atype, "externalId": member.external_id, "resetPasswordEnrolled": member.reset_password_key.is_some(), - "status": member.status, + "status": status, }) } @@ -473,10 +482,11 @@ struct MemberCreateData { struct MemberUpdateData { r#type: NumberOrString, external_id: Option, + // An omitted collections list clears the assignments, but an omitted groups list + // leaves them alone, matching how upstream treats the two. #[serde(default)] collections: Vec, - #[serde(default)] - groups: Vec, + groups: Option>, #[serde(default)] permissions: HashMap, } @@ -486,9 +496,9 @@ struct MemberUpdateData { struct GroupCreateUpdateData { name: String, // Upstream dropped accessAll from its group model, but the Vaultwarden group still - // carries the flag, so it is accepted here and defaults to false when omitted. - #[serde(default)] - access_all: bool, + // carries the flag. It is accepted here so it stays reachable, and left untouched + // when omitted so a client following the upstream model cannot silently clear it. + access_all: Option, external_id: Option, #[serde(default)] collections: Vec, @@ -528,6 +538,23 @@ fn member_type_and_access_all( Some((new_type, access_all)) } +// The internal endpoints only let an Owner grant, change or remove Owner. A Public API +// client has no user behind it to check that against, and the organization API key can be +// created by an Admin, so ownership is placed out of its reach entirely. +fn deny_owner_grant(new_type: MembershipType) -> EmptyResult { + if new_type == MembershipType::Owner { + err!("The Public API cannot grant the Owner role") + } + Ok(()) +} + +fn deny_owner_target(member: &Membership) -> EmptyResult { + if member.atype == MembershipType::Owner { + err!("The Public API cannot modify an organization owner") + } + Ok(()) +} + async fn validate_collections(collections: &[AssociationData], org_id: &OrganizationId, conn: &DbConn) -> EmptyResult { let org_collections = Collection::find_by_organization(org_id, conn).await; let org_collection_ids: HashSet<&CollectionId> = org_collections.iter().map(|c| &c.uuid).collect(); @@ -621,6 +648,7 @@ async fn post_member(data: Json, token: PublicToken, ip: auth: let Some((new_type, access_all)) = member_type_and_access_all(data.r#type, &data.permissions) else { err!("Invalid type") }; + deny_owner_grant(new_type)?; validate_collections(&data.collections, &org_id, &conn).await?; validate_groups(&data.groups, &org_id, &conn).await?; @@ -669,7 +697,11 @@ async fn post_member(data: Json, token: PublicToken, ip: auth: new_member.access_all = access_all; new_member.atype = new_type as i32; new_member.status = member_status; - new_member.set_external_id(data.external_id.clone()); + // Only touch the external id when one was sent. Clearing it on omission would break + // the key "/public/organization/import" matches members and groups on. + if data.external_id.is_some() { + new_member.set_external_id(data.external_id.clone()); + } new_member.save(&conn).await?; if CONFIG.mail_enabled() @@ -708,34 +740,32 @@ async fn put_member( let Some((new_type, access_all)) = member_type_and_access_all(data.r#type, &data.permissions) else { err!("Invalid type") }; + deny_owner_grant(new_type)?; let Some(mut member) = Membership::find_by_uuid_and_org(&member_id, &org_id, &conn).await else { err_code!(format!("Member {member_id} not found in organization"), 404); }; + deny_owner_target(&member)?; validate_collections(&data.collections, &org_id, &conn).await?; - validate_groups(&data.groups, &org_id, &conn).await?; - - if member.atype == MembershipType::Owner - && new_type != MembershipType::Owner - && member.status == MembershipStatus::Confirmed as i32 - { - // Removing owner permission, check that there is at least one other confirmed owner - if Membership::count_confirmed_by_org_and_type(&org_id, MembershipType::Owner, &conn).await <= 1 { - err!("Can't delete the last owner") - } + if let Some(group_ids) = &data.groups { + validate_groups(group_ids, &org_id, &conn).await?; } member.access_all = access_all; member.atype = new_type as i32; - member.set_external_id(data.external_id.clone()); + if data.external_id.is_some() { + member.set_external_id(data.external_id.clone()); + } // This check is also done at accept_invite, _confirm_invite, _activate_member, edit_member, // admin::update_membership_type. We need to perform the check after changing the type. OrgPolicy::check_user_allowed(&member, "modify", &conn).await?; set_member_collections(&member, &data.collections, &org_id, &conn).await?; - set_member_groups(&member, &data.groups, &conn).await?; + if let Some(group_ids) = &data.groups { + set_member_groups(&member, group_ids, &conn).await?; + } member.save(&conn).await?; @@ -757,12 +787,7 @@ async fn delete_member( err_code!(format!("Member {member_id} not found in organization"), 404); }; - if member.atype == MembershipType::Owner && member.status == MembershipStatus::Confirmed as i32 { - // Removing owner, check that there is at least one other confirmed owner - if Membership::count_confirmed_by_org_and_type(&org_id, MembershipType::Owner, &conn).await <= 1 { - err!("Can't delete the last owner") - } - } + deny_owner_target(&member)?; log_public_event(EventType::OrganizationUserRemoved as i32, &member.uuid, &org_id, &ip.ip, &conn).await; @@ -854,16 +879,12 @@ async fn post_member_revoke( err_code!(format!("Member {member_id} not found in organization"), 404); }; + deny_owner_target(&member)?; + if member.status <= MembershipStatus::Revoked as i32 { err!("User is already revoked") } - if member.atype == MembershipType::Owner - && Membership::count_confirmed_by_org_and_type(&org_id, MembershipType::Owner, &conn).await <= 1 - { - err!("Organization must have at least one confirmed owner") - } - member.revoke(); member.save(&conn).await?; @@ -884,7 +905,11 @@ async fn post_member_restore( err_code!(format!("Member {member_id} not found in organization"), 404); }; - if member.status >= MembershipStatus::Accepted as i32 { + deny_owner_target(&member)?; + + // Anything above Revoked is already active. Testing against Accepted would let an + // invited member through, producing a no-op save and a restore event that never happened. + if member.status > MembershipStatus::Revoked as i32 { err!("User is already active") } @@ -914,13 +939,14 @@ async fn post_group( let data = data.into_inner(); validate_collections(&data.collections, &org_id, &conn).await?; - let mut group = Group::new(org_id.clone(), data.name.clone(), data.access_all, data.external_id.clone()); + let mut group = + Group::new(org_id.clone(), data.name.clone(), data.access_all.unwrap_or(false), data.external_id.clone()); group.save(&conn).await?; - set_group_collections(&group, &data.collections, &org_id, &conn).await?; - log_public_event(EventType::GroupCreated as i32, &group.uuid, &org_id, &ip.ip, &conn).await; + set_group_collections(&group, &data.collections, &org_id, &conn).await?; + Ok(Json(group_to_json(&group))) } @@ -945,11 +971,15 @@ async fn put_group( validate_collections(&data.collections, &org_id, &conn).await?; group.name.clone_from(&data.name); - group.access_all = data.access_all; + if let Some(access_all) = data.access_all { + group.access_all = access_all; + } // Unlike the internal endpoint, the external_id is updatable here. The Public API is // the directory integration surface, the same one "/public/organization/import" uses // to assign external ids in the first place. - group.set_external_id(data.external_id.clone()); + if data.external_id.is_some() { + group.set_external_id(data.external_id.clone()); + } group.save(&conn).await?; // Member assignments are owned by "/public/groups//member-ids" and are From 7f2911231a7e4c287c62ca3e3ca8f4edcf7818fc Mon Sep 17 00:00:00 2001 From: Rune Darrud Date: Mon, 10 Aug 2026 02:00:40 +0200 Subject: [PATCH 6/7] Expose member permissions so manage-all survives a write The write endpoints derive the access_all flag from the raw type plus the three collection permissions, but the member serializer emitted neither the custom role nor the permissions object. A client that read a manage-all member and wrote it back unchanged therefore dropped that member from every collection, because the information needed to round-trip was not in the response. The member object now reports the custom role and its permissions the same way the internal serializers do, which is also the shape upstream uses: Permissions is part of the shared member model and is returned on reads as well as accepted on writes. Co-Authored-By: Claude Opus 5 --- scripts/smoke_public_api_write.sh | 33 +++++++++++++++++++++++++++++++ src/api/core/public.rs | 29 ++++++++++++++++++++++++++- 2 files changed, 61 insertions(+), 1 deletion(-) diff --git a/scripts/smoke_public_api_write.sh b/scripts/smoke_public_api_write.sh index 47db57a4..18540d5a 100755 --- a/scripts/smoke_public_api_write.sh +++ b/scripts/smoke_public_api_write.sh @@ -395,6 +395,39 @@ check_eq "restoring an active member -> 400" "$HTTP_CODE" "400" req POST "/api/public/members/$NEWMEMBER/restore" "$TOKEN" check_eq "restoring an invited member -> 400" "$HTTP_CODE" "400" +echo "" +echo "== A manage-all member survives a read-modify-write ==" + +# Vaultwarden stores manage-all as the access_all flag on a Manager. It is exposed as the +# custom role plus its three collection permissions, which is the only shape a client can +# read and send back without silently dropping the access. +PERMS='{"accessEventLogs":false,"accessImportExport":false,"accessReports":false,"createNewCollections":true,"editAnyCollection":true,"deleteAnyCollection":true,"manageGroups":false,"managePolicies":false,"manageSso":false,"manageUsers":false,"manageResetPassword":false,"manageScim":false}' + +reqj POST "/api/public/members" "$TOKEN" \ + "{\"email\":\"manageall@example.com\",\"type\":4,\"permissions\":$PERMS}" +check_eq "create a manage-all member -> 200" "$HTTP_CODE" "200" +MANAGEALL=$(jqval '.id') +jqcheck "manage-all member reports the custom role" '.type' "4" +jqcheck "manage-all member reports its permissions" '.permissions.editAnyCollection' "true" + +# Read it back and send exactly that back again, which is what a sync client does. +req GET "/api/public/members/$MANAGEALL" "$TOKEN" +jqcheck "manage-all member still reports the custom role" '.type' "4" +ROUNDTRIP=$(jq -c '{type: .type, externalId: .externalId, permissions: .permissions}' "$TMP/body") +reqj PUT "/api/public/members/$MANAGEALL" "$TOKEN" "$ROUNDTRIP" +check_eq "write the member back unchanged -> 200" "$HTTP_CODE" "200" + +req GET "/api/public/members/$MANAGEALL" "$TOKEN" +jqcheck "round-trip kept the custom role" '.type' "4" +jqcheck "round-trip kept manage-all" '.permissions.editAnyCollection' "true" + +# A plain member carries no permissions object. +req GET "/api/public/members/$MEMBER3" "$TOKEN" +jqcheck "a plain member has null permissions" '.permissions' "null" + +req DELETE "/api/public/members/$MANAGEALL" "$TOKEN" +check_eq "clean up the manage-all member -> 200" "$HTTP_CODE" "200" + echo "" echo "== Ownership is out of reach for a Public API client ==" diff --git a/src/api/core/public.rs b/src/api/core/public.rs index d790274f..310d6391 100644 --- a/src/api/core/public.rs +++ b/src/api/core/public.rs @@ -252,16 +252,43 @@ async fn member_to_json(member: &Membership, conn: &DbConn) -> Value { member.status }; + // HACK: Convert the manager type to a custom type, the same way the internal + // serializers do. Vaultwarden has no real custom role, it links the three collection + // permissions to the access_all flag instead, and the write endpoints read that flag + // back out of exactly this shape. Emitting both is what lets a client read a member, + // send it back unchanged, and keep its access. + let membership_type = member.type_manager_as_custom(); + let permissions = if membership_type == 4 && member.access_all { + json!({ + "accessEventLogs": false, + "accessImportExport": false, + "accessReports": false, + // If the following 3 Collection roles are set to true a custom user has access all permission + "createNewCollections": true, + "editAnyCollection": true, + "deleteAnyCollection": true, + "manageGroups": false, + "managePolicies": false, + "manageSso": false, // Not supported + "manageUsers": false, + "manageResetPassword": false, + "manageScim": false // Not supported (Not AGPLv3 Licensed) + }) + } else { + json!(null) + }; + json!({ "object": "member", "id": member.uuid, "userId": member.user_uuid, "name": name, "email": email, - "type": member.atype, + "type": membership_type, "externalId": member.external_id, "resetPasswordEnrolled": member.reset_password_key.is_some(), "status": status, + "permissions": permissions, }) } From c9c81f3b02e6cad9e3f7f598ea2ae4a7e21609b7 Mon Sep 17 00:00:00 2001 From: Rune Darrud Date: Mon, 10 Aug 2026 03:04:53 +0200 Subject: [PATCH 7/7] Fix defects found by a second adversarial review The member permissions object could not be written back. The read shape emits permissions null for anyone who is not a manage-all member, but the write models declared a plain map with serde(default), which only covers a missing field: an explicit null failed to deserialize and the request was rejected before the handler ran. Reading a member and sending it straight back therefore failed for every member except the manage-all case the previous test covered. The field is optional now, and the smoke test round-trips a plain member as well. Reinvite was the one member write that could still reach an owner, and it can change an owner from invited to accepted, so it takes the same guard as the rest. Group updates log before mutating, so a failure part way through the collection associations cannot leave an unaudited change. Co-Authored-By: Claude Opus 5 --- scripts/smoke_public_api_write.sh | 7 ++++++- src/api/core/public.rs | 20 ++++++++++++++------ 2 files changed, 20 insertions(+), 7 deletions(-) diff --git a/scripts/smoke_public_api_write.sh b/scripts/smoke_public_api_write.sh index 18540d5a..2f002931 100755 --- a/scripts/smoke_public_api_write.sh +++ b/scripts/smoke_public_api_write.sh @@ -421,9 +421,14 @@ req GET "/api/public/members/$MANAGEALL" "$TOKEN" jqcheck "round-trip kept the custom role" '.type' "4" jqcheck "round-trip kept manage-all" '.permissions.editAnyCollection' "true" -# A plain member carries no permissions object. +# A plain member carries no permissions object, and must still round-trip: the read shape +# has to be acceptable as a write body, null permissions and all. req GET "/api/public/members/$MEMBER3" "$TOKEN" +jqcheck "a plain member has a permissions key" 'has("permissions")' "true" jqcheck "a plain member has null permissions" '.permissions' "null" +PLAIN=$(jq -c '{type: .type, externalId: .externalId, permissions: .permissions, collections: []}' "$TMP/body") +reqj PUT "/api/public/members/$MEMBER3" "$TOKEN" "$PLAIN" +check_eq "a plain member round-trips too" "$HTTP_CODE" "200" req DELETE "/api/public/members/$MANAGEALL" "$TOKEN" check_eq "clean up the manage-all member -> 200" "$HTTP_CODE" "200" diff --git a/src/api/core/public.rs b/src/api/core/public.rs index 310d6391..ebd40e23 100644 --- a/src/api/core/public.rs +++ b/src/api/core/public.rs @@ -500,8 +500,10 @@ struct MemberCreateData { collections: Vec, #[serde(default)] groups: Vec, + // Our own read shape emits null here for anyone who is not a manage-all member, and a + // client is expected to send a read straight back, so null has to deserialize. #[serde(default)] - permissions: HashMap, + permissions: Option>, } #[derive(Deserialize)] @@ -514,8 +516,10 @@ struct MemberUpdateData { #[serde(default)] collections: Vec, groups: Option>, + // Our own read shape emits null here for anyone who is not a manage-all member, and a + // client is expected to send a read straight back, so null has to deserialize. #[serde(default)] - permissions: HashMap, + permissions: Option>, } #[derive(Deserialize)] @@ -672,7 +676,8 @@ async fn post_member(data: Json, token: PublicToken, ip: auth: let org_id = token.0; let data = data.into_inner(); - let Some((new_type, access_all)) = member_type_and_access_all(data.r#type, &data.permissions) else { + let permissions = data.permissions.unwrap_or_default(); + let Some((new_type, access_all)) = member_type_and_access_all(data.r#type, &permissions) else { err!("Invalid type") }; deny_owner_grant(new_type)?; @@ -764,7 +769,8 @@ async fn put_member( let org_id = token.0; let data = data.into_inner(); - let Some((new_type, access_all)) = member_type_and_access_all(data.r#type, &data.permissions) else { + let permissions = data.permissions.unwrap_or_default(); + let Some((new_type, access_all)) = member_type_and_access_all(data.r#type, &permissions) else { err!("Invalid type") }; deny_owner_grant(new_type)?; @@ -866,6 +872,8 @@ async fn post_member_reinvite(member_id: MembershipId, token: PublicToken, conn: err_code!(format!("Member {member_id} not found in organization"), 404); }; + deny_owner_target(&member)?; + if member.status != MembershipStatus::Invited as i32 { err!("The user is already accepted or confirmed to the organization") } @@ -1011,10 +1019,10 @@ async fn put_group( // Member assignments are owned by "/public/groups//member-ids" and are // deliberately left untouched here. - set_group_collections(&group, &data.collections, &org_id, &conn).await?; - log_public_event(EventType::GroupUpdated as i32, &group.uuid, &org_id, &ip.ip, &conn).await; + set_group_collections(&group, &data.collections, &org_id, &conn).await?; + Ok(Json(group_to_json(&group))) }