From bea3ec6207e2e2c71f423e45daf72b1051f399e8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dani=C3=ABl=20Franke?= Date: Mon, 20 Feb 2023 13:02:42 +0100 Subject: [PATCH] Add refint support to user rename. When refint is enabled on an LDAP server, it will rename all references to an entity if its DN is modified. If this happens, the member renames will not be needed, and will also return an error. This PR does the following: * Detects the attribute error, and don't return an error. * Log that the server has been misconfigured. * Add config value that skips renaming if set. --- .../examples/ocis_ldap/docker-compose.yml | 13 +++++--- services/graph/pkg/config/config.go | 1 + services/graph/pkg/identity/ldap.go | 31 +++++++++++++------ 3 files changed, 30 insertions(+), 15 deletions(-) diff --git a/deployments/examples/ocis_ldap/docker-compose.yml b/deployments/examples/ocis_ldap/docker-compose.yml index b5538240e..2de5e021d 100644 --- a/deployments/examples/ocis_ldap/docker-compose.yml +++ b/deployments/examples/ocis_ldap/docker-compose.yml @@ -57,17 +57,17 @@ services: # run ocis init to initialize a configuration file with random secrets # it will fail on subsequent runs, because the config file already exists # therefore we ignore the error and then start the ocis server - command: ["-c", "ocis init || true; ocis server"] + command: [ "-c", "ocis init || true; ocis server" ] environment: # users/gropups from ldap LDAP_URI: ldaps://ldap-server LDAP_INSECURE: "true" LDAP_BIND_DN: "cn=admin,dc=owncloud,dc=com" LDAP_BIND_PASSWORD: ${LDAP_ADMIN_PASSWORD:-admin} - LDAP_GROUP_BASE_DN: "dc=owncloud,dc=com" + LDAP_GROUP_BASE_DN: "ou=groups,dc=owncloud,dc=com" LDAP_GROUP_FILTER: "(objectclass=owncloud)" LDAP_GROUP_OBJECTCLASS: "groupOfNames" - LDAP_USER_BASE_DN: "dc=owncloud,dc=com" + LDAP_USER_BASE_DN: "ou=users,dc=owncloud,dc=com" LDAP_USER_FILTER: "(objectclass=owncloud)" LDAP_USER_OBJECTCLASS: "inetOrgPerson" LDAP_LOGIN_ATTRIBUTES: "uid" @@ -76,18 +76,20 @@ services: IDP_LDAP_LOGIN_ATTRIBUTE: "uid" IDP_LDAP_UUID_ATTRIBUTE: "ownclouduuid" IDP_LDAP_UUID_ATTRIBUTE_TYPE: binary - GRAPH_LDAP_SERVER_WRITE_ENABLED: "false" # assuming the external ldap is readonly + GRAPH_LDAP_SERVER_WRITE_ENABLED: "true" # assuming the external ldap is readonly + GRAPH_LDAP_REFINT_ENABLED: "true" # osixia has refint enabled. # OCIS_RUN_SERVICES specifies to start all services except glauth, idm and accounts. These are replaced by external services OCIS_RUN_SERVICES: app-registry,app-provider,audit,auth-basic,auth-machine,frontend,gateway,graph,groups,idp,nats,notifications,ocdav,ocs,proxy,search,settings,sharing,storage-system,storage-publiclink,storage-shares,storage-users,store,thumbnails,users,web,webdav # General oCIS config OCIS_URL: https://${OCIS_DOMAIN:-ocis.owncloud.test} OCIS_LOG_LEVEL: ${OCIS_LOG_LEVEL:-info} OCIS_LOG_COLOR: "${OCIS_LOG_COLOR:-false}" + GRAPH_LOG_LEVEL: "debug" PROXY_TLS: "false" # do not use SSL between Traefik and oCIS # INSECURE: needed if oCIS / Traefik is using self generated certificates OCIS_INSECURE: "${INSECURE:-false}" # basic auth (not recommended, but needed for eg. WebDav clients that do not support OpenID Connect) - PROXY_ENABLE_BASIC_AUTH: "${PROXY_ENABLE_BASIC_AUTH:-false}" + PROXY_ENABLE_BASIC_AUTH: "${PROXY_ENABLE_BASIC_AUTH:-true}" # admin user password volumes: - ocis-config:/etc/ocis @@ -147,5 +149,6 @@ volumes: ocis-config: ocis-data: + networks: ocis-net: diff --git a/services/graph/pkg/config/config.go b/services/graph/pkg/config/config.go index 78978a754..cd2c8be5c 100644 --- a/services/graph/pkg/config/config.go +++ b/services/graph/pkg/config/config.go @@ -51,6 +51,7 @@ type LDAP struct { UseServerUUID bool `yaml:"use_server_uuid" env:"GRAPH_LDAP_SERVER_UUID" desc:"If set to true, rely on the LDAP Server to generate a unique ID for users and groups, like when using 'entryUUID' as the user ID attribute."` UsePasswordModExOp bool `yaml:"use_password_modify_exop" env:"GRAPH_LDAP_SERVER_USE_PASSWORD_MODIFY_EXOP" desc:"User the Password Modify Extended Operation for updating user passwords."` WriteEnabled bool `yaml:"write_enabled" env:"GRAPH_LDAP_SERVER_WRITE_ENABLED" desc:"Allow to create, modify and delete LDAP users via GRAPH API. This is only works when the default Schema is used."` + RefintEnabled bool `yaml:"refint_enabled" env:"GRAPH_LDAP_REFINT_ENABLED" desc:"Signals that the server has the refint plugin enabled, which makes some actions not needed."` UserBaseDN string `yaml:"user_base_dn" env:"LDAP_USER_BASE_DN;GRAPH_LDAP_USER_BASE_DN" desc:"Search base DN for looking up LDAP users."` UserSearchScope string `yaml:"user_search_scope" env:"LDAP_USER_SCOPE;GRAPH_LDAP_USER_SCOPE" desc:"LDAP search scope to use when looking up users. Supported scopes are 'base', 'one' and 'sub'."` diff --git a/services/graph/pkg/identity/ldap.go b/services/graph/pkg/identity/ldap.go index 4bd7488c6..09522802b 100644 --- a/services/graph/pkg/identity/ldap.go +++ b/services/graph/pkg/identity/ldap.go @@ -27,6 +27,7 @@ const ( type LDAP struct { useServerUUID bool writeEnabled bool + refintEnabled bool usePwModifyExOp bool userBaseDN string @@ -116,6 +117,7 @@ func NewLDAPBackend(lc ldap.Client, config config.LDAP, logger *log.Logger) (*LD logger: logger, conn: lc, writeEnabled: config.WriteEnabled, + refintEnabled: config.RefintEnabled, }, nil } @@ -489,10 +491,6 @@ func (i *LDAP) GetUsers(ctx context.Context, oreq *godata.GoDataRequest) ([]*lib func (i *LDAP) changeUserName(ctx context.Context, dn, originalUserName, newUserName string) (*ldap.Entry, error) { logger := i.logger.SubloggerWithRequestID(ctx) - groups, err := i.getGroupsForUser(dn) - if err != nil { - return nil, err - } newDN := fmt.Sprintf("%s=%s", i.userAttributeMap.userName, newUserName) @@ -525,14 +523,21 @@ func (i *LDAP) changeUserName(ctx context.Context, dn, originalUserName, newUser return nil, err } - for _, g := range groups { - err = i.renameMemberInGroup(ctx, g, dn, u.DN) - // This could leave the groups in an inconsistent state, might be a good idea to - // add a defer that changes everything back on error. Ideally, this entire function - // should be atomic, but LDAP doesn't support that. + if !i.refintEnabled { + groups, err := i.getGroupsForUser(dn) if err != nil { return nil, err } + for _, g := range groups { + logger.Debug().Str("originalDN", dn).Str("newDN", u.DN).Str("group", g.DN).Msg("Changing member in group") + err = i.renameMemberInGroup(ctx, g, dn, u.DN) + // This could leave the groups in an inconsistent state, might be a good idea to + // add a defer that changes everything back on error. Ideally, this entire function + // should be atomic, but LDAP doesn't support that. + if err != nil { + return nil, err + } + } } return u, nil @@ -551,6 +556,13 @@ func (i *LDAP) renameMemberInGroup(ctx context.Context, group *ldap.Entry, oldMe groupID := group.GetEqualFoldAttributeValue(i.groupAttributeMap.id) logger.Warn().Str("group", groupID).Msg("Group no longer exists") return nil + } else if lerr.ResultCode == ldap.LDAPResultNoSuchAttribute { + logger.Warn(). + Str("oldMember", oldMember). + Str("newMember", newMember). + Str("groupDN", group.DN). + Msg("member attribute not found, this probably means that the server has refint enabled, please configure the OCIS to respect that.") + return nil } } return err @@ -692,7 +704,6 @@ func pointerOrNil(val string) *string { func booleanOrNil(val string) *bool { boolValue, err := strconv.ParseBool(val) - if err != nil { return nil }