graph: new config option GRAPH_LDAP_GROUP_CREATE_BASE_DN

By setting GRAPH_LDAP_GROUP_CREATE_BASE_DN a distinct subtree can be
configured where new LDAP groups are created. That subtree needs to be
subordinate to GRAPH_LDAP_GROUP_BASE_DN. All groups outside for
GRAPH_LDAP_GROUP_CREATE_BASE_DN are considered read-only and only groups
below that DN can be updated and deleted.

This is introduced for a pretty specific usecase where most groups are managed
in an external source (e.g. a read-only replica of an LDAP tree). But we still
want to allow the local administrator to create groups in a writeable subtree
attached to that replica.

changelog/unreleased/enhancement-ldap-group-create-basedn.md

@@ -0,0 +1,13 @@
+Enhancement: Make the LDAP base DN for new groups configurable
+
+The LDAP backend for the Graph service introduced a new config option for setting the
+Parent DN for new groups created via the `/groups/` endpoint. (`GRAPH_LDAP_GROUP_CREATE_BASE_DN`)
+
+It defaults to the value of `GRAPH_LDAP_GROUP_BASE_DN`. If set to a different value the
+`GRAPH_LDAP_GROUP_CREATE_BASE_DN` needs to be a subordinate DN of `GRAPH_LDAP_GROUP_BASE_DN`.
+
+All existing groups with a DN outside the `GRAPH_LDAP_GROUP_CREATE_BASE_DN` tree will be treated as
+read-only groups. So it is not possible to edit these groups.
+
+https://github.com/owncloud/ocis/pull/5974
+

services/graph/pkg/config/config.go

@@ -69,6 +69,7 @@ type LDAP struct {
 	LdapDisabledUsersGroupDN string `yaml:"ldap_disabled_users_group_dn" env:"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'."`
 
 	GroupBaseDN        string `yaml:"group_base_dn" env:"LDAP_GROUP_BASE_DN;GRAPH_LDAP_GROUP_BASE_DN" desc:"Search base DN for looking up LDAP groups."`
+	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."`
 	GroupSearchScope   string `yaml:"group_search_scope" env:"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'."`
 	GroupFilter        string `yaml:"group_filter" env:"LDAP_GROUP_FILTER;GRAPH_LDAP_GROUP_FILTER" desc:"LDAP filter to add to the default filters for group searches."`
 	GroupObjectClass   string `yaml:"group_objectclass" env:"LDAP_GROUP_OBJECTCLASS;GRAPH_LDAP_GROUP_OBJECTCLASS" desc:"The object class to use for groups in the default group search filter ('groupOfNames'). "`

services/graph/pkg/config/defaults/defaultconfig.go

@@ -153,6 +153,10 @@ func EnsureDefaults(cfg *config.Config) {
 	if cfg.MachineAuthAPIKey == "" && cfg.Commons != nil && cfg.Commons.MachineAuthAPIKey != "" {
 		cfg.MachineAuthAPIKey = cfg.Commons.MachineAuthAPIKey
 	}
+
+	if cfg.Identity.LDAP.GroupCreateBaseDN == "" {
+		cfg.Identity.LDAP.GroupCreateBaseDN = cfg.Identity.LDAP.GroupBaseDN
+	}
 }
 
 // Sanitize sanitized the configuration

services/graph/pkg/config/parser/parse.go

@@ -4,6 +4,7 @@ import (
 	"errors"
 	"fmt"
 
+	"github.com/go-ldap/ldap/v3"
 	ociscfg "github.com/owncloud/ocis/v2/ocis-pkg/config"
 	defaults2 "github.com/owncloud/ocis/v2/ocis-pkg/config/defaults"
 	"github.com/owncloud/ocis/v2/ocis-pkg/shared"
@@ -40,8 +41,10 @@ func Validate(cfg *config.Config) error {
 		return shared.MissingJWTTokenError(cfg.Service.Name)
 	}
 
-	if cfg.Identity.Backend == "ldap" && cfg.Identity.LDAP.BindPassword == "" {
-		return shared.MissingLDAPBindPassword(cfg.Service.Name)
+	if cfg.Identity.Backend == "ldap" {
+		if err := validateLDAPSettings(cfg); err != nil {
+			return err
+		}
 	}
 
 	if cfg.Application.ID == "" {
@@ -64,3 +67,26 @@ func Validate(cfg *config.Config) error {
 
 	return nil
 }
+
+func validateLDAPSettings(cfg *config.Config) error {
+	if cfg.Identity.LDAP.BindPassword == "" {
+		return shared.MissingLDAPBindPassword(cfg.Service.Name)
+	}
+
+	// ensure that "GroupBaseDN" is below "GroupBaseDN"
+	if cfg.Identity.LDAP.WriteEnabled && cfg.Identity.LDAP.GroupCreateBaseDN != cfg.Identity.LDAP.GroupBaseDN {
+		baseDN, err := ldap.ParseDN(cfg.Identity.LDAP.GroupBaseDN)
+		if err != nil {
+			return fmt.Errorf("Unable to parse the LDAP Group Base DN '%s': %w ", cfg.Identity.LDAP.GroupBaseDN, err)
+		}
+		createBaseDN, err := ldap.ParseDN(cfg.Identity.LDAP.GroupCreateBaseDN)
+		if err != nil {
+			return fmt.Errorf("Unable to parse the LDAP Group Create Base DN '%s': %w ", cfg.Identity.LDAP.GroupCreateBaseDN, err)
+		}
+
+		if !baseDN.AncestorOfFold(createBaseDN) {
+			return fmt.Errorf("The LDAP Group Create Base DN (%s) must be subordinate to the LDAP Group Base DN (%s)", cfg.Identity.LDAP.GroupCreateBaseDN, cfg.Identity.LDAP.GroupBaseDN)
+		}
+	}
+	return nil
+}

services/graph/pkg/identity/ldap.go

@@ -58,6 +58,7 @@ type LDAP struct {
 	localUserDisableGroupDN string
 
 	groupBaseDN       string
+	groupCreateBaseDN string
 	groupFilter       string
 	groupObjectClass  string
 	groupScope        int
@@ -148,6 +149,7 @@ func NewLDAPBackend(lc ldap.Client, config config.LDAP, logger *log.Logger) (*LD
 		userScope:               userScope,
 		userAttributeMap:        uam,
 		groupBaseDN:             config.GroupBaseDN,
+		groupCreateBaseDN:       config.GroupCreateBaseDN,
 		groupFilter:             config.GroupFilter,
 		groupObjectClass:        config.GroupObjectClass,
 		groupScope:              groupScope,

services/graph/pkg/identity/ldap_group.go

@@ -204,10 +204,16 @@ func (i *LDAP) DeleteGroup(ctx context.Context, id string) error {
 	if !i.writeEnabled {
 		return errorcode.New(errorcode.NotAllowed, "server is configured read-only")
 	}
+
 	e, err := i.getLDAPGroupByID(id, false)
 	if err != nil {
 		return err
 	}
+
+	if i.isLDAPGroupReadOnly(e) {
+		return errorcode.New(errorcode.NotAllowed, "group is read-only")
+	}
+
 	dr := ldap.DelRequest{DN: e.DN}
 	if err = i.conn.Del(&dr); err != nil {
 		return err
@@ -219,12 +225,19 @@ func (i *LDAP) DeleteGroup(ctx context.Context, id string) error {
 func (i *LDAP) UpdateGroupName(ctx context.Context, groupID string, groupName string) error {
 	logger := i.logger.SubloggerWithRequestID(ctx)
 	logger.Debug().Str("backend", "ldap").Msg("AddMembersToGroup")
+	if !i.writeEnabled {
+		return errorcode.New(errorcode.NotAllowed, "server is configured read-only")
+	}
 
 	ge, err := i.getLDAPGroupByID(groupID, true)
 	if err != nil {
 		return err
 	}
 
+	if i.isLDAPGroupReadOnly(ge) {
+		return errorcode.New(errorcode.NotAllowed, "group is read-only")
+	}
+
 	if ge.GetEqualFoldAttributeValue(i.groupAttributeMap.name) == groupName {
 		return nil
 	}
@@ -258,11 +271,18 @@ func (i *LDAP) UpdateGroupName(ctx context.Context, groupID string, groupName st
 func (i *LDAP) AddMembersToGroup(ctx context.Context, groupID string, memberIDs []string) error {
 	logger := i.logger.SubloggerWithRequestID(ctx)
 	logger.Debug().Str("backend", "ldap").Msg("AddMembersToGroup")
+	if !i.writeEnabled {
+		return errorcode.New(errorcode.NotAllowed, "server is configured read-only")
+	}
 	ge, err := i.getLDAPGroupByNameOrID(groupID, true)
 	if err != nil {
 		return err
 	}
 
+	if i.isLDAPGroupReadOnly(ge) {
+		return errorcode.New(errorcode.NotAllowed, "group is read-only")
+	}
+
 	mr := ldap.ModifyRequest{DN: ge.DN}
 	// Handle empty groups (using the empty member attribute)
 	current := ge.GetEqualFoldAttributeValues(i.groupAttributeMap.member)
@@ -326,11 +346,20 @@ func (i *LDAP) AddMembersToGroup(ctx context.Context, groupID string, memberIDs
 func (i *LDAP) RemoveMemberFromGroup(ctx context.Context, groupID string, memberID string) error {
 	logger := i.logger.SubloggerWithRequestID(ctx)
 	logger.Debug().Str("backend", "ldap").Msg("RemoveMemberFromGroup")
+	if !i.writeEnabled {
+		return errorcode.New(errorcode.NotAllowed, "server is configured read-only")
+	}
+
 	ge, err := i.getLDAPGroupByID(groupID, true)
 	if err != nil {
 		logger.Debug().Str("backend", "ldap").Str("groupID", groupID).Msg("Error looking up group")
 		return err
 	}
+
+	if i.isLDAPGroupReadOnly(ge) {
+		return errorcode.New(errorcode.NotAllowed, "group is read-only")
+	}
+
 	me, err := i.getLDAPUserByID(memberID)
 	if err != nil {
 		logger.Debug().Str("backend", "ldap").Str("memberID", memberID).Msg("Error looking up group member")
@@ -345,7 +374,7 @@ func (i *LDAP) RemoveMemberFromGroup(ctx context.Context, groupID string, member
 }
 
 func (i *LDAP) groupToAddRequest(group libregraph.Group) (*ldap.AddRequest, error) {
-	ar := ldap.NewAddRequest(i.getGroupLDAPDN(group), nil)
+	ar := ldap.NewAddRequest(i.getGroupCreateLDAPDN(group), nil)
 
 	attrMap, err := i.groupToLDAPAttrValues(group)
 	if err != nil {
@@ -357,12 +386,12 @@ func (i *LDAP) groupToAddRequest(group libregraph.Group) (*ldap.AddRequest, erro
 	return ar, nil
 }
 
-func (i *LDAP) getGroupLDAPDN(group libregraph.Group) string {
+func (i *LDAP) getGroupCreateLDAPDN(group libregraph.Group) string {
 	attributeTypeAndValue := ldap.AttributeTypeAndValue{
 		Type:  "cn",
 		Value: group.GetDisplayName(),
 	}
-	return fmt.Sprintf("%s,%s", attributeTypeAndValue.String(), i.groupBaseDN)
+	return fmt.Sprintf("%s,%s", attributeTypeAndValue.String(), i.groupCreateBaseDN)
 }
 
 func (i *LDAP) groupToLDAPAttrValues(group libregraph.Group) (map[string][]string, error) {
@@ -482,17 +511,43 @@ func (i *LDAP) getGroupsForUser(dn string) ([]*ldap.Entry, error) {
 func (i *LDAP) createGroupModelFromLDAP(e *ldap.Entry) *libregraph.Group {
 	name := e.GetEqualFoldAttributeValue(i.groupAttributeMap.name)
 	id := e.GetEqualFoldAttributeValue(i.groupAttributeMap.id)
+	groupTypes := []string{}
+
+	if i.isLDAPGroupReadOnly(e) {
+		groupTypes = []string{"ReadOnly"}
+	}
 
 	if id != "" && name != "" {
 		return &libregraph.Group{
 			DisplayName: &name,
 			Id:          &id,
+			GroupTypes:  groupTypes,
 		}
 	}
 	i.logger.Warn().Str("dn", e.DN).Msg("Group is missing name or id")
 	return nil
 }
 
+func (i *LDAP) isLDAPGroupReadOnly(e *ldap.Entry) bool {
+	if !i.writeEnabled {
+		return true
+	}
+
+	groupDN, err := ldap.ParseDN(e.DN)
+	if err != nil {
+		i.logger.Warn().Err(err).Str("dn", e.DN).Msg("Failed to parse DN")
+		return false
+	}
+
+	baseDN, err := ldap.ParseDN(i.groupCreateBaseDN)
+	if err != nil {
+		i.logger.Warn().Err(err).Str("dn", i.groupCreateBaseDN).Msg("Failed to parse DN")
+		return false
+	}
+
+	return !baseDN.AncestorOfFold(groupDN)
+}
+
 func (i *LDAP) groupsFromLDAPEntries(e []*ldap.Entry) []libregraph.Group {
 	groups := make([]libregraph.Group, 0, len(e))
 	for _, g := range e {