Skip to content

fix: propagate disabled state to lookup server - #215

Merged
cristianscheid merged 2 commits into
masterfrom
fix/183/disabled-user-propagation
Jul 6, 2026
Merged

fix: propagate disabled state to lookup server#215
cristianscheid merged 2 commits into
masterfrom
fix/183/disabled-user-propagation

Conversation

@cristianscheid

Copy link
Copy Markdown
Member

Summary

Currently, disabled users are kept on the lookup server as if they were enabled.

This PR introduces the following mechanisms to address this:

  • lib/Listeners/UserChanged listens for enable/disable events
    • when a user is disabled, removes them from the lookup server
    • when a user is re-enabled, adds them back to the lookup server
  • lib/Slave::batchUpdate() now checks if a user is enabled before registering them on the lookup server
    • this method syncs all known instance users with the lookup server
    • used by occ globalsiteselector:users:update and the BackgroundJobs/UpdateLookupServer cron job
  • lib/Controller/SlaveController::autoLogin() was updated to handle disabled users that were already present on the lookup server
    • catches DisabledUserException on login and removes the user from the lookup server in that case

Checklist

AI (if applicable)

  • The content of this PR was partly or fully generated using AI

@cristianscheid cristianscheid self-assigned this Jun 30, 2026
@cristianscheid cristianscheid changed the title fix: propagate disabled state to lookup server so users get removed fix: propagate disabled state to lookup server Jun 30, 2026
@cristianscheid
cristianscheid force-pushed the fix/183/disabled-user-propagation branch 2 times, most recently from 2707887 to 8f0d524 Compare June 30, 2026 11:44
Signed-off-by: Cristian Scheid <cristianscheid@gmail.com>
@cristianscheid
cristianscheid force-pushed the fix/183/disabled-user-propagation branch from 8f0d524 to 84ba97a Compare June 30, 2026 11:56
Comment thread tests/stubs/oc_user_disableduserexception.php
Signed-off-by: Cristian Scheid <cristianscheid@gmail.com>
@cristianscheid

cristianscheid commented Jul 2, 2026

Copy link
Copy Markdown
Member Author

As described here, added a new commit to include some changes that I believe would be a good addition to this PR. Tested it locally and looks good.

@cristianscheid
cristianscheid requested a review from artonge July 2, 2026 17:17
@rikled

rikled commented Jul 2, 2026

Copy link
Copy Markdown

Maybe it would make sense to include the tests from here as well?

@cristianscheid

cristianscheid commented Jul 3, 2026

Copy link
Copy Markdown
Member Author

Maybe it would make sense to include the tests from here as well?

hey @rikled, I don't think they'd quite apply here, since this PR takes a different approach:

// lib/Listeners/UserChanged.php

// this PR:
if ($event->getValue() === false) {
	// user was disabled, remove from lookup server
	$this->slave->preDeleteUser($params);
	$this->slave->deleteUser($params);
} else {
	// user was enabled, add to lookup server
	$this->slave->createUser($params);
}

// PR #213:
// updateUser() routes enabled→add, disabled→remove
$this->slave->updateUser($event->getUser());

also since the tests from #213 were specifically checking that enabled/disabled users get pushed/deleted via updateUser(), I don't think should be included here, as that method falls outside the scope of this PR

that said, thanks for taking the time to write the tests, and feel free to open a separate PR with some tests if you feel like it

@cristianscheid
cristianscheid merged commit e09024c into master Jul 6, 2026
38 checks passed
@cristianscheid
cristianscheid deleted the fix/183/disabled-user-propagation branch July 6, 2026 18:30
@ArtificialOwl

Copy link
Copy Markdown
Member

@cristianscheid should we backport this to stable2.7 ?

@cristianscheid

Copy link
Copy Markdown
Member Author

/backport to stable2.7

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Propagate disable state to the lookup-server

4 participants