Added some tests and documentation (#54)

* add docs+tests to the policy module

* small changes

* simplify authentication middleware generics

* fix typo

* fix

---------

Co-authored-by: Tim Dikland <[email protected]>

delta-sharing/core/src/in_memory.rs

@@ -92,7 +92,7 @@ impl<T: Send + Sync> DiscoveryHandler for InMemoryHandler<T> {
             .map(|share| {
                 let id = Uuid::new_v5(&Uuid::NAMESPACE_OID, share.key().as_bytes());
                 t::Share {
-                    id: Some(id.to_string()),
+                    id: Some(id.into()),
                     name: share.key().clone(),
                 }
             })

delta-sharing/core/src/lib.rs

@@ -1,3 +1,5 @@
+use std::sync::Arc;
+
 use serde::{de::DeserializeOwned, Serialize};
 
 #[allow(dead_code)]
@@ -99,8 +101,26 @@ pub enum Resource {
     File(String),
 }
 
+impl Resource {
+    pub fn share(name: impl Into<String>) -> Self {
+        Self::Share(name.into())
+    }
+
+    pub fn schema(name: impl Into<String>) -> Self {
+        Self::Schema(name.into())
+    }
+
+    pub fn table(name: impl Into<String>) -> Self {
+        Self::Table(name.into())
+    }
+
+    pub fn file(name: impl Into<String>) -> Self {
+        Self::File(name.into())
+    }
+}
+
 /// Decision made by a policy.
-#[derive(Debug, Clone, PartialEq)]
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
 pub enum Decision {
     /// Allow the action.
     Allow,

delta-sharing/core/src/policies/mod.rs

@@ -1,16 +1,23 @@
+//! Authorization policies.
+//!
+//! Policies are used to determine whether a recipient is allowed to perform a specific action on a
+//! resource. The action is represented by a [`Permission`] and the resource is represented by a
+//! [`Resource`]. The [`Decision`] represents whether the action is allowed or denied for the given
+//! recipient.
+
 use crate::error::Result;
 use crate::{Decision, Permission, Policy, Resource};
 
 /// Policy that always returns a constant decision.
 ///
 /// This policy is mainly useful for testing and development, or servers that do not require
 /// authorization checks - e.g. when deployed in a trusted environment.
-pub struct ConstantPolicy<T: Send + Sync> {
+pub struct ConstantPolicy<T> {
     decision: Decision,
     _phantom: std::marker::PhantomData<T>,
 }
 
-impl<T: Send + Sync> Default for ConstantPolicy<T> {
+impl<T> Default for ConstantPolicy<T> {
     fn default() -> Self {
         Self {
             decision: Decision::Allow,
@@ -19,8 +26,27 @@ impl<T: Send + Sync> Default for ConstantPolicy<T> {
     }
 }
 
-impl<T: Send + Sync> ConstantPolicy<T> {
+impl<T> ConstantPolicy<T> {
     /// Create a new instance of [`ConstantPolicy`].
+    ///
+    /// The [`ConstantPolicy`] will always return the same decision for all authorization requests.
+    ///
+    /// # Example
+    /// ```
+    /// # async fn doc() -> Result<(), Box<dyn std::error::Error>> {
+    /// use delta_sharing_core::{Policy, Resource, Permission, Decision};
+    /// use delta_sharing_core::policies::ConstantPolicy;
+    ///
+    /// let policy = ConstantPolicy::new(Decision::Allow);
+    /// let resource = Resource::share("test");
+    /// let permission = Permission::Read;
+    /// let recipient = &();
+    ///
+    /// let decision = policy.authorize(resource, permission, recipient).await.unwrap();
+    /// assert_eq!(decision, Decision::Allow);
+    /// # Ok(())
+    /// # }
+    /// ```
     pub fn new(decision: Decision) -> Self {
         Self {
             decision,
@@ -34,6 +60,62 @@ impl<T: Send + Sync> Policy for ConstantPolicy<T> {
     type Recipient = T;
 
     async fn authorize(&self, _: Resource, _: Permission, _: &Self::Recipient) -> Result<Decision> {
-        Ok(self.decision.clone())
+        Ok(self.decision)
+    }
+}
+
+#[cfg(test)]
+mod test {
+    use super::*;
+
+    #[test]
+    fn assert_send_sync() {
+        fn assert_send_sync<T: Send + Sync>() {}
+        assert_send_sync::<ConstantPolicy<()>>();
+    }
+
+    #[tokio::test]
+    async fn allow_by_default() {
+        let policy = ConstantPolicy::default();
+
+        let resource = Resource::Share("test_share".to_string());
+        let permission = Permission::Read;
+        let recipient = &();
+
+        let decision = policy
+            .authorize(resource, permission, recipient)
+            .await
+            .unwrap();
+        assert_eq!(decision, Decision::Allow);
+    }
+
+    #[tokio::test]
+    async fn allow() {
+        let policy = ConstantPolicy::new(Decision::Allow);
+
+        let resource = Resource::Share("test_share".to_string());
+        let permission = Permission::Read;
+        let recipient = &();
+
+        let decision = policy
+            .authorize(resource, permission, recipient)
+            .await
+            .unwrap();
+        assert_eq!(decision, Decision::Allow);
+    }
+
+    #[tokio::test]
+    async fn deny() {
+        let policy = ConstantPolicy::new(Decision::Deny);
+
+        let resource = Resource::Share("test_share".to_string());
+        let permission = Permission::Read;
+        let recipient = &();
+
+        let decision = policy
+            .authorize(resource, permission, recipient)
+            .await
+            .unwrap();
+        assert_eq!(decision, Decision::Deny);
     }
 }

delta-sharing/server/src/auth.rs

@@ -1,15 +1,17 @@
-use std::sync::Arc;
+//! Authentication middleware for Delta Sharing server.
+
 use std::task::{Context, Poll};
 
 use axum::extract::Request;
 use axum::response::{IntoResponse, Response};
-use delta_sharing_core::DeltaRecipient;
-use delta_sharing_core::{Authenticator, Error as CoreError};
+use delta_sharing_core::{Authenticator, DeltaRecipient, Error as CoreError};
 use futures_util::{future::BoxFuture, FutureExt};
 use tower::{Layer, Service};
 
 use crate::error::{Error, Result};
 
+/// Authenticator that always marks the recipient as anonymous.
+#[derive(Clone)]
 pub struct AnonymousAuthenticator;
 
 impl Authenticator for AnonymousAuthenticator {
@@ -21,17 +23,36 @@ impl Authenticator for AnonymousAuthenticator {
     }
 }
 
-/// Middleware that authenticates requests.
+/// Middleware that authenticates requests using the given [`Authenticator`].
 #[derive(Clone)]
-pub struct AuthenticationMiddleware<S, T: Clone + Send + Sync + 'static> {
+pub struct AuthenticationMiddleware<S, T> {
     inner: S,
-    authenticator: Arc<dyn Authenticator<Recipient = T, Request = Request>>,
+    authenticator: T,
+}
+
+impl<S, T> AuthenticationMiddleware<S, T> {
+    /// Create new [`AuthenticationMiddleware`].
+    pub fn new(inner: S, authenticator: T) -> Self {
+        Self {
+            inner,
+            authenticator,
+        }
+    }
+
+    /// Create a new [`AuthorizationLayer`] with the given [`Authenticator`].
+    ///
+    /// This is a convenience method that is equivalent to calling [`AuthorizationLayer::new`].
+    pub fn layer(authenticator: T) -> AuthorizationLayer<T> {
+        AuthorizationLayer::new(authenticator)
+    }
 }
 
-impl<S, T: Clone + Send + Sync> Service<Request> for AuthenticationMiddleware<S, T>
+impl<S, T, R> Service<Request> for AuthenticationMiddleware<S, T>
 where
     S: Service<Request, Response = Response> + Send + 'static,
     S::Future: Send + 'static,
+    T: Authenticator<Recipient = R, Request = Request>,
+    T::Recipient: Clone + Send + Sync + 'static,
 {
     type Response = S::Response;
     type Error = S::Error;
@@ -54,13 +75,13 @@ where
 
 /// Layer that applies the [`AuthenticationMiddleware`].
 #[derive(Clone)]
-pub struct AuthorizationLayer<T: Clone + Send + Sync + 'static> {
-    authenticator: Arc<dyn Authenticator<Recipient = T, Request = Request>>,
+pub struct AuthorizationLayer<T> {
+    authenticator: T,
 }
 
-impl<T: Clone + Send + Sync + 'static> AuthorizationLayer<T> {
-    /// Create a new [`AuthorizationLayer`].
-    pub fn new(authenticator: Arc<dyn Authenticator<Recipient = T, Request = Request>>) -> Self {
+impl<T> AuthorizationLayer<T> {
+    /// Create a new [`AuthorizationLayer`] with the provided [`Authenticator`].
+    pub fn new(authenticator: T) -> Self {
         Self { authenticator }
     }
 }
@@ -78,8 +99,6 @@ impl<S, T: Clone + Send + Sync + 'static> Layer<S> for AuthorizationLayer<T> {
 
 #[cfg(test)]
 mod tests {
-    use std::sync::Arc;
-
     use axum::body::Body;
     use axum::extract::Request;
     use axum::http::{header, StatusCode};
@@ -98,7 +117,7 @@ mod tests {
 
     #[tokio::test]
     async fn test_authentication_middleware() {
-        let authenticator = Arc::new(AnonymousAuthenticator);
+        let authenticator = AnonymousAuthenticator {};
         let mut service = ServiceBuilder::new()
             .layer(AuthorizationLayer::new(authenticator))
             .service_fn(check_recipient);

delta-sharing/server/src/main.rs

@@ -42,7 +42,7 @@ async fn main() -> Result<(), Box<dyn std::error::Error>> {
 
     let listener = TcpListener::bind(format!("{}:{}", args.host, args.port)).await?;
     let server = get_router(state)
-        .layer(AuthorizationLayer::new(Arc::new(AnonymousAuthenticator)))
+        .layer(AuthorizationLayer::new(AnonymousAuthenticator))
         .layer(TraceLayer::new_for_http());
     axum::serve(listener, server)
         .with_graceful_shutdown(shutdown_signal())

delta-sharing/server/src/server.rs

@@ -144,13 +144,13 @@ mod tests {
         }
     }
 
-    fn get_anoymous_router() -> Router {
-        get_router(get_state()).layer(AuthorizationLayer::new(Arc::new(AnonymousAuthenticator)))
+    fn get_anonymous_router() -> Router {
+        get_router(get_state()).layer(AuthorizationLayer::new(AnonymousAuthenticator))
     }
 
     #[tokio::test]
     async fn test_list_shares() {
-        let app = get_anoymous_router();
+        let app = get_anonymous_router();
 
         let request = Request::builder()
             .uri("/shares")
@@ -171,7 +171,7 @@ mod tests {
 
     #[tokio::test]
     async fn test_get_share() {
-        let app = get_anoymous_router();
+        let app = get_anonymous_router();
 
         let request = Request::builder()
             .uri("/shares/share1")
@@ -192,7 +192,7 @@ mod tests {
 
     #[tokio::test]
     async fn test_get_share_not_found() {
-        let app = get_anoymous_router();
+        let app = get_anonymous_router();
 
         let request: Request<Body> = Request::builder()
             .uri("/shares/nonexistent")
@@ -209,7 +209,7 @@ mod tests {
 
     #[tokio::test]
     async fn test_list_schemas() {
-        let app = get_anoymous_router();
+        let app = get_anonymous_router();
 
         let request = Request::builder()
             .uri("/shares/share1/schemas")
@@ -230,7 +230,7 @@ mod tests {
 
     #[tokio::test]
     async fn test_list_schemas_not_found() {
-        let app = get_anoymous_router();
+        let app = get_anonymous_router();
 
         let request: Request<Body> = Request::builder()
             .uri("/shares/nonexistent/schemas")
@@ -247,7 +247,7 @@ mod tests {
 
     #[tokio::test]
     async fn test_list_share_tables() {
-        let app = get_anoymous_router();
+        let app = get_anonymous_router();
 
         let request = Request::builder()
             .uri("/shares/share1/all-tables")
@@ -268,7 +268,7 @@ mod tests {
 
     #[tokio::test]
     async fn test_list_share_tables_not_found() {
-        let app = get_anoymous_router();
+        let app = get_anonymous_router();
 
         let request: Request<Body> = Request::builder()
             .uri("/shares/nonexistent/all-tables")
@@ -285,7 +285,7 @@ mod tests {
 
     #[tokio::test]
     async fn test_list_schema_tables() {
-        let app = get_anoymous_router();
+        let app = get_anonymous_router();
 
         let request = Request::builder()
             .uri("/shares/share1/schemas/schema1/tables")
@@ -306,7 +306,7 @@ mod tests {
 
     #[tokio::test]
     async fn test_list_schema_tables_not_found() {
-        let app = get_anoymous_router();
+        let app = get_anonymous_router();
 
         let request: Request<Body> = Request::builder()
             .uri("/shares/share1/schemas/nonexistent/tables")

docs/src/developer_guide/protocol/authentication.md

@@ -45,8 +45,13 @@ let listener = TcpListener::bind("127.0.0.1:0")
 axum::serve(listener, app).await.expect("server error");
 ```
 
+// TODO: explain policy module
+
+
 ## What's in the box?
 
 The Delta Sharing library comes with a pre-built authentication middleware that can be used out of the box.
 
 // TODO: write about pre-built middleware
+
+