Repository: actix/actix-web
Pull request #540 (2018-10-09 19:32:45)

Added HTTP Authentication for Client
#535 
Attempting to add support for HTTP basic and beaer authentication for the http client. While this could be done manually, I felt it was a common enough thing to have to be their own functions respectively.

side note, first pull request ever.

codecov[bot]: # [Codecov](https://codecov.io/gh/actix/actix-web/pull/540?src=pr&el=h1) Report
> Merging [#540](https://codecov.io/gh/actix/actix-web/pull/540?src=pr&el=desc) into [master](https://codecov.io/gh/actix/actix-web/commit/f45038bbfe338661f3b958b10c37dd64d3d70650?src=pr&el=desc) will **decrease** coverage by `0.05%`.
> The diff coverage is `0%`.

[![Impacted file tree graph](https://codecov.io/gh/actix/actix-web/pull/540/graphs/tree.svg?width=650&token=zeNInsA6hP&height=150&src=pr)](https://codecov.io/gh/actix/actix-web/pull/540?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master     #540      +/-   ##
==========================================
- Coverage   76.22%   76.16%   -0.06%     
==========================================
  Files         103      103              
  Lines       16814    16873      +59     
==========================================
+ Hits        12817    12852      +35     
- Misses       3997     4021      +24
```


| [Impacted Files](https://codecov.io/gh/actix/actix-web/pull/540?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [src/client/request.rs](https://codecov.io/gh/actix/actix-web/pull/540/diff?src=pr&el=tree#diff-c3JjL2NsaWVudC9yZXF1ZXN0LnJz) | `58.05% <0%> (-1.81%)` | :arrow_down: |
| [src/server/h2.rs](https://codecov.io/gh/actix/actix-web/pull/540/diff?src=pr&el=tree#diff-c3JjL3NlcnZlci9oMi5ycw==) | `51.16% <0%> (-1.22%)` | :arrow_down: |
| [src/server/h1.rs](https://codecov.io/gh/actix/actix-web/pull/540/diff?src=pr&el=tree#diff-c3JjL3NlcnZlci9oMS5ycw==) | `76.52% <0%> (-0.17%)` | :arrow_down: |
| [src/server/settings.rs](https://codecov.io/gh/actix/actix-web/pull/540/diff?src=pr&el=tree#diff-c3JjL3NlcnZlci9zZXR0aW5ncy5ycw==) | `84.26% <0%> (+0.24%)` | :arrow_up: |
| [src/server/acceptor.rs](https://codecov.io/gh/actix/actix-web/pull/540/diff?src=pr&el=tree#diff-c3JjL3NlcnZlci9hY2NlcHRvci5ycw==) | `88.15% <0%> (+0.32%)` | :arrow_up: |
| [src/server/builder.rs](https://codecov.io/gh/actix/actix-web/pull/540/diff?src=pr&el=tree#diff-c3JjL3NlcnZlci9idWlsZGVyLnJz) | `86.36% <0%> (+0.64%)` | :arrow_up: |
| [src/server/h2writer.rs](https://codecov.io/gh/actix/actix-web/pull/540/diff?src=pr&el=tree#diff-c3JjL3NlcnZlci9oMndyaXRlci5ycw==) | `49.23% <0%> (+1.43%)` | :arrow_up: |
| [src/server/channel.rs](https://codecov.io/gh/actix/actix-web/pull/540/diff?src=pr&el=tree#diff-c3JjL3NlcnZlci9jaGFubmVsLnJz) | `49.27% <0%> (+25.13%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/actix/actix-web/pull/540?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/actix/actix-web/pull/540?src=pr&el=footer). Last update [f45038b...761b669](https://codecov.io/gh/actix/actix-web/pull/540?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

f0cus3d: > @f0cus3d could you also add test?

Yes, I was thinking about the test last night when the build verification was processing. I will get a test added sometime today and resubmit.
fafhrd91: test could as simple as just check if authorization header is set to write value
crusty-dave: I am not familiar with the process, when does this actually make it into nightly and then stable?
f0cus3d: Hey all I added two tests on the client side for checking the add for basic and bearer auth. Apologies for the delay, have been travelling with the family.

Patch:
diff --git a/CHANGES.md b/CHANGES.md
index d1d838f4..8cce7159 100644
--- a/CHANGES.md
+++ b/CHANGES.md
@@ -8,10 +8,14 @@
 
 * Add `insert` and `remove` methods to `HttpResponseBuilder`
 
+* Add client HTTP Authentication methods `.basic_auth()`  and `.bearer_auth()`. #540
+
 ### Fixed
 
 * Ignored the `If-Modified-Since` if `If-None-Match` is specified. #680
 
+* Do not remove `Content-Length` on `Body::Empty` and insert zero value if it is missing for `POST` and `PUT` methods.
+
 ## [0.7.18] - 2019-01-10
 
 ### Added
diff --git a/src/client/request.rs b/src/client/request.rs
index ad08ad13..89789933 100644
--- a/src/client/request.rs
+++ b/src/client/request.rs
@@ -12,6 +12,7 @@ use serde::Serialize;
 use serde_json;
 use serde_urlencoded;
 use url::Url;
+use base64::encode;
 
 use super::connector::{ClientConnector, Connection};
 use super::pipeline::SendRequest;
@@ -485,6 +486,29 @@ impl ClientRequestBuilder {
         self
     }
 
+    /// Set HTTP basic authorization
+    pub fn basic_auth<U, P>(&mut self, username: U, password: Option<P>) -> &mut Self 
+    where 
+        U: fmt::Display,
+        P: fmt::Display,
+    {
+        let auth = match password {
+            Some(password) => format!("{}:{}", username, password),
+            None => format!("{}", username)
+        };
+        let header_value = format!("Basic {}", encode(&auth));
+        self.header(header::AUTHORIZATION, &*header_value)
+    }
+
+    /// Set HTTP bearer authentication
+    pub fn bearer_auth<T>( &mut self, token: T) -> &mut Self
+    where 
+        T: fmt::Display,
+    {
+        let header_value = format!("Bearer {}", token);
+        self.header(header::AUTHORIZATION, &*header_value)
+    }
+
     /// Set content length
     #[inline]
     pub fn content_length(&mut self, len: u64) -> &mut Self {
diff --git a/src/client/writer.rs b/src/client/writer.rs
index 321753bb..4091ed21 100644
--- a/src/client/writer.rs
+++ b/src/client/writer.rs
@@ -16,9 +16,9 @@ use flate2::write::{GzEncoder, ZlibEncoder};
 use flate2::Compression;
 use futures::{Async, Poll};
 use http::header::{
-    HeaderValue, CONNECTION, CONTENT_ENCODING, CONTENT_LENGTH, DATE, TRANSFER_ENCODING,
+    self, HeaderValue, CONNECTION, CONTENT_ENCODING, CONTENT_LENGTH, DATE, TRANSFER_ENCODING,
 };
-use http::{HttpTryFrom, Version};
+use http::{Method, HttpTryFrom, Version};
 use time::{self, Duration};
 use tokio_io::AsyncWrite;
 
@@ -223,7 +223,19 @@ fn content_encoder(buf: BytesMut, req: &mut ClientRequest) -> Output {
 
     let transfer = match body {
         Body::Empty => {
-            req.headers_mut().remove(CONTENT_LENGTH);
+            match req.method() {
+                //Insert zero content-length only if user hasn't added it.
+                //We don't really need it for other methods as they are not supposed to carry payload
+                &Method::POST | &Method::PUT | &Method::PATCH => {
+                    req.headers_mut()
+                       .entry(CONTENT_LENGTH)
+                       .expect("CONTENT_LENGTH to be valid header name")
+                       .or_insert(header::HeaderValue::from_static("0"));
+                },
+                _ => {
+                    req.headers_mut().remove(CONTENT_LENGTH);
+                }
+            }
             return Output::Empty(buf);
         }
         Body::Binary(ref mut bytes) => {
@@ -410,3 +422,76 @@ impl CachedDate {
         self.next_update.nsec = 0;
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn test_content_encoder_empty_body() {
+        let mut req = ClientRequest::post("http://google.com").finish().expect("Create request");
+
+        let result = content_encoder(BytesMut::new(), &mut req);
+
+        match result {
+            Output::Empty(buf) => {
+                assert_eq!(buf.len(), 0);
+                let content_len = req.headers().get(CONTENT_LENGTH).expect("To set Content-Length for empty POST");
+                assert_eq!(content_len, "0");
+            },
+            _ => panic!("Unexpected result, should be Output::Empty"),
+        }
+
+        req.set_method(Method::GET);
+
+        let result = content_encoder(BytesMut::new(), &mut req);
+
+        match result {
+            Output::Empty(buf) => {
+                assert_eq!(buf.len(), 0);
+                assert!(!req.headers().contains_key(CONTENT_LENGTH));
+            },
+            _ => panic!("Unexpected result, should be Output::Empty"),
+        }
+
+        req.set_method(Method::PUT);
+
+        let result = content_encoder(BytesMut::new(), &mut req);
+
+        match result {
+            Output::Empty(buf) => {
+                assert_eq!(buf.len(), 0);
+                let content_len = req.headers().get(CONTENT_LENGTH).expect("To set Content-Length for empty PUT");
+                assert_eq!(content_len, "0");
+            },
+            _ => panic!("Unexpected result, should be Output::Empty"),
+        }
+
+        req.set_method(Method::DELETE);
+
+        let result = content_encoder(BytesMut::new(), &mut req);
+
+        match result {
+            Output::Empty(buf) => {
+                assert_eq!(buf.len(), 0);
+                assert!(!req.headers().contains_key(CONTENT_LENGTH));
+            },
+            _ => panic!("Unexpected result, should be Output::Empty"),
+        }
+
+        req.set_method(Method::PATCH);
+
+        let result = content_encoder(BytesMut::new(), &mut req);
+
+        match result {
+            Output::Empty(buf) => {
+                assert_eq!(buf.len(), 0);
+                let content_len = req.headers().get(CONTENT_LENGTH).expect("To set Content-Length for empty PATCH");
+                assert_eq!(content_len, "0");
+            },
+            _ => panic!("Unexpected result, should be Output::Empty"),
+        }
+
+
+    }
+}


Test patch:
diff --git a/tests/test_client.rs b/tests/test_client.rs
index 9808f3e6..f3151e3a 100644
--- a/tests/test_client.rs
+++ b/tests/test_client.rs
@@ -506,3 +506,31 @@ fn client_read_until_eof() {
     let bytes = sys.block_on(response.body()).unwrap();
     assert_eq!(bytes, Bytes::from_static(b"welcome!"));
 }
+
+#[test]
+fn client_basic_auth() {
+    let mut srv =
+        test::TestServer::new(|app| app.handler(|_| HttpResponse::Ok().body(STR)));
+    /// set authorization header to Basic <base64 encoded username:password>
+    let request = srv
+                    .get()
+                    .basic_auth("username", Some("password"))
+                    .finish()
+                    .unwrap();
+    let repr = format!("{:?}", request);
+    assert!(repr.contains("Basic dXNlcm5hbWU6cGFzc3dvcmQ="));
+}
+
+#[test]
+fn client_bearer_auth() {
+    let mut srv =
+        test::TestServer::new(|app| app.handler(|_| HttpResponse::Ok().body(STR)));
+    /// set authorization header to Bearer <token>
+    let request = srv
+                    .get()
+                    .bearer_auth("someS3cr3tAutht0k3n")
+                    .finish()
+                    .unwrap();
+    let repr = format!("{:?}", request);
+    assert!(repr.contains("Bearer someS3cr3tAutht0k3n"));
+}
\ No newline at end of file