radicle: add new error type for unauthorized non-delegates

The previous error `UnexpectedState` was opaque, and ended up in a
confusing message when a user would try to edit the identity document
while not being a deleagte themselves.

Instead, provide a more specific error that mentions the `Did`
performing the action, and the action itself.

Note that the `UnexpectedState` variant was matched on during
evaluation, meaning that operations would still appear in the timeline
of the COB. With the change to the other variant, the timeline does
not record the operation anymore.
This commit is contained in:
Adrian Duke 2026-01-14 18:24:00 +00:00 committed by Fintan Halpenny
parent bd5436d791
commit 67a4a712e4
1 changed files with 23 additions and 3 deletions

View File

@ -129,6 +129,24 @@ pub enum ApplyError {
Git(#[from] git::raw::Error), Git(#[from] git::raw::Error),
#[error("identity document error: {0}")] #[error("identity document error: {0}")]
Doc(#[from] DocError), Doc(#[from] DocError),
#[error("{author} is not a delegate, and only delegates are allowed to {action}")]
NonDelegateUnauthorized { author: Did, action: String },
}
impl ApplyError {
fn non_delegate_unauthorized(author: Did, action: &Action) -> Self {
let action = match action {
Action::Revision { .. } => "create a revision",
Action::RevisionEdit { .. } => "edit a revision",
Action::RevisionAccept { .. } => "accept a revision",
Action::RevisionReject { .. } => "reject a revision",
Action::RevisionRedact { .. } => "redact a revision",
};
Self::NonDelegateUnauthorized {
author,
action: action.to_string(),
}
}
} }
/// Error updating or creating proposals. /// Error updating or creating proposals.
@ -444,8 +462,9 @@ impl Identity {
) -> Result<(), ApplyError> { ) -> Result<(), ApplyError> {
let current = self.current().clone(); let current = self.current().clone();
if !current.is_delegate(&author.into()) { let did = author.into();
return Err(ApplyError::UnexpectedState); if !current.is_delegate(&did) {
return Err(ApplyError::non_delegate_unauthorized(did, &action));
} }
match action { match action {
Action::RevisionAccept { Action::RevisionAccept {
@ -1396,6 +1415,7 @@ mod test {
.unwrap() .unwrap()
}); });
// Eve's revision is active. // Eve's revision is active.
assert_eq!(eve_identity.timeline, vec![a0, a1, a2, e1]);
assert!(eve_identity.revision(&e1).unwrap().is_active()); assert!(eve_identity.revision(&e1).unwrap().is_active());
// b1 (Accept "Remove Eve") 2/2 // b1 (Accept "Remove Eve") 2/2
@ -1411,7 +1431,7 @@ mod test {
eve_identity.reload().unwrap(); eve_identity.reload().unwrap();
// Now that Eve reloaded, since Bob's vote to remove Eve went through first (b1 < e1), // Now that Eve reloaded, since Bob's vote to remove Eve went through first (b1 < e1),
// her revision is no longer valid. // her revision is no longer valid.
assert_eq!(eve_identity.timeline, vec![a0, a1, a2, b1, e1]); assert_eq!(eve_identity.timeline, vec![a0, a1, a2, b1]);
assert_eq!(eve_identity.revision(&e1), None); assert_eq!(eve_identity.revision(&e1), None);
assert!(!eve_identity.is_delegate(&eve.signer.public_key().into())); assert!(!eve_identity.is_delegate(&eve.signer.public_key().into()));
} }