Merge pull request #30938 from nextcloud/fix/user_ldap-fix-check-ldap…

…-new-user Fix ldap:check-user method for newly created LDAP users
nextcloud · Feb 17, 2022 · f5d1365 · f5d1365
2 parents ac4978e + 44680b5
commit f5d1365
Show file tree

Hide file tree

Showing 2 changed files with 24 additions and 30 deletions.
diff --git a/apps/user_ldap/lib/Access.php b/apps/user_ldap/lib/Access.php
@@ -488,7 +488,7 @@ public function dn2groupname($fdn, $ldapName = null) {
 	/**
 	 * returns the internal Nextcloud name for the given LDAP DN of the user, false on DN outside of search DN or failure
 	 *
-	 * @param string $dn the dn of the user object
+	 * @param string $fdn the dn of the user object
 	 * @param string $ldapName optional, the display name of the object
 	 * @return string|false with with the name to use in Nextcloud
 	 * @throws \Exception
@@ -1770,7 +1770,7 @@ private function detectUuidAttribute(string $dn, bool $isUser = true, bool $forc
 	/**
 	 * @param string $dn
 	 * @param bool $isUser
-	 * @param null $ldapRecord
+	 * @param array|null $ldapRecord
 	 * @return false|string
 	 * @throws ServerNotAvailableException
 	 */

diff --git a/apps/user_ldap/lib/Command/CheckUser.php b/apps/user_ldap/lib/Command/CheckUser.php
@@ -4,6 +4,7 @@
  *
  * @author Arthur Schiwon <blizzz@arthur-schiwon.de>
  * @author Christoph Wurst <christoph@winzerhof-wurst.at>
+ * @author Côme Chilliet <come.chilliet@nextcloud.com>
  * @author Joas Schilling <coding@schilljs.com>
  * @author Morris Jobke <hey@morrisjobke.de>
  * @author Roeland Jago Douma <roeland@famdouma.nl>
@@ -48,12 +49,6 @@ class CheckUser extends Command {
 	/** @var UserMapping */
 	protected $mapping;
 
-	/**
-	 * @param User_Proxy $uBackend
-	 * @param Helper $helper
-	 * @param DeletedUsersIndex $dui
-	 * @param UserMapping $mapping
-	 */
 	public function __construct(User_Proxy $uBackend, Helper $helper, DeletedUsersIndex $dui, UserMapping $mapping) {
 		$this->backend = $uBackend;
 		$this->helper = $helper;
@@ -62,14 +57,14 @@ public function __construct(User_Proxy $uBackend, Helper $helper, DeletedUsersIn
 		parent::__construct();
 	}
 
-	protected function configure() {
+	protected function configure(): void {
 		$this
 			->setName('ldap:check-user')
 			->setDescription('checks whether a user exists on LDAP.')
 			->addArgument(
 					'ocName',
 					InputArgument::REQUIRED,
-					'the user name as used in Nextcloud'
+					'the user name as used in Nextcloud, or the LDAP DN'
 					 )
 			->addOption(
 					'force',
@@ -88,23 +83,31 @@ protected function configure() {
 
 	protected function execute(InputInterface $input, OutputInterface $output): int {
 		try {
+			$this->assertAllowed($input->getOption('force'));
 			$uid = $input->getArgument('ocName');
-			$this->isAllowed($input->getOption('force'));
-			$this->confirmUserIsMapped($uid);
+			if ($this->backend->getLDAPAccess($uid)->stringResemblesDN($uid)) {
+				$username = $this->backend->dn2UserName($uid);
+				if ($username !== false) {
+					$uid = $username;
+				}
+			}
+			$wasMapped = $this->userWasMapped($uid);
 			$exists = $this->backend->userExistsOnLDAP($uid, true);
 			if ($exists === true) {
 				$output->writeln('The user is still available on LDAP.');
 				if ($input->getOption('update')) {
 					$this->updateUser($uid, $output);
 				}
 				return 0;
+			} elseif ($wasMapped) {
+				$this->dui->markUser($uid);
+				$output->writeln('The user does not exists on LDAP anymore.');
+				$output->writeln('Clean up the user\'s remnants by: ./occ user:delete "'
+					. $uid . '"');
+				return 0;
+			} else {
+				throw new \Exception('The given user is not a recognized LDAP user.');
 			}
-
-			$this->dui->markUser($uid);
-			$output->writeln('The user does not exists on LDAP anymore.');
-			$output->writeln('Clean up the user\'s remnants by: ./occ user:delete "'
-				. $uid . '"');
-			return 0;
 		} catch (\Exception $e) {
 			$output->writeln('<error>' . $e->getMessage(). '</error>');
 			return 1;
@@ -114,24 +117,17 @@ protected function execute(InputInterface $input, OutputInterface $output): int
 	/**
 	 * checks whether a user is actually mapped
 	 * @param string $ocName the username as used in Nextcloud
-	 * @throws \Exception
-	 * @return true
 	 */
-	protected function confirmUserIsMapped($ocName) {
+	protected function userWasMapped(string $ocName): bool {
 		$dn = $this->mapping->getDNByName($ocName);
-		if ($dn === false) {
-			throw new \Exception('The given user is not a recognized LDAP user.');
-		}
-
-		return true;
+		return $dn !== false;
 	}
 
 	/**
 	 * checks whether the setup allows reliable checking of LDAP user existence
 	 * @throws \Exception
-	 * @return true
 	 */
-	protected function isAllowed($force) {
+	protected function assertAllowed(bool $force): void {
 		if ($this->helper->haveDisabledConfigurations() && !$force) {
 			throw new \Exception('Cannot check user existence, because '
 				. 'disabled LDAP configurations are present.');
@@ -140,8 +136,6 @@ protected function isAllowed($force) {
 		// we don't check ldapUserCleanupInterval from config.php because this
 		// action is triggered manually, while the setting only controls the
 		// background job.
-
-		return true;
 	}
 
 	private function updateUser(string $uid, OutputInterface $output): void {