owncloud / ocis

:atom_symbol: ownCloud Infinite Scale Stack
https://doc.owncloud.com/ocis/next/
Apache License 2.0
1.37k stars 180 forks source link

Support for AD and other ldap providers for the graph microservice #4467

Open jvillafanez opened 2 years ago

jvillafanez commented 2 years ago

Is your feature request related to a problem? Please describe.

The graph microservice uses ldap to connect to an internal libregraph-idm service. It can be useful if graph could connect to other ldap providers such as AD if needed.

Describe the solution you'd like

Multiple implementations can be provided taking advantage of provider's details in order to improve performance when accessing the ldap server

Describe alternatives you've considered

Additional context

We can create a package in the ocis-pkg folder that can be reused by multiple microservices if needed. For now, the main "client" will be the graph microservice.

The package will be divided as follows:

Additional implementations can be added later if needed. Note that the libregraph implementation might be the less restrictive and might be used as default. As said, other implementations are expected to provide improved performance.

In order to provide a common access to all available providers, the following interface will be used:

type LdapConnector interface {
  IsReadOnly() bool

  CreateUser(ctx context.Context, user *libregraph.User) (*libregraph.User, error)
  DeleteUser(ctx context.Context, DN string) error
  UpdateUser(ctx context.Context, DN string, user *libregraph.User) (*libregraph.User, error)
  CreateGroup(ctx context.Context, group *libregraph.Group) (*libregraph.Group, error)
  DeleteGroup(ctx context.Context, DN string) error
  AddMembersToGroup(ctx context.Context, groupDN string, memberDNs []string) error
  RemoveMemberFromGroup(ctx context.Context, groupDN string, memberDN string) error

  GetUsers(ctx context.Context, filter string) ([]*libregraph.User, error)
  GetGroups(ctx context.Context, filter string) ([]*libregraph.Group, error)
  GetGroupMembers(ctx context.Context, DN string) ([]*libregraph.User, error)

  GetUserAttrMap() UserAttrMap
  GetGroupAttrMap() GroupAttrMap
}

Note the following restrictions:

Taking that into account, it's expected that the AD implementation will hide a couple of things from the graph service:

Both of those points will be difficult to deal with if we need to take into account additional restrictions that other providers than AD could impose.


While the goal could be to add support for AD, the design should allow more providers to be added as needed. It's expected that the current connection used for the libregraph-idm is generic enough to be used with all the providers, but at expense of having a poor performance on some operations.

From a configuration point of view, graph (and any microservice that needs it) can use a default generic connector if none is provided. Mappings for each connector should be documented, so if they don't match with the provider, the admin should adjust the mapping. Configuring the right connector for the provider should be easier (less mapping should be adjusted if the default ones are good enough) and also provide a performance boost.

Note that it's unlikely that we can ensure, for example, that the AD connector is only used for AD. If the admin configures a wrong connector things can go really wrong.


To be checked:

rhafer commented 2 years ago

Is your feature request related to a problem? Please describe.

The graph microservice uses ldap to connect to an internal libregraph-idm service. It can be useful if graph could connect to other ldap providers such as AD if needed.

Note: The current implementation is in no way specific to libregraph-idm. It tries to implement a generic LDAP interface that should be usable with most (/any) standard compliant LDAP server. You are of course right that in order to support AD in a proper fashion we need to deal with some of the quirks that AD did to LDAP (i.e. you mentioned "range retrivals" for objects with large sets of multi-valiued attributes). I am not sure yet, whether that needs to be implemented as a separte AD backend or can be intergated in to a generic LDAP backend. But yeah adding a specific AD backend might be one way to deal with that to keep the AD specifics contained.

Note however that the LDAP backend in the graph API was mainly introduced as an ad-hoc solution to be able to move forward more quickly to a working solution for builtin Usermanagement (creating/deleteing). In the long run the focus should be on the CS3 users and groups providers. So I think we should probably focus on the LDAP backend for CS3. And use the CS3 backend of the graph-API for all setups that don't require write access to the LDAP server.

This would have the benefit that all other services that use CS3 for user/group lookups (which are basically all our service expect graph) could take advantage of any optimizations/AD specifics that we add.

jvillafanez commented 2 years ago

Ok, so if I understand it correctly, read-only access should go through cs3 while write access should go through ocis-specific code (cs3 APIs don't provide write access to users and groups as far as I know).

If there are no plans to include write access to the cs3 APIs, I think we'll need to use our own interface. Taking into account that the focus is to support cs3, the current interface (https://github.com/owncloud/ocis/blob/master/services/graph/pkg/identity/backend.go#L11-L32) seems good. Maybe, the only change to do would be to move the implementations to a different package ("services/graph/identity/impls"). I assume that the rest of the ocis' services will get the users and groups from graph, so there shouldn't be any need to move it outside of the graph service.

You are of course right that in order to support AD in a proper fashion we need to deal with some of the quirks that AD did to LDAP (i.e. you mentioned "range retrivals" for objects with large sets of multi-valiued attributes). I am not sure yet, whether that needs to be implemented as a separte AD backend or can be integrated in to a generic LDAP backend. But yeah adding a specific AD backend might be one way to deal with that to keep the AD specifics contained.

I'd suggest different implementations per provider. A generic implementation might be possible, but either you'll limit the features hindering the performance, or you'll need to deal with too many cases making the implementation fragile to changes.

Anyway, I don't think we'll need multiple implementations: either we only have read-only access through cs3, or we have read-write access to the builtin service we have full control on. If the admin wants to use an external ldap service, we should access through cs3 and let the admin update any ldap stuff through the provider's UI (pretty sure we don't want to add new users to corporate environments).

jvillafanez commented 2 years ago

I'm ok with closing this issue:

We can move the ticket to reva though if it still make sense there.

rhafer commented 2 years ago

We can move the ticket to reva though if it still make sense there.

Yeah, I'd say it definitely makes sense.

I guess we also need to check for possible gaps in the graph cs3 backend. (I think that one hasn't seen much love recently)

stale[bot] commented 1 year ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 10 days if no further activity occurs. Thank you for your contributions.

stale[bot] commented 1 year ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 10 days if no further activity occurs. Thank you for your contributions.