[#2473] move extract groups claim

Author: pzadroga

backend/server/oidc/controller.go

@@ -343,6 +343,29 @@ func sanitizeReturnURL(returnURL string) string {
 	return sanitizedPath
 }
 
+// Extracts groups claim from raw claims and returns the groups as slice of strings.
+func (ctl *Controller) extractGroupsFromClaim(rawClaims map[string]interface{}) []string {
+	// Do custom unmarshaling of the groups claim, because we can't be sure
+	// how the claim is formatted on the OpenID Provider side.
+	// We should have the groups extracted as slice of strings.
+	var groups []string
+	if val, ok := rawClaims[ctl.settings.GroupsClaim]; ok {
+		switch claim := val.(type) {
+		case []interface{}:
+			for _, g := range claim {
+				if s, ok := g.(string); ok {
+					groups = append(groups, s)
+				}
+			}
+		case []string:
+			groups = claim
+		case string:
+			groups = []string{claim}
+		}
+	}
+	return groups
+}
+
 // Handles OIDC callback endpoint which interprets redirection from OpenID Provider
 // after user successfully authenticates at the OP and authorizes Stork as a
 // Relying Party. It verifies the response, extracts required parameters and
@@ -431,25 +454,8 @@ func (ctl *Controller) callbackHandler(w http.ResponseWriter, r *http.Request) {
 			http.Redirect(w, r, authErrorURLPath, http.StatusFound)
 			return
 		}
-		// Do custom unmarshaling of the groups claim, because we can't be sure
-		// how the claim is formatted on the OpenID Provider side.
-		// We should have the groups extracted as slice of strings.
-		if val, ok := rawClaims[ctl.settings.GroupsClaim]; ok {
-			var groups []string
-			switch claim := val.(type) {
-			case []interface{}:
-				for _, g := range claim {
-					if s, ok := g.(string); ok {
-						groups = append(groups, s)
-					}
-				}
-			case []string:
-				groups = claim
-			case string:
-				groups = []string{claim}
-			}
-			claims.Groups = groups
-		}
+		groups := ctl.extractGroupsFromClaim(rawClaims)
+		claims.Groups = groups
 	}
 	log.Debugf("Claims received during OIDC authentication %+v", claims)
 

backend/server/oidc/controller_test.go

@@ -911,3 +911,69 @@ func TestCallbackEndpointAuthorizesUserGroupMappingDisabled(t *testing.T) {
 	require.Len(t, dbUser.Groups, 1)
 	require.Equal(t, dbmodel.ReadOnlyGroupID, dbUser.Groups[0].ID)
 }
+
+// Test if extractGroupsFromClaim works fine.
+func TestExtractGroupsFromClaim(t *testing.T) {
+	// Arrange
+	db, _, teardown := dbtest.SetupDatabaseTestCase(t)
+	defer teardown()
+	issuerURL, srvTeardown, err := oidctest.PrepareTestOIDCServer()
+	require.NoError(t, err)
+	defer srvTeardown()
+	settings := Settings{
+		IssuerURL:           issuerURL,
+		ClientID:            "clientID",
+		GroupsClaim:         "groups",
+		MandatoryAllowGroup: "stork-users",
+	}
+	controller := NewController(settings, db)
+	require.NotNil(t, controller)
+
+	// Act & Assert
+	m := make(map[string]interface{})
+	t.Run("empty claims", func(t *testing.T) {
+		res := controller.extractGroupsFromClaim(m)
+		require.Empty(t, res)
+	})
+
+	m["sub"] = "foo"
+
+	t.Run("no groups", func(t *testing.T) {
+		res := controller.extractGroupsFromClaim(m)
+		require.Empty(t, res)
+	})
+
+	t.Run("slice of strings", func(t *testing.T) {
+		m["groups"] = []string{"a", "b", "c"}
+		res := controller.extractGroupsFromClaim(m)
+		require.NotEmpty(t, res)
+		require.Len(t, res, 3)
+		require.Contains(t, res, "a")
+		require.Contains(t, res, "b")
+		require.Contains(t, res, "c")
+	})
+
+	t.Run("one string", func(t *testing.T) {
+		m["groups"] = "groupA"
+		res := controller.extractGroupsFromClaim(m)
+		require.NotEmpty(t, res)
+		require.Len(t, res, 1)
+		require.Contains(t, res, "groupA")
+	})
+
+	t.Run("slice of interfaces", func(t *testing.T) {
+		var groupA, groupB, groupC interface{}
+		groupA = "groupA"
+		groupB = "groupB"
+		groupC = "groupC"
+		var groups []interface{}
+		groups = append(groups, groupA, groupB, groupC)
+		m["groups"] = groups
+		res := controller.extractGroupsFromClaim(m)
+		require.NotEmpty(t, res)
+		require.Len(t, res, 3)
+		require.Contains(t, res, "groupA")
+		require.Contains(t, res, "groupB")
+		require.Contains(t, res, "groupC")
+	})
+}