Merge pull request #32378 from nextcloud/fix/psalm-userbackend

Fix psalm issues related to the user backend

build/psalm-baseline.xml

@@ -4157,28 +4157,11 @@
  <FalsableReturnStatement occurrences="1">
  <code>false</code>
  </FalsableReturnStatement>
- <ImplicitToStringCast occurrences="1">
- <code>$query-&gt;func()-&gt;lower('displayname')</code>
- </ImplicitToStringCast>
  </file>
  <file src="lib/private/User/Manager.php">
- <ImplementedReturnTypeMismatch occurrences="1">
- <code>array|int</code>
- </ImplementedReturnTypeMismatch>
- <InvalidArgument occurrences="1">
- <code>$callback</code>
- </InvalidArgument>
- <InvalidNullableReturnType occurrences="1">
- <code>bool|IUser</code>
- </InvalidNullableReturnType>
- <NullableReturnStatement occurrences="2">
- <code>$this-&gt;createUserFromBackend($uid, $password, $backend)</code>
- <code>$this-&gt;createUserFromBackend($uid, $password, $backend)</code>
- </NullableReturnStatement>
- <UndefinedInterfaceMethod occurrences="5">
- <code>checkPassword</code>
- <code>checkPassword</code>
- <code>countUsers</code>
+ <ImplementedReturnTypeMismatch occurrences="1"/>
+ <InvalidArgument occurrences="1"/>
+ <UndefinedInterfaceMethod occurrences="2">
  <code>createUser</code>
  <code>getUsersForUserValueCaseInsensitive</code>
  </UndefinedInterfaceMethod>
@@ -4190,23 +4173,13 @@
  <InvalidArgument occurrences="1">
  <code>IUser::class . '::firstLogin'</code>
  </InvalidArgument>
- <InvalidScalarArgument occurrences="2">
- <code>$this-&gt;timeFactory-&gt;getTime()</code>
- <code>$this-&gt;timeFactory-&gt;getTime()</code>
- </InvalidScalarArgument>
  <NoInterfaceProperties occurrences="2">
  <code>$request-&gt;server</code>
  <code>$request-&gt;server</code>
  </NoInterfaceProperties>
- <NullableReturnStatement occurrences="1">
- <code>null</code>
- </NullableReturnStatement>
  <TooManyArguments occurrences="1">
  <code>dispatch</code>
  </TooManyArguments>
- <UndefinedMethod occurrences="1">
- <code>getByEmail</code>
- </UndefinedMethod>
  </file>
  <file src="lib/private/User/User.php">
  <InvalidArgument occurrences="5">
@@ -4216,36 +4189,13 @@
  <code>IUser::class . '::preDelete'</code>
  <code>IUser::class . '::preSetPassword'</code>
  </InvalidArgument>
- <InvalidNullableReturnType occurrences="1">
- <code>getBackend</code>
- </InvalidNullableReturnType>
- <InvalidReturnStatement occurrences="1">
- <code>$image</code>
- </InvalidReturnStatement>
- <InvalidReturnType occurrences="1">
- <code>IImage|null</code>
- </InvalidReturnType>
- <InvalidScalarArgument occurrences="2">
- <code>$quota</code>
- <code>$this-&gt;lastLogin</code>
- </InvalidScalarArgument>
- <NullableReturnStatement occurrences="1">
- <code>$this-&gt;backend</code>
- </NullableReturnStatement>
  <TooManyArguments occurrences="5">
  <code>dispatch</code>
  <code>dispatch</code>
  <code>dispatch</code>
  <code>dispatch</code>
  <code>dispatch</code>
  </TooManyArguments>
- <UndefinedInterfaceMethod occurrences="5">
- <code>canChangeAvatar</code>
- <code>deleteUserAvatar</code>
- <code>getHome</code>
- <code>setDisplayName</code>
- <code>setPassword</code>
- </UndefinedInterfaceMethod>
  </file>
  <file src="lib/private/legacy/OC_API.php">
  <InvalidNullableReturnType occurrences="1">

lib/private/DB/QueryBuilder/QueryBuilder.php

@@ -1096,7 +1096,7 @@ public function orHaving(...$having) {
  * Specifies an ordering for the query results.
  * Replaces any previously specified orderings, if any.
  *
- * @param string $sort The ordering expression.
+ * @param string|IQueryFunction|ILiteral|IParameter $sort The ordering expression.
  * @param string $order The ordering direction.
  *
  * @return $this This QueryBuilder instance.

lib/private/User/Database.php

@@ -275,7 +275,7 @@ public function getDisplayNames($search = '', $limit = null, $offset = null) {
  ->setMaxResults($limit)
  ->setFirstResult($offset);
 
- $result = $query->execute();
+ $result = $query->executeQuery();
  $displayNames = [];
  while ($row = $result->fetch()) {
  $displayNames[(string)$row['uid']] = (string)$row['displayname'];

lib/private/User/LazyUser.php

@@ -25,6 +25,7 @@
 
 use OCP\IUser;
 use OCP\IUserManager;
+use OCP\UserInterface;
 
 class LazyUser implements IUser {
  private ?IUser $user = null;
@@ -83,7 +84,7 @@ public function getBackendClassName() {
  return $this->getUser()->getBackendClassName();
  }
 
- public function getBackend() {
+ public function getBackend(): ?UserInterface {
  return $this->getUser()->getBackend();
  }
 

lib/private/User/Manager.php

@@ -48,6 +48,8 @@
 use OCP\Support\Subscription\IRegistry;
 use OCP\User\Backend\IGetRealUIDBackend;
 use OCP\User\Backend\ISearchKnownUsersBackend;
+use OCP\User\Backend\ICheckPasswordBackend;
+use OCP\User\Backend\ICountUsersBackend;
 use OCP\User\Events\BeforeUserCreatedEvent;
 use OCP\User\Events\UserCreatedEvent;
 use OCP\UserInterface;
@@ -223,7 +225,7 @@ public function userExists($uid) {
  *
  * @param string $loginName
  * @param string $password
- * @return mixed the User object on success, false otherwise
+ * @return IUser|false the User object on success, false otherwise
  */
  public function checkPassword($loginName, $password) {
  $result = $this->checkPasswordNoLogging($loginName, $password);
@@ -254,7 +256,8 @@ public function checkPasswordNoLogging($loginName, $password) {
  $backends = $this->backends;
  }
  foreach ($backends as $backend) {
- if ($backend->implementsActions(Backend::CHECK_PASSWORD)) {
+ if ($backend instanceof ICheckPasswordBackend || $backend->implementsActions(Backend::CHECK_PASSWORD)) {
+ /** @var ICheckPasswordBackend $backend */
  $uid = $backend->checkPassword($loginName, $password);
  if ($uid !== false) {
  return $this->getUserObject($uid, $backend);
@@ -268,7 +271,8 @@ public function checkPasswordNoLogging($loginName, $password) {
  $password = urldecode($password);
 
  foreach ($backends as $backend) {
- if ($backend->implementsActions(Backend::CHECK_PASSWORD)) {
+ if ($backend instanceof ICheckPasswordBackend || $backend->implementsActions(Backend::CHECK_PASSWORD)) {
+ /** @var ICheckPasswordBackend|UserInterface $backend */
  $uid = $backend->checkPassword($loginName, $password);
  if ($uid !== false) {
  return $this->getUserObject($uid, $backend);
@@ -376,7 +380,7 @@ public function searchKnownUsersByDisplayName(string $searcher, string $pattern,
  * @param string $uid
  * @param string $password
  * @throws \InvalidArgumentException
- * @return bool|IUser the created user or false
+ * @return false|IUser the created user or false
  */
  public function createUser($uid, $password) {
  // DI injection is not used here as IRegistry needs the user manager itself for user count and thus it would create a cyclic dependency
@@ -415,7 +419,7 @@ public function createUser($uid, $password) {
  * @param string $uid
  * @param string $password
  * @param UserInterface $backend
- * @return IUser|null
+ * @return IUser|false
  * @throws \InvalidArgumentException
  */
  public function createUserFromBackend($uid, $password, UserInterface $backend) {
@@ -469,25 +473,23 @@ public function createUserFromBackend($uid, $password, UserInterface $backend) {
  /** @deprecated 21.0.0 use UserCreatedEvent event with the IEventDispatcher instead */
  $this->emit('\OC\User', 'postCreateUser', [$user, $password]);
  $this->eventDispatcher->dispatchTyped(new UserCreatedEvent($user, $password));
+ return $user;
  }
- return $user;
+ return false;
  }
 
  /**
  * returns how many users per backend exist (if supported by backend)
  *
  * @param boolean $hasLoggedIn when true only users that have a lastLogin
  * entry in the preferences table will be affected
- * @return array|int an array of backend class as key and count number as value
- * if $hasLoggedIn is true only an int is returned
+ * @return array<string, int> an array of backend class as key and count number as value
  */
- public function countUsers($hasLoggedIn = false) {
- if ($hasLoggedIn) {
- return $this->countSeenUsers();
- }
+ public function countUsers() {
  $userCountStatistics = [];
  foreach ($this->backends as $backend) {
- if ($backend->implementsActions(Backend::COUNT_USERS)) {
+ if ($backend instanceof ICountUsersBackend || $backend->implementsActions(Backend::COUNT_USERS)) {
+ /** @var ICountUsersBackend|IUserBackend $backend */
  $backendUsers = $backend->countUsers();
  if ($backendUsers !== false) {
  if ($backend instanceof IUserBackend) {
@@ -528,7 +530,7 @@ public function countUsersOfGroups(array $groups) {
  * The callback is executed for each user on each backend.
  * If the callback returns false no further users will be retrieved.
  *
- * @param \Closure $callback
+ * @psalm-param \Closure(\OCP\IUser):?bool $callback
  * @param string $search
  * @param boolean $onlySeen when true only users that have a lastLogin entry
  * in the preferences table will be affected

lib/private/User/Session.php

@@ -90,7 +90,7 @@
  */
 class Session implements IUserSession, Emitter {
 
- /** @var Manager|PublicEmitter $manager */
+ /** @var Manager $manager */
  private $manager;
 
  /** @var ISession $session */
@@ -288,9 +288,9 @@ public function setLoginName($loginName) {
  }
 
  /**
- * get the login name of the current user
+ * Get the login name of the current user
  *
- * @return string
+ * @return ?string
  */
  public function getLoginName() {
  if ($this->activeUser) {
@@ -870,7 +870,7 @@ public function loginWithCookie($uid, $currentToken, $oldSessionId) {
  // replace successfully used token with a new one
  $this->config->deleteUserValue($uid, 'login_token', $currentToken);
  $newToken = $this->random->generate(32);
- $this->config->setUserValue($uid, 'login_token', $newToken, $this->timeFactory->getTime());
+ $this->config->setUserValue($uid, 'login_token', $newToken, (string)$this->timeFactory->getTime());
 
  try {
  $sessionId = $this->session->getId();
@@ -905,7 +905,7 @@ public function loginWithCookie($uid, $currentToken, $oldSessionId) {
  */
  public function createRememberMeToken(IUser $user) {
  $token = $this->random->generate(32);
- $this->config->setUserValue($user->getUID(), 'login_token', $token, $this->timeFactory->getTime());
+ $this->config->setUserValue($user->getUID(), 'login_token', $token, (string)$this->timeFactory->getTime());
  $this->setMagicInCookie($user->getUID(), $token);
  }
 

lib/private/User/User.php

@@ -52,6 +52,10 @@
 use OCP\User\Events\BeforeUserDeletedEvent;
 use OCP\User\Events\UserDeletedEvent;
 use OCP\User\GetQuotaEvent;
+use OCP\User\Backend\ISetDisplayNameBackend;
+use OCP\User\Backend\ISetPasswordBackend;
+use OCP\User\Backend\IProvideAvatarBackend;
+use OCP\User\Backend\IGetHomeBackend;
 use OCP\UserInterface;
 use Symfony\Component\EventDispatcher\EventDispatcherInterface;
 use Symfony\Component\EventDispatcher\GenericEvent;
@@ -155,7 +159,9 @@ public function setDisplayName($displayName) {
  $displayName = trim($displayName);
  $oldDisplayName = $this->getDisplayName();
  if ($this->backend->implementsActions(Backend::SET_DISPLAYNAME) && !empty($displayName) && $displayName !== $oldDisplayName) {
- $result = $this->backend->setDisplayName($this->uid, $displayName);
+ /** @var ISetDisplayNameBackend $backend */
+ $backend = $this->backend;
+ $result = $backend->setDisplayName($this->uid, $displayName);
  if ($result) {
  $this->displayName = $displayName;
  $this->triggerChange('displayName', $displayName, $oldDisplayName);
@@ -241,7 +247,7 @@ public function updateLastLoginTimestamp() {
  $firstTimeLogin = ($this->getLastLogin() === 0);
  $this->lastLogin = time();
  $this->config->setUserValue(
- $this->uid, 'login', 'lastLogin', $this->lastLogin);
+ $this->uid, 'login', 'lastLogin', (string)$this->lastLogin);
 
  return $firstTimeLogin;
  }
@@ -280,7 +286,7 @@ public function delete() {
  \OC::$server->getCommentsManager()->deleteReferencesOfActor('users', $this->uid);
  \OC::$server->getCommentsManager()->deleteReadMarksFromUser($this);
 
- /** @var IAvatarManager $avatarManager */
+ /** @var AvatarManager $avatarManager */
  $avatarManager = \OC::$server->query(AvatarManager::class);
  $avatarManager->deleteUserAvatar($this->uid);
 
@@ -319,7 +325,9 @@ public function setPassword($password, $recoveryPassword = null) {
  $this->emitter->emit('\OC\User', 'preSetPassword', [$this, $password, $recoveryPassword]);
  }
  if ($this->backend->implementsActions(Backend::SET_PASSWORD)) {
- $result = $this->backend->setPassword($this->uid, $password);
+ /** @var ISetPasswordBackend $backend */
+ $backend = $this->backend;
+ $result = $backend->setPassword($this->uid, $password);
 
  if ($result !== false) {
  $this->legacyDispatcher->dispatch(IUser::class . '::postSetPassword', new GenericEvent($this, [
@@ -344,7 +352,8 @@ public function setPassword($password, $recoveryPassword = null) {
  */
  public function getHome() {
  if (!$this->home) {
- if ($this->backend->implementsActions(Backend::GET_HOME) and $home = $this->backend->getHome($this->uid)) {
+ /** @psalm-suppress UndefinedInterfaceMethod Once we get rid of the legacy implementsActions, psalm won't complain anymore */
+ if (($this->backend instanceof IGetHomeBackend || $this->backend->implementsActions(Backend::GET_HOME)) && $home = $this->backend->getHome($this->uid)) {
  $this->home = $home;
  } elseif ($this->config) {
  $this->home = $this->config->getSystemValue('datadirectory', \OC::$SERVERROOT . '/data') . '/' . $this->uid;
@@ -367,18 +376,20 @@ public function getBackendClassName() {
  return get_class($this->backend);
  }
 
- public function getBackend() {
+ public function getBackend(): ?UserInterface {
  return $this->backend;
  }
 
  /**
- * check if the backend allows the user to change his avatar on Personal page
+ * Check if the backend allows the user to change his avatar on Personal page
  *
  * @return bool
  */
  public function canChangeAvatar() {
- if ($this->backend->implementsActions(Backend::PROVIDE_AVATAR)) {
- return $this->backend->canChangeAvatar($this->uid);
+ if ($this->backend instanceof IProvideAvatarBackend || $this->backend->implementsActions(Backend::PROVIDE_AVATAR)) {
+ /** @var IProvideAvatarBackend $backend */
+ $backend = $this->backend;
+ return $backend->canChangeAvatar($this->uid);
  }
  return true;
  }
@@ -501,7 +512,7 @@ public function setQuota($quota) {
  $oldQuota = $this->config->getUserValue($this->uid, 'files', 'quota', '');
  if ($quota !== 'none' and $quota !== 'default') {
  $quota = OC_Helper::computerFileSize($quota);
- $quota = OC_Helper::humanFileSize($quota);
+ $quota = OC_Helper::humanFileSize((int)$quota);
  }
  if ($quota !== $oldQuota) {
  $this->config->setUserValue($this->uid, 'files', 'quota', $quota);

lib/public/DB/QueryBuilder/IQueryBuilder.php

@@ -820,7 +820,7 @@ public function orHaving(...$having);
  * Specifies an ordering for the query results.
  * Replaces any previously specified orderings, if any.
  *
- * @param string $sort The ordering expression.
+ * @param string|IQueryFunction $sort The ordering expression.
  * @param string $order The ordering direction.
  *
  * @return $this This QueryBuilder instance.

lib/public/IAvatar.php

@@ -38,7 +38,7 @@ interface IAvatar {
  /**
  * get the users avatar
  * @param int $size size in px of the avatar, avatars are square, defaults to 64, -1 can be used to not scale the image
- * @return boolean|\OCP\IImage containing the avatar or false if there's no image
+ * @return false|\OCP\IImage containing the avatar or false if there's no image
  * @since 6.0.0 - size of -1 was added in 9.0.0
  */
  public function get($size = 64);

lib/public/IUser.php

@@ -113,10 +113,9 @@ public function getBackendClassName();
  /**
  * Get the backend for the current user object
  *
- * @return UserInterface
  * @since 15.0.0
  */
- public function getBackend();
+ public function getBackend(): ?UserInterface;
 
  /**
  * check if the backend allows the user to change his avatar on Personal page