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

Start porting Admin audit to new events #32019

Closed
wants to merge 1 commit into from
Closed

Start porting Admin audit to new events #32019

wants to merge 1 commit into from

Conversation

CarlSchwan
Copy link
Member

No description provided.

@CarlSchwan CarlSchwan self-assigned this Apr 19, 2022
@CarlSchwan CarlSchwan added this to the Nextcloud 25 milestone Apr 19, 2022
@CarlSchwan CarlSchwan mentioned this pull request Apr 25, 2022
Open
6 tasks
nickvergessen
nickvergessen requested changes Apr 25, 2022
View reviewed changes
Copy link
Member

@nickvergessen nickvergessen left a comment

Choose a reason for hiding this comment

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

Or am I missing something?

apps/user_ldap/lib/Jobs/Sync.php Outdated Show resolved Hide resolved
apps/user_ldap/lib/Jobs/Sync.php Outdated Show resolved Hide resolved
@CarlSchwan CarlSchwan added the 2. developing Work in progress label Apr 25, 2022
nickvergessen
nickvergessen approved these changes Apr 26, 2022
View reviewed changes
Copy link
Member

@nickvergessen nickvergessen left a comment

Choose a reason for hiding this comment

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

Half 🐘

come-nc
come-nc reviewed Apr 26, 2022
View reviewed changes
apps/user_ldap/ajax/clearMappings.php Outdated Show resolved Hide resolved
apps/admin_audit/lib/Actions/UserManagement.php Outdated Show resolved Hide resolved
apps/user_ldap/lib/Jobs/Sync.php Outdated Show resolved Hide resolved
apps/user_ldap/lib/Jobs/Sync.php Show resolved Hide resolved
apps/user_ldap/lib/Mapping/AbstractMapping.php Outdated Show resolved Hide resolved
lib/composer/composer/autoload_classmap.php Show resolved Hide resolved
lib/public/User/Events/BeforeUserIdUnassignedEvent.php Outdated Show resolved Hide resolved
lib/public/User/Events/BeforeUserIdUnassignedEvent.php Outdated Show resolved Hide resolved
blizzz
blizzz reviewed Apr 26, 2022
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.

Questions

apps/admin_audit/lib/Actions/GroupManagement.php Outdated Show resolved Hide resolved
apps/admin_audit/lib/Actions/UserManagement.php Outdated Show resolved Hide resolved
@CarlSchwan CarlSchwan force-pushed the cleanup/event/admin_audit branch 2 times, most recently from 291eb93 to e6e6a95 Compare May 5, 2022 19:57
@CarlSchwan CarlSchwan added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Jun 21, 2022
github-advanced-security[bot]
github-advanced-security bot found potential problems Jun 21, 2022
View reviewed changes
apps/user_ldap/lib/Jobs/Sync.php
$ldapConfig = new Configuration($cycleData['prefix']);
$cycleData['offset'] += (int)$ldapConfig->ldapPagingSize;
$cycleData['offset'] += $ldapConfig->ldapPagingSize;

Check notice

Code scanning / Psalm

PossiblyNullOperand

Right operand cannot be nullable, got mixed|null
apps/user_ldap/lib/Proxy.php
}
$userManager =
new Manager($ocConfig, $fs, $logger, $avatarM, new \OCP\Image(),
$coreUserManager, $coreNotificationManager, $shareManager);
$connector = new Connection($this->ldap, $configPrefix);
$access = new Access($connector, $this->ldap, $userManager, new Helper($ocConfig, \OC::$server->getDatabaseConnection()), $ocConfig, $coreUserManager, $logger);
$access = new Access($connector, $this->ldap, $userManager, new Helper($ocConfig, \OC::$server->getDatabaseConnection()), $ocConfig, $coreUserManager, $logger, $dispatcher);

Check notice

Code scanning / Psalm

PossiblyUndefinedVariable

Possibly undefined variable $dispatcher, first seen on line 87
blizzz
blizzz requested changes Jun 24, 2022
View reviewed changes
apps/user_ldap/lib/Proxy.php
@@ -83,12 +84,13 @@ private function addAccess($configPrefix) {
$coreNotificationManager = \OC::$server->getNotificationManager();
$shareManager = \OC::$server->get(IManager::class);
$logger = \OC::$server->get(LoggerInterface::class);
$dispatcher = \OC::$server->get(IEventDispatcher::class);
Copy link
Member

Choose a reason for hiding this comment

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

make $dispatcher static, otherwise it will be null on the next runs.

Copy link
Contributor

Choose a reason for hiding this comment

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

Why? Is that specific to IEventDispatcher?

Copy link
Member

Choose a reason for hiding this comment

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

the local var, look couple lines above

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok understood, nice catch.

This was referenced Aug 24, 2022
Merged
Merged
This was referenced Sep 6, 2022
Merged
Merged
@skjnldsv skjnldsv mentioned this pull request Sep 15, 2022
Merged
This was referenced Sep 20, 2022
Closed
Merged
@blizzz blizzz modified the milestones: Nextcloud 25, Nextcloud 26 Sep 22, 2022
@come-nc come-nc assigned come-nc and unassigned CarlSchwan Jan 24, 2023
@come-nc come-nc added 2. developing Work in progress and removed 3. to review Waiting for reviews labels Jan 24, 2023
@blizzz blizzz mentioned this pull request Feb 1, 2023
Merged
@skjnldsv skjnldsv mentioned this pull request Feb 23, 2023
Merged
@blizzz blizzz mentioned this pull request Mar 7, 2023
Merged
@blizzz blizzz modified the milestones: Nextcloud 26, Nextcloud 27 Mar 9, 2023
@come-nc come-nc mentioned this pull request Mar 13, 2023
Closed
5 tasks
tcitworld pushed a commit that referenced this pull request Mar 13, 2023
@CarlSchwan @tcitworld
Introduce user id assigned typed events for LDAP usage
eaa6c70 
Based on work from #32019

Signed-off-by: Thomas Citharel <[email protected]>
@tcitworld
Copy link
Member

I've pulled most of this in #37193, apart from the heavier refactoring from AbstractMapping that isn't related.

tcitworld pushed a commit that referenced this pull request Mar 13, 2023
@CarlSchwan @tcitworld
Introduce user id assigned typed events for LDAP usage
fc47793 
Based on work from #32019

Signed-off-by: Thomas Citharel <[email protected]>
@tcitworld
Copy link
Member

Closing here in favor of #37193

@tcitworld tcitworld closed this Mar 13, 2023
@tcitworld tcitworld mentioned this pull request Mar 13, 2023
Open
17 tasks
tcitworld pushed a commit that referenced this pull request May 2, 2023
@CarlSchwan @tcitworld
Introduce user id assigned typed events for LDAP usage
a047c14 
Based on work from #32019

Signed-off-by: Thomas Citharel <[email protected]>
tcitworld pushed a commit that referenced this pull request May 2, 2023
@CarlSchwan @tcitworld
Introduce user id assigned typed events for LDAP usage
ac0c201 
Based on work from #32019

Signed-off-by: Thomas Citharel <[email protected]>
@skjnldsv skjnldsv deleted the cleanup/event/admin_audit branch March 14, 2024 07:46
come-nc pushed a commit that referenced this pull request Sep 10, 2024
@CarlSchwan @come-nc
Introduce user id assigned typed events for LDAP usage
038d067 
Based on work from #32019

Signed-off-by: Thomas Citharel <[email protected]>
come-nc pushed a commit that referenced this pull request Sep 10, 2024
@CarlSchwan @come-nc
feat(user_ldap): Introduce user id assigned typed events for LDAP usage
986a3d4 
Based on work from #32019

Signed-off-by: Thomas Citharel <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@blizzz blizzz blizzz requested changes

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

@nickvergessen nickvergessen nickvergessen approved these changes

@artonge artonge Awaiting requested review from artonge

Assignees

@come-nc come-nc

Labels
2. developing Work in progress
Projects
None yet
Milestone
Nextcloud 27
Development

Successfully merging this pull request may close these issues.
6 participants
@CarlSchwan @mrAceT @tcitworld @nickvergessen @blizzz @come-nc