Backport 18469 stable8.1 by blizzz · Pull Request #19427 · owncloud/core

blizzz · 2015-09-28T21:14:25Z

Backport of #18469 to stable8.1

Please test and review @MorrisJobke @davitol @tkaufm @Xenopathic

blizzz · 2015-09-28T21:16:05Z

@DeepDiver1975 does that thing run tests against stable8.1 somehow? the failing master thing is totally unrelated.

MorrisJobke · 2015-09-29T06:36:44Z

I just tested this and emails are not imported when I checkout this branch.

Then I upgraded the instance to master and rechecked: the emails are properly imported. Also uid and displayName now show up in oc_preferences. (stable8.1 just puts homePath entries into the preferences).

blizzz · 2015-09-29T10:21:12Z

@MorrisJobke how exactly did you test?

MorrisJobke · 2015-09-29T11:21:46Z

@MorrisJobke how exactly did you test?

set up the docker instance
add zombies with emails - see Add fake email address for users administration#56
checkout stable8.1
setup LDAP
check that no emails are prefilled into the DB
checkout this branch
share with a user
❌ check that email is provisioned in the DB

The same steps works if I'm on master.

blizzz · 2015-09-29T12:41:18Z

@MorrisJobke i tried the same… can you verify the email attribute is set in advanced settings?

MorrisJobke · 2015-09-29T13:15:00Z

@MorrisJobke i tried the same… can you verify the email attribute is set in advanced settings?

Yes, it is set. I will retry

blizzz · 2015-09-29T13:26:10Z

This needs to be in brackets, otherwise it returns 0 instead $valueFromLDAP if that was set. I change to more readable, simpler two-line approach. (This will need to get to master as well.)

blizzz · 2015-10-01T13:35:11Z

@MorrisJobke did you retry?

blizzz · 2015-10-01T15:56:01Z

@MorrisJobke thx for the elucidating screenshare – should work now for you, too :)

blizzz · 2015-10-12T14:01:10Z

requires a backport of #19635 after merging.

…search, in one query) read all relevant user attributes on login and user search, in one query. saves us some. Conflicts: apps/user_ldap/user_ldap.php adjust to nested group fix do not throw exception when no attribute is specified

…bute names coming from ldap are lowercased for easy comparison

MorrisJobke · 2015-10-22T10:01:38Z

I rebased this on current stable8.1

MorrisJobke · 2015-10-22T10:12:39Z

requires a backport of #19635 after merging.

I created a backport of #19635 with this PR included - see #19972

MorrisJobke · 2015-10-22T13:53:44Z

I tested this and the email attribute is correctly loaded on demand 👍

@davitol Can you please test this too? Thanks

MorrisJobke · 2015-10-22T13:54:17Z

@davitol Can you please test this too? Thanks

Maybe test this together with #19972 (which has the commits in here included)

davitol · 2015-10-26T09:14:12Z

@MorrisJobke I've tested today in stable8.1 branch WITHOUT applying the patch.

Sharing a file and notifiying by email WFM with a LDAP user never logged before 😕
The mail that contains the following text arrived without problems:

Hey there,

just letting you know that admin shared nuevoservicio.txt with you.
View it!

Cheers!

ownCloud Enterprise Edition 8.1.4 RC1 (daily) Build:2015-10-26T03:40:45+00:00 5062007

butonic · 2015-10-26T14:19:15Z

@davitol did you make sure to wipe the ldap user mapping?

davitol · 2015-10-26T14:55:15Z

@butonic Just after clicking on Cleaning buttons in expert tab in the LDAP wizard I have retested it, and the mail is sent.

After clicking in the cleaning buttons, the ldap_user_mapping table is empty, but using the sharing wizard and searching for the ldap user in order to share the file, the table is populated again. Am I missing some step or those are not the right ones? 👀

ghost · 2015-10-26T15:15:37Z

@davitol, if you test with the patch, are there any issues?

davitol · 2015-10-27T10:24:44Z

@cmonteroluque testing with the patch, I haven't found any issue due to this PR

Maybe another reviewer can try to reproduce the ldap neverlogin mail issue in stable8.1 and see if they can reproduce #17362 in stable8.1

@MorrisJobke @tkaufm @Xenopathic maybe?

blizzz · 2015-10-27T12:22:02Z

If I understand this comment correctly, this one should be closed. Please confirm @MorrisJobke

MorrisJobke · 2015-10-27T12:41:36Z

If I understand this comment correctly, this one should be closed. Please confirm @MorrisJobke

We can do this in two ways:

we merge this and then rebase [stable8.1] allow an attribute to return more than one value #19972
we close this and then only merge [stable8.1] allow an attribute to return more than one value #19972

I just left this open to maybe identify possible problems easier - up to you.

blizzz · 2015-10-27T13:00:40Z

@MorrisJobke then it is more straightforward if we just merge one and don't fiddle around anymore :) thanks for clarifying. Closing this in favor of #19972

blizzz · 2015-10-27T13:01:22Z

let's keep the branch until #19972 is merged

lock · 2019-08-06T19:01:43Z

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

blizzz added enhancement 3 - To Review app:user_ldap labels Sep 28, 2015

blizzz added this to the 8.1.4-current-maintenance milestone Sep 28, 2015

MorrisJobke added the resolves-blue-ticket label Sep 29, 2015

blizzz reviewed Sep 29, 2015
View reviewed changes

blizzz added 2 - Developing 3 - To Review and removed 3 - To Review 2 - Developing labels Sep 29, 2015

MorrisJobke mentioned this pull request Oct 22, 2015

sharing notification by email not working for never logged on users #17362

Closed

MorrisJobke added db:postgresql green-ticket labels Oct 22, 2015

blizzz added 4 commits October 22, 2015 12:01

stable8.1 related adjustments to #18469 backport

0a99049

fix update quota with known value

a05ead5

lowercase configured displayname attribute so isset works - all attri…

7ccd52d

…bute names coming from ldap are lowercased for easy comparison

MorrisJobke force-pushed the backport-18469-stable8.1 branch from e44df7d to 7ccd52d Compare October 22, 2015 10:01

MorrisJobke mentioned this pull request Oct 22, 2015

[stable8.1] allow an attribute to return more than one value #19972

Merged

blizzz mentioned this pull request Oct 27, 2015

[8.2] LDAP - Quota not delivered #20043

Closed

blizzz closed this Oct 27, 2015

MorrisJobke deleted the backport-18469-stable8.1 branch October 27, 2015 13:10

blizzz mentioned this pull request Oct 27, 2015

LDAP fix quota and case, fixes #20043 #20093

Merged

MorrisJobke added blue-ticket and removed resolves-blue-ticket labels Jan 7, 2016

butonic added blue-ticket and removed blue-ticket green-ticket labels Mar 24, 2016

lock Bot locked as resolved and limited conversation to collaborators Aug 6, 2019

Uh oh!

Conversation

blizzz commented Sep 28, 2015

Uh oh!

blizzz commented Sep 28, 2015

Uh oh!

MorrisJobke commented Sep 29, 2015

Uh oh!

blizzz commented Sep 29, 2015

Uh oh!

MorrisJobke commented Sep 29, 2015

Uh oh!

blizzz commented Sep 29, 2015

Uh oh!

MorrisJobke commented Sep 29, 2015

Uh oh!

blizzz Sep 29, 2015

Choose a reason for hiding this comment

Uh oh!

blizzz commented Oct 1, 2015

Uh oh!

blizzz commented Oct 1, 2015

Uh oh!

blizzz commented Oct 12, 2015

Uh oh!

MorrisJobke commented Oct 22, 2015

Uh oh!

MorrisJobke commented Oct 22, 2015

Uh oh!

MorrisJobke commented Oct 22, 2015

Uh oh!

MorrisJobke commented Oct 22, 2015

Uh oh!

davitol commented Oct 26, 2015

Uh oh!

butonic commented Oct 26, 2015

Uh oh!

davitol commented Oct 26, 2015

Uh oh!

ghost commented Oct 26, 2015

Uh oh!

davitol commented Oct 27, 2015

Uh oh!

blizzz commented Oct 27, 2015

Uh oh!

MorrisJobke commented Oct 27, 2015

Uh oh!

blizzz commented Oct 27, 2015

Uh oh!

blizzz commented Oct 27, 2015

Uh oh!

lock Bot commented Aug 6, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants