Fix php code styles

Signed-off-by: Giuliano Mele <giuliano.mele@verdigado.com>
This commit is contained in:
Giuliano Mele 2022-07-14 12:54:54 +02:00 committed by Arthur Schiwon
parent 957a0777ab
commit 727e50cfc7
No known key found for this signature in database
GPG key ID: 7424F1874854DF23
10 changed files with 161 additions and 168 deletions

View file

@ -49,12 +49,12 @@ try {
return;
}
\OC::$server->registerService(GroupDuplicateChecker::class, function(ContainerInterface $c) use($config) {
return new GroupDuplicateChecker(
$config,
$c->get(IGroupManager::class),
$c->get(LoggerInterface::class)
);
\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());
@ -62,17 +62,17 @@ $groupBackend = new \OCA\User_SAML\GroupBackend(\OC::$server->getDatabaseConnect
$samlSettings = \OC::$server->get(\OCA\User_SAML\SAMLSettings::class);
\OC::$server->registerService(GroupManager::class, function(ContainerInterface $c) use($groupBackend, $samlSettings) {
return new GroupManager(
$c->get(IDBConnection::class),
$c->get(SAMLGroupDuplicateChecker::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(IGroupManager::class),
$c->get(IUserManager::class),
$groupBackend,
$c->get(IConfig::class),
$c->get(IJobList::class),
$samlSettings,
);
);
});
$userData = new \OCA\User_SAML\UserData(

View file

@ -43,8 +43,8 @@ class GroupBackend extends ABackend implements IAddToGroupBackend, ICountUsersBa
/** @var array */
private $groupCache = [];
const TABLE_GROUPS = 'user_saml_groups';
const TABLE_MEMBERS = 'user_saml_group_members';
public const TABLE_GROUPS = 'user_saml_groups';
public const TABLE_MEMBERS = 'user_saml_group_members';
public function __construct(IDBConnection $dbc) {
$this->dbc = $dbc;
@ -75,7 +75,7 @@ class GroupBackend extends ABackend implements IAddToGroupBackend, ICountUsersBa
->execute();
$groups = [];
while( $row = $cursor->fetch()) {
while ($row = $cursor->fetch()) {
$groups[] = $row['gid'];
$this->groupCache[$row['gid']] = $row['gid'];
}
@ -195,7 +195,7 @@ class GroupBackend extends ABackend implements IAddToGroupBackend, ICountUsersBa
->setValue('displayname', $builder->createNamedParameter($displayName))
->setValue('saml_gid', $builder->createNamedParameter($samlGid))
->execute();
} catch(UniqueConstraintViolationException $e) {
} catch (UniqueConstraintViolationException $e) {
$result = 0;
}

View file

@ -32,8 +32,7 @@ use OCP\IConfig;
use OCP\IGroupManager;
use Psr\Log\LoggerInterface;
class GroupDuplicateChecker
{
class GroupDuplicateChecker {
/**
* @var IConfig
*/

View file

@ -30,9 +30,7 @@ namespace OCA\User_SAML;
use OC\BackgroundJob\JobList;
use OC\Hooks\PublicEmitter;
use OCA\User_SAML\GroupBackend;
use OCA\User_SAML\Jobs\MigrateGroups;
use OCA\User_SAML\SAMLSettings;
use OCP\IConfig;
use OCP\IDBConnection;
use OCP\IGroup;
@ -40,9 +38,8 @@ use OCP\IGroupManager;
use OCP\IUser;
use OCP\IUserManager;
class GroupManager
{
const LOCAL_GROUPS_CHECK_FOR_MIGRATION = 'localGroupsCheckForMigration';
class GroupManager {
public const LOCAL_GROUPS_CHECK_FOR_MIGRATION = 'localGroupsCheckForMigration';
/**
* @var IDBConnection $db
@ -89,7 +86,7 @@ class GroupManager
private function getGroupsToRemove(array $samlGroups, array $assignedGroups): array {
$groupsToRemove = [];
foreach($assignedGroups as $group) {
foreach ($assignedGroups as $group) {
// if group is not supplied by SAML and group has SAML backend
if (!in_array($group->getGID(), $samlGroups) && $this->hasSamlBackend($group)) {
$groupsToRemove[] = $group->getGID();
@ -100,7 +97,7 @@ class GroupManager
private function getGroupsToAdd(array $samlGroups, array $assignedGroupIds): array {
$groupsToAdd = [];
foreach($samlGroups as $group) {
foreach ($samlGroups as $group) {
// if user is not assigend to the group or the provided group has a non SAML backend
if (!in_array($group, $assignedGroupIds) || !$this->hasSamlBackend($this->groupManager->get($group))) {
$groupsToAdd[] = $group;
@ -111,12 +108,12 @@ class GroupManager
public function replaceGroups(string $uid, array $samlGroups): void {
$user = $this->userManager->get($uid);
if($user === null) {
if ($user === null) {
return;
}
$this->translateGroupToIds($samlGroups);
$assignedGroups = $this->groupManager->getUserGroups($user);
$assignedGroupIds = array_map(function(IGroup $group){
$assignedGroupIds = array_map(function (IGroup $group) {
return $group->getGID();
}, $assignedGroups);
$groupsToRemove = $this->getGroupsToRemove($samlGroups, $assignedGroups);
@ -126,9 +123,9 @@ class GroupManager
}
protected function translateGroupToIds(array &$samlGroups): void {
array_walk($samlGroups, function (&$gid){
array_walk($samlGroups, function (&$gid) {
$altGid = $this->ownGroupBackend->groupExistsWithDifferentGid($gid);
if($altGid !== null) {
if ($altGid !== null) {
$gid = $altGid;
}
});
@ -142,7 +139,7 @@ class GroupManager
public function removeGroup(IUser $user, string $gid): void {
$group = $this->groupManager->get($gid);
if($group === null) {
if ($group === null) {
return;
}
$this->ownGroupBackend->removeFromGroup($user->getUID(), $group->getGID());
@ -161,9 +158,9 @@ class GroupManager
try {
$group = $this->findGroup($gid);
} catch (\RuntimeException $e) {
if($e->getCode() === 1) {
if ($e->getCode() === 1) {
$group = $this->createGroupInBackend($gid);
} else if($e->getCode() === 2) {
} elseif ($e->getCode() === 2) {
//FIXME: probably need config flag. Previous to 17, gid was used as displayname
$providerId = $this->settings->getProviderId();
$settings = $this->settings->get($providerId);
@ -179,15 +176,15 @@ class GroupManager
}
protected function createGroupInBackend(string $gid, ?string $originalGid = null): ?IGroup {
if($this->groupManager instanceof PublicEmitter) {
if ($this->groupManager instanceof PublicEmitter) {
$this->groupManager->emit('\OC\Group', 'preCreate', array($gid));
}
if(!$this->ownGroupBackend->createGroup($gid, $originalGid ?? $gid)) {
if (!$this->ownGroupBackend->createGroup($gid, $originalGid ?? $gid)) {
return null;
}
$group = $this->groupManager->get($gid);
if($this->groupManager instanceof PublicEmitter) {
if ($this->groupManager instanceof PublicEmitter) {
$this->groupManager->emit('\OC\Group', 'postCreate', array($group));
}
@ -204,25 +201,25 @@ class GroupManager
if ($migrationWhiteList !== null) {
$migrationWhiteList = \json_decode($migrationWhiteList, true);
}
if(!$strictBackendCheck && in_array($gid, $migrationWhiteList['groups'], true)) {
if (!$strictBackendCheck && in_array($gid, $migrationWhiteList['groups'], true)) {
$group = $this->groupManager->get($gid);
if($group === null) {
if ($group === null) {
//FIXME: specific Exception and/or constant error code
throw new \RuntimeException('Group not found', 1);
}
return $group;
}
$group = $this->groupManager->get($gid);
if($group === null) {
if ($group === null) {
//FIXME: specific Exception and/or constant error code
throw new \RuntimeException('Group not found', 1);
}
if($this->hasSamlBackend($group)) {
if ($this->hasSamlBackend($group)) {
return $group;
}
$altGid = $this->ownGroupBackend->groupExistsWithDifferentGid($gid);
if($altGid) {
if ($altGid) {
return $this->groupManager->get($altGid);
}
@ -238,7 +235,7 @@ class GroupManager
// available at nextcloud 22
// $backends = $group->getBackendNames();
foreach ($backends as $backend) {
if($backend instanceof GroupBackend) {
if ($backend instanceof GroupBackend) {
return true;
}
}
@ -247,11 +244,11 @@ class GroupManager
public function evaluateGroupMigrations(array $groups): void {
$candidateInfo = $this->config->getAppValue('user_saml', self::LOCAL_GROUPS_CHECK_FOR_MIGRATION, null);
if($candidateInfo === null) {
if ($candidateInfo === null) {
return;
}
$candidateInfo = \json_decode($candidateInfo, true);
if(!isset($candidateInfo['dropAfter']) || $candidateInfo['dropAfter'] < time()) {
if (!isset($candidateInfo['dropAfter']) || $candidateInfo['dropAfter'] < time()) {
$this->config->deleteAppValue('user_saml', self::LOCAL_GROUPS_CHECK_FOR_MIGRATION);
return;
}

View file

@ -1,4 +1,5 @@
<?php
declare(strict_types=1);
/**
* @copyright Copyright (c) 2019 Arthur Schiwon <blizzz@arthur-schiwon.de>
@ -79,11 +80,11 @@ class MigrateGroups extends QueuedJob {
protected function updateCandidatePool($migrateGroups) {
$candidateInfo = $this->config->getAppValue('user_saml', GroupManager::LOCAL_GROUPS_CHECK_FOR_MIGRATION, null);
if($candidateInfo === null) {
if ($candidateInfo === null) {
return;
}
$candidateInfo = \json_decode($candidateInfo, true);
if(!isset($candidateInfo['dropAfter']) || !isset($candidateInfo['groups'])) {
if (!isset($candidateInfo['dropAfter']) || !isset($candidateInfo['groups'])) {
return;
}
$candidateInfo['groups'] = array_diff($candidateInfo['groups'], $migrateGroups);
@ -108,11 +109,11 @@ class MigrateGroups extends QueuedJob {
$affected = $qb->delete('groups')
->where($qb->expr()->eq('gid', $qb->createNamedParameter($gid)))
->execute();
if($affected === 0) {
if ($affected === 0) {
throw new \RuntimeException('Could not delete group from local backend');
}
if(!$this->ownGroupBackend->createGroup($gid)) {
if (!$this->ownGroupBackend->createGroup($gid)) {
throw new \RuntimeException('Could not create group in SAML backend');
}
@ -129,12 +130,12 @@ class MigrateGroups extends QueuedJob {
protected function getGroupsToMigrate(array $samlGroups, array $pool): array {
return array_filter($samlGroups, function (string $gid) use ($pool) {
if(!in_array($gid, $pool)) {
if (!in_array($gid, $pool)) {
return false;
}
$group = $this->groupManager->get($gid);
if($group === null) {
if ($group === null) {
return false;
}
$reflected = new \ReflectionClass($group);
@ -142,7 +143,7 @@ class MigrateGroups extends QueuedJob {
$backendsProperty->setAccessible(true);
$backends = $backendsProperty->getValue($group);
foreach ($backends as $backend) {
if($backend instanceof Database) {
if ($backend instanceof Database) {
return true;
}
}
@ -152,17 +153,15 @@ class MigrateGroups extends QueuedJob {
protected function getMigratableGroups(): array {
$candidateInfo = $this->config->getAppValue('user_saml', GroupManager::LOCAL_GROUPS_CHECK_FOR_MIGRATION, null);
if($candidateInfo === null) {
if ($candidateInfo === null) {
throw new \RuntimeException('No migration of groups to SAML backend anymore');
}
$candidateInfo = \json_decode($candidateInfo, true);
if(!isset($candidateInfo['dropAfter']) || !isset($candidateInfo['groups']) || $candidateInfo['dropAfter'] < time()) {
if (!isset($candidateInfo['dropAfter']) || !isset($candidateInfo['groups']) || $candidateInfo['dropAfter'] < time()) {
$this->config->deleteAppValue('user_saml', GroupManager::LOCAL_GROUPS_CHECK_FOR_MIGRATION);
throw new \RuntimeException('Period for migration groups is over');
}
return $candidateInfo['groups'];
}
}

View file

@ -1,4 +1,5 @@
<?php
declare(strict_types=1);
/**
* @copyright Copyright (c) 2019 Arthur Schiwon <blizzz@arthur-schiwon.de>
@ -86,7 +87,7 @@ class RememberLocalGroupsForPotentialMigrations implements IRepairStep {
protected function findGroupIds(Database $backend): array {
$groupIds = $backend->getGroups();
$adminGroupIndex = array_search('admin', $groupIds, true);
if($adminGroupIndex !== false) {
if ($adminGroupIndex !== false) {
unset($groupIds[$adminGroupIndex]);
}
return $groupIds;
@ -95,7 +96,7 @@ class RememberLocalGroupsForPotentialMigrations implements IRepairStep {
protected function findBackend(): Database {
$groupBackends = $this->groupManager->getBackends();
foreach ($groupBackends as $backend) {
if($backend instanceof Database) {
if ($backend instanceof Database) {
return $backend;
break;
}

View file

@ -29,7 +29,6 @@ use OCP\IDBConnection;
use OCP\ILogger;
use OCP\IUser;
use OCP\IUserManager;
use OCA\User_SAML\GroupManager;
use OCP\UserInterface;
use OCP\IUserBackend;
use OCP\IConfig;

View file

@ -34,119 +34,119 @@ use \OCA\User_SAML\GroupBackend;
*/
class GroupBackendTest extends TestCase {
/** @var GroupBackend */
private static $groupBackend;
private static $users = [
[
'uid' => 'user_saml_integration_test_uid1',
'groups' => [
'user_saml_integration_test_gid1',
'SAML_user_saml_integration_test_gid2'
]
],
[
'uid' => 'user_saml_integration_test_uid2',
'groups' => [
'user_saml_integration_test_gid1'
]
]
];
private static $groups = [
[
'gid' => 'user_saml_integration_test_gid1',
'saml_gid' => 'user_saml_integration_test_gid1',
'members' => [
'user_saml_integration_test_uid1',
'user_saml_integration_test_uid2'
],
'saml_gid_exists' => true
],
[
'gid' => 'SAML_user_saml_integration_test_gid2',
'saml_gid' => 'user_saml_integration_test_gid2',
'members' => [
'user_saml_integration_test_uid1'
],
'saml_gid_exists' => false
],
[
'gid' => 'user_saml_integration_test_gid3',
'saml_gid' => 'user_saml_integration_test_gid3',
'members' => [],
'saml_gid_exists' => true
],
];
/** @var GroupBackend */
private static $groupBackend;
private static $users = [
[
'uid' => 'user_saml_integration_test_uid1',
'groups' => [
'user_saml_integration_test_gid1',
'SAML_user_saml_integration_test_gid2'
]
],
[
'uid' => 'user_saml_integration_test_uid2',
'groups' => [
'user_saml_integration_test_gid1'
]
]
];
private static $groups = [
[
'gid' => 'user_saml_integration_test_gid1',
'saml_gid' => 'user_saml_integration_test_gid1',
'members' => [
'user_saml_integration_test_uid1',
'user_saml_integration_test_uid2'
],
'saml_gid_exists' => true
],
[
'gid' => 'SAML_user_saml_integration_test_gid2',
'saml_gid' => 'user_saml_integration_test_gid2',
'members' => [
'user_saml_integration_test_uid1'
],
'saml_gid_exists' => false
],
[
'gid' => 'user_saml_integration_test_gid3',
'saml_gid' => 'user_saml_integration_test_gid3',
'members' => [],
'saml_gid_exists' => true
],
];
public static function setUpBeforeClass(): void {
parent::setUpBeforeClass();
self::$groupBackend = new \OCA\User_SAML\GroupBackend(\OC::$server->getDatabaseConnection());
foreach(self::$groups as $group){
self::$groupBackend->createGroup($group['gid'], $group['saml_gid']);
}
foreach(self::$users as $user){
foreach($user['groups'] as $group){
self::$groupBackend->addToGroup($user['uid'], $group);
}
}
}
public static function setUpBeforeClass(): void {
parent::setUpBeforeClass();
self::$groupBackend = new \OCA\User_SAML\GroupBackend(\OC::$server->getDatabaseConnection());
foreach (self::$groups as $group) {
self::$groupBackend->createGroup($group['gid'], $group['saml_gid']);
}
foreach (self::$users as $user) {
foreach ($user['groups'] as $group) {
self::$groupBackend->addToGroup($user['uid'], $group);
}
}
}
public static function tearDownAfterClass(): void {
parent::tearDownAfterClass();
self::$groupBackend = new \OCA\User_SAML\GroupBackend(\OC::$server->getDatabaseConnection());
foreach(self::$users as $user){
foreach($user['groups'] as $group){
self::$groupBackend->removeFromGroup($user['uid'], $group);
}
}
foreach (self::$groups as $group) {
self::$groupBackend->deleteGroup($group['gid']);
}
}
public static function tearDownAfterClass(): void {
parent::tearDownAfterClass();
self::$groupBackend = new \OCA\User_SAML\GroupBackend(\OC::$server->getDatabaseConnection());
foreach (self::$users as $user) {
foreach ($user['groups'] as $group) {
self::$groupBackend->removeFromGroup($user['uid'], $group);
}
}
foreach (self::$groups as $group) {
self::$groupBackend->deleteGroup($group['gid']);
}
}
public function testInGroup() {
foreach(self::$groups as $group){
foreach(self::$users as $user){
$result = self::$groupBackend->inGroup($user['uid'], $group['gid']);
if(in_array($group['gid'], $user['groups'])){
$this->assertTrue($result, sprintf("User %s should be member of group %s", $user['uid'], $group['gid']));
} else {
$this->assertFalse($result, sprintf("User %s should not be member of group %s", $user['uid'], $group['gid']));
}
}
}
}
public function testInGroup() {
foreach (self::$groups as $group) {
foreach (self::$users as $user) {
$result = self::$groupBackend->inGroup($user['uid'], $group['gid']);
if (in_array($group['gid'], $user['groups'])) {
$this->assertTrue($result, sprintf("User %s should be member of group %s", $user['uid'], $group['gid']));
} else {
$this->assertFalse($result, sprintf("User %s should not be member of group %s", $user['uid'], $group['gid']));
}
}
}
}
public function testGetGroups() {
$groups = self::$groupBackend->getGroups();
foreach (self::$groups as $group) {
$this->assertContains($group['gid'], $groups, sprintf('Group %s should be retrieved', $group['gid']));
}
}
public function testGetGroups() {
$groups = self::$groupBackend->getGroups();
foreach (self::$groups as $group) {
$this->assertContains($group['gid'], $groups, sprintf('Group %s should be retrieved', $group['gid']));
}
}
public function testGetUserGroups() {
foreach(self::$users as $user){
$userGroups = self::$groupBackend->getUserGroups($user['uid']);
$this->assertCount(count($user['groups']), $userGroups, 'Should retrieve all user groups');
foreach($userGroups as $userGroup){
$this->assertContains($userGroup, $user['groups'], sprintf('Users %s should be member of groups %s', $user['uid'], $userGroup));
}
}
}
public function testGetUserGroups() {
foreach (self::$users as $user) {
$userGroups = self::$groupBackend->getUserGroups($user['uid']);
$this->assertCount(count($user['groups']), $userGroups, 'Should retrieve all user groups');
foreach ($userGroups as $userGroup) {
$this->assertContains($userGroup, $user['groups'], sprintf('Users %s should be member of groups %s', $user['uid'], $userGroup));
}
}
}
public function testGroupExists() {
foreach(self::$groups as $group){
$result = self::$groupBackend->groupExists($group['saml_gid']);
$this->assertSame($group['saml_gid_exists'], $result, sprintf('Group %s should exist', $group['saml_gid']));
}
}
public function testGroupExists() {
foreach (self::$groups as $group) {
$result = self::$groupBackend->groupExists($group['saml_gid']);
$this->assertSame($group['saml_gid_exists'], $result, sprintf('Group %s should exist', $group['saml_gid']));
}
}
public function testUsersInGroups() {
foreach(self::$groups as $group){
$users = self::$groupBackend->usersInGroup($group['gid']);
$this->assertCount(count($group['members']), $users, 'Should retrieve all group members');
foreach($users as $user){
$this->assertContains($user, $group['members'], sprintf('User %s should be member of group %s', $user, $group['gid']));
}
}
}
}
public function testUsersInGroups() {
foreach (self::$groups as $group) {
$users = self::$groupBackend->usersInGroup($group['gid']);
$this->assertCount(count($group['members']), $users, 'Should retrieve all group members');
foreach ($users as $user) {
$this->assertContains($user, $group['members'], sprintf('User %s should be member of group %s', $user, $group['gid']));
}
}
}
}

View file

@ -1,7 +1,7 @@
<?php
/**
* @copyright Copyright (c) 2021 Giuliano Mele <giuliano.mele@verdigado.com>
*
*
* @author Giuliano Mele <giuliano.mele@verdigado.com>
*
* @license GNU AGPL version 3 or any later version
@ -276,5 +276,4 @@ class GroupManagerTest extends TestCase {
$this->ownGroupManager->addGroups($user, ['groupC']);
}
}

View file

@ -27,7 +27,6 @@ use OCA\User_SAML\UserData;
use OCP\EventDispatcher\IEventDispatcher;
use OCP\IConfig;
use OCP\IDBConnection;
use OCP\IGroup;
use OCA\User_SAML\GroupManager;
use OCP\ILogger;
use OCP\ISession;