context for access controls errors (#154)

* context for access controls errors * tests for access_control log messages * name conflict Co-authored-by: Johannes Koch <johannes.koch@avenga.com>
coupergateway · Mar 19, 2021 · 240fb74 · 240fb74
1 parent a877f8f
commit 240fb74
Show file tree

Hide file tree

Showing 7 changed files with 130 additions and 33 deletions.
diff --git a/config/request/context_key.go b/config/request/context_key.go
@@ -4,6 +4,7 @@ type ContextKey uint8
 
 const (
 	UID ContextKey = iota
+	AccessControl
 	AccessControls
 	BackendName
 	Endpoint

diff --git a/config/runtime/server.go b/config/runtime/server.go
@@ -27,6 +27,7 @@ import (
 	"github.com/avenga/couper/errors"
 	"github.com/avenga/couper/eval"
 	"github.com/avenga/couper/handler"
+	hac "github.com/avenga/couper/handler/ac"
 	"github.com/avenga/couper/handler/middleware"
 	"github.com/avenga/couper/handler/producer"
 	"github.com/avenga/couper/handler/transport"
@@ -545,7 +546,7 @@ func configureProtectedHandler(m ac.Map, errTpl *errors.Template, parentAC, hand
 		acList = append(acList, m[acName])
 	}
 	if len(acList) > 0 {
-		return handler.NewAccessControl(h, errTpl, acList...)
+		return hac.NewAccessControl(h, errTpl, acList...)
 	}
 	return h
 }

diff --git a/handler/access_control.go → handler/ac/access_control.go b/handler/access_control.go → handler/ac/access_control.go
@@ -1,25 +1,26 @@
-package handler
+package ac
 
 import (
 	"net/http"
 
-	ac "github.com/avenga/couper/accesscontrol"
+	"github.com/avenga/couper/accesscontrol"
+	"github.com/avenga/couper/config/request"
 	"github.com/avenga/couper/errors"
 )
 
 var (
-	_ http.Handler         = &AccessControl{}
-	_ errors.ErrorTemplate = &AccessControl{}
-	_ ac.ProtectedHandler  = &AccessControl{}
+	_ http.Handler                   = &AccessControl{}
+	_ errors.ErrorTemplate           = &AccessControl{}
+	_ accesscontrol.ProtectedHandler = &AccessControl{}
 )
 
 type AccessControl struct {
-	ac        ac.List
+	ac        accesscontrol.List
 	errorTpl  *errors.Template
 	protected http.Handler
 }
 
-func NewAccessControl(protected http.Handler, errTpl *errors.Template, list ...ac.AccessControl) *AccessControl {
+func NewAccessControl(protected http.Handler, errTpl *errors.Template, list ...accesscontrol.AccessControl) *AccessControl {
 	return &AccessControl{
 		ac:        list,
 		errorTpl:  errTpl,
@@ -31,7 +32,7 @@ func (a *AccessControl) ServeHTTP(rw http.ResponseWriter, req *http.Request) {
 	for _, control := range a.ac {
 		if err := control.Validate(req); err != nil {
 			var code errors.Code
-			if authError, ok := err.(*ac.BasicAuthError); ok {
+			if authError, ok := err.(*accesscontrol.BasicAuthError); ok {
 				code = errors.BasicAuthFailed
 				wwwAuthenticateValue := "Basic"
 				if authError.Realm != "" {
@@ -40,14 +41,17 @@ func (a *AccessControl) ServeHTTP(rw http.ResponseWriter, req *http.Request) {
 				rw.Header().Set("WWW-Authenticate", wwwAuthenticateValue)
 			} else {
 				switch err {
-				case ac.ErrorNotConfigured:
+				case accesscontrol.ErrorNotConfigured:
 					code = errors.Configuration
-				case ac.ErrorEmptyToken:
+				case accesscontrol.ErrorEmptyToken:
 					code = errors.AuthorizationRequired
 				default:
 					code = errors.AuthorizationFailed
 				}
 			}
+			if ctx, ok := req.Context().Value(request.AccessControl).(*AccessControlContext); ok {
+				ctx.errors = append(ctx.errors, err)
+			}
 			a.errorTpl.ServeError(code).ServeHTTP(rw, req)
 			return
 		}

diff --git a/handler/ac/access_control_context.go b/handler/ac/access_control_context.go
@@ -0,0 +1,27 @@
+package ac
+
+import (
+	"context"
+
+	"github.com/avenga/couper/config/request"
+)
+
+type AccessControlContext struct {
+	errors []error
+}
+
+func NewWithContext(ctx context.Context) (context.Context, *AccessControlContext) {
+	octx := &AccessControlContext{}
+	return context.WithValue(ctx, request.AccessControl, octx), octx
+}
+
+func (o *AccessControlContext) Errors() []string {
+	if len(o.errors) == 0 {
+		return nil
+	}
+	var result []string
+	for _, e := range o.errors {
+		result = append(result, e.Error())
+	}
+	return result
+}
diff --git a/handler/access_control_test.go → handler/ac/access_control_test.go b/handler/access_control_test.go → handler/ac/access_control_test.go
@@ -1,4 +1,4 @@
-package handler
+package ac
 
 import (
 	"fmt"

diff --git a/logging/access_log.go b/logging/access_log.go
@@ -11,6 +11,7 @@ import (
 
 	"github.com/avenga/couper/config/request"
 	"github.com/avenga/couper/errors"
+	"github.com/avenga/couper/handler/ac"
 )
 
 type RoundtripHandlerFunc http.HandlerFunc
@@ -35,13 +36,20 @@ func (log *AccessLog) ServeHTTP(rw http.ResponseWriter, req *http.Request, nextH
 	statusRecorder := NewStatusRecorder(rw)
 	rw = statusRecorder
 
+	oCtx, acContext := ac.NewWithContext(req.Context())
+	*req = *req.WithContext(oCtx)
+
 	nextHandler.ServeHTTP(rw, req)
 	serveDone := time.Now()
 
 	fields := Fields{
 		"proto": req.Proto,
 	}
 
+	if acErrors := acContext.Errors(); len(acErrors) > 0 {
+		fields["access_control"] = acErrors
+	}
+
 	backendName, _ := req.Context().Value(request.BackendName).(string)
 	if backendName == "" {
 		endpointName, _ := req.Context().Value(request.Endpoint).(string)

diff --git a/server/http_integration_test.go b/server/http_integration_test.go
@@ -1312,31 +1312,34 @@ func TestConfigBodyContentBackends(t *testing.T) {
 func TestConfigBodyContentAccessControl(t *testing.T) {
 	client := newClient()
 
-	shutdown, _ := newCouper("testdata/integration/config/03_couper.hcl", test.New(t))
+	shutdown, hook := newCouper("testdata/integration/config/03_couper.hcl", test.New(t))
 	defer shutdown()
 
 	type testCase struct {
-		path   string
-		header http.Header
-		status int
-		ct     string
+		path       string
+		header     http.Header
+		status     int
+		ct         string
+		wantErrLog string
 	}
 
 	for _, tc := range []testCase{
-		{"/v1", http.Header{"Auth": []string{"ba1"}}, http.StatusOK, "application/json"},
+		{"/v1", http.Header{"Auth": []string{"ba1"}}, http.StatusOK, "application/json", ""},
 		// TODO: Can a disabled auth being enabled again?
 		//{"/v1", http.Header{"Authorization": []string{"Basic OmFzZGY="}, "Auth": []string{"ba1"}}, http.StatusOK, "application/json"},
 		//{"/v1", http.Header{"Auth": []string{}}, http.StatusUnauthorized, "application/json"},
-		{"/v2", http.Header{"Authorization": []string{"Basic OmFzZGY="}, "Auth": []string{"ba1", "ba2"}}, http.StatusOK, "application/json"}, // minimum ':'
-		{"/v2", http.Header{}, http.StatusUnauthorized, "application/json"},
-		{"/v3", http.Header{}, http.StatusOK, "application/json"},
-		{"/status", http.Header{}, http.StatusOK, "application/json"},
-		{"/superadmin", http.Header{"Authorization": []string{"Basic OmFzZGY="}, "Auth": []string{"ba1", "ba4"}}, http.StatusOK, "application/json"},
-		{"/superadmin", http.Header{}, http.StatusUnauthorized, "application/json"},
-		{"/v4", http.Header{}, http.StatusUnauthorized, "text/html"},
+		{"/v2", http.Header{"Authorization": []string{"Basic OmFzZGY="}, "Auth": []string{"ba1", "ba2"}}, http.StatusOK, "application/json", ""}, // minimum ':'
+		{"/v2", http.Header{}, http.StatusUnauthorized, "application/json", "missing credentials"},
+		{"/v3", http.Header{}, http.StatusOK, "application/json", ""},
+		{"/status", http.Header{}, http.StatusOK, "application/json", ""},
+		{"/superadmin", http.Header{"Authorization": []string{"Basic OmFzZGY="}, "Auth": []string{"ba1", "ba4"}}, http.StatusOK, "application/json", ""},
+		{"/superadmin", http.Header{}, http.StatusUnauthorized, "application/json", "missing credentials"},
+		{"/v4", http.Header{}, http.StatusUnauthorized, "text/html", "missing credentials"},
 	} {
 		t.Run(tc.path[1:], func(subT *testing.T) {
 			helper := test.New(subT)
+			hook.Reset()
+
 			req, err := http.NewRequest(http.MethodGet, "http://back.end:8080"+tc.path, nil)
 			helper.Must(err)
 
@@ -1347,6 +1350,24 @@ func TestConfigBodyContentAccessControl(t *testing.T) {
 			res, err := client.Do(req)
 			helper.Must(err)
 
+			acEntries := getAccessControlMessages(hook)
+			if tc.wantErrLog == "" {
+				if len(acEntries) > 0 {
+					t.Errorf("Expected error log: %q, actual: %#v", tc.wantErrLog, acEntries)
+				}
+			} else {
+				var found bool
+				for _, valMsg := range acEntries {
+					if strings.HasPrefix(valMsg, tc.wantErrLog) {
+						found = true
+						break
+					}
+				}
+				if !found {
+					t.Errorf("Expected error log: %q, actual: %#v", tc.wantErrLog, acEntries)
+				}
+			}
+
 			if res.StatusCode != tc.status {
 				t.Errorf("%q: expected Status %d, got: %d", tc.path, tc.status, res.StatusCode)
 				return
@@ -1385,23 +1406,26 @@ func TestConfigBodyContentAccessControl(t *testing.T) {
 func TestJWTAccessControl(t *testing.T) {
 	client := newClient()
 
-	shutdown, _ := newCouper("testdata/integration/config/03_couper.hcl", test.New(t))
+	shutdown, hook := newCouper("testdata/integration/config/03_couper.hcl", test.New(t))
 	defer shutdown()
 
 	type testCase struct {
-		name   string
-		path   string
-		header http.Header
-		status int
+		name       string
+		path       string
+		header     http.Header
+		status     int
+		wantErrLog string
 	}
 
 	for _, tc := range []testCase{
-		{"no token", "/jwt", http.Header{}, http.StatusUnauthorized},
-		{"expired token", "/jwt", http.Header{"Authorization": []string{"Bearer eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiaWF0IjoxNTE2MjM5MDIyLCJleHAiOjEyMzQ1Njc4OX0.wLWj9XgBZAPoDYPXsmDrEBzR6BUWfwPqQNlR_F0naZA"}}, http.StatusForbidden},
-		{"valid token", "/jwt", http.Header{"Authorization": []string{"Bearer eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiaWF0IjoxNTE2MjM5MDIyfQ.Qf0lkeZKZ3NJrYm3VdgiQiQ6QTrjCvISshD_q9F8GAM"}}, http.StatusOK},
+		{"no token", "/jwt", http.Header{}, http.StatusUnauthorized, "empty token"},
+		{"expired token", "/jwt", http.Header{"Authorization": []string{"Bearer eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiaWF0IjoxNTE2MjM5MDIyLCJleHAiOjEyMzQ1Njc4OX0.wLWj9XgBZAPoDYPXsmDrEBzR6BUWfwPqQNlR_F0naZA"}}, http.StatusForbidden, "token is expired by "},
+		{"valid token", "/jwt", http.Header{"Authorization": []string{"Bearer eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiaWF0IjoxNTE2MjM5MDIyfQ.Qf0lkeZKZ3NJrYm3VdgiQiQ6QTrjCvISshD_q9F8GAM"}}, http.StatusOK, ""},
 	} {
 		t.Run(tc.path[1:], func(subT *testing.T) {
 			helper := test.New(subT)
+			hook.Reset()
+
 			req, err := http.NewRequest(http.MethodGet, "http://back.end:8080"+tc.path, nil)
 			helper.Must(err)
 
@@ -1417,6 +1441,24 @@ func TestJWTAccessControl(t *testing.T) {
 				return
 			}
 
+			acEntries := getAccessControlMessages(hook)
+			if tc.wantErrLog == "" {
+				if len(acEntries) > 0 {
+					t.Errorf("Expected error log: %q, actual: %#v", tc.wantErrLog, acEntries)
+				}
+			} else {
+				var found bool
+				for _, valMsg := range acEntries {
+					if strings.HasPrefix(valMsg, tc.wantErrLog) {
+						found = true
+						break
+					}
+				}
+				if !found {
+					t.Errorf("Expected error log: %q, actual: %#v", tc.wantErrLog, acEntries)
+				}
+			}
+
 			if res.StatusCode != http.StatusOK {
 				return
 			}
@@ -1428,6 +1470,20 @@ func TestJWTAccessControl(t *testing.T) {
 	}
 }
 
+func getAccessControlMessages(hook *logrustest.Hook) []string {
+	var acEntries []string
+	for _, entry := range hook.Entries {
+		if valEntry, ok := entry.Data["access_control"]; ok {
+			if list, ok := valEntry.([]string); ok {
+				for _, valMsg := range list {
+					acEntries = append(acEntries, valMsg)
+				}
+			}
+		}
+	}
+	return acEntries
+}
+
 func TestWrapperHiJack_WebsocketUpgrade(t *testing.T) {
 	t.Skip("TODO fix hijack and endpoint handling for ws")
 	helper := test.New(t)