feature: (#568) Separator allowed in usergroup line for trusted headers (#572)

service/internal/acl/acl.go

@@ -25,8 +25,8 @@ func (p PermissionBits) Has(permission PermissionBits) bool {
 
 // User respresents a person.
 type AuthenticatedUser struct {
-	Username  string
+	Username      string
-	Usergroup string
+	UsergroupLine string
 
 	Provider string
 	SID      string
@@ -40,6 +40,36 @@ func (u *AuthenticatedUser) IsGuest() bool {
 	return u.Username == "guest" && u.Provider == "system"
 }
 
+func (u *AuthenticatedUser) parseUsergroupLine(sep string) []string {
+	ret := []string{}
+
+	if sep != "" {
+		for _, v := range strings.Split(u.UsergroupLine, sep) {
+			trimmed := strings.TrimSpace(v)
+
+			if trimmed != "" {
+				ret = append(ret, trimmed)
+			}
+		}
+	} else {
+		ret = strings.Fields(u.UsergroupLine)
+	}
+	return ret
+}
+
+func (u *AuthenticatedUser) matchesUsergroupAcl(matchUsergroups []string, sep string) bool {
+	groupList := u.parseUsergroupLine(sep)
+
+	for _, group := range groupList {
+		if slices.Contains(matchUsergroups, group) {
+			log.Debugf("Usergroup %v found in %+v (len: %v)", group, groupList, len(groupList))
+			return true
+		}
+	}
+
+	return false
+}
+
 func logAclNotMatched(cfg *config.Config, aclFunction string, user *AuthenticatedUser, action *config.Action, acl *config.AccessControlList) {
 	if cfg.LogDebugOptions.AclNotMatched {
 		log.WithFields(log.Fields{
@@ -101,7 +131,7 @@ func aclCheck(requiredPermission PermissionBits, defaultValue bool, cfg *config.
 		log.WithFields(log.Fields{
 			"actionTitle":        action.Title,
 			"username":           user.Username,
-			"usergroup":          user.Usergroup,
+			"usergroupLine":      user.UsergroupLine,
 			"relevantAcls":       len(relevantAcls),
 			"requiredPermission": requiredPermission,
 		}).Debugf("ACL check - %v", aclFunction)
@@ -162,7 +192,7 @@ func UserFromContext(ctx context.Context, cfg *config.Config) *AuthenticatedUser
 	if ok {
 		ret = &AuthenticatedUser{}
 		ret.Username = getMetadataKeyOrEmpty(md, "username")
-		ret.Usergroup = getMetadataKeyOrEmpty(md, "usergroup")
+		ret.UsergroupLine = getMetadataKeyOrEmpty(md, "usergroup")
 		ret.Provider = getMetadataKeyOrEmpty(md, "provider")
 
 		buildUserAcls(cfg, ret)
@@ -173,10 +203,10 @@ func UserFromContext(ctx context.Context, cfg *config.Config) *AuthenticatedUser
 	}
 
 	log.WithFields(log.Fields{
-		"username":  ret.Username,
+		"username":      ret.Username,
-		"usergroup": ret.Usergroup,
+		"usergroupLine": ret.UsergroupLine,
-		"provider":  ret.Provider,
+		"provider":      ret.Provider,
-		"acls":      ret.Acls,
+		"acls":          ret.Acls,
 	}).Debugf("UserFromContext")
 
 	return ret
@@ -185,7 +215,7 @@ func UserFromContext(ctx context.Context, cfg *config.Config) *AuthenticatedUser
 func UserGuest(cfg *config.Config) *AuthenticatedUser {
 	ret := &AuthenticatedUser{}
 	ret.Username = "guest"
-	ret.Usergroup = "guest"
+	ret.UsergroupLine = "guest"
 	ret.Provider = "system"
 
 	buildUserAcls(cfg, ret)
@@ -195,9 +225,9 @@ func UserGuest(cfg *config.Config) *AuthenticatedUser {
 
 func UserFromSystem(cfg *config.Config, username string) *AuthenticatedUser {
 	ret := &AuthenticatedUser{
-		Username:  username,
+		Username:      username,
-		Usergroup: "system",
+		UsergroupLine: "system",
-		Provider:  "system",
+		Provider:      "system",
 	}
 
 	buildUserAcls(cfg, ret)
@@ -212,8 +242,7 @@ func buildUserAcls(cfg *config.Config, user *AuthenticatedUser) {
 			continue
 		}
 
-		// handle multiple usergroups - groups will be separated by a space
+		if user.matchesUsergroupAcl(acl.MatchUsergroups, cfg.AuthHttpHeaderUserGroupSep) {
-		if hasGroupsMatch(acl.MatchUsergroups, user.Usergroup) {
 			user.Acls = append(user.Acls, acl.Name)
 			continue
 		}
@@ -222,15 +251,6 @@ func buildUserAcls(cfg *config.Config, user *AuthenticatedUser) {
 	user.EffectivePolicy = getEffectivePolicy(cfg, user)
 }
 
-func hasGroupsMatch(matchUsergroups []string, usergroup string) bool {
-	for _, group := range strings.Fields(usergroup) {
-		if slices.Contains(matchUsergroups, group) {
-			return true
-		}
-	}
-	return false
-}
-
 func isACLRelevantToAction(cfg *config.Config, actionAcls []string, acl *config.AccessControlList, user *AuthenticatedUser) bool {
 	if !slices.Contains(user.Acls, acl.Name) {
 		// If the user does not have this ACL, then it is not relevant

service/internal/acl/acl_test.go

@@ -1,37 +1,111 @@
 package acl
 
-import "testing"
+import (
+	"github.com/stretchr/testify/assert"
+	"testing"
+)
 
 func Test_hasGroupsMatch(t *testing.T) {
 	tests := []struct {
-		name            string
+		name               string
-		matchUsergroups []string
+		aclMatchUsergroups []string
-		usergroup       string
+		usergroupLine      string
-		want            bool
+		matches            bool
+		sep                string
 	}{
 		{
-			name:            "No groups match",
+			name:               "No groups match",
-			matchUsergroups: []string{"group1", "group2"},
+			aclMatchUsergroups: []string{"group1", "group2"},
-			usergroup:       "group3",
+			usergroupLine:      "group3",
+			matches:            false,
 		},
 		{
-			name:            "Exact match",
+			name:               "Exact match",
-			matchUsergroups: []string{"group1", "group2"},
+			aclMatchUsergroups: []string{"group1", "group2"},
-			usergroup:       "group1",
+			usergroupLine:      "group1",
-			want:            true,
+			matches:            true,
 		},
 		{
-			name:            "Multiple groups match",
+			name:               "Multiple groups match",
-			matchUsergroups: []string{"group1", "group2"},
+			aclMatchUsergroups: []string{"group1", "group2"},
-			usergroup:       "group1 group2",
+			usergroupLine:      "group1 group2",
-			want:            true,
+			matches:            true,
+		},
+		{
+			name:               "Comma-separated groups match",
+			aclMatchUsergroups: []string{"group1", "group2", "group3"},
+			usergroupLine:      "group1, group2",
+			matches:            true,
+			sep:                ",",
+		},
+		{
+			name:               "Comma-separated groups with default separator does not match",
+			aclMatchUsergroups: []string{"group1"},
+			usergroupLine:      "group1, group2",
+			matches:            false,
+			sep:                "",
 		},
 	}
+
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			if got := hasGroupsMatch(tt.matchUsergroups, tt.usergroup); got != tt.want {
+			user := &AuthenticatedUser{
-				t.Errorf("hasGroupsMatch() = %v, want %v", got, tt.want)
+				Username:      "testuser",
+				UsergroupLine: tt.usergroupLine,
+			}
+
+			if matches := user.matchesUsergroupAcl(tt.aclMatchUsergroups, tt.sep); matches != tt.matches {
+				t.Errorf("AuthenticatedUser.matchesUsergroupAcl() = %v, want %v for usergroups %v", matches, tt.matches, tt.aclMatchUsergroups)
 			}
 		})
 	}
 }
+
+func Test_parseUsergroupLine(t *testing.T) {
+	tests := []struct {
+		name           string
+		usergroupLine  string
+		expectedGroups []string
+		sep            string
+	}{
+		{
+			name:           "Default separator (space)",
+			usergroupLine:  "group1 group2",
+			expectedGroups: []string{"group1", "group2"},
+		},
+		{
+			name:           "Comma-separated groups",
+			usergroupLine:  "group1 , group2",
+			expectedGroups: []string{"group1", "group2"},
+			sep:            ",",
+		},
+		{
+			name:           "Multiple spaces",
+			usergroupLine:  "group1 , group2      , group3",
+			expectedGroups: []string{"group1", "group2", "group3"},
+			sep:            ",",
+		},
+		{
+			name:           "Empty usergroup line",
+			usergroupLine:  "",
+			expectedGroups: []string{},
+		},
+		{
+			name:           "Empty group names",
+			usergroupLine:  "|group1| | group3|",
+			expectedGroups: []string{"group1", "group3"},
+			sep:            "|",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			user := &AuthenticatedUser{
+				Username:      "testuser",
+				UsergroupLine: tt.usergroupLine,
+			}
+
+			assert.Equal(t, tt.expectedGroups, user.parseUsergroupLine(tt.sep))
+		})
+	}
+}

service/internal/config/config.go

@@ -124,6 +124,7 @@ type Config struct {
 	AuthJwtPubKeyPath               string // will read pub key from file on disk
 	AuthHttpHeaderUsername          string
 	AuthHttpHeaderUserGroup         string
+	AuthHttpHeaderUserGroupSep      string
 	AuthLocalUsers                  AuthLocalUsersConfig
 	AuthLoginUrl                    string
 	AuthRequireGuestsToLogin        bool

service/internal/executor/arguments.go

@@ -6,12 +6,12 @@ import (
 	log "github.com/sirupsen/logrus"
 
 	"errors"
+	"fmt"
 	"net/mail"
 	"net/url"
 	"regexp"
 	"strings"
 	"time"
-	"fmt"
 )
 
 var (

service/internal/executor/executor_test.go

@@ -186,8 +186,8 @@ func TestUnsetRequiredArgument(t *testing.T) {
 		Shell: "echo 'Your name is: {{ name }}'",
 		Arguments: []config.ActionArgument{
 			{
-				Name:     "name",
+				Name: "name",
-				Type:     "ascii",
+				Type: "ascii",
 			},
 		},
 	}
@@ -206,12 +206,12 @@ func TestUnusedArgumentStillPassesTypeSafetyCheck(t *testing.T) {
 		Shell: "echo 'Your name is: {{ name }}'",
 		Arguments: []config.ActionArgument{
 			{
-				Name:     "name",
+				Name: "name",
-				Type:     "ascii",
+				Type: "ascii",
 			},
 			{
-				Name:     "age",
+				Name: "age",
-				Type:     "int",
+				Type: "int",
 			},
 		},
 	}

service/internal/grpcapi/grpcApi.go

@@ -316,7 +316,7 @@ func (api *oliveTinAPI) GetDashboardComponents(ctx ctx.Context, req *apiv1.GetDa
 	if len(res.Actions) == 0 {
 		log.WithFields(log.Fields{
 			"username":         user.Username,
-			"usergroup":        user.Usergroup,
+			"usergroupLine":    user.UsergroupLine,
 			"provider":         user.Provider,
 			"acls":             user.Acls,
 			"availableActions": len(cfg.Actions),
@@ -377,14 +377,12 @@ func (api *oliveTinAPI) WhoAmI(ctx ctx.Context, req *apiv1.WhoAmIRequest) (*apiv
 
 	res := &apiv1.WhoAmIResponse{
 		AuthenticatedUser: user.Username,
-		Usergroup:         user.Usergroup,
+		Usergroup:         user.UsergroupLine,
 		Provider:          user.Provider,
 		Sid:               user.SID,
 		Acls:              user.Acls,
 	}
 
-	log.Warnf("usergroup: %v", user.Usergroup)
-
 	return res, nil
 }