Skip to content

Commit

Permalink
revised namespace validation
Browse files Browse the repository at this point in the history
  • Loading branch information
calvinrp committed Nov 16, 2023
1 parent 20e4313 commit 411a9db
Show file tree
Hide file tree
Showing 7 changed files with 172 additions and 54 deletions.
19 changes: 16 additions & 3 deletions crates/api/src/v1/package.rs
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,9 @@ pub enum PackageError {
/// The provided package's namespace is imported from another registry.
#[error("namespace `{0}` is an imported namespace from another registry")]
NamespaceImported(String),
/// The provided package's namespace conflicts with an existing namespace where the name only differs by case.
#[error("namespace conflicts with existing namespace `{0}`; package namespaces must be unique in a case insensitive way")]
NamespaceConflict(String),
/// The provided package name conflicts with an existing package where the name only differs by case.
#[error("the package conflicts with existing package name `{0}`; package names must be unique in a case insensitive way")]
PackageNameConflict(PackageId),
Expand Down Expand Up @@ -157,7 +160,9 @@ impl PackageError {
// HTTP authentication.
Self::Unauthorized { .. } => 403,
Self::LogNotFound(_) | Self::RecordNotFound(_) | Self::NamespaceNotDefined(_) => 404,
Self::NamespaceImported(_) | Self::PackageNameConflict(_) => 409,
Self::NamespaceImported(_)
| Self::NamespaceConflict(_)
| Self::PackageNameConflict(_) => 409,
Self::RecordNotSourcing => 405,
Self::Rejection(_) => 422,
Self::NotSupported(_) => 501,
Expand All @@ -172,6 +177,7 @@ enum EntityType {
Log,
Record,
Namespace,
NamespaceImport,
Name,
}

Expand Down Expand Up @@ -243,10 +249,16 @@ impl Serialize for PackageError {
.serialize(serializer),
Self::NamespaceImported(namespace) => RawError::Conflict {
status: Status::<409>,
ty: EntityType::Namespace,
ty: EntityType::NamespaceImport,
id: Cow::Borrowed(namespace),
}
.serialize(serializer),
Self::NamespaceConflict(existing) => RawError::Conflict {
status: Status::<409>,
ty: EntityType::Namespace,
id: Cow::Borrowed(existing),
}
.serialize(serializer),
Self::PackageNameConflict(existing) => RawError::Conflict {
status: Status::<409>,
ty: EntityType::Name,
Expand Down Expand Up @@ -310,7 +322,8 @@ impl<'de> Deserialize<'de> for PackageError {
)),
},
RawError::Conflict { status: _, ty, id } => match ty {
EntityType::Namespace => Ok(Self::NamespaceImported(id.into_owned())),
EntityType::Namespace => Ok(Self::NamespaceConflict(id.into_owned())),
EntityType::NamespaceImport => Ok(Self::NamespaceImported(id.into_owned())),
EntityType::Name => Ok(Self::PackageNameConflict(
PackageId::new(id.into_owned()).unwrap(),
)),
Expand Down
171 changes: 129 additions & 42 deletions crates/protocol/src/operator/state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,10 +62,23 @@ pub enum ValidationError {
#[error("the namespace `{namespace}` is invalid; namespace must be a kebab case string")]
InvalidNamespace { namespace: String },

#[error("the namespace `{namespace}` conflicts with the existing namespace `{existing}`; namespace must be unique in a case insensitive way")]
NamespaceConflict { namespace: String, existing: String },

#[error("the namespace `{namespace}` is already defined and cannot be redefined")]
NamespaceAlreadyDefined { namespace: String },
}

/// The namespace definition.
#[derive(Clone, Debug, Serialize, Deserialize, PartialEq, Eq)]
#[serde(rename_all = "camelCase")]
struct NamespaceDefinition {
/// Case sensitive namespace name.
namespace: String,
/// Namespace state.
state: NamespaceState,
}

/// The namespace state for defining or importing from other registries.
#[derive(Clone, Debug, Serialize, Deserialize, PartialEq, Eq)]
#[serde(rename_all = "camelCase")]
Expand Down Expand Up @@ -110,9 +123,9 @@ pub struct LogState {
/// The keys known to the validator.
#[serde(skip_serializing_if = "IndexMap::is_empty")]
keys: IndexMap<signing::KeyID, signing::PublicKey>,
/// The lowercased namespaces known to the validator.
/// The namespaces known to the validator. The key is the lowercased namespace.
#[serde(skip_serializing_if = "IndexMap::is_empty")]
namespaces: IndexMap<String, NamespaceState>,
namespaces: IndexMap<String, NamespaceDefinition>,
}

impl LogState {
Expand Down Expand Up @@ -156,9 +169,21 @@ impl LogState {
self.keys.get(key_id)
}

/// Gets the namespace state. Expects the namespace to be lowercase.
pub fn namespace_state(&self, lowerspaced_namespace: &str) -> Option<&NamespaceState> {
self.namespaces.get(lowerspaced_namespace)
/// Gets the namespace state.
pub fn namespace_state(&self, namespace: &str) -> Result<Option<&NamespaceState>, &str> {
if let Some(def) = self.namespaces.get(&namespace.to_ascii_lowercase()) {
if def.namespace == namespace {
// namespace exact match, return namespace state
Ok(Some(&def.state))
} else {
// namespace matches a namespace but differ in a case sensitive way,
// so return error with existing namespace
Err(&def.namespace)
}
} else {
// namespace is not defined
Ok(None)
}
}

/// Checks the key has permission to sign checkpoints.
Expand Down Expand Up @@ -386,19 +411,39 @@ impl LogState {
namespace: &str,
state: NamespaceState,
) -> Result<(), ValidationError> {
let namespace = namespace.to_lowercase();

if !PackageId::is_valid_namespace(&namespace) {
return Err(ValidationError::InvalidNamespace { namespace });
}

if self.namespaces.contains_key(&namespace) {
return Err(ValidationError::NamespaceAlreadyDefined { namespace });
if !PackageId::is_valid_namespace(namespace) {
return Err(ValidationError::InvalidNamespace {
namespace: namespace.to_string(),
});
}

self.namespaces.insert(namespace, state);
let namespace_lowercase = namespace.to_ascii_lowercase();

if let Some(def) = self.namespaces.get(&namespace_lowercase) {
if namespace == def.namespace {
// namespace matches exactly
Err(ValidationError::NamespaceAlreadyDefined {
namespace: namespace.to_string(),
})
} else {
// namespace matches an existing namespace but differs in a case sensitive way
Err(ValidationError::NamespaceConflict {
namespace: namespace.to_string(),
existing: def.namespace.to_string(),
})
}
} else {
// namespace is not defined
self.namespaces.insert(
namespace_lowercase,
NamespaceDefinition {
namespace: namespace.to_string(),
state,
},
);

Ok(())
Ok(())
}
}

fn check_key_permissions(
Expand Down Expand Up @@ -650,45 +695,87 @@ mod tests {
)]),
keys: IndexMap::from([(alice_id, alice_pub)]),
namespaces: IndexMap::from([
("my-namespace".to_string(), NamespaceState::Defined),
(
"my-namespace".to_string(),
NamespaceDefinition {
namespace: "my-namespace".to_string(),
state: NamespaceState::Defined,
},
),
(
"imported-namespace".to_string(),
NamespaceState::Imported {
registry: "registry.example.com".to_string(),
NamespaceDefinition {
namespace: "imported-namespace".to_string(),
state: NamespaceState::Imported {
registry: "registry.example.com".to_string(),
},
},
),
]),
};

assert_eq!(validator, expected);

let record = model::OperatorRecord {
prev: Some(RecordId::operator_record::<Sha256>(&envelope)),
version: 0,
timestamp: SystemTime::now(),
entries: vec![
// This entry is valid
model::OperatorEntry::DefineNamespace {
namespace: "other-namespace".to_string(),
},
// This entry is not valid
model::OperatorEntry::ImportNamespace {
namespace: "my-NAMESPACE".to_string(),
registry: "registry.alternative.com".to_string(),
},
],
};
{
let record = model::OperatorRecord {
prev: Some(RecordId::operator_record::<Sha256>(&envelope)),
version: 0,
timestamp: SystemTime::now(),
entries: vec![
// This entry is valid
model::OperatorEntry::DefineNamespace {
namespace: "other-namespace".to_string(),
},
// This entry is not valid
model::OperatorEntry::ImportNamespace {
namespace: "my-namespace".to_string(),
registry: "registry.alternative.com".to_string(),
},
],
};

let envelope =
ProtoEnvelope::signed_contents(&alice_priv, record).expect("failed to sign envelope");
let envelope = ProtoEnvelope::signed_contents(&alice_priv, record)
.expect("failed to sign envelope");

// This validation should fail and the validator state should remain unchanged
match validator.validate(&envelope).unwrap_err() {
ValidationError::NamespaceAlreadyDefined { .. } => {}
_ => panic!("expected a different error"),
// This validation should fail and the validator state should remain unchanged
match validator.validate(&envelope).unwrap_err() {
ValidationError::NamespaceAlreadyDefined { .. } => {}
_ => panic!("expected a different error"),
}

// The validator should not have changed
assert_eq!(validator, expected);
}

// The validator should not have changed
assert_eq!(validator, expected);
{
let record = model::OperatorRecord {
prev: Some(RecordId::operator_record::<Sha256>(&envelope)),
version: 0,
timestamp: SystemTime::now(),
entries: vec![
// This entry is valid
model::OperatorEntry::DefineNamespace {
namespace: "other-namespace".to_string(),
},
// This entry is not valid
model::OperatorEntry::ImportNamespace {
namespace: "my-NAMESPACE".to_string(),
registry: "registry.alternative.com".to_string(),
},
],
};

let envelope = ProtoEnvelope::signed_contents(&alice_priv, record)
.expect("failed to sign envelope");

// This validation should fail and the validator state should remain unchanged
match validator.validate(&envelope).unwrap_err() {
ValidationError::NamespaceConflict { .. } => {}
_ => panic!("expected a different error"),
}

// The validator should not have changed
assert_eq!(validator, expected);
}
}
}
4 changes: 2 additions & 2 deletions crates/server/openapi.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -278,11 +278,11 @@ paths:
status:
type: integer
description: The HTTP status code for the error.
example: 404
example: 409
type:
type: string
description: The type of entity that was not found.
enum: [name, namespace]
enum: [name, namespace, namespaceImport]
example: namespace
id:
type: string
Expand Down
3 changes: 3 additions & 0 deletions crates/server/src/api/v1/package.rs
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,9 @@ impl From<DataStoreError> for PackageApiError {
PackageError::Unauthorized(e.to_string())
}
DataStoreError::PackageNamespaceNotDefined(id) => PackageError::NamespaceNotDefined(id),
DataStoreError::PackageNamespaceConflict { existing, .. } => {
PackageError::NamespaceConflict(existing)
}
DataStoreError::PackageNamespaceImported(id) => PackageError::NamespaceImported(id),
DataStoreError::PackageNameConflict { existing, .. } => {
PackageError::PackageNameConflict(existing)
Expand Down
12 changes: 9 additions & 3 deletions crates/server/src/datastore/memory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -731,21 +731,27 @@ impl DataStore for MemoryDataStore {
.get(operator_log_id)
.ok_or_else(|| DataStoreError::LogNotFound(operator_log_id.clone()))?
.validator
.namespace_state(&package_id.namespace().to_ascii_lowercase())
.namespace_state(package_id.namespace())
{
Some(state) => match state {
Ok(Some(state)) => match state {
operator::NamespaceState::Defined => {}
operator::NamespaceState::Imported { .. } => {
return Err(DataStoreError::PackageNamespaceImported(
package_id.namespace().to_string(),
))
}
},
None => {
Ok(None) => {
return Err(DataStoreError::PackageNamespaceNotDefined(
package_id.namespace().to_string(),
))
}
Err(existing_namespace) => {
return Err(DataStoreError::PackageNamespaceConflict {
namespace: package_id.namespace().to_string(),
existing: existing_namespace.to_string(),
})
}
}

// verify package name is unique in a case insensitive way
Expand Down
3 changes: 3 additions & 0 deletions crates/server/src/datastore/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,9 @@ pub enum DataStoreError {
existing: PackageId,
},

#[error("the package namespace `{namespace}` conflicts with existing namespace `{existing}`; package namespaces must be unique in a case insensitive way")]
PackageNamespaceConflict { namespace: String, existing: String },

#[error("the package namespace `{0}` is not defined")]
PackageNamespaceNotDefined(String),

Expand Down
14 changes: 10 additions & 4 deletions crates/server/src/datastore/postgres/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -976,20 +976,26 @@ impl DataStore for PostgresDataStore {
.ok_or_else(|| DataStoreError::LogNotFound(operator_log_id.clone()))?;

// verify namespace is defined and not imported
match validator.namespace_state(&package_id.namespace().to_ascii_lowercase()) {
Some(state) => match state {
match validator.namespace_state(package_id.namespace()) {
Ok(Some(state)) => match state {
operator::NamespaceState::Defined => {}
operator::NamespaceState::Imported { .. } => {
return Err(DataStoreError::PackageNamespaceImported(
package_id.namespace().to_string(),
));
))
}
},
None => {
Ok(None) => {
return Err(DataStoreError::PackageNamespaceNotDefined(
package_id.namespace().to_string(),
))
}
Err(existing_namespace) => {
return Err(DataStoreError::PackageNamespaceConflict {
namespace: package_id.namespace().to_string(),
existing: existing_namespace.to_string(),
})
}
}

// verify package name is unique in a case insensitive way
Expand Down

0 comments on commit 411a9db

Please sign in to comment.