Radish alpha
h
Radicle Heartwood Protocol & Stack
Radicle
Git (anonymous pull)
Log in to clone via SSH
cli: extract document update logic
Fintan Halpenny committed 9 months ago
commit a69397386a9b14021f9859229eac2b83aade0139
parent 7f646666bc9b830ab79c717bf541a6c80383599e
5 files changed +318 -151
modified 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`:
modified 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<Self, Self::Err> {
-
        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::<Vec<_>>();
-
                    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::<Vec<_>>();
-
                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 verify_delegates<S>(
-
    proposal: &RawDoc,
-
    repo: &S,
-
) -> anyhow::Result<Option<Vec<VerificationError>>>
-
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,
-
                    });
-
                }
-
            }
+
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"
+
            ));
        }
    }
-

-
    Ok((dids.len() - missing.len() < threshold).then_some(missing))
}
modified crates/radicle/src/identity/doc.rs
@@ -1,3 +1,5 @@
+
pub mod update;
+

mod id;

use std::collections::{BTreeMap, BTreeSet};
added 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<Self, Self::Err> {
+
        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<Did>,
+
    disallow: BTreeSet<Did>,
+
) -> Result<RawDoc, error::PrivacyAllowList> {
+
    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::<Vec<_>>();
+
        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<S>(
+
    mut raw: RawDoc,
+
    additions: Vec<Did>,
+
    removals: Vec<Did>,
+
    repo: &S,
+
) -> Result<Result<RawDoc, Vec<error::DelegateVerification>>, 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::<Vec<_>>();
+
    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<RawDoc, error::PayloadError> {
+
    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<Doc, error::DocVerification> {
+
    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<S>(
+
    proposal: &RawDoc,
+
    repo: &S,
+
) -> Result<Option<Vec<error::DelegateVerification>>, 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))
+
}
added 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<String>),
+
    #[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 },
+
}