NB-Public/user_saml
14
0
Fork 0
mirror of https://github.com/netzbegruenung/user_saml.git synced 2024-05-02 17:14:53 +02:00
Code Issues Projects Releases Wiki Activity

bump backend version and drop support for old Nextcloud releases

Browse source 
- also fixes some logic errors and deprecations
- GroupBackend can now implement INamedBackend

Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de>
This commit is contained in:
Arthur Schiwon 2022-09-12 15:42:19 +02:00 committed by Jonathan Treffler
parent 49aa594874
commit 7d7d56c0ca
11 changed files with 604 additions and 121 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

8
CHANGELOG.md
View file

@ -1,6 +1,14 @@
# Changelog
All notable changes to this project will be documented in this file.
## 6.0.0 (unreleased)
### Added
- Group backend
### Changed
- drop support for Nextcloud 21 and 22
## 5.0.3
### Fixed
- Fix signining in with multiple IdPs

22
appinfo/app.php
View file

@ -19,8 +19,10 @@
*
*/
use OCA\User_SAML\GroupBackend;
use OCA\User_SAML\GroupDuplicateChecker;
use OCA\User_SAML\GroupManager;
use OCA\User_SAML\SAMLSettings;
use OCP\BackgroundJob\IJobList;
use OCP\IConfig;
use OCP\IDBConnection;
@ -31,7 +33,7 @@ use Psr\Log\LoggerInterface;
require_once __DIR__ . '/../3rdparty/vendor/autoload.php';
// If we run in CLI mode do not setup the app as it can fail the OCC execution
// If we run in CLI mode do not set up the app as it can fail the OCC execution
// since the URLGenerator isn't accessible.
$cli = false;
if (OC::$CLI) {
@ -45,27 +47,21 @@ try {
$userSession = \OC::$server->getUserSession();
$session = \OC::$server->getSession();
} catch (Throwable $e) {
\OC::$server->getLogger()->logException($e);
/** @var LoggerInterface $logger */
$logger = \OC::$server->get(LoggerInterface::class);
$logger->critical($e->getMessage(), ['app' => 'user_saml', 'exception' => $e]);
return;
}
\OC::$server->registerService(GroupDuplicateChecker::class, function (ContainerInterface $c) use ($config) {
return new GroupDuplicateChecker(
$config,
$c->get(IGroupManager::class),
$c->get(LoggerInterface::class)
);
});
$groupBackend = new \OCA\User_SAML\GroupBackend(\OC::$server->getDatabaseConnection());
$groupBackend = \OC::$server->get(GroupBackend::class);
\OC::$server->get(IGroupManager::class)->addBackend($groupBackend);
$samlSettings = \OC::$server->get(\OCA\User_SAML\SAMLSettings::class);
$samlSettings = \OC::$server->get(SAMLSettings::class);
\OC::$server->registerService(GroupManager::class, function (ContainerInterface $c) use ($groupBackend, $samlSettings) {
return new GroupManager(
$c->get(IDBConnection::class),
$c->get(SAMLGroupDuplicateChecker::class),
$c->get(GroupDuplicateChecker::class),
$c->get(IGroupManager::class),
$c->get(IUserManager::class),
$groupBackend,

4
appinfo/info.xml
View file

@ -16,7 +16,7 @@ The following providers are supported and tested at the moment:
* Any other provider that authenticates using the environment variable
While theoretically any other authentication provider implementing either one of those standards is compatible, we like to note that they are not part of any internal test matrix.]]></description>
<version>5.0.3</version>
<version>6.0.0</version>
<licence>agpl</licence>
<author>Lukas Reschke</author>
<namespace>User_SAML</namespace>
@ -33,7 +33,7 @@ While theoretically any other authentication provider implementing either one of
<screenshot>https://raw.githubusercontent.com/nextcloud/user_saml/master/screenshots/1.png</screenshot>
<screenshot>https://raw.githubusercontent.com/nextcloud/user_saml/master/screenshots/2.png</screenshot>
<dependencies>
<nextcloud min-version="21" max-version="25" />
<nextcloud min-version="23" max-version="25" />
</dependencies>
<repair-steps>
<post-migration>

68
lib/GroupBackend.php
View file

@ -34,9 +34,13 @@ use OCA\User_SAML\Exceptions\AddUserToGroupException;
use OCP\Group\Backend\ABackend;
use OCP\Group\Backend\IAddToGroupBackend;
use OCP\Group\Backend\ICountUsersBackend;
use OCP\Group\Backend\ICreateGroupBackend;
use OCP\Group\Backend\IDeleteGroupBackend;
use OCP\Group\Backend\INamedBackend;
use OCP\Group\Backend\IRemoveFromGroupBackend;
use OCP\IDBConnection;
class GroupBackend extends ABackend implements IAddToGroupBackend, ICountUsersBackend {
class GroupBackend extends ABackend implements IAddToGroupBackend, ICountUsersBackend, ICreateGroupBackend, IDeleteGroupBackend, IRemoveFromGroupBackend, INamedBackend {
/** @var IDBConnection */
private $dbc;
@ -50,7 +54,7 @@ class GroupBackend extends ABackend implements IAddToGroupBackend, ICountUsersBa
$this->dbc = $dbc;
}
public function inGroup($uid, $gid) {
public function inGroup($uid, $gid): bool {
$qb = $this->dbc->getQueryBuilder();
$stmt = $qb->select('gid')
->from(self::TABLE_MEMBERS)
@ -67,7 +71,7 @@ class GroupBackend extends ABackend implements IAddToGroupBackend, ICountUsersBa
/**
* @return string[] Group names
*/
public function getUserGroups($uid) {
public function getUserGroups($uid): array {
$qb = $this->dbc->getQueryBuilder();
$cursor = $qb->select('gid')
->from(self::TABLE_MEMBERS)
@ -87,7 +91,7 @@ class GroupBackend extends ABackend implements IAddToGroupBackend, ICountUsersBa
/**
* @return string[] Group names
*/
public function getGroups($search = '', $limit = null, $offset = null) {
public function getGroups($search = '', $limit = null, $offset = null): array {
$query = $this->dbc->getQueryBuilder();
$query->select('gid')
->from(self::TABLE_GROUPS)
@ -113,9 +117,10 @@ class GroupBackend extends ABackend implements IAddToGroupBackend, ICountUsersBa
}
/**
* @param string $gid
* @return bool
*/
public function groupExists($gid) {
public function groupExists($gid): bool {
if (isset($this->groupCache[$gid])) {
return true;
}
@ -135,12 +140,12 @@ class GroupBackend extends ABackend implements IAddToGroupBackend, ICountUsersBa
return false;
}
public function groupExistsWithDifferentGid($samlGid): ?string {
public function groupExistsWithDifferentGid(string $samlGid): ?string {
$qb = $this->dbc->getQueryBuilder();
$cursor = $qb->select('gid')
->from(self::TABLE_GROUPS)
->where($qb->expr()->eq('saml_gid', $qb->createNamedParameter($samlGid)))
->execute();
->executeQuery();
$result = $cursor->fetch(FetchMode::NUMERIC);
$cursor->closeCursor();
@ -151,9 +156,13 @@ class GroupBackend extends ABackend implements IAddToGroupBackend, ICountUsersBa
}
/**
* @param string $gid
* @param string $search
* @param int $limit
* @param int $offset
* @return string[] User ids
*/
public function usersInGroup($gid, $search = '', $limit = -1, $offset = 0) {
public function usersInGroup($gid, $search = '', $limit = -1, $offset = 0): array {
$query = $this->dbc->getQueryBuilder();
$query->select('uid')
->from(self::TABLE_MEMBERS)
@ -220,12 +229,12 @@ class GroupBackend extends ABackend implements IAddToGroupBackend, ICountUsersBa
public function removeFromGroup(string $uid, string $gid): bool {
$qb = $this->dbc->getQueryBuilder();
$qb->delete(self::TABLE_MEMBERS)
$rows = $qb->delete(self::TABLE_MEMBERS)
->where($qb->expr()->eq('uid', $qb->createNamedParameter($uid)))
->andWhere($qb->expr()->eq('gid', $qb->createNamedParameter($gid)))
->execute();
->executeStatement();
return true;
return $rows > 0;
}
public function countUsersInGroup(string $gid, string $search = ''): int {
@ -236,11 +245,11 @@ class GroupBackend extends ABackend implements IAddToGroupBackend, ICountUsersBa
if ($search !== '') {
$query->andWhere($query->expr()->like('uid', $query->createNamedParameter(
'%' . $this->dbConn->escapeLikeParameter($search) . '%'
'%' . $this->dbc->escapeLikeParameter($search) . '%'
)));
}
$result = $query->execute();
$result = $query->executeQuery();
$count = $result->fetchOne();
$result->closeCursor();
@ -255,18 +264,31 @@ class GroupBackend extends ABackend implements IAddToGroupBackend, ICountUsersBa
public function deleteGroup(string $gid): bool {
$query = $this->dbc->getQueryBuilder();
// delete the group
$query->delete(self::TABLE_GROUPS)
->where($query->expr()->eq('gid', $query->createNamedParameter($gid)))
->execute();
// delete group user relation
$query->delete(self::TABLE_MEMBERS)
->where($query->expr()->eq('gid', $query->createNamedParameter($gid)))
->execute();
try {
$this->dbc->beginTransaction();
// delete the group
$query->delete(self::TABLE_GROUPS)
->where($query->expr()->eq('gid', $query->createNamedParameter($gid)))
->executeStatement();
// delete group user relation
$query->delete(self::TABLE_MEMBERS)
->where($query->expr()->eq('gid', $query->createNamedParameter($gid)))
->executeStatement();
$this->dbc->commit();
unset($this->groupCache[$gid]);
} catch (\Throwable $t) {
$this->dbc->rollBack();
throw $t;
}
// remove from cache
unset($this->groupCache[$gid]);
return true;
}
public function getBackendName(): string {
return 'user_saml';
}
}

16
lib/GroupManager.php
View file

@ -164,8 +164,7 @@ class GroupManager {
//FIXME: probably need config flag. Previous to 17, gid was used as displayname
$providerId = $this->settings->getProviderId();
$settings = $this->settings->get($providerId);
$groupPrefix = isset($settings['saml-attribute-mapping-group_mapping_prefix'])
? $settings['saml-attribute-mapping-group_mapping_prefix'] : 'SAML_';
$groupPrefix = $settings['saml-attribute-mapping-group_mapping_prefix'] ?? 'SAML_';
$group = $this->createGroupInBackend($groupPrefix . $gid, $gid);
} else {
throw $e;
@ -228,18 +227,7 @@ class GroupManager {
}
protected function hasSamlBackend(IGroup $group): bool {
$reflected = new \ReflectionObject($group);
$backendsProperty = $reflected->getProperty('backends');
$backendsProperty->setAccessible(true);
$backends = $backendsProperty->getValue($group);
// available at nextcloud 22
// $backends = $group->getBackendNames();
foreach ($backends as $backend) {
if ($backend instanceof GroupBackend) {
return true;
}
}
return false;
return in_array('user_saml', $group->getBackendNames());
}
public function evaluateGroupMigrations(array $groups): void {

6
lib/Jobs/MigrateGroups.php
View file

@ -32,13 +32,13 @@ use OCA\User_SAML\GroupManager;
use OCP\IConfig;
use OCP\IDBConnection;
use OCP\IGroupManager;
use OCP\ILogger;
use Psr\Log\LoggerInterface;
/**
* Class MigrateGroups
*
* @package OCA\User_SAML\Jobs
* @todo: remove this, when dropping Nextcloud 18 support
* @todo: remove this, when dropping Nextcloud 23 support
*/
class MigrateGroups extends QueuedJob {
@ -122,7 +122,7 @@ class MigrateGroups extends QueuedJob {
return true;
} catch (\Exception $e) {
$this->dbc->rollBack();
$this->logger->logException($e, ['app' => 'user_saml', 'level' => ILogger::WARN]);
$this->logger->warning($e->getMessage(), ['app' => 'user_saml', 'exception' => $e]);
}
return false;

47
lib/Migration/Version2500Date20191008134400.php → lib/Migration/Version6000Date20220912152700.php
View file

@ -39,7 +39,7 @@ use OCP\Migration\IOutput;
/**
* Auto-generated migration step: Please modify to your needs!
*/
class Version2500Date20191008134400 extends SimpleMigrationStep {
class Version6000Date20220912152700 extends SimpleMigrationStep {
/**
* @return null|ISchemaWrapper
@ -48,26 +48,6 @@ class Version2500Date20191008134400 extends SimpleMigrationStep {
/** @var ISchemaWrapper $schema */
$schema = $schemaClosure();
if (!$schema->hasTable('user_saml_users')) {
$table = $schema->createTable('user_saml_users');
$table->addColumn('uid', Types::STRING, [
'notnull' => true,
'length' => 64,
'default' => '',
]);
$table->addColumn('displayname', Types::STRING, [
'notnull' => true,
'length' => 255,
'default' => '',
]);
$table->addColumn('home', Types::STRING, [
'notnull' => true,
'length' => 255,
'default' => '',
]);
$table->setPrimaryKey(['uid']);
}
if (!$schema->hasTable('user_saml_groups')) {
$table = $schema->createTable('user_saml_groups');
$table->addColumn('gid', Types::STRING, [
@ -89,31 +69,6 @@ class Version2500Date20191008134400 extends SimpleMigrationStep {
$table->addUniqueIndex(['saml_gid']);
}
if (!$schema->hasTable('user_saml_auth_token')) {
$table = $schema->createTable('user_saml_auth_token');
$table->addColumn('id', Types::INTEGER, [
'autoincrement' => true,
'notnull' => true,
'length' => 4,
'unsigned' => true,
]);
$table->addColumn('uid', Types::STRING, [
'notnull' => true,
'length' => 64,
'default' => '',
]);
$table->addColumn('name', Types::TEXT, [
'notnull' => true,
'default' => '',
]);
$table->addColumn('token', Types::STRING, [
'notnull' => true,
'length' => 200,
'default' => '',
]);
$table->setPrimaryKey(['id']);
}
if (!$schema->hasTable('user_saml_group_members')) {
$table = $schema->createTable('user_saml_group_members');
$table->addColumn('uid', Types::STRING, [

9
lib/UserBackend.php
View file

@ -659,21 +659,16 @@ class UserBackend implements IApacheBackend, UserInterface, IUserBackend {
$user->setQuota($newQuota);
}
if ($newGroups === null) {
$newGroups = [];
}
try {
$this->groupManager->replaceGroups($user->getUID(), $newGroups);
$this->groupManager->replaceGroups($user->getUID(), $newGroups ?? []);
} catch (AddUserToGroupException $e) {
$this->logger->error('Failed to add user to group: {exception}', ['app' => 'user_saml', 'exception' => $e->getMessage()]);
}
// TODO: drop following line with dropping NC 18 support
// TODO: drop following line with dropping NC 23 support
$this->groupManager->evaluateGroupMigrations($newGroups);
}
}
public function countUsers() {
$query = $this->db->getQueryBuilder();
$query->select($query->func()->count('uid'))

518
modernize.patch Normal file
View file

@ -0,0 +1,518 @@
commit 987ede67c132a48431dfb4f948d7c466f7b360a8
Author: Arthur Schiwon <blizzz@arthur-schiwon.de>
Date: Mon Sep 12 15:36:04 2022 +0200
bump backend version and drop support for old Nextcloud releases
- also fixes some logic errors and deprecations
- GroupBackend can now implement INamedBackend
Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de>
diff --git a/CHANGELOG.md b/CHANGELOG.md
index b0fc80b..ed662df 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,6 +1,14 @@
# Changelog
All notable changes to this project will be documented in this file.
+## 6.0.0 (unreleased)
+
+### Added
+- Group backend
+
+### Changed
+- drop support for Nextcloud 21 and 22
+
## 5.0.3
### Fixed
- Fix signining in with multiple IdPs
diff --git a/appinfo/app.php b/appinfo/app.php
index 28db1d1..c2ee180 100644
--- a/appinfo/app.php
+++ b/appinfo/app.php
@@ -19,8 +19,10 @@
*
*/
+use OCA\User_SAML\GroupBackend;
use OCA\User_SAML\GroupDuplicateChecker;
use OCA\User_SAML\GroupManager;
+use OCA\User_SAML\SAMLSettings;
use OCP\BackgroundJob\IJobList;
use OCP\IConfig;
use OCP\IDBConnection;
@@ -31,7 +33,7 @@ use Psr\Log\LoggerInterface;
require_once __DIR__ . '/../3rdparty/vendor/autoload.php';
-// If we run in CLI mode do not setup the app as it can fail the OCC execution
+// If we run in CLI mode do not set up the app as it can fail the OCC execution
// since the URLGenerator isn't accessible.
$cli = false;
if (OC::$CLI) {
@@ -45,27 +47,21 @@ try {
$userSession = \OC::$server->getUserSession();
$session = \OC::$server->getSession();
} catch (Throwable $e) {
- \OC::$server->getLogger()->logException($e);
+ /** @var LoggerInterface $logger */
+ $logger = \OC::$server->get(LoggerInterface::class);
+ $logger->critical($e->getMessage(), ['app' => 'user_saml', 'exception' => $e]);
return;
}
-\OC::$server->registerService(GroupDuplicateChecker::class, function (ContainerInterface $c) use ($config) {
- return new GroupDuplicateChecker(
- $config,
- $c->get(IGroupManager::class),
- $c->get(LoggerInterface::class)
- );
-});
-
-$groupBackend = new \OCA\User_SAML\GroupBackend(\OC::$server->getDatabaseConnection());
+$groupBackend = \OC::$server->get(GroupBackend::class);
\OC::$server->get(IGroupManager::class)->addBackend($groupBackend);
-$samlSettings = \OC::$server->get(\OCA\User_SAML\SAMLSettings::class);
+$samlSettings = \OC::$server->get(SAMLSettings::class);
\OC::$server->registerService(GroupManager::class, function (ContainerInterface $c) use ($groupBackend, $samlSettings) {
return new GroupManager(
$c->get(IDBConnection::class),
- $c->get(SAMLGroupDuplicateChecker::class),
+ $c->get(GroupDuplicateChecker::class),
$c->get(IGroupManager::class),
$c->get(IUserManager::class),
$groupBackend,
diff --git a/appinfo/info.xml b/appinfo/info.xml
index ad1b978..cd5f197 100644
--- a/appinfo/info.xml
+++ b/appinfo/info.xml
@@ -16,7 +16,7 @@ The following providers are supported and tested at the moment:
* Any other provider that authenticates using the environment variable
While theoretically any other authentication provider implementing either one of those standards is compatible, we like to note that they are not part of any internal test matrix.]]></description>
- <version>5.0.3</version>
+ <version>6.0.0</version>
<licence>agpl</licence>
<author>Lukas Reschke</author>
<namespace>User_SAML</namespace>
@@ -33,7 +33,7 @@ While theoretically any other authentication provider implementing either one of
<screenshot>https://raw.githubusercontent.com/nextcloud/user_saml/master/screenshots/1.png</screenshot>
<screenshot>https://raw.githubusercontent.com/nextcloud/user_saml/master/screenshots/2.png</screenshot>
<dependencies>
- <nextcloud min-version="21" max-version="25" />
+ <nextcloud min-version="23" max-version="25" />
</dependencies>
<repair-steps>
<post-migration>
diff --git a/lib/GroupBackend.php b/lib/GroupBackend.php
index 7d01d3a..11f9151 100644
--- a/lib/GroupBackend.php
+++ b/lib/GroupBackend.php
@@ -34,9 +34,13 @@ use OCA\User_SAML\Exceptions\AddUserToGroupException;
use OCP\Group\Backend\ABackend;
use OCP\Group\Backend\IAddToGroupBackend;
use OCP\Group\Backend\ICountUsersBackend;
+use OCP\Group\Backend\ICreateGroupBackend;
+use OCP\Group\Backend\IDeleteGroupBackend;
+use OCP\Group\Backend\INamedBackend;
+use OCP\Group\Backend\IRemoveFromGroupBackend;
use OCP\IDBConnection;
-class GroupBackend extends ABackend implements IAddToGroupBackend, ICountUsersBackend {
+class GroupBackend extends ABackend implements IAddToGroupBackend, ICountUsersBackend, ICreateGroupBackend, IDeleteGroupBackend, IRemoveFromGroupBackend, INamedBackend {
/** @var IDBConnection */
private $dbc;
@@ -50,7 +54,7 @@ class GroupBackend extends ABackend implements IAddToGroupBackend, ICountUsersBa
$this->dbc = $dbc;
}
- public function inGroup($uid, $gid) {
+ public function inGroup($uid, $gid): bool {
$qb = $this->dbc->getQueryBuilder();
$stmt = $qb->select('gid')
->from(self::TABLE_MEMBERS)
@@ -67,7 +71,7 @@ class GroupBackend extends ABackend implements IAddToGroupBackend, ICountUsersBa
/**
* @return string[] Group names
*/
- public function getUserGroups($uid) {
+ public function getUserGroups($uid): array {
$qb = $this->dbc->getQueryBuilder();
$cursor = $qb->select('gid')
->from(self::TABLE_MEMBERS)
@@ -87,7 +91,7 @@ class GroupBackend extends ABackend implements IAddToGroupBackend, ICountUsersBa
/**
* @return string[] Group names
*/
- public function getGroups($search = '', $limit = null, $offset = null) {
+ public function getGroups($search = '', $limit = null, $offset = null): array {
$query = $this->dbc->getQueryBuilder();
$query->select('gid')
->from(self::TABLE_GROUPS)
@@ -113,9 +117,10 @@ class GroupBackend extends ABackend implements IAddToGroupBackend, ICountUsersBa
}
/**
+ * @param string $gid
* @return bool
*/
- public function groupExists($gid) {
+ public function groupExists($gid): bool {
if (isset($this->groupCache[$gid])) {
return true;
}
@@ -135,12 +140,12 @@ class GroupBackend extends ABackend implements IAddToGroupBackend, ICountUsersBa
return false;
}
- public function groupExistsWithDifferentGid($samlGid): ?string {
+ public function groupExistsWithDifferentGid(string $samlGid): ?string {
$qb = $this->dbc->getQueryBuilder();
$cursor = $qb->select('gid')
->from(self::TABLE_GROUPS)
->where($qb->expr()->eq('saml_gid', $qb->createNamedParameter($samlGid)))
- ->execute();
+ ->executeQuery();
$result = $cursor->fetch(FetchMode::NUMERIC);
$cursor->closeCursor();
@@ -151,9 +156,13 @@ class GroupBackend extends ABackend implements IAddToGroupBackend, ICountUsersBa
}
/**
+ * @param string $gid
+ * @param string $search
+ * @param int $limit
+ * @param int $offset
* @return string[] User ids
*/
- public function usersInGroup($gid, $search = '', $limit = -1, $offset = 0) {
+ public function usersInGroup($gid, $search = '', $limit = -1, $offset = 0): array {
$query = $this->dbc->getQueryBuilder();
$query->select('uid')
->from(self::TABLE_MEMBERS)
@@ -220,12 +229,12 @@ class GroupBackend extends ABackend implements IAddToGroupBackend, ICountUsersBa
public function removeFromGroup(string $uid, string $gid): bool {
$qb = $this->dbc->getQueryBuilder();
- $qb->delete(self::TABLE_MEMBERS)
+ $rows = $qb->delete(self::TABLE_MEMBERS)
->where($qb->expr()->eq('uid', $qb->createNamedParameter($uid)))
->andWhere($qb->expr()->eq('gid', $qb->createNamedParameter($gid)))
- ->execute();
+ ->executeStatement();
- return true;
+ return $rows > 0;
}
public function countUsersInGroup(string $gid, string $search = ''): int {
@@ -236,11 +245,11 @@ class GroupBackend extends ABackend implements IAddToGroupBackend, ICountUsersBa
if ($search !== '') {
$query->andWhere($query->expr()->like('uid', $query->createNamedParameter(
- '%' . $this->dbConn->escapeLikeParameter($search) . '%'
+ '%' . $this->dbc->escapeLikeParameter($search) . '%'
)));
}
- $result = $query->execute();
+ $result = $query->executeQuery();
$count = $result->fetchOne();
$result->closeCursor();
@@ -255,18 +264,31 @@ class GroupBackend extends ABackend implements IAddToGroupBackend, ICountUsersBa
public function deleteGroup(string $gid): bool {
$query = $this->dbc->getQueryBuilder();
- // delete the group
- $query->delete(self::TABLE_GROUPS)
- ->where($query->expr()->eq('gid', $query->createNamedParameter($gid)))
- ->execute();
- // delete group user relation
- $query->delete(self::TABLE_MEMBERS)
- ->where($query->expr()->eq('gid', $query->createNamedParameter($gid)))
- ->execute();
+ try {
+ $this->dbc->beginTransaction();
+
+ // delete the group
+ $query->delete(self::TABLE_GROUPS)
+ ->where($query->expr()->eq('gid', $query->createNamedParameter($gid)))
+ ->executeStatement();
+
+ // delete group user relation
+ $query->delete(self::TABLE_MEMBERS)
+ ->where($query->expr()->eq('gid', $query->createNamedParameter($gid)))
+ ->executeStatement();
+
+ $this->dbc->commit();
+ unset($this->groupCache[$gid]);
+ } catch (\Throwable $t) {
+ $this->dbc->rollBack();
+ throw $t;
+ }
- // remove from cache
- unset($this->groupCache[$gid]);
return true;
}
+
+ public function getBackendName(): string {
+ return 'user_saml';
+ }
}
diff --git a/lib/GroupManager.php b/lib/GroupManager.php
index 102f95a..1092ea9 100644
--- a/lib/GroupManager.php
+++ b/lib/GroupManager.php
@@ -164,8 +164,7 @@ class GroupManager {
//FIXME: probably need config flag. Previous to 17, gid was used as displayname
$providerId = $this->settings->getProviderId();
$settings = $this->settings->get($providerId);
- $groupPrefix = isset($settings['saml-attribute-mapping-group_mapping_prefix'])
- ? $settings['saml-attribute-mapping-group_mapping_prefix'] : 'SAML_';
+ $groupPrefix = $settings['saml-attribute-mapping-group_mapping_prefix'] ?? 'SAML_';
$group = $this->createGroupInBackend($groupPrefix . $gid, $gid);
} else {
throw $e;
@@ -228,18 +227,7 @@ class GroupManager {
}
protected function hasSamlBackend(IGroup $group): bool {
- $reflected = new \ReflectionObject($group);
- $backendsProperty = $reflected->getProperty('backends');
- $backendsProperty->setAccessible(true);
- $backends = $backendsProperty->getValue($group);
- // available at nextcloud 22
- // $backends = $group->getBackendNames();
- foreach ($backends as $backend) {
- if ($backend instanceof GroupBackend) {
- return true;
- }
- }
- return false;
+ return in_array('user_saml', $group->getBackendNames());
}
public function evaluateGroupMigrations(array $groups): void {
diff --git a/lib/Jobs/MigrateGroups.php b/lib/Jobs/MigrateGroups.php
index d0b0607..4f1b734 100644
--- a/lib/Jobs/MigrateGroups.php
+++ b/lib/Jobs/MigrateGroups.php
@@ -32,13 +32,13 @@ use OCA\User_SAML\GroupManager;
use OCP\IConfig;
use OCP\IDBConnection;
use OCP\IGroupManager;
-use OCP\ILogger;
+use Psr\Log\LoggerInterface;
/**
* Class MigrateGroups
*
* @package OCA\User_SAML\Jobs
- * @todo: remove this, when dropping Nextcloud 18 support
+ * @todo: remove this, when dropping Nextcloud 23 support
*/
class MigrateGroups extends QueuedJob {
@@ -122,7 +122,7 @@ class MigrateGroups extends QueuedJob {
return true;
} catch (\Exception $e) {
$this->dbc->rollBack();
- $this->logger->logException($e, ['app' => 'user_saml', 'level' => ILogger::WARN]);
+ $this->logger->warning($e->getMessage(), ['app' => 'user_saml', 'exception' => $e]);
}
return false;
diff --git a/lib/Migration/Version2500Date20191008134400.php b/lib/Migration/Version6000Date20220912152700.php
similarity index 68%
rename from lib/Migration/Version2500Date20191008134400.php
rename to lib/Migration/Version6000Date20220912152700.php
index b5d2852..0501f1f 100644
--- a/lib/Migration/Version2500Date20191008134400.php
+++ b/lib/Migration/Version6000Date20220912152700.php
@@ -39,7 +39,7 @@ use OCP\Migration\IOutput;
/**
* Auto-generated migration step: Please modify to your needs!
*/
-class Version2500Date20191008134400 extends SimpleMigrationStep {
+class Version6000Date20220912152700 extends SimpleMigrationStep {
/**
* @return null|ISchemaWrapper
@@ -48,26 +48,6 @@ class Version2500Date20191008134400 extends SimpleMigrationStep {
/** @var ISchemaWrapper $schema */
$schema = $schemaClosure();
- if (!$schema->hasTable('user_saml_users')) {
- $table = $schema->createTable('user_saml_users');
- $table->addColumn('uid', Types::STRING, [
- 'notnull' => true,
- 'length' => 64,
- 'default' => '',
- ]);
- $table->addColumn('displayname', Types::STRING, [
- 'notnull' => true,
- 'length' => 255,
- 'default' => '',
- ]);
- $table->addColumn('home', Types::STRING, [
- 'notnull' => true,
- 'length' => 255,
- 'default' => '',
- ]);
- $table->setPrimaryKey(['uid']);
- }
-
if (!$schema->hasTable('user_saml_groups')) {
$table = $schema->createTable('user_saml_groups');
$table->addColumn('gid', Types::STRING, [
@@ -89,31 +69,6 @@ class Version2500Date20191008134400 extends SimpleMigrationStep {
$table->addUniqueIndex(['saml_gid']);
}
- if (!$schema->hasTable('user_saml_auth_token')) {
- $table = $schema->createTable('user_saml_auth_token');
- $table->addColumn('id', Types::INTEGER, [
- 'autoincrement' => true,
- 'notnull' => true,
- 'length' => 4,
- 'unsigned' => true,
- ]);
- $table->addColumn('uid', Types::STRING, [
- 'notnull' => true,
- 'length' => 64,
- 'default' => '',
- ]);
- $table->addColumn('name', Types::TEXT, [
- 'notnull' => true,
- 'default' => '',
- ]);
- $table->addColumn('token', Types::STRING, [
- 'notnull' => true,
- 'length' => 200,
- 'default' => '',
- ]);
- $table->setPrimaryKey(['id']);
- }
-
if (!$schema->hasTable('user_saml_group_members')) {
$table = $schema->createTable('user_saml_group_members');
$table->addColumn('uid', Types::STRING, [
diff --git a/lib/UserBackend.php b/lib/UserBackend.php
index ce1df92..c0ca3fe 100644
--- a/lib/UserBackend.php
+++ b/lib/UserBackend.php
@@ -659,21 +659,16 @@ class UserBackend implements IApacheBackend, UserInterface, IUserBackend {
$user->setQuota($newQuota);
}
- if ($newGroups === null) {
- $newGroups = [];
- }
try {
- $this->groupManager->replaceGroups($user->getUID(), $newGroups);
+ $this->groupManager->replaceGroups($user->getUID(), $newGroups ?? []);
} catch (AddUserToGroupException $e) {
$this->logger->error('Failed to add user to group: {exception}', ['app' => 'user_saml', 'exception' => $e->getMessage()]);
}
- // TODO: drop following line with dropping NC 18 support
+ // TODO: drop following line with dropping NC 23 support
$this->groupManager->evaluateGroupMigrations($newGroups);
}
}
-
-
public function countUsers() {
$query = $this->db->getQueryBuilder();
$query->select($query->func()->count('uid'))
diff --git a/tests/unit/GroupManagerTest.php b/tests/unit/GroupManagerTest.php
index 9f9165f..ea6ed67 100644
--- a/tests/unit/GroupManagerTest.php
+++ b/tests/unit/GroupManagerTest.php
@@ -34,27 +34,28 @@ use OCP\IGroup;
use OCP\IGroupManager;
use OCP\IUser;
use OCP\IUserManager;
+use PHPUnit\Framework\MockObject\MockObject;
use Test\TestCase;
class GroupManagerTest extends TestCase {
- /** @var IDBConnection|\PHPUnit_Framework_MockObject_MockObject */
+ /** @var IDBConnection|MockObject */
private $db;
- /** @var GroupDuplicateChecker|\PHPUnit_Framework_MockObject_MockObject */
+ /** @var GroupDuplicateChecker|MockObject */
private $duplicateChecker;
- /** @var IGroupManager|\PHPUnit_Framework_MockObject_MockObject */
+ /** @var IGroupManager|MockObject */
private $groupManager;
- /** @var IUserManager|\PHPUnit_Framework_MockObject_MockObject */
+ /** @var IUserManager|MockObject */
private $userManager;
- /** @var GroupBackend|\PHPUnit_Framework_MockObject_MockObject */
+ /** @var GroupBackend|MockObject */
private $ownGroupBackend;
- /** @var IConfig|\PHPUnit_Framework_MockObject_MockObject */
+ /** @var IConfig|MockObject */
private $config;
- /** @var JobList|\PHPUnit_Framework_MockObject_MockObject */
+ /** @var JobList|MockObject */
private $jobList;
- /** @var SAMLSettings|\PHPUnit_Framework_MockObject_MockObject */
+ /** @var SAMLSettings|MockObject */
private $settings;
- /** @var GroupManager|\PHPUnit_Framework_MockObject_MockObject */
+ /** @var GroupManager|MockObject */
private $ownGroupManager;
protected function setUp(): void {
@@ -141,7 +142,7 @@ class GroupManagerTest extends TestCase {
->expects($this->once())
->method('addGroups')
->with($user, ['groupC']);
-
+
// assert SAML provides user groups groupB and groupC
$this->ownGroupManager->replaceGroups('ExistingUser', ['groupB', 'groupC']);
}
@@ -200,7 +201,7 @@ class GroupManagerTest extends TestCase {
$this->ownGroupManager
->expects($this->never())
->method('createGroupInBackend');
-
+
$this->ownGroupManager->addGroups($user, ['groupA']);
}
@@ -208,7 +209,7 @@ class GroupManagerTest extends TestCase {
$this->getGroupManager();
$user = $this->createMock(IUser::class);
$groupB = $this->createMock(IGroup::class);
-
+
// assert group does not exist
$this->groupManager
->expects($this->at(0))
diff --git a/tests/unit/UserBackendTest.php b/tests/unit/UserBackendTest.php
index ff16b94..3112280 100644
--- a/tests/unit/UserBackendTest.php
+++ b/tests/unit/UserBackendTest.php
@@ -50,7 +50,7 @@ class UserBackendTest extends TestCase {
private $db;
/** @var IUserManager|MockObject */
private $userManager;
- /** @var GroupManager|\PHPUnit_Framework_MockObject_MockObject */
+ /** @var GroupManager|MockObject */
private $groupManager;
/** @var UserBackend|MockObject */
private $userBackend;

25
tests/unit/GroupManagerTest.php
View file

@ -34,27 +34,28 @@ use OCP\IGroup;
use OCP\IGroupManager;
use OCP\IUser;
use OCP\IUserManager;
use PHPUnit\Framework\MockObject\MockObject;
use Test\TestCase;
class GroupManagerTest extends TestCase {
/** @var IDBConnection|\PHPUnit_Framework_MockObject_MockObject */
/** @var IDBConnection|MockObject */
private $db;
/** @var GroupDuplicateChecker|\PHPUnit_Framework_MockObject_MockObject */
/** @var GroupDuplicateChecker|MockObject */
private $duplicateChecker;
/** @var IGroupManager|\PHPUnit_Framework_MockObject_MockObject */
/** @var IGroupManager|MockObject */
private $groupManager;
/** @var IUserManager|\PHPUnit_Framework_MockObject_MockObject */
/** @var IUserManager|MockObject */
private $userManager;
/** @var GroupBackend|\PHPUnit_Framework_MockObject_MockObject */
/** @var GroupBackend|MockObject */
private $ownGroupBackend;
/** @var IConfig|\PHPUnit_Framework_MockObject_MockObject */
/** @var IConfig|MockObject */
private $config;
/** @var JobList|\PHPUnit_Framework_MockObject_MockObject */
/** @var JobList|MockObject */
private $jobList;
/** @var SAMLSettings|\PHPUnit_Framework_MockObject_MockObject */
/** @var SAMLSettings|MockObject */
private $settings;
/** @var GroupManager|\PHPUnit_Framework_MockObject_MockObject */
/** @var GroupManager|MockObject */
private $ownGroupManager;
protected function setUp(): void {
@ -141,7 +142,7 @@ class GroupManagerTest extends TestCase {
->expects($this->once())
->method('addGroups')
->with($user, ['groupC']);
// assert SAML provides user groups groupB and groupC
$this->ownGroupManager->replaceGroups('ExistingUser', ['groupB', 'groupC']);
}
@ -200,7 +201,7 @@ class GroupManagerTest extends TestCase {
$this->ownGroupManager
->expects($this->never())
->method('createGroupInBackend');
$this->ownGroupManager->addGroups($user, ['groupA']);
}
@ -208,7 +209,7 @@ class GroupManagerTest extends TestCase {
$this->getGroupManager();
$user = $this->createMock(IUser::class);
$groupB = $this->createMock(IGroup::class);
// assert group does not exist
$this->groupManager
->expects($this->at(0))

2
tests/unit/UserBackendTest.php
View file

@ -50,7 +50,7 @@ class UserBackendTest extends TestCase {
private $db;
/** @var IUserManager|MockObject */
private $userManager;
/** @var GroupManager|\PHPUnit_Framework_MockObject_MockObject */
/** @var GroupManager|MockObject */
private $groupManager;
/** @var UserBackend|MockObject */
private $userBackend;