#11 refactor error model

Author: joepio

CHANGELOG.md

@@ -3,6 +3,11 @@
 List of changes for this repo, including `atomic-cli`, `atomic-server` and `atomic-lib`.
 By far most changes relate to `atomic-server`, so if not specified, assume the changes are relevant only for the server.
 
+## v0.29.0
+
+- Add authentication to restrict read access. Works by signing requests with Private Keys. #13
+- Refactor internal error model, Use correct HTTP status codes #11
+
 ## v0.28.2
 
 - Full-text search endpoint, powered by Tantify #40

lib/src/authentication.rs

@@ -14,8 +14,6 @@ pub struct AuthValues {
     pub agent_subject: String,
 }
 
-pub const PUBLIC_AGENT: &str = "https://atomicdata.dev/agents/publicAgent";
-
 /// Checks if the signature is valid for this timestamp.
 /// Does not check if the agent has rights to access the subject.
 pub fn check_auth_signature(subject: &str, auth_header: &AuthValues) -> AtomicResult<()> {
@@ -42,7 +40,7 @@ pub fn get_agent_from_headers_and_check(
     auth_header_values: Option<AuthValues>,
     store: &impl Storelike,
 ) -> AtomicResult<String> {
-    let mut for_agent = PUBLIC_AGENT.to_string();
+    let mut for_agent = crate::urls::PUBLIC_AGENT.to_string();
     if let Some(auth_vals) = auth_header_values {
         // If there are auth headers, check 'em, make sure they are valid.
         check_auth_signature(&auth_vals.requested_subject, &auth_vals)?;

lib/src/db.rs

@@ -8,7 +8,7 @@ use std::{
 
 use crate::{
     datatype::DataType,
-    errors::AtomicResult,
+    errors::{AtomicError, AtomicResult},
     resources::PropVals,
     storelike::{ResourceCollection, Storelike},
     Atom, Resource, Value,
@@ -314,9 +314,10 @@ impl Storelike for Db {
 
         if let Some(agent) = for_agent {
             if !crate::hierarchy::check_read(self, &resource, agent.to_string())? {
-                return Err(
-                    format!("Agent '{}' is not allowed to read '{}'.", agent, subject).into(),
-                );
+                return Err(AtomicError::unauthorized(format!(
+                    "Agent '{}' is not authorized to read '{}'. There should be a `read` right in this resource or one of its parents.",
+                    agent, subject
+                )));
             }
         }
 

lib/src/errors.rs

@@ -1,20 +1,221 @@
 //! The Error type that you can expect when using this library
 
-use std::error::Error;
-use std::fmt;
+use std::{
+    convert::Infallible,
+    num::{ParseFloatError, ParseIntError},
+    str::ParseBoolError,
+};
+
+use base64::DecodeError;
 
 /// The default Error type for all Atomic Lib Errors.
-// TODO: specify & limit error types
-// https://github.com/joepio/atomic/issues/11
-pub type AtomicResult<T> = std::result::Result<T, Box<dyn std::error::Error>>;
+pub type AtomicResult<T> = std::result::Result<T, AtomicError>;
+
+#[derive(Debug)]
+pub struct AtomicError {
+    pub message: String,
+    pub error_type: AtomicErrorType,
+}
 
 #[derive(Debug)]
-struct AtomicError(String);
+pub enum AtomicErrorType {
+    NotFoundError,
+    UnauthorizedError,
+    OtherError,
+}
+
+impl std::error::Error for AtomicError {
+    // fn description(&self) -> &str {
+    //     // Both underlying errors already impl `Error`, so we defer to their
+    //     // implementations.
+    //     match *self {
+    //         CliError::Io(ref err) => err.description(),
+    //         // Normally we can just write `err.description()`, but the error
+    //         // type has a concrete method called `description`, which conflicts
+    //         // with the trait method. For now, we must explicitly call
+    //         // `description` through the `Error` trait.
+    //         CliError::Parse(ref err) => error::Error::description(err),
+    //     }
+    // }
+
+    // fn cause(&self) -> Option<&dyn std::error::Error> {
+    //     match *self {
+    //         // N.B. Both of these implicitly cast `err` from their concrete
+    //         // types (either `&io::Error` or `&num::ParseIntError`)
+    //         // to a trait object `&Error`. This works because both error types
+    //         // implement `Error`.
+    //         CliError::Io(ref err) => Some(err),
+    //         CliError::Parse(ref err) => Some(err),
+    //     }
+    // }
+}
+
+impl AtomicError {
+    #[allow(dead_code)]
+    pub fn not_found(message: String) -> AtomicError {
+        AtomicError {
+            message: format!("Resource not found. {}", message),
+            error_type: AtomicErrorType::NotFoundError,
+        }
+    }
+
+    pub fn unauthorized(message: String) -> AtomicError {
+        AtomicError {
+            message: format!("Unauthorized. {}", message),
+            error_type: AtomicErrorType::UnauthorizedError,
+        }
+    }
+
+    pub fn other_error(message: String) -> AtomicError {
+        AtomicError {
+            message,
+            error_type: AtomicErrorType::OtherError,
+        }
+    }
+}
+
+impl std::fmt::Display for AtomicError {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        write!(f, "{}", &self.message)
+    }
+}
+
+// Error conversions
+impl From<&str> for AtomicError {
+    fn from(message: &str) -> Self {
+        AtomicError {
+            message: message.into(),
+            error_type: AtomicErrorType::OtherError,
+        }
+    }
+}
+
+impl From<String> for AtomicError {
+    fn from(message: String) -> Self {
+        AtomicError {
+            message,
+            error_type: AtomicErrorType::OtherError,
+        }
+    }
+}
+
+// The following feel very redundant. Can this be simplified?
+
+impl From<std::boxed::Box<dyn std::error::Error>> for AtomicError {
+    fn from(error: std::boxed::Box<dyn std::error::Error>) -> Self {
+        AtomicError {
+            message: error.to_string(),
+            error_type: AtomicErrorType::OtherError,
+        }
+    }
+}
+
+impl<T> From<std::sync::PoisonError<T>> for AtomicError {
+    fn from(error: std::sync::PoisonError<T>) -> Self {
+        AtomicError {
+            message: error.to_string(),
+            error_type: AtomicErrorType::OtherError,
+        }
+    }
+}
+
+impl From<std::io::Error> for AtomicError {
+    fn from(error: std::io::Error) -> Self {
+        AtomicError {
+            message: error.to_string(),
+            error_type: AtomicErrorType::OtherError,
+        }
+    }
+}
+
+impl From<url::ParseError> for AtomicError {
+    fn from(error: url::ParseError) -> Self {
+        AtomicError {
+            message: error.to_string(),
+            error_type: AtomicErrorType::OtherError,
+        }
+    }
+}
+
+impl From<serde_json::Error> for AtomicError {
+    fn from(error: serde_json::Error) -> Self {
+        AtomicError {
+            message: error.to_string(),
+            error_type: AtomicErrorType::OtherError,
+        }
+    }
+}
+
+impl From<std::string::FromUtf8Error> for AtomicError {
+    fn from(error: std::string::FromUtf8Error) -> Self {
+        AtomicError {
+            message: error.to_string(),
+            error_type: AtomicErrorType::OtherError,
+        }
+    }
+}
+
+impl From<ParseFloatError> for AtomicError {
+    fn from(error: ParseFloatError) -> Self {
+        AtomicError {
+            message: error.to_string(),
+            error_type: AtomicErrorType::OtherError,
+        }
+    }
+}
+
+impl From<ParseIntError> for AtomicError {
+    fn from(error: ParseIntError) -> Self {
+        AtomicError {
+            message: error.to_string(),
+            error_type: AtomicErrorType::OtherError,
+        }
+    }
+}
 
-impl fmt::Display for AtomicError {
-    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-        write!(f, "There is an error: {}", self.0)
+impl From<DecodeError> for AtomicError {
+    fn from(error: DecodeError) -> Self {
+        AtomicError {
+            message: error.to_string(),
+            error_type: AtomicErrorType::OtherError,
+        }
     }
 }
 
-impl Error for AtomicError {}
+impl From<ParseBoolError> for AtomicError {
+    fn from(error: ParseBoolError) -> Self {
+        AtomicError {
+            message: error.to_string(),
+            error_type: AtomicErrorType::OtherError,
+        }
+    }
+}
+
+impl From<Infallible> for AtomicError {
+    fn from(error: Infallible) -> Self {
+        AtomicError {
+            message: error.to_string(),
+            error_type: AtomicErrorType::OtherError,
+        }
+    }
+}
+
+#[cfg(feature = "db")]
+impl From<sled::Error> for AtomicError {
+    fn from(error: sled::Error) -> Self {
+        AtomicError {
+            message: error.to_string(),
+            error_type: AtomicErrorType::OtherError,
+        }
+    }
+}
+
+#[cfg(feature = "db")]
+impl From<Box<bincode::ErrorKind>> for AtomicError {
+    fn from(error: Box<bincode::ErrorKind>) -> Self {
+        AtomicError {
+            message: error.to_string(),
+            error_type: AtomicErrorType::OtherError,
+        }
+    }
+}

lib/src/storelike.rs

@@ -2,6 +2,7 @@
 
 use crate::{
     agents::Agent,
+    errors::AtomicError,
     hierarchy,
     schema::{Class, Property},
 };
@@ -173,16 +174,12 @@ pub trait Storelike: Sized {
             if hierarchy::check_read(self, &resource, agent)? {
                 return Ok(resource);
             }
-            return Err("No rights".into());
+            return Err(AtomicError::unauthorized("No rights".into()));
         }
         Ok(resource)
     }
 
-    fn handle_not_found(
-        &self,
-        subject: &str,
-        error: Box<dyn std::error::Error>,
-    ) -> AtomicResult<Resource> {
+    fn handle_not_found(&self, subject: &str, error: AtomicError) -> AtomicResult<Resource> {
         if let Some(self_url) = self.get_self_url() {
             if subject.starts_with(&self_url) {
                 return Err(format!("Failed to retrieve locally: '{}'. {}", subject, error).into());

lib/src/urls.rs

@@ -96,3 +96,6 @@ pub const TIMESTAMP: &str = "https://atomicdata.dev/datatypes/timestamp";
 // Methods
 pub const INSERT: &str = "https://atomicdata.dev/methods/insert";
 pub const DELETE: &str = "https://atomicdata.dev/methods/delete";
+
+// Instances
+pub const PUBLIC_AGENT: &str = "https://atomicdata.dev/agents/publicAgent";

server/src/appstate.rs

@@ -1,6 +1,6 @@
 //! App state, which is accessible from handlers
 use crate::{
-    commit_monitor::CommitMonitor, config::Config, errors::BetterResult, search::SearchState,
+    commit_monitor::CommitMonitor, config::Config, errors::AtomicServerResult, search::SearchState,
 };
 use atomic_lib::{
     agents::{generate_public_key, Agent},
@@ -24,7 +24,7 @@ pub struct AppState {
 /// Creates the server context.
 /// Initializes a store on disk.
 /// Creates a new agent, if neccessary.
-pub fn init(config: Config) -> BetterResult<AppState> {
+pub fn init(config: Config) -> AtomicServerResult<AppState> {
     // Enable logging, but hide most tantivy logs
     std::env::set_var("RUST_LOG", "info,tantivy=warn");
     env_logger::init();
@@ -84,7 +84,7 @@ pub fn init(config: Config) -> BetterResult<AppState> {
 }
 
 /// Create a new agent if it does not yet exist.
-fn set_default_agent(config: &Config, store: &impl Storelike) -> BetterResult<()> {
+fn set_default_agent(config: &Config, store: &impl Storelike) -> AtomicServerResult<()> {
     let ag_cfg: atomic_lib::config::Config = match atomic_lib::config::read_config(
         &config.config_file_path,
     ) {
@@ -142,7 +142,7 @@ fn set_default_agent(config: &Config, store: &impl Storelike) -> BetterResult<()
 }
 
 /// Creates the first Invitation that is opened by the user on the Home page.
-fn set_up_initial_invite(store: &impl Storelike) -> BetterResult<()> {
+fn set_up_initial_invite(store: &impl Storelike) -> AtomicServerResult<()> {
     let subject = format!("{}/setup", store.get_base_url());
     log::info!("Creating initial Invite at {}", subject);
     let mut invite = atomic_lib::Resource::new_instance(atomic_lib::urls::INVITE, store)?;

server/src/config.rs

@@ -1,6 +1,6 @@
 //! Parse CLI options, setup on boot, read .env values
 
-use crate::errors::BetterResult;
+use crate::errors::AtomicServerResult;
 use clap::Parser;
 use dotenv::dotenv;
 use std::env;
@@ -127,7 +127,7 @@ pub struct Config {
 }
 
 /// Creates the server config, reads .env values and sets defaults
-pub fn init() -> BetterResult<Config> {
+pub fn init() -> AtomicServerResult<Config> {
     // Parse .env file (do this before parsing the CLI opts)
     dotenv().ok();