Skip to content

Commit

Permalink
Fixed possible null pointer dereference in ldap authprovider (#3086)
Browse files Browse the repository at this point in the history
Return a proper error the userprovider is returning a not OK status
code. Also add some more logging to be able to recognize configuration
issue more easily (e.g. a mismatch in the IDP settings of the
auth/user-providers)
  • Loading branch information
rhafer authored Jul 19, 2022
1 parent 08f504f commit 204772c
Show file tree
Hide file tree
Showing 4 changed files with 15 additions and 1 deletion.
7 changes: 7 additions & 0 deletions changelog/unreleased/fix-authprovider-idp-err.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
Bugfix: Fix crash in ldap authprovider

We fixed possible crash in the LDAP authprovider caused by a null pointer
derefence, when the IDP settings of the userprovider are different from
the authprovider.

https://github.com/cs3org/reva/pull/3086
3 changes: 3 additions & 0 deletions internal/grpc/services/userprovider/userprovider.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import (
"sort"

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/plugin"
"github.com/cs3org/reva/v2/pkg/rgrpc"
Expand Down Expand Up @@ -184,8 +185,10 @@ func (s *service) FindUsers(ctx context.Context, req *userpb.FindUsersRequest) (
}

func (s *service) GetUserGroups(ctx context.Context, req *userpb.GetUserGroupsRequest) (*userpb.GetUserGroupsResponse, error) {
log := appctx.GetLogger(ctx)
groups, err := s.usermgr.GetUserGroups(ctx, req.UserId)
if err != nil {
log.Warn().Err(err).Interface("userid", req.UserId).Msg("error getting user groups")
res := &userpb.GetUserGroupsResponse{
Status: status.NewInternal(ctx, "error getting user groups"),
}
Expand Down
4 changes: 3 additions & 1 deletion pkg/auth/manager/ldap/ldap.go
Original file line number Diff line number Diff line change
Expand Up @@ -148,10 +148,12 @@ func (am *mgr) Authenticate(ctx context.Context, clientID, clientSecret string)
UserId: userID,
})
if err != nil {
log.Warn().Err(err).Msg("error getting user groups")
return nil, nil, errors.Wrap(err, "ldap: error getting user groups")
}
if getGroupsResp.Status.Code != rpc.Code_CODE_OK {
return nil, nil, errors.Wrap(err, "ldap: grpc getting user groups failed")
log.Warn().Err(err).Str("msg", getGroupsResp.Status.Message).Msg("grpc getting user groups failed")
return nil, nil, fmt.Errorf("ldap: grpc getting user groups failed: '%s'", getGroupsResp.Status.Message)
}
gidNumber := am.c.Nobody
gidValue := userEntry.GetEqualFoldAttributeValue(am.c.LDAPIdentity.User.Schema.GIDNumber)
Expand Down
2 changes: 2 additions & 0 deletions pkg/user/manager/ldap/ldap.go
Original file line number Diff line number Diff line change
Expand Up @@ -197,10 +197,12 @@ func (m *manager) FindUsers(ctx context.Context, query string, skipFetchingGroup
func (m *manager) GetUserGroups(ctx context.Context, uid *userpb.UserId) ([]string, error) {
log := appctx.GetLogger(ctx)
if uid.Idp != "" && uid.Idp != m.c.Idp {
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)
if err != nil {
log.Debug().Err(err).Interface("userid", uid).Msg("Failed to lookup user")
return []string{}, err
}
return m.c.LDAPIdentity.GetLDAPUserGroups(log, m.ldapClient, userEntry)
Expand Down

0 comments on commit 204772c

Please sign in to comment.