Repository: istio/fortio
Pull request #261 (2018-06-20 18:27:38)

Basic Authentication Support
Basic authentication support is added.(#256)

istio-testing: [APPROVALNOTIFIER] This PR is **NOT APPROVED**

This pull-request has been approved by: 
To fully approve this pull request, please assign additional approvers.
We suggest the following additional approvers: 

Assign the PR to them by writing `/assign ` in a comment when ready.

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands).

The pull request process is described [here](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process)

<details open>
Needs approval from an approver in each of these files:

- **[OWNERS](https://github.com/istio/fortio/blob/master/OWNERS)**

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":[]} -->
istio-testing: Hi @olimpias. Thanks for your PR.

I'm waiting for a [istio](https://github.com/orgs/istio/people) member to verify that this patch is reasonable to test. If it is, they should reply with `/ok-to-test` on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should [join the org](https://github.com/orgs/istio/people) to skip this step.

I understand the commands that are listed [here](https://go.k8s.io/bot-commands).

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
</details>

olimpias: I couldnt figure out why shifting the sentence isn't worked as expected in README.md file. Any idea?
codecov[bot]: # [Codecov](https://codecov.io/gh/istio/fortio/pull/261?src=pr&el=h1) Report
> Merging [#261](https://codecov.io/gh/istio/fortio/pull/261?src=pr&el=desc) into [master](https://codecov.io/gh/istio/fortio/commit/d7a328cb647137d056aa78e712c10e3e1f281f46?src=pr&el=desc) will **increase** coverage by `<.1%`.
> The diff coverage is `83.3%`.

[![Impacted file tree graph](https://codecov.io/gh/istio/fortio/pull/261/graphs/tree.svg?width=650&height=150&src=pr&token=6HLno2jpvU)](https://codecov.io/gh/istio/fortio/pull/261?src=pr&el=tree)

```diff
@@           Coverage Diff            @@
##           master    #261     +/-   ##
========================================
+ Coverage    90.6%   90.6%   +<.1%     
========================================
  Files          10      10             
  Lines        1911    1951     +40     
========================================
+ Hits         1731    1768     +37     
- Misses        116     117      +1     
- Partials       64      66      +2
```


| [Impacted Files](https://codecov.io/gh/istio/fortio/pull/261?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [fhttp/http\_utils.go](https://codecov.io/gh/istio/fortio/pull/261/diff?src=pr&el=tree#diff-Zmh0dHAvaHR0cF91dGlscy5nbw==) | `100% <100%> (ø)` | :arrow_up: |
| [fhttp/http\_client.go](https://codecov.io/gh/istio/fortio/pull/261/diff?src=pr&el=tree#diff-Zmh0dHAvaHR0cF9jbGllbnQuZ28=) | `76.1% <33.3%> (-0.3%)` | :arrow_down: |
| [fgrpc/grpcrunner.go](https://codecov.io/gh/istio/fortio/pull/261/diff?src=pr&el=tree#diff-ZmdycGMvZ3JwY3J1bm5lci5nbw==) | `92.4% <0%> (+0.9%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/istio/fortio/pull/261?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/istio/fortio/pull/261?src=pr&el=footer). Last update [d7a328c...0a428d8](https://codecov.io/gh/istio/fortio/pull/261?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Patch:
diff --git a/README.md b/README.md
index dff7136..d05f0a7 100644
--- a/README.md
+++ b/README.md
@@ -204,6 +204,9 @@ target is a url (http load tests) or host:port (grpc health test).  flags are:
   -ui-path string
 	http server URI for UI, empty turns off that part (more secure) (default
 	"/fortio/")
+  -user string
+    	User credentials for basic authentication (for http). Input data format
+    	should be user:password
 </pre>
 </details>
 
diff --git a/bincommon/commonflags.go b/bincommon/commonflags.go
index aef57fa..6769d16 100644
--- a/bincommon/commonflags.go
+++ b/bincommon/commonflags.go
@@ -65,6 +65,8 @@ var (
 	headersFlags        headersFlagList
 	httpOpts            fhttp.HTTPOptions
 	followRedirectsFlag = flag.Bool("L", false, "Follow redirects (implies -std-client) - do not use for load test")
+	userCredentialsFlag = flag.String("user", "", "User credentials for basic authentication (for http). Input data format"+
+		" should be user:password")
 	// QuietFlag is the value of -quiet.
 	QuietFlag = flag.Bool("quiet", false, "Quiet mode: sets the loglevel to Error and reduces the output.")
 )
@@ -118,6 +120,7 @@ func SharedHTTPOptions() *fhttp.HTTPOptions {
 	httpOpts.Compression = *compressionFlag
 	httpOpts.HTTPReqTimeOut = *httpReqTimeoutFlag
 	httpOpts.Insecure = *httpsInsecureFlag
+	httpOpts.UserCredentials = *userCredentialsFlag
 	if *followRedirectsFlag {
 		httpOpts.FollowRedirects = true
 		httpOpts.DisableFastClient = true
diff --git a/fhttp/http_client.go b/fhttp/http_client.go
index 2c54cf9..43d87da 100644
--- a/fhttp/http_client.go
+++ b/fhttp/http_client.go
@@ -133,6 +133,8 @@ type HTTPOptions struct {
 	// Host is treated specially, remember that one separately.
 	hostOverride   string
 	HTTPReqTimeOut time.Duration // timeout value for http request
+
+	UserCredentials string // user credentials for authorization
 }
 
 // ResetHeaders resets all the headers, including the User-Agent one.
@@ -145,6 +147,24 @@ func (h *HTTPOptions) ResetHeaders() {
 func (h *HTTPOptions) InitHeaders() {
 	h.ResetHeaders()
 	h.extraHeaders.Add("User-Agent", userAgent)
+	err := h.ValidateAndAddBasicAuthentication()
+	if err != nil {
+		log.Errf("User credential is not valid. %v", err)
+	}
+}
+
+// ValidateAndAddBasicAuthentication validates user credentials and adds basic authentication to http header,
+// if user credentials are valid.
+func (h *HTTPOptions) ValidateAndAddBasicAuthentication() error {
+	if len(h.UserCredentials) <= 0 {
+		return nil // user credential is not entered
+	}
+	s := strings.SplitN(h.UserCredentials, ":", 2)
+	if len(s) != 2 {
+		return fmt.Errorf("invalid user credentials are used %s. Expected format user:password", h.UserCredentials)
+	}
+	h.extraHeaders.Add("Authorization", generateBase64UserCredentials(h.UserCredentials))
+	return nil
 }
 
 // GetHeaders returns the current set of headers.
diff --git a/fhttp/http_utils.go b/fhttp/http_utils.go
index 3023898..da29992 100644
--- a/fhttp/http_utils.go
+++ b/fhttp/http_utils.go
@@ -15,6 +15,7 @@
 package fhttp // import "istio.io/fortio/fhttp"
 
 import (
+	"encoding/base64"
 	"fmt"
 	"html/template"
 	"io"
@@ -467,5 +468,9 @@ func AddHTTPS(url string) string {
 	// url must not contain any prefix, so add https prefix
 	log.Infof("Prepending https:// to url: %s", url)
 	return fnet.PrefixHTTPS + url
+}
 
+// generateBase64UserCredentials encodes the user credential to base64 and adds a Basic as prefix.
+func generateBase64UserCredentials(userCredentials string) string {
+	return "Basic " + base64.StdEncoding.EncodeToString([]byte(userCredentials))
 }


Test patch:
diff --git a/fhttp/http_test.go b/fhttp/http_test.go
index 760440c..ab7a613 100644
--- a/fhttp/http_test.go
+++ b/fhttp/http_test.go
@@ -968,6 +968,30 @@ func TestAddHTTPS(t *testing.T) {
 	}
 }
 
+func TestValidateAndAddBasicAuthentication(t *testing.T) {
+	var tests = []struct {
+		o                  HTTPOptions
+		isCredentialsValid bool
+		isAuthHeaderAdded  bool
+	}{
+		{HTTPOptions{UserCredentials: "foo:foo"}, true, true},
+		{HTTPOptions{UserCredentials: "foofoo"}, false, false},
+		{HTTPOptions{UserCredentials: ""}, true, false},
+	}
+
+	for _, test := range tests {
+		test.o.ResetHeaders()
+		err := test.o.ValidateAndAddBasicAuthentication()
+		if err == nil && !test.isCredentialsValid {
+			t.Errorf("Error was not expected for %s", test.o.UserCredentials)
+		}
+		if test.isAuthHeaderAdded && len(test.o.extraHeaders.Get("Authorization")) <= 0 {
+			t.Errorf("Authorization header was expected for %s credentials", test.o.UserCredentials)
+		}
+
+	}
+}
+
 // --- for bench mark/comparison
 
 func asciiFold0(str string) []byte {