core icon indicating copy to clipboard operation
core copied to clipboard

New sync mechanism

Open jvillafanez opened this issue 1 year ago • 4 comments

Description

New generic sync mechanism, aiming to fix some flaws the current mechanism has. In particular, the errors that could happen during syncing will be reported and be visible to the admin, so he'll know if something goes wrong; the command will fail (exit code different than 0) if something goes wrong. The new sync mechanism will use specialized interfaces, not existing ones.

Advantages:

  • Apps can add their own sync services in order to sync calendar, contacts, federated data, or whatever they want.
  • Apps can also overwrite sync services to provide additional features or optimizations.
  • Expected error handling is documented in the interface in order to be able to forward the exceptions and make them visible to the admin.

This PR includes the user sync service, which will allow admins to sync users from multiple backends. Right now, the DB backend is provided, and support for LDAP will be added in the user_ldap app (https://github.com/owncloud/user_ldap/pull/808). Additional backends can be added in the respective apps.

Some simple examples:

  • occ sync:sync user -> check and sync users from all registered backends (DB included). Missing users will be disabled
  • occ sync:sync user -o 'missingAction=remove' -> check and sync users from all registered backends (DB included). Missing users will be removed
  • occ sync:sync user -o 'backends=OCA\User_LDAP\User_Proxy' -> check and sync users just for that backend (DB users will be ignored). Missing users (from that backend) will be disabled
  • occ sync:sync user -o 'missingAction=remove' -o 'backends=OCA\User_LDAP\User_Proxy' -> check and sync users just for that backend. Missing users will be removed
  • occ sync:sync user --only-one 59a09cfe-f23f-103d-992b-5b46a8a66263 -> check and sync only the user with that uid. If it's missing, the user will be disabled
  • occ sync:sync user --only-one 59a09cfe-f23f-103d-992b-5b46a8a66263 -o 'backends=OC\User\Database' -o 'missingAction=remove' -> check the user with that uid in the specified backend. If the user isn't in that backend (it might be from a different backend), it won't do anything; it will only remove the user if the backend matches and the user is missing. The command will try to sync the user from that backend (which should fail if it belongs to another backend).

Related Issue

https://github.com/owncloud/enterprise/issues/5775

Motivation and Context

The current sync mechanism has an important problems when syncing LDAP users. There could be name collisions and the LDAP part could return less users than requested, causing the sync mechanism to stop and prevent syncing the rest of the LDAP users. The new sync mechanism fixes that problem, and also make the error visible to the admin so it doesn't need to monitor the logs for specific errors.

How Has This Been Tested?

Manually tested, running the command in multiple scenarios

Screenshots (if appropriate):

Types of changes

  • [ ] Bug fix (non-breaking change which fixes an issue)
  • [x] New feature (non-breaking change which adds functionality)
  • [ ] Database schema changes (next release will require increase of minor version instead of patch)
  • [ ] Breaking change (fix or feature that would cause existing functionality to change)
  • [ ] Technical debt
  • [ ] Tests only (no source changes)

Checklist:

  • [x] Code changes
  • [ ] Unit tests added
  • [ ] Acceptance tests added
  • [ ] Documentation ticket raised:
  • [ ] Changelog item, see TEMPLATE

jvillafanez avatar Oct 09 '23 07:10 jvillafanez

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.

update-docs[bot] avatar Oct 09 '23 07:10 update-docs[bot]

Unit tests for the UserSyncer class are incomplete at the moment. New tests need to be added.

jvillafanez avatar Oct 10 '23 16:10 jvillafanez

This is ready to review. The PR could be tested without any additional change in order to sync the DB users (not much useful). LDAP syncing is in https://github.com/owncloud/user_ldap/pull/808 ; the code can be run although it isn't fully finished yet due to missing tests.

jvillafanez avatar Oct 23 '23 11:10 jvillafanez