Add support for user creation and deletion to GraphAPI (LDAP backend)

[rhafer]

Description

Sending a POST request to /graph/v1.0/users with this body:

{
  "displayName": "Test User",
  "mail": "test@example.org",
  "onPremisesSamAccountName": "test",
  "passwordProfile": {
    "password": "secret"
  }
}

Will create the user test. The call will respond with the just created user (including the server generated ID:

{
    "displayName": "Test User",
    "id": "2b7f0f9c-0739-103c-926e-03e52584738a",
    "mail": "test@example.org",
    "onPremisesSamAccountName": "test"
}

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)
• Technical debt
• Tests only (no source changes)

Checklist:

• Code changes
• Unit tests added
• Acceptance tests added
• Documentation ticket raised:

//cc @refs

[update-docs]

Thanks for opening this pull request! The maintainers of this repository would appreciate it if you would create a changelog item based on your changes.

[butonic]

I'm ok with getting this in after getting rid of Msgf, that is a personal vendetta I have.

Aside from that I am aware of the difficulty of configuring an LDAP schema. The current PR does harcode the inetOrgPerson, and I am not entirely sure that is 100% compatible with AD, which we will hit sooner rather than later. But we can defer making the mapping configurable to a subsequent PR.

Then there is the question of the default user id attribute. I tried giving my reasoning but would really like to better understand your opinion on that.

[butonic]

why three sections of packages? does vim add the deps automatically? vscode distinguishes between stdlib at the top, followed by other packages, sorted alphabetically.

[rhafer]

Yeah this one is in error on my side ;-)

Though projects split into 3 sections. (stdlib, external packages, imports from the local project). But yeas this one is borked.

[butonic]

Suggested change

	errmsg = fmt.Sprintf("too many results searching for user '%s'", nameOrID)
	i.logger.Debug().Str("backend", "ldap").Err(lerr).Msg(errmsg)
	i.logger.Debug().Str("backend", "ldap").Err(lerr).Str("user", nameOrID).Msg("too many results searching for user")

[butonic]

I really don't like defaulting to an attribute that will change when a user is reimported. IMO we should stick to a custom attribute defined by us. I know that means admins have to change it when they cannot import custom attributes. But for our ocis binary we can add that attribute to libreidm.

Choosing an existing attrubute like entryUUID, objectGUID or any other UUID requires us to extensively document why using those attributes can fail in corner cases. Admins will run into that and then the support team will ask engineering for help to repair a deployment. I am just trying to make the defailt implementation as failsafe as possible. Using a dedicated ownclouduuid at least forces admins to then THINK about what attribute to choose. Furthermore, the ownclouduuid will leak via the apis.

[rhafer]

I really don't like defaulting to an attribute that will change when a user is reimported.

If "reimported" the entryUUID will not change. (Provided the operator knows what he's doing). But yeah I get your point.

IMO we should stick to a custom attribute defined by us. I know that means admins have to change it when they cannot import custom attributes. But for our ocis binary we can add that attribute to libreidm.

I'll look into generating a custom uuid from here and probalby add a config knob for this. The whole handling is of the user's unique id in ocis is still huge mistery to me. Especially when integrating with external IDPs and Directories (IIRC we talk about that in the past).
IMO we can never rely on the stability of any externally provided unique identifier. We might just need to do our own internal mapping of identyties to uuid in some form (yet another service?)

[rhafer]

The current PR does harcode the inetOrgPerson, and I am not entirely sure that is 100% compatible with AD, which we will hit sooner rather than later. But we can defer making the mapping configurable to a subsequent PR.

@butonic The hardcoded inetOrgPerson is only for the write part of the API. I strongly suggest (and I think we agreed on that long ago) not to try to manage (add, delete, update) users on an LDAP servers where we don't have full control of the installed schema. That is going to fail miserably. I guess we need to add a switch to explicitly turn on the write part of the LDAP backend.

The read part of the API does work without any hardcoded objectclass and attributetypes. All of those are configurable.

[sonarcloud]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
10 Code Smells

20.0% Coverage
15.4% Duplication

[butonic]

running libregraph/idm with

go run cmd/idmd/main.go serve --ldap-handler boltdb --ldap-base-dn='ou=users,dc=owncloud,dc=com' --log-level=debug --ldap-admin-dn='uid=admin,ou=users,dc=owncloud,dc=com'

and importing the ocis deployment example ldif I can add and modify users 🚀

needs thes env vars for ocis:

                "GRAPH_IDENTITY_BACKEND": "ldap",
                "GRAPH_LDAP_URI": "ldap://localhost:10389",
                "GRAPH_LDAP_BIND_DN": "uid=admin,ou=users,dc=owncloud,dc=com",
                "GRAPH_LDAP_BIND_PASSWORD": "admin",
                "GRAPH_LDAP_SERVER_UUID": "false",
                "GRAPH_LDAP_SERVER_WRITE_ENABLED": "true",
                "GRAPH_LDAP_USER_BASE_DN": "ou=users,dc=owncloud,dc=com"

[butonic]

#nice

adressed