diff --git a/crates/radicle-cli/examples/rad-id-private.md b/crates/radicle-cli/examples/rad-id-private.md index 63998838..711ee25e 100644 --- a/crates/radicle-cli/examples/rad-id-private.md +++ b/crates/radicle-cli/examples/rad-id-private.md @@ -79,7 +79,8 @@ then the command will fail since there is no longer an allow list to work with: ``` (fails) $ rad id update --visibility public --allow did:key:z6Mkt67GdsW7715MEfRuP4pSZxJRJh6kj6Y48WRqVv4N1tRk -✗ Error: `--allow` and `--disallow` cannot be used with `--visibility public` +✗ Error: `--allow` and `--disallow` should only be used for private repositories +✗ Hint: use `--visibility private` to make the repository private, or perhaps you meant to use `--delegate`/`--rescind` ``` Let's change the repository to `public`: diff --git a/crates/radicle-cli/src/commands/id.rs b/crates/radicle-cli/src/commands/id.rs index 3a0f4dc5..2690454e 100644 --- a/crates/radicle-cli/src/commands/id.rs +++ b/crates/radicle-cli/src/commands/id.rs @@ -1,16 +1,16 @@ use std::collections::BTreeSet; -use std::str::FromStr; use std::{ffi::OsString, io}; use anyhow::{anyhow, Context}; use radicle::cob::identity::{self, IdentityMut, Revision, RevisionId}; -use radicle::identity::{doc, Doc, Identity, PayloadError, RawDoc, Visibility}; +use radicle::identity::doc::update; +use radicle::identity::doc::update::EditVisibility; +use radicle::identity::{doc, Doc, Identity, RawDoc}; use radicle::node::device::Device; use radicle::node::NodeId; use radicle::prelude::{Did, RepoId}; -use radicle::storage::refs; -use radicle::storage::{ReadRepository, ReadStorage as _, WriteRepository}; +use radicle::storage::{ReadStorage as _, WriteRepository}; use radicle::{cob, crypto, Profile}; use radicle_surf::diff::Diff; use radicle_term::Element; @@ -88,29 +88,6 @@ pub enum Operation { ListRevisions, } -#[derive(Clone, Copy, Debug, Default, PartialEq, Eq)] -pub enum EditVisibility { - #[default] - Public, - Private, -} - -#[derive(thiserror::Error, Debug)] -#[error("'{0}' is not a valid visibility type")] -pub struct EditVisibilityParseError(String); - -impl FromStr for EditVisibility { - type Err = EditVisibilityParseError; - - fn from_str(s: &str) -> Result { - match s { - "public" => Ok(EditVisibility::Public), - "private" => Ok(EditVisibility::Private), - _ => Err(EditVisibilityParseError(s.to_owned())), - } - } -} - #[derive(Default, PartialEq, Eq)] pub enum OperationName { Accept, @@ -398,76 +375,38 @@ pub fn run(options: Options, ctx: impl term::Context) -> anyhow::Result<()> { let mut proposal = current.doc.clone().edit(); proposal.threshold = threshold.unwrap_or(proposal.threshold); - if !allow.is_disjoint(&disallow) { - let overlap = allow - .intersection(&disallow) - .map(Did::to_string) - .collect::>(); - anyhow::bail!("`--allow` and `--disallow` must not overlap: {overlap:?}") - } - - match (&mut proposal.visibility, visibility) { - (Visibility::Public, None | Some(EditVisibility::Public)) if !allow.is_empty() || !disallow.is_empty() => { - return Err(Error::WithHint { + let proposal = match visibility { + Some(edit) => update::visibility(proposal, edit), + None => proposal, + }; + let proposal = match update::privacy_allow_list(proposal, allow, disallow) { + Ok(proposal) => proposal, + Err(e) => match e { + update::error::PrivacyAllowList::Overlapping(overlap) => anyhow::bail!("`--allow` and `--disallow` must not overlap: {overlap:?}"), + update::error::PrivacyAllowList::PublicVisibility => return Err(Error::WithHint { err: anyhow!("`--allow` and `--disallow` should only be used for private repositories"), hint: "use `--visibility private` to make the repository private, or perhaps you meant to use `--delegate`/`--rescind`", }.into()) } - (Visibility::Public, None | Some(EditVisibility::Public)) => { /* no-op */ }, - (Visibility::Private { allow: existing }, None | Some(EditVisibility::Private)) => { - for did in allow { - existing.insert(did); - } - for did in disallow { - existing.remove(&did); + }; + let threshold = proposal.threshold; + let proposal = match update::delegates(proposal, delegates, rescind, &repo)? { + Ok(proposal) => proposal, + Err(errs) => { + term::error(format!("failed to verify delegates for {rid}")); + term::error(format!( + "the threshold of {} delegates cannot be met..", + threshold + )); + for e in errs { + print_delegate_verification_error(&e); } + anyhow::bail!("fatal: refusing to update identity document"); } - (Visibility::Public, Some(EditVisibility::Private)) => { - // We ignore disallow since only allowing matters and the sets are disjoint. - proposal.visibility = Visibility::Private { allow }; - } - (Visibility::Private { .. }, Some(EditVisibility::Public)) if !allow.is_empty() || !disallow.is_empty() => { - anyhow::bail!("`--allow` and `--disallow` cannot be used with `--visibility public`") - } - (Visibility::Private { .. }, Some(EditVisibility::Public)) => { - proposal.visibility = Visibility::Public; - } - } - proposal.delegates = proposal - .delegates - .into_iter() - .chain(delegates) - .filter(|d| !rescind.contains(d)) - .collect::>(); - if let Some(errs) = verify_delegates(&proposal, &repo)? { - term::error(format!("failed to verify delegates for {rid}")); - term::error(format!( - "the threshold of {} delegates cannot be met..", - proposal.threshold - )); - for e in errs { - e.print(); - } - anyhow::bail!("fatal: refusing to update identity document"); - } + }; - for (id, key, val) in payload { - if let Some(ref mut payload) = proposal.payload.get_mut(&id) { - if let Some(obj) = payload.as_object_mut() { - if val.is_null() { - obj.remove(&key); - } else { - obj.insert(key, val); - } - } else { - anyhow::bail!("payload `{id}` is not a map"); - } - } else { - anyhow::bail!("payload `{id}` not found in identity document"); - } - } - proposal + update::payload(proposal, payload)? }; // If `--edit` is specified, the document can also be edited via a text edit. @@ -489,11 +428,7 @@ pub fn run(options: Options, ctx: impl term::Context) -> anyhow::Result<()> { proposal }; - // Verify that the project payload can still be parsed into the `Project` type. - if let Err(PayloadError::Json(e)) = proposal.project() { - anyhow::bail!("failed to verify `xyz.radicle.project`, {e}"); - } - let proposal = proposal.verified()?; + let proposal = update::verify(proposal)?; if proposal == current.doc { if !options.quiet { term::print(term::format::italic( @@ -793,62 +728,19 @@ fn print_diff( Ok(()) } -#[derive(Clone)] -enum VerificationError { - MissingDefaultBranch { - branch: radicle::git::RefString, - did: Did, - }, - MissingDelegate { - did: Did, - }, -} - -impl VerificationError { - fn print(&self) { - match self { - VerificationError::MissingDefaultBranch { branch, did } => term::error(format!( - "missing {} for {} in local storage", - term::format::secondary(branch), - term::format::did(did) - )), - VerificationError::MissingDelegate { did } => { - term::error(format!("the delegate {did} is missing")); - term::hint(format!( - "run `rad follow {did}` to follow this missing peer" - )); - } +fn print_delegate_verification_error(err: &update::error::DelegateVerification) { + use update::error::DelegateVerification::*; + match err { + MissingDefaultBranch { branch, did } => term::error(format!( + "missing {} for {} in local storage", + term::format::secondary(branch), + term::format::did(did) + )), + MissingDelegate { did } => { + term::error(format!("the delegate {did} is missing")); + term::hint(format!( + "run `rad follow {did}` to follow this missing peer" + )); } } } - -fn verify_delegates( - proposal: &RawDoc, - repo: &S, -) -> anyhow::Result>> -where - S: ReadRepository, -{ - let dids = &proposal.delegates; - let threshold = proposal.threshold; - let (canonical, _) = repo.canonical_head()?; - let mut missing = Vec::with_capacity(dids.len()); - - for did in dids { - match refs::SignedRefsAt::load((*did).into(), repo)? { - None => { - missing.push(VerificationError::MissingDelegate { did: *did }); - } - Some(refs::SignedRefsAt { sigrefs, .. }) => { - if sigrefs.get(&canonical).is_none() { - missing.push(VerificationError::MissingDefaultBranch { - branch: canonical.to_ref_string(), - did: *did, - }); - } - } - } - } - - Ok((dids.len() - missing.len() < threshold).then_some(missing)) -} diff --git a/crates/radicle/src/identity/doc.rs b/crates/radicle/src/identity/doc.rs index b3f4c1b3..169876ae 100644 --- a/crates/radicle/src/identity/doc.rs +++ b/crates/radicle/src/identity/doc.rs @@ -1,3 +1,5 @@ +pub mod update; + mod id; use std::collections::{BTreeMap, BTreeSet}; diff --git a/crates/radicle/src/identity/doc/update.rs b/crates/radicle/src/identity/doc/update.rs new file mode 100644 index 00000000..14662319 --- /dev/null +++ b/crates/radicle/src/identity/doc/update.rs @@ -0,0 +1,235 @@ +pub mod error; + +use std::{collections::BTreeSet, str::FromStr}; + +use serde_json as json; + +use crate::{ + identity::crefs::GetCanonicalRefs as _, + prelude::Did, + storage::{refs, ReadRepository, RepositoryError}, +}; + +use super::{Doc, PayloadId, RawDoc, Visibility}; + +/// [`EditVisibility`] allows the visibility of a [`RawDoc`] to be edited using +/// the [`visibility`] function. +/// +/// Note that this differs from [`Visibility`] since the +/// [`EditVisibility::Private`] variant does not hold the allowed set of +/// [`Did`]s. +#[derive(Clone, Copy, Debug, Default, PartialEq, Eq)] +pub enum EditVisibility { + #[default] + Public, + Private, +} + +impl FromStr for EditVisibility { + type Err = error::ParseEditVisibility; + + fn from_str(s: &str) -> Result { + match s { + "public" => Ok(EditVisibility::Public), + "private" => Ok(EditVisibility::Private), + _ => Err(error::ParseEditVisibility(s.to_owned())), + } + } +} + +/// Change the visibility of the [`RawDoc`], using the provided +/// [`EditVisibility`]. +pub fn visibility(mut raw: RawDoc, edit: EditVisibility) -> RawDoc { + match (&mut raw.visibility, edit) { + (Visibility::Public, EditVisibility::Public) => raw, + (Visibility::Private { .. }, EditVisibility::Private) => raw, + (Visibility::Public, EditVisibility::Private) => { + raw.visibility = Visibility::private([]); + raw + } + (Visibility::Private { .. }, EditVisibility::Public) => { + raw.visibility = Visibility::Public; + raw + } + } +} + +/// Change the `allow` set of a document if the visibility is set to +/// [`Visibility::Private`]. +/// +/// All `Did`s in the `allow` set are added to the set, while all `Did`s in the +/// `disallow` set are removed from the set. +/// +/// # Errors +/// +/// This will fail when `allow` and `disallow` are not disjoint, i.e. they +/// contain at least share one `Did`. +/// +/// This will fail when the [`Visibility`] of the document is +/// [`Visibility::Public`]. +pub fn privacy_allow_list( + mut raw: RawDoc, + allow: BTreeSet, + disallow: BTreeSet, +) -> Result { + if allow.is_empty() && disallow.is_empty() { + return Ok(raw); + } + + if !allow.is_disjoint(&disallow) { + let overlap = allow + .intersection(&disallow) + .map(Did::to_string) + .collect::>(); + return Err(error::PrivacyAllowList::Overlapping(overlap)); + } + + match &mut raw.visibility { + Visibility::Public => Err(error::PrivacyAllowList::PublicVisibility), + Visibility::Private { allow: existing } => { + for did in allow { + existing.insert(did); + } + for did in disallow { + existing.remove(&did); + } + Ok(raw) + } + } +} + +/// Change the delegates of the document and perform some verification based on +/// the new set of delegates. +/// +/// The set of `additions` are added to the delegates, while the set to +/// `removals` are removed from the delegates. Note that `removals` will take +/// precedence over the additions, i.e. if an addition and removal overlap, then +/// the [`Did`] will not be in the final set. +/// +/// The result is either the updated [`RawDoc`] or a set of +/// [`error::DelegateVerification`] errors – which may be reported by the caller +/// to provide better error messaging. +/// +/// # Errors +/// +/// This will fail if an operation using the repository fails. +pub fn delegates( + mut raw: RawDoc, + additions: Vec, + removals: Vec, + repo: &S, +) -> Result>, RepositoryError> +where + S: ReadRepository, +{ + if additions.is_empty() && removals.is_empty() { + return Ok(Ok(raw)); + } + + raw.delegates = raw + .delegates + .into_iter() + .chain(additions) + .filter(|d| !removals.contains(d)) + .collect::>(); + match verify_delegates(&raw, repo)? { + Some(errors) => Ok(Err(errors)), + None => Ok(Ok(raw)), + } +} + +// TODO(finto): I think this API would likely be much nicer if we use [JSON Patch][patch] and [JSON Merge Patch][merge] +// +// [patch]: https://datatracker.ietf.org/doc/html/rfc6902 +// [merge]: https://datatracker.ietf.org/doc/html/rfc7396 +/// Change the payload of the document, using the set of triples: +/// +/// - [`PayloadId`]: the identifier for the document [`Payload`] +/// - [`String`]: the key within the [`Payload`] that is being updated +/// - [`json::Value`]: the value to update the [`Payload`] +/// +/// # Errors +/// +/// This fails if one of the [`PayloadId`]s does not point to a JSON object as +/// its value. +/// +/// [`Payload`]: super::Payload +pub fn payload( + mut raw: RawDoc, + payload: Vec<(PayloadId, String, json::Value)>, +) -> Result { + for (id, key, val) in payload { + if let Some(ref mut payload) = raw.payload.get_mut(&id) { + if let Some(obj) = payload.as_object_mut() { + if val.is_null() { + obj.remove(&key); + } else { + obj.insert(key, val); + } + } else { + return Err(error::PayloadError::ExpectedObject { id }); + } + } else { + raw.payload + .insert(id, serde_json::json!({ key: val }).into()); + } + } + Ok(raw) +} + +/// Verify the document. +/// +/// This ensures performs the verification of the [`RawDoc`] into the [`Doc`], +/// while also checking the [`Project`] and [`CanonicalRefs`] will also +/// deserialize correctly. +/// +/// [`Project`]: crate::identity::Project +/// [`CanonicalRefs`]: crate::identity::CanonicalRefs +pub fn verify(raw: RawDoc) -> Result { + let proposal = raw.verified()?; + // Verify that the payloads can still be parsed into the correct types. + if let Err(super::PayloadError::Json(e)) = proposal.project() { + return Err(error::DocVerification::PayloadJson { + id: PayloadId::project(), + err: e, + }); + } + if let Err(super::PayloadError::Json(e)) = proposal.canonical_refs() { + return Err(error::DocVerification::PayloadJson { + id: PayloadId::canonical_refs(), + err: e, + }); + } + Ok(proposal) +} + +fn verify_delegates( + proposal: &RawDoc, + repo: &S, +) -> Result>, RepositoryError> +where + S: ReadRepository, +{ + let dids = &proposal.delegates; + let threshold = proposal.threshold; + let (canonical, _) = repo.canonical_head()?; + let mut missing = Vec::with_capacity(dids.len()); + + for did in dids { + match refs::SignedRefsAt::load((*did).into(), repo)? { + None => { + missing.push(error::DelegateVerification::MissingDelegate { did: *did }); + } + Some(refs::SignedRefsAt { sigrefs, .. }) => { + if sigrefs.get(&canonical).is_none() { + missing.push(error::DelegateVerification::MissingDefaultBranch { + branch: canonical.to_ref_string(), + did: *did, + }); + } + } + } + } + + Ok((dids.len() - missing.len() < threshold).then_some(missing)) +} diff --git a/crates/radicle/src/identity/doc/update/error.rs b/crates/radicle/src/identity/doc/update/error.rs new file mode 100644 index 00000000..02ebd910 --- /dev/null +++ b/crates/radicle/src/identity/doc/update/error.rs @@ -0,0 +1,37 @@ +use serde_json as json; +use thiserror::Error; + +use crate::git::RefString; +use crate::identity::{doc::PayloadId, Did, DocError}; + +#[derive(Debug, Error)] +#[error("'{0}' is not a valid visibility type")] +pub struct ParseEditVisibility(pub(super) String); + +#[derive(Debug, Error)] +pub enum PrivacyAllowList { + #[error("overlapping allow and disallow of DIDs {0:?}")] + Overlapping(Vec), + #[error("the visibility of the document is public")] + PublicVisibility, +} + +#[derive(Debug, Error)] +pub enum PayloadError { + #[error("payload found under `{id}` is expected to be a map")] + ExpectedObject { id: PayloadId }, +} + +#[derive(Debug, Error)] +pub enum DocVerification { + #[error("failed to verify `{id}`, {err}")] + PayloadJson { id: PayloadId, err: json::Error }, + #[error(transparent)] + Doc(#[from] DocError), +} + +#[derive(Clone, Debug)] +pub enum DelegateVerification { + MissingDefaultBranch { branch: RefString, did: Did }, + MissingDelegate { did: Did }, +}