diff --git a/CHANGELOG.md b/CHANGELOG.md index 4ac4f8768..eb41d6651 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,6 +25,7 @@ Unreleased changes are available as `avenga/couper:edge` container. * Documentation of [`request.query.`](./docs/REFERENCE.md#request) ([#278](https://github.com/avenga/couper/pull/278)) * Missing access log on some error cases ([#267](https://github.com/avenga/couper/issues/267)) * Panic during backend origin / url usage with previous parse error ([#206](https://github.com/avenga/couper/issues/206)) + * [Basic Auth](./docs/REFERENCE.md#basic-auth-block) did not work if only the `htpasswd_file` attribute was defined ([#293](https://github.com/avenga/couper/pull/293)) * Missing error handling for backend gzip header reads ([#291](https://github.com/avenga/couper/pull/291)) * ResponseWriter fallback for possible statusCode 0 writes ([#291](https://github.com/avenga/couper/pull/291)) diff --git a/accesscontrol/basic_auth.go b/accesscontrol/basic_auth.go index f017325cc..c517ccf10 100644 --- a/accesscontrol/basic_auth.go +++ b/accesscontrol/basic_auth.go @@ -105,12 +105,6 @@ func (ba *BasicAuth) Validate(req *http.Request) error { return errors.Configuration } - if ba.pass == "" { - // prevent granting access if password is no set - // or evaluated to an empty string. - return errors.BasicAuth.Message("no password configured") - } - user, pass, ok := req.BasicAuth() if !ok { // false is unspecific, determine if credentials are set const prefix = "Basic " @@ -121,10 +115,16 @@ func (ba *BasicAuth) Validate(req *http.Request) error { } if ba.user == user { - if subtle.ConstantTimeCompare([]byte(ba.pass), []byte(pass)) == 1 { - return nil + if ba.pass != "" { + if subtle.ConstantTimeCompare([]byte(ba.pass), []byte(pass)) == 1 { + return nil + } + return errors.BasicAuth.Message("credential mismatch") + } + + if len(ba.htFile) == 0 { + return errors.BasicAuth.Message("no password configured") } - return errors.BasicAuth.Message("credential mismatch") } if len(ba.htFile) > 0 { diff --git a/accesscontrol/basic_auth_test.go b/accesscontrol/basic_auth_test.go index c9e38e8c3..43a0f91ae 100644 --- a/accesscontrol/basic_auth_test.go +++ b/accesscontrol/basic_auth_test.go @@ -32,6 +32,7 @@ func Test_NewBasicAuth(t *testing.T) { {"name", "", "pass", "", "", false}, {"name", "", "", "", "", false}, {"name", "user", "pass", "testdata/htpasswd", "", false}, + {"name", "", "", "testdata/htpasswd", "", false}, {"name", "john", "pass", "testdata/htpasswd", "", false}, {"name", "user", "pass", "file", "open file: no such file or directory", true}, {"name", "user", "pass", "testdata/htpasswd_err_invalid", "parse error: invalid line: 1", true}, @@ -89,7 +90,7 @@ func Test_BasicAuth_Validate(t *testing.T) { } } -func Test_BasicAuth_ValidateEmptyUser(t *testing.T) { +func Test_BasicAuth_ValidateCases(t *testing.T) { var ba *ac.BasicAuth req := &http.Request{Header: make(http.Header)} @@ -97,89 +98,50 @@ func Test_BasicAuth_ValidateEmptyUser(t *testing.T) { t.Errorf("Expected configuration error, got: %v", err) } - ba, err := ac.NewBasicAuth("name", "", "pass", "") - if err != nil || ba == nil { + ba1, err := ac.NewBasicAuth("name", "", "pass", "") + if err != nil || ba1 == nil { t.Fatal("Expected a basic auth object") } - - type testCase struct { - headerValue string - expErr error - } - - for _, testcase := range []testCase{ - {"Basic " + b64.StdEncoding.EncodeToString([]byte("user:pass")), couperErr.BasicAuth}, - {"Basic " + b64.StdEncoding.EncodeToString([]byte("user:")), couperErr.BasicAuth}, - {"Basic " + b64.StdEncoding.EncodeToString([]byte(":pass")), nil}, - {"Basic " + b64.StdEncoding.EncodeToString([]byte(":")), couperErr.BasicAuth}, - } { - t.Run(testcase.headerValue, func(t *testing.T) { - req.Header.Set("Authorization", testcase.headerValue) - err := ba.Validate(req) - if testcase.expErr != nil && !errors.As(err, &testcase.expErr) { - t.Errorf("Expected Unauthorized error, got: %v", err) - } - }) - } -} - -func Test_BasicAuth_ValidateEmptyPassword(t *testing.T) { - var ba *ac.BasicAuth - req := &http.Request{Header: make(http.Header)} - - if err := ba.Validate(req); err != couperErr.Configuration { - t.Errorf("Expected NotConfigured error, got: %v", err) - } - - ba, err := ac.NewBasicAuth("name", "user", "", "") - if err != nil || ba == nil { + ba2, err := ac.NewBasicAuth("name", "user", "", "") + if err != nil || ba2 == nil { t.Fatal("Expected a basic auth object") } - - type testCase struct { - headerValue string - expErr error - } - - for _, testcase := range []testCase{ - {"Basic " + b64.StdEncoding.EncodeToString([]byte("user:pass")), couperErr.BasicAuth}, - {"Basic " + b64.StdEncoding.EncodeToString([]byte("user:")), couperErr.BasicAuth}, - {"Basic " + b64.StdEncoding.EncodeToString([]byte(":pass")), couperErr.BasicAuth}, - {"Basic " + b64.StdEncoding.EncodeToString([]byte(":")), couperErr.BasicAuth}, - } { - t.Run(testcase.headerValue, func(t *testing.T) { - req.Header.Set("Authorization", testcase.headerValue) - err := ba.Validate(req) - if testcase.expErr != nil && !errors.As(err, &testcase.expErr) { - t.Errorf("Expected Unauthorized error, got: %v", err) - } - }) - } -} - -func Test_BasicAuth_ValidateEmptyUserPassword(t *testing.T) { - var ba *ac.BasicAuth - req := &http.Request{Header: make(http.Header)} - - if err := ba.Validate(req); err != couperErr.Configuration { - t.Errorf("Expected NotConfigured error, got: %v", err) + ba3, err := ac.NewBasicAuth("name", "", "", "") + if err != nil || ba3 == nil { + t.Fatal("Expected a basic auth object") } - - ba, err := ac.NewBasicAuth("name", "", "", "") - if err != nil || ba == nil { + ba4, err := ac.NewBasicAuth("name", "", "", "testdata/htpasswd") + if err != nil || ba4 == nil { t.Fatal("Expected a basic auth object") } type testCase struct { + ba *ac.BasicAuth headerValue string expErr error } for _, testcase := range []testCase{ - {"Basic " + b64.StdEncoding.EncodeToString([]byte("user:pass")), couperErr.BasicAuth}, - {"Basic " + b64.StdEncoding.EncodeToString([]byte("user:")), couperErr.BasicAuth}, - {"Basic " + b64.StdEncoding.EncodeToString([]byte(":pass")), couperErr.BasicAuth}, - {"Basic " + b64.StdEncoding.EncodeToString([]byte(":")), couperErr.BasicAuth}, + {ba1, "Basic " + b64.StdEncoding.EncodeToString([]byte("user:pass")), couperErr.BasicAuth}, + {ba1, "Basic " + b64.StdEncoding.EncodeToString([]byte("user:")), couperErr.BasicAuth}, + {ba1, "Basic " + b64.StdEncoding.EncodeToString([]byte(":pass")), nil}, + {ba1, "Basic " + b64.StdEncoding.EncodeToString([]byte(":")), couperErr.BasicAuth}, + {ba2, "Basic " + b64.StdEncoding.EncodeToString([]byte("user:pass")), couperErr.BasicAuth}, + {ba2, "Basic " + b64.StdEncoding.EncodeToString([]byte("user:")), couperErr.BasicAuth}, + {ba2, "Basic " + b64.StdEncoding.EncodeToString([]byte(":pass")), couperErr.BasicAuth}, + {ba2, "Basic " + b64.StdEncoding.EncodeToString([]byte(":")), couperErr.BasicAuth}, + {ba3, "Basic " + b64.StdEncoding.EncodeToString([]byte("user:pass")), couperErr.BasicAuth}, + {ba3, "Basic " + b64.StdEncoding.EncodeToString([]byte("user:")), couperErr.BasicAuth}, + {ba3, "Basic " + b64.StdEncoding.EncodeToString([]byte(":pass")), couperErr.BasicAuth}, + {ba3, "Basic " + b64.StdEncoding.EncodeToString([]byte(":")), couperErr.BasicAuth}, + {ba4, "Basic " + b64.StdEncoding.EncodeToString([]byte("john:my-pass")), nil}, + {ba4, "Basic " + b64.StdEncoding.EncodeToString([]byte("john:")), couperErr.BasicAuth}, + {ba4, "Basic " + b64.StdEncoding.EncodeToString([]byte(":my-pass")), couperErr.BasicAuth}, + {ba4, "Basic " + b64.StdEncoding.EncodeToString([]byte(":")), couperErr.BasicAuth}, + {ba4, "Basic " + b64.StdEncoding.EncodeToString([]byte("user:pass")), couperErr.BasicAuth}, + {ba4, "Basic " + b64.StdEncoding.EncodeToString([]byte("user:")), couperErr.BasicAuth}, + {ba4, "Basic " + b64.StdEncoding.EncodeToString([]byte(":pass")), couperErr.BasicAuth}, + {ba4, "Basic " + b64.StdEncoding.EncodeToString([]byte(":")), couperErr.BasicAuth}, } { t.Run(testcase.headerValue, func(t *testing.T) { req.Header.Set("Authorization", testcase.headerValue) diff --git a/accesscontrol/saml2_test.go b/accesscontrol/saml2_test.go index 4587f3ed6..d36bfae73 100644 --- a/accesscontrol/saml2_test.go +++ b/accesscontrol/saml2_test.go @@ -54,6 +54,9 @@ func Test_NewSAML2ACS(t *testing.T) { func Test_SAML2ACS_Validate(t *testing.T) { metadata, err := reader.ReadFromAttrFile("saml2", "", "testdata/idp-metadata.xml") + if err != nil || metadata == nil { + t.Fatal("Expected a metadata object") + } sa, err := ac.NewSAML2ACS(metadata, "test", "http://www.examle.org/saml/acs", "my-sp-entity-id", []string{"memberOf"}) if err != nil || sa == nil { t.Fatal("Expected a saml acs object") @@ -136,6 +139,9 @@ func Test_SAML2ACS_ValidateAssertionInfo(t *testing.T) { func Test_SAML2ACS_GetAssertionData(t *testing.T) { metadata, err := reader.ReadFromAttrFile("saml2", "", "testdata/idp-metadata.xml") + if err != nil || metadata == nil { + t.Fatal("Expected a metadata object") + } sa, err := ac.NewSAML2ACS(metadata, "test", "http://www.examle.org/saml/acs", "my-sp-entity-id", []string{"memberOf"}) if err != nil || sa == nil { t.Fatal("Expected a saml acs object") diff --git a/docs/REFERENCE.md b/docs/REFERENCE.md index bf295f3fc..72da402e5 100644 --- a/docs/REFERENCE.md +++ b/docs/REFERENCE.md @@ -302,16 +302,16 @@ credentials from the `Authorization` request HTTP header field are checked again `user`/`password` if the user matches, and against the data in the file referenced by `htpasswd_file` otherwise. -|Block name|Context|Label|Nested block(s)| -| :-----------| :-----------| :-----------| :-----------| -|`basic_auth`| [Definitions Block](#definitions-block)| ⚠ required |-| - -| Attribute(s) | Type |Default|Description|Characteristic(s)| Example| -| :------------------------------ | :--------------- | :--------------- | :--------------- | :--------------- | :--------------- | -| `user` |string|-|The user name.|-|-| -| `password` |string|-|The corresponding password.|-|-| -| `htpasswd_file` |string|-|>The htpasswd file.|-|-| -|`realm` |string|-|The realm to be sent in a `WWW-Authenticate` response HTTP header field.|-|-| +| Block name | Context | Label | Nested block(s) | +| :----------- | :------ | :---- | :-------------- | +| `basic_auth` | [Definitions Block](#definitions-block) | ⚠ required | - | + +| Attribute(s) | Type | Default | Description | Characteristic(s) | Example | +| :-------------- | :----- | :------ | :---------- | :---------------- | :------ | +| `user` | string | `""` | The user name. | - | - | +| `password` | string | `""` | The corresponding password. | - | - | +| `htpasswd_file` | string | `""` | The htpasswd file. | Couper uses [Apache's httpasswd](https://httpd.apache.org/docs/current/programs/htpasswd.html) file format. `apr1`, `md5` and `bcrypt` password encryptions are supported. The file is loaded once at startup. Restart Couper after you have changed it. | - | +| `realm` | string | `""` | The realm to be sent in a `WWW-Authenticate` response HTTP header field. | - | - | ### JWT Block diff --git a/server/tls_test.go b/server/tls_test.go index e6aea91e9..2e349616f 100644 --- a/server/tls_test.go +++ b/server/tls_test.go @@ -49,7 +49,7 @@ settings { }, } - time.Sleep(time.Second) // tls server needs some time. + time.Sleep(2 * time.Second) // tls server needs some time. for _, p := range []string{"8443", "9443"} { for _, host := range []string{"127.0.0.1", "localhost", "couper.dev"} {