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.

Sign up for GitHub

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

Jump to bottom

allow using any ldap property as login name when using external storage login credentials #26397

Merged
merged 1 commit into from
Jun 7, 2021
Merged

allow using any ldap property as login name when using external storage login credentials #26397

merged 1 commit into from
Jun 7, 2021

Conversation

icewind1991
Copy link
Member

@icewind1991 icewind1991 commented Mar 31, 2021 •
edited by MorrisJobke
Loading

New try for #24955

@icewind1991 icewind1991 added the 2. developing Work in progress label Mar 31, 2021
@icewind1991 icewind1991 added this to the Nextcloud 22 milestone Mar 31, 2021
@icewind1991 icewind1991 mentioned this pull request Mar 31, 2021
Merged
MorrisJobke
MorrisJobke reviewed Mar 31, 2021
View reviewed changes
Copy link
Member

@MorrisJobke MorrisJobke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Beside my nitpick it looks good and once #25326 is in then I'm fine with the merge 👍

@icewind1991 icewind1991 force-pushed the external-storage-login-ldap-public branch from 0ca0287 to 785a158 Compare March 31, 2021 15:51
@MorrisJobke
Copy link
Member

New try for #24955

* [x]  #25326 to make it not hard-depend on user_ldap

Please rebase :)

@icewind1991 icewind1991 force-pushed the external-storage-login-ldap-public branch from 785a158 to 53640ab Compare April 1, 2021 11:04
@icewind1991
Copy link
Member Author

rebased

blizzz
blizzz reviewed Apr 1, 2021
View reviewed changes
Copy link
Member

@blizzz blizzz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

one hint to decouple from user_ldap internals, otherwise looking good.

apps/files_external/lib/Lib/Auth/Password/LoginCredentials.php Outdated Show resolved Hide resolved
This was referenced May 20, 2021
Merged
Merged
@icewind1991 icewind1991 force-pushed the external-storage-login-ldap-public branch from 53640ab to 0c5fcee Compare May 28, 2021 11:10
@MorrisJobke MorrisJobke force-pushed the external-storage-login-ldap-public branch from 0c5fcee to 0bf3536 Compare May 28, 2021 11:57
@MorrisJobke
Copy link
Member

I just fixed the code style complaint.

MorrisJobke
MorrisJobke reviewed May 28, 2021
View reviewed changes
apps/files_external/lib/Lib/Auth/Password/LoginCredentials.php
$loginKey = $storage->getBackendOption("login_ldap_attr");
if ($loginKey) {
$backend = $user->getBackend();
if ($backend instanceof IUserBackend && $backend->getBackendName() === 'LDAP') {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just for reference: https://github.com/nextcloud/server/blob/master/apps/user_ldap/lib/User_LDAP.php#L575

MorrisJobke
MorrisJobke approved these changes May 28, 2021
View reviewed changes
Copy link
Member

@MorrisJobke MorrisJobke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code makes sense 👍

@MorrisJobke MorrisJobke added 3. to review Waiting for reviews and removed 2. developing Work in progress labels May 28, 2021
@blizzz blizzz mentioned this pull request Jun 2, 2021
Merged
57 tasks
@blizzz
Copy link
Member

blizzz commented Jun 2, 2021

CI is unhappy

@icewind1991 icewind1991 force-pushed the external-storage-login-ldap-public branch from 0bf3536 to 28c0417 Compare June 3, 2021 15:03
@icewind1991
allow using any ldap property as login name when using external stora…
592d6d5 
…ge login credentials

Signed-off-by: Robin Appelman <robin@icewind.nl>
@icewind1991 icewind1991 force-pushed the external-storage-login-ldap-public branch from 28c0417 to 592d6d5 Compare June 3, 2021 16:55
@icewind1991
Copy link
Member Author

ci happy

@blizzz blizzz merged commit ae4907b into master Jun 7, 2021
@blizzz blizzz deleted the external-storage-login-ldap-public branch June 7, 2021 11:13
@MorrisJobke MorrisJobke mentioned this pull request Jun 10, 2021
Merged
59 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MorrisJobke MorrisJobke MorrisJobke approved these changes

@blizzz blizzz blizzz approved these changes

Assignees
No one assigned
Labels
3. to review Waiting for reviews
Projects
None yet
Milestone
Nextcloud 22
Development

Successfully merging this pull request may close these issues.
3 participants
@icewind1991 @MorrisJobke @blizzz