Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

User sync writes twice to account table #72

Closed
PVince81 opened this issue Apr 18, 2017 · 4 comments
Closed

User sync writes twice to account table #72

PVince81 opened this issue Apr 18, 2017 · 4 comments

Comments

@PVince81
Copy link
Contributor

Steps

  1. Setup LDAP and specify a user with an email address
  2. Set a breakpoint in SyncService::run()
  3. Run occ user:sync 'OCA\User_LDAP\User_LDAP'
  4. Track it down into processAttributes

Expected

processAttributes doesn't retrieve the account table's IUser entry.
It must return its own IUser instance that stores email, quota, etc.
It's the SyncService that uses these values then to populate the account table

Actual

processAttributes calls UserManager->get($uid) and $user->setEMailAddress() which internally stores the email already into the account table.
Then later on the sync service does the same again. Or it skips it because the email didn't change.

Versions

ownCloud 10.0 beta2+ master

While the end result might look the same, this makes problems with duplicate email addresses not solvable, see owncloud/core#27652

@PVince81 PVince81 added this to the 10.0 milestone Apr 18, 2017
@PVince81
Copy link
Contributor Author

It's here https://github.com/owncloud/user_ldap/blob/v10.0.0beta2/lib/User/User.php#L451

This is all old behavior which is obsoleted by the account table.
A user backend shouldn't do such gymnastics to set email addresses or other values.

@PVince81
Copy link
Contributor Author

@jvillafanez @DeepDiver1975 do we agree that any user backend should return its own implementation of IUser containing the necessary attributes ?

It should be possible here to make LDAP's User class actually implement IUser and have the getter/setters store into local attributes.

@PVince81
Copy link
Contributor Author

Instead of implementing IUser, the backend should implement IProvidesEmailBackend and IProvidesQuotaBackend

@butonic
Copy link
Member

butonic commented Nov 17, 2017

fixed with #125

@butonic butonic closed this as completed Nov 17, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants