Merge pull request #12232 from kobergj/feat/OCISDEV-794-ldap-group-additional-objectclasses

feat(graph): [OCISDEV-794] allow multiple objectClasses on group creation

Author: kobergj

changelog/unreleased/enhancement-ldap-group-additional-objectclasses.md

@@ -0,0 +1,8 @@
+Enhancement: Allow multiple objectClasses on group creation
+
+Added support for configuring additional LDAP objectClasses when creating groups.
+The new `OCIS_LDAP_GROUP_ADDITIONAL_OBJECTCLASSES` / `GRAPH_LDAP_GROUP_ADDITIONAL_OBJECTCLASSES`
+environment variable accepts a list of extra objectClasses that are set alongside the
+primary `GRAPH_LDAP_GROUP_OBJECTCLASS` when a new group is created in LDAP.
+
+https://github.com/owncloud/ocis/pull/12229

deployments/examples/ocis_multi/config/ldap/schemas/10_owncloud_schema.ldif

@@ -52,6 +52,5 @@ olcObjectClasses: ( ownCloudOid:1.2.2 NAME 'ownCloudUser'
   MAY ( ocExternalIdentity $ ownCloudUserEnabled $ ownCloudUserType $ ocLastSignInTimestamp $ ownCloudMemberOf $ ownCloudGuestOf $ ownCloudRole) )
 olcObjectClasses: ( ownCloudOid:1.2.3 NAME 'ownCloudGroup'
   DESC 'ownCloud Group LDAP Schema'
-  SUP groupOfNames
-  STRUCTURAL
+  AUXILIARY
   MAY ( ownCloudMemberOf ) )

deployments/examples/ocis_multi/docker-compose.yml

@@ -60,7 +60,6 @@ services:
     command: [ "-c", "ocis init || true; exec ocis server" ]
     environment:
       # Keycloak IDP specific configuration
-      PROXY_AUTOPROVISION_ACCOUNTS: "true"
       PROXY_ROLE_ASSIGNMENT_DRIVER: "oidc"
       OCIS_OIDC_ISSUER: https://${KEYCLOAK_DOMAIN:-keycloak.owncloud.test}/realms/${KEYCLOAK_REALM:-oCIS}
       PROXY_OIDC_REWRITE_WELLKNOWN: "true"
@@ -75,6 +74,9 @@ services:
       PROXY_USER_CS3_CLAIM: "username"
       # INSECURE: needed if oCIS / Traefik is using self generated certificates
       OCIS_INSECURE: "${INSECURE:-true}"
+      OCIS_ADD_RUN_SERVICES: "auth-app"
+      PROXY_ENABLE_APP_AUTH: true
+      AUTH_APP_ENABLE_IMPERSONATION: true
       OCIS_EXCLUDE_RUN_SERVICES: "idp,idm"
       GRAPH_ASSIGN_DEFAULT_USER_ROLE: "false"
       GRAPH_USERNAME_MATCH: "none"
@@ -90,7 +92,8 @@ services:
       OCIS_LDAP_BIND_PASSWORD: ${LDAP_ADMIN_PASSWORD:-admin}
       OCIS_LDAP_GROUP_BASE_DN: "ou=groups,dc=owncloud,dc=com"
       GRAPH_LDAP_GROUP_CREATE_BASE_DN: "ou=groups-ec730a6c-1b63-4b45-b83b-9e2311afdf85,ou=groups,dc=owncloud,dc=com"
-      OCIS_LDAP_GROUP_OBJECTCLASS: "owncloudGroup"
+      OCIS_LDAP_GROUP_OBJECTCLASS: "groupOfNames"
+      OCIS_LDAP_GROUP_ADDITIONAL_OBJECTCLASSES: "ownCloudGroup"
       OCIS_LDAP_USER_BASE_DN: "ou=users,dc=owncloud,dc=com"
       OCIS_LDAP_USER_OBJECTCLASS: "inetOrgPerson"
       LDAP_LOGIN_ATTRIBUTES: "uid"
@@ -121,8 +124,6 @@ services:
       OCIS_MULTI_INSTANCE_GUEST_ROLE: "user-light"
       OCIS_LDAP_CROSS_INSTANCE_REFERENCE_TEMPLATE: "{{.Username}}@{{.Instancename}}.owncloud.test"
       OCIS_LDAP_INSTANCE_URL_TEMPLATE: "https://{{.Instancename}}.owncloud.test"
-      # FIXME: sync groups properly to keycloak and remove the next line
-      PROXY_AUTOPROVISION_CLAIM_GROUPS: ""
       # specific for deployment example
       PROXY_ROLE_ASSIGNMENT_OIDC_CLAIM: ownCloudRole
     volumes:
@@ -153,7 +154,6 @@ services:
     command: ["-c", "ocis init || true; ocis server"]
     environment:
       # Keycloak IDP specific configuration
-      PROXY_AUTOPROVISION_ACCOUNTS: "true"
       PROXY_ROLE_ASSIGNMENT_DRIVER: "oidc"
       OCIS_OIDC_ISSUER: https://${KEYCLOAK_DOMAIN:-keycloak.owncloud.test}/realms/${KEYCLOAK_REALM:-oCIS}
       PROXY_OIDC_REWRITE_WELLKNOWN: "true"
@@ -222,8 +222,6 @@ services:
       OCIS_MULTI_INSTANCE_GUEST_ROLE: "user-light"
       OCIS_LDAP_CROSS_INSTANCE_REFERENCE_TEMPLATE: "{{.Username}}@{{.Instancename}}.owncloud.test"
       OCIS_LDAP_INSTANCE_URL_TEMPLATE: "https://{{.Instancename}}.owncloud.test"
-      # FIXME: sync groups properly to keycloak and remove the next line
-      PROXY_AUTOPROVISION_CLAIM_GROUPS: ""
       PROXY_ROLE_ASSIGNMENT_OIDC_CLAIM: ownCloudRole
     volumes:
       - ./config/ocis/csp-ocm.yaml:/etc/ocis/csp-ocm.yaml

services/graph/pkg/config/config.go

@@ -80,15 +80,16 @@ type LDAP struct {
 	DisableUserMechanism     string `yaml:"disable_user_mechanism" env:"OCIS_LDAP_DISABLE_USER_MECHANISM;GRAPH_DISABLE_USER_MECHANISM" desc:"An option to control the behavior for disabling users. Supported options are 'none', 'attribute' and 'group'. If set to 'group', disabling a user via API will add the user to the configured group for disabled users, if set to 'attribute' this will be done in the ldap user entry, if set to 'none' the disable request is not processed. Default is 'attribute'." introductionVersion:"pre5.0"`
 	LdapDisabledUsersGroupDN string `yaml:"ldap_disabled_users_group_dn" env:"OCIS_LDAP_DISABLED_USERS_GROUP_DN;GRAPH_DISABLED_USERS_GROUP_DN" desc:"The distinguished name of the group to which added users will be classified as disabled when 'disable_user_mechanism' is set to 'group'." introductionVersion:"pre5.0"`
 
-	GroupBaseDN          string `yaml:"group_base_dn" env:"OCIS_LDAP_GROUP_BASE_DN;GRAPH_LDAP_GROUP_BASE_DN" desc:"Search base DN for looking up LDAP groups." introductionVersion:"pre5.0"`
-	GroupCreateBaseDN    string `yaml:"group_create_base_dn" env:"GRAPH_LDAP_GROUP_CREATE_BASE_DN" desc:"Parent DN under which new groups are created. This DN needs to be subordinate to the 'GRAPH_LDAP_GROUP_BASE_DN'. This setting is only relevant when 'GRAPH_LDAP_SERVER_WRITE_ENABLED' is 'true'. It defaults to the value of 'GRAPH_LDAP_GROUP_BASE_DN'. All groups outside of this subtree are treated as readonly groups and cannot be updated." introductionVersion:"pre5.0"`
-	GroupSearchScope     string `yaml:"group_search_scope" env:"OCIS_LDAP_GROUP_SCOPE;GRAPH_LDAP_GROUP_SEARCH_SCOPE" desc:"LDAP search scope to use when looking up groups. Supported scopes are 'base', 'one' and 'sub'." introductionVersion:"pre5.0"`
-	GroupFilter          string `yaml:"group_filter" env:"OCIS_LDAP_GROUP_FILTER;GRAPH_LDAP_GROUP_FILTER" desc:"LDAP filter to add to the default filters for group searches." introductionVersion:"pre5.0"`
-	GroupObjectClass     string `yaml:"group_objectclass" env:"OCIS_LDAP_GROUP_OBJECTCLASS;GRAPH_LDAP_GROUP_OBJECTCLASS" desc:"The object class to use for groups in the default group search filter ('groupOfNames')." introductionVersion:"pre5.0"`
-	GroupNameAttribute   string `yaml:"group_name_attribute" env:"OCIS_LDAP_GROUP_SCHEMA_GROUPNAME;GRAPH_LDAP_GROUP_NAME_ATTRIBUTE" desc:"LDAP Attribute to use for the name of groups." introductionVersion:"pre5.0"`
-	GroupMemberAttribute string `yaml:"group_member_attribute" env:"OCIS_LDAP_GROUP_SCHEMA_MEMBER;GRAPH_LDAP_GROUP_MEMBER_ATTRIBUTE" desc:"LDAP Attribute that is used for group members." introductionVersion:"pre5.0"`
-	GroupIDAttribute     string `yaml:"group_id_attribute" env:"OCIS_LDAP_GROUP_SCHEMA_ID;GRAPH_LDAP_GROUP_ID_ATTRIBUTE" desc:"LDAP Attribute to use as the unique id for groups. This should be a stable globally unique ID like a UUID." introductionVersion:"pre5.0"`
-	GroupIDIsOctetString bool   `yaml:"group_id_is_octet_string" env:"OCIS_LDAP_GROUP_SCHEMA_ID_IS_OCTETSTRING;GRAPH_LDAP_GROUP_SCHEMA_ID_IS_OCTETSTRING" desc:"Set this to true if the defined 'ID' attribute for groups is of the 'OCTETSTRING' syntax. This is required when using the 'objectGUID' attribute of Active Directory for the group ID's." introductionVersion:"pre5.0"`
+	GroupBaseDN                  string   `yaml:"group_base_dn" env:"OCIS_LDAP_GROUP_BASE_DN;GRAPH_LDAP_GROUP_BASE_DN" desc:"Search base DN for looking up LDAP groups." introductionVersion:"pre5.0"`
+	GroupCreateBaseDN            string   `yaml:"group_create_base_dn" env:"GRAPH_LDAP_GROUP_CREATE_BASE_DN" desc:"Parent DN under which new groups are created. This DN needs to be subordinate to the 'GRAPH_LDAP_GROUP_BASE_DN'. This setting is only relevant when 'GRAPH_LDAP_SERVER_WRITE_ENABLED' is 'true'. It defaults to the value of 'GRAPH_LDAP_GROUP_BASE_DN'. All groups outside of this subtree are treated as readonly groups and cannot be updated." introductionVersion:"pre5.0"`
+	GroupSearchScope             string   `yaml:"group_search_scope" env:"OCIS_LDAP_GROUP_SCOPE;GRAPH_LDAP_GROUP_SEARCH_SCOPE" desc:"LDAP search scope to use when looking up groups. Supported scopes are 'base', 'one' and 'sub'." introductionVersion:"pre5.0"`
+	GroupFilter                  string   `yaml:"group_filter" env:"OCIS_LDAP_GROUP_FILTER;GRAPH_LDAP_GROUP_FILTER" desc:"LDAP filter to add to the default filters for group searches." introductionVersion:"pre5.0"`
+	GroupObjectClass             string   `yaml:"group_objectclass" env:"OCIS_LDAP_GROUP_OBJECTCLASS;GRAPH_LDAP_GROUP_OBJECTCLASS" desc:"The object class to use for groups in the default group search filter ('groupOfNames')." introductionVersion:"pre5.0"`
+	GroupAdditionalObjectClasses []string `yaml:"group_additional_objectclasses" env:"OCIS_LDAP_GROUP_ADDITIONAL_OBJECTCLASSES;GRAPH_LDAP_GROUP_ADDITIONAL_OBJECTCLASSES" desc:"Additional object classes to set when creating new groups (e.g. 'posixGroup'). The value of 'GRAPH_LDAP_GROUP_OBJECTCLASS' is always included." introductionVersion:"Deledda"`
+	GroupNameAttribute           string   `yaml:"group_name_attribute" env:"OCIS_LDAP_GROUP_SCHEMA_GROUPNAME;GRAPH_LDAP_GROUP_NAME_ATTRIBUTE" desc:"LDAP Attribute to use for the name of groups." introductionVersion:"pre5.0"`
+	GroupMemberAttribute         string   `yaml:"group_member_attribute" env:"OCIS_LDAP_GROUP_SCHEMA_MEMBER;GRAPH_LDAP_GROUP_MEMBER_ATTRIBUTE" desc:"LDAP Attribute that is used for group members." introductionVersion:"pre5.0"`
+	GroupIDAttribute             string   `yaml:"group_id_attribute" env:"OCIS_LDAP_GROUP_SCHEMA_ID;GRAPH_LDAP_GROUP_ID_ATTRIBUTE" desc:"LDAP Attribute to use as the unique id for groups. This should be a stable globally unique ID like a UUID." introductionVersion:"pre5.0"`
+	GroupIDIsOctetString         bool     `yaml:"group_id_is_octet_string" env:"OCIS_LDAP_GROUP_SCHEMA_ID_IS_OCTETSTRING;GRAPH_LDAP_GROUP_SCHEMA_ID_IS_OCTETSTRING" desc:"Set this to true if the defined 'ID' attribute for groups is of the 'OCTETSTRING' syntax. This is required when using the 'objectGUID' attribute of Active Directory for the group ID's." introductionVersion:"pre5.0"`
 
 	EducationResourcesEnabled bool `yaml:"education_resources_enabled" env:"GRAPH_LDAP_EDUCATION_RESOURCES_ENABLED" desc:"Enable LDAP support for managing education related resources." introductionVersion:"pre5.0"`
 	EducationConfig           LDAPEducationConfig

services/graph/pkg/identity/ldap.go

@@ -66,8 +66,9 @@ type LDAP struct {
 	groupBaseDN          string
 	groupCreateBaseDN    string
 	groupFilter          string
-	groupObjectClass     string
-	groupIDisOctetString bool
+	groupObjectClass                string
+	groupAdditionalObjectClasses    []string
+	groupIDisOctetString            bool
 	groupScope           int
 	groupAttributeMap    groupAttributeMap
 
@@ -202,6 +203,7 @@ func NewLDAPBackend(lc ldap.Client, config config.LDAP, logger *log.Logger, inst
 		groupCreateBaseDN:              config.GroupCreateBaseDN,
 		groupFilter:                    config.GroupFilter,
 		groupObjectClass:               config.GroupObjectClass,
+		groupAdditionalObjectClasses:   config.GroupAdditionalObjectClasses,
 		groupIDisOctetString:           config.GroupIDIsOctetString,
 		groupScope:                     groupScope,
 		groupAttributeMap:              gam,
@@ -1306,8 +1308,9 @@ func replaceDN(fullDN *ldap.DN, newDN string) (string, error) {
 func (i *LDAP) CreateLDAPGroupByDN(dn string) error {
 	ar := ldap.NewAddRequest(dn, nil)
 
+	objectClasses := append([]string{i.groupObjectClass, "top"}, i.groupAdditionalObjectClasses...)
 	attrs := map[string][]string{
-		"objectClass": {i.groupObjectClass, "top"},
+		"objectClass": objectClasses,
 		"member":      {""},
 	}
 

services/graph/pkg/identity/ldap_group.go

@@ -459,6 +459,8 @@ func (i *LDAP) groupToLDAPAttrValues(group libregraph.Group) (map[string][]strin
 		i.groupAttributeMap.member: {""},
 	}
 
+	attrs["objectClass"] = append(attrs["objectClass"], i.groupAdditionalObjectClasses...)
+
 	if !i.useServerUUID {
 		attrs["owncloudUUID"] = []string{uuid.Must(uuid.NewV4()).String()}
 		attrs["objectClass"] = append(attrs["objectClass"], "owncloud")

services/graph/pkg/identity/ldap_group_test.go

@@ -455,20 +455,31 @@ func TestUpdateGroupName(t *testing.T) {
 
 func TestGroupToLDAPAttrValuesUsesConfiguredObjectClass(t *testing.T) {
 	tests := []struct {
-		name             string
-		groupObjectClass string
+		name                         string
+		groupObjectClass             string
+		groupAdditionalObjectClasses []string
+		expectedObjectClasses        []string
 	}{
 		{
-			name:             "default groupOfNames",
-			groupObjectClass: "groupOfNames",
+			name:                  "default groupOfNames",
+			groupObjectClass:      "groupOfNames",
+			expectedObjectClasses: []string{"groupOfNames", "top", "owncloud"},
 		},
 		{
-			name:             "custom groupOfUniqueNames",
-			groupObjectClass: "groupOfUniqueNames",
+			name:                  "custom groupOfUniqueNames",
+			groupObjectClass:      "groupOfUniqueNames",
+			expectedObjectClasses: []string{"groupOfUniqueNames", "top", "owncloud"},
 		},
 		{
-			name:             "custom posixGroup",
-			groupObjectClass: "posixGroup",
+			name:                  "custom posixGroup",
+			groupObjectClass:      "posixGroup",
+			expectedObjectClasses: []string{"posixGroup", "top", "owncloud"},
+		},
+		{
+			name:                         "additional objectClasses",
+			groupObjectClass:             "groupOfNames",
+			groupAdditionalObjectClasses: []string{"posixGroup", "extensibleObject"},
+			expectedObjectClasses:        []string{"groupOfNames", "top", "posixGroup", "extensibleObject", "owncloud"},
 		},
 	}
 
@@ -477,6 +488,7 @@ func TestGroupToLDAPAttrValuesUsesConfiguredObjectClass(t *testing.T) {
 			// Setup config with custom groupObjectClass
 			testConfig := lconfig
 			testConfig.GroupObjectClass = tt.groupObjectClass
+			testConfig.GroupAdditionalObjectClasses = tt.groupAdditionalObjectClasses
 
 			lm := &mocks.Client{}
 			b, err := getMockedBackend(lm, testConfig, &logger)
@@ -500,14 +512,17 @@ func TestGroupToLDAPAttrValuesUsesConfiguredObjectClass(t *testing.T) {
 				t.Fatal("Expected objectClass attribute to be present")
 			}
 
-			// Check objectClass has exactly 3 elements
-			if len(objectClasses) != 3 {
-				t.Errorf("Expected objectClass to have exactly 3 elements, got %d: %v", len(objectClasses), objectClasses)
+			if len(objectClasses) != len(tt.expectedObjectClasses) {
+				t.Errorf("Expected objectClass to have %d elements, got %d: %v", len(tt.expectedObjectClasses), len(objectClasses), objectClasses)
 			}
 
-			// Check first element is the configured groupObjectClass (exact match)
-			if objectClasses[0] != tt.groupObjectClass {
-				t.Errorf("Expected first objectClass to be '%s', got '%s'", tt.groupObjectClass, objectClasses[0])
+			for i, expected := range tt.expectedObjectClasses {
+				if i >= len(objectClasses) {
+					break
+				}
+				if objectClasses[i] != expected {
+					t.Errorf("Expected objectClass[%d] to be '%s', got '%s'", i, expected, objectClasses[i])
+				}
 			}
 		})
 	}