Skip to content

Commit

Permalink
Allow multiple permissions per grant/revoke entry
Browse files Browse the repository at this point in the history
  • Loading branch information
@lann
lann committed Aug 28, 2023
1 parent 8dd383e commit dcb1b11
Show file tree
Hide file tree
Showing 16 changed files with 154 additions and 119 deletions.
33 changes: 21 additions & 12 deletions crates/protocol/src/operator/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,11 +69,19 @@ impl TryFrom<protobuf::OperatorEntry> for model::OperatorEntry {
},
Contents::GrantFlat(grant_flat) => model::OperatorEntry::GrantFlat {
key: grant_flat.key.parse()?,
permission: grant_flat.permission.try_into()?,
permissions: grant_flat
.permissions
.into_iter()
.map(TryInto::try_into)
.collect::<Result<_, _>>()?,
},
Contents::RevokeFlat(revoke_flat) => model::OperatorEntry::RevokeFlat {
key_id: revoke_flat.key_id.into(),
permission: revoke_flat.permission.try_into()?,
permissions: revoke_flat
.permissions
.into_iter()
.map(TryInto::try_into)
.collect::<Result<_, _>>()?,
},
};
Ok(output)
Expand Down Expand Up @@ -140,18 +148,19 @@ impl<'a> From<&'a model::OperatorEntry> for protobuf::OperatorEntry {
key: key.to_string(),
hash_algorithm: hash_algorithm.to_string(),
}),
model::OperatorEntry::GrantFlat { key, permission } => {
model::OperatorEntry::GrantFlat { key, permissions } => {
Contents::GrantFlat(protobuf::OperatorGrantFlat {
key: key.to_string(),
permission: permission.into(),
})
}
model::OperatorEntry::RevokeFlat { key_id, permission } => {
Contents::RevokeFlat(protobuf::OperatorRevokeFlat {
key_id: key_id.to_string(),
permission: permission.into(),
permissions: permissions.iter().map(Into::into).collect(),
})
}
model::OperatorEntry::RevokeFlat {
key_id,
permissions,
} => Contents::RevokeFlat(protobuf::OperatorRevokeFlat {
key_id: key_id.to_string(),
permissions: permissions.iter().map(Into::into).collect(),
}),
};
let contents = Some(contents);
protobuf::OperatorEntry { contents }
Expand Down Expand Up @@ -193,11 +202,11 @@ mod tests {
},
model::OperatorEntry::GrantFlat {
key: bob_pub.clone(),
permission: model::Permission::Commit,
permissions: vec![model::Permission::Commit],
},
model::OperatorEntry::RevokeFlat {
key_id: bob_pub.fingerprint(),
permission: model::Permission::Commit,
permissions: vec![model::Permission::Commit],
},
],
};
Expand Down
4 changes: 2 additions & 2 deletions crates/protocol/src/operator/model.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -74,13 +74,13 @@ pub enum OperatorEntry {
/// The author of this entry must have the permission.
GrantFlat {
key: signing::PublicKey,
permission: Permission,
permissions: Vec<Permission>,
},
/// Remove a permission from a key.
/// The author of this entry must have the permission.
RevokeFlat {
key_id: signing::KeyID,
permission: Permission,
permissions: Vec<Permission>,
},
}

Expand Down
75 changes: 41 additions & 34 deletions crates/protocol/src/operator/state.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,7 @@ impl LogState {
) -> Result<(), ValidationError> {
for entry in entries {
if let Some(permission) = entry.required_permission() {
self.check_key_permission(signer_key_id, permission)?;
self.check_key_permissions(signer_key_id, &[permission])?;
}

// Process an init entry specially
Expand All @@ -254,12 +254,13 @@ impl LogState {

match entry {
model::OperatorEntry::Init { .. } => unreachable!(), // handled above
model::OperatorEntry::GrantFlat { key, permission } => {
self.validate_grant_entry(signer_key_id, key, *permission)?
}
model::OperatorEntry::RevokeFlat { key_id, permission } => {
self.validate_revoke_entry(signer_key_id, key_id, *permission)?
model::OperatorEntry::GrantFlat { key, permissions } => {
self.validate_grant_entry(signer_key_id, key, permissions)?
}
model::OperatorEntry::RevokeFlat {
key_id,
permissions,
} => self.validate_revoke_entry(signer_key_id, key_id, permissions)?,
}
}

Expand Down Expand Up @@ -293,17 +294,17 @@ impl LogState {
&mut self,
signer_key_id: &signing::KeyID,
key: &signing::PublicKey,
permission: model::Permission,
permissions: &[model::Permission],
) -> Result<(), ValidationError> {
// Check that the current key has the permission they're trying to grant
self.check_key_permission(signer_key_id, permission)?;
self.check_key_permissions(signer_key_id, permissions)?;

let grant_key_id = key.fingerprint();
self.keys.insert(grant_key_id.clone(), key.clone());
self.permissions
.entry(grant_key_id)
.or_default()
.insert(permission);
.extend(permissions);

Ok(())
}
Expand All @@ -312,40 +313,46 @@ impl LogState {
&mut self,
signer_key_id: &signing::KeyID,
key_id: &signing::KeyID,
permission: model::Permission,
permissions: &[model::Permission],
) -> Result<(), ValidationError> {
// Check that the current key has the permission they're trying to revoke
self.check_key_permission(signer_key_id, permission)?;

if let Some(set) = self.permissions.get_mut(key_id) {
if set.remove(&permission) {
return Ok(());
self.check_key_permissions(signer_key_id, permissions)?;

for permission in permissions {
if !self
.permissions
.get_mut(key_id)
.map(|set| set.remove(permission))
.unwrap_or(false)
{
return Err(ValidationError::PermissionNotFoundToRevoke {
permission: *permission,
key_id: key_id.clone(),
});
}
}

// Permission not found to remove
Err(ValidationError::PermissionNotFoundToRevoke {
permission,
key_id: key_id.clone(),
})
Ok(())
}

fn check_key_permission(
fn check_key_permissions(
&self,
key_id: &signing::KeyID,
permission: model::Permission,
permissions: &[model::Permission],
) -> Result<(), ValidationError> {
if let Some(available_permissions) = self.permissions.get(key_id) {
if available_permissions.contains(&permission) {
return Ok(());
for permission in permissions {
if !self
.permissions
.get(key_id)
.map(|p| p.contains(permission))
.unwrap_or(false)
{
return Err(ValidationError::UnauthorizedAction {
key_id: key_id.clone(),
needed_permission: *permission,
});
}
}

// Needed permission not found
Err(ValidationError::UnauthorizedAction {
key_id: key_id.clone(),
needed_permission: permission,
})
Ok(())
}

fn snapshot(&self) -> Snapshot {
Expand Down Expand Up @@ -488,12 +495,12 @@ mod tests {
// This entry is valid
model::OperatorEntry::GrantFlat {
key: bob_pub,
permission: model::Permission::Commit,
permissions: vec![model::Permission::Commit],
},
// This entry is not valid
model::OperatorEntry::RevokeFlat {
key_id: "not-valid".to_string().into(),
permission: model::Permission::Commit,
permissions: vec![model::Permission::Commit],
},
],
};
Expand Down
33 changes: 21 additions & 12 deletions crates/protocol/src/package/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,11 +69,19 @@ impl TryFrom<protobuf::PackageEntry> for model::PackageEntry {
},
Contents::GrantFlat(grant_flat) => model::PackageEntry::GrantFlat {
key: grant_flat.key.parse()?,
permission: grant_flat.permission.try_into()?,
permissions: grant_flat
.permissions
.into_iter()
.map(TryInto::try_into)
.collect::<Result<_, _>>()?,
},
Contents::RevokeFlat(revoke_flat) => model::PackageEntry::RevokeFlat {
key_id: revoke_flat.key_id.into(),
permission: revoke_flat.permission.try_into()?,
permissions: revoke_flat
.permissions
.into_iter()
.map(TryInto::try_into)
.collect::<Result<_, _>>()?,
},
Contents::Release(release) => model::PackageEntry::Release {
version: release
Expand Down Expand Up @@ -151,18 +159,19 @@ impl<'a> From<&'a model::PackageEntry> for protobuf::PackageEntry {
key: key.to_string(),
hash_algorithm: hash_algorithm.to_string(),
}),
model::PackageEntry::GrantFlat { key, permission } => {
model::PackageEntry::GrantFlat { key, permissions } => {
Contents::GrantFlat(protobuf::PackageGrantFlat {
key: key.to_string(),
permission: permission.into(),
})
}
model::PackageEntry::RevokeFlat { key_id, permission } => {
Contents::RevokeFlat(protobuf::PackageRevokeFlat {
key_id: key_id.to_string(),
permission: permission.into(),
permissions: permissions.iter().map(Into::into).collect(),
})
}
model::PackageEntry::RevokeFlat {
key_id,
permissions,
} => Contents::RevokeFlat(protobuf::PackageRevokeFlat {
key_id: key_id.to_string(),
permissions: permissions.iter().map(Into::into).collect(),
}),
model::PackageEntry::Release { version, content } => {
Contents::Release(protobuf::PackageRelease {
version: version.to_string(),
Expand Down Expand Up @@ -217,11 +226,11 @@ mod tests {
},
model::PackageEntry::GrantFlat {
key: bob_pub.clone(),
permission: model::Permission::Release,
permissions: vec![model::Permission::Release, model::Permission::Yank],
},
model::PackageEntry::RevokeFlat {
key_id: bob_pub.fingerprint(),
permission: model::Permission::Release,
permissions: vec![model::Permission::Release],
},
model::PackageEntry::Release {
version: Version::new(1, 0, 0),
Expand Down
4 changes: 2 additions & 2 deletions crates/protocol/src/package/model.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -81,13 +81,13 @@ pub enum PackageEntry {
/// The author of this entry must have the permission.
GrantFlat {
key: signing::PublicKey,
permission: Permission,
permissions: Vec<Permission>,
},
/// Remove a permission from a key.
/// The author of this entry must have the permission.
RevokeFlat {
key_id: signing::KeyID,
permission: Permission,
permissions: Vec<Permission>,
},
/// Release a version of a package.
/// The version must not have been released yet.
Expand Down
Loading

0 comments on commit dcb1b11

Please sign in to comment.