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

Ldapi unix socket support #24574

Merged
merged 2 commits into from
Dec 8, 2022
Merged

Ldapi unix socket support #24574

merged 2 commits into from
Dec 8, 2022

Conversation

zenlord
Copy link
Contributor

@zenlord zenlord commented Dec 6, 2020

This branch adds ldapi:/// connection support to the user_ldap app (both backend code and frontend code)

@zenlord
Copy link
Contributor Author

zenlord commented Dec 6, 2020 •
edited
Loading

Fixes #24497

@zenlord
Copy link
Contributor Author

zenlord commented Dec 9, 2020

@blizzz Sorry for my noob question, but is there anything I should or could do to proceed this PR? The codesniffer check fails, but not on a line that I have manipulated. If from now on, it's all out of my hands, that's fine with me ;)

@zenlord
Copy link
Contributor Author

zenlord commented Dec 9, 2020

Fixes #24497

@zenlord
Copy link
Contributor Author

zenlord commented Jan 10, 2021

@blizzz ping?

@PVince81 PVince81 requested a review from blizzz March 17, 2021 09:59
@zenlord
Copy link
Contributor Author

zenlord commented Apr 11, 2021

@blizzz just wanted to confirm that these patches still work on version 21.0.1.

@zenlord
Copy link
Contributor Author

zenlord commented Nov 20, 2021

@blizzz Patches still work with newest version (22.2.3)

@zenlord zenlord mentioned this pull request Nov 20, 2021
Closed
@zenlord
Copy link
Contributor Author

zenlord commented Mar 13, 2022

@blizzz Patches still apply cleanly with version 22.2.5. System is running smoothly since the PR was created.

@jospoortvliet Last ping from my end. First time collaborator to an open source project, and very disappointing to see absolutely noone cares.

@szaimen szaimen added this to the Nextcloud 24 milestone Mar 13, 2022
@szaimen szaimen requested a review from come-nc March 13, 2022 15:44
come-nc
come-nc reviewed Mar 14, 2022
View reviewed changes
Copy link
Contributor

@come-nc come-nc left a comment

Choose a reason for hiding this comment

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

See comments.
Please also run composer run cs:fix for code style.

Does this only work with openldap?

apps/user_ldap/lib/Wizard.php Show resolved Hide resolved
apps/user_ldap/lib/Connection.php Outdated Show resolved Hide resolved
@skjnldsv skjnldsv mentioned this pull request Mar 24, 2022
Merged
@blizzz blizzz mentioned this pull request Mar 31, 2022
Merged
@blizzz blizzz mentioned this pull request Apr 7, 2022
Merged
@szaimen
Copy link
Contributor

szaimen commented Apr 7, 2022

@zenlord any update here? :)

@blizzz blizzz mentioned this pull request Apr 13, 2022
Merged
@blizzz blizzz modified the milestones: Nextcloud 24, Nextcloud 25 Apr 21, 2022
This was referenced Aug 12, 2022
Merged
Merged
This was referenced Aug 24, 2022
Merged
Merged
@zenlord
Copy link
Contributor Author

zenlord commented Sep 2, 2022

@szaimen @blizzz @come-nc Finally got around resolving the comments made by @come-nc . Instead of doing a rebase, I have simply created a new clone, applied the original commits as a patch, made the code changes and then force-pushed the result to my fork. I hope this is acceptable, because going through all the merges required from rebasing against Nextcloud 20-something got dull very quickly :/

This was referenced Sep 6, 2022
Merged
Merged
@zenlord
Add ldap-unix-socket support - squashed commits to files Connection.p…
c323022 
…hp, Configuration.php, Wizard.php, LDAP.php and view.js

Signed-off-by: Vincent Van Houtte <vvh@aplusv.be>
@zenlord
Copy link
Contributor Author

zenlord commented Sep 9, 2022

@come-nc Looks like this one is better: single commit and DCO is not complaining.

@zenlord zenlord requested review from come-nc and removed request for blizzz September 9, 2022 14:59
@come-nc come-nc modified the milestones: Nextcloud 25, Nextcloud 26 Sep 12, 2022
@come-nc come-nc added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Sep 12, 2022
@come-nc
Copy link
Contributor

come-nc commented Sep 12, 2022

25 is feature freezed so this will go into 26, waiting for 25 to branch out before merging into master.

Thank you for this contribution @zenlord !

@come-nc come-nc requested review from a team, PVince81 and blizzz and removed request for a team September 12, 2022 08:21
@come-nc come-nc added 3. to review Waiting for reviews and removed 4. to release Ready to be released and/or waiting for tests to finish labels Sep 12, 2022
@come-nc
Copy link
Contributor

come-nc commented Sep 12, 2022 •
edited
Loading

(actually needs 1 more review so I put back "to review" label)

@come-nc come-nc self-assigned this Sep 12, 2022
CarlSchwan
CarlSchwan reviewed Oct 20, 2022
View reviewed changes
Copy link
Member

@CarlSchwan CarlSchwan left a comment

Choose a reason for hiding this comment

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

Need a rebase (last one hopefully)

@come-nc
Merge branch 'master' into ldapi-unix-socket-support
9119678 
Signed-off-by: Côme Chilliet <91878298+come-nc@users.noreply.github.com>
@szaimen
Copy link
Contributor

szaimen commented Nov 23, 2022

/rebase

@come-nc come-nc merged commit f7cd704 into nextcloud:master Dec 8, 2022
@welcome
Copy link

welcome bot commented Dec 8, 2022

Thanks for your first pull request and welcome to the community! Feel free to keep them coming! If you are looking for issues to tackle then have a look at this selection: https://github.com/nextcloud/server/issues?q=is%3Aopen+is%3Aissue+label%3A%22good+first+issue%22

@come-nc come-nc added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Dec 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@CarlSchwan CarlSchwan CarlSchwan approved these changes

@come-nc come-nc come-nc approved these changes

@PVince81 PVince81 Awaiting requested review from PVince81

@blizzz blizzz Awaiting requested review from blizzz

Assignees

@come-nc come-nc

Labels
4. to release Ready to be released and/or waiting for tests to finish enhancement feature: ldap
Projects
None yet
Milestone
Nextcloud 26
Development

Successfully merging this pull request may close these issues.
5 participants
@zenlord @szaimen @come-nc @CarlSchwan @blizzz