Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

graph: user groupTypes property to indicate read-only groups #5974

Merged
merged 2 commits into from
Apr 4, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions changelog/unreleased/enhancement-ldap-group-create-basedn.md
Original file line number Diff line number Diff line change
@@ -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

2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ require (
github.com/onsi/gomega v1.27.4
github.com/open-policy-agent/opa v0.50.2
github.com/orcaman/concurrent-map v1.0.0
github.com/owncloud/libre-graph-api-go v1.0.2-0.20230309112802-ff71ba8c90aa
github.com/owncloud/libre-graph-api-go v1.0.2-0.20230330145712-ea267ccd404a
github.com/pkg/errors v0.9.1
github.com/pkg/xattr v0.4.9
github.com/prometheus/client_golang v1.14.0
Expand Down
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -1373,8 +1373,8 @@ github.com/oracle/oci-go-sdk v24.3.0+incompatible/go.mod h1:VQb79nF8Z2cwLkLS35uk
github.com/orcaman/concurrent-map v1.0.0 h1:I/2A2XPCb4IuQWcQhBhSwGfiuybl/J0ev9HDbW65HOY=
github.com/orcaman/concurrent-map v1.0.0/go.mod h1:Lu3tH6HLW3feq74c2GC+jIMS/K2CFcDWnWD9XkenwhI=
github.com/ovh/go-ovh v1.1.0/go.mod h1:AxitLZ5HBRPyUd+Zl60Ajaag+rNTdVXWIkzfrVuTXWA=
github.com/owncloud/libre-graph-api-go v1.0.2-0.20230309112802-ff71ba8c90aa h1:Lab1HHZ7Z8cBjojLDSfGYbGSZT08iww25uTW+EV1Sao=
github.com/owncloud/libre-graph-api-go v1.0.2-0.20230309112802-ff71ba8c90aa/go.mod h1:iKdVH6nYpI8RBeK9sjeLfzrPByST6r9d+NG2IJHoJmU=
github.com/owncloud/libre-graph-api-go v1.0.2-0.20230330145712-ea267ccd404a h1:C7YCoyXn/8pkapUhw2KoIxEMdgIFUx3JjZQKtsXaaLc=
github.com/owncloud/libre-graph-api-go v1.0.2-0.20230330145712-ea267ccd404a/go.mod h1:iKdVH6nYpI8RBeK9sjeLfzrPByST6r9d+NG2IJHoJmU=
github.com/oxtoacart/bpool v0.0.0-20190530202638-03653db5a59c h1:rp5dCmg/yLR3mgFuSOe4oEnDDmGLROTvMragMUXpTQw=
github.com/oxtoacart/bpool v0.0.0-20190530202638-03653db5a59c/go.mod h1:X07ZCGwUbLaax7L0S3Tw4hpejzu63ZrrQiUe6W0hcy0=
github.com/pascaldekloe/goe v0.0.0-20180627143212-57f6aae5913c/go.mod h1:lzWF7FIEvWOWxwDKqyGYQf6ZUaNfKdP144TG7ZOy1lc=
Expand Down
1 change: 1 addition & 0 deletions services/graph/pkg/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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'). "`
Expand Down
4 changes: 4 additions & 0 deletions services/graph/pkg/config/defaults/defaultconfig.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
30 changes: 28 additions & 2 deletions services/graph/pkg/config/parser/parse.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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 == "" {
Expand All @@ -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
}
2 changes: 2 additions & 0 deletions services/graph/pkg/identity/ldap.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ type LDAP struct {
localUserDisableGroupDN string

groupBaseDN string
groupCreateBaseDN string
groupFilter string
groupObjectClass string
groupScope int
Expand Down Expand Up @@ -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,
Expand Down
61 changes: 58 additions & 3 deletions services/graph/pkg/identity/ldap_group.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
}
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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")
Expand All @@ -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 {
Expand All @@ -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) {
Expand Down Expand Up @@ -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 {
Expand Down
Loading