From b76032d0998a9926b20233b650a94266208bd605 Mon Sep 17 00:00:00 2001 From: Julian Koberg Date: Mon, 20 Apr 2026 12:17:40 +0200 Subject: [PATCH] feat(graph): [OCISDEV-794] allow multiple objectClasses on group creation Add GroupAdditionalObjectClasses config field (env vars OCIS_LDAP_GROUP_ADDITIONAL_OBJECTCLASSES / GRAPH_LDAP_GROUP_ADDITIONAL_OBJECTCLASSES) that appends extra objectClasses when creating groups in LDAP, alongside the existing primary GroupObjectClass. Applied to both groupToLDAPAttrValues and CreateLDAPGroupByDN. Signed-off-by: Julian Koberg --- ...ent-ldap-group-additional-objectclasses.md | 8 ++++ .../ldap/schemas/10_owncloud_schema.ldif | 3 +- .../examples/ocis_multi/docker-compose.yml | 12 +++--- services/graph/pkg/config/config.go | 19 ++++---- services/graph/pkg/identity/ldap.go | 9 ++-- services/graph/pkg/identity/ldap_group.go | 2 + .../graph/pkg/identity/ldap_group_test.go | 43 +++++++++++++------ 7 files changed, 61 insertions(+), 35 deletions(-) create mode 100644 changelog/unreleased/enhancement-ldap-group-additional-objectclasses.md diff --git a/changelog/unreleased/enhancement-ldap-group-additional-objectclasses.md b/changelog/unreleased/enhancement-ldap-group-additional-objectclasses.md new file mode 100644 index 00000000000..ee6957d72d4 --- /dev/null +++ b/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 diff --git a/deployments/examples/ocis_multi/config/ldap/schemas/10_owncloud_schema.ldif b/deployments/examples/ocis_multi/config/ldap/schemas/10_owncloud_schema.ldif index 8d288a00176..950f6856005 100644 --- a/deployments/examples/ocis_multi/config/ldap/schemas/10_owncloud_schema.ldif +++ b/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 ) ) diff --git a/deployments/examples/ocis_multi/docker-compose.yml b/deployments/examples/ocis_multi/docker-compose.yml index c36437ac813..89b9c8099ad 100644 --- a/deployments/examples/ocis_multi/docker-compose.yml +++ b/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 diff --git a/services/graph/pkg/config/config.go b/services/graph/pkg/config/config.go index ab0072af375..517a9d978e9 100644 --- a/services/graph/pkg/config/config.go +++ b/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 diff --git a/services/graph/pkg/identity/ldap.go b/services/graph/pkg/identity/ldap.go index a0458dafde0..f786fb47c83 100644 --- a/services/graph/pkg/identity/ldap.go +++ b/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": {""}, } diff --git a/services/graph/pkg/identity/ldap_group.go b/services/graph/pkg/identity/ldap_group.go index e200875c6b2..727be10f9a6 100644 --- a/services/graph/pkg/identity/ldap_group.go +++ b/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") diff --git a/services/graph/pkg/identity/ldap_group_test.go b/services/graph/pkg/identity/ldap_group_test.go index 634de7af5a3..02aafe3ae99 100644 --- a/services/graph/pkg/identity/ldap_group_test.go +++ b/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]) + } } }) }