optional case-insensitive username and groupname comparisons for RBAC…

… + tests

(cherry picked from commit 13427e8)

internal/authorization/rbac/rbac.go

@@ -28,6 +28,8 @@ type Authorizer struct {
 	syncDuration             time.Duration
 	informerStop             chan struct{}
 	selector                 labels.Selector
+	// If CaseInsensitiveSubjects is true, group and user names are compared case-insensitively (default false)
+	CaseInsensitiveSubjects bool
 }
 
 // NewAuthorizer creates a new RBAC authorizer
@@ -68,7 +70,7 @@ func (ra *Authorizer) getRoleFromGroups(roleNameRef, subjectGroupName string, us
 	// for every user group...
 	for _, group := range userGroups {
 		// if the group matches the group name in the subject, return the role
-		if group == subjectGroupName {
+		if compareSubjects(group, subjectGroupName, ra.CaseInsensitiveSubjects) {
 			return ra.getRoleByName(roleNameRef)
 		}
 	}
@@ -80,7 +82,7 @@ func (ra *Authorizer) getRoleFromGroups(roleNameRef, subjectGroupName string, us
 // getRoleForSubject gets the role bound to the subject depending on the subject kind (user or group).
 // Returns nil if there is no rule matching or an unknown subject Kind is provided
 func (ra *Authorizer) getRoleForSubject(user authorization.User, subject rbacv1.Subject, roleNameRef string) *rbacv1.ClusterRole {
-	if subject.Kind == "User" && subject.Name == user.GetName() {
+	if subject.Kind == "User" && compareSubjects(subject.Name, user.GetName(), ra.CaseInsensitiveSubjects) {
 		return ra.getRoleByName(roleNameRef)
 	} else if subject.Kind == "Group" {
 		return ra.getRoleFromGroups(roleNameRef, subject.Name, user.GetGroups())
@@ -153,7 +155,7 @@ func verbMatches(rule *rbacv1.PolicyRule, requestedVerb string) bool {
 		if ruleVerb == rbacv1.VerbAll {
 			return true
 		}
-		if strings.ToLower(ruleVerb) == strings.ToLower(requestedVerb) {
+		if strings.EqualFold(ruleVerb, requestedVerb) {
 			return true
 		}
 	}
@@ -173,3 +175,13 @@ func nonResourceURLMatches(rule *rbacv1.PolicyRule, requestedURL string) bool {
 	}
 	return false
 }
+
+// compareSubjects determines whether subjects names are equal (string equality is used).
+// If caseInsensitive is true, the case of the characters is ignored, meaning "UserName"
+// would be considered equal to "username".
+func compareSubjects(s1, s2 string, caseInsensitive bool) bool {
+	if caseInsensitive == false {
+		return s1 == s2
+	}
+	return strings.EqualFold(s1, s2)
+}

internal/authorization/rbac/rbac_test.go

@@ -156,3 +156,61 @@ func TestRBACAuthorizer_Authorize2(t *testing.T) {
 	assert.NilError(t, err)
 	assert.Equal(t, result, test.should)
 }
+
+func TestCaseInsensitiveSubjects(t *testing.T) {
+	type testCase struct {
+		authorizer authorization.Authorizer
+		user       authorization.User
+		url        string
+		should     bool
+	}
+
+	// declare roles and bindings all lower-case
+	role := makeRole("grafana-admin", []string{"*"}, []string{"/ops/portal/grafana", "/ops/portal/grafana/*"})
+	rolebindings := makeClusterRoleBindingList(
+		makeBinding("User", "grafana-admin-boyle", "[email protected]", "grafana-admin"),
+		makeBinding("Group", "grafana-admin-oidc-admins", "oidc:admins", "grafana-admin"),
+	)
+
+	// default authorizer
+	defaultAuthorizer := getRBACAuthorizer(&role, rolebindings)
+
+	// case-insensitive authorizer
+	caseInsensitiveAuthorizer := getRBACAuthorizer(&role, rolebindings)
+	caseInsensitiveAuthorizer.CaseInsensitiveSubjects = true
+
+	tests := []testCase{
+		// users
+		{
+			authorizer: defaultAuthorizer,
+			user:       authorization.User{Name: "[email protected]", Groups: []string{"oidc:chemists"}},
+			url:        "/ops/portal/grafana/rnJhmVJw.woff2",
+			should:     false, // default case-sensitive user comparison shouldn't allow Boyle
+		},
+		{
+			authorizer: caseInsensitiveAuthorizer,
+			user:       authorization.User{Name: "[email protected]", Groups: []string{"oidc:chemists"}},
+			url:        "/ops/portal/grafana/rnJhmVJw.woff2",
+			should:     true, // case-insensitive user comparison should allow Boyle
+		},
+		// groups
+		{
+			authorizer: defaultAuthorizer,
+			user:       authorization.User{Name: "[email protected]", Groups: []string{"oidc:Admins"}},
+			url:        "/ops/portal/grafana/rnJhmVJw.woff2",
+			should:     false, // default case-sensitive group comparison shouldn't allow Admins group
+		},
+		{
+			authorizer: caseInsensitiveAuthorizer,
+			user:       authorization.User{Name: "[email protected]", Groups: []string{"oidc:Admins"}},
+			url:        "/ops/portal/grafana/rnJhmVJw.woff2",
+			should:     true, // case-insensitive group comparison should allow Admins group
+		},
+	}
+
+	for _, test := range tests {
+		result, err := test.authorizer.Authorize(test.user, "GET", test.url)
+		assert.NilError(t, err)
+		assert.Equal(t, result, test.should)
+	}
+}

internal/configuration/config.go

@@ -61,8 +61,9 @@ type Config struct {
 	GroupsAttributeName     string               `long:"groups-attribute-name" env:"GROUPS_ATTRIBUTE_NAME" default:"groups" description:"Map the correct attribute that contain the user groups"`
 
 	// RBAC
-	EnableRBAC       bool               `long:"enable-rbac" env:"ENABLE_RBAC" description:"Indicates that RBAC support should be enabled"`
-	AuthZPassThrough CommaSeparatedList `long:"authz-pass-through" env:"AUTHZ_PASS_THROUGH" description:"One or more routes which bypass authorization checks"`
+	EnableRBAC              bool               `long:"enable-rbac" env:"ENABLE_RBAC" description:"Indicates that RBAC support should be enabled"`
+	AuthZPassThrough        CommaSeparatedList `long:"authz-pass-through" env:"AUTHZ_PASS_THROUGH" description:"One or more routes which bypass authorization checks"`
+	CaseInsensitiveSubjects bool               `long:"case-insensitive-subjects" env:"CASE_INSENSITIVE_SUBJECTS" description:"Make case-insensitive comparison of user and group names in the RBAC implementation"`
 
 	// Storage
 	EnableInClusterStorage bool   `long:"enable-in-cluster-storage" env:"ENABLE_IN_CLUSTER_STORAGE" description:"When true, sessions are store in a kubernetes apiserver"`

internal/handlers/server.go

@@ -48,7 +48,9 @@ func NewServer(userinfo v1alpha1.UserInfoInterface, clientset kubernetes.Interfa
 	s.buildRoutes()
 	s.userinfo = userinfo
 	if config.EnableRBAC {
-		s.authorizer = rbac.NewAuthorizer(clientset)
+		rbac := rbac.NewAuthorizer(clientset)
+		rbac.CaseInsensitiveSubjects = config.CaseInsensitiveSubjects
+		s.authorizer = rbac
 	}
 	return s
 }