Rework presign_post to be correct (#358)

* Rework presign_post to be correct - the presigned post needs to provide as a response the post URL and all the fields that need to be added - redo the presign_post function to accept only a PostPolicy which is a builder of the post policy - add a time mock to allow for better testing * Correct creating of https_connector to use possibly customized tls_connector
durch · Oct 15, 2023 · caddfcb · caddfcb
1 parent ea3de75
commit caddfcb
Show file tree

Hide file tree

Showing 13 changed files with 959 additions and 50 deletions.
diff --git a/README.md b/README.md
@@ -78,7 +78,7 @@ All runtimes support either `native-tls` or `rustls-tls`, there are features for
 
 |          |                                                                                                     |
 |----------|-----------------------------------------------------------------------------------------------------|
-| `POST`   | [presign_put](https://docs.rs/rust-s3/latest/s3/bucket/struct.Bucket.html#method.presign_post)      |
+| `POST`   | [presign_post](https://docs.rs/rust-s3/latest/s3/bucket/struct.Bucket.html#method.presign_post)     |
 | `PUT`    | [presign_put](https://docs.rs/rust-s3/latest/s3/bucket/struct.Bucket.html#method.presign_put)       |
 | `GET`    | [presign_get](https://docs.rs/rust-s3/latest/s3/bucket/struct.Bucket.html#method.presign_get)       |
 | `DELETE` | [presign_delete](https://docs.rs/rust-s3/latest/s3/bucket/struct.Bucket.html#method.presign_delete) |
@@ -141,4 +141,3 @@ Each `GET` method has a `PUT` companion `sync` and `async` methods are generic o
 [dependencies]
 rust-s3 = "0.33"
 ```
-
diff --git a/s3/Cargo.toml b/s3/Cargo.toml
@@ -68,6 +68,7 @@ maybe-async = { version = "0.2" }
 md5 = "0.7"
 percent-encoding = "2"
 serde = "1"
+serde_json = "1"
 serde_derive = "1"
 quick-xml = { version = "0.28", features = ["serialize"] }
 sha2 = "0.10"

diff --git a/s3/src/bucket.rs b/s3/src/bucket.rs
@@ -38,12 +38,14 @@ use tokio::io::AsyncRead;
 use futures::io::AsyncRead;
 
 use crate::error::S3Error;
+use crate::post_policy::PresignedPost;
 use crate::request::Request;
 use crate::serde_types::{
     BucketLocationResult, CompleteMultipartUploadData, CorsConfiguration, HeadObjectResult,
     InitiateMultipartUploadResponse, ListBucketResult, ListMultipartUploadsResult, Part,
 };
 use crate::utils::{error_from_response_data, PutStreamResponse};
+use crate::PostPolicy;
 use http::header::HeaderName;
 use http::HeaderMap;
 
@@ -165,36 +167,24 @@ impl Bucket {
     /// ```no_run
     /// use s3::bucket::Bucket;
     /// use s3::creds::Credentials;
-    /// use http::HeaderMap;
-    /// use http::header::HeaderName;
+    /// use s3::post_policy::*;
+    /// use std::borrow::Cow;
     ///
     /// let bucket_name = "rust-s3-test";
     /// let region = "us-east-1".parse().unwrap();
     /// let credentials = Credentials::default().unwrap();
     /// let bucket = Bucket::new(bucket_name, region, credentials).unwrap();
     ///
-    /// let post_policy = "eyAiZXhwaXJhdGlvbiI6ICIyMDE1LTEyLTMwVDEyOjAwOjAwLjAwMFoiLA0KICAiY29uZGl0aW9ucyI6IFsNCiAgICB7ImJ1Y2tldCI6ICJzaWd2NGV4YW1wbGVidWNrZXQifSwNCiAgICBbInN0YXJ0cy13aXRoIiwgIiRrZXkiLCAidXNlci91c2VyMS8iXSwNCiAgICB7ImFjbCI6ICJwdWJsaWMtcmVhZCJ9LA0KICAgIHsic3VjY2Vzc19hY3Rpb25fcmVkaXJlY3QiOiAiaHR0cDovL3NpZ3Y0ZXhhbXBsZWJ1Y2tldC5zMy5hbWF6b25hd3MuY29tL3N1Y2Nlc3NmdWxfdXBsb2FkLmh0bWwifSwNCiAgICBbInN0YXJ0cy13aXRoIiwgIiRDb250ZW50LVR5cGUiLCAiaW1hZ2UvIl0sDQogICAgeyJ4LWFtei1tZXRhLXV1aWQiOiAiMTQzNjUxMjM2NTEyNzQifSwNCiAgICB7IngtYW16LXNlcnZlci1zaWRlLWVuY3J5cHRpb24iOiAiQUVTMjU2In0sDQogICAgWyJzdGFydHMtd2l0aCIsICIkeC1hbXotbWV0YS10YWciLCAiIl0sDQoNCiAgICB7IngtYW16LWNyZWRlbnRpYWwiOiAiQUtJQUlPU0ZPRE5ON0VYQU1QTEUvMjAxNTEyMjkvdXMtZWFzdC0xL3MzL2F3czRfcmVxdWVzdCJ9LA0KICAgIHsieC1hbXotYWxnb3JpdGhtIjogIkFXUzQtSE1BQy1TSEEyNTYifSwNCiAgICB7IngtYW16LWRhdGUiOiAiMjAxNTEyMjlUMDAwMDAwWiIgfQ0KICBdDQp9";
+    /// let post_policy = PostPolicy::new(86400).condition(
+    ///     PostPolicyField::Key,
+    ///     PostPolicyValue::StartsWith(Cow::from("user/user1/"))
+    /// ).unwrap();
     ///
-    /// let url = bucket.presign_post("/test.file", 86400, post_policy.to_string()).unwrap();
-    /// println!("Presigned url: {}", url);
+    /// let presigned_post = bucket.presign_post(post_policy).unwrap();
+    /// println!("Presigned url: {}, fields: {:?}", presigned_post.url, presigned_post.fields);
     /// ```
-    pub fn presign_post<S: AsRef<str>>(
-        &self,
-        path: S,
-        expiry_secs: u32,
-        // base64 encoded post policy document -> https://docs.aws.amazon.com/AmazonS3/latest/API/sigv4-post-example.html
-        post_policy: String,
-    ) -> Result<String, S3Error> {
-        validate_expiry(expiry_secs)?;
-        let request = RequestImpl::new(
-            self,
-            path.as_ref(),
-            Command::PresignPost {
-                expiry_secs,
-                post_policy,
-            },
-        )?;
-        request.presigned()
+    pub fn presign_post(&self, post_policy: PostPolicy) -> Result<PresignedPost, S3Error> {
+        post_policy.sign(self.clone())
     }
 
     /// Get a presigned url for putting object to a given path
@@ -2237,12 +2227,13 @@ impl Bucket {
 mod test {
 
     use crate::creds::Credentials;
+    use crate::post_policy::{PostPolicyField, PostPolicyValue};
     use crate::region::Region;
     use crate::serde_types::CorsConfiguration;
     use crate::serde_types::CorsRule;
-    use crate::Bucket;
     use crate::BucketConfiguration;
     use crate::Tag;
+    use crate::{Bucket, PostPolicy};
     use http::header::HeaderName;
     use http::HeaderMap;
     use std::env;
@@ -2355,6 +2346,7 @@ mod test {
         .with_path_style()
     }
 
+    #[allow(dead_code)]
     fn test_digital_ocean_bucket() -> Bucket {
         Bucket::new("rust-s3", Region::DoFra1, test_digital_ocean_credentials()).unwrap()
     }
@@ -2895,10 +2887,9 @@ mod test {
     }
 
     #[test]
-    #[ignore]
     fn test_presign_put() {
         let s3_path = "/test/test.file";
-        let bucket = test_aws_bucket();
+        let bucket = test_minio_bucket();
 
         let mut custom_headers = HeaderMap::new();
         custom_headers.insert(
@@ -2915,20 +2906,39 @@ mod test {
     }
 
     #[test]
-    #[ignore]
+    fn test_presign_post() {
+        use std::borrow::Cow;
+
+        let bucket = test_minio_bucket();
+
+        // Policy from sample
+        let policy = PostPolicy::new(86400)
+            .condition(
+                PostPolicyField::Key,
+                PostPolicyValue::StartsWith(Cow::from("user/user1/")),
+            )
+            .unwrap();
+
+        let data = bucket.presign_post(policy).unwrap();
+
+        assert_eq!(data.url, "http://localhost:9000/rust-s3");
+        assert_eq!(data.fields.len(), 6);
+        assert_eq!(data.dynamic_fields.len(), 1);
+    }
+
+    #[test]
     fn test_presign_get() {
         let s3_path = "/test/test.file";
-        let bucket = test_aws_bucket();
+        let bucket = test_minio_bucket();
 
         let url = bucket.presign_get(s3_path, 86400, None).unwrap();
         assert!(url.contains("/test/test.file?"))
     }
 
     #[test]
-    #[ignore]
     fn test_presign_delete() {
         let s3_path = "/test/test.file";
-        let bucket = test_aws_bucket();
+        let bucket = test_minio_bucket();
 
         let url = bucket.presign_delete(s3_path, 86400).unwrap();
         assert!(url.contains("/test/test.file?"))

diff --git a/s3/src/command.rs b/s3/src/command.rs
@@ -102,10 +102,6 @@ pub enum Command<'a> {
         expiry_secs: u32,
         custom_headers: Option<HeaderMap>,
     },
-    PresignPost {
-        expiry_secs: u32,
-        post_policy: String,
-    },
     PresignDelete {
         expiry_secs: u32,
     },
@@ -161,7 +157,6 @@ impl<'a> Command<'a> {
                 HttpMethod::Post
             }
             Command::HeadObject => HttpMethod::Head,
-            Command::PresignPost { .. } => HttpMethod::Post,
         }
     }
 

diff --git a/s3/src/error.rs b/s3/src/error.rs
@@ -1,6 +1,7 @@
 use thiserror::Error;
 
 #[derive(Error, Debug)]
+#[non_exhaustive]
 pub enum S3Error {
     #[error("Utf8 decoding error: {0}")]
     Utf8(#[from] std::str::Utf8Error),
@@ -53,4 +54,8 @@ pub enum S3Error {
     TimeFormatError(#[from] time::error::Format),
     #[error("fmt error: {0}")]
     FmtError(#[from] std::fmt::Error),
+    #[error("serde error: {0}")]
+    SerdeError(#[from] serde_json::Error),
+    #[error("post policy error: {0}")]
+    PostPolicyError(#[from] crate::post_policy::PostPolicyError),
 }
diff --git a/s3/src/lib.rs b/s3/src/lib.rs
@@ -10,12 +10,14 @@ pub use awsregion as region;
 pub use bucket::Bucket;
 pub use bucket::Tag;
 pub use bucket_ops::BucketConfiguration;
+pub use post_policy::{PostPolicy, PostPolicyChecksum, PostPolicyField, PostPolicyValue};
 pub use region::Region;
 
 pub mod bucket;
 pub mod bucket_ops;
 pub mod command;
 pub mod deserializer;
+pub mod post_policy;
 pub mod serde_types;
 pub mod signing;