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

Add FirstLoginListener to accept shares upon first ldap user login #30512

Closed
wants to merge 5 commits into from

Conversation

come-nc
Copy link
Contributor

@come-nc come-nc commented Jan 6, 2022

Trigger OCP\Group\Events\UserAddedEvent on first ldap user login for all his groups, to avoid having to wait on the background job before shares get accepted.

Signed-off-by: Côme Chilliet [email protected]

@come-nc come-nc force-pushed the fix/user_ldap-update-groups-on-first-mapping branch 2 times, most recently from 7f7d082 to 2cc0113 Compare January 6, 2022 16:09
apps/user_ldap/lib/AppInfo/Application.php Outdated Show resolved Hide resolved
apps/user_ldap/lib/FirstLoginListener.php Outdated Show resolved Hide resolved
@come-nc come-nc force-pushed the fix/user_ldap-update-groups-on-first-mapping branch from 8505af0 to 4b48b66 Compare February 14, 2022 09:56
@come-nc
Copy link
Contributor Author

come-nc commented Feb 14, 2022

/rebase

@come-nc come-nc added 2. developing Work in progress bug labels Feb 14, 2022
@come-nc come-nc self-assigned this Feb 14, 2022
@come-nc come-nc added this to the Nextcloud 24 milestone Feb 14, 2022
@nextcloud-command nextcloud-command force-pushed the fix/user_ldap-update-groups-on-first-mapping branch from 4b48b66 to ea2afb2 Compare February 14, 2022 10:15
@come-nc come-nc linked an issue Feb 14, 2022 that may be closed by this pull request
@come-nc
Copy link
Contributor Author

come-nc commented Feb 14, 2022

This does not work, the listener never gets called on neither of the 2 events according to the log… I do not understand the problem.

Also, I’m wondering if this solution is enough, because it will only work for new users, not when adding an existing user to a group.
But checking if Nextcloud is aware of group membership on any login, or each time the group membership is queried is way too expensive performance wise I suspect.

@skjnldsv skjnldsv mentioned this pull request Mar 24, 2022
@blizzz blizzz mentioned this pull request Mar 31, 2022
This was referenced Apr 7, 2022
@blizzz blizzz modified the milestones: Nextcloud 24, Nextcloud 25 Apr 21, 2022
This was referenced Aug 12, 2022
This was referenced Aug 24, 2022
This was referenced Sep 6, 2022
@skjnldsv skjnldsv mentioned this pull request Sep 15, 2022
@come-nc come-nc modified the milestones: Nextcloud 25, Nextcloud 26 Sep 15, 2022
@come-nc
Copy link
Contributor Author

come-nc commented Oct 3, 2022

[notes for myself]

The idea is now to change the format of data stored by the update group background job so that we can easily search in it if a user/group association was already detected, and at login we browse known groups of the user, and if association is missing we announce it with a UserAddedEvent.

@blizzz blizzz mentioned this pull request Feb 1, 2023
@come-nc come-nc modified the milestones: Nextcloud 26, Nextcloud 27 Feb 2, 2023
@skjnldsv skjnldsv mentioned this pull request May 3, 2023
@come-nc come-nc modified the milestones: Nextcloud 27, Nextcloud 28 May 4, 2023
@skjnldsv skjnldsv mentioned this pull request Nov 1, 2023
@blizzz blizzz mentioned this pull request Nov 6, 2023
@come-nc
Copy link
Contributor Author

come-nc commented Nov 7, 2023

Replaced by #40367

@come-nc come-nc closed this Nov 7, 2023
@skjnldsv skjnldsv deleted the fix/user_ldap-update-groups-on-first-mapping branch March 14, 2024 07:44
@skjnldsv skjnldsv removed this from the Nextcloud 28 milestone Aug 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2. developing Work in progress bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Sharing still broken with ldap groups
3 participants