Skip to content

Commit

Permalink
usersprovider: Allow admins to read disabled users by ID
Browse files Browse the repository at this point in the history
Users with the account management permission need to be able to lookup
disabled users. (E.g. when listing the spaces of a user). This adds the
"Accounts.ReadWrite" permission that is checked when looking up users.
  • Loading branch information
rhafer committed Dec 21, 2023
1 parent c67404e commit 2090ff5
Show file tree
Hide file tree
Showing 4 changed files with 42 additions and 9 deletions.
1 change: 1 addition & 0 deletions changelog/unreleased/fix-hide-disabled-users.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,5 +3,6 @@ Bugfix: Don't return disabled users in GetUser call
We fixed a bug where it was still possible to lookup a disabled User if
the user's ID was known.

https://github.com/cs3org/reva/pull/4430
https://github.com/cs3org/reva/pull/4426
https://github.com/owncloud/ocis/issues/7962
2 changes: 2 additions & 0 deletions pkg/permission/permission.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@ const (
WriteFavorites string = "Favorites.Write"
// DeleteReadOnlyPassword is the hardcoded name for the ReadOnlyPublicLinkPassword.Delete permission
DeleteReadOnlyPassword string = "ReadOnlyPublicLinkPassword.Delete"
// ManageAccounts is the hardcoded name for the Accounts.ReadWrite permission
ManageAccounts string = "Accounts.ReadWrite"
)

// Manager defines the interface for the permission service driver
Expand Down
36 changes: 31 additions & 5 deletions pkg/user/manager/ldap/ldap.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,13 @@ import (
"fmt"
"strconv"

gateway "github.com/cs3org/go-cs3apis/cs3/gateway/v1beta1"
userpb "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1"
"github.com/cs3org/reva/v2/pkg/appctx"
"github.com/cs3org/reva/v2/pkg/errtypes"
"github.com/cs3org/reva/v2/pkg/permission"
"github.com/cs3org/reva/v2/pkg/rgrpc/todo/pool"
"github.com/cs3org/reva/v2/pkg/sharedconf"
"github.com/cs3org/reva/v2/pkg/user"
"github.com/cs3org/reva/v2/pkg/user/manager/registry"
"github.com/cs3org/reva/v2/pkg/utils"
Expand All @@ -41,12 +45,14 @@ func init() {
}

type manager struct {
c *config
ldapClient ldap.Client
c *config
ldapClient ldap.Client
gatewaySelector pool.Selectable[gateway.GatewayAPIClient]
}

type config struct {
utils.LDAPConn `mapstructure:",squash"`
GatewayAddr string `mapstructure:"gateway_addr"`
LDAPIdentity ldapIdentity.Identity `mapstructure:",squash"`
Idp string `mapstructure:"idp"`
// Nobody specifies the fallback uid number for users that don't have a uidNumber set in LDAP
Expand Down Expand Up @@ -91,6 +97,10 @@ func (m *manager) Configure(ml map[string]interface{}) error {
return fmt.Errorf("error setting up Identity config: %w", err)
}
m.c = c
m.gatewaySelector, err = pool.GatewaySelector(sharedconf.GetGatewaySVC(c.GatewayAddr))
if err != nil {
return err
}
return nil
}

Expand All @@ -104,7 +114,15 @@ func (m *manager) GetUser(ctx context.Context, uid *userpb.UserId, skipFetchingG
return nil, errtypes.NotFound("idp mismatch")
}

userEntry, err := m.c.LDAPIdentity.GetLDAPUserByID(log, m.ldapClient, uid.OpaqueId)
gatewayClient, err := m.gatewaySelector.Next()
if err != nil {
return nil, err
}
returnDisabled, err := utils.CheckPermission(ctx, permission.ManageAccounts, gatewayClient)
if err != nil {
return nil, errtypes.InternalError("permission check failed")
}
userEntry, err := m.c.LDAPIdentity.GetLDAPUserByID(log, m.ldapClient, uid.OpaqueId, returnDisabled)
if err != nil {
return nil, err
}
Expand All @@ -116,7 +134,7 @@ func (m *manager) GetUser(ctx context.Context, uid *userpb.UserId, skipFetchingG
return nil, err
}

if m.c.LDAPIdentity.IsLDAPUserInDisabledGroup(log, m.ldapClient, userEntry) {
if !returnDisabled && m.c.LDAPIdentity.IsLDAPUserInDisabledGroup(log, m.ldapClient, userEntry) {
return nil, errtypes.NotFound("user is locally disabled")
}

Expand Down Expand Up @@ -208,7 +226,15 @@ func (m *manager) GetUserGroups(ctx context.Context, uid *userpb.UserId) ([]stri
log.Debug().Str("useridp", uid.Idp).Str("configured idp", m.c.Idp).Msg("IDP mismatch")
return nil, errtypes.NotFound("idp mismatch")
}
userEntry, err := m.c.LDAPIdentity.GetLDAPUserByID(log, m.ldapClient, uid.OpaqueId)
gatewayClient, err := m.gatewaySelector.Next()
if err != nil {
return nil, err
}
returnDisabled, err := utils.CheckPermission(ctx, permission.ManageAccounts, gatewayClient)
if err != nil {
return nil, errtypes.InternalError("permission check failed")
}
userEntry, err := m.c.LDAPIdentity.GetLDAPUserByID(log, m.ldapClient, uid.OpaqueId, returnDisabled)
if err != nil {
log.Debug().Err(err).Interface("userid", uid).Msg("Failed to lookup user")
return []string{}, err
Expand Down
12 changes: 8 additions & 4 deletions pkg/utils/ldap/identity.go
Original file line number Diff line number Diff line change
Expand Up @@ -176,10 +176,10 @@ func (i *Identity) Setup() error {

// GetLDAPUserByID looks up a user by the supplied Id. Returns the corresponding
// ldap.Entry
func (i *Identity) GetLDAPUserByID(log *zerolog.Logger, lc ldap.Client, id string) (*ldap.Entry, error) {
func (i *Identity) GetLDAPUserByID(log *zerolog.Logger, lc ldap.Client, id string, returnDisabled bool) (*ldap.Entry, error) {
var filter string
var err error
if filter, err = i.getUserFilter(id); err != nil {
if filter, err = i.getUserFilter(id, returnDisabled); err != nil {
return nil, err
}
return i.GetLDAPUserByFilter(log, lc, filter)
Expand Down Expand Up @@ -490,7 +490,7 @@ func filterEscapeBinaryUUID(value uuid.UUID) string {
return filtered
}

func (i *Identity) getUserFilter(uid string) (string, error) {
func (i *Identity) getUserFilter(uid string, returnDisabled bool) (string, error) {
var escapedUUID string
if i.User.Schema.IDIsOctetString {
id, err := uuid.Parse(uid)
Expand All @@ -503,12 +503,16 @@ func (i *Identity) getUserFilter(uid string) (string, error) {
escapedUUID = ldap.EscapeFilter(uid)
}

disabledFilter := ""
if !returnDisabled {
disabledFilter = i.disabledFilter()
}
return fmt.Sprintf("(&%s(objectclass=%s)(%s=%s)%s)",
i.User.Filter,
i.User.Objectclass,
i.User.Schema.ID,
escapedUUID,
i.disabledFilter(),
disabledFilter,
), nil
}

Expand Down

0 comments on commit 2090ff5

Please sign in to comment.