mab122
/
radicle-heartwood-lfs
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

cli: improve rad id allow list UX 

The previous implementation of using the `--allow` option in `rad id
update` would be an absolute list, rather than additive. That is, if
there were existing DIDs not passed in via `--allow` then they would
be remove.

Change this behaviour to check the existing value of the document's
allow list, and add to it if `--allow` is specified.

To enable the user to also remove a DID, the `--disallow` option is
also added.

The correct usage of these options is improved by checking the
validity of the `--visibility` option being used, the current state of
the repository's privacy, and the use of the `--allow` and
`--disallow` options.

Signed-off-by: Fintan Halpenny <fintan.halpenny@gmail.com>
X-Clacks-Overhead: GNU Terry Pratchett
This commit is contained in:
Fintan Halpenny 2024-05-20 17:10:43 +01:00 committed by cloudhead
parent 6f91f2fb8f
commit f26be552c2
No known key found for this signature in database
3 changed files with 206 additions and 9 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

101
radicle-cli/examples/rad-id-private.md Normal file
View File

@ -0,0 +1,101 @@
When we are working with a private repository, we can modify the list of peers
we allow by using the `rad id` command with its `--allow` and `--disallow`
options. Both options can be specified multiple times in the same command line call:
Here we will add Bob and Eve's DIDs to the `allow`list:
```
$ rad id update --title "Allow Bob & Eve" --description "" --allow did:key:z6Mkt67GdsW7715MEfRuP4pSZxJRJh6kj6Y48WRqVv4N1tRk --allow did:key:z6Mkux1aUQD2voWWukVb5nNUR7thrHveQG4pDQua8nVhib7Z -q
...
$ rad inspect --identity
{
"payload": {
"xyz.radicle.project": {
"defaultBranch": "master",
"description": "radicle heartwood protocol & stack",
"name": "heartwood"
}
},
"delegates": [
"did:key:z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi"
],
"threshold": 1,
"visibility": {
"type": "private",
"allow": [
"did:key:z6Mkt67GdsW7715MEfRuP4pSZxJRJh6kj6Y48WRqVv4N1tRk",
"did:key:z6Mkux1aUQD2voWWukVb5nNUR7thrHveQG4pDQua8nVhib7Z"
]
}
}
```
To remove a peer's DID, we can use the `--disallow` option. Let's remove both of them again:
```
$ rad id update --title "Remove allow list" --description "" --disallow did:key:z6Mkt67GdsW7715MEfRuP4pSZxJRJh6kj6Y48WRqVv4N1tRk --disallow did:key:z6Mkux1aUQD2voWWukVb5nNUR7thrHveQG4pDQua8nVhib7Z
...
$ rad inspect --identity
{
"payload": {
"xyz.radicle.project": {
"defaultBranch": "master",
"description": "radicle heartwood protocol & stack",
"name": "heartwood"
}
},
"delegates": [
"did:key:z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi"
],
"threshold": 1,
"visibility": {
"type": "private"
}
}
```
Note that using both `--disallow` and `--allow` with the same DID will result in
an error:
``` (fails)
$ rad id update --title "Remove allow list" --description "" --allow did:key:z6Mkt67GdsW7715MEfRuP4pSZxJRJh6kj6Y48WRqVv4N1tRk --disallow did:key:z6Mkt67GdsW7715MEfRuP4pSZxJRJh6kj6Y48WRqVv4N1tRk
✗ Error: `--allow` and `--disallow` must not overlap: ["did:key:z6Mkt67GdsW7715MEfRuP4pSZxJRJh6kj6Y48WRqVv4N1tRk"]
```
Allowing or disallowing the same peer twice will result in an error the second
call, since there is no update specified:
```
$ rad id update --title "Allow Bob" --description "" --allow did:key:z6Mkt67GdsW7715MEfRuP4pSZxJRJh6kj6Y48WRqVv4N1tRk -q
...
```
``` (fails)
$ rad id update --title "Allow Bob" --description "" --allow did:key:z6Mkt67GdsW7715MEfRuP4pSZxJRJh6kj6Y48WRqVv4N1tRk -q
✗ Error: no update specified
✗ Hint: an update to the identity must be specified, run `rad id update -h` to see the available options
```
If we attempt to change the list while also changing the repository to `public`,
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`
```
Let's change the repository to `public`:
```
$ rad id update --title "IPO" --description "" --visibility public -q
...
```
Now, if we attempt to change the `allow` list we also get an error with a
helpful hint:
``` (fails)
$ rad id update --allow did:key:z6Mkt67GdsW7715MEfRuP4pSZxJRJh6kj6Y48WRqVv4N1tRk
✗ 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`
```

90
radicle-cli/src/commands/id.rs
View File

@ -1,3 +1,5 @@
use std::collections::BTreeSet;
use std::str::FromStr;
use std::{ffi::OsString, io};
use anyhow::{anyhow, Context};
@ -32,7 +34,8 @@ Usage
rad id update [--title <string>] [--description <string>]
[--delegate <did>] [--rescind <did>]
[--threshold <num>] [--visibility <private | public>]
[--allow <did>] [--no-confirm] [--payload <id> <key> <val>...]
[--allow <did>] [--disallow <did>]
[--no-confirm] [--payload <id> <key> <val>...]
[<option>...]
rad id edit <revision-id> [--title <string>] [--description <string>] [<option>...]
rad id show <revision-id> [<option>...]
@ -59,7 +62,9 @@ pub enum Operation {
delegate: Vec<Did>,
rescind: Vec<Did>,
threshold: Option<usize>,
visibility: Option<Visibility>,
visibility: Option<EditVisibility>,
allow: BTreeSet<Did>,
disallow: BTreeSet<Did>,
payload: Vec<(doc::PayloadId, String, json::Value)>,
},
AcceptRevision {
@ -83,6 +88,29 @@ 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,
@ -114,7 +142,9 @@ impl Args for Options {
let mut description: Option<String> = None;
let mut delegate: Vec<Did> = Vec::new();
let mut rescind: Vec<Did> = Vec::new();
let mut visibility: Option<Visibility> = None;
let mut visibility: Option<EditVisibility> = None;
let mut allow: BTreeSet<Did> = BTreeSet::new();
let mut disallow: BTreeSet<Did> = BTreeSet::new();
let mut threshold: Option<usize> = None;
let mut interactive = Interactive::new(io::stdout());
let mut payload = Vec::new();
@ -172,11 +202,12 @@ impl Args for Options {
Long("allow") => {
let value = parser.value()?;
let did = term::args::did(&value)?;
if let Some(Visibility::Private { allow }) = &mut visibility {
allow.insert(did);
} else {
visibility = Some(Visibility::private([did]));
}
allow.insert(did);
}
Long("disallow") => {
let value = parser.value()?;
let did = term::args::did(&value)?;
disallow.insert(did);
}
Long("visibility") => {
let value = parser.value()?;
@ -244,6 +275,8 @@ impl Args for Options {
rescind,
threshold,
visibility,
allow,
disallow,
payload,
},
};
@ -349,12 +382,51 @@ pub fn run(options: Options, ctx: impl term::Context) -> anyhow::Result<()> {
rescind,
threshold,
visibility,
allow,
disallow,
payload,
} => {
let proposal = {
let mut proposal = current.doc.clone();
proposal.threshold = threshold.unwrap_or(proposal.threshold);
proposal.visibility = visibility.unwrap_or(proposal.visibility);
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 {
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);
}
}
(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 = NonEmpty::from_vec(
proposal
.delegates

24
radicle-cli/tests/commands.rs
View File

@ -716,6 +716,30 @@ fn rad_id_unknown_field() {
.unwrap();
}
#[test]
fn rad_id_private() {
let mut environment = Environment::new();
let alice = environment.node(Config::test(Alias::new("alice")));
let working = environment.tmp().join("working");
fixtures::repository(working.join("alice"));
test(
"examples/rad-init-private.md",
working.join("alice"),
Some(&alice.home),
[],
)
.unwrap();
test(
"examples/rad-id-private.md",
working.join("alice"),
Some(&alice.home),
[],
)
.unwrap();
}
#[test]
fn rad_node_connect() {
let mut environment = Environment::new();