#556 constrain URLs

Author: joepio

CHANGELOG.md

@@ -23,6 +23,8 @@ See [STATUS.md](server/STATUS.md) to learn more about which features will remain
 - Update JS assets & playwright
 - Fix initial indexing bug #560
 - Improve check_append error #558
+- Most Collection routes now live under `/collections`, e.g. `/collections/agents` instead of `/agents`. #556
+- Constrain new URLs. Commits for new Resources are now only valid if their parent is part of the current URL. So it's no longer possible to create `/some-path/new-resource` if `new-resource` is its parent is not in its URL. #556
 
 ## [v0.34.0] - 2022-10-31
 

cli/src/new.rs

@@ -173,7 +173,7 @@ fn prompt_field(
         }
         DataType::Date => {
             let msg = format!("date YYYY-MM-DD{}", msg_appendix);
-            let date: Option<String> = prompt_opt(&msg).unwrap();
+            let date: Option<String> = prompt_opt(msg).unwrap();
             let re = Regex::new(atomic_lib::values::DATE_REGEX).unwrap();
             match date {
                 Some(date_val) => {
@@ -259,7 +259,7 @@ fn prompt_field(
         },
         DataType::Timestamp => {
             let msg = format!("timestamp{}", msg_appendix);
-            let number: Option<u64> = prompt_opt(&msg)?;
+            let number: Option<u64> = prompt_opt(msg)?;
             match number {
                 Some(nr) => {
                     input = Some(nr.to_string());
@@ -272,7 +272,7 @@ fn prompt_field(
                 "unsupported datatype {}, defaulting to string{}",
                 unsup, msg_appendix
             );
-            let string: Option<String> = prompt_opt(&msg)?;
+            let string: Option<String> = prompt_opt(msg)?;
             match string {
                 Some(nr) => {
                     input = Some(nr);

lib/src/atomic_url.rs

@@ -36,9 +36,9 @@ impl AtomicUrl {
     pub fn set_route(&self, route: Routes) -> Self {
         let path = match route {
             Routes::AllVersions => "/all-versions".to_string(),
-            Routes::Agents => "/agents".to_string(),
+            Routes::Agents => "/collections/agents".to_string(),
             Routes::Collections => "/collections".to_string(),
-            Routes::Commits => "/commits".to_string(),
+            Routes::Commits => "/collections/commits".to_string(),
             Routes::CommitsUnsigned => "/commits-unsigned".to_string(),
             Routes::Endpoints => "/endpoints".to_string(),
             Routes::Import => "/import".to_string(),

lib/src/collections.rs

@@ -396,14 +396,24 @@ pub fn create_collection_resource_for_class(
 ) -> AtomicResult<Resource> {
     let class = store.get_class(class_subject)?;
 
+    // We use the `Collections` collection as the parent for all collections.
+    // This also influences their URLs.
+    let is_collections_collection = class.subject == urls::COLLECTION;
+
     // Pluralize the shortname
     let pluralized = match class.shortname.as_ref() {
         "class" => "classes".to_string(),
         "property" => "properties".to_string(),
         other => format!("{}s", other),
     };
 
-    let mut collection = CollectionBuilder::class_collection(&class.subject, &pluralized, store);
+    let path = if is_collections_collection {
+        "/collections".to_string()
+    } else {
+        format!("/collections/{}", pluralized)
+    };
+
+    let mut collection = CollectionBuilder::class_collection(&class.subject, &path, store);
 
     collection.sort_by = match class_subject {
         urls::COMMIT => Some(urls::CREATED_AT.to_string()),
@@ -424,7 +434,7 @@ pub fn create_collection_resource_for_class(
         .ok_or("No self_url present in store, can't populate collections")?;
 
     // Let the Collections collection be the top level item
-    let parent = if class.subject == urls::COLLECTION {
+    let parent = if is_collections_collection {
         drive.to_string()
     } else {
         drive

lib/src/commit.rs

@@ -39,6 +39,8 @@ pub struct CommitOpts {
     pub update_index: bool,
     /// For who the right checks will be perormed. If empty, the signer of the Commit will be used.
     pub validate_for_agent: Option<String>,
+    /// Checks if the URL of the parent is present in its Parent URL.
+    pub validate_subject_url_parent: bool,
 }
 
 /// A Commit is a set of changes to a Resource.
@@ -160,6 +162,20 @@ impl Commit {
             .apply_changes(resource_old.clone(), store, false)
             .map_err(|e| format!("Error applying changes to Resource {}. {}", self.subject, e))?;
 
+        // For new subjects, make sure that the parent of the resource is part of the URL of the new subject.
+        if is_new && opts.validate_subject_url_parent {
+            if let Ok(parent) = resource_new.get(urls::PARENT) {
+                let parent_str = parent.to_string();
+                if !self.subject.starts_with(&parent_str) {
+                    return Err(format!(
+                        "The parent '{}' is not part of the URL of the new subject '{}'.",
+                        parent_str, self.subject
+                    )
+                    .into());
+                }
+            }
+        }
+
         if opts.validate_rights {
             let validate_for = opts.validate_for_agent.as_ref().unwrap_or(&self.signer);
             if is_new {
@@ -374,6 +390,7 @@ impl Commit {
             validate_signature: false,
             validate_timestamp: false,
             validate_rights: false,
+            validate_subject_url_parent: false,
             validate_previous_commit: false,
             validate_for_agent: None,
             update_index: false,
@@ -694,6 +711,7 @@ mod test {
             validate_previous_commit: true,
             validate_rights: false,
             validate_for_agent: None,
+            validate_subject_url_parent: true,
             update_index: true,
         };
     }

lib/src/db/test.rs

@@ -187,7 +187,12 @@ fn destroy_resource_and_check_collection_and_commits() {
 #[test]
 fn get_extended_resource_pagination() {
     let store = Db::init_temp("get_extended_resource_pagination").unwrap();
-    let subject = format!("{}commits?current_page=2", store.get_server_url());
+    let num = 3;
+    let subject = format!(
+        "{}collections/commits?current_page={}",
+        store.get_server_url(),
+        num
+    );
     // Should throw, because page 2 is out of bounds for default page size
     let _wrong_resource = store
         .get_resource_extended(&subject, false, None)
@@ -202,7 +207,7 @@ fn get_extended_resource_pagination() {
         .unwrap()
         .to_int()
         .unwrap();
-    assert_eq!(cur_page, 2);
+    assert_eq!(cur_page, num);
     assert_eq!(resource.get_subject(), &subject_with_page_size);
 }
 

lib/src/parse.rs

@@ -338,6 +338,7 @@ fn parse_json_ad_map_to_resource(
                     validate_rights: parse_opts.for_agent.is_some(),
                     validate_previous_commit: false,
                     validate_for_agent: parse_opts.for_agent.clone(),
+                    validate_subject_url_parent: true,
                     update_index: true,
                 };
 
@@ -553,8 +554,8 @@ mod test {
             .import(include_str!("../test_files/local_id.json"), &parse_opts)
             .unwrap();
 
-        let reference_subject = generate_id_from_local_id(&importer, "reference");
-        let my_subject = generate_id_from_local_id(&importer, "my-local-id");
+        let reference_subject = generate_id_from_local_id(&importer, "parent");
+        let my_subject = generate_id_from_local_id(&importer, "parent/my-local-id");
         let found = store.get_resource(&my_subject).unwrap();
         let found_ref = store.get_resource(&reference_subject).unwrap();
 

lib/src/resources.rs

@@ -331,6 +331,7 @@ impl Resource {
             validate_for_agent: Some(agent.subject),
             // TODO: auto-merge should work before we enable this https://github.com/atomicdata-dev/atomic-data-rust/issues/412
             validate_previous_commit: false,
+            validate_subject_url_parent: true,
             update_index: true,
         };
         let commit_response = commit.apply_opts(store, &opts)?;
@@ -359,6 +360,8 @@ impl Resource {
             validate_for_agent: Some(agent.subject),
             // https://github.com/atomicdata-dev/atomic-data-rust/issues/412
             validate_previous_commit: false,
+            // This is contentious: https://github.com/atomicdata-dev/atomic-data-rust/issues/556
+            validate_subject_url_parent: false,
             update_index: true,
         };
         let commit_response = commit.apply_opts(store, &opts)?;
@@ -652,6 +655,7 @@ mod test {
                     validate_signature: true,
                     validate_timestamp: true,
                     validate_rights: false,
+                    validate_subject_url_parent: true,
                     validate_previous_commit: true,
                     validate_for_agent: None,
                     update_index: true,

lib/test_files/local_id.json

@@ -1,17 +1,17 @@
 [
   {
-    "https://atomicdata.dev/properties/localId": "reference",
+    "https://atomicdata.dev/properties/localId": "parent",
     "https://atomicdata.dev/properties/write": [
-      "my-local-id"
+      "parent/my-local-id"
     ],
     "https://atomicdata.dev/properties/name": "My referenced resource"
   },
   {
-    "https://atomicdata.dev/properties/localId": "my-local-id",
+    "https://atomicdata.dev/properties/localId": "parent/my-local-id",
     "https://atomicdata.dev/properties/name": "My resource that refers",
-    "https://atomicdata.dev/properties/parent": "reference",
+    "https://atomicdata.dev/properties/parent": "parent",
     "https://atomicdata.dev/properties/write": [
-      "reference"
+      "parent"
     ]
   }
 ]

server/src/handlers/commit.rs

@@ -13,7 +13,7 @@ pub async fn post_commit(
     let mut builder = HttpResponse::Ok();
     let incoming_commit_resource = parse_json_ad_commit_resource(&body, store)?;
     let incoming_commit = Commit::from_resource(incoming_commit_resource)?;
-if store.is_external_subject(&incoming_commit.subject)? {
+    if store.is_external_subject(&incoming_commit.subject)? {
         return Err("Subject of commit is external, and should be sent to its origin domain. This store can not own this resource. See https://github.com/atomicdata-dev/atomic-data-rust/issues/509".into());
     }
     let opts = CommitOpts {
@@ -24,6 +24,7 @@ if store.is_external_subject(&incoming_commit.subject)? {
         // https://github.com/atomicdata-dev/atomic-data-rust/issues/412
         validate_previous_commit: false,
         validate_for_agent: Some(incoming_commit.signer.to_string()),
+        validate_subject_url_parent: true,
         update_index: true,
     };
     let commit_response = incoming_commit.apply_opts(store, &opts)?;