Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix basic auth #293

Merged
merged 10 commits into from
Aug 20, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ Unreleased changes are available as `avenga/couper:edge` container.
* Documentation of [`request.query.<name>`](./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))

Expand Down
18 changes: 9 additions & 9 deletions accesscontrol/basic_auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 "
Expand All @@ -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 {
Expand Down
102 changes: 32 additions & 70 deletions accesscontrol/basic_auth_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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},
Expand Down Expand Up @@ -89,97 +90,58 @@ 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)}

if err := ba.Validate(req); err != couperErr.Configuration {
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)
Expand Down
6 changes: 6 additions & 0 deletions accesscontrol/saml2_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down Expand Up @@ -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")
Expand Down
20 changes: 10 additions & 10 deletions docs/REFERENCE.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)| &#9888; 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) | &#9888; 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

Expand Down
2 changes: 1 addition & 1 deletion server/tls_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"} {
Expand Down