feat: improvements in allowlist middleware (#975)

Signed-off-by: Miguel Martinez Trivino <[email protected]>
Signed-off-by: Javier Rodriguez <[email protected]>
Co-authored-by: Javier Rodriguez <[email protected]>

Author: migmartri

app/cli/main.go

@@ -63,8 +63,6 @@ func errorInfo(err error, logger zerolog.Logger) (string, int) {
 		msg = "you need to enable a CAS backend first. Refer to `chainloop cas-backend` command or contact your administrator."
 	case v1.IsCasBackendErrorReasonInvalid(err):
 		msg = "the CAS backend you provided is invalid. Refer to `chainloop cas-backend update` command or contact your administrator."
-	case v1.IsAllowListErrorNotInList(err):
-		msg = "your user is not part of the allow-list. Please contact your administrator."
 	case isWrappedErr(st, jwtMiddleware.ErrTokenExpired):
 		msg = "your authentication token has expired, please run chainloop auth login again"
 	case isWrappedErr(st, jwtMiddleware.ErrMissingJwtToken):

app/controlplane/configs/config.devel.yaml

@@ -53,6 +53,11 @@ auth:
 
   # Private key used to sign the JWTs meant to be consumed by the CAS
   cas_robot_account_private_key_path: "../../devel/devkeys/cas.pem"
+  # allow_list:
+    # rules: ["@chainloop.local"]
+    # selected_routes: ["/controlplane.v1.WorkflowRunService/List"]
+    # custom_message: "you need to require access here http://foo.com"
+    
 
 # referrer_shared_index:
 #   enabled: true

app/controlplane/configs/samples/config.yaml

@@ -16,8 +16,13 @@ auth:
   cas_robot_account_private_key_path: "./configs/devkeys/cas.private.key.devel"
   # Optional allow list
   # allow_list:
-  #   - [email protected]
-  #   - @my-domain.com # This will allow any email from this domain
+    # rules:
+    #   - [email protected]
+    #   - @my-domain.com # This will allow any email from this domain
+    # Optional selected routes, empty means all of them
+    # selected_routes: ["/controlplane.v1.WorkflowRunService/List"]
+    # Optional custom message
+    # custom_message: "you need to require access here http://foo.com"
 
 # CAS server where the controlplane will push artifacts to
 cas_server:

app/controlplane/internal/conf/controlplane/config/v1/conf.pb.go

@@ -18,9 +18,9 @@ syntax = "proto3";
 package controlplane.config.v1;
 
 import "buf/validate/validate.proto";
+import "controlplane/v1/response_messages.proto";
 import "credentials/v1/config.proto";
 import "google/protobuf/duration.proto";
-import "controlplane/v1/response_messages.proto";
 
 option go_package = "github.com/chainloop-dev/chainloop/app/controlplane/internal/conf/controlplane/config/v1;conf";
 
@@ -114,9 +114,7 @@ message Data {
 message Auth {
   // Authentication creates a JWT that uses this secret for signing
   string generated_jws_hmac_secret = 2;
-  // allow_list is a list of allowed email addresses or domains
-  // for example ["@chainloop.dev", "[email protected]"]
-  repeated string allow_list = 3;
+  AllowList allow_list = 3;
   string cas_robot_account_private_key_path = 4;
   OIDC oidc = 6;
 
@@ -126,6 +124,16 @@ message Auth {
     string client_secret = 3;
     string redirect_url_scheme = 4;
   }
+
+  message AllowList {
+    // allow_list is a list of allowed email addresses or domains
+    // for example ["@chainloop.dev", "[email protected]"]
+    repeated string rules = 1 [(buf.validate.field).string.min_len = 1];
+    // Custom message to show when a user is not allowed
+    string custom_message = 2;
+    // The list of routes that will be affected by this middleware, by default all of them
+    repeated string selected_routes = 3;
+  }
 }
 
 message CA {
@@ -143,7 +151,7 @@ message CA {
 // OnboardingSpec is a configuration to automatically onboard users in organizations with specific roles
 message OnboardingSpec {
   // Name of the organization
-  string name = 1 [(buf.validate.field).string.min_len = 1];;
+  string name = 1 [(buf.validate.field).string.min_len = 1];
   // Role to assign to the user
   controlplane.v1.MembershipRole role = 2;
 }

app/controlplane/internal/conf/controlplane/config/v1/conf.proto

@@ -21,14 +21,17 @@ import (
 	"strings"
 
 	v1 "github.com/chainloop-dev/chainloop/app/controlplane/api/controlplane/v1"
+	conf "github.com/chainloop-dev/chainloop/app/controlplane/internal/conf/controlplane/config/v1"
 	"github.com/go-kratos/kratos/v2/middleware"
+	"github.com/go-kratos/kratos/v2/transport"
 )
 
 // Middleware that checks that the user is defined in the allow list
-func CheckUserInAllowList(allowList []string) middleware.Middleware {
+func CheckUserInAllowList(allowList *conf.Auth_AllowList) middleware.Middleware {
 	return func(handler middleware.Handler) middleware.Handler {
 		return func(ctx context.Context, req interface{}) (interface{}, error) {
-			if len(allowList) == 0 {
+			// Allowlist disabled
+			if allowList == nil || len(allowList.GetRules()) == 0 {
 				return handler(ctx, req)
 			}
 
@@ -38,21 +41,43 @@ func CheckUserInAllowList(allowList []string) middleware.Middleware {
 				return nil, v1.ErrorAllowListErrorNotInList("user not found")
 			}
 
+			// Skip if we have explicitly set some routes and the current request is not part of them
+			if len(allowList.GetSelectedRoutes()) > 0 && !selectedRoute(ctx, allowList.GetSelectedRoutes()) {
+				return handler(ctx, req)
+			}
+
 			// If there are not items in the allowList we allow all users
-			allow, err := inAllowList(allowList, user.Email)
+			allow, err := inAllowList(allowList.GetRules(), user.Email)
 			if err != nil {
 				return nil, v1.ErrorAllowListErrorNotInList("error checking user in allowList: %v", err)
 			}
 
 			if !allow {
-				return nil, v1.ErrorAllowListErrorNotInList("user %q not in the allowList", user.Email)
+				msg := fmt.Sprintf("user %q not in the allowList", user.Email)
+				if allowList.GetCustomMessage() != "" {
+					msg = allowList.GetCustomMessage()
+				}
+
+				return nil, v1.ErrorAllowListErrorNotInList(msg)
 			}
 
 			return handler(ctx, req)
 		}
 	}
 }
 
+func selectedRoute(ctx context.Context, selectedRoutes []string) bool {
+	if info, ok := transport.FromServerContext(ctx); ok {
+		for _, route := range selectedRoutes {
+			if info.Operation() == route {
+				return true
+			}
+		}
+	}
+
+	return false
+}
+
 func inAllowList(allowList []string, email string) (bool, error) {
 	for _, allowListEntry := range allowList {
 		// it's a direct email match

app/controlplane/internal/usercontext/allowlist_middleware.go

@@ -20,12 +20,45 @@ import (
 	"testing"
 
 	v1 "github.com/chainloop-dev/chainloop/app/controlplane/api/controlplane/v1"
+	conf "github.com/chainloop-dev/chainloop/app/controlplane/internal/conf/controlplane/config/v1"
+	"github.com/go-kratos/kratos/v2/transport"
 	"github.com/stretchr/testify/assert"
 )
 
+// mockTransport is a gRPC transport.
+type mockTransport struct {
+	operation string
+}
+
+// Kind returns the transport kind.
+func (tr *mockTransport) Kind() transport.Kind {
+	return transport.KindGRPC
+}
+
+// Endpoint returns the transport endpoint.
+func (tr *mockTransport) Endpoint() string {
+	return ""
+}
+
+// Operation returns the transport operation.
+func (tr *mockTransport) Operation() string {
+	return tr.operation
+}
+
+// RequestHeader returns the request header.
+func (tr *mockTransport) RequestHeader() transport.Header {
+	return nil
+}
+
+// ReplyHeader returns the reply header.
+func (tr *mockTransport) ReplyHeader() transport.Header {
+	return nil
+}
+
 func TestCheckUserInAllowList(t *testing.T) {
 	const email = "[email protected]"
-	allowList := []string{
+
+	defaultRules := []string{
 		"[email protected]",
 		"[email protected]",
 		// it can also contain domains
@@ -34,80 +67,110 @@ func TestCheckUserInAllowList(t *testing.T) {
 	}
 
 	testCases := []struct {
-		name      string
-		allowList []string
-		email     string
-		wantErr   bool
+		name           string
+		rules          []string
+		selectedRoutes []string
+		runningRoute   string
+		email          string
+		wantErr        bool
+		customErrMsg   string
 	}{
 		{
-			name:      "empty allow list",
-			email:     email,
-			allowList: []string{},
+			name:  "empty allow list",
+			email: email,
+		},
+		{
+			name:    "user not in allow list",
+			email:   email,
+			rules:   []string{"[email protected]"},
+			wantErr: true,
+		},
+		{
+			name:           "user not allowed to access the route",
+			email:          "[email protected]",
+			runningRoute:   "/foo/bar",
+			selectedRoutes: []string{"/foo/bar"},
+			wantErr:        true,
+		},
+		{
+			name:           "route not in selected routes",
+			email:          "[email protected]",
+			runningRoute:   "/foo/bar",
+			selectedRoutes: []string{"/bar/foo", "/request-access", "/forbidden/route"},
 		},
 		{
-			name:      "user not in allow list",
-			email:     email,
-			allowList: []string{"[email protected]"},
-			wantErr:   true,
+			name:         "return custom error message",
+			email:        email,
+			rules:        []string{"[email protected]"},
+			wantErr:      true,
+			customErrMsg: "custom error message",
 		},
 		{
-			name:      "context missing, no user loaded",
-			allowList: allowList,
-			wantErr:   true,
+			name:    "context missing, no user loaded",
+			wantErr: true,
 		},
 		{
-			name:      "user in allow list",
-			email:     email,
-			allowList: allowList,
+			name:  "user in allow list",
+			email: email,
 		},
 		{
-			name:      "user in one of the valid domains",
-			email:     "[email protected]",
-			allowList: allowList,
+			name:  "user in one of the valid domains",
+			email: "[email protected]",
 		},
 		{
-			name:      "user in one of the valid domains",
-			email:     "[email protected]",
-			allowList: allowList,
+			name:  "user in one of the valid domains",
+			email: "[email protected]",
 		},
 		{
-			name:      "and can use modifiers",
-			email:     "[email protected]",
-			allowList: allowList,
+			name:  "and can use modifiers",
+			email: "[email protected]",
 		},
 		{
-			name:      "it needs to be an email",
-			email:     "dyson-industries.io",
-			allowList: allowList,
-			wantErr:   true,
+			name:    "it needs to be an email",
+			email:   "dyson-industries.io",
+			wantErr: true,
 		},
 		{
-			name:      "domain position is important",
-			email:     "dyson-industries.io@john",
-			allowList: allowList,
-			wantErr:   true,
+			name:    "domain position is important",
+			email:   "dyson-industries.io@john",
+			wantErr: true,
 		},
 		{
-			name:      "and can't be typosquated",
-			email:     "[email protected]",
-			allowList: allowList,
-			wantErr:   true,
+			name:    "and can't be typosquated",
+			email:   "[email protected]",
+			wantErr: true,
 		},
 	}
 
 	for _, tc := range testCases {
+		tc := tc
 		t.Run(tc.name, func(t *testing.T) {
-			m := CheckUserInAllowList(tc.allowList)
+			allowList := &conf.Auth_AllowList{
+				Rules:          defaultRules,
+				CustomMessage:  tc.customErrMsg,
+				SelectedRoutes: tc.selectedRoutes,
+			}
+
+			if tc.rules != nil {
+				allowList.Rules = tc.rules
+			}
+
+			m := CheckUserInAllowList(allowList)
 			ctx := context.Background()
 			if tc.email != "" {
-				u := &User{Email: tc.email, ID: "124"}
-				ctx = WithCurrentUser(ctx, u)
+				ctx = WithCurrentUser(ctx, &User{Email: tc.email, ID: "124"})
+			}
+			if tc.runningRoute != "" {
+				ctx = transport.NewServerContext(ctx, &mockTransport{operation: tc.runningRoute})
 			}
 
 			_, err := m(emptyHandler)(ctx, nil)
 
 			if tc.wantErr {
 				assert.True(t, v1.IsAllowListErrorNotInList(err))
+				if tc.customErrMsg != "" {
+					assert.Contains(t, err.Error(), tc.customErrMsg)
+				}
 			} else {
 				assert.NoError(t, err)
 			}