Handle other edge case

grafana · May 3, 2024 · 66f50fb · 66f50fb
1 parent d8bce8e
commit 66f50fb
Show file tree

Hide file tree

Showing 2 changed files with 23 additions and 14 deletions.
diff --git a/pkg/login/social/connectors/org_role_mapper.go b/pkg/login/social/connectors/org_role_mapper.go
@@ -153,8 +153,9 @@ func (m *OrgRoleMapper) ParseOrgMappingSettings(ctx context.Context, mappings []
 			}
 			if err == nil {
 				if roleStrict && (len(kv) < 3 || !org.RoleType(kv[2]).IsValid()) {
+					// Return empty mapping if at least one org mapping is invalid (missing role, invalid role)
 					m.logger.Warn("Skipping org mapping due to missing or invalid role in mapping when roleStrict is enabled.", "mapping", fmt.Sprintf("%v", v))
-					continue
+					return &MappingConfiguration{orgMapping: map[string]map[int64]org.RoleType{}, roleStrict: roleStrict}
 				}
 
 				orga := kv[0]

diff --git a/pkg/login/social/connectors/org_role_mapper_test.go b/pkg/login/social/connectors/org_role_mapper_test.go
@@ -29,14 +29,6 @@ func TestOrgRoleMapper_MapOrgRoles(t *testing.T) {
 			directlyMappedRole: "",
 			expected:           map[int64]org.RoleType{2: org.RoleViewer},
 		},
-		{
-			name:               "should return nil when no org mapping settings are provided and directly mapped role is not set and strict mapping is enabled",
-			externalOrgs:       []string{},
-			orgMappingSettings: []string{},
-			directlyMappedRole: "",
-			roleStrict:         true,
-			expected:           nil,
-		},
 		{
 			name:               "should return the default mapping when no org mapping settings are provided and direcly mapped role is set",
 			externalOrgs:       []string{},
@@ -52,19 +44,27 @@ func TestOrgRoleMapper_MapOrgRoles(t *testing.T) {
 			expected:           map[int64]org.RoleType{2: org.RoleViewer},
 		},
 		{
-			name:               "should return nil when org mapping doesn't match any of the external orgs and no directly mapped role is provided and strict mapping is enabled",
+			name:               "should return Viewer role for the org if the specified role is invalid",
 			externalOrgs:       []string{"First"},
-			orgMappingSettings: []string{"Second:1:Editor"},
+			orgMappingSettings: []string{"First:1:SuperEditor"},
+			directlyMappedRole: "",
+			expected:           map[int64]org.RoleType{1: org.RoleViewer},
+		},
+		{
+			name:               "should return nil when no org mapping settings are provided and directly mapped role is not set and strict mapping is enabled",
+			externalOrgs:       []string{},
+			orgMappingSettings: []string{},
 			directlyMappedRole: "",
 			roleStrict:         true,
 			expected:           nil,
 		},
 		{
-			name:               "should return Viewer role for the org if the specified role is invalid",
+			name:               "should return nil when org mapping doesn't match any of the external orgs and no directly mapped role is provided and strict mapping is enabled",
 			externalOrgs:       []string{"First"},
-			orgMappingSettings: []string{"First:1:SuperEditor"},
+			orgMappingSettings: []string{"Second:1:Editor"},
 			directlyMappedRole: "",
-			expected:           map[int64]org.RoleType{1: org.RoleViewer},
+			roleStrict:         true,
+			expected:           nil,
 		},
 		// In this case the parsed org mapping will be empty because the role is invalid
 		{
@@ -83,6 +83,14 @@ func TestOrgRoleMapper_MapOrgRoles(t *testing.T) {
 			roleStrict:         true,
 			expected:           map[int64]org.RoleType{2: org.RoleEditor},
 		},
+		{
+			name:               "should return nil if the org mapping contains at least one invalid setting and directly mapped role is empty and role strict is enabled",
+			externalOrgs:       []string{"First"},
+			orgMappingSettings: []string{"First:1:SuperEditor", "First:1:Admin"},
+			directlyMappedRole: "",
+			roleStrict:         true,
+			expected:           nil,
+		},
 		{
 			name:               "should map the higher role if directly mapped role is lower than the role found in the org mapping",
 			externalOrgs:       []string{"First"},